Merge pull request #8435 from alphagov/fix-orphaned-attachment-data-e…

…rror Fix orphaned attachment data error
alphagov · Oct 31, 2023 · 6b947aa · 6b947aa
2 parents 0005c56 + f7f7483
commit 6b947aa
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 14 deletions.
diff --git a/app/services/asset_manager/attachment_updater.rb b/app/services/asset_manager/attachment_updater.rb
@@ -1,13 +1,16 @@
 class AssetManager::AttachmentUpdater
   def self.call(attachment_data)
-    return if attachment_data.deleted? || attachment_data.attachments.first.attachable.nil?
+    return if attachment_data.deleted?
 
     asset_attributes = {
       "access_limited_organisation_ids" => attachment_data.access_limitation,
       "draft" => attachment_data.draft? && !(attachment_data.replaced? || attachment_data.unpublished?),
-      "parent_document_url" => attachment_data.attachable_url,
     }
 
+    unless attachment_data.replaced?
+      asset_attributes.merge!({ "parent_document_url" => attachment_data.attachable_url })
+    end
+
     attachment_data.assets.each do |asset|
       AssetManager::AssetUpdater.call(asset.asset_manager_id, attachment_data, nil, asset_attributes)
     end

diff --git a/test/unit/app/services/asset_manager/attachment_updater_test.rb b/test/unit/app/services/asset_manager/attachment_updater_test.rb
@@ -22,21 +22,28 @@ class AssetManager::AttachmentUpdaterTest < ActiveSupport::TestCase
         AssetManager::AttachmentUpdater.call(attachment.attachment_data)
       end
 
-      it "sets the expected attributes when the attachment has been replaced" do
-        attachment.attachment_data.replace_with!(AttachmentData.create!(file: File.open(fixture_path.join("sample.docx"))))
+      context "and the attachment has been replaced" do
+        it "ensures replaced attachment data is still accessible for the publicly visible edition" do
+          replacement = create(:attachment_data)
+          replaced_attachment_data = attachment.attachment_data
 
-        # Draft should be set to false when the attachment data has been replaced to ensure the data is updated in asset manager
-        expected_attribute_hash = {
-          "access_limited_organisation_ids" => [],
-          "draft" => false,
-          "parent_document_url" => edition.public_url(draft: true),
-        }
+          attachment.attachment_data.replace_with!(replacement)
+          attachment.attachment_data = replacement
+          attachment.save!
 
-        attachment.attachment_data.assets.each do |asset|
-          AssetManager::AssetUpdater.expects(:call).with(asset.asset_manager_id, attachment.attachment_data, nil, expected_attribute_hash)
-        end
+          replaced_attachment_data.reload
 
-        AssetManager::AttachmentUpdater.call(attachment.attachment_data)
+          expected_attribute_hash = {
+            "access_limited_organisation_ids" => [],
+            "draft" => false,
+          }
+
+          replaced_attachment_data.assets.each do |asset|
+            AssetManager::AssetUpdater.expects(:call).with(asset.asset_manager_id, replaced_attachment_data, nil, expected_attribute_hash)
+          end
+
+          AssetManager::AttachmentUpdater.call(replaced_attachment_data)
+        end
       end
     end