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

Extension: Multiple scoped rule definitions #788

Open
Taeir opened this issue Jun 10, 2022 · 0 comments
Open

Extension: Multiple scoped rule definitions #788

Taeir opened this issue Jun 10, 2022 · 0 comments

Comments

@Taeir
Copy link

Taeir commented Jun 10, 2022

It is stated in the documentation that you can use scopes (+blocks) as follows:

can :read, Article, Article.published do |article|
  article.published_at <= Time.now
end

However, when defining more than one of these and using accessible_by, CanCanCan cannot combine them (as stated in the documentation). For my project this is a severely limiting factor in how we can use CanCanCan.

I have made an extension to CanCanCan which does allow these scopes to be combined by redefining override_scope in ActiveRecordAdapter.

  def override_scope
    conditions = @compressed_rules.map(&:conditions).compact
    return unless conditions.any? { |c| c.is_a?(ActiveRecord::Relation) }
    return conditions.first if conditions.size == 1

    relation_conditions, hash_conditions = conditions.partition { |c| c.is_a?(ActiveRecord::Relation) }

    # Build the relation for the hash conditions
    if hash_conditions.any?
      if @model_class.respond_to?(:where) && @model_class.respond_to?(:joins)
        relation_conditions << build_relation(*hash_conditions)
      else
        relation_conditions << @model_class.all(conditions: hash_conditions, joins: joins)
      end
    end

    # Compact queries to the largest cliques using or. Stay in-place to avoid array element moving due to deletion (performance)
    (0..relation_conditions.length).each do |i|
      (i+1..relation_conditions.length).each do |j|
        next if relation_conditions[j].nil?

        begin
          relation_conditions[i] = relation_conditions[i].or(relation_conditions[j])
          relation_conditions[j] = nil
        rescue ArgumentError
          next
        end
      end
    end

    # Reduce to single query with union
    relation_conditions.reduce(nil) do |a, b|
      if a.nil?
        b
      elsif b.nil?
        a
      else
        a.union(b)
      end
    end
  end

This way CanCanCan can be used with multiple scopes without issues.

I would propose this as a PR but there is quite some polishing required to get it to the right place. Thus I decided to first open this issue to ask:

  • Are you interested in such an extension?
  • It currently relies on the activerecord union gem which adds union support. Without union I would need to rewrite it to use subqueries (@model_class.where(id: a).or(@model_class.where(id: b))). Is that still acceptable?
  • The code to combine scopes using or is a bit ugly. Perhaps that should be turned into a separate class for compression? It is an approximation algorithm for creating the largest possible combinations of queries, relying on ActiveRecord to do the heavy lifting with its or operation. It yields pretty good results, though there are a few issues:
    • I had assumed it would not care about order, but A.joins(:b).or(A.where(something: 1)) => fine, while A.where(something: 1).or(A.joins(:b)) => error. I consider this a bug in ActiveRecord (either things are structurally compatible or they are not), should we work around it in any way?
    • Rails 7 adds a way to check before combining whether things are structurally compatible which would remove the need for the begin-rescue, but I would not want to add a dependency on such a new version of ActiveRecord.

Would love to hear your thoughts.

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

No branches or pull requests

1 participant