Merge pull request #13751 from chancancode/ar_rollback_fix

Restore ActiveRecord states after a rollback for models w/o callbacks

activerecord/CHANGELOG.md

@@ -1,3 +1,11 @@
+*   ActiveRecord states are now correctly restored after a rollback for
+    models that did not define any transactional callbacks (i.e.
+    `after_commit`, `after_rollback` or `after_create`).
+
+    Fixes #13744.
+
+    *Godfrey Chan*
+
 *   Make `touch` fire the `after_commit` and `after_rollback` callbacks.
 
     *Harry Brundage*

activerecord/lib/active_record/connection_adapters/abstract/transaction.rb

@@ -23,6 +23,10 @@ module ActiveRecord
         @parent = nil
       end
 
+      def finalized?
+        @state
+      end
+
       def committed?
         @state == :committed
       end

activerecord/lib/active_record/core.rb

@@ -397,13 +397,10 @@ module ActiveRecord
     end
 
     def update_attributes_from_transaction_state(transaction_state, depth)
-      if transaction_state && !has_transactional_callbacks?
+      if transaction_state && transaction_state.finalized? && !has_transactional_callbacks?
         unless @reflects_state[depth]
-          if transaction_state.committed?
-            committed!
-          elsif transaction_state.rolledback?
-            rolledback!
-          end
+          restore_transaction_record_state if transaction_state.rolledback?
+          clear_transaction_record_state
           @reflects_state[depth] = true
         end
 

activerecord/test/cases/transactions_test.rb

@@ -430,17 +430,26 @@ class TransactionTest < ActiveRecord::TestCase
   end
 
   def test_restore_active_record_state_for_all_records_in_a_transaction
+    topic_without_callbacks = Class.new(ActiveRecord::Base) do
+      self.table_name = 'topics'
+    end
+
     topic_1 = Topic.new(:title => 'test_1')
     topic_2 = Topic.new(:title => 'test_2')
+    topic_3 = topic_without_callbacks.new(:title => 'test_3')
+
     Topic.transaction do
       assert topic_1.save
       assert topic_2.save
+      assert topic_3.save
       @first.save
       @second.destroy
       assert topic_1.persisted?, 'persisted'
       assert_not_nil topic_1.id
       assert topic_2.persisted?, 'persisted'
       assert_not_nil topic_2.id
+      assert topic_3.persisted?, 'persisted'
+      assert_not_nil topic_3.id
       assert @first.persisted?, 'persisted'
       assert_not_nil @first.id
       assert @second.destroyed?, 'destroyed'
@@ -451,6 +460,8 @@ class TransactionTest < ActiveRecord::TestCase
     assert_nil topic_1.id
     assert !topic_2.persisted?, 'not persisted'
     assert_nil topic_2.id
+    assert !topic_3.persisted?, 'not persisted'
+    assert_nil topic_3.id
     assert @first.persisted?, 'persisted'
     assert_not_nil @first.id
     assert !@second.destroyed?, 'not destroyed'