Merge branch 'group-issues-sorting' into 'master'

Improve performance of getting issues on group level

For testing I used the URL http://localhost:3000/groups/gitlab-org/issues?milestone_title=8.1. Prior to these changes said URL would take about 10-12 seconds to load. By applying these changes the loading time has been reduced to roughly 2-3 seconds. 

There's still some stuff going on in some views that I have to look at, resolving those changes might reduce the loading time a bit more. I also still have to check if I didn't break too many tests.

Fixes: gitlab-org/gitlab-ce#3707 gitlab-org/gitlab-ce#4071 

See merge request !2318

8 files changed, 23 insertions, 13 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index d9a37a4d45f..81cb1367e2c 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -286,7 +286,7 @@ class ApplicationController < ActionController::Base
   end
 
   def set_filters_params
-    params[:sort] ||= 'created_desc'
+    params[:sort] ||= 'id_desc'
     params[:scope] = 'all' if params[:scope].blank?
     params[:state] = 'opened' if params[:state].blank?
 
diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index 3d5e8b6fbe7..4d56b48e3f8 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -79,9 +79,9 @@ class IssuableFinder
     if project?
       @projects = project
     elsif current_user && params[:authorized_only].presence && !current_user_related?
-      @projects = current_user.authorized_projects
+      @projects = current_user.authorized_projects.reorder(nil)
     else
-      @projects = ProjectsFinder.new.execute(current_user)
+      @projects = ProjectsFinder.new.execute(current_user).reorder(nil)
     end
   end
 
diff --git a/app/helpers/sorting_helper.rb b/app/helpers/sorting_helper.rb
index 99d7df64a83..241179b0212 100644
--- a/app/helpers/sorting_helper.rb
+++ b/app/helpers/sorting_helper.rb
@@ -63,11 +63,11 @@ module SortingHelper
   end
 
   def sort_value_oldest_created
-    'created_asc'
+    'id_asc'
   end
 
   def sort_value_recently_created
-    'created_desc'
+    'id_desc'
   end
 
   def sort_value_milestone_soon
diff --git a/app/models/concerns/sortable.rb b/app/models/concerns/sortable.rb
index 7391a77383c..8b47b9e0abd 100644
--- a/app/models/concerns/sortable.rb
+++ b/app/models/concerns/sortable.rb
@@ -11,6 +11,7 @@ module Sortable
     default_scope { order_id_desc }
 
     scope :order_id_desc, -> { reorder(id: :desc) }
+    scope :order_id_asc, -> { reorder(id: :asc) }
     scope :order_created_desc, -> { reorder(created_at: :desc) }
     scope :order_created_asc, -> { reorder(created_at: :asc) }
     scope :order_updated_desc, -> { reorder(updated_at: :desc) }
@@ -28,6 +29,8 @@ module Sortable
       when 'updated_desc' then order_updated_desc
       when 'created_asc' then order_created_asc
       when 'created_desc' then order_created_desc
+      when 'id_desc' then order_id_desc
+      when 'id_asc' then order_id_asc
       else
         all
       end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 80ecd15077f..f52e47f3e62 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -33,7 +33,9 @@ class Issue < ActiveRecord::Base
   belongs_to :project
   validates :project, presence: true
 
-  scope :of_group, ->(group) { where(project_id: group.project_ids) }
+  scope :of_group,
+    ->(group) { where(project_id: group.projects.select(:id).reorder(nil)) }
+
   scope :cared, ->(user) { where(assignee_id: user) }
   scope :open_for, ->(user) { opened.assigned_to(user) }
 
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 5723ff36e02..c63d0c01653 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -131,7 +131,7 @@ class MergeRequest < ActiveRecord::Base
   validate :validate_branches
   validate :validate_fork
 
-  scope :of_group, ->(group) { where("source_project_id in (:group_project_ids) OR target_project_id in (:group_project_ids)", group_project_ids: group.project_ids) }
+  scope :of_group, ->(group) { where("source_project_id in (:group_project_ids) OR target_project_id in (:group_project_ids)", group_project_ids: group.projects.select(:id).reorder(nil)) }
   scope :by_branch, ->(branch_name) { where("(source_branch LIKE :branch) OR (target_branch LIKE :branch)", branch: branch_name) }
   scope :cared, ->(user) { where('assignee_id = :user OR author_id = :user', user: user.id) }
   scope :by_milestone, ->(milestone) { where(milestone_id: milestone) }
diff --git a/db/migrate/20160106162223_add_index_milestones_title.rb b/db/migrate/20160106162223_add_index_milestones_title.rb
new file mode 100644
index 00000000000..767885e2aac
--- /dev/null
+++ b/db/migrate/20160106162223_add_index_milestones_title.rb
@@ -0,0 +1,5 @@
+class AddIndexMilestonesTitle < ActiveRecord::Migration
+  def change
+    add_index :milestones, :title
+  end
+end
diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb
index a2fb3e4c75d..e844e681ebf 100644
--- a/spec/features/issues_spec.rb
+++ b/spec/features/issues_spec.rb
@@ -127,15 +127,15 @@ describe 'Issues', feature: true do
     it 'sorts by newest' do
       visit namespace_project_issues_path(project.namespace, project, sort: sort_value_recently_created)
 
-      expect(first_issue).to include('foo')
-      expect(last_issue).to include('baz')
+      expect(first_issue).to include('baz')
+      expect(last_issue).to include('foo')
     end
 
     it 'sorts by oldest' do
       visit namespace_project_issues_path(project.namespace, project, sort: sort_value_oldest_created)
 
-      expect(first_issue).to include('baz')
-      expect(last_issue).to include('foo')
+      expect(first_issue).to include('foo')
+      expect(last_issue).to include('baz')
     end
 
     it 'sorts by most recently updated' do
@@ -190,8 +190,8 @@ describe 'Issues', feature: true do
                                             sort: sort_value_oldest_created,
                                             assignee_id: user2.id)
 
-        expect(first_issue).to include('bar')
-        expect(last_issue).to include('foo')
+        expect(first_issue).to include('foo')
+        expect(last_issue).to include('bar')
         expect(page).not_to have_content 'baz'
       end
     end