summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaco Guzman <pacoguzmanp@gmail.com>2016-08-05 15:29:20 +0200
committerPaco Guzman <pacoguzmanp@gmail.com>2016-08-08 19:05:16 +0200
commitb6fe79f48b2fc3ab9d64cffb26b39abd30bcd396 (patch)
tree48a5cfedf4087db5dadf9c17db392c0ac01fea61
parent59ce1af53b8d25d1b4ae8b6e59f069c5147ca572 (diff)
downloadgitlab-ce-b6fe79f48b2fc3ab9d64cffb26b39abd30bcd396.tar.gz
Move to project dropdown with infinite scroll for better performance
-rw-r--r--CHANGELOG1
-rw-r--r--app/assets/javascripts/issuable_form.js24
-rw-r--r--app/assets/stylesheets/framework/selects.scss3
-rw-r--r--app/controllers/autocomplete_controller.rb14
-rw-r--r--app/finders/move_to_project_finder.rb30
-rw-r--r--app/views/shared/issuable/_form.html.haml2
-rw-r--r--spec/controllers/autocomplete_controller_spec.rb338
7 files changed, 296 insertions, 116 deletions
diff --git a/CHANGELOG b/CHANGELOG
index f5416434ab1..b9d7cdd9127 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -66,6 +66,7 @@ v 8.11.0 (unreleased)
- Add commit stats in commit api. !5517 (dixpac)
- Add CI configuration button on project page
- Make error pages responsive (Takuya Noguchi)
+ - Move to project dropdown with infinite scroll for better performance
- Fix skip_repo parameter being ignored when destroying a namespace
- Change requests_profiles resource constraint to catch virtually any file
- Bump gitlab_git to lazy load compare commits
diff --git a/app/assets/javascripts/issuable_form.js b/app/assets/javascripts/issuable_form.js
index 297d4f029f0..b7f92ae9883 100644
--- a/app/assets/javascripts/issuable_form.js
+++ b/app/assets/javascripts/issuable_form.js
@@ -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
};
}
},
diff --git a/app/assets/stylesheets/framework/selects.scss b/app/assets/stylesheets/framework/selects.scss
index 21d87cc9d34..b2e22b60440 100644
--- a/app/assets/stylesheets/framework/selects.scss
+++ b/app/assets/stylesheets/framework/selects.scss
@@ -45,7 +45,8 @@
min-width: 175px;
}
-.select2-results .select2-result-label {
+.select2-results .select2-result-label,
+.select2-more-results {
padding: 10px 15px;
}
diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb
index d828d163c28..350106a0b29 100644
--- a/app/controllers/autocomplete_controller.rb
+++ b/app/controllers/autocomplete_controller.rb
@@ -34,19 +34,15 @@ class AutocompleteController < ApplicationController
def projects
project = Project.find_by_id(params[:project_id])
+ projects = projects_finder.execute(project, search: params[:search], offset_id: params[:offset_id])
- projects = current_user.authorized_projects
- projects = projects.search(params[:search]) if params[:search].present?
- projects = projects.select do |project|
- current_user.can?(:admin_issue, project)
- end
+ ActiveRecord::Associations::Preloader.new.preload(projects, { namespace: :owner })
no_project = {
id: 0,
name_with_namespace: 'No project',
}
- projects.unshift(no_project)
- projects.delete(project)
+ projects.unshift(no_project) unless params[:offset_id].present?
render json: projects.to_json(only: [:id, :name_with_namespace], methods: :name_with_namespace)
end
@@ -71,4 +67,8 @@ class AutocompleteController < ApplicationController
User.none
end
end
+
+ def projects_finder
+ MoveToProjectFinder.new(current_user)
+ end
end
diff --git a/app/finders/move_to_project_finder.rb b/app/finders/move_to_project_finder.rb
new file mode 100644
index 00000000000..54e7e55aa05
--- /dev/null
+++ b/app/finders/move_to_project_finder.rb
@@ -0,0 +1,30 @@
+class MoveToProjectFinder
+ PAGE_SIZE = 50
+
+ def initialize(user)
+ @user = user
+ end
+
+ def execute(from_project, search: nil, offset_id: nil)
+ projects = @user.authorized_projects
+ projects = projects.search(search) if search.present?
+ projects = skip_projects_before(projects, offset_id.to_i) if offset_id.present?
+ projects = take_projects(projects)
+ projects.delete(from_project)
+ projects
+ end
+
+ private
+
+ def skip_projects_before(projects, offset_project_id)
+ index = projects.index { |project| project.id == offset_project_id }
+
+ index ? projects.drop(index + 1) : projects
+ end
+
+ def take_projects(projects)
+ projects.lazy.select do |project|
+ @user.can?(:admin_issue, project)
+ end.take(PAGE_SIZE).to_a
+ end
+end
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index c30bdb0ae91..1b12b580a16 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -98,7 +98,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.' }
diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb
index ed0b7f9e240..a121cb2fc97 100644
--- a/spec/controllers/autocomplete_controller_spec.rb
+++ b/spec/controllers/autocomplete_controller_spec.rb
@@ -2,178 +2,312 @@ require 'spec_helper'
describe AutocompleteController do
let!(:project) { create(:project) }
- let!(:user) { create(:user) }
- let!(:user2) { create(:user) }
- let!(:non_member) { create(:user) }
+ let!(:user) { create(:user) }
- context 'project members' do
- before do
- sign_in(user)
- project.team << [user, :master]
- end
+ context 'users and members' do
+ let!(:user2) { create(:user) }
+ let!(:non_member) { create(:user) }
- describe 'GET #users with project ID' do
+ context 'project members' do
before do
- get(:users, project_id: project.id)
+ sign_in(user)
+ project.team << [user, :master]
end
- let(:body) { JSON.parse(response.body) }
+ describe 'GET #users with project ID' do
+ before do
+ get(:users, project_id: project.id)
+ end
- it { expect(body).to be_kind_of(Array) }
- it { expect(body.size).to eq 1 }
- it { expect(body.map { |u| u["username"] }).to include(user.username) }
+ let(:body) { JSON.parse(response.body) }
+
+ it { expect(body).to be_kind_of(Array) }
+ it { expect(body.size).to eq 1 }
+ it { expect(body.map { |u| u["username"] }).to include(user.username) }
+ end
+
+ describe 'GET #users with unknown project' do
+ before do
+ get(:users, project_id: 'unknown')
+ end
+
+ it { expect(response).to have_http_status(404) }
+ end
end
- describe 'GET #users with unknown project' do
+ context 'group members' do
+ let(:group) { create(:group) }
+
before do
- get(:users, project_id: 'unknown')
+ sign_in(user)
+ group.add_owner(user)
end
- it { expect(response).to have_http_status(404) }
- end
- end
+ let(:body) { JSON.parse(response.body) }
- context 'group members' do
- let(:group) { create(:group) }
+ describe 'GET #users with group ID' do
+ before do
+ get(:users, group_id: group.id)
+ end
- before do
- sign_in(user)
- group.add_owner(user)
+ it { expect(body).to be_kind_of(Array) }
+ it { expect(body.size).to eq 1 }
+ it { expect(body.first["username"]).to eq user.username }
+ end
+
+ describe 'GET #users with unknown group ID' do
+ before do
+ get(:users, group_id: 'unknown')
+ end
+
+ it { expect(response).to have_http_status(404) }
+ end
end
- let(:body) { JSON.parse(response.body) }
+ context 'non-member login for public project' do
+ let!(:project) { create(:project, :public) }
- describe 'GET #users with group ID' do
before do
- get(:users, group_id: group.id)
+ sign_in(non_member)
+ project.team << [user, :master]
end
- it { expect(body).to be_kind_of(Array) }
- it { expect(body.size).to eq 1 }
- it { expect(body.first["username"]).to eq user.username }
+ let(:body) { JSON.parse(response.body) }
+
+ describe 'GET #users with project ID' do
+ before do
+ get(:users, project_id: project.id, current_user: true)
+ end
+
+ it { expect(body).to be_kind_of(Array) }
+ it { expect(body.size).to eq 2 }
+ it { expect(body.map { |u| u['username'] }).to match_array([user.username, non_member.username]) }
+ end
end
- describe 'GET #users with unknown group ID' do
+ context 'all users' do
before do
- get(:users, group_id: 'unknown')
+ sign_in(user)
+ get(:users)
end
- it { expect(response).to have_http_status(404) }
+ let(:body) { JSON.parse(response.body) }
+
+ it { expect(body).to be_kind_of(Array) }
+ it { expect(body.size).to eq User.count }
end
- end
- context 'non-member login for public project' do
- let!(:project) { create(:project, :public) }
+ context 'unauthenticated user' do
+ let(:public_project) { create(:project, :public) }
+ let(:body) { JSON.parse(response.body) }
- before do
- sign_in(non_member)
- project.team << [user, :master]
- end
+ describe 'GET #users with public project' do
+ before do
+ public_project.team << [user, :guest]
+ get(:users, project_id: public_project.id)
+ end
+
+ it { expect(body).to be_kind_of(Array) }
+ it { expect(body.size).to eq 1 }
+ end
- let(:body) { JSON.parse(response.body) }
+ describe 'GET #users with project' do
+ before do
+ get(:users, project_id: project.id)
+ end
- describe 'GET #users with project ID' do
- before do
- get(:users, project_id: project.id, current_user: true)
+ it { expect(response).to have_http_status(404) }
end
- it { expect(body).to be_kind_of(Array) }
- it { expect(body.size).to eq 2 }
- it { expect(body.map { |u| u['username'] }).to match_array([user.username, non_member.username]) }
- end
- end
+ describe 'GET #users with unknown project' do
+ before do
+ get(:users, project_id: 'unknown')
+ end
- context 'all users' do
- before do
- sign_in(user)
- get(:users)
- end
+ it { expect(response).to have_http_status(404) }
+ end
- let(:body) { JSON.parse(response.body) }
+ describe 'GET #users with inaccessible group' do
+ before do
+ project.team << [user, :guest]
+ get(:users, group_id: user.namespace.id)
+ end
- it { expect(body).to be_kind_of(Array) }
- it { expect(body.size).to eq User.count }
- end
+ it { expect(response).to have_http_status(404) }
+ end
+
+ describe 'GET #users with no project' do
+ before do
+ get(:users)
+ end
- context 'unauthenticated user' do
- let(:public_project) { create(:project, :public) }
- let(:body) { JSON.parse(response.body) }
+ it { expect(body).to be_kind_of(Array) }
+ it { expect(body.size).to eq 0 }
+ end
+ end
- describe 'GET #users with public project' do
+ context 'author of issuable included' do
before do
- public_project.team << [user, :guest]
- get(:users, project_id: public_project.id)
+ sign_in(user)
end
- it { expect(body).to be_kind_of(Array) }
- it { expect(body.size).to eq 1 }
+ let(:body) { JSON.parse(response.body) }
+
+ it 'includes the author' do
+ get(:users, author_id: non_member.id)
+
+ expect(body.first["username"]).to eq non_member.username
+ end
+
+ it 'rejects non existent user ids' do
+ get(:users, author_id: 99999)
+
+ expect(body.collect { |u| u['id'] }).not_to include(99999)
+ end
end
- describe 'GET #users with project' do
- before do
- get(:users, project_id: project.id)
+ context 'skip_users parameter included' do
+ before { sign_in(user) }
+
+ it 'skips the user IDs passed' do
+ get(:users, skip_users: [user, user2].map(&:id))
+
+ other_user_ids = [non_member, project.owner, project.creator].map(&:id)
+ response_user_ids = JSON.parse(response.body).map { |user| user['id'] }
+
+ expect(response_user_ids).to contain_exactly(*other_user_ids)
end
+ end
+ end
+
+ context 'projects' do
+ let(:authorized_project) { create(:project) }
+ let(:authorized_search_project) { create(:project, name: 'rugged') }
- it { expect(response).to have_http_status(404) }
+ before do
+ sign_in(user)
+ project.team << [user, :master]
end
- describe 'GET #users with unknown project' do
+ context 'authorized projects' do
before do
- get(:users, project_id: 'unknown')
+ authorized_project.team << [user, :master]
end
- it { expect(response).to have_http_status(404) }
+ 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 2
+
+ expect(body.first['id']).to eq 0
+ expect(body.first['name_with_namespace']).to eq 'No project'
+
+ expect(body.last['id']).to eq authorized_project.id
+ expect(body.last['name_with_namespace']).to eq authorized_project.name_with_namespace
+ end
+ end
end
- describe 'GET #users with inaccessible group' do
+ context 'authorized projects and search' do
before do
- project.team << [user, :guest]
- get(:users, group_id: user.namespace.id)
+ authorized_project.team << [user, :master]
+ authorized_search_project.team << [user, :master]
end
- it { expect(response).to have_http_status(404) }
+ describe 'GET #projects with project ID and search' do
+ before do
+ get(:projects, project_id: project.id, search: 'rugged')
+ end
+
+ let(:body) { JSON.parse(response.body) }
+
+ it do
+ expect(body).to be_kind_of(Array)
+ expect(body.size).to eq 2
+
+ expect(body.last['id']).to eq authorized_search_project.id
+ expect(body.last['name_with_namespace']).to eq authorized_search_project.name_with_namespace
+ end
+ end
end
- describe 'GET #users with no project' do
+ context 'authorized projects apply limit' do
before do
- get(:users)
+ 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
- it { expect(body).to be_kind_of(Array) }
- it { expect(body.size).to eq 0 }
- end
- end
+ describe 'GET #projects with project ID' do
+ before do
+ get(:projects, project_id: project.id)
+ end
- context 'author of issuable included' do
- before do
- sign_in(user)
+ 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
- let(:body) { JSON.parse(response.body) }
+ context 'authorized projects with offset' do
+ before do
+ authorized_project2 = create(:project)
+ authorized_project3 = create(:project)
- it 'includes the author' do
- get(:users, author_id: non_member.id)
+ authorized_project.team << [user, :master]
+ authorized_project2.team << [user, :master]
+ authorized_project3.team << [user, :master]
+ end
- expect(body.first["username"]).to eq non_member.username
- 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
- it 'rejects non existent user ids' do
- get(:users, author_id: 99999)
+ let(:body) { JSON.parse(response.body) }
- expect(body.collect { |u| u['id'] }).not_to include(99999)
+ 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
- end
- context 'skip_users parameter included' do
- before { sign_in(user) }
+ context 'authorized projects without admin_issue ability' do
+ before(:each) do
+ authorized_project.team << [user, :guest]
+
+ expect(user.can?(:admin_issue, authorized_project)).to eq(false)
+ end
+
+ describe 'GET #projects with project ID' do
+ before do
+ get(:projects, project_id: project.id)
+ end
- it 'skips the user IDs passed' do
- get(:users, skip_users: [user, user2].map(&:id))
+ let(:body) { JSON.parse(response.body) }
- other_user_ids = [non_member, project.owner, project.creator].map(&:id)
- response_user_ids = JSON.parse(response.body).map { |user| user['id'] }
+ it do
+ expect(body).to be_kind_of(Array)
+ expect(body.size).to eq 1 # 'No project'
- expect(response_user_ids).to contain_exactly(*other_user_ids)
+ expect(body.first['id']).to eq 0
+ end
+ end
end
end
end