aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--activemodel/lib/active_model/callbacks.rb1
-rw-r--r--activemodel/lib/active_model/validations/callbacks.rb1
-rw-r--r--activerecord/lib/active_record/transactions.rb1
-rw-r--r--activerecord/test/cases/attribute_test.rb1
-rw-r--r--activesupport/CHANGELOG.md15
-rw-r--r--activesupport/lib/active_support/callbacks.rb42
-rw-r--r--activesupport/test/callbacks_test.rb20
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