Fix after_commit on: :destroy not being triggered#404
Conversation
| when :destroy | ||
| transaction_include_destroy? | ||
| when :update | ||
| !(transaction_record_state(:new_record) || transaction_include_destroy?) |
There was a problem hiding this comment.
Why not just use a nvm, I had some of the boolean logic mixed up in my head...x && y instead of a !(x || y)?
|
Remade this to accommodate rails 5.1 |
|
@npezza93 @BenMorganIO any idea on when this will be merged? We could use the change quite well. |
|
@rmachielse ¯\_(ツ)_/¯, I haven't heard anything so I think it's up to @BenMorganIO. |
|
@BenMorganIO what are your thoughts about this? |
|
@BenMorganIO is there any chance this gets merged soon? |
npezza93
force-pushed
the
fix-after-commit
branch
from
682ca2a to
4b23356
Compare
|
@BenMorganIO anything I can do to speed this up? |
|
Is there anyone else who can pick this up? @jhawthorn maybe? |
|
That'd be very helpful for me as well, would like to see it on upstream. 👍 |
|
While this probably is the best approach; it differs from how it behaves with Rails 4.2. There a "soft delete" would trigger the update callback, and the destroy callback is called only for |
|
any update on this? I'm running into this now and surprised it hasn't been addressed yet |
|
Any update ? |
npezza93
force-pushed
the
fix-after-commit
branch
from
4b23356 to
8d32d02
Compare
|
+1 |
|
@npezza93 thanks for initial PR ❤️ @BenMorganIO will you merge & release it if we'll rebase && fix travis? /cc @jhawthorn |
|
I believe this is has been fixed with a combo of the latest release and rails >= 5.2 |
9 participants
Shoutout to @skorfmann on #228 for for most of this! Fixes #217
Let me know if there are any issues with this approach.