diff options
author | Iain Beeston <iain.beeston@gmail.com> | 2015-02-21 14:37:07 +0000 |
---|---|---|
committer | Iain Beeston <iain.beeston@gmail.com> | 2015-04-03 09:37:19 +0100 |
commit | d2876141d08341ec67cf6a11a073d1acfb920de7 (patch) | |
tree | 4d69bd159f9c16acb328adb680a090fc9f49470e /activesupport/lib/active_support/callbacks.rb | |
parent | a05f3e5f96f7a8aa55483d91becdbe49b81833fd (diff) | |
download | rails-d2876141d08341ec67cf6a11a073d1acfb920de7.tar.gz rails-d2876141d08341ec67cf6a11a073d1acfb920de7.tar.bz2 rails-d2876141d08341ec67cf6a11a073d1acfb920de7.zip |
Raise ArgumentError if an unrecognised callback is skipped
At present, if you skip a callback that hasn't been defined,
activesupport callbacks silently does nothing. However, it's easy to
mistype the name of a callback and mistakenly think that it's being
skipped, when it is not.
This problem even exists in the current test suite.
CallbacksTest::SkipCallbacksTest#test_skip_person attempts to skip
callbacks that were never set up.
This PR changes `skip_callback` to raise an `ArgumentError` if the
specified callback cannot be found.
Diffstat (limited to 'activesupport/lib/active_support/callbacks.rb')
-rw-r--r-- | activesupport/lib/active_support/callbacks.rb | 18 |
1 files changed, 13 insertions, 5 deletions
diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 9cf09ab266..19d2b1099d 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -691,19 +691,27 @@ module ActiveSupport # class Writer < Person # skip_callback :validate, :before, :check_membership, if: -> { self.age > 18 } # end + # + # An <tt>ArgumentError</tt> will be raised if the callback has not + # already been set (unless the <tt>:raise</tt> option is set to <tt>false</tt>). def skip_callback(name, *filter_list, &block) type, filters, options = normalize_callback_params(filter_list, block) + options[:raise] = true unless options.key?(:raise) __update_callbacks(name) do |target, chain| filters.each do |filter| - filter = chain.find {|c| c.matches?(type, filter) } + callback = chain.find {|c| c.matches?(type, filter) } + + if !callback && options[:raise] + raise ArgumentError, "#{type.to_s.capitalize} #{name} callback #{filter.inspect} has not been defined" + end - if filter && options.any? - new_filter = filter.merge_conditional_options(chain, if_option: options[:if], unless_option: options[:unless]) - chain.insert(chain.index(filter), new_filter) + if callback && (options.key?(:if) || options.key?(:unless)) + new_callback = callback.merge_conditional_options(chain, if_option: options[:if], unless_option: options[:unless]) + chain.insert(chain.index(callback), new_callback) end - chain.delete(filter) + chain.delete(callback) end target.set_callbacks name, chain end |