Merge pull request #15125 from arthurnn/small_refactoring_transaction_clear

Small refactoring on clear_transaction_record_state

2 files changed, 19 insertions, 4 deletions

diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb
index 17f76b63b3..51bda88be9 100644
--- a/activerecord/lib/active_record/transactions.rb
+++ b/activerecord/lib/active_record/transactions.rb
@@ -295,7 +295,7 @@ module ActiveRecord
     def committed! #:nodoc:
       run_callbacks :commit if destroyed? || persisted?
     ensure
-      @_start_transaction_state.clear
+      clear_transaction_record_state(true)
     end
 
     # Call the +after_rollback+ callbacks. The +force_restore_state+ argument indicates if the record
@@ -328,7 +328,7 @@ module ActiveRecord
         begin
           status = yield
         rescue ActiveRecord::Rollback
-          @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) - 1
+          clear_transaction_record_state
           status = nil
         end
 
@@ -353,9 +353,11 @@ module ActiveRecord
     end
 
     # Clear the new record state and id of a record.
-    def clear_transaction_record_state #:nodoc:
+    def clear_transaction_record_state(force = false) #:nodoc:
       @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) - 1
-      @_start_transaction_state.clear if @_start_transaction_state[:level] < 1
+      if force || @_start_transaction_state[:level] < 1
+        @_start_transaction_state.clear
+      end
     end
 
     # Restore the new record state and id of a record that was previously saved by a call to save_record_state.
diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb
index e6ed85394b..de1f624191 100644
--- a/activerecord/test/cases/transactions_test.rb
+++ b/activerecord/test/cases/transactions_test.rb
@@ -123,6 +123,19 @@ class TransactionTest < ActiveRecord::TestCase
     assert !Topic.find(1).approved?
   end
 
+  def test_rolling_back_in_a_callback_rollbacks_before_save
+    def @first.before_save_for_transaction
+      raise ActiveRecord::Rollback
+    end
+    assert !@first.approved
+
+    Topic.transaction do
+      @first.approved  = true
+      @first.save!
+    end
+    assert !Topic.find(@first.id).approved?, "Should not commit the approved flag"
+  end
+
   def test_raising_exception_in_nested_transaction_restore_state_in_save
     topic = Topic.new