Fix MilestonesFinder to pass relations to scope

Instead of querying relations into ids we just pass them to the model
scope because the scope supports it now.

Also changes other calls to `Milestone.for_projects_and_groups`
This commit is contained in:
Heinrich Lee Yu 2019-01-11 22:58:18 +08:00
parent 6b2f81f607
commit 2f76ff19f8
9 changed files with 21 additions and 23 deletions

View file

@ -144,7 +144,7 @@ class Projects::MilestonesController < Projects::ApplicationController
def search_params
if request.format.json? && project_group && can?(current_user, :read_group, project_group)
groups = project_group.self_and_ancestors_ids
groups = project_group.self_and_ancestors.select(:id)
end
params.permit(:state).merge(project_ids: @project.id, group_ids: groups)

View file

@ -3,8 +3,8 @@
# Search for milestones
#
# params - Hash
# project_ids: Array of project ids or single project id.
# group_ids: Array of group ids or single group id.
# project_ids: Array of project ids or single project id or ActiveRecord relation.
# group_ids: Array of group ids or single group id or ActiveRecord relation.
# order - Orders by field default due date asc.
# title - filter by title.
# state - filters by state.
@ -12,17 +12,13 @@
class MilestonesFinder
include FinderMethods
attr_reader :params, :project_ids, :group_ids
attr_reader :params
def initialize(params = {})
@project_ids = Array(params[:project_ids])
@group_ids = Array(params[:group_ids])
@params = params
end
def execute
return Milestone.none if project_ids.empty? && group_ids.empty?
items = Milestone.all
items = by_groups_and_projects(items)
items = by_title(items)
@ -34,7 +30,7 @@ class MilestonesFinder
private
def by_groups_and_projects(items)
items.for_projects_and_groups(project_ids, group_ids)
items.for_projects_and_groups(params[:project_ids], params[:group_ids])
end
# rubocop: disable CodeReuse/ActiveRecord

View file

@ -45,7 +45,7 @@ class Milestone < ActiveRecord::Base
groups = groups.compact if groups.is_a? Array
groups = [] if groups.nil?
where(project: projects).or(where(group: groups))
where(project_id: projects).or(where(group_id: groups))
end
scope :order_by_name_asc, -> { order(Arel::Nodes::Ascending.new(arel_table[:title].lower)) }
@ -191,7 +191,7 @@ class Milestone < ActiveRecord::Base
return STATE_COUNT_HASH unless projects || groups
counts = Milestone
.for_projects_and_groups(projects&.map(&:id), groups&.map(&:id))
.for_projects_and_groups(projects, groups)
.reorder(nil)
.group(:state)
.count
@ -275,8 +275,7 @@ class Milestone < ActiveRecord::Base
if project
relation = Milestone.for_projects_and_groups([project_id], [project.group&.id])
elsif group
project_ids = group.projects.map(&:id)
relation = Milestone.for_projects_and_groups(project_ids, [group.id])
relation = Milestone.for_projects_and_groups(group.projects.select(:id), [group.id])
end
title_exists = relation.find_by_title(title)

View file

@ -61,10 +61,10 @@ class IssuableBaseService < BaseService
return unless milestone_id
params[:milestone_id] = '' if milestone_id == IssuableFinder::NONE
group_ids = project.group&.self_and_ancestors&.pluck(:id)
groups = project.group&.self_and_ancestors&.select(:id)
milestone =
Milestone.for_projects_and_groups([project.id], group_ids).find_by_id(milestone_id)
Milestone.for_projects_and_groups([project.id], groups).find_by_id(milestone_id)
params[:milestone_id] = '' unless milestone
end

View file

@ -82,11 +82,9 @@ module Milestones
end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def group_project_ids
@group_project_ids ||= group.projects.pluck(:id)
group.projects.select(:id)
end
# rubocop: enable CodeReuse/ActiveRecord
def raise_error(message)
raise PromoteMilestoneError, "Promotion failed - #{message}"

View file

@ -14,7 +14,7 @@ module Projects
order: { due_date: :asc, title: :asc }
}
finder_params[:group_ids] = @project.group.self_and_ancestors_ids if @project.group
finder_params[:group_ids] = @project.group.self_and_ancestors.select(:id) if @project.group
MilestonesFinder.new(finder_params).execute.select([:iid, :title])
end

View file

@ -0,0 +1,5 @@
---
title: Improve milestone queries using subqueries instead of separate queries for ids
merge_request: 24325
author:
type: performance

View file

@ -101,9 +101,9 @@ module Banzai
def self_and_ancestors_ids(parent)
if group_context?(parent)
parent.self_and_ancestors_ids
parent.self_and_ancestors.select(:id)
elsif project_context?(parent)
parent.group&.self_and_ancestors_ids
parent.group&.self_and_ancestors&.select(:id)
end
end

View file

@ -286,8 +286,8 @@ describe Milestone do
end
context 'relations as params' do
let(:projects) { Project.where(id: project.id) }
let(:groups) { Group.where(id: group.id) }
let(:projects) { Project.where(id: project.id).select(:id) }
let(:groups) { Group.where(id: group.id).select(:id) }
it_behaves_like 'filters by projects and groups'
end