Problem/Motivation
In Drupal\sitewide_alert\Controller\SitewideAlertsController::load(), ~ line 104, we see the following code:
// Set response cache so it's invalidated whenever alerts get updated, or
// settings are changed.
$cacheableMetadata = (new CacheableMetadata())
->setCacheMaxAge(30)
->addCacheContexts(['languages'])
->setCacheTags(['sitewide_alert_list']);
$cacheableMetadata->addCacheableDependency($sitewideAlertSettings);
I'm not sure why we set the cache max age to 30 seconds here. I don't think we should be, especially given the comment above noting that we are just setting making sure alerts are only cached until an alert is created/updated or the module's settings are altered.
I only noticed this because I installed the Cache Control Override module and, for a reason TBD, with that module enabled, the load route always returns a max age of 30. This is interesting behavior. But I noticed that after removing this line setting max age to 30s, the cache max age of the load response is the expected, module configured value, whether I have Cache Control Override installed or not.
Steps to reproduce
Proposed resolution
Don't set the Cache Max Age to 30 here. I do not believe there is a reason to do so.
Remaining tasks
User interface changes
API changes
Data model changes
Issue fork sitewide_alert-3476702
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3476702-no-max-age-30
changes, plain diff MR !52
Comments
Comment #3
sonfdComment #6
smustgrave commentedAgree could not find a reason to keep and didn't break any existing tests or functionality. Think a good candidate for 3.0.x initial release
Comment #7
smustgrave commented