From 29d502919026446c102f78bee70280a31be39dfb Mon Sep 17 00:00:00 2001 From: Alex Newton Date: Tue, 12 May 2026 16:58:38 +0100 Subject: [PATCH] Allow Welsh-language mainstream URLs to be added to document collection groups The Publishing API's lookup_content_id endpoint does not index translated (Welsh) base paths, causing a false "must reference a GOV.UK page" error when users tried to add Welsh mainstream URLs to a document collection. Fall back to the Content Store when both Publishing API lookups return no content ID, as the Content Store resolves content items by any translated base path. If the Content Store also returns a 404, the original error is still raised. Adds Services.content_store using GdsApi::ContentStore, and updates tests to cover the Welsh URL case and to stub the Content Store in the existing "not found" test. --- .gitignore | 1 + .../govuk_url.rb | 28 ++++----- .../document_collection_steps.rb | 3 +- lib/services.rb | 5 ++ .../govuk_url_test.rb | 63 +++++++++++++++---- 5 files changed, 73 insertions(+), 27 deletions(-) diff --git a/.gitignore b/.gitignore index 298df8de320..571c536cfb5 100644 --- a/.gitignore +++ b/.gitignore @@ -33,3 +33,4 @@ /app/assets/builds/* !/app/assets/builds/.keep +console_history.txt diff --git a/app/models/document_collection_non_whitehall_link/govuk_url.rb b/app/models/document_collection_non_whitehall_link/govuk_url.rb index d575082b658..011cfdebd0a 100644 --- a/app/models/document_collection_non_whitehall_link/govuk_url.rb +++ b/app/models/document_collection_non_whitehall_link/govuk_url.rb @@ -31,24 +31,24 @@ def parsed_url end def content_item - @content_item ||= Services.publishing_api.get_content(content_id).to_h + @content_item ||= content_item_from_content_store end def content_id - @content_id ||= Services.publishing_api.lookup_content_id(base_path: parsed_url.path, with_drafts: true) - - if @content_id.blank? - toplevel_path_segment = parsed_url.path.split("/").second - @content_id = Services.publishing_api.lookup_content_id(base_path: "/#{toplevel_path_segment}", with_drafts: true) - if @content_id.blank? - raise GdsApi::HTTPNotFound, 404 - else - unless content_item["document_type"] == "guide" - raise GdsApi::HTTPNotFound, 404 - end - end + @content_id ||= content_item["content_id"] + end + + def content_item_from_content_store + path = parsed_url.path + + item = Services.content_store.content_item(path).to_h + + if item["base_path"] != path && item["document_type"] != "guide" + raise GdsApi::HTTPNotFound, 404 end - @content_id + item + rescue GdsApi::ContentStore::ItemNotFound + raise GdsApi::HTTPNotFound, 404 end end diff --git a/features/step_definitions/document_collection_steps.rb b/features/step_definitions/document_collection_steps.rb index 83cf4d4d391..c58690b35cb 100644 --- a/features/step_definitions/document_collection_steps.rb +++ b/features/step_definitions/document_collection_steps.rb @@ -157,7 +157,8 @@ content_id = SecureRandom.uuid stub_publishing_api_has_lookups(base_path => content_id) - stub_publishing_api_has_item( + stub_content_store_has_item( + base_path, content_id:, base_path:, publishing_app: "content-publisher", diff --git a/lib/services.rb b/lib/services.rb index c650126b17e..08e899db84d 100644 --- a/lib/services.rb +++ b/lib/services.rb @@ -1,5 +1,6 @@ require "gds_api/publishing_api" require "gds_api/asset_manager" +require "gds_api/content_store" require "gds_api/search" module Services @@ -11,6 +12,10 @@ def self.publishing_api_with_huge_timeout @publishing_api_with_huge_timeout ||= publishing_api_client_with_timeout(60) end + def self.content_store + @content_store ||= GdsApi::ContentStore.new(Plek.find("content-store")) + end + def self.asset_manager @asset_manager ||= GdsApi::AssetManager.new( Plek.find("asset-manager"), diff --git a/test/unit/app/models/document_collection_non_whitehall_link/govuk_url_test.rb b/test/unit/app/models/document_collection_non_whitehall_link/govuk_url_test.rb index 93bb65ddebc..0367011d6e0 100644 --- a/test/unit/app/models/document_collection_non_whitehall_link/govuk_url_test.rb +++ b/test/unit/app/models/document_collection_non_whitehall_link/govuk_url_test.rb @@ -10,6 +10,12 @@ class DocumentCollectionNonWhitehallLink::GovukUrlTest < ActiveSupport::TestCase base_path: "/test", publishing_app: "content-publisher", ) + Services.content_store.stubs(:content_item).with("/test").returns( + "content_id" => @content_id, + "title" => "Test", + "base_path" => "/test", + "publishing_app" => "content-publisher", + ) end test "should be valid without a GOV.UK url that Publishing API knows" do @@ -33,11 +39,13 @@ class DocumentCollectionNonWhitehallLink::GovukUrlTest < ActiveSupport::TestCase test "should be valid when a mainstream guide sub-page url is used" do content_id = SecureRandom.uuid stub_publishing_api_has_lookups("/foo" => content_id) - stub_publishing_api_has_item(content_id:, - title: "Foo Bar", - base_path: "/foo", - document_type: "guide", - publishing_app: "content-publisher") + Services.content_store.stubs(:content_item).with("/foo/subpage").returns( + "content_id" => content_id, + "title" => "Foo Bar", + "base_path" => "/foo", + "publishing_app" => "content-publisher", + "document_type" => "guide", + ) url = DocumentCollectionNonWhitehallLink::GovukUrl.new( url: "https://www.gov.uk/foo/subpage", @@ -91,12 +99,41 @@ class DocumentCollectionNonWhitehallLink::GovukUrlTest < ActiveSupport::TestCase document_collection_group: build(:document_collection_group), ) + Services.content_store.stubs(:content_item).raises(GdsApi::ContentStore::ItemNotFound.new(404)) + assert_not url.valid? assert url.errors.full_messages.include?("Url must reference a GOV.UK page") end + test "should be valid when a Welsh-language GOV.UK URL is used that is not in the Publishing API path reservations" do + welsh_content_id = SecureRandom.uuid + content_store_response = { "content_id" => welsh_content_id, "title" => "Talu cosb hunanasesiad", "base_path" => "/talu-cosb-hunanasesiad", "publishing_app" => "publisher" } + Services.content_store.stubs(:content_item).with("/talu-cosb-hunanasesiad").returns(content_store_response) + + url = DocumentCollectionNonWhitehallLink::GovukUrl.new( + url: "https://www.gov.uk/talu-cosb-hunanasesiad", + document_collection_group: build(:document_collection_group), + ) + + assert url.valid? + end + + test "should be valid when a Welsh-language GOV.UK URL is in the Publishing API but only has a Welsh locale" do + welsh_content_id = SecureRandom.uuid + stub_publishing_api_has_lookups("/talu-treth-twe" => welsh_content_id) + content_store_response = { "content_id" => welsh_content_id, "title" => "Talu TWE y cyflogwr", "base_path" => "/talu-treth-twe", "publishing_app" => "publisher", "locale" => "cy" } + Services.content_store.stubs(:content_item).with("/talu-treth-twe").returns(content_store_response) + + url = DocumentCollectionNonWhitehallLink::GovukUrl.new( + url: "https://www.gov.uk/talu-treth-twe", + document_collection_group: build(:document_collection_group), + ) + + assert url.valid? + end + test "should be invalid when Publishing API returns a 404" do - stub_any_publishing_api_call_to_return_not_found + Services.content_store.stubs(:content_item).raises(GdsApi::ContentStore::ItemNotFound.new(404)) url = DocumentCollectionNonWhitehallLink::GovukUrl.new( url: "https://www.gov.uk/test", @@ -110,11 +147,13 @@ class DocumentCollectionNonWhitehallLink::GovukUrlTest < ActiveSupport::TestCase test "should be invalid when a non-mainstream guide sub-page url is used" do content_id = SecureRandom.uuid stub_publishing_api_has_lookups("/foo" => content_id) - stub_publishing_api_has_item(content_id:, - title: "Foo Bar", - base_path: "/foo", - document_type: "other", - publishing_app: "content-publisher") + Services.content_store.stubs(:content_item).with("/foo/subpage").returns( + "content_id" => content_id, + "title" => "Foo Bar", + "base_path" => "/foo", + "publishing_app" => "content-publisher", + "document_type" => "other", + ) url = DocumentCollectionNonWhitehallLink::GovukUrl.new( url: "https://www.gov.uk/foo/subpage", @@ -125,7 +164,7 @@ class DocumentCollectionNonWhitehallLink::GovukUrlTest < ActiveSupport::TestCase end test "should be invalid when Publishing API is down" do - stub_publishing_api_isnt_available + Services.content_store.stubs(:content_item).raises(GdsApi::HTTPIntermittentServerError.new(503)) url = DocumentCollectionNonWhitehallLink::GovukUrl.new( url: "https://www.gov.uk/test",