From 9b3f787dd0d6f569f1b0140e307efc8285ac1217 Mon Sep 17 00:00:00 2001 From: Joseph Kempster Date: Mon, 26 Jun 2023 14:32:22 +0100 Subject: [PATCH 1/3] Add presenter for worldwide corporate information pages We have decided to publish the Corporate Information Pages associated with Worldwide Organisations as a separate content item (https://github.com/alphagov/publishing-api/pull/2399) for the following reasons: - We require a link to the associated worldwide_organisation. Having a new content item means that we don't have to attach multiple optional links to the corporate information page content item. - We will need a new template for this page in government-frontend as the worldwide organisation corporate information pages are quite different when compared to the organisation corporate information pages. Because government_frontend maps content items to templates through their schema_name (except for a few exceptions), it would make sense to follow this pattern. - Many of the attributes of the corporate information page schema are not needed in order to render the worldwide organisation corporate informaiton pages. This adds a presenter for the new content item. Any shared attributes are DRYed up through a shared PayloadBuilder. --- .../corporate_information_page_presenter.rb | 28 +----- .../corporate_information_page.rb | 41 +++++++++ ...de_corporate_information_page_presenter.rb | 37 ++++++++ ...rporate_information_page_presenter_test.rb | 88 +++++++++++++++++++ .../publishing_api_presenters_test.rb | 4 +- 5 files changed, 171 insertions(+), 27 deletions(-) create mode 100644 app/presenters/publishing_api/payload_builder/corporate_information_page.rb create mode 100644 app/presenters/publishing_api/worldwide_corporate_information_page_presenter.rb create mode 100644 test/unit/presenters/publishing_api/worldwide_corporate_information_page_presenter_test.rb diff --git a/app/presenters/publishing_api/corporate_information_page_presenter.rb b/app/presenters/publishing_api/corporate_information_page_presenter.rb index 41975ff5402..2c6980af1bd 100644 --- a/app/presenters/publishing_api/corporate_information_page_presenter.rb +++ b/app/presenters/publishing_api/corporate_information_page_presenter.rb @@ -19,12 +19,10 @@ def content .new(corporate_information_page, update_type:) .base_attributes .merge(PayloadBuilder::PublicDocumentPath.for(corporate_information_page)) - .merge( - description: corporate_information_page.summary, + .merge(PayloadBuilder::CorporateInformationPage.for(corporate_information_page)) + .deep_merge( details:, document_type:, - public_updated_at:, - rendering_app: corporate_information_page.rendering_app, schema_name: SCHEMA_NAME, links: edition_links, auth_bypass_ids: [corporate_information_page.auth_bypass_id], @@ -59,21 +57,8 @@ def document_type delegate :display_type_key, to: :corporate_information_page - def base_details - { - body:, - } - end - - def body - Whitehall::GovspeakRenderer - .new - .govspeak_edition_to_html(corporate_information_page) - end - def details - base_details - .merge(change_history) + change_history .merge(CorporateInformationGroups.for(corporate_information_page)) .merge(Organisation.for(corporate_information_page)) .merge(PayloadBuilder::TagDetails.for(corporate_information_page)) @@ -84,13 +69,6 @@ def links_presenter @links_presenter ||= LinksPresenter.new(corporate_information_page) end - def public_updated_at - public_updated_at = corporate_information_page.public_timestamp || - corporate_information_page.updated_at - - public_updated_at.rfc3339 - end - def change_history return {} if corporate_information_page.change_history.blank? diff --git a/app/presenters/publishing_api/payload_builder/corporate_information_page.rb b/app/presenters/publishing_api/payload_builder/corporate_information_page.rb new file mode 100644 index 00000000000..3b524177138 --- /dev/null +++ b/app/presenters/publishing_api/payload_builder/corporate_information_page.rb @@ -0,0 +1,41 @@ +module PublishingApi + module PayloadBuilder + class CorporateInformationPage + attr_reader :item + + def self.for(item) + new(item).call + end + + def initialize(item) + @item = item + end + + def call + { + description: item.summary, + rendering_app: item.rendering_app, + public_updated_at:, + details: { + body:, + }, + } + end + + private + + def public_updated_at + public_updated_at = item.public_timestamp || + item.updated_at + + public_updated_at.rfc3339 + end + + def body + Whitehall::GovspeakRenderer + .new + .govspeak_edition_to_html(item) + end + end + end +end diff --git a/app/presenters/publishing_api/worldwide_corporate_information_page_presenter.rb b/app/presenters/publishing_api/worldwide_corporate_information_page_presenter.rb new file mode 100644 index 00000000000..6eaa8fa3de8 --- /dev/null +++ b/app/presenters/publishing_api/worldwide_corporate_information_page_presenter.rb @@ -0,0 +1,37 @@ +module PublishingApi + class WorldwideCorporateInformationPagePresenter + include UpdateTypeHelper + + attr_accessor :item, :update_type + + def initialize(item, update_type: nil) + self.item = item + self.update_type = + update_type || default_update_type(item) + end + + delegate :content_id, to: :item + + def content + content = BaseItemPresenter.new( + item, + update_type:, + ).base_attributes + + content.merge!(PayloadBuilder::CorporateInformationPage.for(item)) + content.merge!(PayloadBuilder::PolymorphicPath.for(item)) + + content.merge!( + document_type: item.display_type_key, + schema_name: "worldwide_corporate_information_page", + ) + end + + def links + { + parent: [item.worldwide_organisation.content_id], + worldwide_organisation: [item.worldwide_organisation.content_id], + } + end + end +end diff --git a/test/unit/presenters/publishing_api/worldwide_corporate_information_page_presenter_test.rb b/test/unit/presenters/publishing_api/worldwide_corporate_information_page_presenter_test.rb new file mode 100644 index 00000000000..04af8cd73f7 --- /dev/null +++ b/test/unit/presenters/publishing_api/worldwide_corporate_information_page_presenter_test.rb @@ -0,0 +1,88 @@ +require "test_helper" + +module PublishingApi::WorldwideCorporateInformationPagePresenterTest + class TestCase < ActiveSupport::TestCase + attr_accessor :corporate_information_page, :update_type + + def presented_corporate_information_page + PublishingApi::WorldwideCorporateInformationPagePresenter.new( + corporate_information_page, + update_type:, + ) + end + + class BasicCorporateInformationPageTest < TestCase + setup do + worldwide_organisation = create(:worldwide_organisation) + self.corporate_information_page = create(:corporate_information_page, worldwide_organisation:, organisation: nil) + end + + test "presents a Worldwide Corporate Information Page ready for adding to the publishing API" do + public_path = corporate_information_page.public_path + + expected_hash = { + base_path: public_path, + title: corporate_information_page.title, + schema_name: "worldwide_corporate_information_page", + document_type: corporate_information_page.display_type_key, + locale: "en", + publishing_app: Whitehall::PublishingApp::WHITEHALL, + rendering_app: Whitehall::RenderingApp::WHITEHALL_FRONTEND, + public_updated_at: corporate_information_page.updated_at, + routes: [{ path: public_path, type: "exact" }], + redirects: [], + description: "edition-summary", + details: { + body: "

Some stuff

\n
", + }, + update_type: "major", + } + + expected_links = { + parent: [ + corporate_information_page.owning_organisation.content_id, + ], + worldwide_organisation: [ + corporate_information_page.owning_organisation.content_id, + ], + } + + presented_item = presented_corporate_information_page + + assert_equal expected_hash, presented_item.content + assert_hash_includes presented_item.links, expected_links + assert_equal "major", presented_item.update_type + assert_equal corporate_information_page.content_id, presented_item.content_id + + assert_valid_against_publisher_schema(presented_item.content, "worldwide_corporate_information_page") + assert_valid_against_links_schema({ links: presented_item.links }, "worldwide_corporate_information_page") + end + end + + class CorporateInformationPageWithMinorChange < TestCase + setup do + self.corporate_information_page = create( + :corporate_information_page, + minor_change: true, + ) + end + + test "update type" do + assert_equal "minor", presented_corporate_information_page.update_type + end + end + + class CorporateInformationPageWithoutMinorChange < TestCase + setup do + self.corporate_information_page = create( + :corporate_information_page, + minor_change: false, + ) + end + + test "update type" do + assert_equal "major", presented_corporate_information_page.update_type + end + end + end +end diff --git a/test/unit/presenters/publishing_api_presenters_test.rb b/test/unit/presenters/publishing_api_presenters_test.rb index 0eca6315bab..57ef67ab508 100644 --- a/test/unit/presenters/publishing_api_presenters_test.rb +++ b/test/unit/presenters/publishing_api_presenters_test.rb @@ -144,7 +144,7 @@ class PublishingApiPresentersTest < ActiveSupport::TestCase ) end - test ".presenter_for returns a GenericEditionPresenter for a " \ + test ".presenter_for returns a GenericEditionPresenter for an " \ "CorporateInformationPage belonging to an WorldwideOrganisation" do presenter = PublishingApiPresenters .presenter_for( @@ -155,7 +155,7 @@ class PublishingApiPresentersTest < ActiveSupport::TestCase ) assert_equal( - PublishingApi::GenericEditionPresenter, + PublishingApi::WorldwideCorporateInformationPagePresenter, presenter.class, ) end From fe069442b90473f077eca4dc27320e278f541e60 Mon Sep 17 00:00:00 2001 From: Joseph Kempster Date: Mon, 26 Jun 2023 15:30:10 +0100 Subject: [PATCH 2/3] Publish worldwide corporate information pages via their own presenter We have added a new WorldwideCorporateInformationPagePresenter. Present the worldwide corporate information pages via this new presenter unless they are "about us" pages. "about us" worldwide corporate information pages are a special case. We want to publish these as `placeholder` content items with a unique base path (see #7844). --- app/presenters/publishing_api_presenters.rb | 8 +++++--- .../publishing_api_presenters_test.rb | 20 ++++++++++++++++++- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/app/presenters/publishing_api_presenters.rb b/app/presenters/publishing_api_presenters.rb index 84cf10bb264..929d3a2be02 100644 --- a/app/presenters/publishing_api_presenters.rb +++ b/app/presenters/publishing_api_presenters.rb @@ -62,10 +62,12 @@ def presenter_class_for_edition(edition) when Consultation PublishingApi::ConsultationPresenter when CorporateInformationPage - if edition.worldwide_organisation.present? - FALLBACK_EDITION_PRESENTER - else + if edition.worldwide_organisation.present? && !edition.about_page? + PublishingApi::WorldwideCorporateInformationPagePresenter + elsif edition.organisation.present? PublishingApi::CorporateInformationPagePresenter + else + FALLBACK_EDITION_PRESENTER end when ::DetailedGuide PublishingApi::DetailedGuidePresenter diff --git a/test/unit/presenters/publishing_api_presenters_test.rb b/test/unit/presenters/publishing_api_presenters_test.rb index 57ef67ab508..f85009c65ab 100644 --- a/test/unit/presenters/publishing_api_presenters_test.rb +++ b/test/unit/presenters/publishing_api_presenters_test.rb @@ -145,15 +145,33 @@ class PublishingApiPresentersTest < ActiveSupport::TestCase end test ".presenter_for returns a GenericEditionPresenter for an " \ - "CorporateInformationPage belonging to an WorldwideOrganisation" do + "AboutUs CorporateInformationPage belonging to an WorldwideOrganisation" do presenter = PublishingApiPresenters .presenter_for( build( :corporate_information_page, worldwide_organisation: build(:worldwide_organisation), + organisation: nil, + corporate_information_page_type_id: CorporateInformationPageType::AboutUs.id, ), ) + assert_equal( + PublishingApi::GenericEditionPresenter, + presenter.class, + ) + end + + test ".presenter_for returns a WorldwideCorporateInformationPagePresenter for a " \ + "CorporateInformationPage belonging to an WorldwideOrganisation" do + presenter = PublishingApiPresenters + .presenter_for( + build( + :corporate_information_page, + worldwide_organisation: build(:worldwide_organisation), + ), + ) + assert_equal( PublishingApi::WorldwideCorporateInformationPagePresenter, presenter.class, From ab8d8c9123328ef824485679046abd686fc817e7 Mon Sep 17 00:00:00 2001 From: Joseph Kempster Date: Mon, 12 Jun 2023 11:52:51 +0100 Subject: [PATCH 3/3] Republish Worldwide Organisation on Corporate Information Pages changes Currently, Organisations are republished when a related Corporate Information Pages changes, but Worldwide Organisations are not. Now that we publish Worldwide Corporate Information Pages, we want to change this. This is important because the Worldwide Organisation content item surfaces information about related Corporate Information Pages on it's `details` rather than it's `links` (though the links are present as well, they aren't used. This is consistent with the Organisation content item). --- app/models/corporate_information_page.rb | 6 ++--- .../models/corporate_information_page_test.rb | 26 +++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/app/models/corporate_information_page.rb b/app/models/corporate_information_page.rb index be9b7e4ad2c..b8ceb2c0593 100644 --- a/app/models/corporate_information_page.rb +++ b/app/models/corporate_information_page.rb @@ -2,7 +2,7 @@ class CorporateInformationPage < Edition include ::Attachable include Searchable - after_commit :republish_organisation_to_publishing_api + after_commit :republish_owning_organisation_to_publishing_api after_commit :republish_about_page_to_publishing_api, unless: :about_page? after_save :reindex_organisation_in_search_index, if: :about_page? @@ -31,8 +31,8 @@ def process_associations_before_save(new_edition) scope :with_organisation_govuk_status, ->(status) { joins(:organisation).where(organisations: { govuk_status: status }) } scope :accessible_documents_policy, -> { where(corporate_information_page_type_id: CorporateInformationPageType::AccessibleDocumentsPolicy.id) } - def republish_organisation_to_publishing_api - Whitehall::PublishingApi.republish_async(owning_organisation) if owning_organisation.is_a?(Organisation) + def republish_owning_organisation_to_publishing_api + Whitehall::PublishingApi.republish_async(owning_organisation) if owning_organisation.present? end def republish_about_page_to_publishing_api diff --git a/test/unit/models/corporate_information_page_test.rb b/test/unit/models/corporate_information_page_test.rb index 21cff745f8e..a06281f0b62 100644 --- a/test/unit/models/corporate_information_page_test.rb +++ b/test/unit/models/corporate_information_page_test.rb @@ -72,4 +72,30 @@ class CorporateInformationPageTest < ActiveSupport::TestCase assert_equal "/world/organisations/#{worldwide_organisation.name}/about/about", corporate_information_page.base_path end + + test "republishes owning organisation after commit when present" do + organisation = create(:organisation) + corporate_information_page = create(:corporate_information_page, organisation:, worldwide_organisation: nil) + + Whitehall::PublishingApi.expects(:republish_async).with(organisation).once + + corporate_information_page.update!(body: "new body") + end + + test "republishes owning worldwide organisation after commit when present" do + worldwide_organisation = create(:worldwide_organisation) + corporate_information_page = create(:corporate_information_page, organisation: nil, worldwide_organisation:) + + Whitehall::PublishingApi.expects(:republish_async).with(worldwide_organisation).once + + corporate_information_page.update!(body: "new body") + end + + test "does not republish owning organisation when absent" do + corporate_information_page = create(:corporate_information_page, organisation: nil, worldwide_organisation: nil) + + Whitehall::PublishingApi.expects(:republish_async).never + + corporate_information_page.update!(body: "new body") + end end