Commit 3b37758c authored by Brett Walker's avatar Brett Walker

Fixed spec and other small cleanup

parent dadadbb3
......@@ -20,10 +20,14 @@ module Banzai
milestone_names = ids.map {|y| y[:milestone_name]}.compact
unless milestone_names.empty?
milestone_relation = find_milestones(parent, false).where(title: milestone_names)
milestone_relation = find_milestones(parent, false).where(name: milestone_names)
end
Milestone.from_union([iid_relation, milestone_relation].compact).includes(:project, :group)
if (relation = [iid_relation, milestone_relation].compact).empty?
Milestone.none
else
Milestone.from_union(relation).includes(:project, :group)
end
end
# Links to project milestones contain the IID, but when we're handling
......@@ -42,8 +46,8 @@ module Banzai
# This method has the contract that if a string `ref` refers to a
# record `record`, then `parse_symbol(ref) == record_identifier(record)`.
#
# This contract is slightly broken here, as we only have either the label_id
# or the label_name, but not both. But below, we have both pieces of information.
# This contract is slightly broken here, as we only have either the milestone_id
# or the milestone_name, but not both. But below, we have both pieces of information.
# But it's accounted for in `find_object`
def parse_symbol(symbol, match_data)
if symbol
......@@ -61,7 +65,7 @@ module Banzai
# record `record`, then `class.parse_symbol(ref) == record_identifier(record)`.
# See note in `parse_symbol` above
def record_identifier(record)
{ milestone_iid: record.iid, milestone_name: record.title }
{ milestone_iid: record.iid, milestone_name: record.name }
end
# def find_object_from_link(parent, iid)
......
......@@ -474,8 +474,8 @@ RSpec.describe Banzai::Filter::References::MilestoneReferenceFilter do
let_it_be(:group2_milestone) { create(:milestone, group: group2) }
let_it_be(:project_reference) { "#{project_milestone.to_reference}" }
let_it_be(:project_reference2) { "#{project_milestone2.to_reference}" }
let_it_be(:project2_reference) { "#{project2_milestone.to_reference}" }
let_it_be(:group2_reference) { "#{project2.full_path}%#{group2_milestone.name}" }
let_it_be(:project2_reference) { "#{project2_milestone.to_reference(full: true)}" }
let_it_be(:group2_reference) { "#{project2.full_path}%\"#{group2_milestone.name}\"" }
it 'does not have N+1 per multiple references per project', :use_sql_query_cache do
markdown = "#{project_reference}"
......@@ -500,40 +500,37 @@ RSpec.describe Banzai::Filter::References::MilestoneReferenceFilter do
reference_filter(markdown, project: project)
end.not_to exceed_all_query_limit(control_count)
# Since we're not batching label queries across projects/groups,
# Since we're not batching milestone queries across projects/groups,
# queries increase when a new project/group is added.
# TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/330359
# first reference to already loaded project (1),
# second reference requires project and namespace (2), and milestone (1)
markdown = "#{project_reference} #{group2_reference}"
max_count = control_count + 3
control_count += 5
expect do
reference_filter(markdown)
end.not_to exceed_all_query_limit(max_count)
end.not_to exceed_all_query_limit(control_count)
# third reference to already queried project/namespace, nothing extra (no N+1 here)
markdown = "#{project_reference} #{group2_reference} #{project2_reference}"
markdown = "#{project_reference} #{group2_reference} #{project_reference2}"
expect do
reference_filter(markdown)
end.not_to exceed_all_query_limit(max_count)
end.not_to exceed_all_query_limit(control_count)
# last reference needs another namespace and label query (2)
# last reference needs additional queries
markdown = "#{project_reference} #{group2_reference} #{project2_reference} #{project3.full_path}%test_milestone"
max_count += 2
control_count += 6
expect do
reference_filter(markdown)
end.not_to exceed_all_query_limit(max_count)
end.not_to exceed_all_query_limit(control_count)
# Use an iid instead of title reference
markdown = "#{project_reference} #{group2_reference} #{project2.full_path}%#{project2_milestone.iid} #{project3.full_path}%test_milestone"
max_count += 4
expect do
reference_filter(markdown)
end.not_to exceed_all_query_limit(max_count)
end.not_to exceed_all_query_limit(control_count)
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