markevans / dragonfly

A Ruby gem for on-the-fly processing - suitable for image uploading in Rails, Sinatra and much more!
http://markevans.github.io/dragonfly
MIT License
2.12k stars 244 forks source link

Replace after_destroy by after_commit on: :destroy #511

Closed dlibanori closed 3 years ago

dlibanori commented 3 years ago

As wrongly stated at #477 changing destroy_dragonfly_attachments callback from before_destroy to after_destroy is not related with transaction/commit. It was failing cause foreign key's constraint. It still doesn't work as expected on rollback and it deletes dragonfly attachment from store:

class Foo < ApplicationRecord
  dragonfly_accessor :bar
end

f = Foo.create(bar: File.new('some.blob'))
Foo.transation do
  f.destroy
  raise ActiveRecord::Rollback
end

The right callback is after_destroy_commit or a more general Rails callback after_commit on: :destroy.

This PR is very conservative. It tries to attach after_commit and fallback to after_destroy, so it should still work even if ORMs doesn't have after_commit.

lucasferronato commented 3 years ago

@markevans could you guys give this PR some attention? we're using a monkey patch as a workaround to this problem and would like to remove it asap

markevans commented 3 years ago

cheers - will put in next release (also thanks for making conservative - it needs to work with ActiveModel-only models)