Save list of previously-published articles when user is subject to an "un-publish all" action (#18535)

* Moved logging and deleting comments to Moderator::UnpublishAllArticlesWorker

* Added logging for admin action (unpublish_all_articles)

* Extracted code from UnpublishAllArticlesWorker to the service

* Make it possible to add a note while unpublishing all via admin action

* Added ability to add notes for unpublishing all from moderator action panel

* Fixed setting slug for the AuditLog on unpublishing + spec

app/controllers/admin/users_controller.rb

@@ -30,7 +30,7 @@ module Admin
       unpublish_all_articles: :unpublish_all_articles?
     }.freeze
 
-    after_action only: %i[update user_status banish full_delete unpublish_all_articles merge] do
+    after_action only: %i[update user_status banish full_delete merge] do
       Audit::Logger.log(:moderator, current_user, params.dup)
     end
 
@@ -193,7 +193,12 @@ module Admin
     end
 
     def unpublish_all_articles
-      Moderator::UnpublishAllArticlesWorker.perform_async(params[:id].to_i)
+      target_user = User.find(params[:id].to_i)
+      Moderator::UnpublishAllArticlesWorker.perform_async(target_user.id, current_user.id, "moderator")
+      if params[:note] && params[:note][:content]
+        Note.create(noteable: target_user, reason: "unpublish_all_articles",
+                    content: params[:note][:content], author: current_user)
+      end
       message = I18n.t("admin.users_controller.unpublished")
       respond_to do |format|
         format.html do

app/controllers/concerns/api/users_controller.rb

@@ -49,23 +49,9 @@ module Api
       authorize(@user, :unpublish_all_articles?)
 
       target_user = User.find(params[:id].to_i)
-      target_comments = target_user.comments.where(deleted: false)
-
-      # Keep track of affected ids for AuditLog trail
-      article_ids = Article.published.where(user_id: target_user.id).ids
-      comment_ids = target_comments.ids
 
       # Unpublish posts and delete comments w/ boolean attr to allow revert
-      Moderator::UnpublishAllArticlesWorker.perform_async(target_user.id)
-      target_comments.update(deleted: true)
-
-      payload = {
-        action: "api_user_unpublish",
-        target_user_id: target_user.id,
-        target_article_ids: article_ids,
-        target_comment_ids: comment_ids
-      }
-      Audit::Logger.log(:admin_api, @user, payload)
+      Moderator::UnpublishAllArticlesWorker.perform_async(target_user.id, @user.id)
 
       render status: :no_content
     end

app/javascript/packs/modals/unpublishAllPosts.js

@@ -5,12 +5,15 @@ const unpublishAllPosts = async (event) => {
   event.preventDefault();
   const { userId } = event.target.dataset;
 
+  const noteTextarea = window.parent.document.getElementById('note_content');
+  const params = { id: userId, note: { content: noteTextarea.value } };
+
   try {
     const response = await request(
       `/admin/member_manager/users/${userId}/unpublish_all_articles`,
       {
         method: 'POST',
-        body: JSON.stringify({ id: userId }),
+        body: JSON.stringify(params),
         credentials: 'same-origin',
       },
     );

app/services/moderator/unpublish_all_articles.rb

@@ -0,0 +1,71 @@
+module Moderator
+  class UnpublishAllArticles
+    # @param target_user_id [Integer] the id of the user whose posts are being unpublished
+    # @param action_user_id [Integer] the id of the user who unpublishes
+    # @param listener [String] listener for the audit logger
+    def initialize(target_user_id:, action_user_id:, listener: :admin_api)
+      @target_user_id = target_user_id
+      @action_user_id = action_user_id
+      @listener = listener
+    end
+
+    delegate :user_data, to: Notifications
+
+    def self.call(...)
+      new(...).call
+    end
+
+    def call
+      user = User.find_by(id: target_user_id)
+      return unless user
+
+      target_comments = user.comments.where(deleted: false)
+      target_articles = user.articles.published
+
+      # cache for logging
+      target_comments_ids = target_comments.ids
+      target_articles_ids = target_articles.ids
+
+      target_comments.update(deleted: true)
+
+      target_articles.find_each do |article|
+        if article.has_frontmatter?
+          article.body_markdown.sub!(/^published:\s*true\s*$/, "published: false")
+        end
+        article.published = false
+        article.save(validate: false)
+        clean_up_notifications(article)
+      end
+
+      payload = {
+        target_user_id: target_user_id,
+        target_article_ids: target_articles_ids,
+        target_comment_ids: target_comments_ids
+      }
+
+      audit_log(payload)
+    end
+
+    private
+
+    attr_reader :target_user_id, :action_user_id, :listener
+
+    def clean_up_notifications(article)
+      Notification.remove_all_by_action_without_delay(notifiable_ids: article.id,
+                                                      notifiable_type: "Article",
+                                                      action: "Published")
+
+      ContextNotification.delete_by(context_id: article.id, context_type: "Article", action: "Published")
+
+      return unless article.comments.exists?
+
+      Notification.remove_all(notifiable_ids: article.comments.ids, notifiable_type: "Comment")
+    end
+
+    def audit_log(payload = {})
+      payload["action"] = listener == :moderator ? "unpublish_all_articles" : "api_user_unpublish"
+      action_user = User.find_by(id: action_user_id)
+      Audit::Logger.log(listener, action_user, payload)
+    end
+  end
+end

app/views/admin/users/modals/_unpublish_modal.html.erb

@@ -1,7 +1,11 @@
 <div id="unpublish-all-posts">
-  <%= form_for(:user, html: { class: "js-unpublish-form flex flex-col gap-4", method: :post, onsubmit: "return confirm('Are you sure? All posts will be unavailable to the community.')", id: nil }) do |f| %>
+  <%= form_tag("/admin/member_manager/users/#{@user.id}/unpublish_all_articles", html: { class: "js-unpublish-form flex flex-col gap-4",
+                                                                                         method: :post,
+                                                                                         onsubmit: "return confirm('Are you sure? All posts will be unavailable to the community.')",
+                                                                                         id: nil }) do %>
     <p>Once unpublished, all posts by <span class="js-user-name"></span> will become hidden and only accessible to themselves.</p>
-    <p>If <span class="js-user-name"></span> is not suspended, they can still re-publish their posts from their dashboard.</p>
+    <p class="mt-2">If <span class="js-user-name"></span> is not suspended, they can still re-publish their posts from their dashboard.</p>
+    <p class="mt-2"><%= label_tag "note[content]", "Note:" %><%= text_area_tag "note[content]", "", class: "crayons-textfield", placeholder: "Note text" %></p>
     <div>
       <button class="c-btn c-btn--destructive c-btn--primary">Unpublish all posts</button>
     </div>

app/views/moderations/modals/_unpublish_all_posts.html.erb

@@ -5,6 +5,7 @@
   <p class="mt-2">
     <%= t("views.moderations.actions.unpublish_all.modal_text2", username: @article.username) %>
   </p>
+  <p class="mt-2"><%= label_tag "note[content]", "Note:" %><%= text_area_tag "note[content]", "", class: "crayons-textfield note_content", placeholder: "Note text" %></p>
   <div class="mt-4">
     <button
       id="unpublish-all-posts-submit-btn"

app/workers/moderator/unpublish_all_articles_worker.rb

@@ -2,32 +2,19 @@ module Moderator
   class UnpublishAllArticlesWorker
     include Sidekiq::Job
 
+    ALLOWED_LISTENERS = %i[admin_api moderator].freeze
+    DEFAULT_LISTENER = :admin_api
+
     sidekiq_options queue: :medium_priority, retry: 10
 
-    def perform(user_id)
-      user = User.find_by(id: user_id)
-      return unless user
-
-      user.articles.published.find_each do |article|
-        if article.has_frontmatter?
-          article.body_markdown.sub!(/^published:\s*true\s*$/, "published: false")
-        end
-        article.published = false
-        article.save(validate: false)
-        clean_up_notifications(article)
-      end
-    end
-
-    def clean_up_notifications(article)
-      Notification.remove_all_by_action_without_delay(notifiable_ids: article.id,
-                                                      notifiable_type: "Article",
-                                                      action: "Published")
-
-      ContextNotification.delete_by(context_id: article.id, context_type: "Article", action: "Published")
-
-      return unless article.comments.exists?
-
-      Notification.remove_all(notifiable_ids: article.comments.ids, notifiable_type: "Comment")
+    # @param target_user_id [Integer] the user who is being unpublished
+    # @param action_user_id [Integer] the user who takes action / unpublishes
+    def perform(target_user_id, action_user_id, listener = "admin_api")
+      listener = listener.to_sym
+      listener = DEFAULT_LISTENER unless ALLOWED_LISTENERS.include?(listener)
+      UnpublishAllArticles.call(target_user_id: target_user_id,
+                                action_user_id: action_user_id,
+                                listener: listener.to_sym)
     end
   end
 end

spec/requests/admin/users_spec.rb

@@ -298,12 +298,67 @@ RSpec.describe "/admin/member_manager/users", type: :request do
   end
 
   describe "POST /admin/member_manager/users/:id/unpublish_all_articles" do
-    let(:user) { create(:user) }
+    let(:target_user) { create(:user) }
+    let!(:target_articles) { create_list(:article, 3, user: target_user, published: true) }
+    let!(:target_comments) { create_list(:comment, 3, user: target_user) }
+
+    it "creates a corresponding note if note content passed" do
+      text = "The articles were not interesting"
+      expect do
+        post unpublish_all_articles_admin_user_path(target_user.id, note: { content: text })
+      end.to change(Note, :count).by(1)
+      note = target_user.notes.last
+      expect(note.content).to eq(text)
+      expect(note.reason).to eq("unpublish_all_articles")
+      expect(note.author_id).to eq(admin.id)
+    end
+
+    it "doesn't create a note if note content was not passed" do
+      expect do
+        post unpublish_all_articles_admin_user_path(target_user.id, note: { content: "" })
+      end.not_to change(Note, :count)
+    end
 
     it "unpublishes all articles" do
       allow(Moderator::UnpublishAllArticlesWorker).to receive(:perform_async)
-      post unpublish_all_articles_admin_user_path(user.id)
-      expect(Moderator::UnpublishAllArticlesWorker).to have_received(:perform_async).with(user.id)
+      post unpublish_all_articles_admin_user_path(target_user.id)
+      expect(Moderator::UnpublishAllArticlesWorker).to have_received(:perform_async).with(target_user.id, admin.id,
+                                                                                          "moderator")
+    end
+
+    it "unpublishes users comments and posts" do
+      # User's articles are published and comments exist
+      expect(target_articles.map(&:published?)).to match_array([true, true, true])
+      expect(target_comments.map(&:deleted)).to match_array([false, false, false])
+
+      sidekiq_perform_enqueued_jobs(only: Moderator::UnpublishAllArticlesWorker) do
+        post unpublish_all_articles_admin_user_path(target_user.id)
+      end
+
+      # Ensure article's aren't published and comments deleted
+      # (with boolean attribute so they can be reverted if needed)
+      expect(target_articles.map(&:reload).map(&:published?)).to match_array([false, false, false])
+      expect(target_comments.map(&:reload).map(&:deleted)).to match_array([true, true, true])
+    end
+
+    it "creates a log record" do
+      Audit::Subscribe.listen :moderator
+
+      create(:article, user: target_user, published: false)
+      create(:comment, user: target_user, deleted: true)
+
+      sidekiq_perform_enqueued_jobs(only: Moderator::UnpublishAllArticlesWorker) do
+        post unpublish_all_articles_admin_user_path(target_user.id)
+      end
+
+      log = AuditLog.last
+      expect(log.category).to eq(AuditLog::MODERATOR_AUDIT_LOG_CATEGORY)
+      expect(log.data["action"]).to eq("unpublish_all_articles")
+      expect(log.user_id).to eq(admin.id)
+
+      # These ids match the affected articles/comments and not the ones created above
+      expect(log.data["target_article_ids"]).to match_array(target_articles.map(&:id))
+      expect(log.data["target_comment_ids"]).to match_array(target_comments.map(&:id))
     end
   end
 

spec/requests/api/v1/users_spec.rb

@@ -226,11 +226,11 @@ RSpec.describe "Api::V1::Users", type: :request do
         expect(target_articles.map(&:published?)).to match_array([true, true, true])
         expect(target_comments.map(&:deleted)).to match_array([false, false, false])
 
-        put api_user_unpublish_path(id: target_user.id),
-            headers: auth_headers
-        expect(response).to have_http_status(:no_content)
+        sidekiq_perform_enqueued_jobs(only: Moderator::UnpublishAllArticlesWorker) do
+          put api_user_unpublish_path(id: target_user.id), headers: auth_headers
+        end
 
-        sidekiq_perform_enqueued_jobs
+        expect(response).to have_http_status(:no_content)
 
         # Ensure article's aren't published and comments deleted
         # (with boolean attribute so they can be reverted if needed)
@@ -246,8 +246,9 @@ RSpec.describe "Api::V1::Users", type: :request do
         create(:article, user: target_user, published: false)
         create(:comment, user: target_user, deleted: true)
 
-        put api_user_unpublish_path(id: target_user.id),
-            headers: auth_headers
+        sidekiq_perform_enqueued_jobs(only: Moderator::UnpublishAllArticlesWorker) do
+          put api_user_unpublish_path(id: target_user.id), headers: auth_headers
+        end
 
         log = AuditLog.last
         expect(log.category).to eq(AuditLog::ADMIN_API_AUDIT_LOG_CATEGORY)

spec/requests/articles/articles_create_spec.rb

@@ -192,7 +192,6 @@ RSpec.describe "ArticlesCreate", type: :request do
       \npublished_at: #{published_at.strftime('%Y-%m-%d %H:%M %z')}\n---\n\nHey this is the article"
       post "/articles", params: { article: { body_markdown: body_markdown } }
       a = Article.find_by(title: "super-article")
-      # binding.pry
       expect(a.published_at).to be_within(1.minute).of(published_at)
     end
   end

spec/services/moderator/unpublish_all_articles_spec.rb

@@ -0,0 +1,76 @@
+require "rails_helper"
+
+RSpec.describe Moderator::UnpublishAllArticles, type: :service do
+  let!(:user) { create(:user) }
+  let!(:admin) { create(:user, :admin) }
+  let!(:articles) { create_list(:article, 3, user: user) }
+  let!(:comments) { create_list(:comment, 3, user: user, commentable: articles.sample) }
+
+  it "unpublishes all articles" do
+    expect do
+      described_class.call(target_user_id: user.id, action_user_id: admin.id)
+    end.to change { user.articles.published.size }.from(3).to(0)
+  end
+
+  it "unpublishes related comments" do
+    expect do
+      described_class.call(target_user_id: user.id, action_user_id: admin.id)
+    end.to change { user.comments.where(deleted: false).size }.from(3).to(0)
+  end
+
+  it "applies proper frontmatter", :aggregate_failures do
+    described_class.call(target_user_id: user.id, action_user_id: admin.id)
+    articles.each(&:reload)
+    expect(articles.map { |a| a.body_markdown.include?("published: false") }.uniq).to eq([true])
+    expect(articles.map { |a| a.body_markdown.include?("published: true") }.uniq).to eq([false])
+  end
+
+  it "destroys the pre-existing notifications" do
+    allow(Notification).to receive(:remove_all_by_action_without_delay).and_call_original
+    described_class.call(target_user_id: user.id, action_user_id: admin.id)
+    articles.map(&:id).each do |id|
+      attrs = { notifiable_ids: id, notifiable_type: "Article", action: "Published" }
+      expect(Notification).to have_received(:remove_all_by_action_without_delay).with(attrs)
+    end
+  end
+
+  it "destroys the pre-existing context notifications" do
+    articles.each do |article|
+      create(:context_notification, context: article, action: "Published")
+    end
+    expect do
+      described_class.call(target_user_id: user.id, action_user_id: admin.id)
+    end.to change(ContextNotification, :count).by(-3)
+  end
+
+  it "creates audit_log records" do
+    Audit::Subscribe.listen :admin_api
+
+    expect do
+      described_class.call(target_user_id: user.id, action_user_id: admin.id, listener: :admin_api)
+    end.to change(AuditLog, :count).by(1)
+
+    log = AuditLog.last
+    expect(log.category).to eq(AuditLog::ADMIN_API_AUDIT_LOG_CATEGORY)
+    expect(log.slug).to eq("api_user_unpublish")
+    expect(log.data["action"]).to eq("api_user_unpublish")
+    expect(log.user_id).to eq(admin.id)
+
+    expect(log.data["target_article_ids"]).to match_array(articles.map(&:id))
+    expect(log.data["target_comment_ids"]).to match_array(comments.map(&:id))
+  end
+
+  it "creates audit_log records for admin action" do
+    Audit::Subscribe.listen :moderator
+
+    expect do
+      described_class.call(target_user_id: user.id, action_user_id: admin.id, listener: :moderator)
+    end.to change(AuditLog, :count).by(1)
+
+    log = AuditLog.last
+    expect(log.category).to eq(AuditLog::MODERATOR_AUDIT_LOG_CATEGORY)
+    expect(log.slug).to eq("unpublish_all_articles")
+    expect(log.data["action"]).to eq("unpublish_all_articles")
+    expect(log.user_id).to eq(admin.id)
+  end
+end

spec/workers/moderator/unpublish_all_articles_worker_spec.rb

@@ -5,34 +5,36 @@ RSpec.describe Moderator::UnpublishAllArticlesWorker, type: :worker do
 
   context "when unpublishing" do
     let!(:user) { create(:user) }
-    let!(:articles) { create_list(:article, 3, user: user) }
+    let!(:admin) { create(:user, :admin) }
 
-    it "unpublishes all articles" do
-      expect { described_class.new.perform(user.id) }.to change { user.articles.published.size }.from(3).to(0)
+    before {  allow(Moderator::UnpublishAllArticles).to receive(:call) }
+
+    it "calls UnpublishAllArticles" do
+      described_class.new.perform(user.id, admin.id)
+      expect(Moderator::UnpublishAllArticles).to have_received(:call).with(target_user_id: user.id,
+                                                                           action_user_id: admin.id,
+                                                                           listener: :admin_api)
     end
 
-    it "applies proper frontmatter", :aggregate_failures do
-      described_class.new.perform(user.id)
-      expect(Article.last.body_markdown).to include("published: false")
-      expect(Article.last.body_markdown).not_to include("published: true")
+    it "calls UnpublishAllArticles with listener" do
+      described_class.new.perform(user.id, admin.id, "moderator")
+      expect(Moderator::UnpublishAllArticles).to have_received(:call).with(target_user_id: user.id,
+                                                                           action_user_id: admin.id,
+                                                                           listener: :moderator)
     end
 
-    it "destroys the pre-existing notifications" do
-      allow(Notification).to receive(:remove_all_by_action_without_delay).and_call_original
-      described_class.new.perform(user.id)
-      articles.map(&:id).each do |id|
-        attrs = { notifiable_ids: id, notifiable_type: "Article", action: "Published" }
-        expect(Notification).to have_received(:remove_all_by_action_without_delay).with(attrs)
-      end
+    it "calls UnpublishAllArticles with the default listener" do
+      described_class.new.perform(user.id, admin.id, "admin_api")
+      expect(Moderator::UnpublishAllArticles).to have_received(:call).with(target_user_id: user.id,
+                                                                           action_user_id: admin.id,
+                                                                           listener: :admin_api)
     end
 
-    it "destroys the pre-existing context notifications" do
-      articles.each do |article|
-        create(:context_notification, context: article, action: "Published")
-      end
-      expect do
-        described_class.new.perform(user.id)
-      end.to change(ContextNotification, :count).by(-3)
+    it "calls UnpublishAllArticles with the default listener if passed invalid listener" do
+      described_class.new.perform(user.id, admin.id, "another_api")
+      expect(Moderator::UnpublishAllArticles).to have_received(:call).with(target_user_id: user.id,
+                                                                           action_user_id: admin.id,
+                                                                           listener: :admin_api)
     end
   end
 end