Merge pull request #36142 from kamipo/should_take_first_record_state

Should take the record's state of first action in the transaction

2 files changed, 11 insertions, 8 deletions

diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb
index bf781b23eb..9e03deede7 100644
--- a/activerecord/lib/active_record/transactions.rb
+++ b/activerecord/lib/active_record/transactions.rb
@@ -333,7 +333,7 @@ module ActiveRecord
     # Ensure that it is not called if the object was never persisted (failed create),
     # but call it after the commit of a destroyed object.
     def committed!(should_run_callbacks: true) #:nodoc:
-      if should_run_callbacks && (destroyed? || persisted?)
+      if should_run_callbacks && trigger_transactional_callbacks?
         @_committed_already_called = true
         _run_commit_without_transaction_enrollment_callbacks
         _run_commit_callbacks
@@ -346,7 +346,7 @@ module ActiveRecord
     # Call the #after_rollback callbacks. The +force_restore_state+ argument indicates if the record
     # state should be rolled back to the beginning or just to the last savepoint.
     def rolledback!(force_restore_state: false, should_run_callbacks: true) #:nodoc:
-      if should_run_callbacks
+      if should_run_callbacks && trigger_transactional_callbacks?
         _run_rollback_callbacks
         _run_rollback_without_transaction_enrollment_callbacks
       end
@@ -364,7 +364,9 @@ module ActiveRecord
     def with_transaction_returning_status
       status = nil
       self.class.transaction do
-        unless has_transactional_callbacks?
+        if has_transactional_callbacks?
+          add_to_transaction
+        else
           sync_with_transaction_state if @transaction_state&.finalized?
           @transaction_state = self.class.connection.transaction_state
         end
@@ -372,11 +374,6 @@ module ActiveRecord
 
         status = yield
         raise ActiveRecord::Rollback unless status
-      ensure
-        if has_transactional_callbacks? &&
-            (@_new_record_before_last_commit && !new_record? || _trigger_update_callback || _trigger_destroy_callback)
-          add_to_transaction
-        end
       end
       status
     end
@@ -460,6 +457,10 @@ module ActiveRecord
         self.class.connection.add_transaction_record(self)
       end
 
+      def trigger_transactional_callbacks?
+        @_new_record_before_last_commit && !new_record? || _trigger_update_callback || _trigger_destroy_callback
+      end
+
       def has_transactional_callbacks?
         !_rollback_callbacks.empty? || !_commit_callbacks.empty? || !_before_commit_callbacks.empty?
       end
diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb
index 53fe31e087..135e2cb382 100644
--- a/activerecord/test/cases/transaction_callbacks_test.rb
+++ b/activerecord/test/cases/transaction_callbacks_test.rb
@@ -36,6 +36,8 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
 
     has_many :replies, class_name: "ReplyWithCallbacks", foreign_key: "parent_id"
 
+    before_destroy { self.class.find(id).touch if persisted? }
+
     before_commit { |record| record.do_before_commit(nil) }
     after_commit { |record| record.do_after_commit(nil) }
     after_save_commit { |record| record.do_after_commit(:save) }