Problem/Motivation
There's no way to show a message again, if conditions are set to "until manually closed" or "once per session" even after the message was changed.
Proposed resolution
We need to keep track of whether the message was changed or not. If the message was altered, ignore conditionalMessageReadStatus
and conditionalMessageUserClosed from the browser's local storage and show the message again.
The solution would be to generate a hash from the message text each time the conditional message's settings are changed. Store that hash in the variable called "conditional_message_hash."
When a user views (or closes) the comment, we are not just marking it with TRUE in the local storage, but we save "conditional_message_hash" for the current message. So when we need to check if the user has already seen/closed the message, we compare the hash value stored at his browser with the actual hash.
To allow users to see the changed in time, we also need to control a browser's cache by setting the max-age parameter of the cache-control meta. Currently, it's hardcoded to 3600secs, but we should allow the administrator to change it using module's settings.
Remaining tasks
Implement the changes- Test it
- Merge and enjoy!
User interface changes
We've added an extra textfield to the module's cache admin page to change max-age.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | interdiff_1-6.txt | 2.78 KB | savel |
| #6 | conditional_message-show-message-again-if-it-was-changed-3106317-6.patch | 5.95 KB | savel |
| #2 | conditional_message-show-message-again-if-it-was-changed-3106317-1.patch | 6.15 KB | savel |
Comments
Comment #2
savel commentedHere is the patch. Please, test.
Comment #3
savel commentedComment #4
nikolas.tatianenko commentedReviewed. Looks Great!
Comment #5
savel commentedComment #6
savel commentedImproved anonymous caching handling by moving hash from Drupal.settings (which may be cached with the page) to the ajax callback.
Comment #7
wranvaud commentedGreat addition! Thank you for catching this!
Will leave this open to port it to drupal 8 as well.
Comment #9
wranvaud commentedThis is now ported to D8/9+ as well. Will be on next beta5 release.