diff options
author | Paco Guzman <pacoguzmanp@gmail.com> | 2016-08-05 15:29:20 +0200 |
---|---|---|
committer | Paco Guzman <pacoguzmanp@gmail.com> | 2016-08-08 19:05:16 +0200 |
commit | b6fe79f48b2fc3ab9d64cffb26b39abd30bcd396 (patch) | |
tree | 48a5cfedf4087db5dadf9c17db392c0ac01fea61 | |
parent | 59ce1af53b8d25d1b4ae8b6e59f069c5147ca572 (diff) | |
download | gitlab-ce-b6fe79f48b2fc3ab9d64cffb26b39abd30bcd396.tar.gz |
Move to project dropdown with infinite scroll for better performance
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/assets/javascripts/issuable_form.js | 24 | ||||
-rw-r--r-- | app/assets/stylesheets/framework/selects.scss | 3 | ||||
-rw-r--r-- | app/controllers/autocomplete_controller.rb | 14 | ||||
-rw-r--r-- | app/finders/move_to_project_finder.rb | 30 | ||||
-rw-r--r-- | app/views/shared/issuable/_form.html.haml | 2 | ||||
-rw-r--r-- | spec/controllers/autocomplete_controller_spec.rb | 338 |
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 } %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 |