Merge pull request #8780 from alphagov/remove-unpublishing-step-when-…

…republshing-unpublished-or-withdraw

Remove the publishing step when trying to republish HTML attachments for unpublished documents

app/services/service_listeners/publishing_api_html_attachments.rb

@@ -18,8 +18,20 @@ def publish
     alias_method :unwithdraw, :publish
 
     def republish
-      update_publishing_api_content
-      unpublish_if_required
+      if Edition::PRE_PUBLICATION_STATES.include?(edition.state)
+        update_draft(update_type: "republish")
+      elsif edition.unpublishing && edition.withdrawn?
+        do_publish("republish")
+        discard_drafts(deleted_html_attachments)
+        withdraw
+      elsif edition.unpublishing
+        update_draft(update_type: "republish")
+        patch_links
+        unpublish(allow_draft: true)
+      else
+        do_publish("republish")
+        discard_drafts(deleted_html_attachments)
+      end
     end
 
     def update_draft(update_type: nil)
@@ -32,7 +44,6 @@ def update_draft(update_type: nil)
       end
       discard_drafts(deleted_html_attachments)
     end
-
     # We don't care whether this is a translation or the main
     # document, we just send the correct html attachments regardless.
     alias_method :update_draft_translation, :update_draft
@@ -81,22 +92,9 @@ def discard_drafts(html_attachments)
       end
     end
 
-    def update_publishing_api_content
-      if Edition::PRE_PUBLICATION_STATES.include?(edition.state)
-        update_draft(update_type: "republish")
-      else
-        do_publish("republish")
-        discard_drafts(deleted_html_attachments)
-      end
-    end
-
-    def unpublish_if_required
-      if edition.unpublishing
-        if edition.withdrawn?
-          withdraw
-        else
-          unpublish(allow_draft: false)
-        end
+    def patch_links
+      current_html_attachments.each do |html_attachment|
+        Whitehall::PublishingApi.patch_links(html_attachment, bulk_publishing: false)
       end
     end
 

test/integration/workers/publishing_api_document_republishing_worker_integration_test.rb

@@ -156,11 +156,10 @@ class PublishingApiDocumentRepublishingWorkerIntegrationTest < ActiveSupport::Te
       }),
       stub_publishing_api_put_content(html_attachment_presenter.content_id, html_attachment_presenter.content),
       stub_publishing_api_patch_links(html_attachment_presenter.content_id, links: html_attachment_presenter.links),
-      stub_publishing_api_publish(html_attachment_presenter.content_id, locale: html_attachment_presenter.content[:locale], update_type: nil),
       stub_publishing_api_unpublish(html_attachment_presenter.content_id, body: {
         type: "redirect",
         alternative_path: edition.base_path,
-        discard_drafts: true,
+        allow_draft: true,
         locale: "en",
       }),
     ]
@@ -199,11 +198,10 @@ class PublishingApiDocumentRepublishingWorkerIntegrationTest < ActiveSupport::Te
       }),
       stub_publishing_api_put_content(html_attachment_presenter.content_id, html_attachment_presenter.content),
       stub_publishing_api_patch_links(html_attachment_presenter.content_id, links: html_attachment_presenter.links),
-      stub_publishing_api_publish(html_attachment_presenter.content_id, locale: html_attachment_presenter.content[:locale], update_type: nil),
       stub_publishing_api_unpublish(html_attachment_presenter.content_id, body: {
         type: "redirect",
         alternative_path: edition.base_path,
-        discard_drafts: true,
+        allow_draft: true,
         locale: "en",
       }),
       stub_publishing_api_put_content(draft_publication_presenter.content_id, with_locale(:en) { draft_publication_presenter.content }),
@@ -236,11 +234,10 @@ class PublishingApiDocumentRepublishingWorkerIntegrationTest < ActiveSupport::Te
       }),
       stub_publishing_api_put_content(html_attachment_presenter.content_id, html_attachment_presenter.content),
       stub_publishing_api_patch_links(html_attachment_presenter.content_id, links: html_attachment_presenter.links),
-      stub_publishing_api_publish(html_attachment_presenter.content_id, locale: html_attachment_presenter.content[:locale], update_type: nil),
       stub_publishing_api_unpublish(html_attachment_presenter.content_id, body: {
         type: "redirect",
         alternative_path: edition.base_path,
-        discard_drafts: true,
+        allow_draft: true,
         locale: "en",
       }),
     ]

test/unit/app/services/service_listeners/publishing_api_html_attachments_test.rb

@@ -436,7 +436,7 @@ class Republish < PublishingApiHtmlAttachmentsTest
         end
       end
 
-      test "for a withdrawn publicaton with an attachment withdraws the attachment" do
+      test "for a withdrawn publication with an attachment withdraws the attachment" do
         publication = create(:withdrawn_publication)
         attachment = publication.html_attachments.first
         PublishingApiWorker.any_instance.expects(:perform).with(
@@ -462,7 +462,7 @@ class Republish < PublishingApiHtmlAttachmentsTest
           attachment.content_id,
           "/government/another/page",
           "en",
-          false,
+          true,
         )
         call(publication)
       end
@@ -474,7 +474,7 @@ class Republish < PublishingApiHtmlAttachmentsTest
           attachment.content_id,
           "/government/another/page",
           "en",
-          false,
+          true,
         )
         call(publication)
       end
@@ -486,10 +486,16 @@ class Republish < PublishingApiHtmlAttachmentsTest
           attachment.content_id,
           publication.search_link,
           "en",
-          false,
+          true,
         )
         call(publication)
       end
+
+      test "for a publication that has been unpublished does not publish the attachment in order to unpublish again" do
+        publication = create(:unpublished_publication_in_error_no_redirect)
+        PublishingApiWorker.any_instance.expects(:perform).never
+        call(publication)
+      end
     end
 
     class Unwithdraw < PublishingApiHtmlAttachmentsTest