Move to project dropdown with infinite scroll for better performance

Use just SQL to check is a user can admin_issue on a project

Using offset pagination instead pages to avoid a count query

Tradeoff

- we duplicate how we check admin_issue in a SQL relation in the Ability class
This commit is contained in:
Paco Guzman 2016-08-05 15:29:20 +02:00
parent ac73de508e
commit ff903e6453
7 changed files with 101 additions and 7 deletions

View file

@ -105,6 +105,7 @@ v 8.11.0 (unreleased)
- Fix merge request new view not changing code view rendering style
- Make error pages responsive (Takuya Noguchi)
- The performance of the project dropdown used for moving issues has been improved
- Move to project dropdown with infinite scroll for better performance
- Fix skip_repo parameter being ignored when destroying a namespace
- Add all builds into stage/job dropdowns on builds page
- Change requests_profiles resource constraint to catch virtually any file

View file

@ -102,20 +102,34 @@
};
IssuableForm.prototype.initMoveDropdown = function() {
var $moveDropdown;
var $moveDropdown, pageSize;
$moveDropdown = $('.js-move-dropdown');
if ($moveDropdown.length) {
pageSize = $moveDropdown.data('page-size');
return $('.js-move-dropdown').select2({
ajax: {
url: $moveDropdown.data('projects-url'),
results: function(data) {
quietMillis: 125,
data: function(term, page, context) {
return {
results: data
search: term,
offset_id: context
};
},
data: function(query) {
results: function(data) {
var context,
more;
if (data.length >= pageSize)
more = true;
if (data[data.length - 1])
context = data[data.length - 1].id;
return {
search: query
results: data,
more: more,
context: context
};
}
},

View file

@ -45,7 +45,8 @@
min-width: 175px;
}
.select2-results .select2-result-label {
.select2-results .select2-result-label,
.select2-more-results {
padding: 10px 15px;
}

View file

@ -1,4 +1,6 @@
class MoveToProjectFinder
PAGE_SIZE = 50
def initialize(user)
@user = user
end
@ -8,6 +10,10 @@ class MoveToProjectFinder
projects = projects.search(search) if search.present?
projects = projects.excluding_project(from_project)
# infinite scroll using offset
projects = projects.where('projects.id < ?', offset_id) if offset_id.present?
projects = projects.limit(PAGE_SIZE)
# to ask for Project#name_with_namespace
projects.includes(namespace: :owner)
end

View file

@ -120,7 +120,7 @@
= label_tag :move_to_project_id, 'Move', class: 'control-label'
.col-sm-10
.issuable-form-select-holder
= hidden_field_tag :move_to_project_id, nil, class: 'js-move-dropdown', data: { placeholder: 'Select project', projects_url: autocomplete_projects_path(project_id: @project.id) }
= hidden_field_tag :move_to_project_id, nil, class: 'js-move-dropdown', data: { placeholder: 'Select project', projects_url: autocomplete_projects_path(project_id: @project.id), page_size: MoveToProjectFinder::PAGE_SIZE }
&nbsp;
%span{ data: { toggle: 'tooltip', placement: 'auto top' }, style: 'cursor: default',
title: 'Moving an issue will copy the discussion to a different project and close it here. All participants will be notified of the new location.' }

View file

@ -237,6 +237,56 @@ describe AutocompleteController do
end
end
context 'authorized projects apply limit' do
before do
authorized_project2 = create(:project)
authorized_project3 = create(:project)
authorized_project.team << [user, :master]
authorized_project2.team << [user, :master]
authorized_project3.team << [user, :master]
stub_const 'MoveToProjectFinder::PAGE_SIZE', 2
end
describe 'GET #projects with project ID' do
before do
get(:projects, project_id: project.id)
end
let(:body) { JSON.parse(response.body) }
it do
expect(body).to be_kind_of(Array)
expect(body.size).to eq 3 # Of a total of 4
end
end
end
context 'authorized projects with offset' do
before do
authorized_project2 = create(:project)
authorized_project3 = create(:project)
authorized_project.team << [user, :master]
authorized_project2.team << [user, :master]
authorized_project3.team << [user, :master]
end
describe 'GET #projects with project ID and offset_id' do
before do
get(:projects, project_id: project.id, offset_id: authorized_project.id)
end
let(:body) { JSON.parse(response.body) }
it do
expect(body.detect { |item| item['id'] == 0 }).to be_nil # 'No project' is not there
expect(body.detect { |item| item['id'] == authorized_project.id }).to be_nil # Offset project is not there either
end
end
end
context 'authorized projects without admin_issue ability' do
before(:each) do
authorized_project.team << [user, :guest]

View file

@ -51,6 +51,28 @@ describe MoveToProjectFinder do
expect(subject.execute(project).to_a).to eq([other_reporter_project])
end
it 'returns a page of projects ordered by id in descending order' do
stub_const 'MoveToProjectFinder::PAGE_SIZE', 2
reporter_project.team << [user, :reporter]
developer_project.team << [user, :developer]
master_project.team << [user, :master]
expect(subject.execute(project).to_a).to eq([master_project, developer_project])
end
it 'returns projects after the given offset id' do
stub_const 'MoveToProjectFinder::PAGE_SIZE', 2
reporter_project.team << [user, :reporter]
developer_project.team << [user, :developer]
master_project.team << [user, :master]
expect(subject.execute(project, search: nil, offset_id: master_project.id).to_a).to eq([developer_project, reporter_project])
expect(subject.execute(project, search: nil, offset_id: developer_project.id).to_a).to eq([reporter_project])
expect(subject.execute(project, search: nil, offset_id: reporter_project.id).to_a).to be_empty
end
end
context 'search' do