Merge pull request #9511 from alphagov/content-modelling/chore/tidy-u…

…p-publish-service

Tidy PublishEditionService

lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/workflow_controller.rb

@@ -82,7 +82,7 @@ def schedule_or_publish
   end
 
   def publish
-    new_edition = ContentBlockManager::PublishEditionService.new(@schema).call(@content_block_edition)
+    new_edition = ContentBlockManager::PublishEditionService.new.call(@content_block_edition)
     redirect_to content_block_manager.content_block_manager_content_block_document_path(new_edition.document),
                 flash: { notice: "#{new_edition.block_type.humanize} created successfully" }
   end

lib/engines/content_block_manager/app/lib/content_block_manager/publishable.rb

@@ -2,32 +2,28 @@ module ContentBlockManager
   module Publishable
     class PublishingFailureError < StandardError; end
 
-    def publish_with_rollback(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
-        organisation_id = content_block_edition.lead_organisation.content_id
-
-        create_publishing_api_edition(
-          content_id:,
-          schema_id: schema.id,
-          title: content_block_edition.title,
-          details: content_block_edition.details,
-          links: {
-            primary_publishing_organisation: [
-              organisation_id,
-            ],
-          },
-        )
-        publish_publishing_api_edition(content_id:)
-        update_content_block_document_with_latest_edition(content_block_edition)
-        content_block_edition.public_send(:publish!)
-      rescue PublishingFailureError => e
-        discard_publishing_api_edition(content_id:)
-        raise e
-      end
+    def publish_with_rollback(content_block_edition)
+      document = content_block_edition.document
+      content_id = document.content_id
+
+      create_publishing_api_edition(
+        content_id:,
+        schema_id: document.block_type,
+        title: content_block_edition.title,
+        details: content_block_edition.details,
+        links: {
+          primary_publishing_organisation: [
+            content_block_edition.lead_organisation.content_id,
+          ],
+        },
+      )
+      publish_publishing_api_edition(content_id:)
+      update_content_block_document_with_latest_edition(content_block_edition)
+      content_block_edition.public_send(:publish!)
+      content_block_edition
+    rescue PublishingFailureError => e
+      discard_publishing_api_edition(content_id:)
+      raise e
     end
 
     def schedule_with_rollback

lib/engines/content_block_manager/app/services/content_block_manager/publish_edition_service.rb

@@ -2,15 +2,8 @@ module ContentBlockManager
   class PublishEditionService
     include Publishable
 
-    def initialize(schema)
-      @schema = schema
-    end
-
     def call(edition)
-      publish_with_rollback(@schema) do
-        edition
-      end
-      edition
+      publish_with_rollback(edition)
     end
   end
 end

lib/engines/content_block_manager/app/workers/content_block_manager/schedule_publishing_worker.rb

@@ -46,12 +46,8 @@ def perform(edition_id)
       edition = ContentBlockManager::ContentBlock::Edition.find(edition_id)
       return if edition.published? || !edition.scheduled?
 
-      schema = ContentBlockManager::ContentBlock::Schema.find_by_block_type(edition.document.block_type)
-
       ContentBlockManager::ContentBlock::Edition::HasAuditTrail.acting_as(publishing_robot) do
-        ContentBlockManager::PublishEditionService.new(
-          schema,
-        ).call(edition)
+        ContentBlockManager::PublishEditionService.new.call(edition)
       end
     rescue ContentBlockManager::Publishable::PublishingFailureError => e
       raise SchedulingFailure, e.message

lib/engines/content_block_manager/test/integration/content_block/workflow_test.rb

@@ -147,8 +147,8 @@ def assert_edition_is_published(&block)
   publishing_api_mock.expect :put_content, fake_put_content_response, [
     @content_id,
     {
-      schema_name: "content_block_type",
-      document_type: "content_block_type",
+      schema_name: document.block_type,
+      document_type: document.block_type,
       publishing_app: "whitehall",
       title: "Some Title",
       details: {

lib/engines/content_block_manager/test/unit/app/lib/content_block_manager/publishable_test.rb

@@ -7,19 +7,6 @@ class PublishableTestClass
     include ContentBlockManager::Publishable
   end
 
-  describe "#publish_with_rollback" do
-    it "raises an error if a block isn't passed since changes need to be made locally" do
-      anything = Object.new
-      test_instance = PublishableTestClass.new
-
-      assert_raises ArgumentError, "Local database changes not given" do
-        test_instance.publish_with_rollback(
-          schema: anything, title: anything, details: anything,
-        )
-      end
-    end
-  end
-
   describe "#create_draft_edition" do
     let(:schema) { build(:content_block_schema) }
     let(:content_block_edition) { build(:content_block_edition, :email_address) }

lib/engines/content_block_manager/test/unit/app/services/publish_edition_service_test.rb

@@ -5,29 +5,25 @@ class ContentBlockManager::PublishEditionServiceTest < ActiveSupport::TestCase
 
   describe "#call" do
     let(:content_id) { "49453854-d8fd-41da-ad4c-f99dbac601c3" }
-    let(:schema) { build(:content_block_schema, block_type: "content_block_type", body: { "properties" => { "foo" => "", "bar" => "" } }) }
     let(:document) { create(:content_block_document, :email_address, content_id:, title: "Some Title") }
     let(:edition) { create(:content_block_edition, document:, details: { "foo" => "Foo text", "bar" => "Bar text" }, organisation: @organisation) }
 
     setup do
-      ContentBlockManager::ContentBlock::Schema.stubs(:find_by_block_type)
-                                              .returns(schema)
       @organisation = create(:organisation)
     end
 
-    test "returns a ContentBlockEdition" do
-      result = ContentBlockManager::PublishEditionService.new(schema).call(edition)
+    it "returns a ContentBlockEdition" do
+      result = ContentBlockManager::PublishEditionService.new.call(edition)
       assert_instance_of ContentBlockManager::ContentBlock::Edition, result
     end
 
-    test "it publishes the Edition in Whitehall" do
-      ContentBlockManager::PublishEditionService.new(schema).call(edition)
-
+    it "publishes the Edition in Whitehall" do
+      ContentBlockManager::PublishEditionService.new.call(edition)
       assert_equal "published", edition.state
       assert_equal edition.id, document.live_edition_id
     end
 
-    test "it creates an Edition in the Publishing API" do
+    it "creates an Edition in the Publishing API" do
       fake_put_content_response = GdsApi::Response.new(
         stub("http_response", code: 200, body: {}),
       )
@@ -39,8 +35,8 @@ class ContentBlockManager::PublishEditionServiceTest < ActiveSupport::TestCase
       publishing_api_mock.expect :put_content, fake_put_content_response, [
         content_id,
         {
-          schema_name: schema.id,
-          document_type: schema.id,
+          schema_name: document.block_type,
+          document_type: document.block_type,
           publishing_app: "whitehall",
           title: "Some Title",
           details: {
@@ -58,15 +54,15 @@ class ContentBlockManager::PublishEditionServiceTest < ActiveSupport::TestCase
       ]
 
       Services.stub :publishing_api, publishing_api_mock do
-        ContentBlockManager::PublishEditionService.new(schema).call(edition)
+        ContentBlockManager::PublishEditionService.new.call(edition)
 
         publishing_api_mock.verify
         assert_equal "published", edition.state
         assert_equal edition.id, document.live_edition_id
       end
     end
 
-    test "if the publishing API request fails, the Whitehall ContentBlockEdition and ContentBlockDocument are rolled back" do
+    it "rolls back the Whitehall ContentBlockEdition and ContentBlockDocument if the publishing API request fails" do
       exception = GdsApi::HTTPErrorResponse.new(
         422,
         "An internal error message",
@@ -79,14 +75,14 @@ class ContentBlockManager::PublishEditionServiceTest < ActiveSupport::TestCase
 
       Services.publishing_api.stub :put_content, raises_exception do
         assert_raises(GdsApi::HTTPErrorResponse) do
-          ContentBlockManager::PublishEditionService.new(schema).call(edition)
+          ContentBlockManager::PublishEditionService.new.call(edition)
         end
         assert_equal "draft", edition.state
         assert_nil document.live_edition_id
       end
     end
 
-    test "if the publish request fails, the latest draft is discarded and the database actions are rolled back" do
+    it "discards the latest draft if the publish request fails" do
       fake_put_content_response = GdsApi::Response.new(
         stub("http_response", code: 200, body: {}),
       )
@@ -112,7 +108,7 @@ class ContentBlockManager::PublishEditionServiceTest < ActiveSupport::TestCase
 
       Services.publishing_api.stub :publish, raises_exception do
         assert_raises(ContentBlockManager::PublishEditionService::PublishingFailureError, "Could not publish #{content_id} because: Some backend error") do
-          ContentBlockManager::PublishEditionService.new(schema).call(edition)
+          ContentBlockManager::PublishEditionService.new.call(edition)
           publishing_api_mock.verify
         end
         assert_equal "draft", edition.state

lib/engines/content_block_manager/test/unit/app/services/schedule_edition_service_test.rb

@@ -75,9 +75,7 @@ class ContentBlockManager::ScheduleEditionServiceTest < ActiveSupport::TestCase
 
       edition.stub :update!, raises_exception do
         assert_raises(ArgumentError) do
-          ContentBlockManager::ScheduleEditionService
-            .new(schema)
-            .call(edition, scheduled_publication_params)
+          ContentBlockManager::ScheduleEditionService.new.call(edition, scheduled_publication_params)
         end
       end
     end