Update Callbacks Documentation: Specify 'after_commit' and 'after_rollback' Behavior for Multiple Objects of the Same Record [ci skip] #49831
Conversation
dhavalsingh
changed the title
dhavalsingh
changed the title
|
Can someone check why the buildkite check is failing? or re-run it. I didn't put the [ci skip] tag at first and then edited it. |
| @@ -554,6 +554,8 @@ WARNING. The code executed within `after_commit` or `after_rollback` callbacks i | |||
|
|
|||
| WARNING. Using both `after_create_commit` and `after_update_commit` with the same method name will only allow the last callback defined to take effect, as they both internally alias to `after_commit` which overrides previously defined callbacks with the same method name. | |||
|
|
|||
| WARNING. In the context of a single transaction, should you interact with multiple instances of the same model, `after_commit` or `after_rollback` callback behavior is designed to execute callbacks for only the first instance of the model. Subsequent instances of the same model within that transaction will _not_ trigger their own `after_commit` or `after_rollback` callback. This is a nuanced behavior that can impact the predictability and flow of callback execution, particularly in scenarios where multiple instances of the same model are undergoing changes in a single transaction. | |||
There was a problem hiding this comment.
Can you provide a simple example of what this would look like or how you're doing this transaction?
There was a problem hiding this comment.
#39400
It's given in this PR.
Or. did you mean i should add an example to the docs?
There was a problem hiding this comment.
I was thinking an actual code example but since it's mentioned in that PR never mind.
There was a problem hiding this comment.
updated the msg to be more clear. I think multiple instances might be confusing
dhavalsingh
changed the title
|
Can anyone review this? I really think this should be added in the guides as there is little to no chance the behaviour will change in the future as old apps can break. |
|
Needs ActiveRecord tag @zzak , also feedback one. Thanks |
dhavalsingh
force-pushed
the
update_callback_docs_for_same_model_multi_instance
branch
from
cde889e
to
e5ba8d9
Compare
… multiple instances of the same model updated warnign to be more clear Update Callbacks Documentation: Clarify commit callback behavior
dhavalsingh
force-pushed
the
update_callback_docs_for_same_model_multi_instance
branch
from
e5ba8d9
to
2e6f1dc
Compare
|
@adrianna-chang-shopify Done! Thanks for the review. |
…ame_model_multi_instance Update Callbacks Documentation: Specify 'after_commit' and 'after_rollback' Behavior for Multiple Objects of the Same Record [ci skip]
|
I noticed the examples underneath the warnings related to the third warning, instead of this new warning. |
… multiple instances of the same model
Motivation / Background
This Pull Request has been created because this is a pretty nuanced case that can come up in complex applications. We had a bug due to this and I couldn't find any other source specifying this behavior. The rails guides should be clear on this case so folks can avoid this and be mindful of this behavior when designing or maintaining systems.
Detail
This Pull Request changes the AR Callback Docs.
Additional information
#39400 This is the only place that references this.
Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]