Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix [#50260] Support :on option in #set_callback #50261

Merged
merged 1 commit into from Jan 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions 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.
Expand Down
19 changes: 19 additions & 0 deletions activerecord/lib/active_record/transactions.rb
Expand Up @@ -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
Expand Down
52 changes: 52 additions & 0 deletions activerecord/test/cases/transaction_callbacks_test.rb
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before patch:

Screenshot 2023-12-04 at 8 54 21 pm

end
end