diff --git a/app/policies/article_policy.rb b/app/policies/article_policy.rb index 7806d5535..43571d9ed 100644 --- a/app/policies/article_policy.rb +++ b/app/policies/article_policy.rb @@ -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? diff --git a/spec/policies/article_policy_spec.rb b/spec/policies/article_policy_spec.rb index ec5df6a7d..36aec1cb3 100644 --- a/spec/policies/article_policy_spec.rb +++ b/spec/policies/article_policy_spec.rb @@ -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 }