Fixes count for collection object when there's explicit rollback in callbacks

activerecord/CHANGELOG.md

@@ -1,3 +1,50 @@
+*   Prevent ActiveRecord::Rollback to be swallowed and not reflect for collection objects
+
+    Previously collection models used to get persisted even when
+    there's an explicit ActiveRecord::Rollback in a callback and
+    change the count of the collection obect without any regard to
+    the callback specification. This used to happen because ActiveRecord::Rollback
+    used to be silently swallowed and the rest of the pipeline triggered
+    commit from it's previously assigned data values.
+
+    For example:
+
+    ```
+    class Post < ApplicationRecord
+        has_many :comments
+    end
+
+    class Comment < ApplicationRecord
+      belongs_to :post
+      after_create -> { raise ActiveRecord::Rollback }
+    end
+    ```
+
+    Before:
+    ```
+        post = Post.first
+        post.comments.count == true     # true
+
+        post.comments.create
+        post.comments.count == 0        # false
+    ```
+
+    After:
+    ```
+        post = Post.first
+        post.comments.count == true     # true
+
+        post.comments.create
+        post.comments.count == 0        # true
+    ```
+
+    This PR fixes the problem and makes the count consistent
+    if there's a rollback on a collection object.
+
+    Fixes #28836 and #28536
+
+    *Sayan Chakraborty*
+
 *   Undefine attribute methods on descendants when resetting column
     information.
 

activerecord/lib/active_record/associations/collection_association.rb

@@ -370,11 +370,14 @@ def _create_record(attributes, raise = false, &block)
 
         # Do the relevant stuff to insert the given record into the association collection.
         def insert_record(record, validate = true, raise = false, &block)
-          if raise
-            record.save!(validate: validate, &block)
-          else
-            record.save(validate: validate, &block)
-          end
+          saved = if raise
+                    record.save!(validate: validate, &block)
+                  else
+                    record.save(validate: validate, &block)
+                  end
+
+          raise ActiveRecord::Rollback unless saved
+          saved
         end
 
         def delete_or_destroy(records, method)

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -2543,6 +2543,21 @@ def self.name
     end
   end
 
+  def test_count_does_not_change_for_collection_if_rolled_back
+    klass = Class.new(Bulb) do
+      def self.name; "Bulb"; end
+      belongs_to :car, class_name: "Car"
+      after_create { raise ActiveRecord::Rollback }
+    end
+
+    car = Car.create!
+    initial_count = car.bulbs.count
+
+    car.bulbs << klass.create
+
+    assert_equal initial_count, car.bulbs.count
+  end
+
   class AuthorWithErrorDestroyingAssociation < ActiveRecord::Base
     self.table_name = "authors"
     has_many :posts_with_error_destroying,