aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorDarwin D Wu <wuddarwin@gmail.com>2018-08-15 17:18:51 -0700
committerDarwin D Wu <wuddarwin@gmail.com>2018-09-11 00:13:09 -0700
commit5291044a1d7866d2942276d812a5d3a72a67ef27 (patch)
tree3bb6f91fdec76505a2ff1104adda74dd6773dd99 /activerecord
parent5b0b1ee8fda1cd086653992f812f96c62fb3c24b (diff)
downloadrails-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.md4
-rw-r--r--activerecord/lib/active_record/relation.rb25
-rw-r--r--activerecord/test/cases/nested_attributes_test.rb12
-rw-r--r--activerecord/test/cases/relations_test.rb11
-rw-r--r--activerecord/test/cases/scoping/default_scoping_test.rb16
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