diff options
Diffstat (limited to 'activemodel')
| -rw-r--r-- | activemodel/CHANGELOG.md | 4 | ||||
| -rw-r--r-- | activemodel/lib/active_model/type.rb | 2 | ||||
| -rw-r--r-- | activemodel/lib/active_model/validations/callbacks.rb | 22 | ||||
| -rw-r--r-- | activemodel/lib/active_model/validations/validates.rb | 2 | ||||
| -rw-r--r-- | activemodel/test/cases/attributes_test.rb | 2 | ||||
| -rw-r--r-- | activemodel/test/cases/type/decimal_test.rb | 16 | ||||
| -rw-r--r-- | activemodel/test/cases/validations/callbacks_test.rb | 43 | ||||
| -rw-r--r-- | activemodel/test/cases/validations/numericality_validation_test.rb | 28 |
8 files changed, 85 insertions, 34 deletions
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/type.rb b/activemodel/lib/active_model/type.rb index 39324999c9..1d7a26fff5 100644 --- a/activemodel/lib/active_model/type.rb +++ b/activemodel/lib/active_model/type.rb @@ -24,7 +24,7 @@ module ActiveModel class << self attr_accessor :registry # :nodoc: - # Add a new type to the registry, allowing it to be get through ActiveModel::Type#lookup + # Add a new type to the registry, allowing it to be gotten through ActiveModel::Type#lookup def register(type_name, klass = nil, **options, &block) registry.register(type_name, klass, **options, &block) end 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/lib/active_model/validations/validates.rb b/activemodel/lib/active_model/validations/validates.rb index 43d9f82d9f..e28e7e9219 100644 --- a/activemodel/lib/active_model/validations/validates.rb +++ b/activemodel/lib/active_model/validations/validates.rb @@ -154,7 +154,7 @@ module ActiveModel # When creating custom validators, it might be useful to be able to specify # additional default keys. This can be done by overwriting this method. def _validates_default_keys - [:if, :unless, :on, :allow_blank, :allow_nil , :strict] + [:if, :unless, :on, :allow_blank, :allow_nil, :strict] end def _parse_validates_options(options) diff --git a/activemodel/test/cases/attributes_test.rb b/activemodel/test/cases/attributes_test.rb index 914aee1ac0..e43bf15335 100644 --- a/activemodel/test/cases/attributes_test.rb +++ b/activemodel/test/cases/attributes_test.rb @@ -33,7 +33,7 @@ module ActiveModel assert_equal 2, data.integer_field assert_equal "Rails FTW", data.string_field - assert_equal BigDecimal.new("12.3"), data.decimal_field + assert_equal BigDecimal("12.3"), data.decimal_field assert_equal "default string", data.string_with_default assert_equal Date.new(2016, 1, 1), data.date_field assert_equal false, data.boolean_field diff --git a/activemodel/test/cases/type/decimal_test.rb b/activemodel/test/cases/type/decimal_test.rb index b91d67f95f..c0cf6ce590 100644 --- a/activemodel/test/cases/type/decimal_test.rb +++ b/activemodel/test/cases/type/decimal_test.rb @@ -7,22 +7,22 @@ module ActiveModel class DecimalTest < ActiveModel::TestCase def test_type_cast_decimal type = Decimal.new - assert_equal BigDecimal.new("0"), type.cast(BigDecimal.new("0")) - assert_equal BigDecimal.new("123"), type.cast(123.0) - assert_equal BigDecimal.new("1"), type.cast(:"1") + assert_equal BigDecimal("0"), type.cast(BigDecimal("0")) + assert_equal BigDecimal("123"), type.cast(123.0) + assert_equal BigDecimal("1"), type.cast(:"1") end def test_type_cast_decimal_from_invalid_string type = Decimal.new assert_nil type.cast("") - assert_equal BigDecimal.new("1"), type.cast("1ignore") - assert_equal BigDecimal.new("0"), type.cast("bad1") - assert_equal BigDecimal.new("0"), type.cast("bad") + assert_equal BigDecimal("1"), type.cast("1ignore") + assert_equal BigDecimal("0"), type.cast("bad1") + assert_equal BigDecimal("0"), type.cast("bad") end def test_type_cast_decimal_from_float_with_large_precision type = Decimal.new(precision: ::Float::DIG + 2) - assert_equal BigDecimal.new("123.0"), type.cast(123.0) + assert_equal BigDecimal("123.0"), type.cast(123.0) end def test_type_cast_from_float_with_unspecified_precision @@ -48,7 +48,7 @@ module ActiveModel def test_type_cast_decimal_from_object_responding_to_d value = Object.new def value.to_d - BigDecimal.new("1") + BigDecimal("1") end type = Decimal.new assert_equal BigDecimal("1"), type.cast(value) 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? diff --git a/activemodel/test/cases/validations/numericality_validation_test.rb b/activemodel/test/cases/validations/numericality_validation_test.rb index fbbaf8a30c..5413255e6b 100644 --- a/activemodel/test/cases/validations/numericality_validation_test.rb +++ b/activemodel/test/cases/validations/numericality_validation_test.rb @@ -20,7 +20,7 @@ class NumericalityValidationTest < ActiveModel::TestCase INTEGER_STRINGS = %w(0 +0 -0 10 +10 -10 0090 -090) FLOATS = [0.0, 10.0, 10.5, -10.5, -0.0001] + FLOAT_STRINGS INTEGERS = [0, 10, -10] + INTEGER_STRINGS - BIGDECIMAL = BIGDECIMAL_STRINGS.collect! { |bd| BigDecimal.new(bd) } + BIGDECIMAL = BIGDECIMAL_STRINGS.collect! { |bd| BigDecimal(bd) } JUNK = ["not a number", "42 not a number", "0xdeadbeef", "0xinvalidhex", "0Xdeadbeef", "00-1", "--3", "+-3", "+3-1", "-+019.0", "12.12.13.12", "123\nnot a number"] INFINITY = [1.0 / 0.0] @@ -81,10 +81,10 @@ class NumericalityValidationTest < ActiveModel::TestCase end def test_validates_numericality_with_greater_than_using_differing_numeric_types - Topic.validates_numericality_of :approved, greater_than: BigDecimal.new("97.18") + Topic.validates_numericality_of :approved, greater_than: BigDecimal("97.18") - invalid!([-97.18, BigDecimal.new("97.18"), BigDecimal("-97.18")], "must be greater than 97.18") - valid!([97.19, 98, BigDecimal.new("98"), BigDecimal.new("97.19")]) + invalid!([-97.18, BigDecimal("97.18"), BigDecimal("-97.18")], "must be greater than 97.18") + valid!([97.19, 98, BigDecimal("98"), BigDecimal("97.19")]) end def test_validates_numericality_with_greater_than_using_string_value @@ -102,10 +102,10 @@ class NumericalityValidationTest < ActiveModel::TestCase end def test_validates_numericality_with_greater_than_or_equal_using_differing_numeric_types - Topic.validates_numericality_of :approved, greater_than_or_equal_to: BigDecimal.new("97.18") + Topic.validates_numericality_of :approved, greater_than_or_equal_to: BigDecimal("97.18") - invalid!([-97.18, 97.17, 97, BigDecimal.new("97.17"), BigDecimal.new("-97.18")], "must be greater than or equal to 97.18") - valid!([97.18, 98, BigDecimal.new("97.19")]) + invalid!([-97.18, 97.17, 97, BigDecimal("97.17"), BigDecimal("-97.18")], "must be greater than or equal to 97.18") + valid!([97.18, 98, BigDecimal("97.19")]) end def test_validates_numericality_with_greater_than_or_equal_using_string_value @@ -123,10 +123,10 @@ class NumericalityValidationTest < ActiveModel::TestCase end def test_validates_numericality_with_equal_to_using_differing_numeric_types - Topic.validates_numericality_of :approved, equal_to: BigDecimal.new("97.18") + Topic.validates_numericality_of :approved, equal_to: BigDecimal("97.18") invalid!([-97.18], "must be equal to 97.18") - valid!([BigDecimal.new("97.18")]) + valid!([BigDecimal("97.18")]) end def test_validates_numericality_with_equal_to_using_string_value @@ -144,10 +144,10 @@ class NumericalityValidationTest < ActiveModel::TestCase end def test_validates_numericality_with_less_than_using_differing_numeric_types - Topic.validates_numericality_of :approved, less_than: BigDecimal.new("97.18") + Topic.validates_numericality_of :approved, less_than: BigDecimal("97.18") - invalid!([97.18, BigDecimal.new("97.18")], "must be less than 97.18") - valid!([-97.0, 97.0, -97, 97, BigDecimal.new("-97"), BigDecimal.new("97")]) + invalid!([97.18, BigDecimal("97.18")], "must be less than 97.18") + valid!([-97.0, 97.0, -97, 97, BigDecimal("-97"), BigDecimal("97")]) end def test_validates_numericality_with_less_than_using_string_value @@ -165,10 +165,10 @@ class NumericalityValidationTest < ActiveModel::TestCase end def test_validates_numericality_with_less_than_or_equal_to_using_differing_numeric_types - Topic.validates_numericality_of :approved, less_than_or_equal_to: BigDecimal.new("97.18") + Topic.validates_numericality_of :approved, less_than_or_equal_to: BigDecimal("97.18") invalid!([97.19, 98], "must be less than or equal to 97.18") - valid!([-97.18, BigDecimal.new("-97.18"), BigDecimal.new("97.18")]) + valid!([-97.18, BigDecimal("-97.18"), BigDecimal("97.18")]) end def test_validates_numericality_with_less_than_or_equal_using_string_value |