Do not mark object as persisted after an association is saved

Callback order in Active Record objects are important. Users should not
define callbacks before the association definition or surprising
behaviours like the described at #3798 will happen. This callback order
dependency is documented at https://github.com/rails/rails/blob/31bfcdc77ca0d8cec9b5fe513bdc6f05814dd4f1/activerecord/lib/active_record/associations.rb#L1222-1227.

This reverts #15728.

Fixes #16620.

3 files changed, 14 insertions, 21 deletions

diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index dd92e29199..a8e4d25df2 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -338,7 +338,6 @@ module ActiveRecord
           autosave = reflection.options[:autosave]
 
           if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave)
-
             if autosave
               records_to_destroy = records.select(&:marked_for_destruction?)
               records_to_destroy.each { |record| association.destroy(record) }
@@ -362,7 +361,6 @@ module ActiveRecord
 
               raise ActiveRecord::Rollback unless saved
             end
-            @new_record_before_save = false
           end
 
           # reconstruct the scope now that we know the owner's id
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 1d28c8dac9..e34b993029 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -28,6 +28,7 @@ require 'models/college'
 require 'models/student'
 require 'models/pirate'
 require 'models/ship'
+require 'models/tyre'
 
 class HasManyAssociationsTestForReorderWithJoinDependency < ActiveRecord::TestCase
   fixtures :authors, :posts, :comments
@@ -1941,4 +1942,17 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
     assert_equal [], authors(:david).posts_with_signature.map(&:title)
   end
+
+  test 'associations autosaves when object is already persited' do
+    bulb = Bulb.create!
+    tyre = Tyre.create!
+
+    car = Car.create! do |c|
+      c.bulbs << bulb
+      c.tyres << tyre
+    end
+
+    assert_equal 1, car.bulbs.count
+    assert_equal 1, car.tyres.count
+  end
 end
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index d9659a72c5..cddf1a1f72 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -1147,23 +1147,4 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     club.members << member
     assert_equal 1, SuperMembership.where(member_id: member.id, club_id: club.id).count
   end
-
-  class ClubWithCallbacks < ActiveRecord::Base
-    self.table_name = 'clubs'
-    after_create :add_a_member
-
-    has_many :memberships, inverse_of: :club, foreign_key: :club_id
-    has_many :members, through: :memberships
-
-    def add_a_member
-      members << Member.last
-    end
-  end
-
-  def test_has_many_with_callback_before_association
-    Member.create!
-    club = ClubWithCallbacks.create!
-
-    assert_equal 1, club.reload.memberships.count
-  end
 end