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.
This commit is contained in:
parent
5b0b1ee8fd
commit
5291044a1d
@ -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`.
|
||||
|
||||
```
|
||||
|
@ -62,7 +62,7 @@ def bind_attribute(name, value) # :nodoc:
|
||||
# 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 @@ def create(attributes = nil, &block)
|
||||
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 @@ def create!(attributes = nil, &block)
|
||||
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 @@ def where_values_hash(relation_table_name = klass.table_name)
|
||||
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 @@ def tables_in_string(string)
|
||||
# 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
|
||||
|
@ -217,6 +217,18 @@ def test_accepts_nested_attributes_for_can_be_overridden_in_subclasses
|
||||
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
|
||||
|
@ -9,6 +9,7 @@
|
||||
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 @@ def test_explicit_create_with
|
||||
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
|
||||
|
@ -4,6 +4,7 @@
|
||||
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 @@ def test_create_with_reset
|
||||
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
|
||||
|
Loading…
Reference in New Issue
Block a user