mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
touch parent model when an attachment is purged
Currently `delete` is used on the `purge` and `purge_later` methods, that prevent any callbacks to be triggered which causes the parent model to not be updated when an attachment is purged. This behaviour cause issues on some caching strategies as reported here: https://github.com/rails/rails/issues/39858 Changes: * Add `record&.touch` on `attachment#purge` * Add `record&.touch` on `attachment#purge_later` * Remove extra blank line on attachment.rb * Add tests which are failing before this change and pass after the change
This commit is contained in:
parent
b949e69c5d
commit
396b43a99d
5 changed files with 25 additions and 7 deletions
|
@ -1,3 +1,7 @@
|
|||
* Touch parent model when an attachment is purged.
|
||||
|
||||
*Víctor Pérez Rodríguez*
|
||||
|
||||
* Files can now be served by proxying them from the underlying storage service
|
||||
instead of redirecting to a signed service URL. Use the
|
||||
`rails_storage_proxy_path` and `_url` helpers to proxy an attached file:
|
||||
|
|
|
@ -21,13 +21,19 @@ class ActiveStorage::Attachment < ActiveRecord::Base
|
|||
|
||||
# Synchronously deletes the attachment and {purges the blob}[rdoc-ref:ActiveStorage::Blob#purge].
|
||||
def purge
|
||||
delete
|
||||
transaction do
|
||||
delete
|
||||
record&.touch
|
||||
end
|
||||
blob&.purge
|
||||
end
|
||||
|
||||
# Deletes the attachment and {enqueues a background job}[rdoc-ref:ActiveStorage::Blob#purge_later] to purge the blob.
|
||||
def purge_later
|
||||
delete
|
||||
transaction do
|
||||
delete
|
||||
record&.touch
|
||||
end
|
||||
blob&.purge_later
|
||||
end
|
||||
|
||||
|
@ -48,7 +54,6 @@ class ActiveStorage::Attachment < ActiveRecord::Base
|
|||
blob&.purge_later if dependent == :purge_later
|
||||
end
|
||||
|
||||
|
||||
def dependent
|
||||
record.attachment_reflections[name]&.options[:dependent]
|
||||
end
|
||||
|
|
|
@ -5,6 +5,7 @@ class ActiveStorageCreateUsers < ActiveRecord::Migration[5.2]
|
|||
create_table :users do |t|
|
||||
t.string :name
|
||||
t.integer :group_id
|
||||
t.timestamps
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -445,7 +445,9 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase
|
|||
@user.highlights.attach blobs
|
||||
assert @user.highlights.attached?
|
||||
|
||||
@user.highlights.purge
|
||||
assert_changes -> { @user.updated_at } do
|
||||
@user.highlights.purge
|
||||
end
|
||||
assert_not @user.highlights.attached?
|
||||
assert_not ActiveStorage::Blob.exists?(blobs.first.id)
|
||||
assert_not ActiveStorage::Blob.exists?(blobs.second.id)
|
||||
|
@ -487,7 +489,9 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase
|
|||
assert @user.highlights.attached?
|
||||
|
||||
perform_enqueued_jobs do
|
||||
@user.highlights.purge_later
|
||||
assert_changes -> { @user.updated_at } do
|
||||
@user.highlights.purge_later
|
||||
end
|
||||
end
|
||||
|
||||
assert_not @user.highlights.attached?
|
||||
|
|
|
@ -449,7 +449,9 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase
|
|||
@user.avatar.attach blob
|
||||
assert @user.avatar.attached?
|
||||
|
||||
@user.avatar.purge
|
||||
assert_changes -> { @user.updated_at } do
|
||||
@user.avatar.purge
|
||||
end
|
||||
assert_not @user.avatar.attached?
|
||||
assert_not ActiveStorage::Blob.exists?(blob.id)
|
||||
assert_not ActiveStorage::Blob.service.exist?(blob.key)
|
||||
|
@ -478,7 +480,9 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase
|
|||
assert @user.avatar.attached?
|
||||
|
||||
perform_enqueued_jobs do
|
||||
@user.avatar.purge_later
|
||||
assert_changes -> { @user.updated_at } do
|
||||
@user.avatar.purge_later
|
||||
end
|
||||
end
|
||||
|
||||
assert_not @user.avatar.attached?
|
||||
|
|
Loading…
Reference in a new issue