Merge pull request rails#52333 from joshuay03/fix-52332

[Fix rails#52332] Handle record changed case in `#save_has_one_association`'s inverse `belongs_to` association updated check

Author: byroot

activerecord/lib/active_record/autosave_association.rb

@@ -436,9 +436,6 @@ def save_has_one_association(reflection)
         record      = association && association.load_target
         return unless record && !record.destroyed?
 
-        inverse_belongs_to_association = inverse_belongs_to_association_for(reflection, record)
-        return if inverse_belongs_to_association && inverse_belongs_to_association.updated?
-
         autosave = reflection.options[:autosave]
 
         if autosave && record.marked_for_destruction?
@@ -447,22 +444,27 @@ def save_has_one_association(reflection)
           primary_key = Array(compute_primary_key(reflection, self)).map(&:to_s)
           primary_key_value = primary_key.map { |key| _read_attribute(key) }
 
-          if (autosave && record.changed_for_autosave?) || _record_changed?(reflection, record, primary_key_value)
-            unless reflection.through_reflection
-              foreign_key = Array(reflection.foreign_key)
-              primary_key_foreign_key_pairs = primary_key.zip(foreign_key)
+          return unless (autosave && record.changed_for_autosave?) || (record_changed = _record_changed?(reflection, record, primary_key_value))
 
-              primary_key_foreign_key_pairs.each do |primary_key, foreign_key|
-                association_id = _read_attribute(primary_key)
-                record[foreign_key] = association_id unless record[foreign_key] == association_id
-              end
-              association.set_inverse_instance(record)
-            end
+          unless record_changed
+            inverse_belongs_to_association = inverse_belongs_to_association_for(reflection, record)
+            return if inverse_belongs_to_association && inverse_belongs_to_association.updated?
+          end
+
+          unless reflection.through_reflection
+            foreign_key = Array(reflection.foreign_key)
+            primary_key_foreign_key_pairs = primary_key.zip(foreign_key)
 
-            saved = record.save(validate: !autosave)
-            raise ActiveRecord::Rollback if !saved && autosave
-            saved
+            primary_key_foreign_key_pairs.each do |primary_key, foreign_key|
+              association_id = _read_attribute(primary_key)
+              record[foreign_key] = association_id unless record[foreign_key] == association_id
+            end
+            association.set_inverse_instance(record)
           end
+
+          saved = record.save(validate: !autosave)
+          raise ActiveRecord::Rollback if !saved && autosave
+          saved
         end
       end
 

activerecord/test/cases/autosave_association_test.rb

@@ -314,6 +314,17 @@ def test_callbacks_on_child_when_parent_autosaves_child
     assert_equal 1, eye.iris.after_save_callbacks_counter
   end
 
+  def test_callbacks_on_child_when_parent_autosaves_child_twice
+    eye = Eye.create!(iris: Iris.new)
+    eye.update!(iris: Iris.new)
+    assert_equal 1, eye.iris.before_validation_callbacks_counter
+    assert_equal 1, eye.iris.before_create_callbacks_counter
+    assert_equal 1, eye.iris.before_save_callbacks_counter
+    assert_equal 1, eye.iris.after_validation_callbacks_counter
+    assert_equal 1, eye.iris.after_create_callbacks_counter
+    assert_equal 1, eye.iris.after_save_callbacks_counter
+  end
+
   def test_callbacks_on_child_when_parent_autosaves_polymorphic_child_with_inverse_of
     drink_designer = DrinkDesigner.create!(chef: ChefWithPolymorphicInverseOf.new)
     assert_equal 1, drink_designer.chef.before_validation_callbacks_counter
@@ -334,6 +345,17 @@ def test_callbacks_on_child_when_child_autosaves_parent
     assert_equal 1, iris.after_save_callbacks_counter
   end
 
+  def test_callbacks_on_child_when_child_autosaves_parent_twice
+    iris = Iris.create!(eye: Eye.new)
+    iris.update!(eye: Eye.new)
+    assert_equal 2, iris.before_validation_callbacks_counter
+    assert_equal 1, iris.before_create_callbacks_counter
+    assert_equal 2, iris.before_save_callbacks_counter
+    assert_equal 2, iris.after_validation_callbacks_counter
+    assert_equal 1, iris.after_create_callbacks_counter
+    assert_equal 2, iris.after_save_callbacks_counter
+  end
+
   def test_callbacks_on_child_when_polymorphic_child_with_inverse_of_autosaves_parent
     chef = ChefWithPolymorphicInverseOf.create!(employable: DrinkDesigner.new)
     assert_equal 1, chef.before_validation_callbacks_counter
@@ -1812,6 +1834,12 @@ def test_should_save_with_non_nullable_foreign_keys
     child.save!
     assert_equal child.reload.post, parent.reload
   end
+
+  def test_should_save_if_previously_saved
+    ship = Ship.create(name: "Nights Dirty Lightning", pirate: Pirate.new(catchphrase: "Arrrr"))
+    ship.create_pirate(catchphrase: "Savvy?")
+    assert_equal "Savvy?", ship.reload.pirate.catchphrase
+  end
 end
 
 module AutosaveAssociationOnACollectionAssociationTests