diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 2dfde11707..b67a803b9d 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,3 +1,7 @@ +* Fix to working before/after validation callbacks on multiple contexts. + + *Yoshiyuki Hirano* + ## Rails 5.2.0.beta2 (November 28, 2017) ## * No changes. diff --git a/activemodel/lib/active_model/validations/callbacks.rb b/activemodel/lib/active_model/validations/callbacks.rb index 4d0ab2a2fe..11a8b2b229 100644 --- a/activemodel/lib/active_model/validations/callbacks.rb +++ b/activemodel/lib/active_model/validations/callbacks.rb @@ -54,14 +54,16 @@ module ActiveModel # person.valid? # => true # person.name # => "bob" def before_validation(*args, &block) - options = args.last - if options.is_a?(Hash) && options[:on] - options[:if] = Array(options[:if]) - options[:on] = Array(options[:on]) + options = args.extract_options! + options[:if] = Array(options[:if]) + + if options.key?(:on) options[:if].unshift ->(o) { - options[:on].include? o.validation_context + !(Array(options[:on]) & Array(o.validation_context)).empty? } end + + args << options set_callback(:validation, :before, *args, &block) end @@ -95,13 +97,15 @@ module ActiveModel options = args.extract_options! options[:prepend] = true options[:if] = Array(options[:if]) - if options[:on] - options[:on] = Array(options[:on]) + + if options.key?(:on) options[:if].unshift ->(o) { - options[:on].include? o.validation_context + !(Array(options[:on]) & Array(o.validation_context)).empty? } end - set_callback(:validation, :after, *(args << options), &block) + + args << options + set_callback(:validation, :after, *args, &block) end end diff --git a/activemodel/test/cases/validations/callbacks_test.rb b/activemodel/test/cases/validations/callbacks_test.rb index d3a9a17a05..ff3cf61746 100644 --- a/activemodel/test/cases/validations/callbacks_test.rb +++ b/activemodel/test/cases/validations/callbacks_test.rb @@ -59,6 +59,18 @@ class DogValidatorWithOnCondition < Dog def set_after_validation_marker; history << "after_validation_marker" ; end end +class DogValidatorWithOnMultipleCondition < Dog + before_validation :set_before_validation_marker_on_context_a, on: :context_a + before_validation :set_before_validation_marker_on_context_b, on: :context_b + after_validation :set_after_validation_marker_on_context_a, on: :context_a + after_validation :set_after_validation_marker_on_context_b, on: :context_b + + def set_before_validation_marker_on_context_a; history << "before_validation_marker on context_a"; end + def set_before_validation_marker_on_context_b; history << "before_validation_marker on context_b"; end + def set_after_validation_marker_on_context_a; history << "after_validation_marker on context_a" ; end + def set_after_validation_marker_on_context_b; history << "after_validation_marker on context_b" ; end +end + class DogValidatorWithIfCondition < Dog before_validation :set_before_validation_marker1, if: -> { true } before_validation :set_before_validation_marker2, if: -> { false } @@ -98,6 +110,37 @@ class CallbacksWithMethodNamesShouldBeCalled < ActiveModel::TestCase assert_equal [], d.history end + def test_on_multiple_condition_is_respected_for_validation_with_matching_context + d = DogValidatorWithOnMultipleCondition.new + d.valid?(:context_a) + assert_equal ["before_validation_marker on context_a", "after_validation_marker on context_a"], d.history + + d = DogValidatorWithOnMultipleCondition.new + d.valid?(:context_b) + assert_equal ["before_validation_marker on context_b", "after_validation_marker on context_b"], d.history + + d = DogValidatorWithOnMultipleCondition.new + d.valid?([:context_a, :context_b]) + assert_equal([ + "before_validation_marker on context_a", + "before_validation_marker on context_b", + "after_validation_marker on context_a", + "after_validation_marker on context_b" + ], d.history) + end + + def test_on_multiple_condition_is_respected_for_validation_without_matching_context + d = DogValidatorWithOnMultipleCondition.new + d.valid?(:save) + assert_equal [], d.history + end + + def test_on_multiple_condition_is_respected_for_validation_without_context + d = DogValidatorWithOnMultipleCondition.new + d.valid? + assert_equal [], d.history + end + def test_before_validation_and_after_validation_callbacks_should_be_called d = DogWithMethodCallbacks.new d.valid?