From c36a76b9eb2e5e6809956e012028d726e113dd50 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 24 Sep 2024 17:19:55 +0200 Subject: [PATCH] Fix error when accepting appeal for sensitive posts deleted in the meantime (#32037) Co-authored-by: David Roetzel --- app/services/approve_appeal_service.rb | 2 +- spec/services/approve_appeal_service_spec.rb | 60 ++++++++++++++------ 2 files changed, 45 insertions(+), 17 deletions(-) diff --git a/app/services/approve_appeal_service.rb b/app/services/approve_appeal_service.rb index b8a522b2a1..3f8d1e2a3b 100644 --- a/app/services/approve_appeal_service.rb +++ b/app/services/approve_appeal_service.rb @@ -53,7 +53,7 @@ class ApproveAppealService < BaseService def undo_mark_statuses_as_sensitive! representative_account = Account.representative - @strike.statuses.includes(:media_attachments).find_each do |status| + @strike.statuses.kept.includes(:media_attachments).reorder(nil).find_each do |status| UpdateStatusService.new.call(status, representative_account.id, sensitive: false) if status.with_media? end end diff --git a/spec/services/approve_appeal_service_spec.rb b/spec/services/approve_appeal_service_spec.rb index 5707c5d7f4..52e073df0c 100644 --- a/spec/services/approve_appeal_service_spec.rb +++ b/spec/services/approve_appeal_service_spec.rb @@ -4,27 +4,55 @@ require 'rails_helper' RSpec.describe ApproveAppealService do describe '#call' do - context 'with an existing appeal' do - let(:appeal) { Fabricate(:appeal) } - let(:account) { Fabricate(:account) } + let(:appeal) { Fabricate(:appeal) } + let(:account) { Fabricate(:account) } - it 'processes the appeal approval' do - expect { subject.call(appeal, account) } - .to mark_overruled - .and record_approver + it 'processes the appeal approval' do + expect { subject.call(appeal, account) } + .to mark_overruled + .and record_approver + end + + context 'with an appeal about then-deleted posts marked as sensitive by moderators' do + let(:target_account) { Fabricate(:account) } + let(:appeal) { Fabricate(:appeal, strike: strike, account: target_account) } + let(:deleted_media) { Fabricate(:media_attachment, type: :video, status: Fabricate(:status, account: target_account), account: target_account) } + let(:kept_media) { Fabricate(:media_attachment, type: :video, status: Fabricate(:status, account: target_account), account: target_account) } + let(:strike) { Fabricate(:account_warning, target_account: target_account, action: :mark_statuses_as_sensitive, status_ids: [deleted_media.status.id, kept_media.status.id]) } + + before do + target_account.unsuspend! + deleted_media.status.discard! end - def mark_overruled - change(appeal.strike, :overruled_at) - .from(nil) - .to(be > 1.minute.ago) - end + it 'approves the appeal, marks the statuses as not sensitive and notifies target account about the approval', :inline_jobs do + emails = capture_emails { subject.call(appeal, account) } - def record_approver - change(appeal, :approved_by_account) - .from(nil) - .to(account) + expect(appeal.reload).to be_approved + expect(strike.reload).to be_overruled + + expect(kept_media.status.reload).to_not be_sensitive + + expect(emails.size) + .to eq(1) + expect(emails.first) + .to have_attributes( + to: contain_exactly(target_account.user.email), + subject: eq(I18n.t('user_mailer.appeal_approved.subject', date: I18n.l(appeal.created_at))) + ) end end + + def mark_overruled + change(appeal.strike, :overruled_at) + .from(nil) + .to(be > 1.minute.ago) + end + + def record_approver + change(appeal, :approved_by_account) + .from(nil) + .to(account) + end end end