Merge branch 'issue_828' into 'master'

Prevent wrong markdown on issue ids when project has Jira service activated

fixes gitlab-org/gitlab-ee#828

See merge request !6728

2 files changed, 63 insertions, 26 deletions

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 7116c09fb21..2f9343fadaf 100644
--- a/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/external_issue_reference_filter_spec.rb
@@ -7,12 +7,7 @@ describe Banzai::Filter::ExternalIssueReferenceFilter, lib: true do
     IssuesHelper
   end
 
-  let(:project) { create(:jira_project) }
-
-  context 'JIRA issue references' do
-    let(:issue)     { ExternalIssue.new('JIRA-123', project) }
-    let(:reference) { issue.to_reference }
-
+  shared_examples_for "external issue tracker" do
     it 'requires project context' do
       expect { described_class.call('') }.to raise_error(ArgumentError, /:project/)
     end
@@ -20,6 +15,7 @@ describe Banzai::Filter::ExternalIssueReferenceFilter, lib: true do
     %w(pre code a style).each do |elem|
       it "ignores valid references contained inside '#{elem}' element" do
         exp = act = "<#{elem}>Issue #{reference}</#{elem}>"
+
         expect(filter(act).to_html).to eq exp
       end
     end
@@ -33,25 +29,30 @@ describe Banzai::Filter::ExternalIssueReferenceFilter, lib: true do
 
     it 'links to a valid reference' do
       doc = filter("Issue #{reference}")
+      issue_id = doc.css('a').first.attr("data-external-issue")
+
       expect(doc.css('a').first.attr('href'))
-        .to eq helper.url_for_issue(reference, project)
+        .to eq helper.url_for_issue(issue_id, project)
     end
 
     it 'links to the external tracker' do
       doc = filter("Issue #{reference}")
+
       link = doc.css('a').first.attr('href')
+      issue_id = doc.css('a').first.attr("data-external-issue")
 
-      expect(link).to eq "http://jira.example/browse/#{reference}"
+      expect(link).to eq(helper.url_for_issue(issue_id, project))
     end
 
     it 'links with adjacent text' do
       doc = filter("Issue (#{reference}.)")
+
       expect(doc.to_html).to match(/\(<a.+>#{reference}<\/a>\.\)/)
     end
 
     it 'includes a title attribute' do
       doc = filter("Issue #{reference}")
-      expect(doc.css('a').first.attr('title')).to eq "Issue in JIRA tracker"
+      expect(doc.css('a').first.attr('title')).to include("Issue in #{project.issues_tracker.title}")
     end
 
     it 'escapes the title attribute' do
@@ -69,9 +70,60 @@ describe Banzai::Filter::ExternalIssueReferenceFilter, lib: true do
 
     it 'supports an :only_path context' do
       doc = filter("Issue #{reference}", only_path: true)
+
       link = doc.css('a').first.attr('href')
+      issue_id = doc.css('a').first["data-external-issue"]
+
+      expect(link).to eq helper.url_for_issue(issue_id, project, only_path: true)
+    end
+
+    context 'with RequestStore enabled' do
+      let(:reference_filter) { HTML::Pipeline.new([described_class]) }
+
+      before { allow(RequestStore).to receive(:active?).and_return(true) }
+
+      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
+
+        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)
+
+        cached = reference_filter.call("look for #{reference}", { project: project })
 
-      expect(link).to eq helper.url_for_issue("#{reference}", project, only_path: true)
+        # Links must be the same
+        expect(cached[:output].css('a').first[:href]).to eq(not_cached[:output].css('a').first[:href])
+      end
+    end
+  end
+
+  context "redmine project" do
+    let(:project) { create(:redmine_project) }
+    let(:issue) { ExternalIssue.new("#123", project) }
+    let(:reference) { issue.to_reference }
+
+    it_behaves_like "external issue tracker"
+  end
+
+  context "jira project" do
+    let(:project) { create(:jira_project) }
+    let(:reference) { issue.to_reference }
+
+    context "with right markdown" do
+      let(:issue) { ExternalIssue.new("JIRA-123", project) }
+
+      it_behaves_like "external issue tracker"
+    end
+
+    context "with wrong markdown" do
+      let(:issue) { ExternalIssue.new("#123", project) }
+
+      it "ignores reference" do
+        exp = act = "Issue #{reference}"
+        expect(filter(act).to_html).to eq exp
+      end
     end
   end
 end
diff --git a/spec/lib/banzai/filter/issue_reference_filter_spec.rb b/spec/lib/banzai/filter/issue_reference_filter_spec.rb
index fce86a9b6ad..a2025672ad9 100644
--- a/spec/lib/banzai/filter/issue_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/issue_reference_filter_spec.rb
@@ -25,9 +25,7 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do
     let(:reference) { issue.to_reference }
 
     it 'ignores valid references when using non-default tracker' do
-      expect_any_instance_of(described_class).to receive(:find_object).
-        with(project, issue.iid).
-        and_return(nil)
+      allow(project).to receive(:default_issues_tracker?).and_return(false)
 
       exp = act = "Issue #{reference}"
       expect(reference_filter(act).to_html).to eq exp
@@ -199,19 +197,6 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do
     end
   end
 
-  context 'referencing external issues' do
-    let(:project) { create(:redmine_project) }
-
-    it 'renders internal issue IDs as external issue links' do
-      doc = reference_filter('#1')
-      link = doc.css('a').first
-
-      expect(link.attr('data-reference-type')).to eq('external_issue')
-      expect(link.attr('title')).to eq('Issue in Redmine')
-      expect(link.attr('data-external-issue')).to eq('1')
-    end
-  end
-
   describe '#issues_per_Project' do
     context 'using an internal issue tracker' do
       it 'returns a Hash containing the issues per project' do