Check if existing base path is associated with same content ID

WHAT

This commit adds logic at the point of saving a draft to
Publishing API. The logic checks whether the base path being
saved to already has a live document at the end of it, and if
so, if that document's content ID matches what we're sending.
If it doesn't match, then that indicates that a different
document (in Whitehall, or originating from another app) already
owns the route and we've ended up in the not great situation of
having allocated a slug that already exists. We raise an
exception in this case, which is better than proceeding and
letting the document silently overwrite an existing one.

The lookup only checks for _published_ documents, as we haven't
passed the `with_drafts: true` keyword arg. This is deliberate
as we wouldn't want a situation of a publisher creating a draft
document Foo, deleting it, then going and creating another draft
document Foo and seeing an error on save. We only care about
base paths that are already live. This does mean there could
be an edge case where two draft documents have the same slug
and we only get the exception when trying to save the second
one after publishing the first one, but that seems a very low
likelihood edge case.

Note also that this logic change depends on Whitehall always
saving drafts via the `Whitehall::PublishingApi` helper module.
There are a few cases where Whitehall updates Publishing API
outside of this module (by making calls to
`Services.publishing_api.save_draft` instead), but these all
seem to be for index pages and the like, where the content ID
is always fixed, so refactoring the Publishing API calls to
all go through the helper module is outside of the scope of this
change.

WHY

It has long been possible for Whitehall to create a draft which
has the same base path (`/government/foo`) as an already live
page that was published by another publishing app. The draft has
been previewable, and it's even been possible to schedule the
draft for publication. Only at the point of Whitehall making a
'publish' call to Publishing API - either manually via the UI, or
via the scheduled publishing worker - would Publishing API then
raise an exception and cause Whitehall to raise a
`Whitehall::UnpublishableInstanceError`.

Historically this hasn't caused too many issues, except for
occasional news article clashes with the now retired Content
Publisher application. Most publishing apps publish different
content types with different base path prefixes, thus there is
never usually a conflict.

_Internal_ base path conflicts - where Whitehall alone is responsible
for producing two different documents under the same base path -
have so far been avoided by a uniqueness constraint on the
Document table, ensuring that every slug is unique amongst other
slugs of the same document type:
https://github.com/alphagov/whitehall/blob/e024e2eea25ca9cb48a8a98d2298aaea1df13b92/db/schema.rb#L241

This uniqueness constraint only works on the assumption that every
document type has a distinct base path prefix (e.g.
`/government/news` for NewsArticle, `/government/consultations`
for Consultation, and so on). That assumption no longer holds true
as we are gradually migrating content types to use the same shared
StandardEdition model (with a dynamic base path prefix based on the
`configurable_document_type` of the Edition).

The consequence of this is that one may have a legacy NewsArticle
with generated base path `/government/news/foo` and a StandardEdition
document with generated base path `/government/news/foo`, and the
uniqueness constraint passes because both are of different document
types.

There are several problems to solve here:

1. A newer StandardEdition document might silently overwrite an
   older legacy Edition which has the same base path.
2. Our slug enumeration logic (that automatically appends `--1`
   in the event of a clash) does not apply across document types.
3. Even if the slug enumeration logic DID apply, that wouldn't be
   desirable. It should be allowable to have a stanard-edition
   flavoured news article of path `/government/news/foo` alongside
   a standard-edition flavoured consultation of path
   `/government/consultations/foo` without one of them being relegated
   the slug `foo--1`.

The first and most important problem is the silent overwriting.
These cases are (currently) very few and far between, and the best
approach is to capture it as an exception where we can help the
user out as a support task. Further down the line we can capture
the exception and give the user an actionable error message instead
(i.e. to change their title), or potentially explore enumerating the
slug to avoid conflicts automatically.

We'll need to think about whether the uniqueness constraint index
is still valuable or whether this dynamic lookup of Publishing API
content items is a sufficient replacement.

Jira: https://gov-uk.atlassian.net/browse/WHIT-1842

Author: ChrisBAshton

features/support/publishing_api.rb

@@ -19,6 +19,8 @@
     .to_return(body: { links: {} }.to_json)
   stub_request(:get, %r{\A#{Plek.find('publishing-api')}/v2/expanded-links})
     .to_return(body: { expanded_links: {} }.to_json)
+  # Prevent publishing API base path checks from interfering with tests
+  Whitehall::PublishingApi.stubs(:ensure_base_path_is_associated_with_this_content_id!).returns(nil)
 end
 
 World(GdsApi::TestHelpers::PublishingApi)

lib/whitehall/publishing_api.rb

@@ -69,6 +69,8 @@ def self.save_draft_translation(
 
         content.merge!(bulk_publishing: true) if bulk_publishing
 
+        ensure_base_path_is_associated_with_this_content_id!(content[:base_path], model_instance.content_id)
+
         Services.publishing_api.put_content(presenter.content_id, content)
       end
     rescue GdsApi::HTTPUnprocessableEntity => e
@@ -178,5 +180,12 @@ def self.assert_public_edition!(instance)
         raise UnpublishableInstanceError, "#{instance.class} with ID #{instance.id} is not publishable"
       end
     end
+
+    def self.ensure_base_path_is_associated_with_this_content_id!(base_path, content_id)
+      existing_content_id = Services.publishing_api.lookup_content_id(base_path:)
+      return if existing_content_id.nil? || existing_content_id == content_id
+
+      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."
+    end
   end
 end

test/test_helper.rb

@@ -76,6 +76,8 @@ class ActiveSupport::TestCase
     Services.publishing_api.stubs(:get_events_for_content_id).returns([])
     Services.stubs(:asset_manager).returns(stub_everything("asset-manager"))
     TaxonValidator.any_instance.stubs(:validate)
+    # Prevent publishing API base path checks from interfering with tests
+    Whitehall::PublishingApi.stubs(:ensure_base_path_is_associated_with_this_content_id!).returns(nil)
   end
 
   teardown do

test/unit/lib/whitehall/publishing_api_test.rb

@@ -274,16 +274,46 @@ class Whitehall::PublishingApiTest < ActiveSupport::TestCase
     assert_equal english_path, PublishingApiUnscheduleWorker.jobs[1]["args"].first
   end
 
-  test ".save_draft publishes a draft edition" do
+  test ".save_draft publishes a draft edition if no content exists at the route yet" do
     draft_edition = create(:draft_case_study)
     payload = PublishingApiPresenters.presenter_for(draft_edition)
     request = stub_publishing_api_put_content(payload.content_id, payload.content)
+    Whitehall::PublishingApi.unstub(:ensure_base_path_is_associated_with_this_content_id!)
+    Services.publishing_api.expects(:lookup_content_id).with(base_path: draft_edition.public_path).returns(nil)
 
     Whitehall::PublishingApi.save_draft(draft_edition)
 
     assert_requested request
   end
 
+  test ".save_draft publishes a draft edition if there is a live content item with the same base path and same content ID" do
+    draft_edition = create(:draft_case_study)
+    payload = PublishingApiPresenters.presenter_for(draft_edition)
+    request = stub_publishing_api_put_content(payload.content_id, payload.content)
+    Whitehall::PublishingApi.unstub(:ensure_base_path_is_associated_with_this_content_id!)
+    Services.publishing_api.expects(:lookup_content_id).with(base_path: draft_edition.public_path).returns(payload.content_id)
+
+    Whitehall::PublishingApi.save_draft(draft_edition)
+
+    assert_requested request
+  end
+
+  test ".save_draft raises exception if there is a live content item with the same base path but different content ID" do
+    ConfigurableDocumentType.setup_test_types(build_configurable_document_type("test_type"))
+    edition = create(:draft_standard_edition)
+    Whitehall::PublishingApi.unstub(:ensure_base_path_is_associated_with_this_content_id!)
+    Services.publishing_api.expects(:lookup_content_id).with(base_path: edition.public_path).returns("different-content-id")
+
+    error = assert_raises Whitehall::UnpublishableInstanceError do
+      Whitehall::PublishingApi.save_draft(edition)
+    end
+
+    assert_equal(
+      "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.",
+      error.message,
+    )
+  end
+
   test ".publish_redirect_async publishes a redirect to the Publishing API" do
     document = create(:document)
     destination = "/government/people/milli-vanilli"