From d2e43fbde60589c905aae9d2500c63355ba8fdc4 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 15 Feb 2018 19:33:35 +0100 Subject: Removed pagination from AutocompleteUsersFinder The frontend code doesn't use this so there's no practical point in supporting this. We also hardcode the limit to 20 so users can no longer request their own limit, which could overload the database (depending on any upper bounds perhaps enforced by Kaminari). --- app/assets/javascripts/filtered_search/dropdown_user.js | 1 - app/assets/javascripts/users_select.js | 2 -- app/finders/autocomplete_users_finder.rb | 12 ++++++++---- spec/controllers/autocomplete_controller_spec.rb | 10 ++++++---- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/filtered_search/dropdown_user.js b/app/assets/javascripts/filtered_search/dropdown_user.js index 22421fc4868..d36f38a70b5 100644 --- a/app/assets/javascripts/filtered_search/dropdown_user.js +++ b/app/assets/javascripts/filtered_search/dropdown_user.js @@ -14,7 +14,6 @@ export default class DropdownUser extends FilteredSearchDropdown { endpoint: `${gon.relative_url_root || ''}/autocomplete/users.json`, searchKey: 'search', params: { - per_page: 20, active: true, group_id: this.getGroupId(), project_id: this.getProjectId(), diff --git a/app/assets/javascripts/users_select.js b/app/assets/javascripts/users_select.js index 8958534689c..3385aba0279 100644 --- a/app/assets/javascripts/users_select.js +++ b/app/assets/javascripts/users_select.js @@ -39,7 +39,6 @@ function UsersSelect(currentUser, els, options = {}) { options.showCurrentUser = $dropdown.data('currentUser'); options.todoFilter = $dropdown.data('todoFilter'); options.todoStateFilter = $dropdown.data('todoStateFilter'); - options.perPage = $dropdown.data('perPage'); showNullUser = $dropdown.data('nullUser'); defaultNullUser = $dropdown.data('nullUserDefault'); showMenuAbove = $dropdown.data('showMenuAbove'); @@ -669,7 +668,6 @@ UsersSelect.prototype.users = function(query, options, callback) { const url = this.buildUrl(this.usersPath); const params = { search: query, - per_page: options.perPage || 20, active: true, project_id: options.projectId || null, group_id: options.groupId || null, diff --git a/app/finders/autocomplete_users_finder.rb b/app/finders/autocomplete_users_finder.rb index 3f10727be8c..49eb6af8f11 100644 --- a/app/finders/autocomplete_users_finder.rb +++ b/app/finders/autocomplete_users_finder.rb @@ -1,6 +1,12 @@ class AutocompleteUsersFinder + # The number of users to display in the results is hardcoded to 20, and + # pagination is not supported. This ensures that performance remains + # consistent and removes the need for implementing keyset pagination to ensure + # good performance. + LIMIT = 20 + attr_reader :current_user, :project, :group, :search, :skip_users, - :page, :per_page, :author_id, :params + :author_id, :params def initialize(params:, current_user:, project:, group:) @current_user = current_user @@ -8,8 +14,6 @@ class AutocompleteUsersFinder @group = group @search = params[:search] @skip_users = params[:skip_users] - @page = params[:page] - @per_page = params[:per_page] @author_id = params[:author_id] @params = params end @@ -20,7 +24,7 @@ class AutocompleteUsersFinder items = items.reorder(:name) items = items.search(search) if search.present? items = items.where.not(id: skip_users) if skip_users.present? - items = items.page(page).per(per_page) + items = items.limit(LIMIT) if params[:todo_filter].present? && current_user items = items.todo_authors(current_user.id, params[:todo_state_filter]) diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 73fff6eb5ca..b7257fac608 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -109,15 +109,17 @@ describe AutocompleteController do end context 'limited users per page' do - let(:per_page) { 2 } - before do + 25.times do + create(:user) + end + sign_in(user) - get(:users, per_page: per_page) + get(:users) end it { expect(json_response).to be_kind_of(Array) } - it { expect(json_response.size).to eq(per_page) } + it { expect(json_response.size).to eq(20) } end context 'unauthenticated user' do -- cgit v1.2.1