Best way to do multiple reasons

[tomdalling]

Hi @palkan and co,

We're looking at using ActionPolicy reasons, and I was wondering if there is any convention or best practice around generating multiple reasons instead of a single one.

A typical policy might look like this:

class CommentPolicy < ApplicationPolicy
  def create?
    check?(:post_allows_comments?) && check?(:user_not_banned?)
  end

  #...
end

Which would give a single failure reason.

allowance_to(:create?, @comment)[:reasons]
# => {comment: [:post_allows_comments?]}

But I noticed that if I run multiple checks.

class CommentPolicy < ApplicationPolicy
  def create?
    [
      check?(:post_allows_comments?),
      check?(:user_not_banned?),
    ].all?
  end

  #...
end

Then I get multiple reasons.

allowance_to(:create?, @comment)[:reasons]
# => {comment: [:post_allows_comments?, :user_not_banned?]}

I was thinking of making something like a check_all? method as a standard way to do this.

class CommentPolicy < ApplicationPolicy
  def create?
    check_all?(:post_allows_comments?, :user_not_banned?)
  end

  #...
end

Are multiple reasons an intended use case of ActionPolicy that I can rely on? I couldn't see an example of it in the docs. Can you think of any better way to do this?

[palkan]

Hey!

Are multiple reasons an intended use case of ActionPolicy that I can rely on?

Yes and no. Action Policy assumes fail-fast rules execution, thus, in case of AND logic, only the first failing predicate would produce a reason. The reason for that is performance: no need to evaluate rest predicates if we already know that the return value is false.
If you have, for example, check?(:x) || check?(:y) and both return false, then you have two reasons, x and y.

What you can do?

1. Evaluate checks independently (like in your all? example, or by assigning local vars).
2. Use & instead of &&. Assuming that check? returns true, false or nil, that would work exactly the same as && but without short-circuit evaluation:

def create?
  check?(:post_allows_comments?) & check?(:user_not_banned?)
end

[tomdalling]

Ok great, thanks. I like your suggestion to use &, because we won't need to add anything to the base class.