From f3a12ddfd0b5b9379c7cfe4229697765851f4738 Mon Sep 17 00:00:00 2001 From: Jakub Mendyk Date: Sun, 26 Aug 2018 21:30:17 +0200 Subject: [PATCH] Make Api::V1::MutesController paginate properly (#8472) Fixes #8463 --- app/controllers/api/v1/mutes_controller.rb | 26 ++++----- .../api/v1/mutes_controller_spec.rb | 56 ++++++++++++++++--- 2 files changed, 61 insertions(+), 21 deletions(-) diff --git a/app/controllers/api/v1/mutes_controller.rb b/app/controllers/api/v1/mutes_controller.rb index faa7d16cd9..df6c8e86c5 100644 --- a/app/controllers/api/v1/mutes_controller.rb +++ b/app/controllers/api/v1/mutes_controller.rb @@ -15,19 +15,17 @@ class Api::V1::MutesController < Api::BaseController private def load_accounts - default_accounts.merge(paginated_mutes).to_a - end - - def default_accounts - Account.includes(:muted_by).references(:muted_by) + paginated_mutes.map(&:target_account) end def paginated_mutes - Mute.where(account: current_account).paginate_by_max_id( - limit_param(DEFAULT_ACCOUNTS_LIMIT), - params[:max_id], - params[:since_id] - ) + @paginated_mutes ||= Mute.eager_load(:target_account) + .where(account: current_account) + .paginate_by_max_id( + limit_param(DEFAULT_ACCOUNTS_LIMIT), + params[:max_id], + params[:since_id] + ) end def insert_pagination_headers @@ -41,21 +39,21 @@ class Api::V1::MutesController < Api::BaseController end def prev_path - unless @accounts.empty? + unless paginated_mutes.empty? api_v1_mutes_url pagination_params(since_id: pagination_since_id) end end def pagination_max_id - @accounts.last.muted_by_ids.last + paginated_mutes.last.id end def pagination_since_id - @accounts.first.muted_by_ids.first + paginated_mutes.first.id end def records_continue? - @accounts.size == limit_param(DEFAULT_ACCOUNTS_LIMIT) + paginated_mutes.size == limit_param(DEFAULT_ACCOUNTS_LIMIT) end def pagination_params(core_params) diff --git a/spec/controllers/api/v1/mutes_controller_spec.rb b/spec/controllers/api/v1/mutes_controller_spec.rb index f9603b7ff1..a2b814a690 100644 --- a/spec/controllers/api/v1/mutes_controller_spec.rb +++ b/spec/controllers/api/v1/mutes_controller_spec.rb @@ -3,19 +3,61 @@ require 'rails_helper' RSpec.describe Api::V1::MutesController, type: :controller do render_views - let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } - let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:mutes') } + let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } + let(:scopes) { 'read:mutes' } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } - before do - Fabricate(:mute, account: user.account, hide_notifications: false) - allow(controller).to receive(:doorkeeper_token) { token } - end + before { allow(controller).to receive(:doorkeeper_token) { token } } describe 'GET #index' do - it 'returns http success' do + it 'limits according to limit parameter' do + 2.times.map { Fabricate(:mute, account: user.account) } get :index, params: { limit: 1 } + expect(body_as_json.size).to eq 1 + end + + it 'queries mutes in range according to max_id' do + mutes = 2.times.map { Fabricate(:mute, account: user.account) } + + get :index, params: { max_id: mutes[1] } + expect(body_as_json.size).to eq 1 + expect(body_as_json[0][:id]).to eq mutes[0].target_account_id.to_s + end + + it 'queries mutes in range according to since_id' do + mutes = 2.times.map { Fabricate(:mute, account: user.account) } + + get :index, params: { since_id: mutes[0] } + + expect(body_as_json.size).to eq 1 + expect(body_as_json[0][:id]).to eq mutes[1].target_account_id.to_s + end + + it 'sets pagination header for next path' do + mutes = 2.times.map { Fabricate(:mute, account: user.account) } + get :index, params: { limit: 1, since_id: mutes[0] } + expect(response.headers['Link'].find_link(['rel', 'next']).href).to eq api_v1_mutes_url(limit: 1, max_id: mutes[1]) + end + + it 'sets pagination header for previous path' do + mute = Fabricate(:mute, account: user.account) + get :index + expect(response.headers['Link'].find_link(['rel', 'prev']).href).to eq api_v1_mutes_url(since_id: mute) + end + + it 'returns http success' do + get :index expect(response).to have_http_status(200) end + + context 'with wrong scopes' do + let(:scopes) { 'write:mutes' } + + it 'returns http forbidden' do + get :index + expect(response).to have_http_status(403) + end + end end end