localgovdrupal / localgov_services

Provides the pages and navigation for presenting the Services provided by Local Government. A part of the LocalGovDrupal distribution.
GNU General Public License v2.0
3 stars 4 forks source link

Deleted service status updates only disappear from landing page after a cache rebuild #229

Closed dedavidson closed 4 months ago

dedavidson commented 11 months ago

I haven't been able to fully test this yet but I believe that modules/localgov_services_status/localgov_services_status.module should include an implementation of hook_ENTITY_TYPE_predelete() so that the landing page cache is invalidated when a status is deleted in same way as it is in the implementation of hook_ENTITY_TYPE_presave() when a status page is saved.

ekes commented 11 months ago

I've not looked at the logic of this very much, but am I correct that what you mean by the 'landing page' not being updated is the block on the 'service landingpage'?

If so looking at it https://github.com/localgovdrupal/localgov_services/blob/00a9ac79d83e5251eafc3719c09b6f5c81c8985b/modules/localgov_services_status/src/Plugin/Block/ServiceStatusMessage.php#L95 I'm wondering if all of the nodes iterated over there (they're the status' right?) should be included in the cache data so that any change to any of them invalidates the cache? @stephen-cox does that sound right?

Or if we don't mind all of the status blocks getting invalidated when new status' are posted node_list:localgov_service_status cache tag might even make that presave unnecessary?

stephen-cox commented 10 months ago

So node_list:localgov_services_status cache tag is added to the block cache tags, so in theory any change to service status nodes should invalidate the cache for the block, but it's not working as expected. Adding all the individual cache tags seems to fix it though.

rupertj commented 9 months ago

I've been testing the linked PR, and I can't recreate the original issue. (Although I have found a sort of related one...)

With either the 2.1.5 release, or the code in the PR, the behaviour is the same for me: 1) Create a service landing page and view it. 2) Create a service status, set to appear on the service landing page. 3) View the service landing page again: status does not appear. 4) Clear cache: status does appear 5) Delete status node. 6) View the service page again: status does not appear. No cache clear is required this time.

So for me, removing the status works with or without the code in the PR. Adding a new status does not.

rupertj commented 8 months ago

And just to prove that this isn't my install being weird, I've added just the test change from PR #234, and not the code change to a separate PR: #239 . The Unit tests still pass.

rupertj commented 8 months ago

@dedavidson Just to check, is your issue with the block, or the deprecated updates panel? See #129 for a screenshot of the panel.

dedavidson commented 8 months ago

@rupertj it was the panel.

stephen-cox commented 8 months ago

@rupertj This error can be reproduced on the https://test.localgovdrupal.org/ site.

I have deleted the 'Adult social care service is working normally' service status, but it still appears on this page: https://test.localgovdrupal.org/adult-health-and-social-care

image

If you're testing locally make sure all the caches are enabled; they're disabled in Lando by default.