Merge pull request #8403 from alphagov/fix-attachment-redirects-when-…

…draft-exists

Fix unpublished edition attachment redirects when draft edition exists

app/models/attachment_data.rb

@@ -102,7 +102,7 @@ def draft?
 
   delegate :access_limited_object, to: :last_attachable
 
-  delegate :unpublished?, to: :last_attachable
+  delegate :unpublished?, to: :unpublished_attachable
 
   def replaced?
     replaced_by.present?
@@ -151,6 +151,10 @@ def last_attachable
     last_attachment.attachable || Attachable::Null.new
   end
 
+  def unpublished_attachable
+    unpublished_attachment&.attachable || Attachable::Null.new
+  end
+
   def significant_attachment(**args)
     last_publicly_visible_attachment || last_attachment(**args)
   end
@@ -159,6 +163,10 @@ def last_attachment(**args)
     filtered_attachments(**args).last || Attachment::Null.new
   end
 
+  def unpublished_attachment
+    attachments.reverse.detect { |a| a.attachable&.unpublished? }
+  end
+
   def last_publicly_visible_attachment
     attachments.reverse.detect { |a| (a.attachable || Attachable::Null.new).publicly_visible? }
   end

app/services/asset_manager/attachment_updater.rb

@@ -55,7 +55,7 @@ def self.get_link_header(attachment_data)
   def self.get_redirect_url(attachment_data)
     return nil unless attachment_data.unpublished?
 
-    attachment_data.last_attachable.unpublishing.document_url
+    attachment_data.unpublished_attachable.unpublishing.document_url
   end
 
   def self.get_replacement_id(replaced_attachment_data, variant)

test/integration/attachment_redirect_due_to_unpublishing_integration_test.rb

@@ -61,6 +61,17 @@ class AttachmentRedirectDueToUnpublishingIntegrationTest < ActionDispatch::Integ
       end
     end
 
+    context "given a published document with file attachment and a draft" do
+      let(:edition) { create(:published_news_article) }
+      let!(:draft) { edition.create_draft(edition.creator) }
+
+      it "sets redirect URL for attachment in Asset Manager when document is unpublished" do
+        visit admin_news_article_path(edition)
+        unpublish_document_published_in_error
+        assert_sets_redirect_url_in_asset_manager_to redirect_url
+      end
+    end
+
     context "given a published document with HTML attachment" do
       let(:edition) { create(:published_publication, :with_html_attachment) }
 

test/unit/app/services/asset_manager/attachment_updater_test.rb

@@ -169,7 +169,7 @@ class AssetManager::AttachmentUpdaterTest < ActiveSupport::TestCase
 
       before do
         attachment_data.stubs(:unpublished?).returns(unpublished)
-        attachment_data.stubs(:last_attachable).returns(unpublished_edition)
+        attachment_data.stubs(:unpublished_attachable).returns(unpublished_edition)
       end
 
       it "updates redirect URL for all assets" do