Skip to content

Commit 2f76ff1

Browse files
committed
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`
1 parent 6b2f81f commit 2f76ff1

File tree

9 files changed

+21
-23
lines changed

9 files changed

+21
-23
lines changed

app/controllers/projects/milestones_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ def milestone_params
144144

145145
def search_params
146146
if request.format.json? && project_group && can?(current_user, :read_group, project_group)
147-
groups = project_group.self_and_ancestors_ids
147+
groups = project_group.self_and_ancestors.select(:id)
148148
end
149149

150150
params.permit(:state).merge(project_ids: @project.id, group_ids: groups)

app/finders/milestones_finder.rb

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,26 +3,22 @@
33
# Search for milestones
44
#
55
# params - Hash
6-
# project_ids: Array of project ids or single project id.
7-
# group_ids: Array of group ids or single group id.
6+
# project_ids: Array of project ids or single project id or ActiveRecord relation.
7+
# group_ids: Array of group ids or single group id or ActiveRecord relation.
88
# order - Orders by field default due date asc.
99
# title - filter by title.
1010
# state - filters by state.
1111

1212
class MilestonesFinder
1313
include FinderMethods
1414

15-
attr_reader :params, :project_ids, :group_ids
15+
attr_reader :params
1616

1717
def initialize(params = {})
18-
@project_ids = Array(params[:project_ids])
19-
@group_ids = Array(params[:group_ids])
2018
@params = params
2119
end
2220

2321
def execute
24-
return Milestone.none if project_ids.empty? && group_ids.empty?
25-
2622
items = Milestone.all
2723
items = by_groups_and_projects(items)
2824
items = by_title(items)
@@ -34,7 +30,7 @@ def execute
3430
private
3531

3632
def by_groups_and_projects(items)
37-
items.for_projects_and_groups(project_ids, group_ids)
33+
items.for_projects_and_groups(params[:project_ids], params[:group_ids])
3834
end
3935

4036
# rubocop: disable CodeReuse/ActiveRecord

app/models/milestone.rb

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class Milestone < ActiveRecord::Base
4545
groups = groups.compact if groups.is_a? Array
4646
groups = [] if groups.nil?
4747

48-
where(project: projects).or(where(group: groups))
48+
where(project_id: projects).or(where(group_id: groups))
4949
end
5050

5151
scope :order_by_name_asc, -> { order(Arel::Nodes::Ascending.new(arel_table[:title].lower)) }
@@ -191,7 +191,7 @@ def self.states_count(projects, groups = nil)
191191
return STATE_COUNT_HASH unless projects || groups
192192

193193
counts = Milestone
194-
.for_projects_and_groups(projects&.map(&:id), groups&.map(&:id))
194+
.for_projects_and_groups(projects, groups)
195195
.reorder(nil)
196196
.group(:state)
197197
.count
@@ -275,8 +275,7 @@ def uniqueness_of_title
275275
if project
276276
relation = Milestone.for_projects_and_groups([project_id], [project.group&.id])
277277
elsif group
278-
project_ids = group.projects.map(&:id)
279-
relation = Milestone.for_projects_and_groups(project_ids, [group.id])
278+
relation = Milestone.for_projects_and_groups(group.projects.select(:id), [group.id])
280279
end
281280

282281
title_exists = relation.find_by_title(title)

app/services/issuable_base_service.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,10 @@ def filter_milestone
6161
return unless milestone_id
6262

6363
params[:milestone_id] = '' if milestone_id == IssuableFinder::NONE
64-
group_ids = project.group&.self_and_ancestors&.pluck(:id)
64+
groups = project.group&.self_and_ancestors&.select(:id)
6565

6666
milestone =
67-
Milestone.for_projects_and_groups([project.id], group_ids).find_by_id(milestone_id)
67+
Milestone.for_projects_and_groups([project.id], groups).find_by_id(milestone_id)
6868

6969
params[:milestone_id] = '' unless milestone
7070
end

app/services/milestones/promote_service.rb

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,9 @@ def destroy_old_milestones(milestone)
8282
end
8383
# rubocop: enable CodeReuse/ActiveRecord
8484

85-
# rubocop: disable CodeReuse/ActiveRecord
8685
def group_project_ids
87-
@group_project_ids ||= group.projects.pluck(:id)
86+
group.projects.select(:id)
8887
end
89-
# rubocop: enable CodeReuse/ActiveRecord
9088

9189
def raise_error(message)
9290
raise PromoteMilestoneError, "Promotion failed - #{message}"

app/services/projects/autocomplete_service.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ def milestones
1414
order: { due_date: :asc, title: :asc }
1515
}
1616

17-
finder_params[:group_ids] = @project.group.self_and_ancestors_ids if @project.group
17+
finder_params[:group_ids] = @project.group.self_and_ancestors.select(:id) if @project.group
1818

1919
MilestonesFinder.new(finder_params).execute.select([:iid, :title])
2020
end
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
title: Improve milestone queries using subqueries instead of separate queries for ids
3+
merge_request: 24325
4+
author:
5+
type: performance

lib/banzai/filter/milestone_reference_filter.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,9 @@ def milestone_finder_params(parent, find_by_iid)
101101

102102
def self_and_ancestors_ids(parent)
103103
if group_context?(parent)
104-
parent.self_and_ancestors_ids
104+
parent.self_and_ancestors.select(:id)
105105
elsif project_context?(parent)
106-
parent.group&.self_and_ancestors_ids
106+
parent.group&.self_and_ancestors&.select(:id)
107107
end
108108
end
109109

spec/models/milestone_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,8 @@
286286
end
287287

288288
context 'relations as params' do
289-
let(:projects) { Project.where(id: project.id) }
290-
let(:groups) { Group.where(id: group.id) }
289+
let(:projects) { Project.where(id: project.id).select(:id) }
290+
let(:groups) { Group.where(id: group.id).select(:id) }
291291

292292
it_behaves_like 'filters by projects and groups'
293293
end

0 commit comments

Comments
 (0)