From e2b3ccd1aa56ae467b0fe5c7466136a4d18fa7ef Mon Sep 17 00:00:00 2001 From: Roque Pinel Date: Thu, 24 Sep 2015 21:33:58 -0400 Subject: [PATCH] Refactor AS::Callbacks halt config and fix the documentation Move from `AS::Callbacks::CallbackChain.halt_and_display_warning_on_return_false` to `AS::Callbacks.halt_and_display_warning_on_return_false` base on [this discussion](https://github.com/rails/rails/pull/21218#discussion_r39354580) Fix the documentation broken by 0a120a818d413c64ff9867125f0b03788fc306f8 --- activemodel/CHANGELOG.md | 6 ++--- activerecord/CHANGELOG.md | 2 +- activesupport/CHANGELOG.md | 9 +++---- activesupport/lib/active_support.rb | 4 +-- activesupport/lib/active_support/callbacks.rb | 15 +++++------ activesupport/test/callbacks_test.rb | 4 +-- guides/source/configuring.md | 2 +- guides/source/upgrading_ruby_on_rails.md | 25 +++++++++++-------- railties/CHANGELOG.md | 7 +++--- 9 files changed, 38 insertions(+), 36 deletions(-) diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index bf0120122d6d2..a3368cd197a12 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 50a556daffea3..6a40d32ef95d7 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 a39344e464163..19588d622c7cc 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 588d6c49f9511..63277a65b4770 100644 --- a/activesupport/lib/active_support.rb +++ b/activesupport/lib/active_support.rb @@ -75,11 +75,11 @@ def self.eager_load! 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 3db9ea2ac0e0e..252374e817575 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -4,6 +4,7 @@ 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 Callbacks 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 @@ class CallbackChain #:nodoc:# 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 @@ def deprecated_false_terminator 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 4f47e0519fdef..3b00ff87a09be 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -776,7 +776,7 @@ def test_returning_false_does_not_halt_callback_if_config_variable_is_not_set 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 @@ def test_returning_false_does_not_halt_callback_if_config_variable_is_true 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 f0d87e4dc8c8f..afdb0ba7eb430 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 52464a1c51635..490bda357165d 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 80ef1af7b557c..3e45a09dec090 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