Commit 5500f915 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

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
parents 1ede18bf 19a0db30
...@@ -286,7 +286,7 @@ class ApplicationController < ActionController::Base ...@@ -286,7 +286,7 @@ class ApplicationController < ActionController::Base
end end
def set_filters_params def set_filters_params
params[:sort] ||= 'created_desc' params[:sort] ||= 'id_desc'
params[:scope] = 'all' if params[:scope].blank? params[:scope] = 'all' if params[:scope].blank?
params[:state] = 'opened' if params[:state].blank? params[:state] = 'opened' if params[:state].blank?
......
...@@ -79,9 +79,9 @@ class IssuableFinder ...@@ -79,9 +79,9 @@ class IssuableFinder
if project? if project?
@projects = project @projects = project
elsif current_user && params[:authorized_only].presence && !current_user_related? elsif current_user && params[:authorized_only].presence && !current_user_related?
@projects = current_user.authorized_projects @projects = current_user.authorized_projects.reorder(nil)
else else
@projects = ProjectsFinder.new.execute(current_user) @projects = ProjectsFinder.new.execute(current_user).reorder(nil)
end end
end end
......
...@@ -63,11 +63,11 @@ module SortingHelper ...@@ -63,11 +63,11 @@ module SortingHelper
end end
def sort_value_oldest_created def sort_value_oldest_created
'created_asc' 'id_asc'
end end
def sort_value_recently_created def sort_value_recently_created
'created_desc' 'id_desc'
end end
def sort_value_milestone_soon def sort_value_milestone_soon
......
...@@ -11,6 +11,7 @@ module Sortable ...@@ -11,6 +11,7 @@ module Sortable
default_scope { order_id_desc } default_scope { order_id_desc }
scope :order_id_desc, -> { reorder(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_desc, -> { reorder(created_at: :desc) }
scope :order_created_asc, -> { reorder(created_at: :asc) } scope :order_created_asc, -> { reorder(created_at: :asc) }
scope :order_updated_desc, -> { reorder(updated_at: :desc) } scope :order_updated_desc, -> { reorder(updated_at: :desc) }
...@@ -28,6 +29,8 @@ module Sortable ...@@ -28,6 +29,8 @@ module Sortable
when 'updated_desc' then order_updated_desc when 'updated_desc' then order_updated_desc
when 'created_asc' then order_created_asc when 'created_asc' then order_created_asc
when 'created_desc' then order_created_desc when 'created_desc' then order_created_desc
when 'id_desc' then order_id_desc
when 'id_asc' then order_id_asc
else else
all all
end end
......
...@@ -33,7 +33,9 @@ class Issue < ActiveRecord::Base ...@@ -33,7 +33,9 @@ class Issue < ActiveRecord::Base
belongs_to :project belongs_to :project
validates :project, presence: true 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 :cared, ->(user) { where(assignee_id: user) }
scope :open_for, ->(user) { opened.assigned_to(user) } scope :open_for, ->(user) { opened.assigned_to(user) }
......
...@@ -131,7 +131,7 @@ class MergeRequest < ActiveRecord::Base ...@@ -131,7 +131,7 @@ class MergeRequest < ActiveRecord::Base
validate :validate_branches validate :validate_branches
validate :validate_fork 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 :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 :cared, ->(user) { where('assignee_id = :user OR author_id = :user', user: user.id) }
scope :by_milestone, ->(milestone) { where(milestone_id: milestone) } scope :by_milestone, ->(milestone) { where(milestone_id: milestone) }
......
class AddIndexMilestonesTitle < ActiveRecord::Migration
def change
add_index :milestones, :title
end
end
...@@ -127,15 +127,15 @@ describe 'Issues', feature: true do ...@@ -127,15 +127,15 @@ describe 'Issues', feature: true do
it 'sorts by newest' do it 'sorts by newest' do
visit namespace_project_issues_path(project.namespace, project, sort: sort_value_recently_created) visit namespace_project_issues_path(project.namespace, project, sort: sort_value_recently_created)
expect(first_issue).to include('foo') expect(first_issue).to include('baz')
expect(last_issue).to include('baz') expect(last_issue).to include('foo')
end end
it 'sorts by oldest' do it 'sorts by oldest' do
visit namespace_project_issues_path(project.namespace, project, sort: sort_value_oldest_created) visit namespace_project_issues_path(project.namespace, project, sort: sort_value_oldest_created)
expect(first_issue).to include('baz') expect(first_issue).to include('foo')
expect(last_issue).to include('foo') expect(last_issue).to include('baz')
end end
it 'sorts by most recently updated' do it 'sorts by most recently updated' do
...@@ -190,8 +190,8 @@ describe 'Issues', feature: true do ...@@ -190,8 +190,8 @@ describe 'Issues', feature: true do
sort: sort_value_oldest_created, sort: sort_value_oldest_created,
assignee_id: user2.id) assignee_id: user2.id)
expect(first_issue).to include('bar') expect(first_issue).to include('foo')
expect(last_issue).to include('foo') expect(last_issue).to include('bar')
expect(page).not_to have_content 'baz' expect(page).not_to have_content 'baz'
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment