about summary refs log tree commit diff
diff options
context:
space:
mode:
authorClaire <claire.github-309c@sitedethib.com>2023-01-18 16:40:09 +0100
committerGitHub <noreply@github.com>2023-01-18 16:40:09 +0100
commit343e1fe8e9ce94ea4f86d3a3df71f22f5fb2319d (patch)
tree551b4b1abb37bb40431699d3837e3ac14da86651
parent4b92e59f4fea4486ee6e5af7421e7945d5f7f998 (diff)
Add confirmation screen when handling reports (#22375)
* Add confirmation screen on moderation actions

* Add flash notice when a report has been processed

* Refactor tests

* Add tests
-rw-r--r--app/controllers/admin/account_actions_controller.rb2
-rw-r--r--app/controllers/admin/reports/actions_controller.rb17
-rw-r--r--app/models/admin/status_batch_action.rb9
-rw-r--r--app/views/admin/reports/_actions.html.haml2
-rw-r--r--app/views/admin/reports/actions/preview.html.haml78
-rw-r--r--config/i18n-tasks.yml2
-rw-r--r--config/locales/en.yml21
-rw-r--r--config/routes.rb6
-rw-r--r--spec/controllers/admin/reports/actions_controller_spec.rb128
9 files changed, 238 insertions, 27 deletions
diff --git a/app/controllers/admin/account_actions_controller.rb b/app/controllers/admin/account_actions_controller.rb
index 3f2e28b6a..e89404b60 100644
--- a/app/controllers/admin/account_actions_controller.rb
+++ b/app/controllers/admin/account_actions_controller.rb
@@ -21,7 +21,7 @@ module Admin
       account_action.save!
 
       if account_action.with_report?
-        redirect_to admin_reports_path
+        redirect_to admin_reports_path, notice: I18n.t('admin.reports.processed_msg', id: params[:report_id])
       else
         redirect_to admin_account_path(@account.id)
       end
diff --git a/app/controllers/admin/reports/actions_controller.rb b/app/controllers/admin/reports/actions_controller.rb
index 5cb5c744f..554f7906f 100644
--- a/app/controllers/admin/reports/actions_controller.rb
+++ b/app/controllers/admin/reports/actions_controller.rb
@@ -3,6 +3,11 @@
 class Admin::Reports::ActionsController < Admin::BaseController
   before_action :set_report
 
+  def preview
+    authorize @report, :show?
+    @moderation_action = action_from_button
+  end
+
   def create
     authorize @report, :show?
 
@@ -13,7 +18,8 @@ class Admin::Reports::ActionsController < Admin::BaseController
         status_ids: @report.status_ids,
         current_account: current_account,
         report_id: @report.id,
-        send_email_notification: !@report.spam?
+        send_email_notification: !@report.spam?,
+        text: params[:text]
       )
 
       status_batch_action.save!
@@ -23,13 +29,16 @@ class Admin::Reports::ActionsController < Admin::BaseController
         report_id: @report.id,
         target_account: @report.target_account,
         current_account: current_account,
-        send_email_notification: !@report.spam?
+        send_email_notification: !@report.spam?,
+        text: params[:text]
       )
 
       account_action.save!
+    else
+      return redirect_to admin_report_path(@report), alert: I18n.t('admin.reports.unknown_action_msg', action: action_from_button)
     end
 
-    redirect_to admin_reports_path
+    redirect_to admin_reports_path, notice: I18n.t('admin.reports.processed_msg', id: @report.id)
   end
 
   private
@@ -47,6 +56,8 @@ class Admin::Reports::ActionsController < Admin::BaseController
       'silence'
     elsif params[:suspend]
       'suspend'
+    elsif params[:moderation_action]
+      params[:moderation_action]
     end
   end
 end
diff --git a/app/models/admin/status_batch_action.rb b/app/models/admin/status_batch_action.rb
index 39cd7d0eb..b8bdec722 100644
--- a/app/models/admin/status_batch_action.rb
+++ b/app/models/admin/status_batch_action.rb
@@ -6,7 +6,8 @@ class Admin::StatusBatchAction
   include Authorization
 
   attr_accessor :current_account, :type,
-                :status_ids, :report_id
+                :status_ids, :report_id,
+                :text
 
   attr_reader :send_email_notification
 
@@ -57,7 +58,8 @@ class Admin::StatusBatchAction
         action: :delete_statuses,
         account: current_account,
         report: report,
-        status_ids: status_ids
+        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?
@@ -95,7 +97,8 @@ class Admin::StatusBatchAction
       action: :mark_statuses_as_sensitive,
       account: current_account,
       report: report,
-      status_ids: status_ids
+      status_ids: status_ids,
+      text: text
     )
 
     UserMailer.warning(target_account.user, @warning).deliver_later! if warnable?
diff --git a/app/views/admin/reports/_actions.html.haml b/app/views/admin/reports/_actions.html.haml
index 486eb486c..aad441625 100644
--- a/app/views/admin/reports/_actions.html.haml
+++ b/app/views/admin/reports/_actions.html.haml
@@ -1,4 +1,4 @@
-= form_tag admin_report_actions_path(@report), method: :post do
+= form_tag preview_admin_report_actions_path(@report), method: :post do
   .report-actions
     .report-actions__item
       .report-actions__item__button
diff --git a/app/views/admin/reports/actions/preview.html.haml b/app/views/admin/reports/actions/preview.html.haml
new file mode 100644
index 000000000..58745319c
--- /dev/null
+++ b/app/views/admin/reports/actions/preview.html.haml
@@ -0,0 +1,78 @@
+- target_acct = @report.target_account.acct
+- warning_action = { 'delete' => 'delete_statuses', 'mark_as_sensitive' => 'mark_statuses_as_sensitive' }.fetch(@moderation_action, @moderation_action)
+
+- content_for :page_title do
+  = t('admin.reports.confirm_action', acct: target_acct)
+
+= form_tag admin_report_actions_path(@report), class: 'simple_form', method: :post do
+  = hidden_field_tag :moderation_action, @moderation_action
+
+  %p.hint= t("admin.reports.summary.action_preambles.#{@moderation_action}_html", acct: target_acct)
+  %ul.hint
+    %li.warning-hint= t("admin.reports.summary.actions.#{@moderation_action}_html", acct: target_acct)
+    - if @moderation_action == 'suspend'
+      %li.warning-hint= t('admin.reports.summary.delete_data_html', acct: target_acct)
+    - if %w(silence suspend).include?(@moderation_action)
+      %li.warning-hint= t('admin.reports.summary.close_reports_html', acct: target_acct)
+    - else
+      %li= t('admin.reports.summary.close_report', id: @report.id)
+    %li= t('admin.reports.summary.record_strike_html', acct: target_acct)
+    - if @report.target_account.local? && !@report.spam?
+      %li= t('admin.reports.summary.send_email_html', acct: target_acct)
+
+  %hr.spacer/
+
+  - if @report.target_account.local?
+    %p.hint= t('admin.reports.summary.preview_preamble_html', acct: target_acct)
+
+    .strike-card
+      - unless warning_action == 'none'
+        %p= t "user_mailer.warning.explanation.#{warning_action}", instance: Rails.configuration.x.local_domain
+
+      .fields-group
+        = text_area_tag :text, nil, placeholder: t('admin.reports.summary.warning_placeholder')
+
+      - if !@report.other?
+        %p
+          %strong= t('user_mailer.warning.reason')
+          = t("user_mailer.warning.categories.#{@report.category}")
+
+        - if @report.violation? && @report.rule_ids.present?
+          %ul.strike-card__rules
+            - @report.rules.each do |rule|
+              %li
+                %span.strike-card__rules__text= rule.text
+
+      - if @report.status_ids.present? && !@report.status_ids.empty?
+        %p
+          %strong= t('user_mailer.warning.statuses')
+
+        .strike-card__statuses-list
+          - status_map = @report.statuses.includes(:application, :media_attachments).index_by(&:id)
+
+          - @report.status_ids.each do |status_id|
+            .strike-card__statuses-list__item
+              - if (status = status_map[status_id.to_i])
+                .one-liner
+                  = link_to short_account_status_url(@report.target_account, status_id), class: 'emojify' do
+                    = one_line_preview(status)
+
+                    - status.ordered_media_attachments.each do |media_attachment|
+                      %abbr{ title: media_attachment.description }
+                        = fa_icon 'link'
+                        = media_attachment.file_file_name
+                .strike-card__statuses-list__item__meta
+                  %time.formatted{ datetime: status.created_at.iso8601, title: l(status.created_at) }= l(status.created_at)
+                  - unless status.application.nil?
+                    ·
+                    = status.application.name
+              - else
+                .one-liner= t('disputes.strikes.status', id: status_id)
+                .strike-card__statuses-list__item__meta
+                  = t('disputes.strikes.status_removed')
+
+    %hr.spacer/
+
+  .actions
+    = link_to t('admin.reports.cancel'), admin_report_path(@report), class: 'button button-tertiary'
+    = button_tag t('admin.reports.confirm'), name: :confirm, class: 'button', type: :submit
diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml
index c1da42bd8..46dd3124b 100644
--- a/config/i18n-tasks.yml
+++ b/config/i18n-tasks.yml
@@ -58,6 +58,8 @@ ignore_unused:
   - 'errors.429'
   - 'admin.accounts.roles.*'
   - 'admin.action_logs.actions.*'
+  - 'admin.reports.summary.action_preambles.*'
+  - 'admin.reports.summary.actions.*'
   - 'admin_mailer.new_appeal.actions.*'
   - 'statuses.attached.*'
   - 'move_handler.carry_{mutes,blocks}_over_text'
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 4143aab04..3de2f2772 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -590,6 +590,7 @@ en:
       comment:
         none: None
       comment_description_html: 'To provide more information, %{name} wrote:'
+      confirm_action: Confirm moderation action against @%{acct}
       created_at: Reported
       delete_and_resolve: Delete posts
       forwarded: Forwarded
@@ -606,6 +607,7 @@ en:
         placeholder: Describe what actions have been taken, or any other related updates...
         title: Notes
       notes_description_html: View and leave notes to other moderators and your future self
+      processed_msg: 'Report #%{id} successfully processed'
       quick_actions_description_html: 'Take a quick action or scroll down to see reported content:'
       remote_user_placeholder: the remote user from %{instance}
       reopen: Reopen report
@@ -618,9 +620,28 @@ en:
       status: Status
       statuses: Reported content
       statuses_description_html: Offending content will be cited in communication with the reported account
+      summary:
+        action_preambles:
+          delete_html: 'You are about to <strong>remove</strong> some of <strong>@%{acct}</strong>''s posts. This will:'
+          mark_as_sensitive_html: 'You are about to <strong>mark</strong> some of <strong>@%{acct}</strong>''s posts as <strong>sensitive</strong>. This will:'
+          silence_html: 'You are about to <strong>limit</strong> <strong>@%{acct}</strong>''s account. This will:'
+          suspend_html: 'You are about to <strong>suspend</strong> <strong>@%{acct}</strong>''s account. This will:'
+        actions:
+          delete_html: Remove the offending posts
+          mark_as_sensitive_html: Mark the offending posts' media as sensitive
+          silence_html: Severely limit <strong>@%{acct}</strong>'s reach by making their profile and contents only visible to people already following them or manually looking it profile up
+          suspend_html: Suspend <strong>@%{acct}</strong>, making their profile and contents inaccessible and impossible to interact with
+        close_report: 'Mark report #%{id} as resolved'
+        close_reports_html: Mark <strong>all</strong> reports against <strong>@%{acct}</strong> as resolved
+        delete_data_html: Delete <strong>@%{acct}</strong>'s profile and contents 30 days from now unless they get unsuspended in the meantime
+        preview_preamble_html: "<strong>@%{acct}</strong> will receive a warning with the following contents:"
+        record_strike_html: Record a strike against <strong>@%{acct}</strong> to help you escalate on future violations from this account
+        send_email_html: Send <strong>@%{acct}</strong> a warning e-mail
+        warning_placeholder: Optional additional reasoning for the moderation action.
       target_origin: Origin of reported account
       title: Reports
       unassign: Unassign
+      unknown_action_msg: 'Unknown action: %{action}'
       unresolved: Unresolved
       updated_at: Updated
       view_profile: View profile
diff --git a/config/routes.rb b/config/routes.rb
index 98e19667c..0bee2f639 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -310,7 +310,11 @@ Rails.application.routes.draw do
     end
 
     resources :reports, only: [:index, :show] do
-      resources :actions, only: [:create], controller: 'reports/actions'
+      resources :actions, only: [:create], controller: 'reports/actions' do
+        collection do
+          post :preview
+        end
+      end
 
       member do
         post :assign_to_self
diff --git a/spec/controllers/admin/reports/actions_controller_spec.rb b/spec/controllers/admin/reports/actions_controller_spec.rb
index 6609798dc..9890ac9ce 100644
--- a/spec/controllers/admin/reports/actions_controller_spec.rb
+++ b/spec/controllers/admin/reports/actions_controller_spec.rb
@@ -4,39 +4,131 @@ describe Admin::Reports::ActionsController do
   render_views
 
   let(:user) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')) }
-  let(:account) { Fabricate(:account) }
-  let!(:status) { Fabricate(:status, account: account) }
-  let(:media_attached_status) { Fabricate(:status, account: account) }
-  let!(:media_attachment) { Fabricate(:media_attachment, account: account, status: media_attached_status) }
-  let(:media_attached_deleted_status) { Fabricate(:status, account: account, deleted_at: 1.day.ago) }
-  let!(:media_attachment2) { Fabricate(:media_attachment, account: account, status: media_attached_deleted_status) }
-  let(:last_media_attached_status) { Fabricate(:status, account: account) }
-  let!(:last_media_attachment) { Fabricate(:media_attachment, account: account, status: last_media_attached_status) }
-  let!(:last_status) { Fabricate(:status, account: account) }
 
   before do
     sign_in user, scope: :user
   end
 
-  describe 'POST #create' do
-    let(:report) { Fabricate(:report, status_ids: status_ids, account: user.account, target_account: account) }
-    let(:status_ids) { [media_attached_status.id, media_attached_deleted_status.id] }
+  describe 'POST #preview' do
+    let(:report) { Fabricate(:report) }
 
     before do
-      post :create, params: { report_id: report.id, action => '' }
+      post :preview, params: { report_id: report.id, action => '' }
+    end
+
+    context 'when the action is "suspend"' do
+      let(:action) { 'suspend' }
+
+      it 'returns http success' do
+        expect(response).to have_http_status(200)
+      end
     end
 
-    context 'when action is mark_as_sensitive' do
+    context 'when the action is "silence"' do
+      let(:action) { 'silence' }
 
+      it 'returns http success' do
+        expect(response).to have_http_status(200)
+      end
+    end
+
+    context 'when the action is "delete"' do
+      let(:action) { 'delete' }
+
+      it 'returns http success' do
+        expect(response).to have_http_status(200)
+      end
+    end
+
+    context 'when the action is "mark_as_sensitive"' do
       let(:action) { 'mark_as_sensitive' }
 
-      it 'resolves the report' do
-        expect(report.reload.action_taken_at).to_not be_nil
+      it 'returns http success' do
+        expect(response).to have_http_status(200)
+      end
+    end
+  end
+
+  describe 'POST #create' do
+    let(:target_account) { Fabricate(:account) }
+    let(:statuses)       { [Fabricate(:status, account: target_account), Fabricate(:status, account: target_account)] }
+    let!(:media)         { Fabricate(:media_attachment, account: target_account, status: statuses[0]) }
+    let(:report)         { Fabricate(:report, target_account: target_account, status_ids: statuses.map(&:id)) }
+    let(:text)           { 'hello' }
+
+    shared_examples 'common behavior' do
+      it 'closes the report' do
+        expect { subject }.to change { report.reload.action_taken? }.from(false).to(true)
       end
 
-      it 'marks the non-deleted as sensitive' do
-        expect(media_attached_status.reload.sensitive).to eq true
+      it 'creates a strike with the expected text' do
+        expect { subject }.to change { report.target_account.strikes.count }.by(1)
+        expect(report.target_account.strikes.last.text).to eq text
       end
+
+      it 'redirects' do
+        subject
+        expect(response).to redirect_to(admin_reports_path)
+      end
+    end
+
+    shared_examples 'all action types' do
+      context 'when the action is "suspend"' do
+        let(:action) { 'suspend' }
+
+        it_behaves_like 'common behavior'
+
+        it 'suspends the target account' do
+          expect { subject }.to change { report.target_account.reload.suspended? }.from(false).to(true)
+        end
+      end
+
+      context 'when the action is "silence"' do
+        let(:action) { 'silence' }
+
+        it_behaves_like 'common behavior'
+
+        it 'suspends the target account' do
+          expect { subject }.to change { report.target_account.reload.silenced? }.from(false).to(true)
+        end
+      end
+
+      context 'when the action is "delete"' do
+        let(:action) { 'delete' }
+
+        it_behaves_like 'common behavior'
+      end
+
+      context 'when the action is "mark_as_sensitive"' do
+        let(:action) { 'mark_as_sensitive' }
+        let(:statuses) { [media_attached_status, media_attached_deleted_status] }
+
+        let!(:status) { Fabricate(:status, account: target_account) }
+        let(:media_attached_status) { Fabricate(:status, account: target_account) }
+        let!(:media_attachment) { Fabricate(:media_attachment, account: target_account, status: media_attached_status) }
+        let(:media_attached_deleted_status) { Fabricate(:status, account: target_account, deleted_at: 1.day.ago) }
+        let!(:media_attachment2) { Fabricate(:media_attachment, account: target_account, status: media_attached_deleted_status) }
+        let(:last_media_attached_status) { Fabricate(:status, account: target_account) }
+        let!(:last_media_attachment) { Fabricate(:media_attachment, account: target_account, status: last_media_attached_status) }
+        let!(:last_status) { Fabricate(:status, account: target_account) }
+
+        it_behaves_like 'common behavior'
+
+        it 'marks the non-deleted as sensitive' do
+          subject
+          expect(media_attached_status.reload.sensitive).to eq true
+        end
+      end
+    end
+
+    context 'action as submit button' do
+      subject { post :create, params: { report_id: report.id, text: text, action => '' } }
+      it_behaves_like 'all action types'
+    end
+
+    context 'action as submit button' do
+      subject { post :create, params: { report_id: report.id, text: text, moderation_action: action } }
+      it_behaves_like 'all action types'
     end
   end
 end