diff --git a/app/sidekiq/asset_manager_create_asset_worker.rb b/app/sidekiq/asset_manager_create_asset_worker.rb index c560997f494..21361cdaadd 100644 --- a/app/sidekiq/asset_manager_create_asset_worker.rb +++ b/app/sidekiq/asset_manager_create_asset_worker.rb @@ -42,14 +42,16 @@ def create_asset(asset_options, asset_variant, assetable_id, assetable_type) end def enqueue_downstream_service_updates(assetable_id, assetable_type, attachable_model_class, attachable_model_id) - assetable = assetable_type.constantize.find(assetable_id) - assetable.republish_on_assets_ready if assetable.respond_to? :republish_on_assets_ready - if attachable_model_class if attachable_model_class.constantize.ancestors.include?(Edition) PublishingApiDraftUpdateWorker.perform_async(attachable_model_class, attachable_model_id) - else - AssetManagerAttachmentMetadataWorker.perform_async(assetable_id) + end + + AssetManagerAttachmentMetadataWorker.perform_async(assetable_id) + else + assetable = assetable_type.constantize.find(assetable_id) + if assetable.respond_to?(:republish_on_assets_ready) + assetable.republish_on_assets_ready end end end diff --git a/test/integration/asset_manager_test.rb b/test/integration/asset_manager_test.rb index 212156727a4..4b2db6f3d54 100644 --- a/test/integration/asset_manager_test.rb +++ b/test/integration/asset_manager_test.rb @@ -9,6 +9,7 @@ class CreatingAFileAttachment < ActiveSupport::TestCase @edition = create(:draft_publication) @attachment = FactoryBot.build(:file_attachment_with_no_assets, file: file_fixture(@filename), attachable: @edition) @asset_manager_response = { "id" => "http://asset-manager/assets/asset_manager_id", "name" => @filename } + Services.asset_manager.stubs(:asset).returns(@asset_manager_response) end test "sends the attachment to Asset Manager" do diff --git a/test/integration/attachment_replacement_integration_test.rb b/test/integration/attachment_replacement_integration_test.rb index fe2d6aabd0d..0b5307da350 100644 --- a/test/integration/attachment_replacement_integration_test.rb +++ b/test/integration/attachment_replacement_integration_test.rb @@ -10,9 +10,11 @@ class AttachmentReplacementIntegrationTest < ActionDispatch::IntegrationTest describe "attachment replacement" do let(:managing_editor) { create(:managing_editor) } let(:filename) { "sample.csv" } - let(:asset_manager_id) { "asset_manager_id" } let(:replacement_filename) { "sample.rtf" } + let(:double_replacement_filename) { "sample.docx" } + let(:asset_manager_id) { "asset_manager_id" } let(:replacement_asset_manager_id) { "replacement-asset-id" } + let(:double_replacement_asset_manager_id) { "double-replacement-asset-id" } let(:variant) { Asset.variants[:original] } let(:attachment) { build(:csv_attachment, title: "attachment-title", attachable: edition) } @@ -92,6 +94,71 @@ class AttachmentReplacementIntegrationTest < ActionDispatch::IntegrationTest AssetManagerAttachmentMetadataWorker.drain end end + + context "when new draft is created and attachment is replaced twice" do + before do + [ + { id: replacement_asset_manager_id, filename: replacement_filename }, + { id: double_replacement_asset_manager_id, filename: double_replacement_filename }, + ].each do |item| + Services.asset_manager.expects(:create_asset).with { |args| + args[:file].path =~ /#{item[:filename]}/ + }.returns("id" => "http://asset-manager/assets/#{item[:id]}", "name" => item[:filename]) + end + stub_asset(double_replacement_asset_manager_id) + + Sidekiq::Job.clear_all + end + + it "without pre-saving the edition - updates draft & replacement for asset in Asset Manager" do + visit admin_news_article_path(edition) + click_button "Create new edition" + + AssetManagerAttachmentMetadataWorker.drain + AssetManagerCreateAssetWorker.drain + PublishingApiDraftUpdateWorker.drain + AssetManagerAttachmentMetadataWorker.drain + + Services.asset_manager.expects(:update_asset) + .with(asset_manager_id, { "replacement_id" => replacement_asset_manager_id }) + .once + Services.asset_manager.expects(:update_asset) + .with(replacement_asset_manager_id, has_entry({ "draft" => true })) + .once + + click_link "Attachments 1" + within page.find("li", text: filename) do + click_link "Edit attachment" + end + attach_file "Replace file", path_to_attachment(replacement_filename) + click_button "Save" + assert_text "Attachment 'attachment-title' updated" + + AssetManagerAttachmentMetadataWorker.drain + AssetManagerCreateAssetWorker.drain + PublishingApiDraftUpdateWorker.drain + AssetManagerAttachmentMetadataWorker.drain + + Services.asset_manager.expects(:update_asset) + .with(replacement_asset_manager_id, { "replacement_id" => double_replacement_asset_manager_id }) + .once + Services.asset_manager.expects(:update_asset) + .with(double_replacement_asset_manager_id, has_entry({ "draft" => true })) + .once + + within page.find("li", text: replacement_filename) do + click_link "Edit attachment" + end + attach_file "Replace file", path_to_attachment(double_replacement_filename) + click_button "Save" + assert_text "Attachment 'attachment-title' updated" + + AssetManagerAttachmentMetadataWorker.drain + AssetManagerCreateAssetWorker.drain + PublishingApiDraftUpdateWorker.drain + AssetManagerAttachmentMetadataWorker.drain + end + end end private diff --git a/test/unit/app/sidekiq/asset_manager_create_asset_worker_test.rb b/test/unit/app/sidekiq/asset_manager_create_asset_worker_test.rb index c3aa588845b..679f9c51490 100644 --- a/test/unit/app/sidekiq/asset_manager_create_asset_worker_test.rb +++ b/test/unit/app/sidekiq/asset_manager_create_asset_worker_test.rb @@ -6,7 +6,8 @@ class AssetManagerCreateAssetWorkerTest < ActiveSupport::TestCase @worker = AssetManagerCreateAssetWorker.new @asset_manager_id = "asset_manager_id" @organisation = FactoryBot.create(:organisation) - @model_without_assets = FactoryBot.create(:attachment_data_with_no_assets, attachable: create(:draft_publication)) + @attachable = create(:draft_publication) + @model_without_assets = FactoryBot.create(:attachment_data_with_no_assets, attachable: @attachable) @asset_manager_response = { "id" => "http://asset-manager/assets/#{@asset_manager_id}", "name" => File.basename(@file), @@ -23,26 +24,26 @@ class AssetManagerCreateAssetWorkerTest < ActiveSupport::TestCase args[:file].path == @file.path }.returns(@asset_manager_response) - @worker.perform(@file.path, @asset_params) + @worker.perform(@file.path, @asset_params, false, @attachable.class.to_s, @attachable.id) end test "marks the asset as draft if instructed" do Services.asset_manager.expects(:create_asset).with(has_entry(draft: true)).returns(@asset_manager_response) - @worker.perform(@file.path, @asset_params, true) + @worker.perform(@file.path, @asset_params, true, @attachable.class.to_s, @attachable.id) end test "removes the local temp file after the file has been successfully uploaded" do Services.asset_manager.stubs(:create_asset).returns(@asset_manager_response) - @worker.perform(@file.path, @asset_params) + @worker.perform(@file.path, @asset_params, false, @attachable.class.to_s, @attachable.id) assert_not File.exist?(@file.path) end test "removes the local temp directory after the file has been successfully uploaded" do Services.asset_manager.stubs(:create_asset).returns(@asset_manager_response) - @worker.perform(@file.path, @asset_params) + @worker.perform(@file.path, @asset_params, false, @attachable.class.to_s, @attachable.id) assert_not Dir.exist?(File.dirname(@file)) end @@ -100,7 +101,7 @@ class AssetManagerCreateAssetWorkerTest < ActiveSupport::TestCase test "stores corresponding asset_manager_id and filename for current file attachment" do Services.asset_manager.stubs(:create_asset).returns(@asset_manager_response) - @worker.perform(@file.path, @asset_params) + @worker.perform(@file.path, @asset_params, false, @attachable.class.to_s, @attachable.id) assert_equal 1, Asset.where(asset_manager_id: @asset_manager_id, variant: Asset.variants[:original], filename: File.basename(@file)).count end