From 2c15aec3bc3d73541ee6478374740758d3c76efd Mon Sep 17 00:00:00 2001 From: James Mead Date: Sat, 3 Mar 2018 12:55:03 +0000 Subject: [PATCH] Delete underlying file/asset when parent edition is discarded This rectifies the inconsistency in behaviour explained in the previous commits. Calling `Attachment#destroy` for each attachment on the edition triggers the `after_destroy` callback on `FileAttachment` which in turn will delete its `AttachmentData` and its underlying file and Asset Manager asset if no other attachments are still referring to the `AttachmentData`. --- app/models/attachable.rb | 2 +- .../attachment_deletion_integration_test.rb | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/models/attachable.rb b/app/models/attachable.rb index 956c6a7d94c1..193b56af8af9 100644 --- a/app/models/attachable.rb +++ b/app/models/attachable.rb @@ -124,7 +124,7 @@ def next_ordering end def delete_all_attachments - attachments.each { |attachment| attachment.update(deleted: true) } + attachments.each(&:destroy) end def reorder_attachments(ordered_attachment_ids) diff --git a/test/integration/attachment_deletion_integration_test.rb b/test/integration/attachment_deletion_integration_test.rb index 12102a3fe000..1be4b72f043f 100644 --- a/test/integration/attachment_deletion_integration_test.rb +++ b/test/integration/attachment_deletion_integration_test.rb @@ -64,8 +64,15 @@ class AttachmentDeletionIntegrationTest < ActionDispatch::IntegrationTest it 'responds with 404 Not Found for attachment URL' do logout - get @attachment_url - assert_response :not_found + assert_raises(ActiveRecord::RecordNotFound) do + get @attachment_url + end + end + + it 'deletes attachment in Asset Manager' do + Services.asset_manager.expects(:delete_asset) + .with(asset_id) + AssetManagerDeleteAssetWorker.drain end end end