From ec093232a99af740149dd0b82b2e47f8ed45a9ce Mon Sep 17 00:00:00 2001 From: George Montana Harkin Date: Wed, 23 Jun 2010 11:17:01 -0400 Subject: [PATCH] Fixes #2415 by creating a new instance of the Model when saving attributes to that model and the associated attributes already exist. Tests included. [#2415 state:resolved] --- .../lib/active_record/nested_attributes.rb | 19 ++++++++----- activerecord/test/cases/nested_attributes_test.rb | 28 ++++++++++--------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 65434fb..b7a4b7d 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -286,7 +286,9 @@ module ActiveRecord assign_to_or_mark_for_destruction(record, attributes, options[:allow_destroy]) elsif attributes['id'] - raise_nested_attributes_record_not_found(association_name, attributes['id']) + existing_record = self.class.reflect_on_association(association_name).klass.find(attributes['id']) + assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) + self.send(association_name.to_s+'=', existing_record) elsif !reject_new_record?(association_name, attributes) method = "build_#{association_name}" @@ -356,11 +358,16 @@ module ActiveRecord unless reject_new_record?(association_name, attributes) association.build(attributes.except(*UNASSIGNABLE_KEYS)) end + + elsif existing_records.count == 0 #Existing record but not yet associated + existing_record = self.class.reflect_on_association(association_name).klass.find(attributes['id']) + 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]) + 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']) + end end end @@ -380,7 +387,7 @@ module ActiveRecord ConnectionAdapters::Column.value_to_boolean(hash['_destroy']) end - # Determines if a new record should be build by checking for + # Determines if a new record should be built by checking for # has_destroy_flag? or if a :reject_if proc exists for this # association and evaluates to +true+. def reject_new_record?(association_name, attributes) @@ -396,9 +403,5 @@ module ActiveRecord end end - def raise_nested_attributes_record_not_found(association_name, record_id) - reflection = self.class.reflect_on_association(association_name) - raise RecordNotFound, "Couldn't find #{reflection.klass.name} with ID=#{record_id} for #{self.class.name} with ID=#{id}" - end end end diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index dc095c5..34436d0 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -175,12 +175,6 @@ class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase assert_equal 'Davy Jones Gold Dagger', @pirate.ship.name end - def test_should_raise_RecordNotFound_if_an_id_is_given_but_doesnt_return_a_record - assert_raise_with_message ActiveRecord::RecordNotFound, "Couldn't find Ship with ID=1234567890 for Pirate with ID=#{@pirate.id}" do - @pirate.ship_attributes = { :id => 1234567890 } - end - end - def test_should_take_a_hash_with_string_keys_and_update_the_associated_model @pirate.reload.ship_attributes = { 'id' => @ship.id, 'name' => 'Davy Jones Gold Dagger' } @@ -330,10 +324,13 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase assert_equal 'Arr', @ship.pirate.catchphrase end - def test_should_raise_RecordNotFound_if_an_id_is_given_but_doesnt_return_a_record - assert_raise_with_message ActiveRecord::RecordNotFound, "Couldn't find Pirate with ID=1234567890 for Ship with ID=#{@ship.id}" do - @ship.pirate_attributes = { :id => 1234567890 } - end + def test_should_associate_with_record_if_parent_record_is_not_saved + @ship.destroy + @pirate = Pirate.create(:catchphrase => 'Arr') + @ship = Ship.new(:name => 'Nights Dirty Lightning', :pirate_attributes => { :id => @pirate.id, :catchphrase => @pirate.catchphrase}) + + assert_equal @ship.name, 'Nights Dirty Lightning' + assert_equal @pirate, @ship.pirate end def test_should_take_a_hash_with_string_keys_and_update_the_associated_model @@ -437,6 +434,11 @@ module NestedAttributesOnACollectionAssociationTests assert_equal ['Grace OMalley', 'Privateers Greed'], [@child_1.reload.name, @child_2.reload.name] end + def test_should_assign_existing_children_if_parent_is_new + @pirate = Pirate.new({:catchphrase => "Don' botharr talkin' like one, savvy?"}.merge(@alternate_params)) + assert_equal ['Grace OMalley', 'Privateers Greed'], [@pirate.send(@association_name)[0].name, @pirate.send(@association_name)[1].name] + end + def test_should_take_an_array_and_assign_the_attributes_to_the_associated_models @pirate.send(association_setter, @alternate_params[association_getter].values) @pirate.save @@ -500,8 +502,8 @@ module NestedAttributesOnACollectionAssociationTests assert_equal ['Grace OMalley', 'Privateers Greed'], [@child_1.name, @child_2.name] end - def test_should_raise_RecordNotFound_if_an_id_is_given_but_doesnt_return_a_record - assert_raise_with_message ActiveRecord::RecordNotFound, "Couldn't find #{@child_1.class.name} with ID=1234567890 for Pirate with ID=#{@pirate.id}" do + def test_should_not_raise_RecordNotFound_if_an_id_is_given_but_doesnt_return_a_record + assert_nothing_raised ActiveRecord::RecordNotFound do @pirate.attributes = { association_getter => [{ :id => 1234567890 }] } end end @@ -831,4 +833,4 @@ class TestHasManyAutosaveAssoictaionWhichItselfHasAutosaveAssociations < ActiveR ShipPart.create!(:ship => @ship, :name => "Stern") assert_no_queries { @ship.valid? } end -end \ No newline at end of file +end -- 1.6.4.4