How to make pundit policies more DRY?

Question

In one of my project I started to using pundit gem and I have a very simply policy that looks like this:

class CompanyPolicy < ApplicationPolicy
  def index?
    true if user.is_a? Administrator
  end

  def new?
    true if user.is_a? Administrator
  end

  def create?
    new?
  end

  def edit?
    true if user.is_a? Administrator
  end

  def update?
    edit?
  end
end

And the question is how can I avoid repeating this:

true if user.is_a? Administrator

Cacilia

try using before_filter and call it on all the above methods – Wyatt

This is pundit policy class not controller and I can't use before_filter... – Sandell

Answer 1

I do trick which looks like this:

class ApplicationPolicy

  private

  def self.permit_owner_to(*actions)
    actions.each do |action|
      define_method("#{action}?") do
        owner?
      end
    end
  end

  def owner?
    # owner logic
  end

end

And used it in other policies

class ItemPolicy < ApplicationPolicy

  permit_owner_to :show, :update, :destroy, :confirm

end

Othello

what about the case when you have like admin, staff, manager and so on? would you define a permit method for everyone in application controller? – Etymologize

Answer 2

I don't actually think you need to remove this. By repeating this you are explicitly saying that this user must be an administrator to access this method. If you did want to though, you could just create a private method.

class CompanyPolicy < ApplicationPolicy
  def index?
    admin?
  end

  def new?
    admin?
  end

  def create?
    new?
  end

  def edit?
    admin?
  end

  def update?
    edit?
  end

  private 
     def admin?
        user.is_a? Administrator
     end
end

Guess this is a matter of personal preference.

Seychelles

By keeping this test spread throughout the code base you risk a big problem if/when you decide to implement the test in a different way. For example, what if you chose to have an administrator role rather than administrator class? Lots of refactoring. – Eurhythmy

Answer 3

You could use alias_method.

class CompanyPolicy < ApplicationPolicy
  def index?
    user.is_a? Administrator
  end

  alias_method :create?, :index?
  alias_method :update?, :index?
end

You have a base class ApplicationPolicy which probably already contains:

def new?
  create?
end

def edit?
  update?
end

so you don't need to repeat these methods in your subclass.

.is_a? returns true or false so no need to explicitly return true if true.

That's a lot more succinct eh? :)

Fabriane

Answer 4

I combined answers from above and came up with the following:

class ApplicationPolicy
  attr_reader :user

  def initialize(user)
    @user = user
  end

  def self.permit(roles, options)
    return if options[:to].none?

    options[:to].each do |action|
      define_method("#{action}?") do
        return @user.roles? Array.wrap(roles) if options[:when].blank?

        send(options[:when]) and @user.roles? Array.wrap(roles)
      end
    end
  end
end

which allows one to use it like this:

class CommentPolicy < ApplicationPolicy
  attr_reader :user, :record

  def initialize(user, record)
    @record = record
    super(user)
  end

  permit %i[admin member], to: %i[show edit destroy update], when: :created_by_user

  def created_by_user
    @record.user == @user
  end
end

and

permit :admin, to: %i[index update edit]

works as well

my roles method from user model looks like:

def roles?(user_roles)
    user_roles.each do |role|
      return true if role?(role)
    end
    false
  end

  def role?(role)
    roles.any? { |r| r.name.underscore.to_sym == role }
  end

Etymologize