From 719d52db42a0eeebf0f1fa2f8ac3173544dd521e Mon Sep 17 00:00:00 2001
From: Agis- <corestudiosinc@gmail.com>
Date: Mon, 13 Oct 2014 17:50:32 +0300
Subject: [PATCH] Autosave callbacks shouldn't be `after_save`

068f092ced8483e557725542dd919ab7c516e567 registered autosave callbacks
as `after_save` callbacks. This caused the regression described in #17209.

Autosave callbacks should be registered as `after_update` and
`after_create` callbacks, just like before.

This is a partial revert of 068f092ced8483e557725542dd919ab7c516e567.

Fixes #17209.
---
 activerecord/CHANGELOG.md                              | 7 +++++++
 activerecord/lib/active_record/autosave_association.rb | 4 +++-
 activerecord/test/cases/autosave_association_test.rb   | 9 +++++++++
 activerecord/test/models/post.rb                       | 9 +++++++++
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 6bdb53ac5a..b12d048169 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,10 @@
+*   Fix regression causing `after_create` callbacks to run before associated
+    records are autosaved.
+
+    Fixes #17209.
+
+    *Agis Anastasopoulos*
+
 *   Honor overridden `rack.test` in Rack environment for the connection
     management middleware.
 
diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index c384e8c413..a0d70435fa 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -184,7 +184,9 @@ def add_autosave_association_callbacks(reflection)
             before_save :before_save_collection_association
 
             define_non_cyclic_method(save_method) { save_collection_association(reflection) }
-            after_save save_method
+            # Doesn't use after_save as that would save associations added in after_create/after_update twice
+            after_create save_method
+            after_update save_method
           elsif reflection.has_one?
             define_method(save_method) { save_has_one_association(reflection) } unless method_defined?(save_method)
             # Configures two callbacks instead of a single after_save so that
diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index b2a7d3956d..734fd5fe18 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -1,5 +1,6 @@
 require 'cases/helper'
 require 'models/bird'
+require 'models/comment'
 require 'models/company'
 require 'models/customer'
 require 'models/developer'
@@ -616,6 +617,14 @@ def test_autosave_new_record_on_has_many_can_be_disabled_per_relationship
     firm.save!
     assert !account.persisted?
   end
+
+  def test_autosave_new_record_with_after_create_callback
+    post = PostWithAfterCreateCallback.new(title: 'Captain Murphy', body: 'is back')
+    post.comments.build(body: 'foo')
+    post.save!
+
+    assert_not_nil post.author_id
+  end
 end
 
 class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb
index 256b720c9a..67027cbc22 100644
--- a/activerecord/test/models/post.rb
+++ b/activerecord/test/models/post.rb
@@ -219,6 +219,15 @@ class PostThatLoadsCommentsInAnAfterSaveHook < ActiveRecord::Base
   end
 end
 
+class PostWithAfterCreateCallback < ActiveRecord::Base
+  self.table_name = 'posts'
+  has_many :comments, foreign_key: :post_id
+
+  after_create do |post|
+    update_attribute(:author_id, comments.first.id)
+  end
+end
+
 class PostWithCommentWithDefaultScopeReferencesAssociation < ActiveRecord::Base
   self.table_name = 'posts'
   has_many :comment_with_default_scope_references_associations, foreign_key: :post_id
-- 
GitLab