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
This commit is contained in:
		| @@ -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 | ||||
|   | ||||
| @@ -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 | ||||
|   | ||||
| @@ -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? | ||||
|   | ||||
| @@ -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 | ||||
|   | ||||
							
								
								
									
										78
									
								
								app/views/admin/reports/actions/preview.html.haml
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										78
									
								
								app/views/admin/reports/actions/preview.html.haml
									
									
									
									
									
										Normal file
									
								
							| @@ -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 | ||||
| @@ -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' | ||||
|   | ||||
| @@ -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 | ||||
|   | ||||
| @@ -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 | ||||
|   | ||||
| @@ -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 action is mark_as_sensitive' do | ||||
|     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 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 | ||||
|       end | ||||
|  | ||||
|       it 'marks the non-deleted as sensitive' do | ||||
|         expect(media_attached_status.reload.sensitive).to eq true | ||||
|       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 '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 | ||||
|   | ||||
		Reference in New Issue
	
	Block a user