cancancan
cancancan copied to clipboard
Use `relation.and` instead of `relation.merge` whenever possible.
Currently cancancan uses relation.merge to apply scope in ability rule to the scope of load_resource through.
However, since relation.merge intends to mimic Hash#merge,
(i.e. later condition on the same column replaces the previous one),
this causes the load_resource through become ineffective when the scope in ability also specifics conditions on the same column.
This PR adds model adapter for ActiveRecord >= 6.1 and overrides database_records to use relation.and instead. It also tries to keep the query simple by only wrapping scopes to sub queries when necessary, which relies on private ActiveRecord API structurally_incompatible_values_for
Example:
ability file:
can :manage, Group
can :manage, Post, Post.where(group_id: user.groups) do |post|
post.gorup.users.exists?(user.id)
end
controller:
class PostsController < ApplicationController
load_and_authorize_resource :group
load_and_authorize_resource through: :group
def index
# @posts will be from all groups,
# as the condition for Post#group_id setted on `load_and_authorize_resource through`
# is replaced by the condition specified in ability rule during relation.merge
end
end
References
For more information on the behavior of relation.merge and relation.and that was introduced in rails 6.1,
please refer to rails/rails#39250 & rails/rails#40944
I'd like to tackle this by the next release that focuses on nested associations. Would it be possible for you to add the test for this case and rebase on the latest develop? If you don't have time, I'll take over. Thanks for your feedback and work
@coorasse I won't have a lot of free time until mid April, plus I'm not very familiar with the test suite of this project. Please take this over, thanks!
Would love to see this one go through :). Rails 7 forced us to monkeypatch with @albb0920 solution.