diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index 2c599d8318..716a349b26 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,7 @@ +* Allow to detach an attachment when record is not persisted + + *Jacopo Beschi* + * Use libvips instead of ImageMagick to analyze images when `active_storage.variant_processor = vips` *Breno Gazzola* diff --git a/activestorage/lib/active_storage/attached/changes.rb b/activestorage/lib/active_storage/attached/changes.rb index a3b2cf4ab5..48b46f587a 100644 --- a/activestorage/lib/active_storage/attached/changes.rb +++ b/activestorage/lib/active_storage/attached/changes.rb @@ -12,6 +12,9 @@ module ActiveStorage autoload :DeleteOne autoload :DeleteMany + autoload :DetachOne + autoload :DetachMany + autoload :PurgeOne autoload :PurgeMany end diff --git a/activestorage/lib/active_storage/attached/changes/detach_many.rb b/activestorage/lib/active_storage/attached/changes/detach_many.rb new file mode 100644 index 0000000000..5d1fcf3784 --- /dev/null +++ b/activestorage/lib/active_storage/attached/changes/detach_many.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module ActiveStorage + class Attached::Changes::DetachMany #:nodoc: + attr_reader :name, :record, :attachments + + def initialize(name, record, attachments) + @name, @record, @attachments = name, record, attachments + end + + def detach + if attachments.any? + attachments.delete_all if attachments.respond_to?(:delete_all) + record.attachment_changes.delete(name) + end + end + end +end diff --git a/activestorage/lib/active_storage/attached/changes/detach_one.rb b/activestorage/lib/active_storage/attached/changes/detach_one.rb new file mode 100644 index 0000000000..2f2f3fe68d --- /dev/null +++ b/activestorage/lib/active_storage/attached/changes/detach_one.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module ActiveStorage + class Attached::Changes::DetachOne #:nodoc: + attr_reader :name, :record, :attachment + + def initialize(name, record, attachment) + @name, @record, @attachment = name, record, attachment + end + + def detach + if attachment.present? + attachment.delete + reset + end + end + + private + def reset + record.attachment_changes.delete(name) + record.public_send("#{name}_attachment=", nil) + end + end +end diff --git a/activestorage/lib/active_storage/attached/many.rb b/activestorage/lib/active_storage/attached/many.rb index de78751b9d..f0ee58bec6 100644 --- a/activestorage/lib/active_storage/attached/many.rb +++ b/activestorage/lib/active_storage/attached/many.rb @@ -16,6 +16,12 @@ module ActiveStorage # Purges each associated attachment through the queuing system. delegate :purge_later, to: :purge_many + ## + # :method: detach + # + # Deletes associated attachments without purging them, leaving their respective blobs in place. + delegate :detach, to: :detach_many + delegate_missing_to :attachments # Returns all the associated attachment records. @@ -60,14 +66,13 @@ module ActiveStorage attachments.any? end - # Deletes associated attachments without purging them, leaving their respective blobs in place. - def detach - attachments.delete_all if attached? - end - private def purge_many Attached::Changes::PurgeMany.new(name, record, attachments) end + + def detach_many + Attached::Changes::DetachMany.new(name, record, attachments) + end end end diff --git a/activestorage/lib/active_storage/attached/one.rb b/activestorage/lib/active_storage/attached/one.rb index f34d051232..6efbfa7a54 100644 --- a/activestorage/lib/active_storage/attached/one.rb +++ b/activestorage/lib/active_storage/attached/one.rb @@ -16,6 +16,12 @@ module ActiveStorage # Purges the attachment through the queuing system. delegate :purge_later, to: :purge_one + ## + # :method: detach + # + # Deletes the attachment without purging it, leaving its blob in place. + delegate :detach, to: :detach_one + delegate_missing_to :attachment, allow_nil: true # Returns the associated attachment record. @@ -67,21 +73,13 @@ module ActiveStorage attachment.present? end - # Deletes the attachment without purging it, leaving its blob in place. - def detach - if attached? - attachment.delete - write_attachment nil - end - end - private - def write_attachment(attachment) - record.public_send("#{name}_attachment=", attachment) - end - def purge_one Attached::Changes::PurgeOne.new(name, record, attachment) end + + def detach_one + Attached::Changes::DetachOne.new(name, record, attachment) + end end end diff --git a/activestorage/test/models/attached/many_test.rb b/activestorage/test/models/attached/many_test.rb index 576e52b56f..ac633520d0 100644 --- a/activestorage/test/models/attached/many_test.rb +++ b/activestorage/test/models/attached/many_test.rb @@ -440,6 +440,24 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase end end + test "detaching when record is not persisted" do + [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |blobs| + user = User.new + user.highlights.attach blobs + assert user.highlights.attached? + + perform_enqueued_jobs do + user.highlights.detach + end + + assert_not user.highlights.attached? + assert ActiveStorage::Blob.exists?(blobs.first.id) + assert ActiveStorage::Blob.exists?(blobs.second.id) + assert ActiveStorage::Blob.service.exist?(blobs.first.key) + assert ActiveStorage::Blob.service.exist?(blobs.second.key) + end + end + test "purging" do [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |blobs| @user.highlights.attach blobs diff --git a/activestorage/test/models/attached/one_test.rb b/activestorage/test/models/attached/one_test.rb index f39b44a9bf..186b572e27 100644 --- a/activestorage/test/models/attached/one_test.rb +++ b/activestorage/test/models/attached/one_test.rb @@ -451,6 +451,22 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase end end + test "detaching when record is not persisted" do + create_blob(filename: "funky.jpg").tap do |blob| + user = User.new + user.avatar.attach blob + assert user.avatar.attached? + + perform_enqueued_jobs do + user.avatar.detach + end + + assert_not user.avatar.attached? + assert ActiveStorage::Blob.exists?(blob.id) + assert ActiveStorage::Blob.service.exist?(blob.key) + end + end + test "purging" do create_blob(filename: "funky.jpg").tap do |blob| @user.avatar.attach blob