From 64952cef52ca026199b6e35d8e26f46dcc7a5357 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 13 Jan 2023 10:46:52 +0100 Subject: [PATCH] Fix crash when marking statuses as sensitive while some statuses are deleted (#22134) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Do not offer to mark statuses as sensitive if there is no undeleted status with media attachments * Fix crash when marking statuses as sensitive while some statuses are deleted Fixes #21910 * Fix multiple strikes being created for a single report when selecting “Mark as sensitive” * Add tests --- app/models/admin/status_batch_action.rb | 16 +++---- app/views/admin/reports/_actions.html.haml | 2 +- .../admin/reports/actions_controller_spec.rb | 42 +++++++++++++++++++ 3 files changed, 51 insertions(+), 9 deletions(-) create mode 100644 spec/controllers/admin/reports/actions_controller_spec.rb diff --git a/app/models/admin/status_batch_action.rb b/app/models/admin/status_batch_action.rb index 0f019b854d..39cd7d0eb8 100644 --- a/app/models/admin/status_batch_action.rb +++ b/app/models/admin/status_batch_action.rb @@ -73,7 +73,7 @@ class Admin::StatusBatchAction # Can't use a transaction here because UpdateStatusService queues # Sidekiq jobs statuses.includes(:media_attachments, :preview_cards).find_each do |status| - next unless status.with_media? || status.with_preview_card? + next if status.discarded? || !(status.with_media? || status.with_preview_card?) authorize([:admin, status], :update?) @@ -89,15 +89,15 @@ class Admin::StatusBatchAction report.resolve!(current_account) log_action(:resolve, report) end - - @warning = target_account.strikes.create!( - action: :mark_statuses_as_sensitive, - account: current_account, - report: report, - status_ids: status_ids - ) end + @warning = target_account.strikes.create!( + action: :mark_statuses_as_sensitive, + account: current_account, + report: report, + status_ids: status_ids + ) + UserMailer.warning(target_account.user, @warning).deliver_later! if warnable? end diff --git a/app/views/admin/reports/_actions.html.haml b/app/views/admin/reports/_actions.html.haml index 404d53a773..486eb486c7 100644 --- a/app/views/admin/reports/_actions.html.haml +++ b/app/views/admin/reports/_actions.html.haml @@ -5,7 +5,7 @@ = link_to t('admin.reports.mark_as_resolved'), resolve_admin_report_path(@report), method: :post, class: 'button' .report-actions__item__description = t('admin.reports.actions.resolve_description_html') - - if @statuses.any? { |status| status.with_media? || status.with_preview_card? } + - if @statuses.any? { |status| (status.with_media? || status.with_preview_card?) && !status.discarded? } .report-actions__item .report-actions__item__button = button_tag t('admin.reports.mark_as_sensitive'), name: :mark_as_sensitive, class: 'button' diff --git a/spec/controllers/admin/reports/actions_controller_spec.rb b/spec/controllers/admin/reports/actions_controller_spec.rb new file mode 100644 index 0000000000..6609798dc0 --- /dev/null +++ b/spec/controllers/admin/reports/actions_controller_spec.rb @@ -0,0 +1,42 @@ +require 'rails_helper' + +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] } + + before do + post :create, params: { report_id: report.id, action => '' } + end + + context 'when 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 + end + end + end +end