550 cache busting #9447

Harriethw · 2024-09-18T11:08:09Z

When a content block is updated we do not want to wait the default cache time of 5 mins in order to see the changes in Documents that use this content block.

Publish intents allow us to reset the cache time for a Document
https://docs.publishing.service.gov.uk/repos/content-store/publish_intents.html

This PR creates a publish intent for any host Documents whenever a block is published.
I've put it at the point of publishing, rather than scheduling or updating, because we can instantly override the cache (or almost instantly depending on length of Sidekiq queue) whereas if we were to call it at point of scheduling, or only when we are updating, it gets a bit trickier to ensure that the method definitely gets called (we would also then have to cancel the intent if a scheduled Block was cancelled)

questions

I've put the PUT requests onto a Worker to be performed asynchronously as Whitehall do. I haven't stress tested this and would need help to do so if we wanted to do so- I'm not sure how slow it would be if a Block had 100-1000s of host Documents, but if it was slower then 5 mins the cache would be refreshed anyway.

To see locally

follow instructions to be able to see published changes locally https://github.com/alphagov/govuk-docker/blob/ddd2f32114416dd543892436df1c144e2bb44b03/docs/how-tos/publishing-content-on-whitehall.md
Create a published Content Block in Whitehall. Get the embed code from the details view e.g. {{embed:content_block_email_address:8603b86f-281d-43ba-b047-9de6d471db05}}
embed that code into a Document (.e.g News Article) and force publish it.
Create log/breakpoint in the content-store repo to see the cache time set for a content item
Go to your published Document on the govuk-frontend, e.g. http://government-frontend.dev.gov.uk/government/news/w-cache and observe the log in content-store should be 300
Update/edit the original published Content Block and publish changes
Observe in Publishing API logs that a PUT request has been made to publish-intent
Visit the published Document in the frontend and observe cache log should now be the difference between the time you re-published the Content Block and now... after 5 minutes the publishing intent will be ignored and cache time set back to 300.

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

Harriethw · 2024-09-19T08:41:31Z

db/seeds.rb

@@ -7,7 +7,7 @@
  gds_organisation_id = "af07d5a5-df63-4ddc-9383-6a666845ebe9"
  User.create!(
    name: "Test user",
-    permissions: ["signin", "GDS Admin", "GDS Editor", "Managing Editor", "Export data"],
+    permissions: ["signin", "GDS Admin", "GDS Editor", "Managing Editor", "Export data", "Sidekiq Admin"],


should we just add this for the Test User for good? I feel like it will continue to be useful as we continue to use Sidekiq...

Yeah, I can't see the harm 👍

Harriethw · 2024-09-19T08:59:55Z

lib/engines/content_object_store/app/lib/content_object_store/publishable.rb

@@ -51,6 +52,15 @@ def update_content_block_document(new_content_block_edition:, update_document_pa

  private

+    def remove_cache_for_dependent_content(content_block_edition:)


just realised I'm switching between dependent and host content... i've seen both used elsewhere, shall we stick with Host in the code as that's what we've got already?

I think let's stick with one or the other. I prefer dependent, but as host is used in a lot of places, let's use that.

cool, have changed to use host

pezholio

LGTM - just one thing about the tests

pezholio · 2024-09-19T12:27:06Z

lib/engines/content_object_store/app/lib/content_object_store/publishable.rb

@@ -51,6 +52,15 @@ def update_content_block_document(new_content_block_edition:, update_document_pa

  private

+    def remove_cache_for_dependent_content(content_block_edition:)


I think let's stick with one or the other. I prefer dependent, but as host is used in a lot of places, let's use that.

pezholio · 2024-09-19T12:29:55Z

lib/engines/content_object_store/test/unit/app/workers/publish_intent_worker_test.rb

+    timestamp = Time.zone.now.to_s
+    publish_intent = { foo: "bar" }
+
+    PublishingApi::PublishIntentPresenter.expects(:new).once.returns(publish_intent)


Should we check the arguments the presenter receives too?

good spot, have done

We will shortly use this to add publishing intents for Documents that have embedded content in them, when the embedded content is updated and we do not want to wait the default 5 mins for the cache to update.

When a content block is updated we do not want to wait the default cache time of 5 mins in order to see the changes in Documents that use this content block. Publish intents allow us to reset the cache time https://docs.publishing.service.gov.uk/repos/content-store/publish_intents.html

This returns the publishing app name for all host editions (Editions with Embedded content in them) In this previous Whitehal PR [1] we send publish intents for host Documents of embedded content, but we hardcoded the publishing app to Whitehall - we need to see the correct publishing app for all host editions in order to send these intents. [1] alphagov/whitehall#9447

we want to send Publish Intents for Host Content (e.g. Editions that are using embedded content/Content Blocks) [1] that is published by other apps - this allows an optional app to be passed in, but retains the default Whitehall value in order not to break existing functionality. [1] #9447

Harriethw force-pushed the 550-cache-busting branch 4 times, most recently from e13cf95 to fe78e48 Compare September 18, 2024 17:06

Harriethw commented Sep 19, 2024

View reviewed changes

Harriethw marked this pull request as ready for review September 19, 2024 08:58

Harriethw commented Sep 19, 2024

View reviewed changes

pezholio approved these changes Sep 19, 2024

View reviewed changes

Harriethw added 3 commits September 19, 2024 14:25

add Sidekiq Admin permissions to Test User

774fb8b

move workers unit tests to correct app folder

f05cf16

add a Publish Intent Worker

2c8cc52

We will shortly use this to add publishing intents for Documents that have embedded content in them, when the embedded content is updated and we do not want to wait the default 5 mins for the cache to update.

Harriethw force-pushed the 550-cache-busting branch from fe78e48 to 50733e1 Compare September 19, 2024 13:49

Harriethw force-pushed the 550-cache-busting branch from 50733e1 to c5b9360 Compare September 19, 2024 13:53

Harriethw enabled auto-merge September 19, 2024 13:55

Harriethw merged commit 084c94b into main Sep 19, 2024
19 checks passed

Harriethw deleted the 550-cache-busting branch September 19, 2024 13:58

Harriethw mentioned this pull request Sep 20, 2024

Content-modelling/550 Return publishing app for host Editions alphagov/publishing-api#2876

Merged

Harriethw mentioned this pull request Sep 20, 2024

Content modelling/550 fix cache busting #9459

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

550 cache busting #9447

550 cache busting #9447

Harriethw commented Sep 18, 2024 •

edited

Loading

Harriethw Sep 19, 2024

pezholio Sep 19, 2024

Harriethw Sep 19, 2024

pezholio Sep 19, 2024

Harriethw Sep 19, 2024

pezholio left a comment

pezholio Sep 19, 2024

pezholio Sep 19, 2024

Harriethw Sep 19, 2024

		@@ -51,6 +52,15 @@ def update_content_block_document(new_content_block_edition:, update_document_pa

		private

		def remove_cache_for_dependent_content(content_block_edition:)

550 cache busting #9447

550 cache busting #9447

Conversation

Harriethw commented Sep 18, 2024 • edited Loading

questions

To see locally

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pezholio left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Harriethw commented Sep 18, 2024 •

edited

Loading