From d98cedcbaaa0b48dfd3f1317d030c6d2fbc9a54c Mon Sep 17 00:00:00 2001 From: pezholio Date: Fri, 29 Nov 2024 11:59:25 +0000 Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20create=20edition=20in=20Publish?= =?UTF-8?q?ing=20API=20initially?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In https://github.com/alphagov/whitehall/pull/9509 we decided to create draft editions in Publishing API as soon as they were created at our end on the assumption that we’d need the content block to be in the draft content store for previewing to work. However, we’ve since discovered that if a user creates a Document with an initial draft that they do not publish, this gets deleted by the `DeleteDraftContentBlocksWorker` without deleting the draft in Publishing API. This causes a discrepancy between the Content Block Manager and Publishing API. Additionally, the way we handle previews doesn’t involve the draft content store at all, so instead of making the job to delete drafts more complex, we remove the call the `create_draft_edition` altogether and wait until the end of the journey to send the block to Publishing API --- .../lib/content_block_manager/publishable.rb | 25 ---------- .../create_edition_service.rb | 9 ++-- .../content_block_manager/publishable_test.rb | 47 ------------------- .../services/create_edition_service_test.rb | 35 -------------- 4 files changed, 3 insertions(+), 113 deletions(-) delete mode 100644 lib/engines/content_block_manager/test/unit/app/lib/content_block_manager/publishable_test.rb diff --git a/lib/engines/content_block_manager/app/lib/content_block_manager/publishable.rb b/lib/engines/content_block_manager/app/lib/content_block_manager/publishable.rb index ffb3fcb4ac3..9d8292c88ea 100644 --- a/lib/engines/content_block_manager/app/lib/content_block_manager/publishable.rb +++ b/lib/engines/content_block_manager/app/lib/content_block_manager/publishable.rb @@ -41,31 +41,6 @@ def schedule_with_rollback end end - def create_draft_edition(schema) - raise ArgumentError, "Local database changes not given" unless block_given? - - ActiveRecord::Base.transaction do - content_block_edition = yield - content_id = content_block_edition.document.content_id - content_id_alias = content_block_edition.document.content_id_alias - organisation_id = content_block_edition.lead_organisation.content_id - - create_publishing_api_edition( - content_id:, - content_id_alias:, - schema_id: schema.id, - title: content_block_edition.title, - instructions_to_publishers: content_block_edition.instructions_to_publishers, - details: content_block_edition.details.to_h, - links: { - primary_publishing_organisation: [ - organisation_id, - ], - }, - ) - end - end - def update_content_block_document(new_content_block_edition:, update_document_params:) # Updates to a Document should never change its block type update_document_params.delete(:block_type) diff --git a/lib/engines/content_block_manager/app/services/content_block_manager/create_edition_service.rb b/lib/engines/content_block_manager/app/services/content_block_manager/create_edition_service.rb index 44096ce8b91..6c2856b3105 100644 --- a/lib/engines/content_block_manager/app/services/content_block_manager/create_edition_service.rb +++ b/lib/engines/content_block_manager/app/services/content_block_manager/create_edition_service.rb @@ -7,12 +7,9 @@ def initialize(schema) end def call(edition_params, document_id: nil) - create_draft_edition(@schema) do - @new_edition = build_edition(edition_params, document_id) - @new_edition.assign_attributes(edition_params) - @new_edition.save! - @new_edition - end + @new_edition = build_edition(edition_params, document_id) + @new_edition.assign_attributes(edition_params) + @new_edition.save! @new_edition end diff --git a/lib/engines/content_block_manager/test/unit/app/lib/content_block_manager/publishable_test.rb b/lib/engines/content_block_manager/test/unit/app/lib/content_block_manager/publishable_test.rb deleted file mode 100644 index 992db9d35b4..00000000000 --- a/lib/engines/content_block_manager/test/unit/app/lib/content_block_manager/publishable_test.rb +++ /dev/null @@ -1,47 +0,0 @@ -require "test_helper" - -class ContentBlockManager::PublishableTest < ActiveSupport::TestCase - extend Minitest::Spec::DSL - - class PublishableTestClass - include ContentBlockManager::Publishable - end - - describe "#create_draft_edition" do - let(:schema) { build(:content_block_schema) } - let(:content_block_edition) { build(:content_block_edition, :email_address) } - - let(:publishable_test_instance) { PublishableTestClass.new } - - it "raises an error if a block isn't passed since changes need to be made locally" do - assert_raises ArgumentError, "Local database changes not given" do - publishable_test_instance.create_draft_edition(schema) - end - end - - it "creates a draft edition" do - Services.publishing_api.expects(:put_content).with( - content_block_edition.document.content_id, - { - schema_name: schema.id, - document_type: schema.id, - publishing_app: Whitehall::PublishingApp::WHITEHALL, - title: content_block_edition.title, - content_id_alias: content_block_edition.document.content_id_alias, - details: content_block_edition.details, - instructions_to_publishers: content_block_edition.instructions_to_publishers, - links: { - primary_publishing_organisation: [ - content_block_edition.lead_organisation.content_id, - ], - }, - update_type: "major", - }, - ) - - publishable_test_instance.create_draft_edition(schema) do - content_block_edition - end - end - end -end diff --git a/lib/engines/content_block_manager/test/unit/app/services/create_edition_service_test.rb b/lib/engines/content_block_manager/test/unit/app/services/create_edition_service_test.rb index 216f0a1c784..1665fab023c 100644 --- a/lib/engines/content_block_manager/test/unit/app/services/create_edition_service_test.rb +++ b/lib/engines/content_block_manager/test/unit/app/services/create_edition_service_test.rb @@ -56,12 +56,6 @@ class ContentBlockManager::CreateEditionServiceTest < ActiveSupport::TestCase assert_equal new_edition.lead_organisation.id, organisation.id end - it "sends the content block to the Publishing API as a draft" do - assert_draft_created_in_publishing_api(content_id:, content_id_alias: new_title.parameterize) do - ContentBlockManager::CreateEditionService.new(schema).call(edition_params) - end - end - describe "when a document id is provided" do let(:document) { create(:content_block_document, :email_address) } let!(:previous_edition) { create(:content_block_edition, :email_address, document:) } @@ -80,35 +74,6 @@ class ContentBlockManager::CreateEditionServiceTest < ActiveSupport::TestCase assert_equal new_edition.document_id, document.id assert_equal new_edition.lead_organisation.id, organisation.id end - - it "sends the content block to the Publishing API as a draft" do - assert_draft_created_in_publishing_api(content_id: document.content_id, content_id_alias: document.content_id_alias) do - ContentBlockManager::CreateEditionService.new(schema).call(edition_params, document_id: document.id) - end - end end end end - -def assert_draft_created_in_publishing_api(content_id:, content_id_alias:, &block) - Services.publishing_api.expects(:put_content).with( - content_id, - { - schema_name: schema.id, - document_type: schema.id, - publishing_app: Whitehall::PublishingApp::WHITEHALL, - title: new_title, - content_id_alias:, - details: edition_params[:details], - instructions_to_publishers: edition_params[:instructions_to_publishers], - links: { - primary_publishing_organisation: [ - organisation.content_id, - ], - }, - update_type: "major", - }, - ) - - block.call -end