Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions app/models/document_collection_non_whitehall_link/govuk_url.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ def parsed_url
end

def content_item
@content_item ||= Services.publishing_api.get_content(content_id).to_h
content_id
@content_item ||= content_item_from_content_store
Comment on lines +34 to +35
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kind of 'side effect' method (content_id) isn't very readable. Probably best being more open that one is dependent on the other:

Suggested change
content_id
@content_item ||= content_item_from_content_store
@content_item ||= content_item_from_content_store(content_id)

end

def content_id
Expand All @@ -40,15 +41,21 @@ def content_id
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
@content_id = content_item_from_content_store["content_id"]
raise GdsApi::HTTPNotFound, 404 if @content_id.blank?
else
unless content_item["document_type"] == "guide"
raise GdsApi::HTTPNotFound, 404
end
raise GdsApi::HTTPNotFound, 404 unless content_item["document_type"] == "guide"
end
end

@content_id
end

def content_item_from_content_store
Services.content_store.content_item(parsed_url.path).to_h
rescue GdsApi::ContentStore::ItemNotFound
raise GdsApi::HTTPNotFound, 404
end
end
3 changes: 2 additions & 1 deletion features/step_definitions/document_collection_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 5 additions & 0 deletions lib/services.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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",
Expand Down Expand Up @@ -91,10 +99,39 @@ 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

Expand All @@ -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",
Expand Down