Merge pull request #15895 from sgrif/sg-numeric-changes

Always assume strings with non-numeric characters change numeric types

3 files changed, 35 insertions, 3 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 33dec99ec7..6fc770d293 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,17 @@
+*   Assume numeric types have changed if they were assigned to a value that
+    would fail numericality validation, regardless of the old value. Previously
+    this would only occur if the old value was 0.
+
+    Example:
+
+        model = Model.create!(number: 5)
+        model.number = '5wibble'
+        model.number_changed? # => true
+
+    Fixes #14731.
+
+    *Sean Griffin*
+
 *   `reload` no longer merges with the existing attributes.
     The attribute hash is fully replaced. The record is put into the same state
     as it would be with `Model.find(model.id)`.
diff --git a/activerecord/lib/active_record/type/numeric.rb b/activerecord/lib/active_record/type/numeric.rb
index a7bf0657b9..fa43266504 100644
--- a/activerecord/lib/active_record/type/numeric.rb
+++ b/activerecord/lib/active_record/type/numeric.rb
@@ -16,13 +16,13 @@ module ActiveRecord
       end
 
       def changed?(old_value, _new_value, new_value_before_type_cast) # :nodoc:
-        super || zero_to_non_number?(old_value, new_value_before_type_cast)
+        super || number_to_non_number?(old_value, new_value_before_type_cast)
       end
 
       private
 
-      def zero_to_non_number?(old_value, new_value_before_type_cast)
-        old_value == 0 && non_numeric_string?(new_value_before_type_cast)
+      def number_to_non_number?(old_value, new_value_before_type_cast)
+        old_value != nil && non_numeric_string?(new_value_before_type_cast)
       end
 
       def non_numeric_string?(value)
diff --git a/activerecord/test/cases/types_test.rb b/activerecord/test/cases/types_test.rb
index 731f8cfba3..961aae88cb 100644
--- a/activerecord/test/cases/types_test.rb
+++ b/activerecord/test/cases/types_test.rb
@@ -79,11 +79,29 @@ module ActiveRecord
         assert_nil type.type_cast_from_user(1.0/0.0)
       end
 
+      def test_changing_integers
+        type = Type::Integer.new
+
+        assert type.changed?(5, 5, '5wibble')
+        assert_not type.changed?(5, 5, '5')
+        assert_not type.changed?(5, 5, '5.0')
+        assert_not type.changed?(nil, nil, nil)
+      end
+
       def test_type_cast_float
         type = Type::Float.new
         assert_equal 1.0, type.type_cast_from_user("1")
       end
 
+      def test_changing_float
+        type = Type::Float.new
+
+        assert type.changed?(5.0, 5.0, '5wibble')
+        assert_not type.changed?(5.0, 5.0, '5')
+        assert_not type.changed?(5.0, 5.0, '5.0')
+        assert_not type.changed?(nil, nil, nil)
+      end
+
       def test_type_cast_decimal
         type = Type::Decimal.new
         assert_equal BigDecimal.new("0"), type.type_cast_from_user(BigDecimal.new("0"))