Как я мог реорганизовать Политику Pundit, чтобы сделать ее более СУХОЙ? - PullRequest
Новая
       26

Как я мог реорганизовать Политику Pundit, чтобы сделать ее более СУХОЙ?

0 голосов
/ 17 мая 2018

Я новичок в использовании Pundit с Rails и у меня есть новая политика для моей модели Artist, которая работает так, как я этого ожидаю, но я не знаю, как правильно реорганизовать ее, чтобы сделать ее более СУХОЙ. В частности, кажется, что я authorize @artist слишком часто звоню в artists_controller.rb, и в моем artist_policy.rb.

много дублирования кода.

Для контекста, у Художника есть имя, например, «Клод Моне», и все.

Вот мой artist_policy.rb: https://gist.github.com/leemcalilly/799d5f9136b92fcf92c6074e6a28bfdb

И, мое application_policy.rb: https://gist.github.com/leemcalilly/09d37a42c6f2500f98be3f1518c945e9

А вот и мой artist_controller.rb: https://gist.github.com/leemcalilly/c0dd8f33416b002f3b4c9a7baf0a3a75

А, модели / artist.rb: https://gist.github.com/leemcalilly/c190322af41f3e91739b53391d8b7834

Является ли способ, которым я в настоящее время работаю, нормально, потому что я очищаю выражение каждой политики (так же, как некоторый дублирующий код в интеграционных тестах, в порядке), или я должен реорганизовать это? Если да, то есть ли стандартный способ, которым люди структурируют свою политику Pundit, которую я пропускаю?

1 Ответ

0 голосов
/ 17 мая 2018

Кажется, я звоню authorize @artist слишком много раз в artists_controller.rb

Честно говоря, я думаю, что у вас там все в порядке.

Есть несколько способов, которыми вы могли бы попытаться быть умными в этом и "автоматически вызывать authorize" для каждого действия контроллера, но (предупреждение: основанный на мнении ответ) из прошлогоОпыт, который я обнаружил, что такие попытки сделать его более СУХИМ добавляет значительную путаницу.Особенно когда вы заканчиваете тем, что пишете некоторые действия контроллера, которые не нуждаются в авторизации или требуют авторизации необычным способом.

В моем artist_policy.rb

много дублирования кода

Один шаг за раз ... Вот оригинал: 

class ArtistPolicy < ApplicationPolicy
  attr_reader :user, :artist

  def initialize(user, artist)
    @user   = user
    @artist = artist
  end

  def create?
    if user.admin? || user.moderator? || user.contributor?
      true
    elsif user.banned?
      false
    end
  end

  def update?
    if user.admin? || user.moderator? || user.contributor? && user.id == @artist.user_id
      true
    elsif user.banned?
      false
    end
  end

  def destroy?
    if user.admin? || user.moderator? || user.contributor? && user.id == @artist.user_id
      true
    elsif user.banned?
      false
    end
  end
end

Нет необходимости определять свой собственный метод initialize таким образом, если вы будете рады ссылаться на болееуниверсальное имя переменной: record вместо artist (которое должно быть определено в ApplicationPolicy): 

class ArtistPolicy < ApplicationPolicy
  def create?
    if user.admin? || user.moderator? || user.contributor?
      true
    elsif user.banned?
      false
    end
  end

  def update?
    if user.admin? || user.moderator? || user.contributor? && user.id == record.user_id
      true
    elsif user.banned?
      false
    end
  end

  def destroy?
    if user.admin? || user.moderator? || user.contributor? && user.id == record.user_id
      true
    elsif user.banned?
      false
    end
  end
end

Далее, в подобных ситуациях хорошо ссылаться на одно правило политики из другого- при условии, что они в равной степени относятся к типам пользователей: 

class ArtistPolicy < ApplicationPolicy
  def create?
    if user.admin? || user.moderator? || user.contributor?
      true
    elsif user.banned?
      false
    end
  end

  def update?
    if user.admin? || user.moderator? || user.contributor? && user.id == record.user_id
      true
    elsif user.banned?
      false
    end
  end

  def destroy?
    update?
  end
end

Далее, обратите внимание, что record.user_id - это зарегистрированный пользователь для действия create!Таким образом, вы можете упростить это еще больше: 

class ArtistPolicy < ApplicationPolicy
  def create?
    if user.admin? || user.moderator? || user.contributor? && user.id == record.user_id
      true
    elsif user.banned?
      false
    end
  end

  def update?
    create?
  end

  def destroy?
    create?
  end
end

И, наконец, логика в этом методе на самом деле немного неправильна.(Вы могли бы поднять это с помощью тестов ...) Если пользователь является администратором , а они забанены, то вы все еще, вероятно, хотите, чтобы это вернуло false, а не true.Имея это в виду, мы можем исправить + снова упростить код до: 

class ArtistPolicy < ApplicationPolicy
  def create?
    return false if user.banned?
    user.admin? || user.moderator? || user.contributor? && user.id == record.user_id
  end

  def update?
    create?
  end

  def destroy?
    create?
  end
end
...