Move to project dropdown with infinite scroll for better performance17932-move-to-project-dropdown

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

7 files changed, 101 insertions, 7 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 61c654002ad..a3186a8afb9 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -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
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/finders/move_to_project_finder.rb b/app/finders/move_to_project_finder.rb
index 3334b8556df..79eb45568be 100644
--- a/app/finders/move_to_project_finder.rb
+++ b/app/finders/move_to_project_finder.rb
@@ -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
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 9e2e096d5f9..901351f2620 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -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.' }
diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb
index 44128a43362..a121cb2fc97 100644
--- a/spec/controllers/autocomplete_controller_spec.rb
+++ b/spec/controllers/autocomplete_controller_spec.rb
@@ -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]
diff --git a/spec/finders/move_to_project_finder_spec.rb b/spec/finders/move_to_project_finder_spec.rb
index 4f3304f7b6d..fdce4e714ff 100644
--- a/spec/finders/move_to_project_finder_spec.rb
+++ b/spec/finders/move_to_project_finder_spec.rb
@@ -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