Fix accessible_by fires query for rules using association as condition. #569
Conversation
|
Rubocop doesn't like it, do you want |
There was a problem hiding this comment.
As discussed also in the issue, I need a real use case where the example is appropriate.
In this test, the ability is not defined correctly and there would be a better way of defining it.
I am not willing to fix an issue caused by a misuse of the library: I'd rather prefer the user to feel the pain for not using it correctly 😄.
Please, try to build an effective example where this happens, with the usage of a scope, for example, as you showed me in the issue. Thank you @albb0920 , I appreciate the effort you are putting into this issue! 🙇
| @@ -115,6 +115,17 @@ class User < ActiveRecord::Base | |||
| expect(Article.accessible_by(@ability)).to eq([article]) | |||
| end | |||
|
|
|||
| it 'does not fires query with accessible_by() for abilities defined with association' do | |||
| user = User.create! | |||
| @ability.can(:read, Article, user.articles) { true } | |||
There was a problem hiding this comment.
this is not how you write rules in cancancan. you should replace it with can :read, Article, user_id: user.id, as discussed also #552.
albb0920
force-pushed
the
avoid_load_target_on_ar_collection_proxy
branch
from
8107ded to
75f85f6
Compare
|
I've updated the test case, does this look more reasonable? |
if condition is an ActiveRecord::Associations::CollectionProxy, calling it's `==` method would load all records.
albb0920
force-pushed
the
avoid_load_target_on_ar_collection_proxy
branch
from
75f85f6 to
1916566
Compare
|
closed in #620 |
see #552
In turns out this could be fixed by swapping the left hand side and right hand side of
==.