kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Merge branch 'adam-external-issue-references-spike' into 'master'

Browse source 
Improve support for external issue references

Closes #33679, #34076, and #34082

See merge request !12485
This commit is contained in:
Sean McGivern 2017-06-30 14:23:50 +00:00
commit 0ca6bf4dae
22 changed files with 87 additions and 118 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/models/concerns/mentionable/reference_regexes.rb
View file

@ -14,7 +14,7 @@ module Mentionable
end
EXTERNAL_PATTERN = begin
issue_pattern = ExternalIssue.reference_pattern
issue_pattern = IssueTrackerService.reference_pattern
link_patterns = URI.regexp(%w(http https))
reference_pattern(link_patterns, issue_pattern)
end

5
app/models/external_issue.rb
View file

@ -38,11 +38,6 @@ class ExternalIssue
@project.id
end
# Pattern used to extract `JIRA-123` issue references from text
def self.reference_pattern
@reference_pattern ||= %r{(?<issue>\b([A-Z][A-Z0-9_]+-)\d+)}
end
def to_reference(_from_project = nil, full: nil)
id
end

4
app/models/project.rb
View file

@ -727,8 +727,8 @@ class Project < ActiveRecord::Base
end
end
def issue_reference_pattern
issues_tracker.reference_pattern
def external_issue_reference_pattern
external_issue_tracker.class.reference_pattern
end
def default_issues_tracker?

5
app/models/project_services/issue_tracker_service.rb
View file

@ -5,7 +5,10 @@ class IssueTrackerService < Service
# Pattern used to extract links from comments
# Override this method on services that uses different patterns
def reference_pattern
# This pattern does not support cross-project references
# The other code assumes that this pattern is a superset of all
# overriden patterns. See ReferenceRegexes::EXTERNAL_PATTERN
def self.reference_pattern
@reference_pattern ||= %r{(\b[A-Z][A-Z0-9_]+-|#{Issue.reference_prefix})(?<issue>\d+)}
end

2
app/models/project_services/jira_service.rb
View file

@ -18,7 +18,7 @@ class JiraService < IssueTrackerService
end
# {PROJECT-KEY}-{NUMBER} Examples: JIRA-1, PROJECT-1
def reference_pattern
def self.reference_pattern
@reference_pattern ||= %r{(?<issue>\b([A-Z][A-Z0-9_]+-)\d+)}
end

4
changelogs/unreleased/adam-external-issue-references-spike.yml Normal file
View file

@ -0,0 +1,4 @@
---
title: Improve support for external issue references
merge_request: 12485
author:

3
doc/integration/external-issue-tracker.md
View file

@ -8,6 +8,9 @@ you to do the following:
issue index of the external tracker
- clicking **New issue** on the project dashboard creates a new issue on the
external tracker
- you can reference these external issues inside GitLab interface
(merge requests, commits, comments) and they will be automatically converted
into links
## Configuration

11
doc/user/project/integrations/bugzilla.md
View file

@ -16,3 +16,14 @@ Once you have configured and enabled Bugzilla:
- the **Issues** link on the GitLab project pages takes you to the appropriate
Bugzilla product page
- clicking **New issue** on the project dashboard takes you to Bugzilla for entering a new issue
## Referencing issues in Bugzilla
Issues in Bugzilla can be referenced in two alternative ways:
1. `#<ID>` where `<ID>` is a number (example `#143`)
2. `<PROJECT>-<ID>` where `<PROJECT>` starts with a capital letter which is
then followed by capital letters, numbers or underscores, and `<ID>` is
a number (example `API_32-143`).
Please note that `<PROJECT>` part is ignored and links always point to the
address specified in `issues_url`.

11
doc/user/project/integrations/redmine.md
View file

@ -21,3 +21,14 @@ Once you have configured and enabled Redmine:
As an example, below is a configuration for a project named gitlab-ci.
![Redmine configuration](img/redmine_configuration.png)
## Referencing issues in Redmine
Issues in Redmine can be referenced in two alternative ways:
1. `#<ID>` where `<ID>` is a number (example `#143`)
2. `<PROJECT>-<ID>` where `<PROJECT>` starts with a capital letter which is
then followed by capital letters, numbers or underscores, and `<ID>` is
a number (example `API_32-143`).
Please note that `<PROJECT>` part is ignored and links always point to the
address specified in `issues_url`.

15
lib/banzai/filter/abstract_reference_filter.rb
View file

@ -216,12 +216,7 @@ module Banzai
@references_per_project ||= begin
refs = Hash.new { |hash, key| hash[key] = Set.new }
regex =
if uses_reference_pattern?
Regexp.union(object_class.reference_pattern, object_class.link_reference_pattern)
else
object_class.link_reference_pattern
end
regex = Regexp.union(object_class.reference_pattern, object_class.link_reference_pattern)
nodes.each do |node|
node.to_html.scan(regex) do
@ -323,14 +318,6 @@ module Banzai
value
end
end
# There might be special cases like filters
# that should ignore reference pattern
# eg: IssueReferenceFilter when using a external issues tracker
# In those cases this method should be overridden on the filter subclass
def uses_reference_pattern?
true
end
end
end
end

4
lib/banzai/filter/external_issue_reference_filter.rb
View file

@ -3,6 +3,8 @@ module Banzai
# HTML filter that replaces external issue tracker references with links.
# References are ignored if the project doesn't use an external issue
# tracker.
#
# This filter does not support cross-project references.
class ExternalIssueReferenceFilter < ReferenceFilter
self.reference_type = :external_issue
@ -87,7 +89,7 @@ module Banzai
end
def issue_reference_pattern
external_issues_cached(:issue_reference_pattern)
external_issues_cached(:external_issue_reference_pattern)
end
private

32
lib/banzai/filter/issue_reference_filter.rb
View file

@ -15,10 +15,6 @@ module Banzai
Issue
end
def uses_reference_pattern?
context[:project].default_issues_tracker?
end
def find_object(project, iid)
issues_per_project[project][iid]
end
@ -38,13 +34,7 @@ module Banzai
projects_per_reference.each do |path, project|
issue_ids = references_per_project[path]
issues =
if project.default_issues_tracker?
project.issues.where(iid: issue_ids.to_a)
else
issue_ids.map { |id| ExternalIssue.new(id, project) }
end
issues = project.issues.where(iid: issue_ids.to_a)
issues.each do |issue|
hash[project][issue.iid.to_i] = issue
@ -55,26 +45,6 @@ module Banzai
end
end
def object_link_title(object)
if object.is_a?(ExternalIssue)
"Issue in #{object.project.external_issue_tracker.title}"
else
super
end
end
def data_attributes_for(text, project, object, link: false)
if object.is_a?(ExternalIssue)
data_attribute(
project: project.id,
external_issue: object.id,
reference_type: ExternalIssueReferenceFilter.reference_type
)
else
super
end
end
def projects_relation_for_paths(paths)
super(paths).includes(:gitlab_issue_tracker_service)
end

3
lib/banzai/reference_parser/issue_parser.rb
View file

@ -4,9 +4,6 @@ module Banzai
self.reference_type = :issue
def nodes_visible_to_user(user, nodes)
# It is not possible to check access rights for external issue trackers
return nodes if project && project.external_issue_tracker
issues = issues_for_nodes(nodes)
readable_issues = Ability

2
spec/factories/projects.rb
View file

@ -220,7 +220,7 @@ FactoryGirl.define do
active: true,
properties: {
'project_url' => 'http://redmine/projects/project_name_in_redmine',
'issues_url' => "http://redmine/#{project.id}/project_name_in_redmine/:id",
'issues_url' => 'http://redmine/projects/project_name_in_redmine/issues/:id',
'new_issue_url' => 'http://redmine/projects/project_name_in_redmine/issues/new'
}
)

4
spec/lib/banzai/filter/external_issue_reference_filter_spec.rb
View file

@ -88,12 +88,12 @@ describe Banzai::Filter::ExternalIssueReferenceFilter, lib: true do
it 'queries the collection on the first call' do
expect_any_instance_of(Project).to receive(:default_issues_tracker?).once.and_call_original
expect_any_instance_of(Project).to receive(:issue_reference_pattern).once.and_call_original
expect_any_instance_of(Project).to receive(:external_issue_reference_pattern).once.and_call_original
not_cached = reference_filter.call("look for #{reference}", { project: project })
expect_any_instance_of(Project).not_to receive(:default_issues_tracker?)
expect_any_instance_of(Project).not_to receive(:issue_reference_pattern)
expect_any_instance_of(Project).not_to receive(:external_issue_reference_pattern)
cached = reference_filter.call("look for #{reference}", { project: project })

25
spec/lib/banzai/filter/issue_reference_filter_spec.rb
View file

@ -39,13 +39,6 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do
let(:reference) { "##{issue.iid}" }
it 'ignores valid references when using non-default tracker' do
allow(project).to receive(:default_issues_tracker?).and_return(false)
exp = act = "Issue #{reference}"
expect(reference_filter(act).to_html).to eq exp
end
it 'links to a valid reference' do
doc = reference_filter("Fixed #{reference}")
@ -340,24 +333,6 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do
.to eq({ project => { issue.iid => issue } })
end
end
context 'using an external issue tracker' do
it 'returns a Hash containing the issues per project' do
doc = Nokogiri::HTML.fragment('')
filter = described_class.new(doc, project: project)
expect(project).to receive(:default_issues_tracker?).and_return(false)
expect(filter).to receive(:projects_per_reference)
.and_return({ project.path_with_namespace => project })
expect(filter).to receive(:references_per_project)
.and_return({ project.path_with_namespace => Set.new([1]) })
expect(filter.issues_per_project[project][1])
.to be_an_instance_of(ExternalIssue)
end
end
end
describe '.references_in' do

33
spec/lib/banzai/pipeline/gfm_pipeline_spec.rb Normal file
View file

@ -0,0 +1,33 @@
require 'rails_helper'
describe Banzai::Pipeline::GfmPipeline do
describe 'integration between parsing regular and external issue references' do
let(:project) { create(:redmine_project, :public) }
it 'allows to use shorthand external reference syntax for Redmine' do
markdown = '#12'
result = described_class.call(markdown, project: project)[:output]
link = result.css('a').first
expect(link['href']).to eq 'http://redmine/projects/project_name_in_redmine/issues/12'
end
it 'parses cross-project references to regular issues' do
other_project = create(:empty_project, :public)
issue = create(:issue, project: other_project)
markdown = issue.to_reference(project, full: true)
result = described_class.call(markdown, project: project)[:output]
link = result.css('a').first
expect(link['href']).to eq(
Gitlab::Routing.url_helpers.namespace_project_issue_path(
other_project.namespace,
other_project,
issue
)
)
end
end
end

10
spec/lib/banzai/reference_parser/issue_parser_spec.rb
View file

@ -39,16 +39,6 @@ describe Banzai::ReferenceParser::IssueParser, lib: true do
expect(subject.nodes_visible_to_user(user, [link])).to eq([])
end
end
context 'when the project uses an external issue tracker' do
it 'returns all nodes' do
link = double(:link)
expect(project).to receive(:external_issue_tracker).and_return(true)
expect(subject.nodes_visible_to_user(user, [link])).to eq([link])
end
end
end
describe '#referenced_by' do

6
spec/models/project_services/jira_service_spec.rb
View file

@ -64,12 +64,12 @@ describe JiraService, models: true do
end
end
describe '#reference_pattern' do
describe '.reference_pattern' do
it_behaves_like 'allows project key on reference pattern'
it 'does not allow # on the code' do
expect(subject.reference_pattern.match('#123')).to be_nil
expect(subject.reference_pattern.match('1#23#12')).to be_nil
expect(described_class.reference_pattern.match('#123')).to be_nil
expect(described_class.reference_pattern.match('1#23#12')).to be_nil
end
end

4
spec/models/project_services/redmine_service_spec.rb
View file

@ -31,11 +31,11 @@ describe RedmineService, models: true do
end
end
describe '#reference_pattern' do
describe '.reference_pattern' do
it_behaves_like 'allows project key on reference pattern'
it 'does allow # on the reference' do
expect(subject.reference_pattern.match('#123')[:issue]).to eq('123')
expect(described_class.reference_pattern.match('#123')[:issue]).to eq('123')
end
end
end

12
spec/services/git_push_service_spec.rb
View file

@ -401,18 +401,6 @@ describe GitPushService, services: true do
expect(SystemNoteService).not_to receive(:cross_reference)
execute_service(project, commit_author, @oldrev, @newrev, @ref )
end
it "doesn't close issues when external issue tracker is in use" do
allow_any_instance_of(Project).to receive(:default_issues_tracker?)
.and_return(false)
external_issue_tracker = double(title: 'My Tracker', issue_path: issue.iid, reference_pattern: project.issue_reference_pattern)
allow_any_instance_of(Project).to receive(:external_issue_tracker).and_return(external_issue_tracker)
# The push still shouldn't create cross-reference notes.
expect do
execute_service(project, commit_author, @oldrev, @newrev, 'refs/heads/hurf' )
end.not_to change { Note.where(project_id: project.id, system: true).count }
end
end
context "to non-default branches" do

8
spec/support/issue_tracker_service_shared_example.rb
View file

@ -8,15 +8,15 @@ end
RSpec.shared_examples 'allows project key on reference pattern' do |url_attr|
it 'allows underscores in the project name' do
expect(subject.reference_pattern.match('EXT_EXT-1234')[0]).to eq 'EXT_EXT-1234'
expect(described_class.reference_pattern.match('EXT_EXT-1234')[0]).to eq 'EXT_EXT-1234'
end
it 'allows numbers in the project name' do
expect(subject.reference_pattern.match('EXT3_EXT-1234')[0]).to eq 'EXT3_EXT-1234'
expect(described_class.reference_pattern.match('EXT3_EXT-1234')[0]).to eq 'EXT3_EXT-1234'
end
it 'requires the project name to begin with A-Z' do
expect(subject.reference_pattern.match('3EXT_EXT-1234')).to eq nil
expect(subject.reference_pattern.match('EXT_EXT-1234')[0]).to eq 'EXT_EXT-1234'
expect(described_class.reference_pattern.match('3EXT_EXT-1234')).to eq nil
expect(described_class.reference_pattern.match('EXT_EXT-1234')[0]).to eq 'EXT_EXT-1234'
end
end