Do not overwrite manually built records during one-to-one nested attribute assignment

For one-to-one nested associations, if you build the new (in-memory)
child object yourself before assignment, then the NestedAttributes
module will not overwrite it, e.g.:

    class Member < ActiveRecord::Base
      has_one :avatar
      accepts_nested_attributes_for :avatar

      def avatar
        super || build_avatar(width: 200)
      end
    end

    member = Member.new
    member.avatar_attributes = {icon: 'sad'}
    member.avatar.width # => 200

4 files changed, 75 insertions, 11 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index c5b94763ff..917f606d0e 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,24 @@
+*   Do not overwrite manually built records during one-to-one nested attribute assignment
+
+    For one-to-one nested associations, if you build the new (in-memory)
+    child object yourself before assignment, then the NestedAttributes
+    module will not overwrite it, e.g.:
+
+        class Member < ActiveRecord::Base
+          has_one :avatar
+          accepts_nested_attributes_for :avatar
+
+          def avatar
+            super || build_avatar(width: 200)
+          end
+        end
+
+        member = Member.new
+        member.avatar_attributes = {icon: 'sad'}
+        member.avatar.width # => 200
+
+    *Olek Janiszewski*
+
 *   fixes bug introduced by #3329.  Now, when autosaving associations,
     deletions happen before inserts and saves.  This prevents a 'duplicate
     unique value' database error that would occur if a record being created had
diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb
index db0553ea76..710babe024 100644
--- a/activerecord/lib/active_record/associations/association.rb
+++ b/activerecord/lib/active_record/associations/association.rb
@@ -164,6 +164,13 @@ module ActiveRecord
         @reflection = @owner.class.reflect_on_association(reflection_name)
       end
 
+      def initialize_attributes(record) #:nodoc:
+        skip_assign = [reflection.foreign_key, reflection.type].compact
+        attributes = create_scope.except(*(record.changed - skip_assign))
+        record.assign_attributes(attributes)
+        set_inverse_instance(record)
+      end
+
       private
 
         def find_target?
@@ -233,10 +240,7 @@ module ActiveRecord
 
         def build_record(attributes)
           reflection.build_association(attributes) do |record|
-            skip_assign = [reflection.foreign_key, reflection.type].compact
-            attributes = create_scope.except(*(record.changed - skip_assign))
-            record.assign_attributes(attributes)
-            set_inverse_instance(record)
+            initialize_attributes(record)
           end
         end
     end
diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb
index d607f49e2b..021832de46 100644
--- a/activerecord/lib/active_record/nested_attributes.rb
+++ b/activerecord/lib/active_record/nested_attributes.rb
@@ -229,6 +229,23 @@ module ActiveRecord
     #     belongs_to :member, inverse_of: :posts
     #     validates_presence_of :member
     #   end
+    #
+    # For one-to-one nested associations, if you build the new (in-memory)
+    # child object yourself before assignment, then this module will not
+    # overwrite it, e.g.:
+    #
+    #   class Member < ActiveRecord::Base
+    #     has_one :avatar
+    #     accepts_nested_attributes_for :avatar
+    #
+    #     def avatar
+    #       super || build_avatar(width: 200)
+    #     end
+    #   end
+    #
+    #   member = Member.new
+    #   member.avatar_attributes = {icon: 'sad'}
+    #   member.avatar.width # => 200
     module ClassMethods
       REJECT_ALL_BLANK_PROC = proc { |attributes| attributes.all? { |key, value| key == '_destroy' || value.blank? } }
 
@@ -356,20 +373,28 @@ module ActiveRecord
     def assign_nested_attributes_for_one_to_one_association(association_name, attributes)
       options = self.nested_attributes_options[association_name]
       attributes = attributes.with_indifferent_access
+      existing_record = send(association_name)
 
-      if (options[:update_only] || !attributes['id'].blank?) && (record = send(association_name)) &&
-          (options[:update_only] || record.id.to_s == attributes['id'].to_s)
-        assign_to_or_mark_for_destruction(record, attributes, options[:allow_destroy]) unless call_reject_if(association_name, attributes)
+      if (options[:update_only] || !attributes['id'].blank?) && existing_record &&
+          (options[:update_only] || existing_record.id.to_s == attributes['id'].to_s)
+        assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) unless call_reject_if(association_name, attributes)
 
       elsif attributes['id'].present?
         raise_nested_attributes_record_not_found!(association_name, attributes['id'])
 
       elsif !reject_new_record?(association_name, attributes)
-        method = "build_#{association_name}"
-        if respond_to?(method)
-          send(method, attributes.except(*UNASSIGNABLE_KEYS))
+        assignable_attributes = attributes.except(*UNASSIGNABLE_KEYS)
+
+        if existing_record && existing_record.new_record?
+          existing_record.assign_attributes(assignable_attributes)
+          association(association_name).initialize_attributes(existing_record)
         else
-          raise ArgumentError, "Cannot build association `#{association_name}'. Are you trying to build a polymorphic one-to-one association?"
+          method = "build_#{association_name}"
+          if respond_to?(method)
+            send(method, assignable_attributes)
+          else
+            raise ArgumentError, "Cannot build association `#{association_name}'. Are you trying to build a polymorphic one-to-one association?"
+          end
         end
       end
     end
diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb
index b6e140b912..165b7454b7 100644
--- a/activerecord/test/cases/nested_attributes_test.rb
+++ b/activerecord/test/cases/nested_attributes_test.rb
@@ -131,6 +131,20 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase
     assert_equal 's1', ship.reload.name
   end
 
+  def test_reuse_already_built_new_record
+    pirate = Pirate.new
+    ship_built_first = pirate.build_ship
+    pirate.ship_attributes = { name: 'Ship 1' }
+    assert_equal ship_built_first.object_id, pirate.ship.object_id
+  end
+
+  def test_do_not_allow_assigning_foreign_key_when_reusing_existing_new_record
+    pirate = Pirate.create!(catchphrase: "Don' botharrr talkin' like one, savvy?")
+    pirate.build_ship
+    pirate.ship_attributes = { name: 'Ship 1', pirate_id: pirate.id + 1 }
+    assert_equal pirate.id, pirate.ship.pirate_id
+  end
+
   def test_reject_if_with_a_proc_which_returns_true_always_for_has_many
     Man.accepts_nested_attributes_for :interests, :reject_if => proc {|attributes| true }
     man = Man.create(name: "John")