Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure deleted assets are marked as live #9828

Merged
merged 7 commits into from
Jan 22, 2025
2 changes: 0 additions & 2 deletions app/controllers/admin/attachments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ def update
def confirm_destroy; end

def destroy
attachment_data = attachment.attachment_data
attachment.destroy!
attachment_updater(attachment_data)
redirect_to attachable_attachments_path(attachable), notice: "Attachment deleted"
end

Expand Down
5 changes: 5 additions & 0 deletions app/models/attachment_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ def draft?
!significant_attachable.publicly_visible?
end

def needs_publishing?
attachments.size == 1 && attachments.first.attachable.publicly_visible?
end

delegate :accessible_to?, to: :significant_attachable

delegate :access_limited?, to: :last_attachable
Expand Down Expand Up @@ -125,6 +129,7 @@ def visible_attachable_for(user)

def visible_edition_for(user)
visible_attachable = visible_attachable_for(user)
# below code seems wrong, policy group is not a edition but could be visible
visible_attachable.is_a?(Edition) ? visible_attachable : nil
end

Expand Down
4 changes: 4 additions & 0 deletions app/models/call_for_evidence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ def attachables
[self, outcome].compact
end

def delete_all_attachments
attachables.map(&:attachments).flatten.each(&:destroy)
end

def rendering_app
Whitehall::RenderingApp::GOVERNMENT_FRONTEND
end
Expand Down
4 changes: 4 additions & 0 deletions app/models/consultation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ def attachables
[self, outcome, public_feedback].compact
end

def delete_all_attachments
attachables.map(&:attachments).flatten.each(&:destroy)
end

def rendering_app
Whitehall::RenderingApp::GOVERNMENT_FRONTEND
end
Expand Down
9 changes: 9 additions & 0 deletions app/models/policy_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class PolicyGroup < ApplicationRecord
after_create :extract_dependencies
after_update :extract_dependencies
after_destroy :remove_all_dependencies
after_update :publish_deleted_attachments

def extract_dependencies
remove_all_dependencies
Expand All @@ -28,6 +29,14 @@ def extract_dependencies
end
end

def publish_deleted_attachments
deleted_attachments.each do |attachment|
next unless attachment.attachment_data.present? && attachment.deleted?

DeleteAttachmentAssetJob.perform_async(attachment.attachment_data.id)
end
end

def remove_all_dependencies
policy_group_dependencies.delete_all
end
Expand Down
5 changes: 0 additions & 5 deletions app/services/asset_manager/asset_updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@ def update_with_asset_manager_id(asset_manager_id, new_attributes)
raise AssetAttributesEmpty, asset_manager_id if new_attributes.empty?

attributes = find_asset_by_id(asset_manager_id)
asset_deleted = attributes["deleted"]

if asset_deleted
logger.warn("Asset with asset_manager_id: '#{asset_manager_id}' expected not to be deleted in Asset Manager")
end

keys = new_attributes.keys
unless attributes.slice(*keys) == new_attributes.slice(*keys)
Expand Down
9 changes: 0 additions & 9 deletions app/services/asset_manager/attachment_deleter.rb

This file was deleted.

13 changes: 13 additions & 0 deletions app/services/service_listeners/attachment_asset_deleter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module ServiceListeners
class AttachmentAssetDeleter
def self.call(attachable)
Attachment.includes(:attachment_data).where(attachable: attachable.attachables).find_each do |attachment|
attachment_data = attachment.attachment_data

next unless attachment_data&.deleted?

DeleteAttachmentAssetJob.perform_async(attachment_data.id)
end
end
end
end
11 changes: 11 additions & 0 deletions app/services/service_listeners/attachment_asset_publisher.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module ServiceListeners
class AttachmentAssetPublisher
def self.call(attachable)
Attachment.includes(:attachment_data).where(attachable: attachable.attachables).find_each do |attachment|
next unless attachment.attachment_data

PublishAttachmentAssetJob.perform_async(attachment.attachment_data.id)
end
end
end
end
2 changes: 0 additions & 2 deletions app/sidekiq/asset_manager_attachment_metadata_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ def perform(attachment_data_id)

return if attachment_data.blank?

AssetManager::AttachmentDeleter.call(attachment_data)

return unless attachment_data.all_asset_variants_uploaded?

AssetManager::AttachmentUpdater.call(attachment_data)
Expand Down
13 changes: 13 additions & 0 deletions app/sidekiq/delete_attachment_asset_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
class DeleteAttachmentAssetJob < WorkerBase
sidekiq_options queue: "asset_manager"

def perform(attachment_data_id)
attachment_data = AttachmentData.find(attachment_data_id)

attachment_data.assets.each do |asset|
AssetManager::AssetDeleter.call(asset.asset_manager_id)
rescue AssetManager::ServiceHelper::AssetNotFound
logger.info("Asset #{asset.asset_manager_id} has already been deleted from Asset Manager")
end
end
end
22 changes: 22 additions & 0 deletions app/sidekiq/publish_attachment_asset_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
class PublishAttachmentAssetJob < WorkerBase
sidekiq_options queue: "asset_manager"

def perform(attachment_data_id)
attachment_data = AttachmentData.find(attachment_data_id)

asset_attributes = {
"draft" => false,
}

if attachment_data.last_attachable.respond_to?(:public_url)
asset_attributes.merge!({ "parent_document_url" => attachment_data.last_attachable.public_url })
end

# We need to delete the asset first, because if we delete it after updating the draft value to false, and the delete request fails,
# the asset will remain live.
attachment_data.assets.each do |asset|
AssetManager::AssetDeleter.call(asset.asset_manager_id) if attachment_data.deleted?
AssetManager::AssetUpdater.call(asset.asset_manager_id, asset_attributes) if attachment_data.needs_publishing?
end
end
end
10 changes: 8 additions & 2 deletions config/initializers/edition_services.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
Whitehall::Application.config.to_prepare do
Whitehall.edition_services.tap do |coordinator|
coordinator.subscribe do |_event, edition, _options|
ServiceListeners::AttachmentUpdater.call(attachable: edition)
coordinator.subscribe do |event, edition, _options|
if %w[publish force_publish].include?(event)
ServiceListeners::AttachmentAssetPublisher.call(edition)
elsif event == "delete"
ServiceListeners::AttachmentAssetDeleter.call(edition)
else
ServiceListeners::AttachmentUpdater.call(attachable: edition)
end
end

coordinator.subscribe("unpublish") do |_event, edition, _options|
Expand Down
Loading
Loading