Split status batch and moderation actions (#37970)

app/controllers/admin/reports/actions_controller.rb

@@ -13,7 +13,7 @@ class Admin::Reports::ActionsController < Admin::BaseController
 
     case action_from_button
     when 'delete', 'mark_as_sensitive'
-      Admin::StatusBatchAction.new(status_batch_action_params).save!
+      Admin::ModerationAction.new(moderation_action_params).save!
     when 'silence', 'suspend'
       Admin::AccountAction.new(account_action_params).save!
     else
@@ -25,9 +25,8 @@ class Admin::Reports::ActionsController < Admin::BaseController
 
   private
 
-  def status_batch_action_params
+  def moderation_action_params
     shared_params
-      .merge(status_ids: @report.status_ids)
   end
 
   def account_action_params

app/controllers/admin/statuses_controller.rb

@@ -78,8 +78,6 @@ module Admin
         'report'
       elsif params[:remove_from_report]
         'remove_from_report'
-      elsif params[:delete]
-        'delete'
       end
     end
   end

app/models/admin/account_action.rb

@@ -66,20 +66,6 @@ class Admin::AccountAction < Admin::BaseAction
     end
   end
 
-  def process_strike!
-    @warning = target_account.strikes.create!(
-      account: current_account,
-      report: report,
-      action: type,
-      text: text_for_warning,
-      status_ids: status_ids
-    )
-
-    # A log entry is only interesting if the warning contains
-    # custom text from someone. Otherwise it's just noise.
-    log_action(:create, @warning) if @warning.text.present? && type == 'none'
-  end
-
   def process_reports!
     # If we're doing "mark as resolved" on a single report,
     # then we want to keep other reports open in case they
@@ -131,17 +117,6 @@ class Admin::AccountAction < Admin::BaseAction
     queue_suspension_worker! if type == 'suspend'
   end
 
-  def process_notification!
-    return unless warnable?
-
-    UserMailer.warning(target_account.user, warning).deliver_later!
-    LocalNotificationWorker.perform_async(target_account.id, warning.id, 'AccountWarning', 'moderation_warning')
-  end
-
-  def warnable?
-    send_email_notification? && target_account.local?
-  end
-
   def status_ids
     report.status_ids if with_report? && include_statuses?
   end

app/models/admin/base_action.rb

@@ -39,4 +39,27 @@ class Admin::BaseAction
   def with_report?
     !report.nil?
   end
+
+  private
+
+  def process_strike!(action = type)
+    @warning = target_account.strikes.create!(
+      account: current_account,
+      report: report,
+      action:,
+      text: text_for_warning,
+      status_ids: status_ids
+    )
+  end
+
+  def process_notification!
+    return unless warnable?
+
+    UserMailer.warning(target_account.user, warning).deliver_later!
+    LocalNotificationWorker.perform_async(target_account.id, warning.id, 'AccountWarning', 'moderation_warning')
+  end
+
+  def warnable?
+    send_email_notification? && target_account.local?
+  end
 end

app/models/admin/moderation_action.rb

@@ -0,0 +1,84 @@
+# frozen_string_literal: true
+
+class Admin::ModerationAction < Admin::BaseAction
+  TYPES = %w(
+    delete
+    mark_as_sensitive
+  ).freeze
+
+  validates :report_id, presence: true
+
+  private
+
+  def status_ids
+    report.status_ids
+  end
+
+  def statuses
+    @statuses ||= Status.with_discarded.where(id: status_ids).reorder(nil)
+  end
+
+  def process_action!
+    case type
+    when 'delete'
+      handle_delete!
+    when 'mark_as_sensitive'
+      handle_mark_as_sensitive!
+    end
+  end
+
+  def handle_delete!
+    statuses.each { |status| authorize([:admin, status], :destroy?) }
+
+    ApplicationRecord.transaction do
+      statuses.each do |status|
+        status.discard_with_reblogs
+        log_action(:destroy, status)
+      end
+
+      report.resolve!(current_account)
+      log_action(:resolve, report)
+
+      process_strike!(:delete_statuses)
+
+      statuses.each { |status| Tombstone.find_or_create_by(uri: status.uri, account: status.account, by_moderator: true) } unless target_account.local?
+    end
+
+    process_notification!
+
+    RemovalWorker.push_bulk(status_ids) { |status_id| [status_id, { 'preserve' => target_account.local?, 'immediate' => !target_account.local? }] }
+  end
+
+  def handle_mark_as_sensitive!
+    representative_account = Account.representative
+
+    # Can't use a transaction here because UpdateStatusService queues
+    # Sidekiq jobs
+    statuses.includes(:media_attachments, preview_cards_status: :preview_card).find_each do |status|
+      next if status.discarded? || !(status.with_media? || status.with_preview_card?)
+
+      authorize([:admin, status], :update?)
+
+      if target_account.local?
+        UpdateStatusService.new.call(status, representative_account.id, sensitive: true)
+      else
+        status.update(sensitive: true)
+      end
+
+      log_action(:update, status)
+
+      report.resolve!(current_account)
+      log_action(:resolve, report)
+    end
+
+    process_strike!(:mark_statuses_as_sensitive)
+
+    process_notification!
+  end
+
+  def target_account
+    report.target_account
+  end
+
+  def text_for_warning = text
+end

app/models/admin/status_batch_action.rb

@@ -2,8 +2,6 @@
 
 class Admin::StatusBatchAction < Admin::BaseAction
   TYPES = %w(
-    delete
-    mark_as_sensitive
     report
     remove_from_report
   ).freeze
@@ -20,10 +18,6 @@ class Admin::StatusBatchAction < Admin::BaseAction
     return if status_ids.empty?
 
     case type
-    when 'delete'
-      handle_delete!
-    when 'mark_as_sensitive'
-      handle_mark_as_sensitive!
     when 'report'
       handle_report!
     when 'remove_from_report'
@@ -31,71 +25,6 @@ class Admin::StatusBatchAction < Admin::BaseAction
     end
   end
 
-  def handle_delete!
-    statuses.each { |status| authorize([:admin, status], :destroy?) }
-
-    ApplicationRecord.transaction do
-      statuses.each do |status|
-        status.discard_with_reblogs
-        log_action(:destroy, status)
-      end
-
-      if with_report?
-        report.resolve!(current_account)
-        log_action(:resolve, report)
-      end
-
-      @warning = target_account.strikes.create!(
-        action: :delete_statuses,
-        account: current_account,
-        report: report,
-        status_ids: status_ids,
-        text: text
-      )
-
-      statuses.each { |status| Tombstone.find_or_create_by(uri: status.uri, account: status.account, by_moderator: true) } unless target_account.local?
-    end
-
-    process_notification!
-
-    RemovalWorker.push_bulk(status_ids) { |status_id| [status_id, { 'preserve' => target_account.local?, 'immediate' => !target_account.local? }] }
-  end
-
-  def handle_mark_as_sensitive!
-    representative_account = Account.representative
-
-    # Can't use a transaction here because UpdateStatusService queues
-    # Sidekiq jobs
-    statuses.includes(:media_attachments, preview_cards_status: :preview_card).find_each do |status|
-      next if status.discarded? || !(status.with_media? || status.with_preview_card?)
-
-      authorize([:admin, status], :update?)
-
-      if target_account.local?
-        UpdateStatusService.new.call(status, representative_account.id, sensitive: true)
-      else
-        status.update(sensitive: true)
-      end
-
-      log_action(:update, status)
-
-      if with_report?
-        report.resolve!(current_account)
-        log_action(:resolve, report)
-      end
-    end
-
-    @warning = target_account.strikes.create!(
-      action: :mark_statuses_as_sensitive,
-      account: current_account,
-      report: report,
-      status_ids: status_ids,
-      text: text
-    )
-
-    process_notification!
-  end
-
   def handle_report!
     @report = Report.new(report_params) unless with_report?
     @report.status_ids = (@report.status_ids + allowed_status_ids).uniq
@@ -111,17 +40,6 @@ class Admin::StatusBatchAction < Admin::BaseAction
     report.save!
   end
 
-  def process_notification!
-    return unless warnable?
-
-    UserMailer.warning(target_account.user, @warning).deliver_later!
-    LocalNotificationWorker.perform_async(target_account.id, @warning.id, 'AccountWarning', 'moderation_warning')
-  end
-
-  def warnable?
-    send_email_notification && target_account.local?
-  end
-
   def target_account
     @target_account ||= statuses.first.account
   end

spec/models/admin/moderation_action_spec.rb

@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Admin::ModerationAction do
+  subject do
+    described_class.new(
+      current_account:,
+      type:,
+      report_id:,
+      text:
+    )
+  end
+
+  let(:current_account) { Fabricate(:admin_user).account }
+  let(:target_account) { Fabricate(:account) }
+  let(:statuses) { Fabricate.times(2, :status, account: target_account) }
+  let(:status_ids) { statuses.map(&:id) }
+  let(:report) { Fabricate(:report, target_account:, status_ids:) }
+  let(:report_id) { report.id }
+  let(:text) { 'test' }
+
+  describe '#save!' do
+    context 'when `type` is `delete`' do
+      let(:type) { 'delete' }
+
+      it 'discards the statuses' do
+        subject.save!
+
+        statuses.each do |status|
+          expect(status.reload).to be_discarded
+        end
+        expect(report.reload).to be_action_taken
+      end
+    end
+
+    context 'when `type` is `mark_as_sensitive`' do
+      let(:type) { 'mark_as_sensitive' }
+
+      before do
+        preview_card = Fabricate(:preview_card)
+        statuses.each do |status|
+          PreviewCardsStatus.create!(status:, preview_card:)
+        end
+      end
+
+      it 'marks the statuses as sensitive' do
+        subject.save!
+
+        statuses.each do |status|
+          expect(status.reload).to be_sensitive
+        end
+        expect(report.reload).to be_action_taken
+      end
+    end
+  end
+end

spec/models/admin/status_batch_action_spec.rb

@@ -22,39 +22,6 @@ RSpec.describe Admin::StatusBatchAction do
   let(:text) { 'test' }
 
   describe '#save!' do
-    context 'when `type` is `delete`' do
-      let(:type) { 'delete' }
-
-      it 'discards the statuses' do
-        subject.save!
-
-        statuses.each do |status|
-          expect(status.reload).to be_discarded
-        end
-        expect(report.reload).to be_action_taken
-      end
-    end
-
-    context 'when `type` is `mark_as_sensitive`' do
-      let(:type) { 'mark_as_sensitive' }
-
-      before do
-        preview_card = Fabricate(:preview_card)
-        statuses.each do |status|
-          PreviewCardsStatus.create!(status:, preview_card:)
-        end
-      end
-
-      it 'marks the statuses as sensitive' do
-        subject.save!
-
-        statuses.each do |status|
-          expect(status.reload).to be_sensitive
-        end
-        expect(report.reload).to be_action_taken
-      end
-    end
-
     context 'when `type` is `report`' do
       let(:report_id) { nil }
       let(:type) { 'report' }