diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index d289f616b8..304c48bf44 100644 --- a/activerecord/CHANGELOG.md +++ b/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* diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index 2b6685499a..bc4884b538 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/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 diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index cd8690d500..a4fe1efd33 100644 --- a/activerecord/lib/active_record/core.rb +++ b/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 diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 89dab16975..1664f1a096 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/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'