From 589af7a1cca643199907fbaaaf795314f9a09d3e Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 30 Sep 2025 11:56:03 +0200 Subject: [PATCH] Change `GET /api/v1/statuses/:id/quotes` to allow listing quotes to other people's posts (#36291) --- .../api/v1/statuses/quotes_controller.rb | 34 ++++++++++--------- app/policies/status_policy.rb | 4 --- spec/requests/api/v1/statuses/quotes_spec.rb | 27 ++++++++++++--- 3 files changed, 40 insertions(+), 25 deletions(-) diff --git a/app/controllers/api/v1/statuses/quotes_controller.rb b/app/controllers/api/v1/statuses/quotes_controller.rb index 962855884e..be3a4edc83 100644 --- a/app/controllers/api/v1/statuses/quotes_controller.rb +++ b/app/controllers/api/v1/statuses/quotes_controller.rb @@ -4,13 +4,13 @@ class Api::V1::Statuses::QuotesController < Api::V1::Statuses::BaseController before_action -> { doorkeeper_authorize! :read, :'read:statuses' }, only: :index before_action -> { doorkeeper_authorize! :write, :'write:statuses' }, only: :revoke - before_action :check_owner! + before_action :set_statuses, only: :index + before_action :set_quote, only: :revoke after_action :insert_pagination_headers, only: :index def index cache_if_unauthenticated! - @statuses = load_statuses render json: @statuses, each_serializer: REST::StatusSerializer end @@ -24,18 +24,26 @@ class Api::V1::Statuses::QuotesController < Api::V1::Statuses::BaseController private - def check_owner! - authorize @status, :list_quotes? - end - def set_quote @quote = @status.quotes.find_by!(status_id: params[:id]) end - def load_statuses + def set_statuses scope = default_statuses scope = scope.not_excluded_by_account(current_account) unless current_account.nil? - scope.merge(paginated_quotes).to_a + @statuses = scope.merge(paginated_quotes).to_a + + # Store next page info before filtering + @records_continue = @statuses.size == limit_param(DEFAULT_STATUSES_LIMIT) + @pagination_since_id = @statuses.first.quote.id unless @statuses.empty? + @pagination_max_id = @statuses.last.quote.id if @records_continue + + if current_account&.id != @status.account_id + domains = @statuses.filter_map(&:account_domain).uniq + account_ids = @statuses.map(&:account_id).uniq + relations = current_account&.relations_map(account_ids, domains) || {} + @statuses.reject! { |status| StatusFilter.new(status, current_account, relations).filtered? } + end end def default_statuses @@ -58,15 +66,9 @@ class Api::V1::Statuses::QuotesController < Api::V1::Statuses::BaseController api_v1_status_quotes_url pagination_params(since_id: pagination_since_id) unless @statuses.empty? end - def pagination_max_id - @statuses.last.quote.id - end - - def pagination_since_id - @statuses.first.quote.id - end + attr_reader :pagination_max_id, :pagination_since_id def records_continue? - @statuses.size == limit_param(DEFAULT_STATUSES_LIMIT) + @records_continue end end diff --git a/app/policies/status_policy.rb b/app/policies/status_policy.rb index b746386995..03ceae718e 100644 --- a/app/policies/status_policy.rb +++ b/app/policies/status_policy.rb @@ -36,10 +36,6 @@ class StatusPolicy < ApplicationPolicy owned? end - def list_quotes? - owned? - end - alias unreblog? destroy? def update? diff --git a/spec/requests/api/v1/statuses/quotes_spec.rb b/spec/requests/api/v1/statuses/quotes_spec.rb index 9456556ce9..01e9e17b07 100644 --- a/spec/requests/api/v1/statuses/quotes_spec.rb +++ b/spec/requests/api/v1/statuses/quotes_spec.rb @@ -17,7 +17,7 @@ RSpec.describe 'API V1 Statuses Quotes' do let!(:accepted_quote) { Fabricate(:quote, quoted_status: status, state: :accepted) } let!(:rejected_quote) { Fabricate(:quote, quoted_status: status, state: :rejected) } let!(:pending_quote) { Fabricate(:quote, quoted_status: status, state: :pending) } - let!(:another_accepted_quote) { Fabricate(:quote, quoted_status: status, state: :accepted) } + let!(:accepted_private_quote) { Fabricate(:quote, status: Fabricate(:status, visibility: :private), quoted_status: status, state: :accepted) } context 'with an OAuth token' do let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } @@ -30,7 +30,7 @@ RSpec.describe 'API V1 Statuses Quotes' do expect(response) .to have_http_status(200) .and include_pagination_headers( - prev: api_v1_status_quotes_url(limit: 2, since_id: another_accepted_quote.id), + prev: api_v1_status_quotes_url(limit: 2, since_id: accepted_private_quote.id), next: api_v1_status_quotes_url(limit: 2, max_id: accepted_quote.id) ) expect(response.content_type) @@ -39,7 +39,7 @@ RSpec.describe 'API V1 Statuses Quotes' do expect(response.parsed_body) .to contain_exactly( include(id: accepted_quote.status.id.to_s), - include(id: another_accepted_quote.status.id.to_s) + include(id: accepted_private_quote.status.id.to_s) ) expect(response.parsed_body) @@ -52,12 +52,29 @@ RSpec.describe 'API V1 Statuses Quotes' do context 'with a different user than the post owner' do let(:status) { Fabricate(:status) } - it 'returns http forbidden' do + it 'returns http success and statuses but not private ones' do subject - expect(response).to have_http_status(403) + expect(response) + .to have_http_status(200) + .and include_pagination_headers( + prev: api_v1_status_quotes_url(limit: 2, since_id: accepted_private_quote.id), + next: api_v1_status_quotes_url(limit: 2, max_id: accepted_quote.id) + ) expect(response.content_type) .to start_with('application/json') + + expect(response.parsed_body) + .to contain_exactly( + include(id: accepted_quote.status.id.to_s) + ) + + expect(response.parsed_body) + .to_not include( + include(id: rejected_quote.status.id.to_s), + include(id: pending_quote.status.id.to_s), + include(id: accepted_private_quote.id.to_s) + ) end end end