Merge pull request rails#50261 from joshuay03/activerecord-transactions-set-callback

Fix [rails#50260] Support `:on` option in `#set_callback`

Author: byroot

activerecord/CHANGELOG.md

@@ -1,3 +1,10 @@
+*   Introduce `ActiveRecord::Transactions::ClassMethods#set_callback`
+
+     It is identical to `ActiveSupport::Callbacks::ClassMethods#set_callback`
+     but with support for `after_commit` and `after_rollback` callback options.
+
+    *Joshua Young*
+
 *   Make `ActiveRecord::Encryption::Encryptor` agnostic of the serialization format used for encrypted data.
 
     Previously, the encryptor instance only allowed an encrypted value serialized as a `String` to be passed to the message serializer.

activerecord/lib/active_record/transactions.rb

@@ -266,6 +266,25 @@ def after_rollback(*args, &block)
         set_callback(:rollback, :after, *args, &block)
       end
 
+      # Similar to ActiveSupport::Callbacks::ClassMethods#set_callback, but with
+      # support for options available on #after_commit and #after_rollback callbacks.
+      def set_callback(name, *filter_list, &block)
+        options = filter_list.extract_options!
+        filter_list << options
+
+        if name.in?([:commit, :rollback]) && options[:on]
+          fire_on = Array(options[:on])
+          assert_valid_transaction_action(fire_on)
+          options[:if] = [
+            -> { transaction_include_any_action?(fire_on) },
+            *options[:if]
+          ]
+        end
+
+
+        super(name, *filter_list, &block)
+      end
+
       private
         def prepend_option
           if ActiveRecord.run_after_transaction_callbacks_in_order_defined

activerecord/test/cases/transaction_callbacks_test.rb

@@ -1029,3 +1029,55 @@ def with_run_commit_callbacks_on_first_saved_instances_in_transaction(value, mod
     end
   end
 end
+
+class SetCallbackTest < ActiveRecord::TestCase
+  self.use_transactional_tests = false
+
+  class TopicWithHistory < ActiveRecord::Base
+    self.table_name = :topics
+    self.run_commit_callbacks_on_first_saved_instances_in_transaction = true
+
+    def self.clear_history
+      @@history = []
+    end
+
+    def self.history
+      @@history ||= []
+    end
+  end
+
+  class TopicWithCallbacksOnUpdate < TopicWithHistory
+    after_commit :after_commit_on_update_1, on: :update
+    after_update_commit :after_commit_on_update_2
+
+    private
+      def after_commit_on_update_1
+        self.class.history << :after_commit_on_update_1
+      end
+
+      def after_commit_on_update_2
+        self.class.history << :after_commit_on_update_2
+      end
+  end
+
+  def test_set_callback_with_on
+    topic = TopicWithCallbacksOnUpdate.create!(title: "New topic", written_on: Date.today)
+    assert_empty TopicWithCallbacksOnUpdate.history
+
+    topic.update!(title: "Updated topic 1")
+    expected_history = [:after_commit_on_update_2, :after_commit_on_update_1]
+    assert_equal expected_history, TopicWithCallbacksOnUpdate.history
+
+    TopicWithCallbacksOnUpdate.skip_callback(:commit, :after, :after_commit_on_update_2)
+    topic.update!(title: "Updated topic 2")
+    expected_history << :after_commit_on_update_1
+    assert_equal expected_history, TopicWithCallbacksOnUpdate.history
+
+    TopicWithCallbacksOnUpdate.set_callback(:commit, :after, :after_commit_on_update_2, on: :update)
+    topic = TopicWithCallbacksOnUpdate.create!(title: "New topic", written_on: Date.today)
+    topic.update!(title: "Updated topic 3")
+    expected_history << :after_commit_on_update_2
+    expected_history << :after_commit_on_update_1
+    assert_equal expected_history, TopicWithCallbacksOnUpdate.history
+  end
+end