Merge pull request rails#48416 from Shopify/fix-has-one-deletion

Add tests around has_one changing the target and validators

Author: byroot

activerecord/lib/active_record/associations/has_one_association.rb

@@ -92,6 +92,10 @@ def set_new_record(record)
           replace(record, false)
         end
 
+        def replace_keys(record, force: false)
+          # Has one association doesn't have foreign keys to replace.
+        end
+
         def remove_target!(method)
           case method
           when :delete

activerecord/lib/active_record/associations/singular_association.rb

@@ -57,7 +57,7 @@ def _create_record(attributes, raise_error = false, &block)
           reflection.klass.transaction do
             record = build(attributes, &block)
             saved = record.save
-            set_new_record(record)
+            replace_keys(record, force: true)
             raise RecordInvalid.new(record) if !saved && raise_error
             record
           end

activerecord/test/cases/associations/has_one_associations_test.rb

@@ -20,6 +20,8 @@
 require "models/membership"
 require "models/parrot"
 require "models/cpk"
+require "models/user"
+require "models/content"
 
 class HasOneAssociationsTest < ActiveRecord::TestCase
   self.use_transactional_tests = false unless supports_savepoints?
@@ -905,6 +907,46 @@ def test_has_one_with_touch_option_on_nonpersisted_built_associations_doesnt_upd
     end
   end
 
+  class InvalidPost < Post
+    validate :always_fail
+
+    def always_fail
+      errors.add(:base, "Oops")
+    end
+  end
+
+  test "preserve old record if new one is invalid" do
+    author = Author.create!(id: 33, name: "Hank Moody")
+    author.create_post!(id: 1234, title: "Some title", body: "Some content")
+
+    assert_no_difference -> { Post.count } do
+      assert_raises ActiveRecord::RecordNotSaved do
+        author.post = InvalidPost.new(title: "Some title", body: "Some content")
+      end
+      author.save!
+    end
+  end
+
+  class SpecialContentPosition < ActiveRecord::Base
+    self.table_name = "content_positions"
+    belongs_to :content, class_name: name + "::SpecialContentPosition"
+    validates :content_id, presence: true, uniqueness: true
+  end
+
+  class SpecialContent < ActiveRecord::Base
+    self.table_name = "content"
+    has_one :content_position, dependent: :destroy, foreign_key: :content_id, class_name: SpecialContentPosition.name
+  end
+
+  test "uniqueness validator doesn't prevent from replacing the old record if dependent: :destroy is set" do
+    content = SpecialContent.create!
+    content.create_content_position!
+
+    assert_no_difference -> { ContentPosition.count } do
+      content.create_content_position!
+    end
+  end
+
   test "composite primary key malformed association" do
     error = assert_raises(ActiveRecord::CompositePrimaryKeyMismatchError) do
       order = Cpk::BrokenOrder.new(id: [1, 2], book: Cpk::Book.new(title: "Some book"))