This repository was archived by the owner on Dec 12, 2021. It is now read-only.
Allow multiple abilities with associations #726
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There are two issues with the current way cancan handles associations: 1) Records are returned multiple times in some circumstances 2) Several defined abilities prevent some records to show up under certain circumstances This commit includes tests for both cases. It fixes both problems by changing `joins` to `includes` for the AR adapters. This could have performance implications, since `includes` will also select all columns in the associated records. We tried various ways of achieving the same thing using Arel directly, but were unable to make this work due to lack of support for outer joins in Rails 3.1. This closes issues ryanb#724, ryanb#566 and ryanb#613
|
+1 for this PR. Currently I'm working around by chaining uniq every time I call accessible_by in one of these cases. |
|
If you don't mind the extra dependency, I wrote this in the meantime, which adds outer joins: https://github.com/elabs/ar_outer_joins This solves the problem even better because it doesn't have the ugly side effect of affecting the select clause. In that case |
|
+1 |
|
@ryanb why did you pull in 02841bbc195cdb17c6a4f511da44e0040e80f5f7, when this does the exact same change, except this also adds tests for the incorrect behaviour? |
|
@jnicklas You are right. This should be the pull request to pull in. Or at least pull in the specs. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are two issues with the current way cancan handles associations:
This commit includes tests for both cases. It fixes both problems by changing
joinstoincludesfor the AR adapters. This could have performance implications, sinceincludeswill also select all columns in the associated records. We tried various ways of achieving the same thing using Arel directly, but were unable to make this work due to lack of support for outer joins in Rails 3.1.This closes issues #724, #566 and #613