Merge pull request #28063 from Erol/prioritize-callback-on-action-before-if

Do not evaluate :if arguments when :on is not satisfied for transaction callbacks

2 files changed, 41 insertions, 1 deletions

diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb
index 08417aaa0f..690deee508 100644
--- a/activerecord/lib/active_record/transactions.rb
+++ b/activerecord/lib/active_record/transactions.rb
@@ -283,7 +283,7 @@ module ActiveRecord
             fire_on = Array(options[:on])
             assert_valid_transaction_action(fire_on)
             options[:if] = Array(options[:if])
-            options[:if] << "transaction_include_any_action?(#{fire_on})"
+            options[:if].unshift("transaction_include_any_action?(#{fire_on})")
           end
         end
 
diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb
index 391bbe8877..eaa4dd09a9 100644
--- a/activerecord/test/cases/transaction_callbacks_test.rb
+++ b/activerecord/test/cases/transaction_callbacks_test.rb
@@ -551,3 +551,43 @@ class TransactionEnrollmentCallbacksTest < ActiveRecord::TestCase
     assert_equal [:rollback], @topic.history
   end
 end
+
+class CallbacksOnActionAndConditionTest < ActiveRecord::TestCase
+  self.use_transactional_tests = false
+
+  class TopicWithCallbacksOnActionAndCondition < ActiveRecord::Base
+    self.table_name = :topics
+
+    after_commit(on: [:create, :update], if: :run_callback?) { |record| record.history << :create_or_update }
+
+    def clear_history
+      @history = []
+    end
+
+    def history
+      @history ||= []
+    end
+
+    def run_callback?
+      self.history << :run_callback?
+      true
+    end
+
+    attr_accessor :save_before_commit_history, :update_title
+  end
+
+  def test_callback_on_action_with_condition
+    topic = TopicWithCallbacksOnActionAndCondition.new
+    topic.save
+    assert_equal [:run_callback?, :create_or_update], topic.history
+
+    topic.clear_history
+    topic.approved = true
+    topic.save
+    assert_equal [:run_callback?, :create_or_update], topic.history
+
+    topic.clear_history
+    topic.destroy
+    assert_equal [], topic.history
+  end
+end