diff options
22 files changed, 87 insertions, 118 deletions
diff --git a/app/models/concerns/mentionable/reference_regexes.rb b/app/models/concerns/mentionable/reference_regexes.rb index 1848230ec7e..2d86a70c395 100644 --- a/app/models/concerns/mentionable/reference_regexes.rb +++ b/app/models/concerns/mentionable/reference_regexes.rb @@ -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 diff --git a/app/models/external_issue.rb b/app/models/external_issue.rb index e63f89a9f85..0bf18e529f0 100644 --- a/app/models/external_issue.rb +++ b/app/models/external_issue.rb @@ -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 diff --git a/app/models/project.rb b/app/models/project.rb index a75c5209955..8140ed3763f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -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? diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index ff138b9066d..fcc7c4bec06 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -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 diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index 2450fb43212..00328892b4a 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -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 diff --git a/changelogs/unreleased/adam-external-issue-references-spike.yml b/changelogs/unreleased/adam-external-issue-references-spike.yml new file mode 100644 index 00000000000..aeec6688425 --- /dev/null +++ b/changelogs/unreleased/adam-external-issue-references-spike.yml @@ -0,0 +1,4 @@ +--- +title: Improve support for external issue references +merge_request: 12485 +author: diff --git a/doc/integration/external-issue-tracker.md b/doc/integration/external-issue-tracker.md index 265c891cf83..2dd9b33273c 100644 --- a/doc/integration/external-issue-tracker.md +++ b/doc/integration/external-issue-tracker.md @@ -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 diff --git a/doc/user/project/integrations/bugzilla.md b/doc/user/project/integrations/bugzilla.md index 0b219e84478..6a040516231 100644 --- a/doc/user/project/integrations/bugzilla.md +++ b/doc/user/project/integrations/bugzilla.md @@ -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`. diff --git a/doc/user/project/integrations/redmine.md b/doc/user/project/integrations/redmine.md index 89c0312d3c2..8026f1f57bc 100644 --- a/doc/user/project/integrations/redmine.md +++ b/doc/user/project/integrations/redmine.md @@ -21,3 +21,14 @@ Once you have configured and enabled Redmine: As an example, below is a configuration for a project named gitlab-ci.  + +## 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`. diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb index 8bc2dd18bda..7a262dd025c 100644 --- a/lib/banzai/filter/abstract_reference_filter.rb +++ b/lib/banzai/filter/abstract_reference_filter.rb @@ -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 diff --git a/lib/banzai/filter/external_issue_reference_filter.rb b/lib/banzai/filter/external_issue_reference_filter.rb index dce4de3ceaf..53a229256a5 100644 --- a/lib/banzai/filter/external_issue_reference_filter.rb +++ b/lib/banzai/filter/external_issue_reference_filter.rb @@ -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 diff --git a/lib/banzai/filter/issue_reference_filter.rb b/lib/banzai/filter/issue_reference_filter.rb index 044d18ff824..ba1a5ac84b3 100644 --- a/lib/banzai/filter/issue_reference_filter.rb +++ b/lib/banzai/filter/issue_reference_filter.rb @@ -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 diff --git a/lib/banzai/reference_parser/issue_parser.rb b/lib/banzai/reference_parser/issue_parser.rb index 9fd4bd68d43..a65bbe23958 100644 --- a/lib/banzai/reference_parser/issue_parser.rb +++ b/lib/banzai/reference_parser/issue_parser.rb @@ -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 diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index aef1c17a239..1bb2db11e7f 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -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' } ) diff --git a/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb b/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb index a4bb043f8f1..b7d82c36ddd 100644 --- a/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb @@ -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 }) diff --git a/spec/lib/banzai/filter/issue_reference_filter_spec.rb b/spec/lib/banzai/filter/issue_reference_filter_spec.rb index e5c1deb338b..a79d365d6c5 100644 --- a/spec/lib/banzai/filter/issue_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/issue_reference_filter_spec.rb @@ -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 diff --git a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb new file mode 100644 index 00000000000..2b8c76f2bb8 --- /dev/null +++ b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb @@ -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 diff --git a/spec/lib/banzai/reference_parser/issue_parser_spec.rb b/spec/lib/banzai/reference_parser/issue_parser_spec.rb index 58e1a0c1bc1..acdd23f81f3 100644 --- a/spec/lib/banzai/reference_parser/issue_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/issue_parser_spec.rb @@ -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 diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index c86f56c55eb..4a1de76f099 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -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 diff --git a/spec/models/project_services/redmine_service_spec.rb b/spec/models/project_services/redmine_service_spec.rb index 6631d9040b1..441b3f896ca 100644 --- a/spec/models/project_services/redmine_service_spec.rb +++ b/spec/models/project_services/redmine_service_spec.rb @@ -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 diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index ca827fc0f39..8e8816870e1 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -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 diff --git a/spec/support/issue_tracker_service_shared_example.rb b/spec/support/issue_tracker_service_shared_example.rb index e70b3963d9d..a6ab03cb808 100644 --- a/spec/support/issue_tracker_service_shared_example.rb +++ b/spec/support/issue_tracker_service_shared_example.rb @@ -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 |