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

a test case that ensures AR::Relation#merge can merge associations #6605

Merged
merged 1 commit into from Jun 3, 2012

Conversation

amatsuda
Copy link
Member

@amatsuda amatsuda commented Jun 3, 2012

I was writing a tiny patch enabling AR::Relation#merge to accept a collection association instance.
In AR 3 we need to call scoped like this, but I thought it could be done implicitly.

euruko = Conference.find_by_name('euruko')
# runs separate queries for Relation and association
Attendee.where(name: 'matz').merge(euruko.attendees)

  Attendee Load (0.3ms)  SELECT "attendees".* FROM "attendees" WHERE "attendees"."conference_id" = 1
  Attendee Load (0.2ms)  SELECT "attendees".* FROM "attendees" WHERE "attendees"."name" = 'matz'
# merging into one query
Attendee.where(name: 'matz').merge(euruko.attendees.scoped)

  Attendee Load (0.3ms)  SELECT "attendees".* FROM "attendees" WHERE "attendees"."name" = 'matz' AND "attendees"."conference_id" = 1

So I finished writing my patch, rebased my patch against master, then noticed that this feature was already implemented in the series of merge refactoring done by @jonleighton
I'm not sure whether this was intentional or not (because there's no explicit test case for this), but I love this feature to be officially supported. So let me push my test case alone.

jonleighton added a commit that referenced this pull request Jun 3, 2012
a test case that ensures AR::Relation#merge can merge associations
@jonleighton jonleighton merged commit 9dbcaeb into rails:master Jun 3, 2012
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