Merge pull request #21760 from repinel/refactor-as-callbacks-halt-config

Refactor AS::Callbacks halt config and fix the documentation

9 files changed, 38 insertions, 36 deletions

diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md
index bf0120122d..a3368cd197 100644
--- a/activemodel/CHANGELOG.md
+++ b/activemodel/CHANGELOG.md
@@ -119,10 +119,10 @@
 
     The preferred method to halt a callback chain from now on is to explicitly
     `throw(:abort)`.
-    In the past, returning `false` in an ActiveModel or ActiveModel::Validations
-    `before_` callback had the side effect of halting the callback chain.
+    In the past, returning `false` in an Active Model `before_` callback had
+    the side effect of halting the callback chain.
     This is not recommended anymore and, depending on the value of the
-    `config.active_support.halt_callback_chains_on_return_false` option, will
+    `ActiveSupport.halt_callback_chains_on_return_false` option, will
     either not work at all or display a deprecation warning.
 
 
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 50a556daff..6a40d32ef9 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1117,7 +1117,7 @@
     In the past, returning `false` in an Active Record `before_` callback had the
     side effect of halting the callback chain.
     This is not recommended anymore and, depending on the value of the
-    `config.active_support.halt_callback_chains_on_return_false` option, will
+    `ActiveSupport.halt_callback_chains_on_return_false` option, will
     either not work at all or display a deprecation warning.
 
     *claudiob*
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index a39344e464..19588d622c 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -291,18 +291,15 @@
     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`
+*   Add `ActiveSupport.halt_callback_chains_on_return_false`
 
-    Setting `Callbacks::CallbackChain.halt_and_display_warning_on_return_false`
+    Setting `ActiveSupport.halt_callback_chains_on_return_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`.
+    and Active Model callback chains by returning `false`.
 
     Setting the value to `false` will tell the app to ignore any `false` value
     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`.
-
     When the configuration option is missing, its value is `true`, so older apps
     ported to Rails 5.0 will not break (but display a deprecation warning).
     For new Rails 5.0 apps, its value is set to `false` in an initializer, so
diff --git a/activesupport/lib/active_support.rb b/activesupport/lib/active_support.rb
index 588d6c49f9..63277a65b4 100644
--- a/activesupport/lib/active_support.rb
+++ b/activesupport/lib/active_support.rb
@@ -75,11 +75,11 @@ module ActiveSupport
   cattr_accessor :test_order # :nodoc:
 
   def self.halt_callback_chains_on_return_false
-    Callbacks::CallbackChain.halt_and_display_warning_on_return_false
+    Callbacks.halt_and_display_warning_on_return_false
   end
 
   def self.halt_callback_chains_on_return_false=(value)
-    Callbacks::CallbackChain.halt_and_display_warning_on_return_false = value
+    Callbacks.halt_and_display_warning_on_return_false = value
   end
 end
 
diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb
index 3db9ea2ac0..252374e817 100644
--- a/activesupport/lib/active_support/callbacks.rb
+++ b/activesupport/lib/active_support/callbacks.rb
@@ -4,6 +4,7 @@ require 'active_support/core_ext/array/extract_options'
 require 'active_support/core_ext/class/attribute'
 require 'active_support/core_ext/kernel/reporting'
 require 'active_support/core_ext/kernel/singleton_class'
+require 'active_support/core_ext/module/attribute_accessors'
 require 'active_support/core_ext/string/filters'
 require 'active_support/deprecation'
 require 'thread'
@@ -66,6 +67,12 @@ module ActiveSupport
 
     CALLBACK_FILTER_TYPES = [:before, :after, :around]
 
+    # If true, Active Record and Active Model callbacks returning +false+ will
+    # halt the entire callback chain and display a deprecation message.
+    # If false, callback chains will only be halted by calling +throw :abort+.
+    # Defaults to +true+.
+    mattr_accessor(:halt_and_display_warning_on_return_false) { true }
+
     # Runs the callbacks for the given event.
     #
     # Calls the before and around callbacks in the order they were set, yields
@@ -450,12 +457,6 @@ module ActiveSupport
 
       attr_reader :name, :config
 
-      # If true, any callback returning +false+ will halt the entire callback
-      # chain and display a deprecation message. If false, callback chains will
-      # only be halted by calling +throw :abort+. Defaults to +true+.
-      class_attribute :halt_and_display_warning_on_return_false
-      self.halt_and_display_warning_on_return_false = true
-
       def initialize(name, config)
         @name = name
         @config = {
@@ -760,7 +761,7 @@ module ActiveSupport
           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
+            if Callbacks.halt_and_display_warning_on_return_false && result == false
               display_deprecation_warning_for_false_terminator
             else
               terminate = false
diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb
index 4f47e0519f..3b00ff87a0 100644
--- a/activesupport/test/callbacks_test.rb
+++ b/activesupport/test/callbacks_test.rb
@@ -776,7 +776,7 @@ module CallbacksTest
 
   class CallbackFalseTerminatorWithConfigTrueTest < ActiveSupport::TestCase
     def setup
-      ActiveSupport::Callbacks::CallbackChain.halt_and_display_warning_on_return_false = true
+      ActiveSupport::Callbacks.halt_and_display_warning_on_return_false = true
     end
 
     def test_returning_false_does_not_halt_callback_if_config_variable_is_true
@@ -789,7 +789,7 @@ module CallbacksTest
 
   class CallbackFalseTerminatorWithConfigFalseTest < ActiveSupport::TestCase
     def setup
-      ActiveSupport::Callbacks::CallbackChain.halt_and_display_warning_on_return_false = false
+      ActiveSupport::Callbacks.halt_and_display_warning_on_return_false = false
     end
 
     def test_returning_false_does_not_halt_callback_if_config_variable_is_false
diff --git a/guides/source/configuring.md b/guides/source/configuring.md
index f0d87e4dc8..afdb0ba7eb 100644
--- a/guides/source/configuring.md
+++ b/guides/source/configuring.md
@@ -535,7 +535,7 @@ There are a few configuration options available in Active Support:
 
 * `config.active_support.time_precision` sets the precision of JSON encoded time values. Defaults to `3`.
 
-* `config.active_support.halt_callback_chains_on_return_false` specifies whether ActiveRecord, ActiveModel and ActiveModel::Validations callback chains can be halted by returning `false` in a 'before' callback. Defaults to `true`.
+* `ActiveSupport.halt_callback_chains_on_return_false` specifies whether Active Record and Active Model callback chains can be halted by returning `false` in a 'before' callback. Defaults to `true`.
 
 * `ActiveSupport::Logger.silencer` is set to `false` to disable the ability to silence logging in a block. The default is `true`.
 
diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md
index 52464a1c51..490bda3571 100644
--- a/guides/source/upgrading_ruby_on_rails.md
+++ b/guides/source/upgrading_ruby_on_rails.md
@@ -55,23 +55,26 @@ Upgrading from Rails 4.2 to Rails 5.0
 
 ### Halting callback chains by returning `false`
 
-In Rails 4.2, when a 'before' callback returns `false` in ActiveRecord,
-ActiveModel and ActiveModel::Validations, then the entire callback chain
-is halted. In other words, successive 'before' callbacks are not executed,
-and neither is the action wrapped in callbacks.
+In Rails 4.2, when a 'before' callback returns `false` in Active Record
+and Active Model, then the entire callback chain is halted. In other words,
+successive 'before' callbacks are not executed, and neither is the action wrapped
+in callbacks.
 
-In Rails 5.0, returning `false` in a callback will not have this side effect
-of halting the callback chain. Instead, callback chains must be explicitly
-halted by calling `throw(:abort)`.
+In Rails 5.0, returning `false` in an Active Record or Active Model callback
+will not have this side effect of halting the callback chain. Instead, callback
+chains must be explicitly halted by calling `throw(:abort)`.
 
-When you upgrade from Rails 4.2 to Rails 5.0, returning `false` in a callback
-will still halt the callback chain, but you will receive a deprecation warning
-about this upcoming change.
+When you upgrade from Rails 4.2 to Rails 5.0, returning `false` in those kind of
+callbacks will still halt the callback chain, but you will receive a deprecation
+warning about this upcoming change.
 
 When you are ready, you can opt into the new behavior and remove the deprecation
 warning by adding the following configuration to your `config/application.rb`:
 
-    config.active_support.halt_callback_chains_on_return_false = false
+    ActiveSupport.halt_callback_chains_on_return_false = false
+
+Note that this option will not affect Active Support callbacks since they never
+halted the chain when any value was returned.
 
 See [#17227](https://github.com/rails/rails/pull/17227) for more details.
 
diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md
index 80ef1af7b5..3e45a09dec 100644
--- a/railties/CHANGELOG.md
+++ b/railties/CHANGELOG.md
@@ -296,10 +296,11 @@
 
     Newly generated Rails apps have a new initializer called
     `callback_terminator.rb` which sets the value of the configuration option
-    `config.active_support.halt_callback_chains_on_return_false` to `false`.
+    `ActiveSupport.halt_callback_chains_on_return_false` to `false`.
 
-    As a result, new Rails apps do not halt callback chains when a callback
-    returns `false`; only when they are explicitly halted with `throw(:abort)`.
+    As a result, new Rails apps do not halt Active Record and Active Model
+    callback chains when a callback returns `false`; only when they are
+    explicitly halted with `throw(:abort)`.
 
     The terminator is *not* added when running `rake rails:update`, so returning
     `false` will still work on old apps ported to Rails 5, displaying a