Skip to content
This repository has been archived by the owner on Feb 24, 2025. It is now read-only.

Commit

Permalink
Merge pull request #54 from MozillaSocial/SOCIALPLAT-766-immediately-…
Browse files Browse the repository at this point in the history
…remove-public-copy-of-media

feat(trust & safety): [SOCIALPLAT-766] Immediately remove public copy of media when status is deleted

- Set up a new MediaAttachmentDeletionWorker to delete media.
- Add unit tests for MediaAttachmentDeletionWorker.
- Use the new worker in the two spots it's needed in status controllers.
- Update unit tests for status controllers to check for media deletion. 

JIRA reference: https://mozilla-hub.atlassian.net/browse/SOCTRUST-31
nina-py authored Dec 5, 2023
2 parents 3e5a2d9 + 09bfd22 commit d0e1834
Showing 5 changed files with 54 additions and 1 deletion.
7 changes: 7 additions & 0 deletions app/controllers/api/v1/admin/statuses_controller.rb
Original file line number Diff line number Diff line change
@@ -24,7 +24,14 @@ def destroy
@status.discard_with_reblogs
log_action :destroy, @status
Tombstone.find_or_create_by(uri: @status.uri, account: @status.account, by_moderator: true)

if @status.with_media?
# Immediately remove public copy of media instead of waiting for
# the vacuum_orphaned_records job to take care of it later on
Admin::MediaAttachmentDeletionWorker.perform_inline(@status.media_attachments)
end
end

json = render_to_body json: @status, serializer: REST::StatusSerializer, source_requested: true

RemovalWorker.perform_async(@status.id, { 'preserve' => @status.account.local?, 'immediate' => !@status.account.local? })
6 changes: 6 additions & 0 deletions app/models/admin/status_batch_action.rb
Original file line number Diff line number Diff line change
@@ -47,6 +47,12 @@ def handle_delete!
statuses.each do |status|
status.discard_with_reblogs
log_action(:destroy, status)

next unless status.with_media?

# Immediately remove public copy of media instead of waiting for
# the vacuum_orphaned_records job to take care of it later on
Admin::MediaAttachmentDeletionWorker.perform_inline(status.media_attachments)
end

if with_report?
11 changes: 11 additions & 0 deletions app/workers/admin/media_attachment_deletion_worker.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# frozen_string_literal: true

class Admin::MediaAttachmentDeletionWorker
include Sidekiq::Worker

sidekiq_options queue: 'pull', lock: :until_executed, lock_ttl: 1.week.to_i

def perform(media_attachments)
AttachmentBatch.new(MediaAttachment, media_attachments).clear
end
end
7 changes: 6 additions & 1 deletion spec/controllers/api/v1/admin/statuses_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -31,7 +31,8 @@

describe 'DELETE #destroy' do
let(:scopes) { 'admin:write:statuses' }
let(:status) { Fabricate(:status) }
let(:media) { Fabricate(:media_attachment) }
let(:status) { Fabricate(:status, media_attachments: [media]) }

before do
post :destroy, params: { id: status.id }
@@ -48,6 +49,10 @@
it 'removes the status' do
expect(Status.find_by(id: status.id)).to be_nil
end

it 'removes associated media' do
expect(MediaAttachment.find_by(id: media.id)).to be_nil
end
end

describe 'POST #unsensitive' do
24 changes: 24 additions & 0 deletions spec/workers/admin/media_attachment_deletion_worker_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# frozen_string_literal: true

require 'rails_helper'

describe Admin::MediaAttachmentDeletionWorker do
let(:worker) { described_class.new }
let(:local_status) { Fabricate(:status) }
let(:remote_status) { Fabricate(:status, account: Fabricate(:account, domain: 'example.com')) }

describe 'perform' do
let!(:remote_media) { Fabricate(:media_attachment, remote_url: 'https://example.com/foo.png', status: remote_status) }
let!(:local_media) { Fabricate(:media_attachment, status: local_status) }

it 'deletes remote media attachments' do
subject.perform([remote_media])
expect(remote_media.reload.file).to be_blank
end

it 'deletes local media attachments' do
subject.perform([local_media])
expect(local_media.reload.file).to be_blank
end
end
end

0 comments on commit d0e1834

Please sign in to comment.