diff options
author | Darwin D Wu <wuddarwin@gmail.com> | 2018-08-15 17:18:51 -0700 |
---|---|---|
committer | Darwin D Wu <wuddarwin@gmail.com> | 2018-09-11 00:13:09 -0700 |
commit | 5291044a1d7866d2942276d812a5d3a72a67ef27 (patch) | |
tree | 3bb6f91fdec76505a2ff1104adda74dd6773dd99 /activerecord | |
parent | 5b0b1ee8fda1cd086653992f812f96c62fb3c24b (diff) | |
download | rails-5291044a1d7866d2942276d812a5d3a72a67ef27.tar.gz rails-5291044a1d7866d2942276d812a5d3a72a67ef27.tar.bz2 rails-5291044a1d7866d2942276d812a5d3a72a67ef27.zip |
Fixes #33610
In order to avoid double assignments of nested_attributes for `has_many`
relations during record initialization, nested_attributes in `create_with`
should not be passed into `klass.new` and have them populate during
`initialize_internals_callback` with scope attributes.
However, `create_with` keys should always have precedence over where
clauses, so if there are same keys in both `create_with` and
`where_values_hash`, the value in `create_with` should be the one that's
used.
Diffstat (limited to 'activerecord')
-rw-r--r-- | activerecord/CHANGELOG.md | 4 | ||||
-rw-r--r-- | activerecord/lib/active_record/relation.rb | 25 | ||||
-rw-r--r-- | activerecord/test/cases/nested_attributes_test.rb | 12 | ||||
-rw-r--r-- | activerecord/test/cases/relations_test.rb | 11 | ||||
-rw-r--r-- | activerecord/test/cases/scoping/default_scoping_test.rb | 16 |
5 files changed, 61 insertions, 7 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 968cce39c1..4487d70ce9 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Fix duplicated record creation when using nested attributes with `create_with`. + + *Darwin Wu* + * Configuration item `config.filter_parameters` could also filter out sensitive value of database column when call `#inspect`. ``` diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 29a3ceab7d..c4e48cdb67 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -62,7 +62,7 @@ module ActiveRecord # user = users.new { |user| user.name = 'Oscar' } # user.name # => Oscar def new(attributes = nil, &block) - scoping { klass.new(scope_for_create(attributes), &block) } + scoping { klass.new(values_for_create(attributes), &block) } end alias build new @@ -90,7 +90,7 @@ module ActiveRecord if attributes.is_a?(Array) attributes.collect { |attr| create(attr, &block) } else - scoping { klass.create(scope_for_create(attributes), &block) } + scoping { klass.create(values_for_create(attributes), &block) } end end @@ -104,7 +104,7 @@ module ActiveRecord if attributes.is_a?(Array) attributes.collect { |attr| create!(attr, &block) } else - scoping { klass.create!(scope_for_create(attributes), &block) } + scoping { klass.create!(values_for_create(attributes), &block) } end end @@ -554,10 +554,8 @@ module ActiveRecord where_clause.to_h(relation_table_name) end - def scope_for_create(attributes = nil) - scope = where_values_hash.merge!(create_with_value.stringify_keys) - scope.merge!(attributes) if attributes - scope + def scope_for_create + where_values_hash.merge!(create_with_value.stringify_keys) end # Returns true if relation needs eager loading. @@ -708,5 +706,18 @@ module ActiveRecord # ignore raw_sql_ that is used by Oracle adapter as alias for limit/offset subqueries string.scan(/([a-zA-Z_][.\w]+).?\./).flatten.map(&:downcase).uniq - ["raw_sql_"] end + + def values_for_create(attributes = nil) + result = attributes ? where_values_hash.merge!(attributes) : where_values_hash + + # NOTE: if there are same keys in both create_with and result, create_with should be used. + # This is to make sure nested attributes don't get passed to the klass.new, + # while keeping the precedence of the duplicate keys in create_with. + create_with_value.stringify_keys.each do |k, v| + result[k] = v if result.key?(k) + end + + result + end end end diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index aa519fd332..bb1c1ea17d 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -217,6 +217,18 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase mean_pirate.parrot_attributes = { name: "James" } assert_equal "James", mean_pirate.parrot.name end + + def test_should_not_create_duplicates_with_create_with + Man.accepts_nested_attributes_for(:interests) + + assert_difference("Interest.count", 1) do + Man.create_with( + interests_attributes: [{ topic: "Pirate king" }] + ).find_or_create_by!( + name: "Monkey D. Luffy" + ) + end + end end class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index d03b412efb..c14dc23cf3 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -9,6 +9,7 @@ require "models/comment" require "models/author" require "models/entrant" require "models/developer" +require "models/project" require "models/person" require "models/computer" require "models/reply" @@ -1405,6 +1406,16 @@ class RelationTest < ActiveRecord::TestCase assert_equal "cock", hens.new.name end + def test_create_with_nested_attributes + assert_difference("Project.count", 1) do + developers = Developer.where(name: "Aaron") + developers = developers.create_with( + projects_attributes: [{ name: "p1" }] + ) + developers.create! + end + end + def test_except relation = Post.where(author_id: 1).order("id ASC").limit(1) assert_equal [posts(:welcome)], relation.to_a diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb index e3a34aa50d..6281712df6 100644 --- a/activerecord/test/cases/scoping/default_scoping_test.rb +++ b/activerecord/test/cases/scoping/default_scoping_test.rb @@ -4,6 +4,7 @@ require "cases/helper" require "models/post" require "models/comment" require "models/developer" +require "models/project" require "models/computer" require "models/vehicle" require "models/cat" @@ -366,6 +367,21 @@ class DefaultScopingTest < ActiveRecord::TestCase assert_equal "Jamis", jamis.name end + def test_create_with_takes_precedence_over_where + developer = Developer.where(name: nil).create_with(name: "Aaron").new + assert_equal "Aaron", developer.name + end + + def test_create_with_nested_attributes + assert_difference("Project.count", 1) do + Developer.create_with( + projects_attributes: [{ name: "p1" }] + ).scoping do + Developer.create!(name: "Aaron") + end + end + end + # FIXME: I don't know if this is *desired* behavior, but it is *today's* # behavior. def test_create_with_empty_hash_will_not_reset |