From b3f09018c6bdf4ebbb835dce8c573c191d05f4fe Mon Sep 17 00:00:00 2001 From: Bruce Bolt Date: Mon, 18 Mar 2024 12:02:13 +0000 Subject: [PATCH 1/8] Do not republish office associated with editionable worldwide org When an office is associated with an editionable worldwide organisation, we do not want it to be published immediately to Publishing API, as the office is also part of the editionable workflow. --- app/models/contact.rb | 2 +- app/models/worldwide_office.rb | 6 ++++++ test/unit/app/models/worldwide_office_test.rb | 20 +++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/app/models/contact.rb b/app/models/contact.rb index 8812bc72388..0321fb62e0d 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -40,7 +40,7 @@ def republish_organisation_to_publishing_api end def republish_worldwide_office_to_publishing_api - Whitehall::PublishingApi.republish_async(contactable) if contactable.is_a?(WorldwideOffice) + Whitehall::PublishingApi.republish_async(contactable) if contactable.is_a?(WorldwideOffice) && !contactable.edition end def contactable_name diff --git a/app/models/worldwide_office.rb b/app/models/worldwide_office.rb index acaf4d5d813..97c9a35cb33 100644 --- a/app/models/worldwide_office.rb +++ b/app/models/worldwide_office.rb @@ -29,6 +29,12 @@ class WorldwideOffice < ApplicationRecord delegate(:non_english_translated_locales, to: :worldwide_organisation) delegate(:embassy_office?, to: :worldwide_office_type) + def can_publish_to_publishing_api? + return false if edition + + super + end + def worldwide_organisation super || edition end diff --git a/test/unit/app/models/worldwide_office_test.rb b/test/unit/app/models/worldwide_office_test.rb index 263c690b07a..39d32bfbb7e 100644 --- a/test/unit/app/models/worldwide_office_test.rb +++ b/test/unit/app/models/worldwide_office_test.rb @@ -118,4 +118,24 @@ class WorldwideOfficeTest < ActiveSupport::TestCase office.destroy! end end + + test "is published to Publishing API on update if associated with a non-editionable worldwide organisation" do + office = create(:worldwide_office) + + Whitehall::PublishingApi.expects(:republish_async).with(office) + + Sidekiq::Testing.inline! do + office.contact.update!(title: "New title") + end + end + + test "is not published to Publishing API on update if associated with an editionable worldwide organisation" do + office = create(:worldwide_office, edition: create(:editionable_worldwide_organisation), worldwide_organisation: nil) + + Whitehall::PublishingApi.expects(:republish_async).with(office).never + + Sidekiq::Testing.inline! do + office.contact.update!(title: "New title") + end + end end From cfe98256a009b29f34ff63133165eac14516dc8f Mon Sep 17 00:00:00 2001 From: Bruce Bolt Date: Wed, 20 Mar 2024 13:49:01 +0000 Subject: [PATCH 2/8] Do not republish contacts associated with editionable worldwide org When a contact is associated with an editionable worldwide organisation (through a worldwide office), we do not want it to be published immediately to Publishing API, as the office is also part of the editionable workflow. --- app/models/contact.rb | 6 +++++ test/unit/app/models/contact_test.rb | 33 ++++++++++++++++++++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/app/models/contact.rb b/app/models/contact.rb index 0321fb62e0d..6721b41a1d8 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -35,6 +35,12 @@ class Contact < ApplicationRecord extend HomePageList::ContentItem is_stored_on_home_page_lists + def can_publish_to_publishing_api? + return false if contactable.is_a?(WorldwideOffice) && contactable.edition + + super + end + def republish_organisation_to_publishing_api Whitehall::PublishingApi.republish_async(contactable) if contactable.is_a?(Organisation) end diff --git a/test/unit/app/models/contact_test.rb b/test/unit/app/models/contact_test.rb index c8151cf3db2..e328eb982fd 100644 --- a/test/unit/app/models/contact_test.rb +++ b/test/unit/app/models/contact_test.rb @@ -223,7 +223,36 @@ class ContactTest < ActiveSupport::TestCase assert_nil Contact.find_by(id: contact.id) end - test "publishes to the publishing api" do - assert Contact.included_modules.include? PublishesToPublishingApi + test "is published to Publishing API on update when associated with an organisation" do + contact = create(:contact, contactable: create(:organisation)) + + Whitehall::PublishingApi.expects(:patch_links).with(contact) + Whitehall::PublishingApi.expects(:publish).with(contact) + + Sidekiq::Testing.inline! do + contact.update!(title: "New title") + end + end + + test "is published to Publishing API on update when associated with a worldwide office that has a non-editionable worldwide organisation" do + office = create(:worldwide_office) + + Whitehall::PublishingApi.expects(:patch_links).with(office.contact) + Whitehall::PublishingApi.expects(:publish).with(office.contact) + + Sidekiq::Testing.inline! do + office.contact.update!(title: "New title") + end + end + + test "is not published to Publishing API on update when associated with a worldwide office that has an editionable worldwide organisation" do + office = create(:worldwide_office, edition: create(:editionable_worldwide_organisation), worldwide_organisation: nil) + + Whitehall::PublishingApi.expects(:patch_links).with(office.contact).never + Whitehall::PublishingApi.expects(:publish).with(office.contact).never + + Sidekiq::Testing.inline! do + office.contact.update!(title: "New title") + end end end From 7d2a51e3e88363717a52929d0b35c4baf6edd9c9 Mon Sep 17 00:00:00 2001 From: Bruce Bolt Date: Wed, 20 Mar 2024 16:24:56 +0000 Subject: [PATCH 3/8] Do not unpublish worldwide offices when destroyed When an office (and it's contact) is associated with an editionable worldwide organisation, we do not want it to be unpublished from Publishing API, as the office is also part of the editionable workflow so we want the draft to be removed instead (handled in the controller). --- app/models/contact.rb | 6 ++++ app/models/worldwide_office.rb | 6 ++++ lib/publishes_to_publishing_api.rb | 6 +++- test/unit/app/models/contact_test.rb | 30 +++++++++++++++++++ .../publishes_to_publishing_api_test.rb | 2 ++ test/unit/app/models/worldwide_office_test.rb | 22 ++++++++++++++ 6 files changed, 71 insertions(+), 1 deletion(-) diff --git a/app/models/contact.rb b/app/models/contact.rb index 6721b41a1d8..46d1eb3f14a 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -41,6 +41,12 @@ def can_publish_to_publishing_api? super end + def can_publish_gone_to_publishing_api? + return false if contactable.is_a?(WorldwideOffice) && contactable.edition + + super + end + def republish_organisation_to_publishing_api Whitehall::PublishingApi.republish_async(contactable) if contactable.is_a?(Organisation) end diff --git a/app/models/worldwide_office.rb b/app/models/worldwide_office.rb index 97c9a35cb33..1405323a6d5 100644 --- a/app/models/worldwide_office.rb +++ b/app/models/worldwide_office.rb @@ -35,6 +35,12 @@ def can_publish_to_publishing_api? super end + def can_publish_gone_to_publishing_api? + return false if edition + + super + end + def worldwide_organisation super || edition end diff --git a/lib/publishes_to_publishing_api.rb b/lib/publishes_to_publishing_api.rb index 2aeec845fd3..08ce8874654 100644 --- a/lib/publishes_to_publishing_api.rb +++ b/lib/publishes_to_publishing_api.rb @@ -4,7 +4,7 @@ module PublishesToPublishingApi included do after_commit :publish_to_publishing_api, if: :can_publish_to_publishing_api? - after_commit :publish_gone_to_publishing_api, on: :destroy + after_commit :publish_gone_to_publishing_api, on: :destroy, if: :can_publish_gone_to_publishing_api? define_callbacks :published, :published_gone end @@ -12,6 +12,10 @@ def can_publish_to_publishing_api? persisted? end + def can_publish_gone_to_publishing_api? + true + end + def republish_to_publishing_api_async if can_publish_to_publishing_api? Whitehall::PublishingApi.republish_async(self) diff --git a/test/unit/app/models/contact_test.rb b/test/unit/app/models/contact_test.rb index e328eb982fd..8d3542252b6 100644 --- a/test/unit/app/models/contact_test.rb +++ b/test/unit/app/models/contact_test.rb @@ -255,4 +255,34 @@ class ContactTest < ActiveSupport::TestCase office.contact.update!(title: "New title") end end + + test "is deleted from Publishing API on destroy when associated with a worldwide office that has an organisation" do + contact = create(:contact, contactable: create(:organisation)) + + Whitehall::PublishingApi.expects(:publish_gone_async).with(contact.content_id, nil, nil) + + Sidekiq::Testing.inline! do + contact.destroy! + end + end + + test "is deleted from Publishing API on destroy when associated with a worldwide office that has a non-editionable worldwide organisation" do + office = create(:worldwide_office) + + Whitehall::PublishingApi.expects(:publish_gone_async).with(office.contact.content_id, nil, nil) + + Sidekiq::Testing.inline! do + office.contact.destroy! + end + end + + test "is not deleted from Publishing API on destroy when associated with a worldwide office that has an editionable worldwide organisation" do + office = create(:worldwide_office, edition: create(:editionable_worldwide_organisation), worldwide_organisation: nil) + + Whitehall::PublishingApi.expects(:publish_gone_async).with(office.contact.content_id, nil, nil).never + + Sidekiq::Testing.inline! do + office.contact.destroy! + end + end end diff --git a/test/unit/app/models/publishes_to_publishing_api_test.rb b/test/unit/app/models/publishes_to_publishing_api_test.rb index 8bcf3c43cdd..04c53ab4a20 100644 --- a/test/unit/app/models/publishes_to_publishing_api_test.rb +++ b/test/unit/app/models/publishes_to_publishing_api_test.rb @@ -31,6 +31,7 @@ class << object TestObject.stubs(:after_commit).with( :publish_gone_to_publishing_api, on: :destroy, + if: :can_publish_gone_to_publishing_api?, ) end @@ -46,6 +47,7 @@ class << object TestObject.expects(:after_commit).with( :publish_gone_to_publishing_api, on: :destroy, + if: :can_publish_gone_to_publishing_api?, ) include_module(TestObject.new) end diff --git a/test/unit/app/models/worldwide_office_test.rb b/test/unit/app/models/worldwide_office_test.rb index 39d32bfbb7e..4ad6edc6336 100644 --- a/test/unit/app/models/worldwide_office_test.rb +++ b/test/unit/app/models/worldwide_office_test.rb @@ -138,4 +138,26 @@ class WorldwideOfficeTest < ActiveSupport::TestCase office.contact.update!(title: "New title") end end + + test "is deleted from Publishing API on destroy when associated with a non-editionable worldwide organisation" do + office = create(:worldwide_office) + + Whitehall::PublishingApi.expects(:publish_gone_async).with(office.content_id, nil, nil) + Whitehall::PublishingApi.expects(:publish_gone_async).with(office.contact.content_id, nil, nil) + + Sidekiq::Testing.inline! do + office.destroy! + end + end + + test "is not deleted from Publishing API on destroy when associated with an editionable worldwide organisation" do + office = create(:worldwide_office, edition: create(:editionable_worldwide_organisation), worldwide_organisation: nil) + + Whitehall::PublishingApi.expects(:publish_gone_async).with(office.content_id, nil, nil).never + Whitehall::PublishingApi.expects(:publish_gone_async).with(office.contact.content_id, nil, nil).never + + Sidekiq::Testing.inline! do + office.destroy! + end + end end From 425edc8f446541134a8d4b6d3c423849c8cdcaf0 Mon Sep 17 00:00:00 2001 From: Bruce Bolt Date: Tue, 19 Mar 2024 11:34:33 +0000 Subject: [PATCH 4/8] Make `PublishingApiHtmlAttachments` more generic This will allow us to publish other things (e.g. worldwide offices) using the same workflow as HTML attachments. By doing this, the associated documents can be kept in the correct state when the parent document updates. --- ...=> publishing_api_associated_documents.rb} | 58 +++++++++---------- .../publishing_api_pusher.rb | 6 +- ...ishing_api_document_republishing_worker.rb | 2 +- ...blishing_api_associated_documents_test.rb} | 18 +++--- .../publishing_api_pusher_test.rb | 26 ++++----- ...g_api_document_republishing_worker_test.rb | 2 +- 6 files changed, 56 insertions(+), 56 deletions(-) rename app/services/service_listeners/{publishing_api_html_attachments.rb => publishing_api_associated_documents.rb} (63%) rename test/unit/app/services/service_listeners/{publishing_api_html_attachments_test.rb => publishing_api_associated_documents_test.rb} (97%) diff --git a/app/services/service_listeners/publishing_api_html_attachments.rb b/app/services/service_listeners/publishing_api_associated_documents.rb similarity index 63% rename from app/services/service_listeners/publishing_api_html_attachments.rb rename to app/services/service_listeners/publishing_api_associated_documents.rb index 9db8cbc3671..a36e8493ed1 100644 --- a/app/services/service_listeners/publishing_api_html_attachments.rb +++ b/app/services/service_listeners/publishing_api_associated_documents.rb @@ -1,5 +1,5 @@ module ServiceListeners - class PublishingApiHtmlAttachments + class PublishingApiAssociatedDocuments attr_reader :edition def self.process(edition, event) @@ -22,7 +22,7 @@ def republish update_draft(update_type: "republish") elsif edition.unpublishing && edition.withdrawn? do_publish("republish") - discard_drafts(deleted_html_attachments) + discard_drafts(deleted_associated_documents) withdraw elsif edition.unpublishing update_draft(update_type: "republish") @@ -30,19 +30,19 @@ def republish unpublish(allow_draft: true) else do_publish("republish") - discard_drafts(deleted_html_attachments) + discard_drafts(deleted_associated_documents) end end def update_draft(update_type: nil) - current_html_attachments.each do |html_attachment| + current_associated_documents.each do |associated_document| Whitehall::PublishingApi.save_draft_translation( - html_attachment, - html_attachment.locale || I18n.default_locale.to_s, + associated_document, + associated_document.locale || I18n.default_locale.to_s, update_type || (edition.minor_change? ? "minor" : "major"), ) end - discard_drafts(deleted_html_attachments) + discard_drafts(deleted_associated_documents) end # We don't care whether this is a translation or the main # document, we just send the correct html attachments regardless. @@ -55,22 +55,22 @@ def unpublish(allow_draft: false) edition.public_path end - current_html_attachments.each do |html_attachment| + current_associated_documents.each do |associated_document| PublishingApiRedirectWorker.new.perform( - html_attachment.content_id, + associated_document.content_id, destination, - html_attachment.locale || I18n.default_locale.to_s, + associated_document.locale || I18n.default_locale.to_s, allow_draft, ) end end def withdraw - current_html_attachments.each do |html_attachment| + current_associated_documents.each do |associated_document| PublishingApiWithdrawalWorker.new.perform( - html_attachment.content_id, + associated_document.content_id, edition.unpublishing.explanation, - html_attachment.locale || I18n.default_locale.to_s, + associated_document.locale || I18n.default_locale.to_s, false, edition.unpublishing.unpublished_at.to_s, ) @@ -78,23 +78,23 @@ def withdraw end def delete - discard_drafts(current_html_attachments + deleted_html_attachments) + discard_drafts(current_associated_documents + deleted_associated_documents) end private - def discard_drafts(html_attachments) - html_attachments.each do |html_attachment| + def discard_drafts(associated_documents) + associated_documents.each do |associated_document| PublishingApiDiscardDraftWorker.perform_async( - html_attachment.content_id, + associated_document.content_id, edition.primary_locale, ) end end def patch_links - current_html_attachments.each do |html_attachment| - Whitehall::PublishingApi.patch_links(html_attachment, bulk_publishing: false) + current_associated_documents.each do |associated_document| + Whitehall::PublishingApi.patch_links(associated_document, bulk_publishing: false) end end @@ -102,11 +102,11 @@ def previous_edition @previous_edition ||= edition.previous_edition end - def current_html_attachments + def current_associated_documents edition.attachables.flat_map(&:html_attachments) end - def previous_html_attachments + def previous_associated_documents return [] unless previous_edition previous_edition.attachables.flat_map(&:html_attachments) @@ -115,14 +115,14 @@ def previous_html_attachments def content_ids_to_remove return Set[] unless previous_edition - deleted_content_ids = deleted_html_attachments.map(&:content_id).to_set - old_content_ids = previous_html_attachments.map(&:content_id).to_set - new_content_ids = current_html_attachments.map(&:content_id).to_set + deleted_content_ids = deleted_associated_documents.map(&:content_id).to_set + old_content_ids = previous_associated_documents.map(&:content_id).to_set + new_content_ids = current_associated_documents.map(&:content_id).to_set deleted_content_ids + old_content_ids - new_content_ids end - def deleted_html_attachments + def deleted_associated_documents edition.attachables.flat_map(&:deleted_html_attachments) end @@ -135,12 +135,12 @@ def do_publish(update_type) ) end - current_html_attachments.each do |html_attachment| + current_associated_documents.each do |associated_document| PublishingApiWorker.new.perform( - html_attachment.class.name, - html_attachment.id, + associated_document.class.name, + associated_document.id, update_type, - html_attachment.locale || I18n.default_locale.to_s, + associated_document.locale || I18n.default_locale.to_s, ) end end diff --git a/app/services/service_listeners/publishing_api_pusher.rb b/app/services/service_listeners/publishing_api_pusher.rb index b84c626407b..cad55e1f301 100644 --- a/app/services/service_listeners/publishing_api_pusher.rb +++ b/app/services/service_listeners/publishing_api_pusher.rb @@ -40,14 +40,14 @@ def push(event:, options: {}) api.discard_draft_async(edition) end - handle_html_attachments(event) + handle_associated_documents(event) end private - def handle_html_attachments(event) + def handle_associated_documents(event) if edition.respond_to?(:html_attachments) - PublishingApiHtmlAttachments.process(edition, event) + PublishingApiAssociatedDocuments.process(edition, event) end end diff --git a/app/workers/publishing_api_document_republishing_worker.rb b/app/workers/publishing_api_document_republishing_worker.rb index 7c8b30ca5a1..58e4d735522 100644 --- a/app/workers/publishing_api_document_republishing_worker.rb +++ b/app/workers/publishing_api_document_republishing_worker.rb @@ -120,7 +120,7 @@ def locales_for(edition) end def handle_attachments_for(edition) - ServiceListeners::PublishingApiHtmlAttachments.process( + ServiceListeners::PublishingApiAssociatedDocuments.process( edition, "republish", ) diff --git a/test/unit/app/services/service_listeners/publishing_api_html_attachments_test.rb b/test/unit/app/services/service_listeners/publishing_api_associated_documents_test.rb similarity index 97% rename from test/unit/app/services/service_listeners/publishing_api_html_attachments_test.rb rename to test/unit/app/services/service_listeners/publishing_api_associated_documents_test.rb index eecccdf84db..3bf30a85504 100644 --- a/test/unit/app/services/service_listeners/publishing_api_html_attachments_test.rb +++ b/test/unit/app/services/service_listeners/publishing_api_associated_documents_test.rb @@ -1,13 +1,13 @@ require "test_helper" module ServiceListeners - class PublishingApiHtmlAttachmentsTest < ActiveSupport::TestCase + class PublishingApiAssociatedDocumentsTest < ActiveSupport::TestCase def call(edition) event = self.class.name.demodulize.underscore - PublishingApiHtmlAttachments.process(edition, event) + PublishingApiAssociatedDocuments.process(edition, event) end - class Publish < PublishingApiHtmlAttachmentsTest + class Publish < PublishingApiAssociatedDocumentsTest test "for something that can't have html attachments doesn't publish" do call(create(:published_news_article)) end @@ -204,7 +204,7 @@ class Publish < PublishingApiHtmlAttachmentsTest end end - class UpdateDraft < PublishingApiHtmlAttachmentsTest + class UpdateDraft < PublishingApiAssociatedDocumentsTest test "for something that can't have html attachments doesn't save draft" do call(create(:published_news_article)) end @@ -280,7 +280,7 @@ class UpdateDraft < PublishingApiHtmlAttachmentsTest end end - class Unpublish < PublishingApiHtmlAttachmentsTest + class Unpublish < PublishingApiAssociatedDocumentsTest test "for something that can't have html attachments doesn't publish a redirect" do edition = create(:unpublished_edition) assert_equal edition.attachments.count, 0 @@ -336,7 +336,7 @@ class Unpublish < PublishingApiHtmlAttachmentsTest call(publication) end - class Withdraw < PublishingApiHtmlAttachmentsTest + class Withdraw < PublishingApiAssociatedDocumentsTest test "for something that can't have html attachments doesn't publish a withdrawal" do call(create(:published_news_article)) end @@ -378,7 +378,7 @@ class Withdraw < PublishingApiHtmlAttachmentsTest end end - class Delete < PublishingApiHtmlAttachmentsTest + class Delete < PublishingApiAssociatedDocumentsTest test "for something that can't have html attachments doesn't discard any drafts" do call(create(:published_news_article)) end @@ -412,7 +412,7 @@ class Delete < PublishingApiHtmlAttachmentsTest end end - class Republish < PublishingApiHtmlAttachmentsTest + class Republish < PublishingApiAssociatedDocumentsTest test "for a draft publication with an attachment saves the draft" do publication = create(:draft_publication) attachment = publication.html_attachments.first @@ -511,7 +511,7 @@ class Republish < PublishingApiHtmlAttachmentsTest end end - class Unwithdraw < PublishingApiHtmlAttachmentsTest + class Unwithdraw < PublishingApiAssociatedDocumentsTest test "with an html attachment on a new document publishes the attachment" do publication = create(:published_publication) attachment = publication.html_attachments.first diff --git a/test/unit/app/services/service_listeners/publishing_api_pusher_test.rb b/test/unit/app/services/service_listeners/publishing_api_pusher_test.rb index 0284098782b..50bc48a16d0 100644 --- a/test/unit/app/services/service_listeners/publishing_api_pusher_test.rb +++ b/test/unit/app/services/service_listeners/publishing_api_pusher_test.rb @@ -2,8 +2,8 @@ module ServiceListeners class PublishingApiPusherTest < ActiveSupport::TestCase - def stub_html_attachment_pusher(edition, event) - PublishingApiHtmlAttachments + def stub_associated_document_pusher(edition, event) + PublishingApiAssociatedDocuments .expects(:process) .with(edition, event) end @@ -11,7 +11,7 @@ def stub_html_attachment_pusher(edition, event) test "saves draft async for update_draft" do edition = build(:draft_publication, document: build(:document)) Whitehall::PublishingApi.expects(:save_draft).with(edition) - stub_html_attachment_pusher(edition, "update_draft") + stub_associated_document_pusher(edition, "update_draft") Sidekiq::Testing.inline! do PublishingApiPusher.new(edition).push(event: "update_draft") end @@ -24,7 +24,7 @@ def stub_html_attachment_pusher(edition, event) document: build(:document), ) Whitehall::PublishingApi.expects(:save_draft).with(edition) - stub_html_attachment_pusher(edition, "update_draft") + stub_associated_document_pusher(edition, "update_draft") Sidekiq::Testing.inline! do PublishingApiPusher.new(edition).push(event: "update_draft") end @@ -33,7 +33,7 @@ def stub_html_attachment_pusher(edition, event) test "publish publishes" do edition = build(:publication, document: build(:document)) Whitehall::PublishingApi.expects(:publish).with(edition) - stub_html_attachment_pusher(edition, "publish") + stub_associated_document_pusher(edition, "publish") Sidekiq::Testing.inline! do PublishingApiPusher.new(edition).push(event: "publish") end @@ -42,7 +42,7 @@ def stub_html_attachment_pusher(edition, event) test "force_publish publishes" do edition = build(:publication, document: build(:document)) Whitehall::PublishingApi.expects(:publish).with(edition) - stub_html_attachment_pusher(edition, "force_publish") + stub_associated_document_pusher(edition, "force_publish") Sidekiq::Testing.inline! do PublishingApiPusher.new(edition).push(event: "force_publish") end @@ -51,7 +51,7 @@ def stub_html_attachment_pusher(edition, event) test "update_draft_translation saves draft translation" do edition = build(:publication, document: build(:document)) Whitehall::PublishingApi.expects(:save_draft_translation).with(edition, "en") - stub_html_attachment_pusher(edition, "update_draft_translation") + stub_associated_document_pusher(edition, "update_draft_translation") Sidekiq::Testing.inline! do PublishingApiPusher.new(edition).push(event: "update_draft_translation", options: { locale: "en" }) end @@ -74,7 +74,7 @@ def stub_html_attachment_pusher(edition, event) .with(edition.document.content_id, edition.unpublishing.explanation, edition.unpublishing.unpublished_at, translation.to_s) end - stub_html_attachment_pusher(edition, "withdraw") + stub_associated_document_pusher(edition, "withdraw") Sidekiq::Testing.inline! do PublishingApiPusher.new(edition).push(event: "withdraw") @@ -84,7 +84,7 @@ def stub_html_attachment_pusher(edition, event) test "unpublish publishes the unpublishing" do edition = create(:unpublished_publication) Whitehall::PublishingApi.expects(:unpublish_async).with(edition.unpublishing) - stub_html_attachment_pusher(edition, "unpublish") + stub_associated_document_pusher(edition, "unpublish") Sidekiq::Testing.inline! do PublishingApiPusher.new(edition).push(event: "unpublish") end @@ -93,7 +93,7 @@ def stub_html_attachment_pusher(edition, event) test "force_schedule schedules the edition" do edition = build(:publication, document: build(:document)) Whitehall::PublishingApi.expects(:schedule_async).with(edition) - stub_html_attachment_pusher(edition, "force_schedule") + stub_associated_document_pusher(edition, "force_schedule") Sidekiq::Testing.inline! do PublishingApiPusher.new(edition).push(event: "force_schedule") end @@ -102,7 +102,7 @@ def stub_html_attachment_pusher(edition, event) test "schedule schedules the edition" do edition = build(:publication, document: build(:document)) Whitehall::PublishingApi.expects(:schedule_async).with(edition) - stub_html_attachment_pusher(edition, "schedule") + stub_associated_document_pusher(edition, "schedule") Sidekiq::Testing.inline! do PublishingApiPusher.new(edition).push(event: "schedule") end @@ -111,7 +111,7 @@ def stub_html_attachment_pusher(edition, event) test "unschedule unschedules the edition" do edition = build(:publication, document: build(:document)) Whitehall::PublishingApi.expects(:unschedule_async).with(edition) - stub_html_attachment_pusher(edition, "unschedule") + stub_associated_document_pusher(edition, "unschedule") Sidekiq::Testing.inline! do PublishingApiPusher.new(edition).push(event: "unschedule") end @@ -120,7 +120,7 @@ def stub_html_attachment_pusher(edition, event) test "delete discards draft" do edition = build(:publication, document: build(:document)) Whitehall::PublishingApi.expects(:discard_draft_async).with(edition) - stub_html_attachment_pusher(edition, "delete") + stub_associated_document_pusher(edition, "delete") Sidekiq::Testing.inline! do PublishingApiPusher.new(edition).push(event: "delete") end diff --git a/test/unit/app/workers/publishing_api_document_republishing_worker_test.rb b/test/unit/app/workers/publishing_api_document_republishing_worker_test.rb index c4f3843d1f8..b3af4e0b54f 100644 --- a/test/unit/app/workers/publishing_api_document_republishing_worker_test.rb +++ b/test/unit/app/workers/publishing_api_document_republishing_worker_test.rb @@ -10,7 +10,7 @@ class PublishingApiDocumentRepublishingWorkerTest < ActiveSupport::TestCase Whitehall::PublishingApi.expects(:locales_for).never Whitehall::PublishingApi.expects(:patch_links).never PublishingApiUnpublishingWorker.any_instance.expects(:perform).never - ServiceListeners::PublishingApiHtmlAttachments.expects(:process).never + ServiceListeners::PublishingApiAssociatedDocuments.expects(:process).never PublishingApiDocumentRepublishingWorker.new.perform(document.id) end From 62f85ecbbb1aa9bb4a86416f4ec68a28d9ef6d14 Mon Sep 17 00:00:00 2001 From: Bruce Bolt Date: Thu, 21 Mar 2024 16:04:11 +0000 Subject: [PATCH 5/8] Add `associated_documents` methods to some editions This allows us to specify which documents are associated with an edition at the model level, removing the hardcoding of associated documents from `PublishingApiAssociatedDocuments`. --- app/models/call_for_evidence.rb | 8 ++++++++ app/models/call_for_evidence_response.rb | 8 ++++++++ app/models/consultation.rb | 8 ++++++++ app/models/consultation_response.rb | 8 ++++++++ app/models/edition.rb | 8 ++++++++ app/models/publication.rb | 8 ++++++++ .../publishing_api_associated_documents.rb | 6 +++--- app/services/service_listeners/publishing_api_pusher.rb | 2 +- 8 files changed, 52 insertions(+), 4 deletions(-) diff --git a/app/models/call_for_evidence.rb b/app/models/call_for_evidence.rb index f7f9e8b8ef0..0be3c27e419 100644 --- a/app/models/call_for_evidence.rb +++ b/app/models/call_for_evidence.rb @@ -127,6 +127,14 @@ def allows_html_attachments? true end + def associated_documents + attachables.flat_map(&:html_attachments) + end + + def deleted_associated_documents + attachables.flat_map(&:deleted_html_attachments) + end + def previously_published false end diff --git a/app/models/call_for_evidence_response.rb b/app/models/call_for_evidence_response.rb index 1cfc2bc0d1a..87670f5d93f 100644 --- a/app/models/call_for_evidence_response.rb +++ b/app/models/call_for_evidence_response.rb @@ -38,6 +38,14 @@ def allows_html_attachments? true end + def associated_documents + attachables.flat_map(&:html_attachments) + end + + def deleted_associated_documents + attachables.flat_map(&:deleted_html_attachments) + end + def path_name to_model.class.name.underscore end diff --git a/app/models/consultation.rb b/app/models/consultation.rb index 9c45044a70b..136bc402473 100644 --- a/app/models/consultation.rb +++ b/app/models/consultation.rb @@ -141,6 +141,14 @@ def allows_html_attachments? true end + def associated_documents + attachables.flat_map(&:html_attachments) + end + + def deleted_associated_documents + attachables.flat_map(&:deleted_html_attachments) + end + def previously_published false end diff --git a/app/models/consultation_response.rb b/app/models/consultation_response.rb index 6a78eadefd1..37ecd229691 100644 --- a/app/models/consultation_response.rb +++ b/app/models/consultation_response.rb @@ -38,6 +38,14 @@ def allows_html_attachments? true end + def associated_documents + attachables.flat_map(&:html_attachments) + end + + def deleted_associated_documents + attachables.flat_map(&:deleted_html_attachments) + end + def path_name to_model.class.name.underscore end diff --git a/app/models/edition.rb b/app/models/edition.rb index e883b434b51..1fd09f5d552 100644 --- a/app/models/edition.rb +++ b/app/models/edition.rb @@ -742,6 +742,14 @@ def images_have_unique_filenames? names.uniq.length == names.length end + def associated_documents + [] + end + + def deleted_associated_documents + [] + end + private def date_for_government diff --git a/app/models/publication.rb b/app/models/publication.rb index e119c0c0d25..4daa10f38d4 100644 --- a/app/models/publication.rb +++ b/app/models/publication.rb @@ -151,6 +151,14 @@ def allows_html_attachments? true end + def associated_documents + attachables.flat_map(&:html_attachments) + end + + def deleted_associated_documents + attachables.flat_map(&:deleted_html_attachments) + end + def assign_statistics_announcement if statistics_announcement_id.present? self.statistics_announcement = StatisticsAnnouncement.find(statistics_announcement_id) diff --git a/app/services/service_listeners/publishing_api_associated_documents.rb b/app/services/service_listeners/publishing_api_associated_documents.rb index a36e8493ed1..b866834620c 100644 --- a/app/services/service_listeners/publishing_api_associated_documents.rb +++ b/app/services/service_listeners/publishing_api_associated_documents.rb @@ -103,13 +103,13 @@ def previous_edition end def current_associated_documents - edition.attachables.flat_map(&:html_attachments) + edition.associated_documents end def previous_associated_documents return [] unless previous_edition - previous_edition.attachables.flat_map(&:html_attachments) + previous_edition.associated_documents end def content_ids_to_remove @@ -123,7 +123,7 @@ def content_ids_to_remove end def deleted_associated_documents - edition.attachables.flat_map(&:deleted_html_attachments) + edition.deleted_associated_documents end def do_publish(update_type) diff --git a/app/services/service_listeners/publishing_api_pusher.rb b/app/services/service_listeners/publishing_api_pusher.rb index cad55e1f301..5484c77ee1f 100644 --- a/app/services/service_listeners/publishing_api_pusher.rb +++ b/app/services/service_listeners/publishing_api_pusher.rb @@ -46,7 +46,7 @@ def push(event:, options: {}) private def handle_associated_documents(event) - if edition.respond_to?(:html_attachments) + if edition.respond_to?(:associated_documents) || edition.respond_to?(:deleted_associated_documents) PublishingApiAssociatedDocuments.process(edition, event) end end From e06a59b0e04799b7e9badfd1fe4a7c5653b73c17 Mon Sep 17 00:00:00 2001 From: Bruce Bolt Date: Tue, 19 Mar 2024 09:44:39 +0000 Subject: [PATCH 6/8] Publish offices and their contacts in the same way as HTML attachments Worldwide Offices are part of the editionable workflow, so need to be published/unpublished on certain triggers. The HTML attachment workflow already offers this functionality, so we are extending this to work for Worldwide Offices and their Contact. In later work, this will be extended to include the Worldwide Corporate Information Pages. --- .../editionable_worldwide_organisation.rb | 4 + .../publishing_api_associated_documents.rb | 16 +- .../editionable_worldwide_organisations.rb | 5 + ...ublishing_api_associated_documents_test.rb | 225 ++++++++++++++++++ 4 files changed, 246 insertions(+), 4 deletions(-) diff --git a/app/models/editionable_worldwide_organisation.rb b/app/models/editionable_worldwide_organisation.rb index 6995dbd1401..70597b40af0 100644 --- a/app/models/editionable_worldwide_organisation.rb +++ b/app/models/editionable_worldwide_organisation.rb @@ -158,4 +158,8 @@ def republish_dependent_documents documents.each { |d| Whitehall::PublishingApi.republish_document_async(d) } end + + def associated_documents + (offices + offices.map(&:contact)).compact.flatten + end end diff --git a/app/services/service_listeners/publishing_api_associated_documents.rb b/app/services/service_listeners/publishing_api_associated_documents.rb index b866834620c..e7fe2eed364 100644 --- a/app/services/service_listeners/publishing_api_associated_documents.rb +++ b/app/services/service_listeners/publishing_api_associated_documents.rb @@ -38,7 +38,7 @@ def update_draft(update_type: nil) current_associated_documents.each do |associated_document| Whitehall::PublishingApi.save_draft_translation( associated_document, - associated_document.locale || I18n.default_locale.to_s, + locale_for_document(associated_document), update_type || (edition.minor_change? ? "minor" : "major"), ) end @@ -59,7 +59,7 @@ def unpublish(allow_draft: false) PublishingApiRedirectWorker.new.perform( associated_document.content_id, destination, - associated_document.locale || I18n.default_locale.to_s, + locale_for_document(associated_document), allow_draft, ) end @@ -70,7 +70,7 @@ def withdraw PublishingApiWithdrawalWorker.new.perform( associated_document.content_id, edition.unpublishing.explanation, - associated_document.locale || I18n.default_locale.to_s, + locale_for_document(associated_document), false, edition.unpublishing.unpublished_at.to_s, ) @@ -83,6 +83,14 @@ def delete private + def locale_for_document(document) + if document.respond_to?(:locale) && document.locale + document.locale + else + I18n.default_locale.to_s + end + end + def discard_drafts(associated_documents) associated_documents.each do |associated_document| PublishingApiDiscardDraftWorker.perform_async( @@ -140,7 +148,7 @@ def do_publish(update_type) associated_document.class.name, associated_document.id, update_type, - associated_document.locale || I18n.default_locale.to_s, + locale_for_document(associated_document), ) end end diff --git a/test/factories/editionable_worldwide_organisations.rb b/test/factories/editionable_worldwide_organisations.rb index ad511e6e2ba..a5ec392deef 100644 --- a/test/factories/editionable_worldwide_organisations.rb +++ b/test/factories/editionable_worldwide_organisations.rb @@ -50,4 +50,9 @@ factory :deleted_editionable_worldwide_organisation, parent: :editionable_worldwide_organisation, traits: [:deleted] factory :superseded_editionable_worldwide_organisation, parent: :editionable_worldwide_organisation, traits: [:superseded] factory :scheduled_editionable_worldwide_organisation, parent: :editionable_worldwide_organisation, traits: [:scheduled] + factory :unpublished_editionable_worldwide_organisation, parent: :editionable_worldwide_organisation, traits: [:unpublished] + factory :unpublished_editionable_worldwide_organisation_consolidated, parent: :editionable_worldwide_organisation, traits: [:consolidated_redirect] + factory :unpublished_editionable_worldwide_organisation_in_error_redirect, parent: :editionable_worldwide_organisation, traits: [:published_in_error_redirect] + factory :unpublished_editionable_worldwide_organisation_in_error_no_redirect, parent: :editionable_worldwide_organisation, traits: [:published_in_error_no_redirect] + factory :withdrawn_editionable_worldwide_organisation, parent: :editionable_worldwide_organisation, traits: [:withdrawn] end diff --git a/test/unit/app/services/service_listeners/publishing_api_associated_documents_test.rb b/test/unit/app/services/service_listeners/publishing_api_associated_documents_test.rb index 3bf30a85504..6688cfc022e 100644 --- a/test/unit/app/services/service_listeners/publishing_api_associated_documents_test.rb +++ b/test/unit/app/services/service_listeners/publishing_api_associated_documents_test.rb @@ -202,6 +202,52 @@ class Publish < PublishingApiAssociatedDocumentsTest call(new_edition) end + + test "with an office on a new editionable worldwide organisation publishes the office and it's contact" do + worldwide_organisation = create(:editionable_worldwide_organisation, :with_main_office) + + PublishingApiWorker.any_instance.expects(:perform).with( + "WorldwideOffice", + worldwide_organisation.main_office.id, + "major", + "en", + ) + + PublishingApiWorker.any_instance.expects(:perform).with( + "Contact", + worldwide_organisation.main_office.contact.id, + "major", + "en", + ) + + call(worldwide_organisation) + end + + test "with an office on the old version of an editionable worldwide organisation redirects the office" do + worldwide_organisation = create(:published_editionable_worldwide_organisation, :with_main_office) + + new_edition = worldwide_organisation.create_draft(create(:writer)) + new_edition.main_office.destroy! + new_edition.minor_change = true + new_edition.submit! + new_edition.publish! + + old_office = worldwide_organisation.main_office + + PublishingApiRedirectWorker.any_instance.expects(:perform).with( + old_office.content_id, + new_edition.search_link, + "en", + ) + + PublishingApiRedirectWorker.any_instance.expects(:perform).with( + old_office.contact.content_id, + new_edition.search_link, + "en", + ) + + call(new_edition) + end end class UpdateDraft < PublishingApiAssociatedDocumentsTest @@ -278,6 +324,24 @@ class UpdateDraft < PublishingApiAssociatedDocumentsTest call(publication) end end + + test "with an office on a new editionable worldwide organisation saves the office as draft" do + worldwide_organisation = create(:editionable_worldwide_organisation, :with_main_office) + + Whitehall::PublishingApi.expects(:save_draft_translation).with( + worldwide_organisation.main_office, + "en", + "major", + ) + + Whitehall::PublishingApi.expects(:save_draft_translation).with( + worldwide_organisation.main_office.contact, + "en", + "major", + ) + + call(worldwide_organisation) + end end class Unpublish < PublishingApiAssociatedDocumentsTest @@ -299,6 +363,27 @@ class Unpublish < PublishingApiAssociatedDocumentsTest call(publication) end + test "for an editionable worldwide organisation that has been consolidated publishes a redirect to the alternative url" do + worldwide_organisation = create(:unpublished_editionable_worldwide_organisation_consolidated, :with_main_office) + office = worldwide_organisation.main_office + + PublishingApiRedirectWorker.any_instance.expects(:perform).with( + office.content_id, + "/government/another/page", + "en", + false, + ) + + PublishingApiRedirectWorker.any_instance.expects(:perform).with( + office.contact.content_id, + "/government/another/page", + "en", + false, + ) + + call(worldwide_organisation) + end + test "for a publication that has been unpublished with a redirect publishes a redirect to the alternative url" do publication = create(:unpublished_publication_in_error_redirect) attachment = publication.html_attachments.first @@ -311,6 +396,27 @@ class Unpublish < PublishingApiAssociatedDocumentsTest call(publication) end + test "for an editionable worldwide organisation that has been unpublished with a redirect publishes a redirect to the alternative url" do + worldwide_organisation = create(:unpublished_editionable_worldwide_organisation_in_error_redirect, :with_main_office) + office = worldwide_organisation.main_office + + PublishingApiRedirectWorker.any_instance.expects(:perform).with( + office.content_id, + "/government/another/page", + "en", + false, + ) + + PublishingApiRedirectWorker.any_instance.expects(:perform).with( + office.contact.content_id, + "/government/another/page", + "en", + false, + ) + + call(worldwide_organisation) + end + test "for a publication that has been unpublished with an external redirect publishes a redirect to the alternative url" do external_url = "https://test.ukri.org/some-page" publication = create(:unpublished_publication, { unpublishing: build(:unpublishing, { redirect: true, alternative_url: external_url }) }) @@ -324,6 +430,28 @@ class Unpublish < PublishingApiAssociatedDocumentsTest call(publication) end + test "for an editionable worldwide organisation that has been unpublished with an external redirect publishes a redirect to the alternative url" do + external_url = "https://test.ukri.org/some-page" + worldwide_organisation = create(:unpublished_editionable_worldwide_organisation_in_error_redirect, :with_main_office, { unpublishing: build(:unpublishing, { redirect: true, alternative_url: external_url }) }) + office = worldwide_organisation.main_office + + PublishingApiRedirectWorker.any_instance.expects(:perform).with( + office.content_id, + external_url, + "en", + false, + ) + + PublishingApiRedirectWorker.any_instance.expects(:perform).with( + office.contact.content_id, + external_url, + "en", + false, + ) + + call(worldwide_organisation) + end + test "for a publication that has been unpublished without a redirect publishes a redirect to the parent document" do publication = create(:unpublished_publication_in_error_no_redirect) attachment = publication.html_attachments.first @@ -336,6 +464,27 @@ class Unpublish < PublishingApiAssociatedDocumentsTest call(publication) end + test "for an editionable worldwide organisation that has been unpublished without a redirect publishes a redirect to the parent docuemnt" do + worldwide_organisation = create(:unpublished_editionable_worldwide_organisation_in_error_no_redirect, :with_main_office) + office = worldwide_organisation.main_office + + PublishingApiRedirectWorker.any_instance.expects(:perform).with( + office.content_id, + worldwide_organisation.search_link, + "en", + false, + ) + + PublishingApiRedirectWorker.any_instance.expects(:perform).with( + office.contact.content_id, + worldwide_organisation.search_link, + "en", + false, + ) + + call(worldwide_organisation) + end + class Withdraw < PublishingApiAssociatedDocumentsTest test "for something that can't have html attachments doesn't publish a withdrawal" do call(create(:published_news_article)) @@ -359,6 +508,28 @@ class Withdraw < PublishingApiAssociatedDocumentsTest call(publication) end + test "for an editionable worldwide organisation that has been withdrawn publishes a withdrawal for the office" do + worldwide_organisation = create(:withdrawn_editionable_worldwide_organisation, :with_main_office) + + PublishingApiWithdrawalWorker.any_instance.expects(:perform).with( + worldwide_organisation.main_office.content_id, + "content was withdrawn", + "en", + false, + worldwide_organisation.unpublishing.unpublished_at.to_s, + ) + + PublishingApiWithdrawalWorker.any_instance.expects(:perform).with( + worldwide_organisation.main_office.contact.content_id, + "content was withdrawn", + "en", + false, + worldwide_organisation.unpublishing.unpublished_at.to_s, + ) + + call(worldwide_organisation) + end + test "for a publication with a translated HTML attachment publishes a withdrawal with the expected locale for each attachment" do en_attachment = build(:html_attachment) cy_attachment = build(:html_attachment, locale: "cy") @@ -398,6 +569,22 @@ class Delete < PublishingApiAssociatedDocumentsTest call(publication) end + test "for a draft editionable worldwide organisation with offices discards the draft" do + worldwide_organisation = create(:draft_editionable_worldwide_organisation, :with_main_office) + + PublishingApiDiscardDraftWorker.expects(:perform_async).with( + worldwide_organisation.main_office.content_id, + "en", + ) + + PublishingApiDiscardDraftWorker.expects(:perform_async).with( + worldwide_organisation.main_office.contact.content_id, + "en", + ) + + call(worldwide_organisation) + end + test "for a draft publication with deleted html attachments discards the deleted attachment drafts" do publication = create(:draft_publication) attachment = publication.html_attachments.first @@ -424,6 +611,24 @@ class Republish < PublishingApiAssociatedDocumentsTest call(publication) end + test "for a draft editionable worldwide organisation with an office publishes the draft office" do + worldwide_organisation = create(:draft_editionable_worldwide_organisation, :with_main_office) + + Whitehall::PublishingApi.expects(:save_draft_translation).with( + worldwide_organisation.main_office, + "en", + "republish", + ) + + Whitehall::PublishingApi.expects(:save_draft_translation).with( + worldwide_organisation.main_office.contact, + "en", + "republish", + ) + + call(worldwide_organisation) + end + test "for a published publication with an attachment publishes the attachment" do publication = create(:published_publication) attachment = publication.html_attachments.first @@ -436,6 +641,26 @@ class Republish < PublishingApiAssociatedDocumentsTest call(publication) end + test "for a published editionable worldwide organisation with an office publishes the office" do + worldwide_organisation = create(:published_editionable_worldwide_organisation, :with_main_office) + + PublishingApiWorker.any_instance.expects(:perform).with( + "WorldwideOffice", + worldwide_organisation.main_office.id, + "republish", + "en", + ) + + PublishingApiWorker.any_instance.expects(:perform).with( + "Contact", + worldwide_organisation.main_office.contact.id, + "republish", + "en", + ) + + call(worldwide_organisation) + end + test "for a published publication with a deleted attachment discards the attachment draft" do publication = create(:published_publication) attachment = publication.html_attachments.first From 79513d3cfe9d4285859ae6d4b4c994cf2d2c3a69 Mon Sep 17 00:00:00 2001 From: Bruce Bolt Date: Wed, 20 Mar 2024 09:57:25 +0000 Subject: [PATCH 7/8] Republish editionable worldwide organisation when offices changes When a Worldwide Office is created, updated or deleted, we need to republish the associated Editionable Worldwide Organisation. By means of `PublishingApiAssociatedDocuments`, this will also have the effect of the Worldwide Office (and it's Contact) also having their draft published or updated. --- .../admin/worldwide_offices_controller.rb | 7 +++ .../worldwide_offices_controller_test.rb | 51 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/app/controllers/admin/worldwide_offices_controller.rb b/app/controllers/admin/worldwide_offices_controller.rb index 7f12b47793c..7fefc642a82 100644 --- a/app/controllers/admin/worldwide_offices_controller.rb +++ b/app/controllers/admin/worldwide_offices_controller.rb @@ -20,6 +20,7 @@ def update worldwide_office_params[:service_ids] ||= [] if @worldwide_office.update(worldwide_office_params) handle_show_on_home_page_param + republish_draft_worldwide_organisation redirect_to admin_worldwide_organisation_worldwide_offices_path(@worldwide_organisation), notice: "#{@worldwide_office.title} has been edited" else @worldwide_office.contact.contact_numbers.build if @worldwide_office.contact.contact_numbers.blank? @@ -31,6 +32,7 @@ def create @worldwide_office = @worldwide_organisation.offices.build(worldwide_office_params) if @worldwide_office.save handle_show_on_home_page_param + republish_draft_worldwide_organisation redirect_to admin_worldwide_organisation_worldwide_offices_path(@worldwide_organisation), notice: "#{@worldwide_office.title} has been added" else @worldwide_office.contact.contact_numbers.build if @worldwide_office.contact.contact_numbers.blank? @@ -48,6 +50,7 @@ def destroy title = @worldwide_office.title if @worldwide_office.destroy + republish_draft_worldwide_organisation redirect_to admin_worldwide_organisation_worldwide_offices_path(@worldwide_organisation), notice: "#{title} has been deleted" else render :edit @@ -126,4 +129,8 @@ def worldwide_office_params { contact_numbers_attributes: %i[id label number _destroy] }, ]) end + + def republish_draft_worldwide_organisation + Whitehall.edition_services.draft_updater(@worldwide_office.edition).perform! if @worldwide_office.edition + end end diff --git a/test/functional/admin/worldwide_offices_controller_test.rb b/test/functional/admin/worldwide_offices_controller_test.rb index 563b0aa18fb..9b8f5bf53d1 100644 --- a/test/functional/admin/worldwide_offices_controller_test.rb +++ b/test/functional/admin/worldwide_offices_controller_test.rb @@ -389,6 +389,57 @@ class Admin::WorldwideOfficesControllerTest < ActionController::TestCase assert_equal [office1, office2], assigns(:reorderable_offices) end + test "POST :create for an office attached to an editionable worldwide organisation republishes the draft of the editionable worldwide organisation" do + feature_flags.switch! :editionable_worldwide_organisations, true + + worldwide_organisation = create(:draft_editionable_worldwide_organisation) + + Whitehall::PublishingApi.expects(:save_draft).with(worldwide_organisation) + + post :create, + params: { + worldwide_office: { + worldwide_office_type_id: WorldwideOfficeType::Other.id, + contact_attributes: { + title: "Main office", + contact_type_id: ContactType::General.id, + }, + }, + worldwide_organisation_id: worldwide_organisation, + } + end + + test "PUT :update for an office attached to an editionable worldwide organisation republishes the draft of the editionable worldwide organisation" do + feature_flags.switch! :editionable_worldwide_organisations, true + + office = create(:worldwide_office, edition: create(:draft_editionable_worldwide_organisation), worldwide_organisation: nil) + + Whitehall::PublishingApi.expects(:save_draft).with(office.edition) + + put :update, + params: { + worldwide_office: { + access_and_opening_times: "New times", + }, + id: office, + worldwide_organisation_id: office.edition, + } + end + + test "DELETE :destroy for an office attached to an editionable worldwide organisation republishes the draft of the editionable worldwide organisation" do + feature_flags.switch! :editionable_worldwide_organisations, true + + office = create(:worldwide_office, edition: create(:draft_editionable_worldwide_organisation), worldwide_organisation: nil) + + Whitehall::PublishingApi.expects(:save_draft).with(office.edition) + + delete :destroy, + params: { + id: office, + worldwide_organisation_id: office.edition, + } + end + private def create_worldwide_organisation_and_office From a585bc6603bf7cf1267b50ee3c437e714bd6af7e Mon Sep 17 00:00:00 2001 From: Bruce Bolt Date: Tue, 19 Mar 2024 11:59:43 +0000 Subject: [PATCH 8/8] Unpublish draft worldwide office when deleted If a worldwide office is attached to a draft editionable worldwide organisation and the office itself is deleted, we need to unpublish the draft office and it's contact from Publishing API. This cannot be done using `PublishingApiAssociatedDocuments` as worldwide offices are hard deleted and therefore we can't use a query to identify which offices have been deleted, like we can for HTML attachments. --- .../admin/worldwide_offices_controller.rb | 5 +++++ .../admin/worldwide_offices_controller_test.rb | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/app/controllers/admin/worldwide_offices_controller.rb b/app/controllers/admin/worldwide_offices_controller.rb index 7fefc642a82..2a312b0bcc2 100644 --- a/app/controllers/admin/worldwide_offices_controller.rb +++ b/app/controllers/admin/worldwide_offices_controller.rb @@ -50,6 +50,11 @@ def destroy title = @worldwide_office.title if @worldwide_office.destroy + if @worldwide_office.edition + PublishingApiDiscardDraftWorker.perform_async(@worldwide_office.content_id, I18n.default_locale.to_s) + PublishingApiDiscardDraftWorker.perform_async(@worldwide_office.contact.content_id, I18n.default_locale.to_s) + end + republish_draft_worldwide_organisation redirect_to admin_worldwide_organisation_worldwide_offices_path(@worldwide_organisation), notice: "#{title} has been deleted" else diff --git a/test/functional/admin/worldwide_offices_controller_test.rb b/test/functional/admin/worldwide_offices_controller_test.rb index 9b8f5bf53d1..0657d52a736 100644 --- a/test/functional/admin/worldwide_offices_controller_test.rb +++ b/test/functional/admin/worldwide_offices_controller_test.rb @@ -440,6 +440,23 @@ class Admin::WorldwideOfficesControllerTest < ActionController::TestCase } end + test "DELETE :destroy for an office attached to an editionable worldwide organisation discards draft of the office and the contact" do + feature_flags.switch! :editionable_worldwide_organisations, true + + office = create(:worldwide_office, edition: create(:draft_editionable_worldwide_organisation), worldwide_organisation: nil) + + PublishingApiDiscardDraftWorker.any_instance.expects(:perform).with(office.content_id, "en") + PublishingApiDiscardDraftWorker.any_instance.expects(:perform).with(office.contact.content_id, "en") + + Sidekiq::Testing.inline! do + delete :destroy, + params: { + id: office, + worldwide_organisation_id: office.edition, + } + end + end + private def create_worldwide_organisation_and_office