Adding ArticlePolicy#moderate? (#16786)

This commit does three things:

1) Adds and tests the ArticlePolicy#moderate? feature
2) Refactors the spec helpers to be a bit more flexible
3) Adds a few more edge case tests around ArticlePolicy

My apologies for conflating these changes, as it makes this commit
more challenging to review.

The goal is to adequate demonstrate the logic around users who can or
cannot moderate.  At present, the rules for who can moderate is here:

841491c6ee/app/javascript/packs/articleModerationTools.js (L17-L27)

```js
if (user?.trusted) {
   if (user?.id !== articleAuthorId && !isModerationPage()) {
     initializeActionsPanel(user, path);
     initializeFlagUserModal(articleAuthorId);
     // "/mod" page
   } else if (isModerationPage()) {
     initializeActionsPanel(user, path);
     initializeFlagUserModal(articleAuthorId);
   }
}
```

- Related to forem/forem#16783
- Closes forem/forem#16784
This commit is contained in:
Jeremy Friesen 2022-03-07 15:27:43 -05:00 committed by GitHub
parent 61554af5e6
commit d1069af35c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 78 additions and 52 deletions

View file

@ -112,6 +112,24 @@ class ArticlePolicy < ApplicationPolicy
user_author? || user_super_admin? || user_org_admin? || user_any_admin?
end
# @see https://github.com/forem/forem/blob/841491c6ee7f9a46d8033b4b55052316db251863/app/javascript/packs/articleModerationTools.js#L17-L27
# for details regarding original authorization logic for this method.
def moderate?
# Technically, we could check the limit_post_creation_to_admins? first, but [@jeremyf]'s
# operating on a "trying to maintain consistency" approach.
require_user_in_good_standing!
return false if self.class.limit_post_creation_to_admins?
# Don't let a user moderate their own article. See for prior UI logic reinforcing this
# https://github.com/forem/forem/blob/841491c6ee7f9a46d8033b4b55052316db251863/app/javascript/packs/articleModerationTools.js#L17-L27
return false if user_author?
# Beware a trusted user does not guarantee that they are an admin. And more specifically, being
# an admin does not guarantee being trusted.
user.trusted?
end
alias admin_featured_toggle? admin_unpublish?
alias new? create?

View file

@ -4,83 +4,77 @@ require "rails_helper"
# And to extract would require more explict interface definition.
RSpec.shared_examples "it requires an authenticated user" do
let(:user) { nil }
specify "otherwise it raises ApplicationPolicy::UserRequiredError" do
it "otherwise raises ApplicationPolicy::UserRequiredError" do
expect { subject }.to raise_error(ApplicationPolicy::UserRequiredError)
end
end
RSpec.shared_examples "it requires a user in good standing" do
let(:author) { create(:user, :suspended) }
specify "otherwise it raises ApplicationPolicy::UserSuspendedError" do
let(:user) { create(:user, :suspended) }
it "otherwise raises ApplicationPolicy::UserSuspendedError" do
expect { subject }.to raise_error(ApplicationPolicy::UserSuspendedError)
end
end
RSpec.shared_examples "permitted roles" do |kwargs|
Array(kwargs.fetch(:to)).each do |role|
context "#{role.inspect} authorization" do
case role
when :suspended_author
let(:author) { create(:user, :suspended) }
when :org_admin
let(:organization) { user.organizations.first }
let(:user) { create(:user, role, :org_admin) }
when :anyone
let(:user) { create(:user) }
RSpec.shared_examples "permitted roles" do |**kwargs|
to = kwargs.delete(:to)
label = kwargs.except(:to).map { |key, value| "#{key} is #{value}" }.join(" AND ")
label = "when #{label} " if label.present?
Array(to).each do |role|
context "#{label}#{role.inspect} authorization" do
before { kwargs.each { |k, v| allow(described_class).to receive(k).and_return(v) } }
if role == :suspended_author
let(:author) { suspended_user }
let(:user) { author }
else
let(:user) { create(:user, role) }
let(:user) { public_send(role) }
end
it { is_expected.to be_truthy }
end
end
end
RSpec.shared_examples "disallowed roles" do |kwargs|
Array(kwargs.fetch(:to)).each do |role|
context "#{role.inspect} authorization" do
case role
when :org_admin
let(:organization) { user.organizations.first }
let(:user) { create(:user, role, :org_admin) }
when :author
RSpec.shared_examples "disallowed roles" do |**kwargs|
to = kwargs.delete(:to)
label = kwargs.map { |key, value| "#{key} is #{value}" }.join(" AND ")
label = "when #{label} " if label.present?
Array(to).each do |role|
context "#{label}#{role.inspect} authorization" do
before { kwargs.each { |k, v| allow(described_class).to receive(k).and_return(v) } }
if role == :suspended_author
let(:author) { suspended_user }
let(:user) { author }
when :other_users
let(:user) { create(:user) }
else
let(:user) { create(:user, role) }
let(:user) { public_send(role) }
end
it { is_expected.to be_falsey }
end
end
end
RSpec.shared_examples "it is otherwise unavailable" do
let(:user) { create(:user) }
let(:author) { create(:user) }
it { is_expected.to be_falsey }
end
RSpec.shared_examples "when limit_post_creation_to_admins is enabled" do |kwargs|
before { allow(described_class).to receive(:limit_post_creation_to_admins?).and_return(true) }
Array(kwargs.fetch(:authorizes)).each do |role|
context "when user is #{role.inspect}" do
let(:user) { create(:user, role) }
it { is_expected.to be_truthy }
end
end
context "with the \"default\" user" do
it { is_expected.to be_falsey }
end
end
RSpec.describe ArticlePolicy do
subject(:method_call) { policy.public_send(policy_method) }
let(:organization) { org_admin&.organizations&.first }
# The named "folks" with one or more roles
let(:org_admin) { create(:user, :org_admin) }
let(:anyone) { create(:user) }
let(:super_admin) { create(:user, :super_admin) }
let(:suspended_user) { create(:user, :suspended) }
let(:admin) { create(:user, :admin) }
let(:trusted) { create(:user, :trusted) }
let(:other_users) { create(:user) }
let(:author) { create(:user) }
let(:organization) { nil }
let(:user) { author }
let(:resource) { build(:article, user: author, organization: organization) }
let(:policy) { described_class.new(user, resource) }
@ -111,6 +105,7 @@ RSpec.describe ArticlePolicy do
describe "#feed?" do
let(:policy_method) { :feed? }
let(:user) { build(:user) }
it { is_expected.to be_truthy }
end
@ -121,11 +116,24 @@ RSpec.describe ArticlePolicy do
it_behaves_like "it requires an authenticated user"
it_behaves_like "it requires a user in good standing"
it_behaves_like "permitted roles", to: [:anyone]
it_behaves_like "when limit_post_creation_to_admins is enabled", authorizes: %i[super_admin admin]
it_behaves_like "permitted roles", to: %i[anyone], limit_post_creation_to_admins?: false
it_behaves_like "permitted roles", to: %i[super_admin admin], limit_post_creation_to_admins?: true
it_behaves_like "disallowed roles", to: %i[anyone], limit_post_creation_to_admins?: true
end
end
describe "#moderate?" do
let(:policy_method) { :moderate? }
it_behaves_like "it requires a user in good standing"
it_behaves_like "it requires an authenticated user"
it_behaves_like "permitted roles", to: %i[trusted], limit_post_creation_to_admins?: false
it_behaves_like "disallowed roles", to: %i[super_admin admin author], limit_post_creation_to_admins?: false
it_behaves_like "disallowed roles", to: %i[trusted], limit_post_creation_to_admins?: true
end
%i[update? edit?].each do |method_name|
describe "##{method_name}" do
let(:policy_method) { method_name }