Add test for :skip_after_callbacks_if_terminated
#18031
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`define_callbacks` from `ActiveSupport::Callbacks` accepts the `:skip_after_callbacks_if_terminated` option since rails#4866 but the option is not tested anywhere. This commit adds tests and fixes documentation for the option, making it clear that halting a callback chain only stops following `before_` and `around_` callbacks by default.
claudiob
force-pushed
the
better-tests-for-callbacks-terminator
branch
from
f60062b
to
8c1889c
Compare
|
If any before callback halts, is the event executed? If not I'd expect to after callbacks being stopped too. |
|
Test seems good but I'd revisit this default |
rafaelfranca
added a commit
that referenced
this pull request
Dec 16, 2014
…rminator Add test for `:skip_after_callbacks_if_terminated`
|
@rafaelfranca said:
I believe that is the same thing that @dhh said in this comment:
I agree that it's a good point, but I think we can deal with that after #17227 is merged, right? :) |
|
Sure. We also need a good upgrade path or it may be a huge problem when upgrading. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
define_callbacksfromActiveSupport::Callbacksaccepts the:skip_after_callbacks_if_terminatedoption since #4866 but the optionis not tested anywhere.
This commit adds tests and fixes documentation for the option, making it clear
that halting a callback chain only stops further
before_andaround_callbacks by default.
By default,
after_callbacks are not halted.