diff options
| -rw-r--r-- | activemodel/lib/active_model/callbacks.rb | 1 | ||||
| -rw-r--r-- | activemodel/lib/active_model/validations/callbacks.rb | 1 | ||||
| -rw-r--r-- | activerecord/lib/active_record/transactions.rb | 1 | ||||
| -rw-r--r-- | activerecord/test/cases/attribute_test.rb | 1 | ||||
| -rw-r--r-- | activesupport/CHANGELOG.md | 15 | ||||
| -rw-r--r-- | activesupport/lib/active_support/callbacks.rb | 42 | ||||
| -rw-r--r-- | activesupport/test/callbacks_test.rb | 20 |
7 files changed, 45 insertions, 36 deletions
diff --git a/activemodel/lib/active_model/callbacks.rb b/activemodel/lib/active_model/callbacks.rb index 2cf39b68fb..0d6a3dc52d 100644 --- a/activemodel/lib/active_model/callbacks.rb +++ b/activemodel/lib/active_model/callbacks.rb @@ -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] diff --git a/activemodel/lib/active_model/validations/callbacks.rb b/activemodel/lib/active_model/validations/callbacks.rb index 4b58ef66e3..52111e5442 100644 --- a/activemodel/lib/active_model/validations/callbacks.rb +++ b/activemodel/lib/active_model/validations/callbacks.rb @@ -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 diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 4a569fc242..1a2988ea77 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -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 diff --git a/activerecord/test/cases/attribute_test.rb b/activerecord/test/cases/attribute_test.rb index 0ec368f51d..c69782d93f 100644 --- a/activerecord/test/cases/attribute_test.rb +++ b/activerecord/test/cases/attribute_test.rb @@ -4,7 +4,6 @@ module ActiveRecord class AttributeTest < ActiveRecord::TestCase setup do @type = Minitest::Mock.new - @type.expect(:==, false, [false]) end teardown do diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 69413fef47..309d842da1 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -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 diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 80c5fdba17..3db9ea2ac0 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -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 diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index cda9732cae..4f47e0519f 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -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 |