Skip to content

Commit ef2e164

Browse files
authored
Merge pull request rails#52909 from Shopify/active_record_nested_attribute_regression
Use less queries when updating nested attributes
2 parents e4aeaa0 + c8a4eef commit ef2e164

File tree

2 files changed

+7
-7
lines changed

2 files changed

+7
-7
lines changed

activerecord/lib/active_record/nested_attributes.rb

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -524,12 +524,12 @@ def assign_nested_attributes_for_collection_association(association_name, attrib
524524
unless reject_new_record?(association_name, attributes)
525525
association.reader.build(attributes.except(*UNASSIGNABLE_KEYS))
526526
end
527-
elsif existing_record = find_record_by_id(existing_records, attributes["id"])
527+
elsif existing_record = find_record_by_id(association.klass, existing_records, attributes["id"])
528528
unless call_reject_if(association_name, attributes)
529529
# Make sure we are operating on the actual object which is in the association's
530530
# proxy_target array (either by finding it, or adding it if not found)
531531
# Take into account that the proxy_target may have changed due to callbacks
532-
target_record = find_record_by_id(association.target, attributes["id"])
532+
target_record = find_record_by_id(association.klass, association.target, attributes["id"])
533533
if target_record
534534
existing_record = target_record
535535
else
@@ -621,10 +621,8 @@ def raise_nested_attributes_record_not_found!(association_name, record_id)
621621
model, "id", record_id)
622622
end
623623

624-
def find_record_by_id(records, id)
625-
return if records.empty?
626-
627-
if records.first.class.composite_primary_key?
624+
def find_record_by_id(klass, records, id)
625+
if klass.composite_primary_key?
628626
id = Array(id).map(&:to_s)
629627
records.find { |record| Array(record.id).map(&:to_s) == id }
630628
else

activerecord/test/cases/nested_attributes_test.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,9 @@ def test_updating_models_with_cpk_provided_as_strings
238238
book = Cpk::Book.create!(id: [1, 2], shop_id: 3)
239239
book.chapters.create!(id: [1, 3], title: "Title")
240240

241-
book.update!(chapters_attributes: { id: ["1", "3"], title: "New title" })
241+
assert_queries_count(4) do
242+
book.update!(chapters_attributes: { id: ["1", "3"], title: "New title" })
243+
end
242244
assert_equal 1, book.reload.chapters.count
243245
assert_equal "New title", book.chapters.first.title
244246
end

0 commit comments

Comments
 (0)