Merge pull request rails#54845 from Edouard-chin/ec-rollback-transaction

Fix `create_or_find_by` not rolling back a transaction:

Author: byroot

activerecord/lib/active_record/relation.rb

@@ -272,7 +272,12 @@ def find_or_create_by!(attributes, &block)
     # such situation.
     def create_or_find_by(attributes, &block)
       with_connection do |connection|
-        transaction(requires_new: true) { create(attributes, &block) }
+        record = nil
+        transaction(requires_new: true) do
+          record = new(attributes, &block)
+          record.save || raise(ActiveRecord::Rollback)
+        end
+        record
       rescue ActiveRecord::RecordNotUnique
         if connection.transaction_open?
           where(attributes).lock.find_by!(attributes)
@@ -287,7 +292,12 @@ def create_or_find_by(attributes, &block)
     # is raised if the created record is invalid.
     def create_or_find_by!(attributes, &block)
       with_connection do |connection|
-        transaction(requires_new: true) { create!(attributes, &block) }
+        record = nil
+        transaction(requires_new: true) do
+          record = new(attributes, &block)
+          record.save! || raise(ActiveRecord::Rollback)
+        end
+        record
       rescue ActiveRecord::RecordNotUnique
         if connection.transaction_open?
           where(attributes).lock.find_by!(attributes)

activerecord/test/cases/relations_test.rb

@@ -1593,6 +1593,24 @@ def test_create_or_find_by
     assert_not_equal subscriber, Subscriber.create_or_find_by(nick: "cat")
   end
 
+  def test_create_or_find_by_rollbacks_a_transaction
+    assert_no_difference(-> { Car.count }) do
+      car = BrokenCar.create_or_find_by(name: "Civic")
+
+      assert_instance_of(BrokenCar, car)
+      assert_not_predicate(car, :persisted?)
+    end
+  end
+
+  def test_create_or_find_by_bang_rollbacks_a_transaction
+    assert_no_difference(-> { Car.count }) do
+      car = BrokenCar.create_or_find_by!(name: "Civic")
+
+      assert_instance_of(BrokenCar, car)
+      assert_not_predicate(car, :persisted?)
+    end
+  end
+
   def test_create_or_find_by_with_block
     assert_nil Subscriber.find_by(nick: "bob")
 

activerecord/test/models/car.rb

@@ -35,3 +35,9 @@ class CoolCar < Car
 class FastCar < Car
   default_scope { order("name desc") }
 end
+
+class BrokenCar < Car
+  after_save do
+    raise ActiveRecord::Rollback
+  end
+end