From bcfbd523d4faec13c73e5c470c40edfc3602ebe0 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Wed, 11 May 2022 08:18:13 -0400 Subject: [PATCH] Moving from user.trusted tests to policy tests (#17619) The [`ArticlePolicy#moderate?`][1] method answers the questions around article moderation. At present this change does not introduce any significant changes (aside from the JS logic loop). The next "step" is to adjust the `ArticlePolicy#moderate?` method to test if the user has a moderator role. Further, we'd need to see what is available on the action panel. Related to forem/forem#17606 [1]:https://github.com/forem/forem/blob/5ac3216a5ae68edc0a0a41821c3ccf49ff100efd/app/policies/article_policy.rb#L160-L174 --- app/javascript/packs/articleModerationTools.js | 14 +++++++++++++- app/models/async_info.rb | 4 ++++ app/policies/article_policy.rb | 5 +---- spec/models/async_info_spec.rb | 5 ++++- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/app/javascript/packs/articleModerationTools.js b/app/javascript/packs/articleModerationTools.js index ecce323fb..eb85429da 100644 --- a/app/javascript/packs/articleModerationTools.js +++ b/app/javascript/packs/articleModerationTools.js @@ -14,8 +14,20 @@ getCsrfToken().then(() => { ); const { initializeFlagUserModal } = await import('./flagUserModal'); + // If the user can moderate an article give them access to this panel. + // Note: this assumes we're within the article context (which is the case + // given the articleContainer) + const canModerateArticles = user?.policies?.find( + (o) => + o.dom_class === 'js-policy-article-moderate' && o.visible === true, + ); + // article show page - if (user?.trusted) { + if (canModerateArticles) { + // <2022-05-09 Mon> [@jeremyf] the user.id is an integer and + // articleAuthorId is a string so our logic is such that we always + // initializeActionsPanel and initializeFlagUserModal; I'm asking + // product to clarify if we want mods to boost their own posts. if (user?.id !== articleAuthorId && !isModerationPage()) { initializeActionsPanel(user, path); initializeFlagUserModal(articleAuthorId); diff --git a/app/models/async_info.rb b/app/models/async_info.rb index 230da38cb..b2f548b48 100644 --- a/app/models/async_info.rb +++ b/app/models/async_info.rb @@ -54,6 +54,10 @@ class AsyncInfo dom_class: ApplicationPolicy.base_dom_class_for(record: Article, query: :create?), visible: visible?(record: Article, query: :create?) }, + { + dom_class: ApplicationPolicy.base_dom_class_for(record: Article, query: :moderate?), + visible: visible?(record: Article, query: :moderate?) + }, ], apple_auth: user.email.to_s.end_with?("@privaterelay.appleid.com") } diff --git a/app/policies/article_policy.rb b/app/policies/article_policy.rb index 6d0bedd7b..57ed310a5 100644 --- a/app/policies/article_policy.rb +++ b/app/policies/article_policy.rb @@ -155,8 +155,6 @@ 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. @@ -164,8 +162,7 @@ class ArticlePolicy < ApplicationPolicy 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 + # <2022-05-09 Mon> Don't let a user moderate their own article; though this may not be the desired behavior. return false if user_author? # Beware a trusted user does not guarantee that they are an admin. And more specifically, being diff --git a/spec/models/async_info_spec.rb b/spec/models/async_info_spec.rb index bfc858211..5fe084d17 100644 --- a/spec/models/async_info_spec.rb +++ b/spec/models/async_info_spec.rb @@ -12,12 +12,15 @@ RSpec.describe AsyncInfo do # controller, we should always have an authenticated user and it is not edge cached. before { allow(context).to receive(:current_user).and_return(user) } - it "has a policies key with an array of policies" do + it "has a policies key with an array of policies", :aggregate_failures do policies = async_info.fetch(:policies) expect(policies.length).to be > 0 # All policy keys will have dom_class and forbidden expect(policies.map(&:keys).uniq).to eq([%i[dom_class visible]]) + + expect(policies.map { |p| p.fetch(:dom_class) }.sort) + .to eq(%w[js-policy-article-create js-policy-article-moderate]) end end end