From e71fb450e08515b53a33bdef1744e0ba7ce6f23b Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 28 Feb 2025 14:39:01 +0100 Subject: [PATCH 1/4] Add optional `delete_media` parameter to `DELETE /api/v1/statuses/:id` (#33988) --- app/controllers/api/v1/statuses_controller.rb | 2 +- app/javascript/mastodon/actions/statuses.js | 2 +- spec/requests/api/v1/statuses_spec.rb | 19 ++++++++++++++++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 19cc71ae58..2027bc6016 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -111,7 +111,7 @@ class Api::V1::StatusesController < Api::BaseController @status.account.statuses_count = @status.account.statuses_count - 1 json = render_to_body json: @status, serializer: REST::StatusSerializer, source_requested: true - RemovalWorker.perform_async(@status.id, { 'redraft' => true }) + RemovalWorker.perform_async(@status.id, { 'redraft' => !truthy_param?(:delete_media) }) render json: json end diff --git a/app/javascript/mastodon/actions/statuses.js b/app/javascript/mastodon/actions/statuses.js index 1e4e545d8c..1e5b53c687 100644 --- a/app/javascript/mastodon/actions/statuses.js +++ b/app/javascript/mastodon/actions/statuses.js @@ -138,7 +138,7 @@ export function deleteStatus(id, withRedraft = false) { dispatch(deleteStatusRequest(id)); - api().delete(`/api/v1/statuses/${id}`).then(response => { + api().delete(`/api/v1/statuses/${id}`, { params: { delete_media: !withRedraft } }).then(response => { dispatch(deleteStatusSuccess(id)); dispatch(deleteFromTimelines(id)); dispatch(importFetchedAccount(response.data.account)); diff --git a/spec/requests/api/v1/statuses_spec.rb b/spec/requests/api/v1/statuses_spec.rb index ddf5945d25..285fa93655 100644 --- a/spec/requests/api/v1/statuses_spec.rb +++ b/spec/requests/api/v1/statuses_spec.rb @@ -257,13 +257,30 @@ RSpec.describe '/api/v1/statuses' do it_behaves_like 'forbidden for wrong scope', 'read read:statuses' - it 'removes the status', :aggregate_failures do + it 'discards the status and schedules removal as a redraft', :aggregate_failures do subject expect(response).to have_http_status(200) expect(response.content_type) .to start_with('application/json') expect(Status.find_by(id: status.id)).to be_nil + expect(RemovalWorker).to have_enqueued_sidekiq_job(status.id, { 'redraft' => true }) + end + + context 'when called with truthy delete_media' do + subject do + delete "/api/v1/statuses/#{status.id}?delete_media=true", headers: headers + end + + it 'discards the status and schedules removal without the redraft flag', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(response.content_type) + .to start_with('application/json') + expect(Status.find_by(id: status.id)).to be_nil + expect(RemovalWorker).to have_enqueued_sidekiq_job(status.id, { 'redraft' => false }) + end end end From 4960193ed0db67dde94b5acd5f983957ffca2a80 Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Fri, 28 Feb 2025 14:41:42 +0100 Subject: [PATCH 2/4] 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 From bdc9cb27e247a7c667f32dd29f0174db985c9d44 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Fri, 28 Feb 2025 09:26:43 -0500 Subject: [PATCH 3/4] Update rubocop to version 1.73.1 (#34034) --- .rubocop_todo.yml | 2 +- Gemfile.lock | 6 +++--- app/models/concerns/account/interactions.rb | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 5cf43a3d5b..e612721ac7 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config --auto-gen-only-exclude --no-offense-counts --no-auto-gen-timestamp` -# using RuboCop version 1.72.2. +# using RuboCop version 1.73.1. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new diff --git a/Gemfile.lock b/Gemfile.lock index 3e97c94d35..19bb8ccc92 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -719,7 +719,7 @@ GEM rspec-mocks (~> 3.0) sidekiq (>= 5, < 8) rspec-support (3.13.2) - rubocop (1.72.2) + rubocop (1.73.1) json (~> 2.3) language_server-protocol (~> 3.17.0.2) lint_roller (~> 1.1.0) @@ -730,7 +730,7 @@ GEM rubocop-ast (>= 1.38.0, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 4.0) - rubocop-ast (1.38.0) + rubocop-ast (1.38.1) parser (>= 3.3.1.0) rubocop-capybara (2.21.0) rubocop (~> 1.41) @@ -854,7 +854,7 @@ GEM unicode-display_width (3.1.4) unicode-emoji (~> 4.0, >= 4.0.4) unicode-emoji (4.0.4) - uri (1.0.2) + uri (1.0.3) useragent (0.16.11) validate_email (0.1.6) activemodel (>= 3.0) diff --git a/app/models/concerns/account/interactions.rb b/app/models/concerns/account/interactions.rb index 6e2dc9126c..33d51abed9 100644 --- a/app/models/concerns/account/interactions.rb +++ b/app/models/concerns/account/interactions.rb @@ -115,7 +115,7 @@ module Account::Interactions end def follow!(other_account, reblogs: nil, notify: nil, languages: nil, uri: nil, rate_limit: false, bypass_limit: false) - rel = active_relationships.create_with(show_reblogs: reblogs.nil? ? true : reblogs, notify: notify.nil? ? false : notify, languages: languages, uri: uri, rate_limit: rate_limit, bypass_follow_limit: bypass_limit) + rel = active_relationships.create_with(show_reblogs: reblogs.nil? || reblogs, notify: notify.nil? ? false : notify, languages: languages, uri: uri, rate_limit: rate_limit, bypass_follow_limit: bypass_limit) .find_or_create_by!(target_account: other_account) rel.show_reblogs = reblogs unless reblogs.nil? @@ -128,7 +128,7 @@ module Account::Interactions end def request_follow!(other_account, reblogs: nil, notify: nil, languages: nil, uri: nil, rate_limit: false, bypass_limit: false) - rel = follow_requests.create_with(show_reblogs: reblogs.nil? ? true : reblogs, notify: notify.nil? ? false : notify, uri: uri, languages: languages, rate_limit: rate_limit, bypass_follow_limit: bypass_limit) + rel = follow_requests.create_with(show_reblogs: reblogs.nil? || reblogs, notify: notify.nil? ? false : notify, uri: uri, languages: languages, rate_limit: rate_limit, bypass_follow_limit: bypass_limit) .find_or_create_by!(target_account: other_account) rel.show_reblogs = reblogs unless reblogs.nil? From 894b96cf1fef16416d2780565e19f59e9cf6c6dc Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Fri, 28 Feb 2025 09:33:15 -0500 Subject: [PATCH 4/4] Rely on `haml-lint` parallel default (#34036) --- .github/workflows/lint-haml.yml | 2 +- lint-staged.config.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/lint-haml.yml b/.github/workflows/lint-haml.yml index 9361358078..499be2010a 100644 --- a/.github/workflows/lint-haml.yml +++ b/.github/workflows/lint-haml.yml @@ -43,4 +43,4 @@ jobs: - name: Run haml-lint run: | echo "::add-matcher::.github/workflows/haml-lint-problem-matcher.json" - bin/haml-lint --parallel --reporter github + bin/haml-lint --reporter github diff --git a/lint-staged.config.js b/lint-staged.config.js index baf5d0d454..63f5258a94 100644 --- a/lint-staged.config.js +++ b/lint-staged.config.js @@ -3,7 +3,7 @@ const config = { 'Gemfile|*.{rb,ruby,ru,rake}': 'bin/rubocop --force-exclusion -a', '*.{js,jsx,ts,tsx}': 'eslint --fix', '*.{css,scss}': 'stylelint --fix', - '*.haml': 'bin/haml-lint -a --parallel', + '*.haml': 'bin/haml-lint -a', '**/*.ts?(x)': () => 'tsc -p tsconfig.json --noEmit', };