Skip to content

Commit

Permalink
Merge pull request #9828 from alphagov/ensure-deleted-assets-are-mark…
Browse files Browse the repository at this point in the history
…ed-as-live

Ensure deleted assets are marked as live
  • Loading branch information
lauraghiorghisor-tw authored Jan 22, 2025
2 parents 409ba6a + ab930d7 commit dd8a728
Show file tree
Hide file tree
Showing 24 changed files with 469 additions and 120 deletions.
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

0 comments on commit dd8a728

Please sign in to comment.