From 253cb27559869a6241b78ea22e90a36052c7f750 Mon Sep 17 00:00:00 2001 From: Arit Developer <32520970+msarit@users.noreply.github.com> Date: Mon, 25 Jul 2022 07:58:41 -0400 Subject: [PATCH] Refactor :moderation_routes? method to use correct policies (#18183) * complete implementation * add spec coverage; remove test-db cleansers --- .../article_approvals_controller.rb | 4 ++-- app/controllers/moderations_controller.rb | 5 +++-- app/policies/article_policy.rb | 4 +++- app/policies/comment_policy.rb | 6 ++++++ spec/policies/article_policy_spec.rb | 6 +++--- spec/policies/comment_policy_spec.rb | 18 ++++++++++-------- 6 files changed, 27 insertions(+), 16 deletions(-) diff --git a/app/controllers/article_approvals_controller.rb b/app/controllers/article_approvals_controller.rb index 9dc513a80..9c26a5392 100644 --- a/app/controllers/article_approvals_controller.rb +++ b/app/controllers/article_approvals_controller.rb @@ -2,8 +2,8 @@ class ArticleApprovalsController < ApplicationController def create @article = Article.find(params[:id]) unless current_user.any_admin? - # Check that user is trusted - authorize(User, :moderation_routes?) + # Check that the article can be moderated by the user + authorize(@article, :moderate?) tags = @article.decorate.tags # Raise if no tags require approval to begin with raise Pundit::NotAuthorizedError unless tags.pluck(:requires_approval).include?(true) diff --git a/app/controllers/moderations_controller.rb b/app/controllers/moderations_controller.rb index b1e3ea545..8f148599a 100644 --- a/app/controllers/moderations_controller.rb +++ b/app/controllers/moderations_controller.rb @@ -30,8 +30,9 @@ class ModerationsController < ApplicationController end def comment - authorize(User, :moderation_routes?) + authorize(Comment, :moderate?) @moderatable = Comment.find(params[:id_code].to_i(26)) + render template: "moderations/mod" end @@ -43,7 +44,7 @@ class ModerationsController < ApplicationController private def load_article - authorize(User, :moderation_routes?) + authorize(Article, :moderate?) @tag_adjustment = TagAdjustment.new @moderatable = Article.find_by(slug: params[:slug]) diff --git a/app/policies/article_policy.rb b/app/policies/article_policy.rb index a180f529e..2811109f8 100644 --- a/app/policies/article_policy.rb +++ b/app/policies/article_policy.rb @@ -207,7 +207,9 @@ class ArticlePolicy < ApplicationPolicy # 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? + return true if user.trusted? + + elevated_user? end alias admin_featured_toggle? revoke_publication? diff --git a/app/policies/comment_policy.rb b/app/policies/comment_policy.rb index aa39a5417..d9b350e1a 100644 --- a/app/policies/comment_policy.rb +++ b/app/policies/comment_policy.rb @@ -25,6 +25,12 @@ class CommentPolicy < ApplicationPolicy true end + def moderate? + return true if user.trusted? + + moderator_create? + end + def moderator_create? user_moderator? || user_any_admin? end diff --git a/spec/policies/article_policy_spec.rb b/spec/policies/article_policy_spec.rb index aeed9906b..df648b316 100644 --- a/spec/policies/article_policy_spec.rb +++ b/spec/policies/article_policy_spec.rb @@ -124,10 +124,10 @@ RSpec.describe ArticlePolicy do 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 "permitted roles", to: %i[trusted super_admin admin], 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 + it_behaves_like "disallowed roles", to: %i[author], limit_post_creation_to_admins?: false + it_behaves_like "disallowed roles", to: %i[trusted super_admin admin], limit_post_creation_to_admins?: true end describe "#allow_tag_adjustment?" do diff --git a/spec/policies/comment_policy_spec.rb b/spec/policies/comment_policy_spec.rb index 549135e32..db61dab0e 100644 --- a/spec/policies/comment_policy_spec.rb +++ b/spec/policies/comment_policy_spec.rb @@ -28,7 +28,7 @@ RSpec.describe CommentPolicy, type: :policy do let!(:user) { create(:user) } it { is_expected.to permit_actions(%i[create]) } - it { is_expected.to forbid_actions(%i[edit update destroy delete_confirm hide unhide moderator_create]) } + it { is_expected.to forbid_actions(%i[edit update destroy delete_confirm hide unhide moderator_create moderate]) } it { is_expected.to forbid_actions(%i[admin_delete]) } it { is_expected.to permit_mass_assignment_of(valid_attributes_for_create).for_action(:create) } @@ -37,12 +37,14 @@ RSpec.describe CommentPolicy, type: :policy do before { user.add_role(:suspended) } it { is_expected.to forbid_actions(%i[create edit update destroy delete_confirm hide unhide admin_delete]) } + it { is_expected.to forbid_actions(%i[moderate]) } end context "with comment_suspended role" do before { user.add_role(:comment_suspended) } it { is_expected.to forbid_actions(%i[create edit update destroy delete_confirm hide unhide admin_delete]) } + it { is_expected.to forbid_actions(%i[moderate]) } end context "when user is a tag moderator" do @@ -51,7 +53,7 @@ RSpec.describe CommentPolicy, type: :policy do user.add_role(:tag_moderator, tag) end - it { is_expected.to permit_actions(%i[create moderator_create]) } + it { is_expected.to permit_actions(%i[create moderator_create moderate]) } it do expect(comment_policy).to permit_mass_assignment_of(valid_attributes_for_moderator_create) @@ -62,7 +64,7 @@ RSpec.describe CommentPolicy, type: :policy do context "when user is an admin or super_admin" do before { user.add_role(:admin) } - it { is_expected.to permit_actions(%i[create moderator_create admin_delete]) } + it { is_expected.to permit_actions(%i[create moderator_create admin_delete moderate]) } it do expect(comment_policy).to permit_mass_assignment_of(valid_attributes_for_moderator_create) @@ -75,7 +77,7 @@ RSpec.describe CommentPolicy, type: :policy do let(:user) { comment.user } it { is_expected.to permit_actions(%i[edit update new create delete_confirm destroy]) } - it { is_expected.to forbid_actions(%i[moderator_create admin_delete]) } + it { is_expected.to forbid_actions(%i[moderator_create admin_delete moderate]) } it { is_expected.to permit_mass_assignment_of(valid_attributes_for_create).for_action(:create) } it { is_expected.to permit_mass_assignment_of(valid_attributes_for_update).for_action(:update) } @@ -84,7 +86,7 @@ RSpec.describe CommentPolicy, type: :policy do before { user.add_role(:suspended) } it { is_expected.to permit_actions(%i[preview destroy delete_confirm]) } - it { is_expected.to forbid_actions(%i[edit update create hide unhide moderator_create admin_delete]) } + it { is_expected.to forbid_actions(%i[edit update create hide unhide moderator_create admin_delete moderate]) } it do expect(comment_policy).to permit_mass_assignment_of(valid_attributes_for_update).for_action(:update) @@ -95,7 +97,7 @@ RSpec.describe CommentPolicy, type: :policy do before { user.add_role(:comment_suspended) } it { is_expected.to permit_actions(%i[edit update destroy delete_confirm]) } - it { is_expected.to forbid_actions(%i[create hide unhide moderator_create admin_delete]) } + it { is_expected.to forbid_actions(%i[create hide unhide moderator_create admin_delete moderate]) } it do expect(comment_policy).to permit_mass_assignment_of(valid_attributes_for_update).for_action(:update) @@ -108,7 +110,7 @@ RSpec.describe CommentPolicy, type: :policy do user.add_role(:tag_moderator, tag) end - it { is_expected.to permit_actions(%i[edit update destroy delete_confirm moderator_create create]) } + it { is_expected.to permit_actions(%i[edit update destroy delete_confirm moderator_create create moderate]) } it { is_expected.to forbid_actions(%i[admin_delete]) } it do @@ -126,7 +128,7 @@ RSpec.describe CommentPolicy, type: :policy do let(:comment) { build_stubbed(:comment, commentable: article) } it { is_expected.to permit_actions(%i[hide unhide create]) } - it { is_expected.to forbid_actions(%i[edit update destroy delete_confirm]) } + it { is_expected.to forbid_actions(%i[edit update destroy delete_confirm moderate]) } it { is_expected.to forbid_actions(%i[moderator_create admin_delete]) } end end