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 doc: set_callback also accepts an array of if: #19595

Merged
merged 1 commit into from
Mar 31, 2015

Conversation

claudiob
Copy link
Member

When Active Record calls set_callback inside after_commit, these lines of code pass an array of methods as the :if condition:

options[:if] = Array(options[:if])
options[:if] << "transaction_include_any_action?(#{fire_on})"

That made me realize that anyone could pass an array of :if and :unless conditions to set_callback, since Active Support transforms these conditions into an array anyways in these lines of code:

@if      = Array(options[:if])
@unless  = Array(options[:unless])

Long story short, this commit updates the documentation of the set_callback method to explain that arrays are also accepted.

It also replaces +false+ and +true+ with false and true, since any falsey or truthy value will work.

[ci skip]

# callback will be called only when it returns a +false+ value.
# * <tt>:if</tt> - A symbol, a string or an array of symbols and strings,
# each naming an instance method or a proc; the callback will be called
# only when they all return a +true+ value.
Copy link
Member

Choose a reason for hiding this comment

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

I think we should not quote true as we are referring to Ruby's definition of a "true value" and not the exact value true.

Copy link
Member Author

Choose a reason for hiding this comment

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

I totally agree! I had the same thought when I reviewed my PR 🎀

When Active Record calls `set_callback` inside `after_commit`,
[these lines of code](https://github.com/rails/rails/blob/master/activerecord/lib/active_record/transactions.rb#L276)
pass an **array** of methods as the `:if` condition:

```ruby
options[:if] = Array(options[:if])
options[:if] << "transaction_include_any_action?(#{fire_on})"
```

That made me realize that anyone could pass an **array** of `:if` and `:unless`
conditions to `set_callback`, since Active Support transforms these conditions
into an array anyways in [these lines of code](https://github.com/rails/rails/blob/master/activesupport/lib/active_support/callbacks.rb#L365):

```ruby
@if      = Array(options[:if])
@unless  = Array(options[:unless])
```

Long story short, this commit updates the documentation of the `set_callback`
method to explain that arrays are also accepted.

It also replaces +false+ and +true+ with false and true, since any _falsey_ or
_truthy_ value will work.

[ci skip]
senny added a commit that referenced this pull request Mar 31, 2015
Fix doc: set_callback also accepts an array of if:
@senny senny merged commit 3472662 into rails:master Mar 31, 2015
@senny
Copy link
Member

senny commented Mar 31, 2015

@claudiob thanks man! 💛

senny added a commit that referenced this pull request Mar 31, 2015
Fix doc: set_callback also accepts an array of if:
@claudiob claudiob deleted the fix-docs-set-callback branch March 31, 2015 08:01
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