Merge pull request rails#54922 from Edouard-chin/ec-transaction-return-status

Fix `find_or_create_by!` not checking owner persistence:

Author: byroot

activerecord/lib/active_record/relation.rb

@@ -274,8 +274,8 @@ def create_or_find_by(attributes, &block)
       with_connection do |connection|
         record = nil
         transaction(requires_new: true) do
-          record = new(attributes, &block)
-          record.save || raise(ActiveRecord::Rollback)
+          record = create(attributes, &block)
+          record._last_transaction_return_status || raise(ActiveRecord::Rollback)
         end
         record
       rescue ActiveRecord::RecordNotUnique
@@ -294,8 +294,8 @@ def create_or_find_by!(attributes, &block)
       with_connection do |connection|
         record = nil
         transaction(requires_new: true) do
-          record = new(attributes, &block)
-          record.save! || raise(ActiveRecord::Rollback)
+          record = create!(attributes, &block)
+          record._last_transaction_return_status || raise(ActiveRecord::Rollback)
         end
         record
       rescue ActiveRecord::RecordNotUnique

activerecord/lib/active_record/transactions.rb

@@ -13,7 +13,7 @@ module Transactions
                        scope: [:kind, :name]
     end
 
-    attr_accessor :_new_record_before_last_commit # :nodoc:
+    attr_accessor :_new_record_before_last_commit, :_last_transaction_return_status # :nodoc:
 
     # = Active Record \Transactions
     #
@@ -436,6 +436,7 @@ def with_transaction_returning_status
           status = yield
           raise ActiveRecord::Rollback unless status
         end
+        @_last_transaction_return_status = status
         status
       end
     end
@@ -451,6 +452,7 @@ def trigger_transactional_callbacks? # :nodoc:
       def init_internals
         super
         @_start_transaction_state = nil
+        @_last_transaction_status = nil
         @_committed_already_called = nil
         @_new_record_before_last_commit = nil
       end

activerecord/test/cases/relations_test.rb

@@ -26,6 +26,7 @@
 require "models/category"
 require "models/categorization"
 require "models/edge"
+require "models/wheel"
 require "models/subscriber"
 require "models/cpk"
 
@@ -1611,6 +1612,15 @@ def test_create_or_find_by_bang_rollbacks_a_transaction
     end
   end
 
+  def test_create_or_find_by_on_a_collections_rollbacks_a_transaction_when_owner_is_not_persisted
+    car = BrokenCar.create(name: "Civic")
+    assert_not_predicate(car, :persisted?)
+
+    assert_raises(ActiveRecord::RecordNotSaved) do
+      car.wheels.create_or_find_by!(size: 1500)
+    end
+  end
+
   def test_create_or_find_by_with_block
     assert_nil Subscriber.find_by(nick: "bob")