Render site alerts as part of the page content

I would like to add to this to mention that on my Drupal 9 site, this module accounts for between 20%-50% of all the resources uses by a page load depending on the cache status of the page when loaded. While it's cool that it's fancy for folks who need that up to the second detail, we really just use it to display a note at the top of the page. So, spending 50% of our used server resources displaying a note at the top of the screen seems... like something we want to improve.

Regarding the cache lifetime, that has been made configurable in ✨ sitewide_alert cache max age value is hard coded Fixed , but I agree that this module shouldn't be making so many requests, and ideally would include everything it needs in one page load.

I like this idea. I am wondering if we can implement it in a way that allows the alert rendering method to be configurable. For some projects I have worked on, the advantage of this module over other similar modules or a solution using normal blocks/views is that this module was designed to avoid breaking fully cached pages, including those cached downstream (varnish, reverse proxy), at a time when a new alert is published site wide. One of the main goals of loading the alerts as a separate request was to allow pages to be fully cached independently of active alerts. In addition, this module also supports showing alerts for those who have already loaded the page.

Example situation when this is important:

A university, school, or government agency needs to publish a very important time-sensitive alert sitewide at a time when traffic to the site is abnormally high. This may be an alert regarding public safety. With this change, adding a sitewide alert would cause the page cache of all the pages on the site to be invalidated (due to the cache tag), resulting in excessive stress on the site as it needs to rebuild all of the pages at a time when traffic is high and the information is important.

However, this situation/use does not apply to all sites, so it should be configurable to allow the site builder to choose which loading method fits their project.

The module would literally crash any website using it while their traffic was abnormally high. So that feature does not exist despite what the project description says.

Work in progress on a slightly different approach in https://git.drupalcode.org/project/sitewide_alert/-/merge_requests/41

The change is fairly minimal - just moving the alerts into drupalSettings

This probably needs further thought from a security and performance point of view.

Also, as per #5, ideally we would make this configurable.

Would absolutely love this with the further enhancement that if "Automatically Update (Refresh) Alerts" (polling) is disabled in /admin/config/sitewide_alerts and "Dismissible" is disabled for the alert(s), there is no reason to load the init.js (or any other) JavaScript— yet more optional performance enhancement and site admin flexibility for this excellent module.

Would it be possile to include this feature as a configuration option so that the existing functionality is preserved for those who need it and the new ability to avoid the extra request is avoided for others?

We tested it on a client project, worked well.

I added some suggestions and fixed the test error. Please review.

+1 for RTBC, been using in the wild for a few months now without issue.

+1 for this. works as expected

Create a patch from the MR

@qusai taha, you can just click on the plain diff and that's a patch already, no need for a separate patch

Re-roll the patch to make it working with latest version

Re-roll the patch to make it working with latest version

Re-roll the patch to make it working with latest version

Fixes should be in MRs not patches just fyi

MR for version 3 https://git.drupalcode.org/project/sitewide_alert/-/merge_requests/65

Thanks for the 3.x patch. That's working well for me, but I'm getting some errors thrown from code in SitewideAlertsController.php.

$sitewideAlerts = $this->sitewideAlertManager->activeVisibleSitewideAlerts();
was replaced by
$alerts = $this->sitewideAlertManager->activeVisibleSitewideAlerts();
but a few lines below the variable name adjustment was not reflected, so the first argument to krsort isn't an array.
krsort($sitewideAlerts, SORT_NUMERIC);

Still appears to be missing tests

mangesh.borukar → made their first commit to this issue’s fork.

I have manually tested the latest commit for MR!65. The patch works fine. We’ve also had this patch running on our production site for around 5 months without issues.

@smustgrave I'm new to contributing tests, but I am interested in helping to create them for this patch. Could you please clarify what types of tests would be helpful to advance this issue?