Fix SQL query in /api/v1/directory (#28412)

This commit is contained in:
Claire 2023-12-18 11:03:20 +01:00 committed by GitHub
parent 363bedd050
commit 2e4d43933d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 140 additions and 1 deletions

View file

@ -112,7 +112,7 @@ class Account < ApplicationRecord
scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) }
scope :without_unapproved, -> { left_outer_joins(:user).remote.or(left_outer_joins(:user).merge(User.approved.confirmed)) }
scope :searchable, -> { without_unapproved.without_suspended.where(moved_to_account_id: nil) }
scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).left_outer_joins(:account_stat) }
scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).joins(:account_stat) }
scope :followable_by, ->(account) { joins(arel_table.join(Follow.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(Follow.arel_table[:target_account_id]).and(Follow.arel_table[:account_id].eq(account.id))).join_sources).where(Follow.arel_table[:id].eq(nil)).joins(arel_table.join(FollowRequest.arel_table, Arel::Nodes::OuterJoin).on(arel_table[:id].eq(FollowRequest.arel_table[:target_account_id]).and(FollowRequest.arel_table[:account_id].eq(account.id))).join_sources).where(FollowRequest.arel_table[:id].eq(nil)) }
scope :by_recent_status, -> { includes(:account_stat).merge(AccountStat.order('last_status_at DESC NULLS LAST')).references(:account_stat) }
scope :by_recent_sign_in, -> { order(Arel.sql('users.current_sign_in_at DESC NULLS LAST')) }

View file

@ -0,0 +1,139 @@
# frozen_string_literal: true
require 'rails_helper'
describe 'Directories API' do
let(:user) { Fabricate(:user, confirmed_at: nil) }
let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) }
let(:scopes) { 'read:follows' }
let(:headers) { { 'Authorization' => "Bearer #{token.token}" } }
describe 'GET /api/v1/directories' do
context 'with no params' do
before do
local_unconfirmed_account = Fabricate(
:account,
domain: nil,
user: Fabricate(:user, confirmed_at: nil, approved: true),
username: 'local_unconfirmed'
)
local_unconfirmed_account.create_account_stat!
local_unapproved_account = Fabricate(
:account,
domain: nil,
user: Fabricate(:user, confirmed_at: 10.days.ago),
username: 'local_unapproved'
)
local_unapproved_account.create_account_stat!
local_unapproved_account.user.update(approved: false)
local_undiscoverable_account = Fabricate(
:account,
domain: nil,
user: Fabricate(:user, confirmed_at: 10.days.ago, approved: true),
discoverable: false,
username: 'local_undiscoverable'
)
local_undiscoverable_account.create_account_stat!
excluded_from_timeline_account = Fabricate(
:account,
domain: 'host.example',
discoverable: true,
username: 'remote_excluded_from_timeline'
)
excluded_from_timeline_account.create_account_stat!
Fabricate(:block, account: user.account, target_account: excluded_from_timeline_account)
domain_blocked_account = Fabricate(
:account,
domain: 'test.example',
discoverable: true,
username: 'remote_domain_blocked'
)
domain_blocked_account.create_account_stat!
Fabricate(:account_domain_block, account: user.account, domain: 'test.example')
local_discoverable_account.create_account_stat!
eligible_remote_account.create_account_stat!
end
let(:local_discoverable_account) do
Fabricate(
:account,
domain: nil,
user: Fabricate(:user, confirmed_at: 10.days.ago, approved: true),
discoverable: true,
username: 'local_discoverable'
)
end
let(:eligible_remote_account) do
Fabricate(
:account,
domain: 'host.example',
discoverable: true,
username: 'eligible_remote'
)
end
it 'returns the local discoverable account and the remote discoverable account' do
get '/api/v1/directory', headers: headers
expect(response).to have_http_status(200)
expect(body_as_json.size).to eq(2)
expect(body_as_json.pluck(:id)).to contain_exactly(eligible_remote_account.id.to_s, local_discoverable_account.id.to_s)
end
end
context 'when asking for local accounts only' do
let(:user) { Fabricate(:user, confirmed_at: 10.days.ago, approved: true) }
let(:local_account) { Fabricate(:account, domain: nil, user: user) }
let(:remote_account) { Fabricate(:account, domain: 'host.example') }
before do
local_account.create_account_stat!
remote_account.create_account_stat!
end
it 'returns only the local accounts' do
get '/api/v1/directory', headers: headers, params: { local: '1' }
expect(response).to have_http_status(200)
expect(body_as_json.size).to eq(1)
expect(body_as_json.first[:id]).to include(local_account.id.to_s)
expect(response.body).to_not include(remote_account.id.to_s)
end
end
context 'when ordered by active' do
it 'returns accounts in order of most recent status activity' do
old_stat = Fabricate(:account_stat, last_status_at: 1.day.ago)
new_stat = Fabricate(:account_stat, last_status_at: 1.minute.ago)
get '/api/v1/directory', headers: headers, params: { order: 'active' }
expect(response).to have_http_status(200)
expect(body_as_json.size).to eq(2)
expect(body_as_json.first[:id]).to include(new_stat.account_id.to_s)
expect(body_as_json.second[:id]).to include(old_stat.account_id.to_s)
end
end
context 'when ordered by new' do
it 'returns accounts in order of creation' do
account_old = Fabricate(:account_stat).account
travel_to 10.seconds.from_now
account_new = Fabricate(:account_stat).account
get '/api/v1/directory', headers: headers, params: { order: 'new' }
expect(response).to have_http_status(200)
expect(body_as_json.size).to eq(2)
expect(body_as_json.first[:id]).to include(account_new.id.to_s)
expect(body_as_json.second[:id]).to include(account_old.id.to_s)
end
end
end
end