Merge branch 'master' into 'master'

Update ExternalIssue regex for JIRA integration

The pattern in the `::reference_pattern` class method in the
`ExternalIssue` model does not match all valid forms of JIRA project
names. I have updated the regex to match JIRA project names with numbers
and underscores. More information on valid JIRA project names can be
found here:
https://confluence.atlassian.com/jira/changing-the-project-key-format-192534.html

* The first character must be a letter,
* All letters used in the project key must be from the Modern Roman Alphabet and upper case, and
* Only letters, numbers or the underscore character can be used.

See merge request !2598

5 files changed, 29 insertions, 2 deletions

diff --git a/CHANGELOG b/CHANGELOG
index e20330cd60a..6660b33bb69 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -19,6 +19,7 @@ v 8.4.1
     and Nokogiri (1.6.7.2)
   - Fix redirect loop during import
   - Fix diff highlighting for all syntax themes
+  - Update the ExternalIssue regex pattern (Blake Hitchcock)
 
 v 8.4.0
   - Allow LDAP users to change their email if it was not set by the LDAP server
diff --git a/app/models/external_issue.rb b/app/models/external_issue.rb
index 49f6c95e045..2ca79df0a29 100644
--- a/app/models/external_issue.rb
+++ b/app/models/external_issue.rb
@@ -31,7 +31,7 @@ class ExternalIssue
 
   # Pattern used to extract `JIRA-123` issue references from text
   def self.reference_pattern
-    %r{(?<issue>([A-Z\-]+-)\d+)}
+    %r{(?<issue>\b([A-Z][A-Z0-9_]+-)\d+)}
   end
 
   def to_reference(_from_project = nil)
diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index 04a7c16ebde..d8170557f7e 100644
--- a/config/initializers/1_settings.rb
+++ b/config/initializers/1_settings.rb
@@ -176,7 +176,7 @@ Settings.gitlab['signin_enabled'] ||= true if Settings.gitlab['signin_enabled'].
 Settings.gitlab['twitter_sharing_enabled'] ||= true if Settings.gitlab['twitter_sharing_enabled'].nil?
 Settings.gitlab['restricted_visibility_levels'] = Settings.send(:verify_constant_array, Gitlab::VisibilityLevel, Settings.gitlab['restricted_visibility_levels'], [])
 Settings.gitlab['username_changing_enabled'] = true if Settings.gitlab['username_changing_enabled'].nil?
-Settings.gitlab['issue_closing_pattern'] = '((?:[Cc]los(?:e[sd]?|ing)|[Ff]ix(?:e[sd]|ing)?|[Rr]esolv(?:e[sd]?|ing)) +(?:(?:issues? +)?%{issue_ref}(?:(?:, *| +and +)?)|([A-Z]*-\d*))+)' if Settings.gitlab['issue_closing_pattern'].nil?
+Settings.gitlab['issue_closing_pattern'] = '((?:[Cc]los(?:e[sd]?|ing)|[Ff]ix(?:e[sd]|ing)?|[Rr]esolv(?:e[sd]?|ing)) +(?:(?:issues? +)?%{issue_ref}(?:(?:, *| +and +)?)|([A-Z][A-Z0-9_]+-\d+))+)' if Settings.gitlab['issue_closing_pattern'].nil?
 Settings.gitlab['default_projects_features'] ||= {}
 Settings.gitlab['webhook_timeout'] ||= 10
 Settings.gitlab['max_attachment_size'] ||= 10
diff --git a/spec/lib/gitlab/closing_issue_extractor_spec.rb b/spec/lib/gitlab/closing_issue_extractor_spec.rb
index 99288da1e43..04cf11fc6f1 100644
--- a/spec/lib/gitlab/closing_issue_extractor_spec.rb
+++ b/spec/lib/gitlab/closing_issue_extractor_spec.rb
@@ -135,6 +135,17 @@ describe Gitlab::ClosingIssueExtractor, lib: true do
         message = "resolve #{reference}"
         expect(subject.closed_by_message(message)).to eq([issue])
       end
+
+      context 'with an external issue tracker reference' do
+        it 'extracts the referenced issue' do
+          jira_project = create(:jira_project, name: 'JIRA_EXT1')
+          jira_issue = ExternalIssue.new("#{jira_project.name}-1", project: jira_project)
+          closing_issue_extractor = described_class.new jira_project
+          message = "Resolve #{jira_issue.to_reference}"
+
+          expect(closing_issue_extractor.closed_by_message(message)).to eq([jira_issue])
+        end
+      end
     end
 
     context "with a cross-project reference" do
diff --git a/spec/models/external_issue_spec.rb b/spec/models/external_issue_spec.rb
index 6ec6b9037a4..9b144dd1ecc 100644
--- a/spec/models/external_issue_spec.rb
+++ b/spec/models/external_issue_spec.rb
@@ -10,6 +10,21 @@ describe ExternalIssue, models: true do
     it { is_expected.to include_module(Referable) }
   end
 
+  describe '.reference_pattern' do
+    it 'allows underscores in the project name' do
+      expect(ExternalIssue.reference_pattern.match('EXT_EXT-1234')[0]).to eq 'EXT_EXT-1234'
+    end
+
+    it 'allows numbers in the project name' do
+      expect(ExternalIssue.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(ExternalIssue.reference_pattern.match('3EXT_EXT-1234')).to eq nil
+      expect(ExternalIssue.reference_pattern.match('EXT_EXT-1234')[0]).to eq 'EXT_EXT-1234'
+    end
+  end
+
   describe '#to_reference' do
     it 'returns a String reference to the object' do
       expect(issue.to_reference).to eq issue.id