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

collection_singular_ids ignores association :include option #1120

Merged
merged 1 commit into from Jul 7, 2011
Merged

collection_singular_ids ignores association :include option #1120

merged 1 commit into from Jul 7, 2011

Conversation

lysenko
Copy link
Contributor

@lysenko lysenko commented May 18, 2011

This code was already on lighthouse. No one pay attention, so I make this pull request.
Github issue #925

@jasonnoble
Copy link
Contributor

@spastorino
Copy link
Contributor

Can you squash both commits into one?, thanks

…th conditions and includes,

when condtions references tables from includes.
Test fail because of invalid sql:
ActiveRecord::StatementInvalid: SQLite3::SQLException: no such column: comments.id:
SELECT "posts".id FROM "posts" INNER JOIN "readers" ON "posts"."id" = "readers"."post_id"
WHERE "readers"."person_id" = 1 AND (comments.id is null)

Bug described in github#925

This commit will revert fix from 3436fdf , but tests is ok.

Bug described in #6569 ticket.
@lysenko
Copy link
Contributor Author

lysenko commented Jul 6, 2011

Gist is good enough https://gist.github.com/1068249 or should I create another pull request?
Santiago, thanks for response, can you also look at this one #1119, please. I found a bug, but I can't find right solution or help from anyone.

Thanks

@spastorino
Copy link
Contributor

you can just push -f to your collection_singular_ids branch and github will do the right thing

@lysenko
Copy link
Contributor Author

lysenko commented Jul 6, 2011

thanks, github is really smart

spastorino added a commit that referenced this pull request Jul 7, 2011
collection_singular_ids ignores association :include option
@spastorino spastorino merged commit 1bad08f into rails:master Jul 7, 2011
@spastorino
Copy link
Contributor

@lysenko should this be backported to 3-1-stable?

@lysenko
Copy link
Contributor Author

lysenko commented Jul 7, 2011

@spastorino it should not, I think this patch can wait for next release.

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

3 participants