Commit 729636bd authored by Sean McGivern's avatar Sean McGivern

Merge branch 'speed-up-group-milestone-index' into 'master'

Speed up group milestone index by passing group_id to IssuesFinder

See merge request !8363
parents e553c3b4 f13c650c
module GlobalMilestones
extend ActiveSupport::Concern
def milestones
epoch = DateTime.parse('1970-01-01')
@milestones = MilestonesFinder.new.execute(@projects, params)
@milestones = GlobalMilestone.build_collection(@milestones)
@milestones = @milestones.sort_by { |x| x.due_date.nil? ? epoch : x.due_date }
end
def milestone
milestones = Milestone.of_projects(@projects).where(title: params[:title])
if milestones.present?
@milestone = GlobalMilestone.new(params[:title], milestones)
else
render_404
end
end
end
class Dashboard::MilestonesController < Dashboard::ApplicationController class Dashboard::MilestonesController < Dashboard::ApplicationController
include GlobalMilestones
before_action :projects before_action :projects
before_action :milestone, only: [:show] before_action :milestone, only: [:show]
...@@ -17,4 +15,15 @@ class Dashboard::MilestonesController < Dashboard::ApplicationController ...@@ -17,4 +15,15 @@ class Dashboard::MilestonesController < Dashboard::ApplicationController
def show def show
end end
private
def milestones
@milestones = GlobalMilestone.build_collection(@projects, params)
end
def milestone
@milestone = GlobalMilestone.build(@projects, params[:title])
render_404 unless @milestone
end
end end
class Groups::MilestonesController < Groups::ApplicationController class Groups::MilestonesController < Groups::ApplicationController
include GlobalMilestones
before_action :group_projects before_action :group_projects
before_action :milestone, only: [:show, :update] before_action :milestone, only: [:show, :update]
before_action :authorize_admin_milestones!, only: [:new, :create, :update] before_action :authorize_admin_milestones!, only: [:new, :create, :update]
...@@ -73,4 +71,13 @@ class Groups::MilestonesController < Groups::ApplicationController ...@@ -73,4 +71,13 @@ class Groups::MilestonesController < Groups::ApplicationController
def milestone_path(title) def milestone_path(title)
group_milestone_path(@group, title.to_slug.to_s, title: title) group_milestone_path(@group, title.to_slug.to_s, title: title)
end end
def milestones
@milestones = GroupMilestone.build_collection(@group, @projects, params)
end
def milestone
@milestone = GroupMilestone.build(@group, @projects, params[:title])
render_404 unless @milestone
end
end end
...@@ -36,8 +36,8 @@ module Milestoneish ...@@ -36,8 +36,8 @@ module Milestoneish
def issues_visible_to_user(user) def issues_visible_to_user(user)
memoize_per_user(user, :issues_visible_to_user) do memoize_per_user(user, :issues_visible_to_user) do
params = try(:project_id) ? { project_id: project_id } : {} IssuesFinder.new(user, issues_finder_params)
IssuesFinder.new(user, params).execute.where(milestone_id: milestoneish_ids) .execute.where(milestone_id: milestoneish_ids)
end end
end end
...@@ -72,4 +72,10 @@ module Milestoneish ...@@ -72,4 +72,10 @@ module Milestoneish
@memoized[method_name] ||= {} @memoized[method_name] ||= {}
@memoized[method_name][user.try!(:id)] ||= yield @memoized[method_name][user.try!(:id)] ||= yield
end end
# override in a class that includes this module to get a faster query
# from IssuesFinder
def issues_finder_params
{}
end
end end
class GlobalMilestone class GlobalMilestone
include Milestoneish include Milestoneish
EPOCH = DateTime.parse('1970-01-01')
attr_accessor :title, :milestones attr_accessor :title, :milestones
alias_attribute :name, :title alias_attribute :name, :title
...@@ -8,13 +10,22 @@ class GlobalMilestone ...@@ -8,13 +10,22 @@ class GlobalMilestone
@first_milestone @first_milestone
end end
def self.build_collection(milestones) def self.build_collection(projects, params)
milestones = milestones.group_by(&:title) child_milestones = MilestonesFinder.new.execute(projects, params)
milestones.map do |title, milestones| milestones = child_milestones.select(:id, :title).group_by(&:title).map do |title, grouped|
milestones_relation = Milestone.where(id: milestones.map(&:id)) milestones_relation = Milestone.where(id: grouped.map(&:id))
new(title, milestones_relation) new(title, milestones_relation)
end end
milestones.sort_by { |milestone| milestone.due_date || EPOCH }
end
def self.build(projects, title)
child_milestones = Milestone.of_projects(projects).where(title: title)
return if child_milestones.blank?
new(title, child_milestones)
end end
def initialize(title, milestones) def initialize(title, milestones)
......
class GroupMilestone < GlobalMilestone
attr_accessor :group
def self.build_collection(group, projects, params)
super(projects, params).each do |milestone|
milestone.group = group
end
end
def self.build(group, projects, title)
super(projects, title).tap do |milestone|
milestone.group = group if milestone
end
end
def issues_finder_params
{ group_id: group.id }
end
end
...@@ -202,4 +202,8 @@ class Milestone < ActiveRecord::Base ...@@ -202,4 +202,8 @@ class Milestone < ActiveRecord::Base
errors.add(:start_date, "Can't be greater than due date") errors.add(:start_date, "Can't be greater than due date")
end end
end end
def issues_finder_params
{ project_id: project_id }
end
end end
---
title: Speed up group milestone index by passing group_id to IssuesFinder
merge_request: 8363
author:
...@@ -7,26 +7,72 @@ describe GlobalMilestone, models: true do ...@@ -7,26 +7,72 @@ describe GlobalMilestone, models: true do
let(:project1) { create(:project, group: group) } let(:project1) { create(:project, group: group) }
let(:project2) { create(:project, path: 'gitlab-ci', group: group) } let(:project2) { create(:project, path: 'gitlab-ci', group: group) }
let(:project3) { create(:project, path: 'cookbook-gitlab', group: group) } let(:project3) { create(:project, path: 'cookbook-gitlab', group: group) }
let(:milestone1_project1) { create(:milestone, title: "Milestone v1.2", project: project1) }
let(:milestone1_project2) { create(:milestone, title: "Milestone v1.2", project: project2) }
let(:milestone1_project3) { create(:milestone, title: "Milestone v1.2", project: project3) }
let(:milestone2_project1) { create(:milestone, title: "VD-123", project: project1) }
let(:milestone2_project2) { create(:milestone, title: "VD-123", project: project2) }
let(:milestone2_project3) { create(:milestone, title: "VD-123", project: project3) }
describe '.build_collection' do describe '.build_collection' do
let(:milestone1_due_date) { 2.weeks.from_now.to_date }
let!(:milestone1_project1) do
create(
:milestone,
title: "Milestone v1.2",
project: project1,
due_date: milestone1_due_date
)
end
let!(:milestone1_project2) do
create(
:milestone,
title: "Milestone v1.2",
project: project2,
due_date: milestone1_due_date
)
end
let!(:milestone1_project3) do
create(
:milestone,
title: "Milestone v1.2",
project: project3,
due_date: milestone1_due_date
)
end
let!(:milestone2_project1) do
create(
:milestone,
title: "VD-123",
project: project1,
due_date: nil
)
end
let!(:milestone2_project2) do
create(
:milestone,
title: "VD-123",
project: project2,
due_date: nil
)
end
let!(:milestone2_project3) do
create(
:milestone,
title: "VD-123",
project: project3,
due_date: nil
)
end
before do before do
milestones = projects = [
[ project1,
milestone1_project1, project2,
milestone1_project2, project3
milestone1_project3,
milestone2_project1,
milestone2_project2,
milestone2_project3
] ]
@global_milestones = GlobalMilestone.build_collection(milestones) @global_milestones = GlobalMilestone.build_collection(projects, {})
end end
it 'has all project milestones' do it 'has all project milestones' do
...@@ -40,9 +86,17 @@ describe GlobalMilestone, models: true do ...@@ -40,9 +86,17 @@ describe GlobalMilestone, models: true do
it 'has all project milestones' do it 'has all project milestones' do
expect(@global_milestones.map { |group_milestone| group_milestone.milestones.count }.sum).to eq(6) expect(@global_milestones.map { |group_milestone| group_milestone.milestones.count }.sum).to eq(6)
end end
it 'sorts collection by due date' do
expect(@global_milestones.map(&:due_date)).to eq [nil, milestone1_due_date]
end
end end
describe '#initialize' do describe '#initialize' do
let(:milestone1_project1) { create(:milestone, title: "Milestone v1.2", project: project1) }
let(:milestone1_project2) { create(:milestone, title: "Milestone v1.2", project: project2) }
let(:milestone1_project3) { create(:milestone, title: "Milestone v1.2", project: project3) }
before do before do
milestones = milestones =
[ [
......
require 'spec_helper'
describe GroupMilestone, models: true do
let(:group) { create(:group) }
let(:project) { create(:project, group: group) }
let(:project_milestone) do
create(:milestone, title: "Milestone v1.2", project: project)
end
describe '.build' do
it 'returns milestone with group assigned' do
milestone = GroupMilestone.build(
group,
[project],
project_milestone.title
)
expect(milestone.group).to eq group
end
end
describe '.build_collection' do
before do
project_milestone
end
it 'returns array of milestones, each with group assigned' do
milestones = GroupMilestone.build_collection(group, [project], {})
expect(milestones).to all(have_attributes(group: group))
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