Commit 3e831453 authored by Jan Provaznik's avatar Jan Provaznik

Fix reference filters in group context

Since !18150 abstract reference filter supports also `group`
as a resource parent (not only `project`). Some reference filters,
which inherit from abstract filter, depend on parent being `project`
when checking a reference.

This patch adds explicit check of parent class to the given filters.
parent 8e0e9d6d
---
title: Ignore project internal references in group context
merge_request:
author:
type: fixed
...@@ -56,29 +56,29 @@ module Banzai ...@@ -56,29 +56,29 @@ module Banzai
# Implement in child class # Implement in child class
# Example: project.merge_requests.find # Example: project.merge_requests.find
def find_object(project, id) def find_object(parent_object, id)
end end
# Override if the link reference pattern produces a different ID (global # Override if the link reference pattern produces a different ID (global
# ID vs internal ID, for instance) to the regular reference pattern. # ID vs internal ID, for instance) to the regular reference pattern.
def find_object_from_link(project, id) def find_object_from_link(parent_object, id)
find_object(project, id) find_object(parent_object, id)
end end
# Implement in child class # Implement in child class
# Example: project_merge_request_url # Example: project_merge_request_url
def url_for_object(object, project) def url_for_object(object, parent_object)
end end
def find_object_cached(project, id) def find_object_cached(parent_object, id)
cached_call(:banzai_find_object, id, path: [object_class, project.id]) do cached_call(:banzai_find_object, id, path: [object_class, parent_object.id]) do
find_object(project, id) find_object(parent_object, id)
end end
end end
def find_object_from_link_cached(project, id) def find_object_from_link_cached(parent_object, id)
cached_call(:banzai_find_object_from_link, id, path: [object_class, project.id]) do cached_call(:banzai_find_object_from_link, id, path: [object_class, parent_object.id]) do
find_object_from_link(project, id) find_object_from_link(parent_object, id)
end end
end end
...@@ -88,9 +88,9 @@ module Banzai ...@@ -88,9 +88,9 @@ module Banzai
end end
end end
def url_for_object_cached(object, project) def url_for_object_cached(object, parent_object)
cached_call(:banzai_url_for_object, object, path: [object_class, project.id]) do cached_call(:banzai_url_for_object, object, path: [object_class, parent_object.id]) do
url_for_object(object, project) url_for_object(object, parent_object)
end end
end end
......
...@@ -23,6 +23,8 @@ module Banzai ...@@ -23,6 +23,8 @@ module Banzai
end end
def find_object(project, id) def find_object(project, id)
return unless project.is_a?(Project)
range = CommitRange.new(id, project) range = CommitRange.new(id, project)
range.valid_commits? ? range : nil range.valid_commits? ? range : nil
......
...@@ -17,6 +17,8 @@ module Banzai ...@@ -17,6 +17,8 @@ module Banzai
end end
def find_object(project, id) def find_object(project, id)
return unless project.is_a?(Project)
if project && project.valid_repo? if project && project.valid_repo?
# n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/43894 # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/43894
Gitlab::GitalyClient.allow_n_plus_1_calls { project.commit(id) } Gitlab::GitalyClient.allow_n_plus_1_calls { project.commit(id) }
......
...@@ -8,8 +8,8 @@ module Banzai ...@@ -8,8 +8,8 @@ module Banzai
Label Label
end end
def find_object(project, id) def find_object(parent_object, id)
find_labels(project).find(id) find_labels(parent_object).find(id)
end end
def self.references_in(text, pattern = Label.reference_pattern) def self.references_in(text, pattern = Label.reference_pattern)
......
...@@ -12,10 +12,14 @@ module Banzai ...@@ -12,10 +12,14 @@ module Banzai
# 'regular' references, we need to use the global ID to disambiguate # 'regular' references, we need to use the global ID to disambiguate
# between group and project milestones. # between group and project milestones.
def find_object(project, id) def find_object(project, id)
return unless project.is_a?(Project)
find_milestone_with_finder(project, id: id) find_milestone_with_finder(project, id: id)
end end
def find_object_from_link(project, iid) def find_object_from_link(project, iid)
return unless project.is_a?(Project)
find_milestone_with_finder(project, iid: iid) find_milestone_with_finder(project, iid: iid)
end end
...@@ -40,7 +44,7 @@ module Banzai ...@@ -40,7 +44,7 @@ module Banzai
project_path = full_project_path(namespace_ref, project_ref) project_path = full_project_path(namespace_ref, project_ref)
project = parent_from_ref(project_path) project = parent_from_ref(project_path)
return unless project return unless project && project.is_a?(Project)
milestone_params = milestone_params(milestone_id, milestone_name) milestone_params = milestone_params(milestone_id, milestone_name)
......
...@@ -12,6 +12,8 @@ module Banzai ...@@ -12,6 +12,8 @@ module Banzai
end end
def find_object(project, id) def find_object(project, id)
return unless project.is_a?(Project)
project.snippets.find_by(id: id) project.snippets.find_by(id: id)
end end
......
...@@ -29,6 +29,8 @@ module Banzai ...@@ -29,6 +29,8 @@ module Banzai
end end
def find_object(project, id) def find_object(project, id)
return unless project.is_a?(Project)
range = CommitRange.new(id, project) range = CommitRange.new(id, project)
range.valid_commits? ? range : nil range.valid_commits? ? range : nil
......
...@@ -233,4 +233,20 @@ describe Banzai::Filter::CommitRangeReferenceFilter do ...@@ -233,4 +233,20 @@ describe Banzai::Filter::CommitRangeReferenceFilter do
expect(reference_filter(act).to_html).to eq exp expect(reference_filter(act).to_html).to eq exp
end end
end end
context 'group context' do
let(:context) { { project: nil, group: create(:group) } }
it 'ignores internal references' do
exp = act = "See #{range.to_reference}"
expect(reference_filter(act, context).to_html).to eq exp
end
it 'links to a full-path reference' do
reference = "#{project.full_path}@#{commit1.short_id}...#{commit2.short_id}"
expect(reference_filter("See #{reference}", context).css('a').first.text).to eql(reference)
end
end
end end
...@@ -238,4 +238,20 @@ describe Banzai::Filter::CommitReferenceFilter do ...@@ -238,4 +238,20 @@ describe Banzai::Filter::CommitReferenceFilter do
expect(doc.text).to eq("See (#{commit.reference_link_text(project)} (builds).patch)") expect(doc.text).to eq("See (#{commit.reference_link_text(project)} (builds).patch)")
end end
end end
context 'group context' do
let(:context) { { project: nil, group: create(:group) } }
it 'ignores internal references' do
exp = act = "See #{commit.id}"
expect(reference_filter(act, context).to_html).to eq exp
end
it 'links to a valid reference' do
act = "See #{project.full_path}@#{commit.id}"
expect(reference_filter(act, context).css('a').first.text).to eql("#{project.full_path}@#{commit.short_id}")
end
end
end end
...@@ -343,14 +343,22 @@ describe Banzai::Filter::MilestoneReferenceFilter do ...@@ -343,14 +343,22 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end end
context 'group context' do context 'group context' do
let(:context) { { project: nil, group: create(:group) } }
let(:milestone) { create(:milestone, project: project) }
it 'links to a valid reference' do it 'links to a valid reference' do
milestone = create(:milestone, project: project)
reference = "#{project.full_path}%#{milestone.iid}" reference = "#{project.full_path}%#{milestone.iid}"
result = reference_filter("See #{reference}", { project: nil, group: create(:group) } ) result = reference_filter("See #{reference}", context)
expect(result.css('a').first.attr('href')).to eq(urls.milestone_url(milestone)) expect(result.css('a').first.attr('href')).to eq(urls.milestone_url(milestone))
end end
it 'ignores internal references' do
exp = act = "See %#{milestone.iid}"
expect(reference_filter(act, context).to_html).to eq exp
end
end end
context 'when milestone is open' do context 'when milestone is open' do
......
...@@ -210,5 +210,11 @@ describe Banzai::Filter::SnippetReferenceFilter do ...@@ -210,5 +210,11 @@ describe Banzai::Filter::SnippetReferenceFilter do
expect(result.css('a').first.attr('href')).to eq(urls.project_snippet_url(project, snippet)) expect(result.css('a').first.attr('href')).to eq(urls.project_snippet_url(project, snippet))
end end
it 'ignores internal references' do
exp = act = "See $#{snippet.id}"
expect(reference_filter(act, project: nil, group: create(:group)).to_html).to eq exp
end
end end
end end
...@@ -107,12 +107,9 @@ describe Banzai::ReferenceParser::CommitRangeParser do ...@@ -107,12 +107,9 @@ describe Banzai::ReferenceParser::CommitRangeParser do
describe '#find_object' do describe '#find_object' do
let(:range) { double(:range) } let(:range) { double(:range) }
before do
expect(CommitRange).to receive(:new).and_return(range)
end
context 'when the range has valid commits' do context 'when the range has valid commits' do
it 'returns the commit range' do it 'returns the commit range' do
expect(CommitRange).to receive(:new).and_return(range)
expect(range).to receive(:valid_commits?).and_return(true) expect(range).to receive(:valid_commits?).and_return(true)
expect(subject.find_object(project, '123..456')).to eq(range) expect(subject.find_object(project, '123..456')).to eq(range)
...@@ -121,10 +118,19 @@ describe Banzai::ReferenceParser::CommitRangeParser do ...@@ -121,10 +118,19 @@ describe Banzai::ReferenceParser::CommitRangeParser do
context 'when the range does not have any valid commits' do context 'when the range does not have any valid commits' do
it 'returns nil' do it 'returns nil' do
expect(CommitRange).to receive(:new).and_return(range)
expect(range).to receive(:valid_commits?).and_return(false) expect(range).to receive(:valid_commits?).and_return(false)
expect(subject.find_object(project, '123..456')).to be_nil expect(subject.find_object(project, '123..456')).to be_nil
end end
end end
context 'group context' do
it 'returns nil' do
group = create(:group)
expect(subject.find_object(group, '123..456')).to be_nil
end
end
end 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