Merge pull request #8592 from alphagov/prepare-clean-up-legacy-url-path

Prepare Clean up legacy url path

app/services/edition_auth_bypass_updater.rb

@@ -26,15 +26,15 @@ def update_file_attachments(edition)
 
     edition.attachments.files.each do |file_attachment|
       attachment_data_id = file_attachment.attachment_data.id
-      AssetManagerUpdateWhitehallAssetWorker.perform_async_in_queue("asset_manager_updater", "AttachmentData", attachment_data_id, new_attributes)
+      AssetManagerUpdateAssetWorker.perform_async_in_queue("asset_manager_updater", "AttachmentData", attachment_data_id, new_attributes)
     end
   end
 
   def update_image_attachments(edition)
     edition.images.each do |image|
       image_data_id = image.image_data.id
       new_attributes = { "auth_bypass_ids" => [edition.auth_bypass_id] }
-      AssetManagerUpdateWhitehallAssetWorker.perform_async_in_queue("asset_manager_updater", "ImageData", image_data_id, new_attributes)
+      AssetManagerUpdateAssetWorker.perform_async_in_queue("asset_manager_updater", "ImageData", image_data_id, new_attributes)
     end
   end
 
@@ -46,22 +46,22 @@ def update_consultation_attachments(edition)
       response_form_data_id = response_form.consultation_response_form_data.id
       new_attributes = { "auth_bypass_ids" => [edition.auth_bypass_id] }
 
-      AssetManagerUpdateWhitehallAssetWorker.perform_async_in_queue("asset_manager_updater", "ConsultationResponseFormData", response_form_data_id, new_attributes)
+      AssetManagerUpdateAssetWorker.perform_async_in_queue("asset_manager_updater", "ConsultationResponseFormData", response_form_data_id, new_attributes)
     end
 
     if edition.outcome.present?
       edition.outcome.attachments.files.each do |file_attachment|
         new_attributes = { "auth_bypass_ids" => [edition.auth_bypass_id] }
         attachment_data_id = file_attachment.attachment_data.id
-        AssetManagerUpdateWhitehallAssetWorker.perform_async_in_queue("asset_manager_updater", "AttachmentData", attachment_data_id, new_attributes)
+        AssetManagerUpdateAssetWorker.perform_async_in_queue("asset_manager_updater", "AttachmentData", attachment_data_id, new_attributes)
       end
     end
 
     if edition.public_feedback.present?
       edition.public_feedback.attachments.files.each do |file_attachment|
         new_attributes = { "auth_bypass_ids" => [edition.auth_bypass_id] }
         attachment_data_id = file_attachment.attachment_data.id
-        AssetManagerUpdateWhitehallAssetWorker.perform_async_in_queue("asset_manager_updater", "AttachmentData", attachment_data_id, new_attributes)
+        AssetManagerUpdateAssetWorker.perform_async_in_queue("asset_manager_updater", "AttachmentData", attachment_data_id, new_attributes)
       end
     end
   end

app/workers/asset_manager_delete_asset_worker.rb

@@ -1,7 +1,11 @@
 class AssetManagerDeleteAssetWorker < WorkerBase
   sidekiq_options queue: "asset_manager"
 
-  def perform(legacy_url_path, asset_manager_id)
+  def perform(legacy_url_path, asset_manager_id = nil)
+    if legacy_url_path && !legacy_url_path.match(/.*government.*/)
+      asset_manager_id = legacy_url_path
+    end
+
     begin
       AssetManager::AssetDeleter.call(legacy_url_path, asset_manager_id)
     rescue AssetManager::ServiceHelper::AssetNotFound

app/workers/asset_manager_update_asset_worker.rb

@@ -0,0 +1,33 @@
+class AssetManagerUpdateAssetWorker < WorkerBase
+  sidekiq_options queue: "asset_manager_updater"
+
+  def perform(klass, id, attributes)
+    model = klass.constantize.find(id)
+    asset_data = GlobalID::Locator.locate(model.to_global_id)
+
+    if should_use_non_legacy_endpoints?(asset_data)
+      asset_data.assets.each do |asset|
+        AssetManager::AssetUpdater.call(asset.asset_manager_id, asset_data, nil, attributes)
+      end
+    else
+      path = asset_data.file.asset_manager_path
+      AssetManager::AssetUpdater.call(nil, asset_data, path, attributes)
+
+      # Update any other versions of the file (e.g. PDF thumbnail, or resized versions of an image)
+      asset_versions = asset_data.file.versions.values.select(&:present?)
+      asset_versions.each do |version|
+        AssetManager::AssetUpdater.call(nil, asset_data, version.asset_manager_path, attributes)
+      end
+    end
+  rescue AssetManager::ServiceHelper::AssetNotFound,
+         AssetManager::AssetUpdater::AssetAlreadyDeleted,
+         ActiveRecord::RecordNotFound => e
+    logger.error "AssetManagerUpdateAssetWorker: #{e.message}"
+  end
+
+private
+
+  def should_use_non_legacy_endpoints?(asset_data)
+    asset_data.instance_of?(AttachmentData) || asset_data.instance_of?(ImageData) || asset_data.instance_of?(ConsultationResponseFormData)
+  end
+end

lib/whitehall/asset_manager_storage.rb

@@ -53,10 +53,10 @@ def delete
       if Whitehall::AssetManagerStorage.use_non_legacy_behaviour?(@model)
         asset = get_asset
         if asset
-          AssetManagerDeleteAssetWorker.perform_async(nil, asset.asset_manager_id)
+          AssetManagerDeleteAssetWorker.perform_async(asset.asset_manager_id)
         end
       else
-        AssetManagerDeleteAssetWorker.perform_async(@legacy_url_path, nil)
+        AssetManagerDeleteAssetWorker.perform_async(@legacy_url_path)
       end
     end
 

test/functional/admin/promotional_feature_items_controller_test.rb

@@ -130,7 +130,7 @@ class Admin::PromotionalFeatureItemsControllerTest < ActionController::TestCase
     promotional_feature_item = create(:promotional_feature_item, promotional_feature: @promotional_feature)
 
     promotional_feature_item.assets.each do |asset|
-      AssetManagerDeleteAssetWorker.expects(:perform_async).with(nil, asset.asset_manager_id)
+      AssetManagerDeleteAssetWorker.expects(:perform_async).with(asset.asset_manager_id)
     end
 
     delete :destroy, params: { organisation_id: @organisation, promotional_feature_id: @promotional_feature, id: promotional_feature_item }

test/unit/app/services/editon_auth_bypass_updater_test.rb

@@ -43,7 +43,7 @@ class EditionAuthBypassUpdaterTest < ActiveSupport::TestCase
         updater:,
       )
 
-      AssetManagerUpdateWhitehallAssetWorker.expects(:perform_async_in_queue).with(
+      AssetManagerUpdateAssetWorker.expects(:perform_async_in_queue).with(
         "asset_manager_updater",
         "AttachmentData",
         file_attachment.attachment_data.id,
@@ -66,7 +66,7 @@ class EditionAuthBypassUpdaterTest < ActiveSupport::TestCase
         updater:,
       )
 
-      AssetManagerUpdateWhitehallAssetWorker.expects(:perform_async_in_queue).with(
+      AssetManagerUpdateAssetWorker.expects(:perform_async_in_queue).with(
         "asset_manager_updater",
         "ImageData",
         image.image_data.id,
@@ -88,7 +88,7 @@ class EditionAuthBypassUpdaterTest < ActiveSupport::TestCase
         updater:,
       )
 
-      AssetManagerUpdateWhitehallAssetWorker.expects(:perform_async_in_queue).never
+      AssetManagerUpdateAssetWorker.expects(:perform_async_in_queue).never
 
       service.call
     end
@@ -108,7 +108,7 @@ class EditionAuthBypassUpdaterTest < ActiveSupport::TestCase
         updater:,
       )
 
-      AssetManagerUpdateWhitehallAssetWorker.expects(:perform_async_in_queue).with(
+      AssetManagerUpdateAssetWorker.expects(:perform_async_in_queue).with(
         "asset_manager_updater",
         "ConsultationResponseFormData",
         consultation_response_form.consultation_response_form_data.id,
@@ -132,7 +132,7 @@ class EditionAuthBypassUpdaterTest < ActiveSupport::TestCase
         updater:,
       )
 
-      AssetManagerUpdateWhitehallAssetWorker.expects(:perform_async_in_queue).with(
+      AssetManagerUpdateAssetWorker.expects(:perform_async_in_queue).with(
         "asset_manager_updater",
         "AttachmentData",
         file_attachment.attachment_data.id,
@@ -156,7 +156,7 @@ class EditionAuthBypassUpdaterTest < ActiveSupport::TestCase
         updater:,
       )
 
-      AssetManagerUpdateWhitehallAssetWorker.expects(:perform_async_in_queue).with(
+      AssetManagerUpdateAssetWorker.expects(:perform_async_in_queue).with(
         "asset_manager_updater",
         "AttachmentData",
         file_attachment.attachment_data.id,

test/unit/app/workers/asset_manager_delete_asset_worker_test.rb

@@ -8,9 +8,9 @@ class AssetManagerDeleteAssetWorkerTest < ActiveSupport::TestCase
   end
 
   test "it calls AssetManager::AssetDeleter" do
-    AssetManager::AssetDeleter.expects(:call).with("legacy-url-path", "any-asset-id")
+    AssetManager::AssetDeleter.expects(:call).with("/government/legacy-url-path", "any-asset-id")
 
-    @worker.perform("legacy-url-path", "any-asset-id")
+    @worker.perform("/government/legacy-url-path", "any-asset-id")
   end
 
   test "it raises an error if the asset deletion fails for an unknown reason" do
@@ -37,4 +37,12 @@ class AssetManagerDeleteAssetWorkerTest < ActiveSupport::TestCase
 
     assert_not Asset.where(asset_manager_id: @asset_manager_id).exists?
   end
+
+  test "it deletes the Asset from Asset table with asset manager id as first param " do
+    AssetManager::AssetDeleter.stubs(:call)
+
+    @worker.perform(@asset_manager_id)
+
+    assert_not Asset.where(asset_manager_id: @asset_manager_id).exists?
+  end
 end

test/unit/app/workers/asset_manager_update_whitehall_asset_worker_test.rb → test/unit/app/workers/asset_manager_update_asset_worker_test.rb

@@ -1,6 +1,6 @@
 require "test_helper"
 
-class AssetManagerUpdateWhitehallAssetWorkerTest < ActiveSupport::TestCase
+class AssetManagerUpdateAssetWorkerTest < ActiveSupport::TestCase
   extend Minitest::Spec::DSL
 
   let(:auth_bypass_id_attributes) do
@@ -12,26 +12,26 @@ class AssetManagerUpdateWhitehallAssetWorkerTest < ActiveSupport::TestCase
     AssetManager::AssetUpdater.expects(:call).with("asset_manager_id_original", attachment_data, nil, auth_bypass_id_attributes)
     AssetManager::AssetUpdater.expects(:call).with("asset_manager_id_thumbnail", attachment_data, nil, auth_bypass_id_attributes)
 
-    AssetManagerUpdateWhitehallAssetWorker.perform_async_in_queue("asset_manager_updater", "AttachmentData", attachment_data.id, auth_bypass_id_attributes)
-    AssetManagerUpdateWhitehallAssetWorker.drain
+    AssetManagerUpdateAssetWorker.perform_async_in_queue("asset_manager_updater", "AttachmentData", attachment_data.id, auth_bypass_id_attributes)
+    AssetManagerUpdateAssetWorker.drain
   end
 
   test "ignores missing assets in Asset Manager" do
     expected_error = AssetManager::ServiceHelper::AssetNotFound.new("asset_manager_id_original")
     AssetManager::AssetUpdater.expects(:call).once.raises(expected_error)
     Logger.any_instance.stubs(:error).with(includes(expected_error.message)).once
 
-    AssetManagerUpdateWhitehallAssetWorker.perform_async_in_queue("asset_manager_updater", "AttachmentData", attachment_data.id, auth_bypass_id_attributes)
-    AssetManagerUpdateWhitehallAssetWorker.drain
+    AssetManagerUpdateAssetWorker.perform_async_in_queue("asset_manager_updater", "AttachmentData", attachment_data.id, auth_bypass_id_attributes)
+    AssetManagerUpdateAssetWorker.drain
   end
 
   test "ignores assets that have been deleted in Asset Manager" do
     expected_error = AssetManager::AssetUpdater::AssetAlreadyDeleted.new(attachment_data.id, "asset_manager_id_original")
     AssetManager::AssetUpdater.expects(:call).raises(expected_error)
     Logger.any_instance.stubs(:error).with(includes(expected_error.message)).once
 
-    AssetManagerUpdateWhitehallAssetWorker.perform_async_in_queue("asset_manager_updater", "AttachmentData", attachment_data.id, auth_bypass_id_attributes)
-    AssetManagerUpdateWhitehallAssetWorker.drain
+    AssetManagerUpdateAssetWorker.perform_async_in_queue("asset_manager_updater", "AttachmentData", attachment_data.id, auth_bypass_id_attributes)
+    AssetManagerUpdateAssetWorker.drain
   end
 
   test "ignores assets that have been deleted in Whitehall" do
@@ -40,8 +40,8 @@ class AssetManagerUpdateWhitehallAssetWorkerTest < ActiveSupport::TestCase
 
     Logger.any_instance.stubs(:error).with(includes(attachment_data_id.to_s)).once
 
-    AssetManagerUpdateWhitehallAssetWorker.perform_async_in_queue("asset_manager_updater", "AttachmentData", attachment_data_id, auth_bypass_id_attributes)
-    AssetManagerUpdateWhitehallAssetWorker.drain
+    AssetManagerUpdateAssetWorker.perform_async_in_queue("asset_manager_updater", "AttachmentData", attachment_data_id, auth_bypass_id_attributes)
+    AssetManagerUpdateAssetWorker.drain
   end
 
   test "updates an image and its resized versions" do
@@ -58,8 +58,8 @@ class AssetManagerUpdateWhitehallAssetWorkerTest < ActiveSupport::TestCase
       AssetManager::AssetUpdater.expects(:call).with(asset_manager_id, image_data, nil, @auth_bypass_id_attributes).once
     end
 
-    AssetManagerUpdateWhitehallAssetWorker.perform_async_in_queue("asset_manager_updater", "ImageData", image_data.id, @auth_bypass_id_attributes)
-    AssetManagerUpdateWhitehallAssetWorker.drain
+    AssetManagerUpdateAssetWorker.perform_async_in_queue("asset_manager_updater", "ImageData", image_data.id, @auth_bypass_id_attributes)
+    AssetManagerUpdateAssetWorker.drain
   end
 
   test "updates the consultation response form variant" do
@@ -68,7 +68,7 @@ class AssetManagerUpdateWhitehallAssetWorkerTest < ActiveSupport::TestCase
 
     AssetManager::AssetUpdater.expects(:call).with("asset_manager_id_original", form_data, nil, @auth_bypass_id_attributes)
 
-    AssetManagerUpdateWhitehallAssetWorker.perform_async_in_queue("asset_manager_updater", "ConsultationResponseFormData", form_data.id, @auth_bypass_id_attributes)
-    AssetManagerUpdateWhitehallAssetWorker.drain
+    AssetManagerUpdateAssetWorker.perform_async_in_queue("asset_manager_updater", "ConsultationResponseFormData", form_data.id, @auth_bypass_id_attributes)
+    AssetManagerUpdateAssetWorker.drain
   end
 end

test/unit/lib/whitehall/asset_manager_storage_test.rb

@@ -155,7 +155,7 @@ def assets_protected?
     test "should call deleteAssetWorker with asset manager id" do
       model = create(:image)
 
-      AssetManagerDeleteAssetWorker.expects(:perform_async).times(7).with(nil, regexp_matches(/asset_manager_id./))
+      AssetManagerDeleteAssetWorker.expects(:perform_async).times(7).with(regexp_matches(/asset_manager_id./))
 
       model.destroy!
     end