aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDaniel Fox <romaimperator@gmail.com>2014-12-23 00:19:14 -0600
committerDaniel Fox <romaimperator@gmail.com>2014-12-23 00:19:14 -0600
commit2859341c383daac74608f978d8e8fad2229042a3 (patch)
treee738d92c864195844e57878d52c419145ed078a4
parent38e9f5b1d80018cb32b918f7ddd0ec677e79dfd2 (diff)
downloadrails-2859341c383daac74608f978d8e8fad2229042a3.tar.gz
rails-2859341c383daac74608f978d8e8fad2229042a3.tar.bz2
rails-2859341c383daac74608f978d8e8fad2229042a3.zip
Fixing numeric attrs when set to same negative value
This bug occurs when an attribute of an ActiveRecord model is an ActiveRecord::Type::Integer type or a ActiveRecord::Type::Decimal type (or any other type that includes the ActiveRecord::Type::Numeric module. When the value of the attribute is negative and is set to the same negative value, it is marked as changed. Take the following example of a Person model with the integer attribute age: class Person < ActiveRecord::Base # age :integer(4) end The following will produce the error: person = Person.new(age: -1) person.age = -1 person.changes => { "age" => [-1, -1] } person.age_changed? => true The problematic line is here: module ActiveRecord module Type module Numeric ... def non_numeric_string?(value) # 'wibble'.to_i will give zero, we want to make sure # that we aren't marking int zero to string zero as # changed. value.to_s !~ /\A\d+\.?\d*\z/ end end end end The regex match doesn't accept numbers with a leading '-'.
-rw-r--r--activerecord/CHANGELOG.md7
-rw-r--r--activerecord/lib/active_record/type/numeric.rb2
-rw-r--r--activerecord/test/cases/type/decimal_test.rb8
-rw-r--r--activerecord/test/cases/type/integer_test.rb2
4 files changed, 18 insertions, 1 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 6494c7374e..e5a4356523 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,10 @@
+* Fixes bug with 'ActiveRecord::Type::Numeric' that causes negative values to
+ be marked as having changed when set to the same negative value.
+
+ Closes GH#18161
+
+ *Daniel Fox*
+
* Introduce `force: :cascade` option for `create_table`. Using this option
will recreate tables even if they have dependent objects (like foreign keys).
`db/schema.rb` now uses `force: :cascade`. This makes it possible to
diff --git a/activerecord/lib/active_record/type/numeric.rb b/activerecord/lib/active_record/type/numeric.rb
index fa43266504..674f996f38 100644
--- a/activerecord/lib/active_record/type/numeric.rb
+++ b/activerecord/lib/active_record/type/numeric.rb
@@ -29,7 +29,7 @@ module ActiveRecord
# 'wibble'.to_i will give zero, we want to make sure
# that we aren't marking int zero to string zero as
# changed.
- value.to_s !~ /\A\d+\.?\d*\z/
+ value.to_s !~ /\A-?\d+\.?\d*\z/
end
end
end
diff --git a/activerecord/test/cases/type/decimal_test.rb b/activerecord/test/cases/type/decimal_test.rb
index c028aa52af..34ed1d7b19 100644
--- a/activerecord/test/cases/type/decimal_test.rb
+++ b/activerecord/test/cases/type/decimal_test.rb
@@ -38,6 +38,14 @@ module ActiveRecord
type = Decimal.new
assert_equal BigDecimal("1"), type.type_cast_from_user(value)
end
+
+ def test_changed?
+ type = Decimal.new
+
+ assert type.changed?(5.0, 5.0, '5.0wibble')
+ assert_not type.changed?(5.0, 5.0, '5.0')
+ assert_not type.changed?(-5.0, -5.0, '-5.0')
+ end
end
end
end
diff --git a/activerecord/test/cases/type/integer_test.rb b/activerecord/test/cases/type/integer_test.rb
index 5942f77e18..af4d0b4642 100644
--- a/activerecord/test/cases/type/integer_test.rb
+++ b/activerecord/test/cases/type/integer_test.rb
@@ -47,6 +47,8 @@ module ActiveRecord
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?(-5, -5, '-5')
+ assert_not type.changed?(-5, -5, '-5.0')
assert_not type.changed?(nil, nil, nil)
end