From 1679fbf47df7a8e069afeda5be2270d28d561396 Mon Sep 17 00:00:00 2001 From: Arit Amana <32520970+msarit@users.noreply.github.com> Date: Fri, 3 Jun 2022 12:48:20 -0400 Subject: [PATCH] Allow Moderator Role to Unpublish Single Post (#17795) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * relocate btn; build functionality for admin only * remove commented code * add explanatory comment * refactor * build unpublish and feature-post policies * mod role in policies fails specs 😓 * fix failing specs * add end-to-end tests * fix hardcoded name in modal 😓 --- app/javascript/actionsPanel/actionsPanel.js | 40 +---- .../packs/articleModerationTools.js | 19 ++- app/javascript/packs/unpublishPostModal.jsx | 156 ++++++++++++++++++ app/models/user.rb | 1 + app/policies/application_policy.rb | 2 +- app/policies/article_policy.rb | 16 +- app/policies/authorizer.rb | 4 + app/views/articles/show.html.erb | 6 +- app/views/moderations/actions_panel.html.erb | 31 ++-- app/views/moderations/index.html.erb | 4 +- .../articleFlows/postModerationTools.spec.js | 24 +++ spec/factories/users.rb | 4 + spec/policies/article_policy_spec.rb | 26 ++- spec/policies/authorizer_spec.rb | 10 +- 14 files changed, 273 insertions(+), 70 deletions(-) create mode 100644 app/javascript/packs/unpublishPostModal.jsx diff --git a/app/javascript/actionsPanel/actionsPanel.js b/app/javascript/actionsPanel/actionsPanel.js index 76ea69cc8..1f37610a5 100644 --- a/app/javascript/actionsPanel/actionsPanel.js +++ b/app/javascript/actionsPanel/actionsPanel.js @@ -1,4 +1,5 @@ import { toggleFlagUserModal } from '../packs/flagUserModal'; +import { toggleUnpublishPostModal } from '../packs/unpublishPostModal'; import { request } from '@utilities/http'; export function addCloseListener() { @@ -149,33 +150,6 @@ export async function updateExperienceLevel( } } -const adminUnpublishArticle = async (id, username, slug) => { - try { - const response = await request(`/articles/${id}/admin_unpublish`, { - method: 'PATCH', - body: JSON.stringify({ id, username, slug }), - credentials: 'same-origin', - }); - - const outcome = await response.json(); - - /* eslint-disable no-restricted-globals */ - if (outcome.message == 'success') { - window.top.location.assign(`${window.location.origin}${outcome.path}`); - } else { - top.addSnackbarItem({ - message: `Error: ${outcome.message}`, - addCloseButton: true, - }); - } - } catch (error) { - top.addSnackbarItem({ - message: `Error: ${error}`, - addCloseButton: true, - }); - } -}; - const adminFeatureArticle = async (id, featured) => { try { const response = await request(`/articles/${id}/admin_featured_toggle`, { @@ -415,17 +389,7 @@ export function addBottomActionsListeners() { const unpublishArticleBtn = document.getElementById('unpublish-article-btn'); if (unpublishArticleBtn) { - unpublishArticleBtn.addEventListener('click', () => { - const { - articleId: id, - articleAuthor: username, - articleSlug: slug, - } = unpublishArticleBtn.dataset; - - if (confirm('You are unpublishing this post; are you sure?')) { - adminUnpublishArticle(id, username, slug); - } - }); + unpublishArticleBtn.addEventListener('click', toggleUnpublishPostModal); } document diff --git a/app/javascript/packs/articleModerationTools.js b/app/javascript/packs/articleModerationTools.js index eb85429da..88cad26ff 100644 --- a/app/javascript/packs/articleModerationTools.js +++ b/app/javascript/packs/articleModerationTools.js @@ -6,13 +6,23 @@ getCsrfToken().then(() => { if (articleContainer) { const user = userData(); - const { authorId: articleAuthorId, path } = articleContainer.dataset; + const { + articleId, + articleSlug, + authorId: articleAuthorId, + authorName, + authorUsername, + path, + } = articleContainer.dataset; const initializeModerationsTools = async () => { const { initializeActionsPanel } = await import( '../actionsPanel/initializeActionsPanelToggle' ); const { initializeFlagUserModal } = await import('./flagUserModal'); + const { initializeUnpublishPostModal } = await import( + './unpublishPostModal.jsx' + ); // 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 @@ -31,10 +41,17 @@ getCsrfToken().then(() => { if (user?.id !== articleAuthorId && !isModerationPage()) { initializeActionsPanel(user, path); initializeFlagUserModal(articleAuthorId); + initializeUnpublishPostModal( + articleId, + authorName, + authorUsername, + articleSlug, + ); // "/mod" page } else if (isModerationPage()) { initializeActionsPanel(user, path); initializeFlagUserModal(articleAuthorId); + initializeUnpublishPostModal(articleId, authorUsername, articleSlug); } } }; diff --git a/app/javascript/packs/unpublishPostModal.jsx b/app/javascript/packs/unpublishPostModal.jsx new file mode 100644 index 000000000..4eb379fd7 --- /dev/null +++ b/app/javascript/packs/unpublishPostModal.jsx @@ -0,0 +1,156 @@ +import { h, render } from 'preact'; +import PropTypes from 'prop-types'; +import { request } from '../utilities/http'; +import { ButtonNew as Button } from '@crayons'; +import RemoveIcon from '@images/x.svg'; + +async function confirmAdminUnpublishPost(id, username, slug) { + try { + const response = await request(`/articles/${id}/admin_unpublish`, { + method: 'PATCH', + body: JSON.stringify({ id, username, slug }), + credentials: 'same-origin', + }); + + const outcome = await response.json(); + + /* eslint-disable no-restricted-globals */ + if (outcome.message == 'success') { + window.top.location.assign(`${window.location.origin}${outcome.path}`); + } else { + top.addSnackbarItem({ + message: `Error: ${outcome.message}`, + addCloseButton: true, + }); + } + } catch (error) { + top.addSnackbarItem({ + message: `Error: ${error}`, + addCloseButton: true, + }); + } + + toggleUnpublishPostModal(); +} + +/** + * Shows or hides the flag user modal. + */ +export function toggleUnpublishPostModal() { + const modalContainer = top.document.getElementsByClassName( + 'unpublish-post-modal-container', + )[0]; + modalContainer.classList.toggle('hidden'); + + if (!modalContainer.classList.contains('hidden')) { + top.window.scrollTo(0, 0); + top.document.body.style.height = '100vh'; + top.document.body.style.overflowY = 'hidden'; + } else { + top.document.body.style.height = 'inherit'; + top.document.body.style.overflowY = 'inherit'; + } +} + +/** + * Initializes the Unpublish Post modal for the given article ID, author username and article slug. + * + * @param {number} articleId + * @param {string} authorUsername + * @param {string} articleSlug + */ +export function initializeUnpublishPostModal( + articleId, + authorName, + authorUsername, + articleSlug, +) { + // Check whether context is ModCenter or Friday-Night-Mode + const modContainer = document.getElementById('mod-container'); + + if (!modContainer) { + return; + } + + render( + , + document.getElementsByClassName('unpublish-post-modal-container')[0], + ); +} + +/** + * A modal for unpublishing a post. This can be used in the moderation center + * or on an article page. + * + * @param {number} props.articleId ID of the article to be unpublished. + * @param {string} props.authorUsername Username of the article's author. + * @param {string} props.articleSlug Slug of the article to be unpublished. + */ +export function UnpublishPostModal({ + articleId, + authorName, + authorUsername, + articleSlug, +}) { + return ( +
+
+
+

Unpublish post

+
+
+
+

+ Once unpublished, this post will become invisible to the public + and only accessible to {authorName}. +

+

They can still re-publish the post if they are not suspended.

+
+ +
+
+
+
+ + ); +} + +UnpublishPostModal.displayName = 'UnpublishPostModal'; +UnpublishPostModal.propTypes = { + articleId: PropTypes.number.isRequired, + authorUsername: PropTypes.string.isRequired, + articleSlug: PropTypes.string.isRequired, +}; diff --git a/app/models/user.rb b/app/models/user.rb index ba6af7a08..24cbbb7cd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -402,6 +402,7 @@ class User < ApplicationRecord :comment_suspended?, :creator?, :has_trusted_role?, + :moderator?, :podcast_admin_for?, :restricted_liquid_tag_for?, :single_resource_admin_for?, diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index 4e37e769c..c4632cf2e 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -203,7 +203,7 @@ class ApplicationPolicy delegate :support_admin?, to: :user - delegate :super_admin?, :any_admin?, :suspended?, to: :user, prefix: true + delegate :moderator?, :super_admin?, :any_admin?, :suspended?, to: :user, prefix: true alias minimal_admin? user_any_admin? deprecate minimal_admin?: "Deprecating #{self}#minimal_admin?, use #{self}#user_any_admin?" diff --git a/app/policies/article_policy.rb b/app/policies/article_policy.rb index 57ed310a5..1b3194377 100644 --- a/app/policies/article_policy.rb +++ b/app/policies/article_policy.rb @@ -144,9 +144,13 @@ class ArticlePolicy < ApplicationPolicy user_author? || user_super_admin? end - def admin_unpublish? + # this method performs the same checks that determine + # if the record can be featured + def revoke_publication? require_user! - user_any_admin? + return false unless @record.published? + + user_any_admin? || user_moderator? end def destroy? @@ -170,7 +174,13 @@ class ArticlePolicy < ApplicationPolicy user.trusted? end - alias admin_featured_toggle? admin_unpublish? + alias admin_featured_toggle? revoke_publication? + + alias toggle_featured_status? revoke_publication? + + # Due to the associated controller method "admin_unpublish", we + # alias "admin_ubpublish" to the "revoke_publication" method. + alias admin_unpublish? revoke_publication? alias new? create? diff --git a/app/policies/authorizer.rb b/app/policies/authorizer.rb index 85390002f..7ad3ee3ef 100644 --- a/app/policies/authorizer.rb +++ b/app/policies/authorizer.rb @@ -92,6 +92,10 @@ module Authorizer has_role?(:trusted) end + def moderator? + has_role?(:moderator) + end + def podcast_admin_for?(podcast) has_role?(:podcast_admin, podcast) end diff --git a/app/views/articles/show.html.erb b/app/views/articles/show.html.erb index 095527975..7aadd2e09 100644 --- a/app/views/articles/show.html.erb +++ b/app/views/articles/show.html.erb @@ -81,12 +81,15 @@
" data-path="<%= @article.path %>" - data-published="<%= @article.published? %>" data-pin-path="<%= stories_feed_pinned_article_path %>" data-pinned-article-id="<%= @pinned_article_id %>" + data-published="<%= @article.published? %>" <%= @article.pinned? ? "data-pinned" : " " %>>
<% if @article.video.present? %> @@ -216,6 +219,7 @@
+
<%= javascript_packs_with_chunks_tag "followButtons", defer: true %> diff --git a/app/views/moderations/actions_panel.html.erb b/app/views/moderations/actions_panel.html.erb index 3f7fce89d..018159f59 100644 --- a/app/views/moderations/actions_panel.html.erb +++ b/app/views/moderations/actions_panel.html.erb @@ -48,8 +48,14 @@ <%= crayons_icon_tag(:checkmark, class: "vomit-checkmark", title: t("views.moderations.actions.checkmark")) %> - <% if current_user.any_admin? && @moderatable.published %> - <% @recent_featured_count = Article.where(featured: true).where("published_at > ?", 1.day.ago).size %> + <% if policy(@moderatable).revoke_publication? %> + + <% end %> + <% if policy(@moderatable).toggle_featured_status? %>
- <%= t("views.moderations.actions.featured_past_day", count: @recent_featured_count) %> + <% recent_featured_count = Article.where(featured: true).where("published_at > ?", 1.day.ago).size %> + <%= t("views.moderations.actions.featured_past_day", count: recent_featured_count) %>
<% end %> @@ -175,8 +182,9 @@
- <% if current_user.any_admin? && @moderatable.published %> - - - <% end %> diff --git a/app/views/moderations/index.html.erb b/app/views/moderations/index.html.erb index c21ba8ab5..b0229657a 100644 --- a/app/views/moderations/index.html.erb +++ b/app/views/moderations/index.html.erb @@ -26,8 +26,8 @@ - + + <% else %>
diff --git a/cypress/integration/seededFlows/articleFlows/postModerationTools.spec.js b/cypress/integration/seededFlows/articleFlows/postModerationTools.spec.js index 6a3006016..1359e077a 100644 --- a/cypress/integration/seededFlows/articleFlows/postModerationTools.spec.js +++ b/cypress/integration/seededFlows/articleFlows/postModerationTools.spec.js @@ -79,6 +79,18 @@ describe('Moderation Tools for Posts', () => { }); }); }); + + it('should show Unpublish Post button on a published post for an admin user', () => { + cy.get('@adminUser').then((user) => { + cy.loginAndVisit(user, '/admin_mcadmin/test-article-slug').then(() => { + cy.findByRole('button', { name: 'Moderation' }).click(); + + cy.getIframeBody('[title="Moderation panel actions"]').within(() => { + cy.findByRole('button', { name: 'Unpublish Post' }).should('exist'); + }); + }); + }); + }); }); context('as moderator user', () => { @@ -111,6 +123,18 @@ describe('Moderation Tools for Posts', () => { ); }); }); + + it('should show Unpublish Post button on a published post for a moderator user', () => { + cy.get('@moderatorUser').then((user) => { + cy.loginAndVisit(user, '/admin_mcadmin/test-article-slug').then(() => { + cy.findByRole('button', { name: 'Moderation' }).click(); + + cy.getIframeBody('[title="Moderation panel actions"]').within(() => { + cy.findByRole('button', { name: 'Unpublish Post' }).should('exist'); + }); + }); + }); + }); }); context('as trusted user', () => { diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 7880fe09d..ee2c1560b 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -71,6 +71,10 @@ FactoryBot.define do after(:build) { |user| user.add_role(:admin) } end + trait :moderator do + after(:build) { |user| user.add_role(:moderator) } + end + trait :single_resource_admin do transient do resource { nil } diff --git a/spec/policies/article_policy_spec.rb b/spec/policies/article_policy_spec.rb index 43d251961..b895ee989 100644 --- a/spec/policies/article_policy_spec.rb +++ b/spec/policies/article_policy_spec.rb @@ -15,6 +15,7 @@ RSpec.describe ArticlePolicy do let(:trusted) { create(:user, :trusted) } let(:other_users) { create(:user) } let(:author) { create(:user) } + let(:moderator) { create(:user, :moderator) } let(:resource) { build(:article, user: author, organization: organization) } let(:policy) { described_class.new(user, resource) } @@ -153,13 +154,28 @@ RSpec.describe ArticlePolicy do it_behaves_like "disallowed roles", to: %i[admin org_admin other_users] end - %i[admin_unpublish? admin_featured_toggle?].each do |method_name| - describe "admin_unpublish?" do + %i[admin_unpublish? admin_featured_toggle? revoke_publication? toggle_featured_status?].each do |method_name| + describe "##{method_name}" do let(:policy_method) { method_name } - it_behaves_like "it requires an authenticated user" - it_behaves_like "permitted roles", to: %i[super_admin admin] - it_behaves_like "disallowed roles", to: %i[org_admin author other_users] + context "when published article" do + # need "create" (as opposed to "build") for the article to be published + let(:resource) { create(:article, user: author, organization: organization) } + + it_behaves_like "it requires an authenticated user" + it_behaves_like "permitted roles", to: %i[super_admin admin moderator] + it_behaves_like "disallowed roles", to: %i[org_admin author other_users] + end + + context "when unpublished article" do + let(:resource) do + build(:article, user: author, organization: organization, published: false, published_at: nil) + end + + it_behaves_like "it requires an authenticated user" + it_behaves_like "permitted roles", to: %i[] + it_behaves_like "disallowed roles", to: %i[super_admin admin moderator org_admin author other_users] + end end end diff --git a/spec/policies/authorizer_spec.rb b/spec/policies/authorizer_spec.rb index d55e6c363..6f35b3bcf 100644 --- a/spec/policies/authorizer_spec.rb +++ b/spec/policies/authorizer_spec.rb @@ -4,10 +4,11 @@ require "rails_helper" RSpec.describe Authorizer, type: :policy do subject(:authorizer) { described_class.for(user: user) } + let(:authorizer_mod_role) { described_class.for(user: mod_user) } let(:user) { create(:user) } + let(:mod_user) { create(:user, :moderator) } describe "#any_admin?" do - # This test che it "queries the user's roles" do # I want to test `expect(authorizer.admin?)` but our rubocop # version squaks. @@ -15,6 +16,13 @@ RSpec.describe Authorizer, type: :policy do end end + describe "#moderator?" do + it "queries the user's roles" do + expect(authorizer.moderator?).to be_falsey + expect(authorizer_mod_role.moderator?).to be_truthy + end + end + describe "#administrative_access_to?" do subject(:method_call) { authorizer.administrative_access_to?(resource: resource) }