mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Fix the AS::Callbacks terminator regression from 4.2.3
Rails 4.2.3 AS::Callbacks will not halt chain if `false` is returned. That is the behavior of specific callbacks like AR::Callbacks and AM::Callbacks.
This commit is contained in:
parent
eb52c8979b
commit
35cd365621
7 changed files with 45 additions and 36 deletions
|
@ -103,6 +103,7 @@ module ActiveModel
|
|||
def define_model_callbacks(*callbacks)
|
||||
options = callbacks.extract_options!
|
||||
options = {
|
||||
terminator: deprecated_false_terminator,
|
||||
skip_after_callbacks_if_terminated: true,
|
||||
scope: [:kind, :name],
|
||||
only: [:before, :around, :after]
|
||||
|
|
|
@ -23,6 +23,7 @@ module ActiveModel
|
|||
included do
|
||||
include ActiveSupport::Callbacks
|
||||
define_callbacks :validation,
|
||||
terminator: deprecated_false_terminator,
|
||||
skip_after_callbacks_if_terminated: true,
|
||||
scope: [:kind, :name]
|
||||
end
|
||||
|
|
|
@ -11,6 +11,7 @@ module ActiveRecord
|
|||
:before_commit_without_transaction_enrollment,
|
||||
:commit_without_transaction_enrollment,
|
||||
:rollback_without_transaction_enrollment,
|
||||
terminator: deprecated_false_terminator,
|
||||
scope: [:kind, :name]
|
||||
end
|
||||
|
||||
|
|
|
@ -4,7 +4,6 @@ module ActiveRecord
|
|||
class AttributeTest < ActiveRecord::TestCase
|
||||
setup do
|
||||
@type = Minitest::Mock.new
|
||||
@type.expect(:==, false, [false])
|
||||
end
|
||||
|
||||
teardown do
|
||||
|
|
|
@ -277,20 +277,17 @@
|
|||
|
||||
The preferred method to halt a callback chain from now on is to explicitly
|
||||
`throw(:abort)`.
|
||||
In the past, returning `false` in an ActiveSupport callback had the side
|
||||
effect of halting the callback chain. This is not recommended anymore and,
|
||||
depending on the value of
|
||||
`Callbacks::CallbackChain.halt_and_display_warning_on_return_false`, will
|
||||
either not work at all or display a deprecation warning.
|
||||
In the past, callbacks could only be halted by explicitly providing a
|
||||
terminator and by having a callback match the conditions of the terminator.
|
||||
|
||||
* Add `Callbacks::CallbackChain.halt_and_display_warning_on_return_false`
|
||||
|
||||
Setting `Callbacks::CallbackChain.halt_and_display_warning_on_return_false`
|
||||
to `true` will let an app support the deprecated way of halting callback
|
||||
chains by returning `false`.
|
||||
to `true` will let an app support the deprecated way of halting Active Record,
|
||||
Active Model and Active Model validations callback chains by returning `false`.
|
||||
|
||||
Setting the value to `false` will tell the app to ignore any `false` value
|
||||
returned by callbacks, and only halt the chain upon `throw(:abort)`.
|
||||
returned by those callbacks, and only halt the chain upon `throw(:abort)`.
|
||||
|
||||
The value can also be set with the Rails configuration option
|
||||
`config.active_support.halt_callback_chains_on_return_false`.
|
||||
|
@ -300,7 +297,7 @@
|
|||
For new Rails 5.0 apps, its value is set to `false` in an initializer, so
|
||||
these apps will support the new behavior by default.
|
||||
|
||||
*claudiob*
|
||||
*claudiob*, *Roque Pinel*
|
||||
|
||||
* Changes arguments and default value of CallbackChain's `:terminator` option
|
||||
|
||||
|
|
|
@ -536,23 +536,12 @@ module ActiveSupport
|
|||
Proc.new do |target, result_lambda|
|
||||
terminate = true
|
||||
catch(:abort) do
|
||||
result = result_lambda.call if result_lambda.is_a?(Proc)
|
||||
if halt_and_display_warning_on_return_false && result == false
|
||||
display_deprecation_warning_for_false_terminator
|
||||
else
|
||||
terminate = false
|
||||
end
|
||||
result_lambda.call if result_lambda.is_a?(Proc)
|
||||
terminate = false
|
||||
end
|
||||
terminate
|
||||
end
|
||||
end
|
||||
|
||||
def display_deprecation_warning_for_false_terminator
|
||||
ActiveSupport::Deprecation.warn(<<-MSG.squish)
|
||||
Returning `false` in a callback will not implicitly halt a callback chain in the next release of Rails.
|
||||
To explicitly halt a callback chain, please use `throw :abort` instead.
|
||||
MSG
|
||||
end
|
||||
end
|
||||
|
||||
module ClassMethods
|
||||
|
@ -686,7 +675,8 @@ module ActiveSupport
|
|||
#
|
||||
# In this example, if any before validate callbacks returns +false+,
|
||||
# any successive before and around callback is not executed.
|
||||
# Defaults to +false+, meaning no value halts the chain.
|
||||
#
|
||||
# The default terminator halts the chain when a callback throws +:abort+.
|
||||
#
|
||||
# * <tt>:skip_after_callbacks_if_terminated</tt> - Determines if after
|
||||
# callbacks should be terminated by the <tt>:terminator</tt> option. By
|
||||
|
@ -764,6 +754,30 @@ module ActiveSupport
|
|||
def set_callbacks(name, callbacks)
|
||||
send "_#{name}_callbacks=", callbacks
|
||||
end
|
||||
|
||||
def deprecated_false_terminator
|
||||
Proc.new do |target, result_lambda|
|
||||
terminate = true
|
||||
catch(:abort) do
|
||||
result = result_lambda.call if result_lambda.is_a?(Proc)
|
||||
if CallbackChain.halt_and_display_warning_on_return_false && result == false
|
||||
display_deprecation_warning_for_false_terminator
|
||||
else
|
||||
terminate = false
|
||||
end
|
||||
end
|
||||
terminate
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def display_deprecation_warning_for_false_terminator
|
||||
ActiveSupport::Deprecation.warn(<<-MSG.squish)
|
||||
Returning `false` in Active Record and Active Model callbacks will not implicitly halt a callback chain in the next release of Rails.
|
||||
To explicitly halt the callback chain, please use `throw :abort` instead.
|
||||
MSG
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -766,13 +766,11 @@ module CallbacksTest
|
|||
end
|
||||
|
||||
class CallbackFalseTerminatorWithoutConfigTest < ActiveSupport::TestCase
|
||||
def test_returning_false_halts_callback_if_config_variable_is_not_set
|
||||
def test_returning_false_does_not_halt_callback_if_config_variable_is_not_set
|
||||
obj = CallbackFalseTerminator.new
|
||||
assert_deprecated do
|
||||
obj.save
|
||||
assert_equal :second, obj.halted
|
||||
assert !obj.saved
|
||||
end
|
||||
obj.save
|
||||
assert_equal nil, obj.halted
|
||||
assert obj.saved
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -781,13 +779,11 @@ module CallbacksTest
|
|||
ActiveSupport::Callbacks::CallbackChain.halt_and_display_warning_on_return_false = true
|
||||
end
|
||||
|
||||
def test_returning_false_halts_callback_if_config_variable_is_true
|
||||
def test_returning_false_does_not_halt_callback_if_config_variable_is_true
|
||||
obj = CallbackFalseTerminator.new
|
||||
assert_deprecated do
|
||||
obj.save
|
||||
assert_equal :second, obj.halted
|
||||
assert !obj.saved
|
||||
end
|
||||
obj.save
|
||||
assert_equal nil, obj.halted
|
||||
assert obj.saved
|
||||
end
|
||||
end
|
||||
|
||||
|
|
Loading…
Reference in a new issue