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

  1. Implement the changes
  2. Test it
  3. Merge and enjoy!

User interface changes

We've added an extra textfield to the module's cache admin page to change max-age.

Comments

SAVEL created an issue. See original summary.

savel’s picture

Here is the patch. Please, test.

savel’s picture

Status: Active » Needs review
nikolas.tatianenko’s picture

Status: Needs review » Reviewed & tested by the community

Reviewed. Looks Great!

savel’s picture

Version: 7.x-1.x-dev » 7.x-1.0
savel’s picture

Version: 7.x-1.0 » 7.x-1.x-dev
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new5.95 KB
new2.78 KB

Improved anonymous caching handling by moving hash from Drupal.settings (which may be cached with the page) to the ajax callback.

wranvaud’s picture

Version: 7.x-1.x-dev » 8.x-1.x-dev
Status: Needs review » Patch (to be ported)

Great addition! Thank you for catching this!
Will leave this open to port it to drupal 8 as well.

  • SAVEL authored 65230c9 on 7.x-1.x
    Issue #3106317 by SAVEL, nikolas.tatianenko: Show message again if it...
wranvaud’s picture

Status: Patch (to be ported) » Fixed

This is now ported to D8/9+ as well. Will be on next beta5 release.

  • wranvaud committed ea7687a on 8.x-1.x
    Issue #3106317 by wranvaud, SAVEL: Show message again if it was changed
    

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.