From 7877fcd83ce13309b5752a7766b93f65e71313bc Mon Sep 17 00:00:00 2001 From: Kevin Bongart <154600+KevinBongart@users.noreply.github.com> Date: Thu, 23 Nov 2023 05:00:09 -0500 Subject: [PATCH] Deduplicate IDs in relationships and familiar_followers APIs (#27982) --- .../v1/accounts/familiar_followers_controller.rb | 2 +- .../api/v1/accounts/relationships_controller.rb | 7 ++----- .../familiar_followers_controller_spec.rb | 12 ++++++++++++ .../api/v1/accounts/relationships_spec.rb | 16 ++++++++++++++++ 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/app/controllers/api/v1/accounts/familiar_followers_controller.rb b/app/controllers/api/v1/accounts/familiar_followers_controller.rb index b0bd8018a2..a49eb2eb27 100644 --- a/app/controllers/api/v1/accounts/familiar_followers_controller.rb +++ b/app/controllers/api/v1/accounts/familiar_followers_controller.rb @@ -12,7 +12,7 @@ class Api::V1::Accounts::FamiliarFollowersController < Api::BaseController private def set_accounts - @accounts = Account.without_suspended.where(id: account_ids).select('id, hide_collections').index_by(&:id).values_at(*account_ids).compact + @accounts = Account.without_suspended.where(id: account_ids).select('id, hide_collections') end def familiar_followers diff --git a/app/controllers/api/v1/accounts/relationships_controller.rb b/app/controllers/api/v1/accounts/relationships_controller.rb index 320084efb5..e5ae5b007b 100644 --- a/app/controllers/api/v1/accounts/relationships_controller.rb +++ b/app/controllers/api/v1/accounts/relationships_controller.rb @@ -5,11 +5,8 @@ class Api::V1::Accounts::RelationshipsController < Api::BaseController before_action :require_user! def index - scope = Account.where(id: account_ids).select('id') - scope.merge!(Account.without_suspended) unless truthy_param?(:with_suspended) - # .where doesn't guarantee that our results are in the same order - # we requested them, so return the "right" order to the requestor. - @accounts = scope.index_by(&:id).values_at(*account_ids).compact + @accounts = Account.where(id: account_ids).select('id') + @accounts.merge!(Account.without_suspended) unless truthy_param?(:with_suspended) render json: @accounts, each_serializer: REST::RelationshipSerializer, relationships: relationships end diff --git a/spec/controllers/api/v1/accounts/familiar_followers_controller_spec.rb b/spec/controllers/api/v1/accounts/familiar_followers_controller_spec.rb index bb075261f3..2261773094 100644 --- a/spec/controllers/api/v1/accounts/familiar_followers_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/familiar_followers_controller_spec.rb @@ -19,5 +19,17 @@ describe Api::V1::Accounts::FamiliarFollowersController do expect(response).to have_http_status(200) end + + context 'when there are duplicate account IDs in the params' do + let(:account_a) { Fabricate(:account) } + let(:account_b) { Fabricate(:account) } + + it 'removes duplicate account IDs from params' do + account_ids = [account_a, account_b, account_b, account_a, account_a].map { |a| a.id.to_s } + get :index, params: { id: account_ids } + + expect(body_as_json.pluck(:id)).to eq [account_a.id.to_s, account_b.id.to_s] + end + end end end diff --git a/spec/requests/api/v1/accounts/relationships_spec.rb b/spec/requests/api/v1/accounts/relationships_spec.rb index e06ffdfae9..cea45168a2 100644 --- a/spec/requests/api/v1/accounts/relationships_spec.rb +++ b/spec/requests/api/v1/accounts/relationships_spec.rb @@ -79,6 +79,22 @@ describe 'GET /api/v1/accounts/relationships' do end end + context 'when there are duplicate IDs in the params' do + let(:params) { { id: [simon.id, lewis.id, lewis.id, lewis.id, simon.id] } } + + it 'removes duplicate account IDs from params' do + subject + + expect(body_as_json) + .to be_an(Enumerable) + .and have_attributes( + size: 2, + first: include(simon_item), + second: include(lewis_item) + ) + end + end + def simon_item { id: simon.id.to_s,