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 NoMethodError if callback is conditional and aborts using throw #19

Merged
merged 3 commits into from Sep 26, 2019

Conversation

Haniyya
Copy link
Contributor

@Haniyya Haniyya commented Sep 10, 2019

Added a test and a fix for an issue where _update_record may return
false instead of a number if one of the callbacks has a "filter" (so a
conditional if: or unless: key) and does a throw(:abort) to halt the
callback sequence.

This error is caused in

/activesupport/lib/active_support/callbacks.rb:130

but since it is part of the internal API there is no sense in reporting
that issue.

`false` instead of a number if one of the callbacks has a "filter" (so a
conditional if: or unless: key) and does a `throw(:abort)` to halt the
callback sequence.

This error is caused in

/activesupport/lib/active_support/callbacks.rb:130

but since it is part of the internal API there is no sense in reporting
that issue.
@Haniyya
Copy link
Contributor Author

Haniyya commented Sep 10, 2019

The changes to travis config were prompted by this.

Copy link
Owner

@stex stex left a comment

Choose a reason for hiding this comment

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

Looks good, thank you for the MR!

@@ -83,7 +83,9 @@ def changed_attributes
def _update_record(*)
super.tap do |affected_rows|
# Workaround to trigger a #touch if necessary, see +after_save+ callback further up
@_setting_accessors_touch_assignable = affected_rows.zero?
# The callback chain can, if a filter was provided for a certain callback and the callback throws and :abort,
Copy link
Owner

Choose a reason for hiding this comment

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

Typo, but I'll fix that later

@stex stex merged commit 6bed5af into stex:develop Sep 26, 2019
@stex stex mentioned this pull request Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants