Skip to content

Commit

Permalink
Publish worldwide corporate information about pages as redirects
Browse files Browse the repository at this point in the history
Worldwide corporate information about pages aren't rendered, instead they are
surfaced in the body of the owning worldwide organisation.

There have been various attempts to devise a strategy for these pages.
Currently, they are published with the `schema_name` of
`placeholder_corporate_information_page` at the path
`world/organisation/:organisation_id/about/about`.

Unfortunately, anywhere in Whitehall that links to the base path of these pages
now 404s. Instead, we should redirect to the worldwide organisation itself.

This:
- Updates the base path of worldwide corporate
  information pages to
  `world/organisation/:organisation_id/about` to
  match the non-worldwide
  organisation corporate information pages.
- Adds a generic presenter for a redirect.
- Publishes worldwide corporate information pages as
  redirects.

This means any worldwide corporate information about pages will now live at
`world/organisation/:organisation_id/about`. When we republish these content
items, Pulishing API will create additional redirect from each
`world/organisation/:organisation_id/about/about` to
`world/organisation/:organisation_id/about` as well.
  • Loading branch information
jkempster34 committed Aug 7, 2023
1 parent 724baed commit 83c047d
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 10 deletions.
9 changes: 7 additions & 2 deletions app/models/corporate_information_page.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ def sorted_organisations
organisations
end

def api_presenter_redirect_to
raise "only worldwide about pages should redirect" unless about_page? && worldwide_organisation.present?

worldwide_organisation.public_path(locale: I18n.locale)
end

def self.for_slug(slug)
if (type = CorporateInformationPageType.find(slug))
find_by(corporate_information_page_type_id: type.id)
Expand Down Expand Up @@ -175,8 +181,7 @@ def base_path
return if owning_organisation.blank?

url = owning_organisation.base_path + "/about/#{slug}"
url.gsub!("/about/about", "/about") if organisation.present?
url
url.gsub("/about/about", "/about")
end

private
Expand Down
39 changes: 39 additions & 0 deletions app/presenters/publishing_api/redirect_presenter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
module PublishingApi
class RedirectPresenter
attr_accessor :item, :update_type

def initialize(item, update_type: nil)
self.item = item
self.update_type = update_type || "major"
end

delegate :content_id, to: :item

def content
{
title: nil,
locale: I18n.locale.to_s,
base_path: item.public_path(locale: I18n.locale),
document_type: "redirect",
schema_name: "redirect",
redirects:,
publishing_app: Whitehall::PublishingApp::WHITEHALL,
update_type:,
}
end

def links
{}
end

private

def redirects
[{
path: item.public_path(locale: I18n.locale),
type: "exact",
destination: item.api_presenter_redirect_to,
}]
end
end
end
8 changes: 4 additions & 4 deletions app/presenters/publishing_api_presenters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,12 @@ def presenter_class_for_edition(edition)
when Consultation
PublishingApi::ConsultationPresenter
when CorporateInformationPage
if edition.worldwide_organisation.present? && !edition.about_page?
if edition.worldwide_organisation.present? && edition.about_page?
PublishingApi::RedirectPresenter
elsif edition.worldwide_organisation.present?
PublishingApi::WorldwideCorporateInformationPagePresenter
elsif edition.organisation.present?
PublishingApi::CorporateInformationPagePresenter
else
FALLBACK_EDITION_PRESENTER
PublishingApi::CorporateInformationPagePresenter
end
when ::DetailedGuide
PublishingApi::DetailedGuidePresenter
Expand Down
25 changes: 23 additions & 2 deletions test/unit/app/models/corporate_information_page_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,17 @@ class CorporateInformationPageTest < ActiveSupport::TestCase
assert_equal "/government/organisations/#{organisation.name}/about", corporate_information_page.base_path
end

test "base_path appends /about to the associated Worldwide Organisation base_path when about page" do
worldwide_organisation = create(:worldwide_organisation)
corporate_information_page = create(
:about_corporate_information_page,
organisation: nil,
worldwide_organisation:,
)

assert_equal "/world/organisations/#{worldwide_organisation.name}/about", corporate_information_page.base_path
end

test "base_path appends Corporate Information Page path to the associated WorldwideOrganisation base_path" do
worldwide_organisation = create(:worldwide_organisation)
corporate_information_page = create(
Expand All @@ -62,15 +73,25 @@ class CorporateInformationPageTest < ActiveSupport::TestCase
assert_equal "/world/organisations/#{worldwide_organisation.name}/about/#{corporate_information_page.slug}", corporate_information_page.base_path
end

test "#base_path appends /about/about to WorldwideOrganisation base_path when about page" do
test "api_presenter_redirect_to returns the base_path of the owning Worldwide Organisation for about us pages" do
worldwide_organisation = create(:worldwide_organisation)
corporate_information_page = create(
:about_corporate_information_page,
organisation: nil,
worldwide_organisation:,
)

assert_equal "/world/organisations/#{worldwide_organisation.name}/about/about", corporate_information_page.base_path
assert_equal "/world/organisations/#{worldwide_organisation.name}", corporate_information_page.api_presenter_redirect_to
end

test "api_presenter_redirect_to returns a #{RuntimeError} when not a Worldwide Organisation about page" do
organisation = create(:organisation)
corporate_information_page = create(
:about_corporate_information_page,
organisation:,
)

assert_raises(RuntimeError, "only worldwide about pages should redirect") { corporate_information_page.api_presenter_redirect_to }
end

test "republishes owning organisation after commit when present" do
Expand Down
73 changes: 73 additions & 0 deletions test/unit/app/presenters/publishing_api/redirect_presenter_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
require "test_helper"

class PublishingApi::RedirectPresenterTest < ActiveSupport::TestCase
def present(...)
PublishingApi::RedirectPresenter.new(...)
end

test "presents an item as a redirect to the publishing API" do
item = create(
:corporate_information_page,
:published,
organisation: nil,
worldwide_organisation: create(:worldwide_organisation),
corporate_information_page_type_id: CorporateInformationPageType::AboutUs.id,
)

expected_hash = {
title: nil,
locale: "en",
publishing_app: "whitehall",
redirects: [{
path: item.base_path,
type: "exact",
destination: item.api_presenter_redirect_to,
}],
update_type: "major",
base_path: item.base_path,
document_type: "redirect",
schema_name: "redirect",
}

presented_item = present(item)

assert_equal item.content_id, presented_item.content_id
assert_equal expected_hash, presented_item.content
assert_equal presented_item.links, {}
assert_valid_against_publisher_schema(presented_item.content, "redirect")
end

test "presents an item as a redirect to publishing API when translated" do
I18n.with_locale(:ar) do
item = create(
:corporate_information_page,
:published,
organisation: nil,
worldwide_organisation: create(:worldwide_organisation),
corporate_information_page_type_id: CorporateInformationPageType::AboutUs.id,
)

expected_hash = {
title: nil,
locale: "ar",
publishing_app: "whitehall",
redirects: [{
path: item.public_path(locale: I18n.locale),
type: "exact",
destination: item.api_presenter_redirect_to,
}],
update_type: "major",
base_path: item.public_path(locale: I18n.locale),
document_type: "redirect",
schema_name: "redirect",
}

presented_item = present(item)

assert_equal item.content_id, presented_item.content_id
assert_equal expected_hash, presented_item.content
assert_equal presented_item.links, {}
assert_valid_against_publisher_schema(presented_item.content, "redirect")
end
end
end
4 changes: 2 additions & 2 deletions test/unit/app/presenters/publishing_api_presenters_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class PublishingApiPresentersTest < ActiveSupport::TestCase
)
end

test ".presenter_for returns a GenericEditionPresenter for an " \
test ".presenter_for returns a RedirectPresenter for an " \
"AboutUs CorporateInformationPage belonging to an WorldwideOrganisation" do
presenter = PublishingApiPresenters
.presenter_for(
Expand All @@ -162,7 +162,7 @@ class PublishingApiPresentersTest < ActiveSupport::TestCase
)

assert_equal(
PublishingApi::GenericEditionPresenter,
PublishingApi::RedirectPresenter,
presenter.class,
)
end
Expand Down

0 comments on commit 83c047d

Please sign in to comment.