aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorSean Griffin <sean@thoughtbot.com>2015-05-28 16:33:38 -0600
committerSean Griffin <sean@thoughtbot.com>2015-05-28 16:40:26 -0600
commit2f9d88954caf4fd75098e813a3ab8bf50ef6ace4 (patch)
tree6a105c43031fc521144021284553905344e4bc79 /activerecord
parenta6e3cdae0ce1d05a6b9f7dff4499f6be3fbf63c2 (diff)
downloadrails-2f9d88954caf4fd75098e813a3ab8bf50ef6ace4.tar.gz
rails-2f9d88954caf4fd75098e813a3ab8bf50ef6ace4.tar.bz2
rails-2f9d88954caf4fd75098e813a3ab8bf50ef6ace4.zip
Persist user provided default values, even if unchanged
This is a usability change to fix a quirk from our definition of partial writes. By default, we only persist changed attributes. When creating a new record, this is assumed that the default values came from the database. However, if the user provided a default, it will not be persisted, since we didn't see it as "changed". Since this is a very specific case, I wanted to isolate it with the other quirks that come from user provided default values. The number of edge cases which are presenting themselves are starting to make me wonder if we should just remove the ability to assign a default, in favor of overriding `initialize`. For the time being, this is required for the attributes API to not have confusing behavior. We had to delete one test, since this actually changes the meaning of `.changed?` on Active Record models. It now specifically means `changed_from_database?`. While I think this will make the attributes API more ergonomic to use, it is a subtle change in definition (though not a backwards incompatible one). We should probably figure out the right place to document this. (Feel free to open a PR doing that if you're reading this). /cc @rafaelfranca @kirs @senny This is an alternate implementation of #19921. Close #19921. [Sean Griffin & Kir Shatrov]
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/lib/active_record/attribute/user_provided_default.rb15
-rw-r--r--activerecord/lib/active_record/attributes.rb1
-rw-r--r--activerecord/test/cases/attributes_test.rb6
-rw-r--r--activerecord/test/cases/dirty_test.rb26
4 files changed, 21 insertions, 27 deletions
diff --git a/activerecord/lib/active_record/attribute/user_provided_default.rb b/activerecord/lib/active_record/attribute/user_provided_default.rb
index 29d37ac689..e0bee8c17e 100644
--- a/activerecord/lib/active_record/attribute/user_provided_default.rb
+++ b/activerecord/lib/active_record/attribute/user_provided_default.rb
@@ -3,8 +3,9 @@ require 'active_record/attribute'
module ActiveRecord
class Attribute # :nodoc:
class UserProvidedDefault < FromUser
- def initialize(name, value, type)
+ def initialize(name, value, type, database_default)
super(name, value, type)
+ @database_default = database_default
end
def type_cast(value)
@@ -14,6 +15,18 @@ module ActiveRecord
super
end
end
+
+ def changed_in_place_from?(old_value)
+ super || changed_from?(database_default.value)
+ end
+
+ def with_type(type)
+ self.class.new(name, value_before_type_cast, type, database_default)
+ end
+
+ protected
+
+ attr_reader :database_default
end
end
end
diff --git a/activerecord/lib/active_record/attributes.rb b/activerecord/lib/active_record/attributes.rb
index e574aebd6c..c89099589e 100644
--- a/activerecord/lib/active_record/attributes.rb
+++ b/activerecord/lib/active_record/attributes.rb
@@ -242,6 +242,7 @@ module ActiveRecord
name,
value,
type,
+ _default_attributes[name],
)
else
default_attribute = Attribute.from_database(name, value, type)
diff --git a/activerecord/test/cases/attributes_test.rb b/activerecord/test/cases/attributes_test.rb
index 4619293ac6..eeda9335ad 100644
--- a/activerecord/test/cases/attributes_test.rb
+++ b/activerecord/test/cases/attributes_test.rb
@@ -135,6 +135,12 @@ module ActiveRecord
assert_equal 2, klass.new.counter
end
+ test "user provided defaults are persisted even if unchanged" do
+ model = OverloadedType.create!
+
+ assert_equal "the overloaded default", model.reload.string_with_default
+ end
+
if current_adapter?(:PostgreSQLAdapter)
test "arrays types can be specified" do
klass = Class.new(OverloadedType) do
diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb
index 3a7cc572e6..216f228142 100644
--- a/activerecord/test/cases/dirty_test.rb
+++ b/activerecord/test/cases/dirty_test.rb
@@ -623,32 +623,6 @@ class DirtyTest < ActiveRecord::TestCase
end
end
- test "defaults with type that implements `serialize`" do
- type = Class.new(ActiveRecord::Type::Value) do
- def cast(value)
- value.to_i
- end
-
- def serialize(value)
- value.to_s
- end
- end
-
- model_class = Class.new(ActiveRecord::Base) do
- self.table_name = 'numeric_data'
- attribute :foo, type.new, default: 1
- end
-
- model = model_class.new
- assert_not model.foo_changed?
-
- model = model_class.new(foo: 1)
- assert_not model.foo_changed?
-
- model = model_class.new(foo: '1')
- assert_not model.foo_changed?
- end
-
test "in place mutation detection" do
pirate = Pirate.create!(catchphrase: "arrrr")
pirate.catchphrase << " matey!"