Fix `collection.create` to could be rolled back by `after_save`

In `_create_record`, explicit `transaction` block requires rollback
handling manually when `insert_record` is failed.

We need to handle it in `_create_record`, not in `insert_record`, since
our test cases expect a record added to target and returned even if
`insert_record` is failed,

Closes #31488.

3 files changed, 30 insertions, 3 deletions

diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 7f1df9a21d..48cb819484 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -358,14 +358,18 @@ module ActiveRecord
           if attributes.is_a?(Array)
             attributes.collect { |attr| _create_record(attr, raise, &block) }
           else
+            record = build_record(attributes, &block)
             transaction do
-              add_to_target(build_record(attributes, &block)) do |record|
-                insert_record(record, true, raise) {
+              result = nil
+              add_to_target(record) do
+                result = insert_record(record, true, raise) {
                   @_was_loaded = loaded?
                   @association_ids = nil
                 }
               end
+              raise ActiveRecord::Rollback unless result
             end
+            record
           end
         end
 
@@ -443,7 +447,9 @@ module ActiveRecord
             end
           end
 
-          result && records
+          raise ActiveRecord::Rollback unless result
+
+          records
         end
 
         def replace_on_target(record, index, skip_callbacks)
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index cc8f33f142..0ca902385a 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -2705,6 +2705,17 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     end
   end
 
+  def test_create_children_could_be_rolled_back_by_after_save
+    firm = Firm.create!(name: "A New Firm, Inc")
+    assert_no_difference "Client.count" do
+      client = firm.clients.create(name: "New Client") do |cli|
+        cli.rollback_on_save = true
+        assert_not cli.rollback_on_create_called
+      end
+      assert client.rollback_on_create_called
+    end
+  end
+
   private
 
     def force_signal37_to_load_all_clients_of_firm
diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb
index 6219f57fa1..d4d5275b78 100644
--- a/activerecord/test/models/company.rb
+++ b/activerecord/test/models/company.rb
@@ -150,6 +150,16 @@ class Client < Company
     throw :abort if throw_on_save
   end
 
+  attr_accessor :rollback_on_save
+  after_save do
+    raise ActiveRecord::Rollback if rollback_on_save
+  end
+
+  attr_accessor :rollback_on_create_called
+  after_rollback(on: :create) do |client|
+    client.rollback_on_create_called = true
+  end
+
   class RaisedOnDestroy < RuntimeError; end
   attr_accessor :raise_on_destroy
   before_destroy do