Add policy_class to permitted_attributes #789
Conversation
Signed-off-by: Marcel Hoppe <[email protected]>
Signed-off-by: Marcel Hoppe <[email protected]>
Signed-off-by: Marcel Hoppe <[email protected]>
| @@ -80,7 +80,8 @@ def authorize(user, possibly_namespaced_record, query, policy_class: nil, cache: | |||
| policy = if policy_class | |||
| policy_class.new(user, record) | |||
| else | |||
| cache[possibly_namespaced_record] ||= policy!(user, possibly_namespaced_record) | |||
| cache[{ policy_class: policy_class, | |||
There was a problem hiding this comment.
This is always nil here. What's the intention?
|
Hi! Thanks for the PR! I posted a comment in the ol' issue #742 (comment) about what I think we should probably do. I'd rather avoid having to modify other methods when adding support for this, to avoid breaking existing code. By adding a |
| def policy(record) | ||
| policies[record] ||= Pundit.policy!(pundit_user, record) | ||
| def policy(record, policy_class: nil) | ||
| policies[{ policy_class: policy_class, record: record }] ||= if policy_class |
There was a problem hiding this comment.
While policies is technically marked as private, I'm cautious against modifying its behaviour/content.
We maybe should cache when policy_class is passed, but that's a bigger consideration than adding the feature to permitted_attributes.
I believe that the cache behaviour of authorize(..., policy_class: SomePolicy) should be the same for permitted_attributes(..., policy_class: SomePolicy), and today the behaviour is to entirely bypass the cache.
hobbypunk90
deleted the
feature/add_policy_class_to_permitted_attributes
branch
Fixes #742 again 😅
Sorry, i cleaned my github a bit too fast.