-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Preserve Array#take(n) behaviour of HasManyAssociation #19105
Conversation
@rafaelfranca Please take a 👀 |
def test_taking_with_a_number | ||
assert_equal [posts(:misc_by_bob)], authors(:bob).posts.take(1) | ||
assert_equal [posts(:misc_by_bob), posts(:other_by_bob)], authors(:bob).posts.take(2) | ||
authors(:bob).posts.to_a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You want to assert prior to this line that authors(:bob).association(:post)
is not loaded. Otherwise you are not testing the "not loaded" code path.
Or for a more black box approach you can do:
assert_queries(1) do
authors(:bob).posts.to_a
end
Which will ensure to_a
makes the load happen rather than something earlier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comment. Just made it more comprehensible and push -f
ed.
Preserve Array#take(n) behaviour of HasManyAssociation
Preserve Array#take(n) behaviour of HasManyAssociation
I guess this should be backported to 4.1.x and 4.2.x , as this was broken on those branches |
I backported this to 4-1-stable, and to 4-1-10 rc, and 4-2-1 rc. (this was on 4-2-stable already) |
I'm not sure whether this behaviour change was by design, so let me PR.
take
method with an argument had been available on HasManyAssociation until 2b63a35. It used to fallback toArray#take(n)
.But now
association.take(n)
raisesArgumentError: wrong number of arguments (1 for 0)
.This patch retrieves
Array#take(n)
behaviour without breaking the AR version oftake
.Note that I didn't touch
take!
method because Ruby Array does not havetake!
.So
take
can take an argument buttake!
can not take an argument after this change.I think it's acceptable here, but I'd mention that the API would look a little bit inconsistent.