Merge pull request #19851 from repinel/numericality-validation2

Use the post-type-cast version of the attribute to validate numericality

2 files changed, 47 insertions, 13 deletions

diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb
index 4ba4e3e8f7..9c1e8b4ba7 100644
--- a/activemodel/lib/active_model/validations/numericality.rb
+++ b/activemodel/lib/active_model/validations/numericality.rb
@@ -20,7 +20,7 @@ module ActiveModel
       def validate_each(record, attr_name, value)
         before_type_cast = :"#{attr_name}_before_type_cast"
 
-        raw_value = record.send(before_type_cast) if record.respond_to?(before_type_cast)
+        raw_value = record.send(before_type_cast) if record.respond_to?(before_type_cast) && record.send(before_type_cast) != value
         raw_value ||= value
 
         if record_attribute_changed_in_place?(record, attr_name)
@@ -29,16 +29,14 @@ module ActiveModel
 
         return if options[:allow_nil] && raw_value.nil?
 
-        unless value = parse_raw_value_as_a_number(raw_value)
+        unless is_number?(raw_value)
           record.errors.add(attr_name, :not_a_number, filtered_options(raw_value))
           return
         end
 
-        if allow_only_integer?(record)
-          unless value = parse_raw_value_as_an_integer(raw_value)
-            record.errors.add(attr_name, :not_an_integer, filtered_options(raw_value))
-            return
-          end
+        if allow_only_integer?(record) && !is_integer?(raw_value)
+          record.errors.add(attr_name, :not_an_integer, filtered_options(raw_value))
+          return
         end
 
         options.slice(*CHECKS.keys).each do |option, option_value|
@@ -64,14 +62,15 @@ module ActiveModel
 
     protected
 
-      def parse_raw_value_as_a_number(raw_value)
-        Kernel.Float(raw_value) if raw_value !~ /\A0[xX]/
+      def is_number?(raw_value)
+        parsed_value = Kernel.Float(raw_value) if raw_value !~ /\A0[xX]/
+        !parsed_value.nil?
       rescue ArgumentError, TypeError
-        nil
+        false
       end
 
-      def parse_raw_value_as_an_integer(raw_value)
-        raw_value.to_i if raw_value.to_s =~ /\A[+-]?\d+\z/
+      def is_integer?(raw_value)
+        /\A[+-]?\d+\z/ === raw_value.to_s
       end
 
       def filtered_options(value)
diff --git a/activemodel/test/cases/validations/numericality_validation_test.rb b/activemodel/test/cases/validations/numericality_validation_test.rb
index 05432abaff..4db6c828f8 100644
--- a/activemodel/test/cases/validations/numericality_validation_test.rb
+++ b/activemodel/test/cases/validations/numericality_validation_test.rb
@@ -71,6 +71,13 @@ class NumericalityValidationTest < ActiveModel::TestCase
     valid!([11])
   end
 
+  def test_validates_numericality_with_greater_than_using_differing_numeric_types
+    Topic.validates_numericality_of :approved, greater_than: BigDecimal.new('97.18')
+
+    invalid!([-97.18, BigDecimal.new('97.18'), BigDecimal('-97.18')], 'must be greater than 97.18')
+    valid!([97.18, 98, BigDecimal.new('98')]) # Notice the 97.18 as a float is greater than 97.18 as a BigDecimal due to floating point precision
+  end
+
   def test_validates_numericality_with_greater_than_or_equal
     Topic.validates_numericality_of :approved, greater_than_or_equal_to: 10
 
@@ -78,6 +85,13 @@ class NumericalityValidationTest < ActiveModel::TestCase
     valid!([10])
   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') 
+
+    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')])
+  end
+
   def test_validates_numericality_with_equal_to
     Topic.validates_numericality_of :approved, equal_to: 10
 
@@ -85,6 +99,13 @@ class NumericalityValidationTest < ActiveModel::TestCase
     valid!([10])
   end
 
+  def test_validates_numericality_with_equal_to_using_differing_numeric_types
+    Topic.validates_numericality_of :approved, equal_to: BigDecimal.new('97.18')
+
+    invalid!([-97.18, 97.18], 'must be equal to 97.18')
+    valid!([BigDecimal.new('97.18')])
+  end
+
   def test_validates_numericality_with_less_than
     Topic.validates_numericality_of :approved, less_than: 10
 
@@ -92,6 +113,13 @@ class NumericalityValidationTest < ActiveModel::TestCase
     valid!([-9, 9])
   end
 
+  def test_validates_numericality_with_less_than_using_differing_numeric_types
+    Topic.validates_numericality_of :approved, less_than: BigDecimal.new('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')])
+  end
+
   def test_validates_numericality_with_less_than_or_equal_to
     Topic.validates_numericality_of :approved, less_than_or_equal_to: 10
 
@@ -99,6 +127,13 @@ class NumericalityValidationTest < ActiveModel::TestCase
     valid!([-10, 10])
   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')
+
+    invalid!([97.18, 98], 'must be less than or equal to 97.18')
+    valid!([-97.18, BigDecimal.new('-97.18'), BigDecimal.new('97.18')])
+  end
+
   def test_validates_numericality_with_odd
     Topic.validates_numericality_of :approved, odd: true
 
@@ -196,7 +231,7 @@ class NumericalityValidationTest < ActiveModel::TestCase
 
   def valid!(values)
     with_each_topic_approved_value(values) do |topic, value|
-      assert topic.valid?, "#{value.inspect} not accepted as a number"
+      assert topic.valid?, "#{value.inspect} not accepted as a number with validation error: #{topic.errors[:approved].first}"
     end
   end