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]

Signed-off-by: José Valim <jose.valim@gmail.com>

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 12a75f5d16..c0302136ea 100644
--- a/activerecord/lib/active_record/nested_attributes.rb
+++ b/activerecord/lib/active_record/nested_attributes.rb
@@ -296,7 +296,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}"
@@ -366,11 +368,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
@@ -390,7 +397,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 <tt>:reject_if</tt> proc exists for this
     # association and evaluates to +true+.
     def reject_new_record?(association_name, attributes)
@@ -406,9 +413,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 65d6080ea5..3c797076e0 100644
--- a/activerecord/test/cases/nested_attributes_test.rb
+++ b/activerecord/test/cases/nested_attributes_test.rb
@@ -176,12 +176,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' }
 
@@ -331,11 +325,14 @@ 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
-  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
     @ship.reload.pirate_attributes = { 'id' => @pirate.id, 'catchphrase' => 'Arr' }
@@ -444,6 +441,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_also_work_with_a_HashWithIndifferentAccess
     @pirate.send(association_setter, HashWithIndifferentAccess.new('foo' => HashWithIndifferentAccess.new(:id => @child_1.id, :name => 'Grace OMalley')))
     @pirate.save
@@ -501,8 +503,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