From 4960193ed0db67dde94b5acd5f983957ffca2a80 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Fri, 28 Feb 2025 14:41:42 +0100 Subject: [PATCH] Add API to delete media attachments that are not in use (#33991) --- app/controllers/api/v1/media_controller.rb | 17 ++++++- config/routes/api.rb | 2 +- spec/requests/api/v1/media_spec.rb | 53 ++++++++++++++++++++++ 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/v1/media_controller.rb b/app/controllers/api/v1/media_controller.rb index 5ea26d55bd..c427e055ea 100644 --- a/app/controllers/api/v1/media_controller.rb +++ b/app/controllers/api/v1/media_controller.rb @@ -3,8 +3,8 @@ class Api::V1::MediaController < Api::BaseController before_action -> { doorkeeper_authorize! :write, :'write:media' } before_action :require_user! - before_action :set_media_attachment, except: [:create] - before_action :check_processing, except: [:create] + before_action :set_media_attachment, except: [:create, :destroy] + before_action :check_processing, except: [:create, :destroy] def show render json: @media_attachment, serializer: REST::MediaAttachmentSerializer, status: status_code_for_media_attachment @@ -25,6 +25,15 @@ class Api::V1::MediaController < Api::BaseController render json: @media_attachment, serializer: REST::MediaAttachmentSerializer, status: status_code_for_media_attachment end + def destroy + @media_attachment = current_account.media_attachments.find(params[:id]) + + return render json: in_usage_error, status: 422 unless @media_attachment.status_id.nil? + + @media_attachment.destroy + render_empty + end + private def status_code_for_media_attachment @@ -54,4 +63,8 @@ class Api::V1::MediaController < Api::BaseController def processing_error { error: 'Error processing thumbnail for uploaded media' } end + + def in_usage_error + { error: 'Media attachment is currently used by a status' } + end end diff --git a/config/routes/api.rb b/config/routes/api.rb index 34a267b35d..ce63cda1bf 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -77,7 +77,7 @@ namespace :api, format: false do end end - resources :media, only: [:create, :update, :show] + resources :media, only: [:create, :update, :show, :destroy] resources :blocks, only: [:index] resources :mutes, only: [:index] resources :favourites, only: [:index] diff --git a/spec/requests/api/v1/media_spec.rb b/spec/requests/api/v1/media_spec.rb index d7d0b92f11..4d6e250207 100644 --- a/spec/requests/api/v1/media_spec.rb +++ b/spec/requests/api/v1/media_spec.rb @@ -193,4 +193,57 @@ RSpec.describe 'Media' do end end end + + describe 'DELETE /api/v1/media/:id' do + subject do + delete "/api/v1/media/#{media.id}", headers: headers + end + + context 'when media is not attached to a status' do + let(:media) { Fabricate(:media_attachment, account: user.account, status: nil) } + + it 'returns http empty response' do + subject + + expect(response).to have_http_status(200) + expect(response.content_type) + .to start_with('application/json') + + expect(MediaAttachment.where(id: media.id)).to_not exist + end + end + + context 'when media is attached to a status' do + let(:media) { Fabricate(:media_attachment, account: user.account, status: Fabricate.build(:status)) } + + it 'returns http unprocessable entity' do + subject + + expect(response).to have_http_status(422) + expect(response.content_type) + .to start_with('application/json') + expect(response.parsed_body).to match( + a_hash_including( + error: 'Media attachment is currently used by a status' + ) + ) + + expect(MediaAttachment.where(id: media.id)).to exist + end + end + + context 'when the media belongs to somebody else' do + let(:media) { Fabricate(:media_attachment, status: nil) } + + it 'returns http not found' do + subject + + expect(response).to have_http_status(404) + expect(response.content_type) + .to start_with('application/json') + + expect(MediaAttachment.where(id: media.id)).to exist + end + end + end end