Permalink
Browse files

Ensure not to load the entire association when bulk updating existing…

… records using nested attributes
  • Loading branch information...
1 parent c69dc1a commit 5efb1503dd88b59fe491dade92790c3f06293445 @lifo lifo committed Apr 14, 2010
@@ -407,6 +407,16 @@ def find_target
records
end
+ def add_record_to_target_with_callbacks(record)
+ callback(:before_add, record)
+ yield(record) if block_given?
+ @target ||= [] unless loaded?
+ @target << record unless @reflection.options[:uniq] && @target.include?(record)
+ callback(:after_add, record)
+ set_inverse_instance(record, @owner)
+ record
+ end
+
private
def create_record(attrs)
attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash)
@@ -431,16 +441,6 @@ def build_record(attrs)
end
end
- def add_record_to_target_with_callbacks(record)
- callback(:before_add, record)
- yield(record) if block_given?
- @target ||= [] unless loaded?
- @target << record unless @reflection.options[:uniq] && @target.include?(record)
- callback(:after_add, record)
- set_inverse_instance(record, @owner)
- record
- end
-
def remove_records(*records)
records = flatten_deeper(records)
records.each { |record| raise_on_type_mismatch(record) }
@@ -340,14 +340,24 @@ def assign_nested_attributes_for_collection_association(association_name, attrib
attributes_collection = attributes_collection.sort_by { |index, _| index.to_i }.map { |_, attributes| attributes }
end
+ association = send(association_name)
+
+ existing_records = if association.loaded?
+ association.to_a
+ else
+ attribute_ids = attributes_collection.map {|a| a['id'] || a[:id] }.compact
+ attribute_ids.present? ? association.all(:conditions => {:id => attribute_ids}) : []
+ end
+
attributes_collection.each do |attributes|
attributes = attributes.with_indifferent_access
if attributes['id'].blank?
unless reject_new_record?(association_name, attributes)
- send(association_name).build(attributes.except(*UNASSIGNABLE_KEYS))
+ association.build(attributes.except(*UNASSIGNABLE_KEYS))
end
- elsif existing_record = send(association_name).detect { |record| record.id.to_s == attributes['id'].to_s }
+ elsif existing_record = existing_records.detect { |record| record.id.to_s == attributes['id'].to_s }
+ association.send(:add_record_to_target_with_callbacks, existing_record) unless association.loaded?
assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy])
else
raise_nested_attributes_record_not_found(association_name, attributes['id'])
@@ -452,6 +452,16 @@ def test_should_take_a_hash_and_assign_the_attributes_to_the_associated_models
assert_equal 'Privateers Greed', @pirate.send(@association_name).last.name
end
+ def test_should_not_load_association_when_updating_existing_records
+ @pirate.reload
+ @pirate.send(association_setter, [{ :id => @child_1.id, :name => 'Grace OMalley' }])
+ assert ! @pirate.send(@association_name).loaded?
+
+ @pirate.save
+ assert ! @pirate.send(@association_name).loaded?
+ assert_equal 'Grace OMalley', @child_1.reload.name
+ end
+
def test_should_take_a_hash_with_composite_id_keys_and_assign_the_attributes_to_the_associated_models
@child_1.stubs(:id).returns('ABC1X')
@child_2.stubs(:id).returns('ABC2X')
@@ -506,7 +516,7 @@ def test_should_ignore_new_associated_records_with_truthy_destroy_attribute
def test_should_ignore_new_associated_records_if_a_reject_if_proc_returns_false
@alternate_params[association_getter]['baz'] = {}
- assert_no_difference("@pirate.send(@association_name).length") do
+ assert_no_difference("@pirate.send(@association_name).count") do
@pirate.attributes = @alternate_params
end
end

1 comment on commit 5efb150

perezd commented on 5efb150 Jul 11, 2010

How does this changeset effect polymorphic associations? In Rails 2.3.8, my polymorphic nested attributes don't work anymore, it tells me I don't have an initialized constant that matches MyClass::PolymorphicAssociationClassName as a NameError. PolymorphicAssociationClassName would be MyClass::Owner which does not exist as a true class, but is the name of my polymorph association.

Is this a bug? I cannot upgrade to Rails 2.3.8, things worked fine in 2.3.5!

Please sign in to comment.