Skip to content

Commit bb90ade

Browse files
authored
Merge pull request #10912 from alphagov/avoid-accidentally-overwriting-base-path
Check if existing base path is associated with same content ID [WHIT-1842]
2 parents a573806 + 9de162c commit bb90ade

4 files changed

Lines changed: 44 additions & 1 deletion

File tree

features/support/publishing_api.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
.to_return(body: { links: {} }.to_json)
2020
stub_request(:get, %r{\A#{Plek.find('publishing-api')}/v2/expanded-links})
2121
.to_return(body: { expanded_links: {} }.to_json)
22+
# Prevent publishing API base path checks from interfering with tests
23+
Whitehall::PublishingApi.stubs(:ensure_base_path_is_associated_with_this_content_id!).returns(nil)
2224
end
2325

2426
World(GdsApi::TestHelpers::PublishingApi)

lib/whitehall/publishing_api.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ def self.save_draft_translation(
6969

7070
content.merge!(bulk_publishing: true) if bulk_publishing
7171

72+
ensure_base_path_is_associated_with_this_content_id!(content[:base_path], model_instance.content_id)
73+
7274
Services.publishing_api.put_content(presenter.content_id, content)
7375
end
7476
rescue GdsApi::HTTPUnprocessableEntity => e
@@ -178,5 +180,12 @@ def self.assert_public_edition!(instance)
178180
raise UnpublishableInstanceError, "#{instance.class} with ID #{instance.id} is not publishable"
179181
end
180182
end
183+
184+
def self.ensure_base_path_is_associated_with_this_content_id!(base_path, content_id)
185+
existing_content_id = Services.publishing_api.lookup_content_id(base_path:)
186+
return if existing_content_id.nil? || existing_content_id == content_id
187+
188+
raise UnpublishableInstanceError, "Cannot save draft (content_id #{content_id}). There is existing content at the '#{base_path}' route, under a different content_id (#{existing_content_id}). Try changing your title to resolve the conflict."
189+
end
181190
end
182191
end

test/test_helper.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ class ActiveSupport::TestCase
7676
Services.publishing_api.stubs(:get_events_for_content_id).returns([])
7777
Services.stubs(:asset_manager).returns(stub_everything("asset-manager"))
7878
TaxonValidator.any_instance.stubs(:validate)
79+
# Prevent publishing API base path checks from interfering with tests
80+
Whitehall::PublishingApi.stubs(:ensure_base_path_is_associated_with_this_content_id!).returns(nil)
7981
end
8082

8183
teardown do

test/unit/lib/whitehall/publishing_api_test.rb

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,16 +274,46 @@ class Whitehall::PublishingApiTest < ActiveSupport::TestCase
274274
assert_equal english_path, PublishingApiUnscheduleWorker.jobs[1]["args"].first
275275
end
276276

277-
test ".save_draft publishes a draft edition" do
277+
test ".save_draft publishes a draft edition if no content exists at the route yet" do
278278
draft_edition = create(:draft_case_study)
279279
payload = PublishingApiPresenters.presenter_for(draft_edition)
280280
request = stub_publishing_api_put_content(payload.content_id, payload.content)
281+
Whitehall::PublishingApi.unstub(:ensure_base_path_is_associated_with_this_content_id!)
282+
Services.publishing_api.expects(:lookup_content_id).with(base_path: draft_edition.public_path).returns(nil)
281283

282284
Whitehall::PublishingApi.save_draft(draft_edition)
283285

284286
assert_requested request
285287
end
286288

289+
test ".save_draft publishes a draft edition if there is a live content item with the same base path and same content ID" do
290+
draft_edition = create(:draft_case_study)
291+
payload = PublishingApiPresenters.presenter_for(draft_edition)
292+
request = stub_publishing_api_put_content(payload.content_id, payload.content)
293+
Whitehall::PublishingApi.unstub(:ensure_base_path_is_associated_with_this_content_id!)
294+
Services.publishing_api.expects(:lookup_content_id).with(base_path: draft_edition.public_path).returns(payload.content_id)
295+
296+
Whitehall::PublishingApi.save_draft(draft_edition)
297+
298+
assert_requested request
299+
end
300+
301+
test ".save_draft raises exception if there is a live content item with the same base path but different content ID" do
302+
ConfigurableDocumentType.setup_test_types(build_configurable_document_type("test_type"))
303+
edition = create(:draft_standard_edition)
304+
Whitehall::PublishingApi.unstub(:ensure_base_path_is_associated_with_this_content_id!)
305+
Services.publishing_api.expects(:lookup_content_id).with(base_path: edition.public_path).returns("different-content-id")
306+
307+
error = assert_raises Whitehall::UnpublishableInstanceError do
308+
Whitehall::PublishingApi.save_draft(edition)
309+
end
310+
311+
assert_equal(
312+
"Cannot save draft (content_id #{edition.content_id}). There is existing content at the '#{edition.public_path}' route, under a different content_id (different-content-id). Try changing your title to resolve the conflict.",
313+
error.message,
314+
)
315+
end
316+
287317
test ".publish_redirect_async publishes a redirect to the Publishing API" do
288318
document = create(:document)
289319
destination = "/government/people/milli-vanilli"

0 commit comments

Comments
 (0)