Commit 37e681ac authored by Yorick Peterse's avatar Yorick Peterse

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).
parent ce1f0ad2
...@@ -14,7 +14,6 @@ export default class DropdownUser extends FilteredSearchDropdown { ...@@ -14,7 +14,6 @@ export default class DropdownUser extends FilteredSearchDropdown {
endpoint: `${gon.relative_url_root || ''}/autocomplete/users.json`, endpoint: `${gon.relative_url_root || ''}/autocomplete/users.json`,
searchKey: 'search', searchKey: 'search',
params: { params: {
per_page: 20,
active: true, active: true,
group_id: this.getGroupId(), group_id: this.getGroupId(),
project_id: this.getProjectId(), project_id: this.getProjectId(),
......
...@@ -39,7 +39,6 @@ function UsersSelect(currentUser, els, options = {}) { ...@@ -39,7 +39,6 @@ function UsersSelect(currentUser, els, options = {}) {
options.showCurrentUser = $dropdown.data('currentUser'); options.showCurrentUser = $dropdown.data('currentUser');
options.todoFilter = $dropdown.data('todoFilter'); options.todoFilter = $dropdown.data('todoFilter');
options.todoStateFilter = $dropdown.data('todoStateFilter'); options.todoStateFilter = $dropdown.data('todoStateFilter');
options.perPage = $dropdown.data('perPage');
showNullUser = $dropdown.data('nullUser'); showNullUser = $dropdown.data('nullUser');
defaultNullUser = $dropdown.data('nullUserDefault'); defaultNullUser = $dropdown.data('nullUserDefault');
showMenuAbove = $dropdown.data('showMenuAbove'); showMenuAbove = $dropdown.data('showMenuAbove');
...@@ -669,7 +668,6 @@ UsersSelect.prototype.users = function(query, options, callback) { ...@@ -669,7 +668,6 @@ UsersSelect.prototype.users = function(query, options, callback) {
const url = this.buildUrl(this.usersPath); const url = this.buildUrl(this.usersPath);
const params = { const params = {
search: query, search: query,
per_page: options.perPage || 20,
active: true, active: true,
project_id: options.projectId || null, project_id: options.projectId || null,
group_id: options.groupId || null, group_id: options.groupId || null,
......
class AutocompleteUsersFinder 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, attr_reader :current_user, :project, :group, :search, :skip_users,
:page, :per_page, :author_id, :params :author_id, :params
# EE # EE
attr_reader :skip_ldap attr_reader :skip_ldap
...@@ -11,8 +17,6 @@ class AutocompleteUsersFinder ...@@ -11,8 +17,6 @@ class AutocompleteUsersFinder
@group = group @group = group
@search = params[:search] @search = params[:search]
@skip_users = params[:skip_users] @skip_users = params[:skip_users]
@page = params[:page]
@per_page = params[:per_page]
@author_id = params[:author_id] @author_id = params[:author_id]
@params = params @params = params
...@@ -30,9 +34,10 @@ class AutocompleteUsersFinder ...@@ -30,9 +34,10 @@ class AutocompleteUsersFinder
items = items.reorder(:name) items = items.reorder(:name)
items = items.search(search) if search.present? items = items.search(search) if search.present?
items = items.where.not(id: skip_users) if skip_users.present? items = items.where.not(id: skip_users) if skip_users.present?
items = items.limit(LIMIT)
# EE # EE
items = load_users_by_push_ability(items) || items.page(page).per(per_page) items = load_users_by_push_ability(items)
if params[:todo_filter].present? && current_user if params[:todo_filter].present? && current_user
items = items.todo_authors(current_user.id, params[:todo_state_filter]) items = items.todo_authors(current_user.id, params[:todo_state_filter])
...@@ -66,7 +71,7 @@ class AutocompleteUsersFinder ...@@ -66,7 +71,7 @@ class AutocompleteUsersFinder
def users_from_project def users_from_project
if author_id.present? if author_id.present?
union = Gitlab::SQL::Union union = Gitlab::SQL::Union
.new([project.team.users, User.where(id: author_id)]) .new([project.authorized_users, User.where(id: author_id)])
User.from("(#{union.to_sql}) #{User.table_name}") User.from("(#{union.to_sql}) #{User.table_name}")
else else
...@@ -76,14 +81,13 @@ class AutocompleteUsersFinder ...@@ -76,14 +81,13 @@ class AutocompleteUsersFinder
# EE # EE
def load_users_by_push_ability(items) def load_users_by_push_ability(items)
return unless project return items unless project
ability = push_ability ability = push_ability
return if ability.blank? return items if ability.blank?
items.to_a items.to_a
.select { |user| user.can?(ability, project) } .select { |user| user.can?(ability, project) }
.take(per_page&.to_i || Kaminari.config.default_per_page)
end end
def push_ability def push_ability
......
...@@ -109,15 +109,17 @@ describe AutocompleteController do ...@@ -109,15 +109,17 @@ describe AutocompleteController do
end end
context 'limited users per page' do context 'limited users per page' do
let(:per_page) { 2 }
before do before do
25.times do
create(:user)
end
sign_in(user) sign_in(user)
get(:users, per_page: per_page) get(:users)
end end
it { expect(json_response).to be_kind_of(Array) } 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 end
context 'unauthenticated user' do context 'unauthenticated user' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment