diff options
| -rw-r--r-- | lib/banzai/filter/milestone_reference_filter.rb | 66 | ||||
| -rw-r--r-- | spec/lib/banzai/filter/milestone_reference_filter_spec.rb | 47 |
2 files changed, 88 insertions, 25 deletions
diff --git a/lib/banzai/filter/milestone_reference_filter.rb b/lib/banzai/filter/milestone_reference_filter.rb index 328c8c1803b..c70c3f0c04e 100644 --- a/lib/banzai/filter/milestone_reference_filter.rb +++ b/lib/banzai/filter/milestone_reference_filter.rb @@ -4,6 +4,8 @@ module Banzai module Filter # HTML filter that replaces milestone references with links. class MilestoneReferenceFilter < AbstractReferenceFilter + include Gitlab::Utils::StrongMemoize + self.reference_type = :milestone def self.object_class @@ -13,16 +15,34 @@ module Banzai # Links to project milestones contain the IID, but when we're handling # 'regular' references, we need to use the global ID to disambiguate # between group and project milestones. - def find_object(project, id) - return unless project.is_a?(Project) + def find_object(parent, id) + return unless valid_context?(parent) - find_milestone_with_finder(project, id: id) + find_milestone_with_finder(parent, id: id) end - def find_object_from_link(project, iid) - return unless project.is_a?(Project) + def find_object_from_link(parent, iid) + return unless valid_context?(parent) + + find_milestone_with_finder(parent, iid: iid) + end + + def valid_context?(parent) + strong_memoize(:valid_context) do + group_context?(parent) || project_context?(parent) + end + end + + def group_context?(parent) + strong_memoize(:group_context) do + parent.is_a?(Group) + end + end - find_milestone_with_finder(project, iid: iid) + def project_context?(parent) + strong_memoize(:project_context) do + parent.is_a?(Project) + end end def references_in(text, pattern = Milestone.reference_pattern) @@ -44,13 +64,15 @@ module Banzai def find_milestone(project_ref, namespace_ref, milestone_id, milestone_name) project_path = full_project_path(namespace_ref, project_ref) - project = parent_from_ref(project_path) - return unless project && project.is_a?(Project) + # Returns group if project is not found by path + parent = parent_from_ref(project_path) + + return unless parent milestone_params = milestone_params(milestone_id, milestone_name) - find_milestone_with_finder(project, milestone_params) + find_milestone_with_finder(parent, milestone_params) end def milestone_params(iid, name) @@ -61,16 +83,28 @@ module Banzai end end - def find_milestone_with_finder(project, params) - finder_params = { project_ids: [project.id], order: nil, state: 'all' } + def find_milestone_with_finder(parent, params) + finder_params = milestone_finder_params(parent, params[:iid].present?) + + MilestonesFinder.new(finder_params).find_by(params) + end - # We don't support IID lookups for group milestones, because IIDs can - # clash between group and project milestones. - if project.group && !params[:iid] - finder_params[:group_ids] = project.group.self_and_ancestors_ids + def milestone_finder_params(parent, find_by_iid) + { order: nil, state: 'all' }.tap do |params| + params[:project_ids] = parent.id if project_context?(parent) + + # We don't support IID lookups because IIDs can clash between + # group/project milestones and group/subgroup milestones. + params[:group_ids] = self_and_ancestors_ids(parent) unless find_by_iid end + end - MilestonesFinder.new(finder_params).find_by(params) + def self_and_ancestors_ids(parent) + if group_context?(parent) + parent.self_and_ancestors_ids + elsif project_context?(parent) + parent.group&.self_and_ancestors_ids + end end def url_for_object(milestone, project) diff --git a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb index 91d4a60ba95..1a87cfa5b45 100644 --- a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb @@ -351,21 +351,50 @@ describe Banzai::Filter::MilestoneReferenceFilter do end context 'group context' do - let(:context) { { project: nil, group: create(:group) } } - let(:milestone) { create(:milestone, project: project) } + let(:group) { create(:group) } + let(:context) { { project: nil, group: group } } - it 'links to a valid reference' do - reference = "#{project.full_path}%#{milestone.iid}" + context 'when project milestone' do + let(:milestone) { create(:milestone, project: project) } - result = reference_filter("See #{reference}", context) + it 'links to a valid reference' do + reference = "#{project.full_path}%#{milestone.iid}" - expect(result.css('a').first.attr('href')).to eq(urls.milestone_url(milestone)) + result = reference_filter("See #{reference}", context) + + expect(result.css('a').first.attr('href')).to eq(urls.milestone_url(milestone)) + end + + it 'ignores internal references' do + exp = act = "See %#{milestone.iid}" + + expect(reference_filter(act, context).to_html).to eq exp + end end - it 'ignores internal references' do - exp = act = "See %#{milestone.iid}" + context 'when group milestone' do + let(:group_milestone) { create(:milestone, title: 'group_milestone', group: group) } - expect(reference_filter(act, context).to_html).to eq exp + context 'for subgroups', :nested_groups do + let(:sub_group) { create(:group, parent: group) } + let(:sub_group_milestone) { create(:milestone, title: 'sub_group_milestone', group: sub_group) } + + it 'links to a valid reference of subgroup and group milestones' do + [group_milestone, sub_group_milestone].each do |milestone| + reference = "%#{milestone.title}" + + result = reference_filter("See #{reference}", { project: nil, group: sub_group }) + + expect(result.css('a').first.attr('href')).to eq(urls.milestone_url(milestone)) + end + end + end + + it 'ignores internal references' do + exp = act = "See %#{group_milestone.iid}" + + expect(reference_filter(act, context).to_html).to eq exp + end end end |