Make CommitRange and Snippets cross-referable

4 files changed, 96 insertions, 67 deletions

diff --git a/lib/gitlab/markdown/snippet_reference_filter.rb b/lib/gitlab/markdown/snippet_reference_filter.rb
index 0e34ab4bc1e..42f035ce295 100644
--- a/lib/gitlab/markdown/snippet_reference_filter.rb
+++ b/lib/gitlab/markdown/snippet_reference_filter.rb
@@ -7,11 +7,14 @@ module Gitlab
     # snippets that do not exist are ignored.
     #
     # Context options:
-    #   :project (required) - Current project.
+    #   :project (required) - Current project, ignored when reference is
+    #                         cross-project.
     #   :reference_class    - Custom CSS class added to reference links.
     #   :only_path          - Generate path-only links.
     #
     class SnippetReferenceFilter < HTML::Pipeline::Filter
+      include CrossProjectReference
+
       # Public: Find `$123` snippet references in text
       #
       #   SnippetReferenceFilter.references_in(text) do |match, snippet|
@@ -20,17 +23,20 @@ module Gitlab
       #
       # text - String text to search.
       #
-      # Yields the String match and the Integer snippet ID.
+      # Yields the String match, the Integer snippet ID, and an optional String
+      # of the external project reference.
       #
       # Returns a String replaced with the return of the block.
       def self.references_in(text)
         text.gsub(SNIPPET_PATTERN) do |match|
-          yield match, $~[:snippet].to_i
+          yield match, $~[:snippet].to_i, $~[:project]
         end
       end
 
       # Pattern used to extract `$123` snippet references from text
-      SNIPPET_PATTERN = /\$(?<snippet>\d+)/
+      #
+      # This pattern supports cross-project references.
+      SNIPPET_PATTERN = /#{PROJECT_PATTERN}?\$(?<snippet>\d+)/
 
       # Don't look for references in text nodes that are children of these
       # elements.
@@ -40,7 +46,7 @@ module Gitlab
         doc.search('text()').each do |node|
           content = node.to_html
 
-          next if project.nil?
+          next if context[:project].nil?
           next unless content.match(SNIPPET_PATTERN)
           next if has_ancestor?(node, IGNORE_PARENTS)
 
@@ -66,9 +72,9 @@ module Gitlab
       # Returns a String with `$123` references replaced with links. All links
       # have `gfm` and `gfm-snippet` class names attached for styling.
       def snippet_link_filter(text)
-        project = context[:project]
+        self.class.references_in(text) do |match, id, project_ref|
+          project = self.project_from_ref(project_ref)
 
-        self.class.references_in(text) do |match, id|
           if snippet = project.snippets.find_by(id: id)
             title = "Snippet: #{snippet.title}"
             klass = "gfm gfm-snippet #{context[:reference_class]}".strip
@@ -77,17 +83,13 @@ module Gitlab
 
             %(<a href="#{url}"
                  title="#{title}"
-                 class="#{klass}">$#{id}</a>)
+                 class="#{klass}">#{project_ref}$#{id}</a>)
           else
             match
           end
         end
       end
 
-      def project
-        context[:project]
-      end
-
       def url_for_snippet(snippet, project)
         h = Rails.application.routes.url_helpers
         h.namespace_project_snippet_url(project.namespace, project, snippet,
diff --git a/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb b/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb
index 3545d7f8085..67de23014f8 100644
--- a/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb
@@ -51,7 +51,7 @@ module Gitlab::Markdown
         expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/)
       end
 
-      it 'ignores invalid issue IDs' do
+      it 'ignores invalid commit IDs' do
         exp = act = "See #{commit1.id.reverse}...#{commit2.id}"
 
         expect(project).to receive(:valid_repo?).and_return(true)
@@ -83,31 +83,32 @@ module Gitlab::Markdown
       end
     end
 
-    # TODO (rspeicher): Remove or re-enable
-    # context 'cross-project reference' do
-    #   let(:namespace) { create(:namespace, name: 'cross-reference') }
-    #   let(:project2)  { create(:project, namespace: namespace) }
-    #   let(:commit1)   { project.repository.commit }
-    #   let(:commit2)   { project.repository.commit("HEAD~2") }
-    #   let(:reference) { "#{project2.path_with_namespace}@#{commit.id}" }
+    context 'cross-project reference' do
+      let(:namespace) { create(:namespace, name: 'cross-reference') }
+      let(:project2)  { create(:project, namespace: namespace) }
+      let(:commit1)   { project.repository.commit }
+      let(:commit2)   { project.repository.commit("HEAD~2") }
+      let(:reference) { "#{project2.path_with_namespace}@#{commit1.id}...#{commit2.id}" }
 
-    #   it 'links to a valid reference' do
-    #     doc = filter("See #{reference}")
+      it 'links to a valid reference' do
+        doc = filter("See #{reference}")
 
-    #     expect(doc.css('a').first.attr('href')).
-    #       to eq urls.namespace_project_commit_url(project2.namespace, project2, commit.id)
-    #   end
+        expect(doc.css('a').first.attr('href')).
+          to eq urls.namespace_project_compare_url(project2.namespace, project2, from: commit1.id, to: commit2.id)
+      end
 
-    #   it 'links with adjacent text' do
-    #     doc = filter("Fixed (#{reference}.)")
-    #     expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/)
-    #   end
+      it 'links with adjacent text' do
+        doc = filter("Fixed (#{reference}.)")
+        expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/)
+      end
 
-    #   it 'ignores invalid issue IDs on the referenced project' do
-    #     exp = act = "Fixed #{project2.path_with_namespace}##{commit.id.reverse}"
+      it 'ignores invalid commit IDs on the referenced project' do
+        exp = act = "Fixed #{project2.path_with_namespace}##{commit1.id.reverse}...#{commit2.id}"
+        expect(filter(act).to_html).to eq exp
 
-    #     expect(filter(act).to_html).to eq exp
-    #   end
-    # end
+        exp = act = "Fixed #{project2.path_with_namespace}##{commit1.id}...#{commit2.id.reverse}"
+        expect(filter(act).to_html).to eq exp
+      end
+    end
   end
 end
diff --git a/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb b/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb
index 15851d3bd34..f72953fd84e 100644
--- a/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb
@@ -38,7 +38,7 @@ module Gitlab::Markdown
         expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/)
       end
 
-      it 'ignores invalid issue IDs' do
+      it 'ignores invalid commit IDs' do
         exp = act = "See #{reference.reverse}"
 
         expect(project).to receive(:valid_repo?).and_return(true)
@@ -88,9 +88,8 @@ module Gitlab::Markdown
         expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/)
       end
 
-      it 'ignores invalid issue IDs on the referenced project' do
-        exp = act = "Fixed #{project2.path_with_namespace}##{commit.id.reverse}"
-
+      it 'ignores invalid commit IDs on the referenced project' do
+        exp = act = "Committed #{project2.path_with_namespace}##{commit.id.reverse}"
         expect(filter(act).to_html).to eq exp
       end
     end
diff --git a/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb b/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb
index d79a7e544a3..a3285998ac1 100644
--- a/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb
@@ -20,45 +20,72 @@ module Gitlab::Markdown
       end
     end
 
-    it 'links to a valid reference' do
-      doc = filter("See #{reference}")
+    context 'internal reference' do
+      it 'links to a valid reference' do
+        doc = filter("See #{reference}")
 
-      expect(doc.css('a').first.attr('href')).to eq urls.
-        namespace_project_snippet_url(project.namespace, project, snippet)
-    end
+        expect(doc.css('a').first.attr('href')).to eq urls.
+          namespace_project_snippet_url(project.namespace, project, snippet)
+      end
 
-    it 'links with adjacent text' do
-      doc = filter("Snippet (#{reference}.)")
-      expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/)
-    end
+      it 'links with adjacent text' do
+        doc = filter("Snippet (#{reference}.)")
+        expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/)
+      end
 
-    it 'ignores invalid snippet IDs' do
-      exp = act = "Snippet $#{snippet.id + 1}"
+      it 'ignores invalid snippet IDs' do
+        exp = act = "Snippet $#{snippet.id + 1}"
 
-      expect(filter(act).to_html).to eq exp
-    end
+        expect(filter(act).to_html).to eq exp
+      end
 
-    it 'includes a title attribute' do
-      doc = filter("Snippet #{reference}")
-      expect(doc.css('a').first.attr('title')).to eq "Snippet: #{snippet.title}"
-    end
+      it 'includes a title attribute' do
+        doc = filter("Snippet #{reference}")
+        expect(doc.css('a').first.attr('title')).to eq "Snippet: #{snippet.title}"
+      end
 
-    it 'includes default classes' do
-      doc = filter("Snippet #{reference}")
-      expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-snippet'
-    end
+      it 'includes default classes' do
+        doc = filter("Snippet #{reference}")
+        expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-snippet'
+      end
 
-    it 'includes an optional custom class' do
-      doc = filter("Snippet #{reference}", reference_class: 'custom')
-      expect(doc.css('a').first.attr('class')).to include 'custom'
+      it 'includes an optional custom class' do
+        doc = filter("Snippet #{reference}", reference_class: 'custom')
+        expect(doc.css('a').first.attr('class')).to include 'custom'
+      end
+
+      it 'supports an :only_path context' do
+        doc = filter("Snippet #{reference}", only_path: true)
+        link = doc.css('a').first.attr('href')
+
+        expect(link).not_to match %r(https?://)
+        expect(link).to eq urls.namespace_project_snippet_url(project.namespace, project, snippet, only_path: true)
+      end
     end
 
-    it 'supports an :only_path context' do
-      doc = filter("Snippet #{reference}", only_path: true)
-      link = doc.css('a').first.attr('href')
+    context 'cross-project reference' do
+      let(:namespace) { create(:namespace, name: 'cross-reference') }
+      let(:project2)  { create(:empty_project, namespace: namespace) }
+      let(:snippet)   { create(:project_snippet, project: project2) }
+      let(:reference) { "#{project2.path_with_namespace}$#{snippet.id}" }
+
+      it 'links to a valid reference' do
+        doc = filter("See #{reference}")
+
+        expect(doc.css('a').first.attr('href')).
+          to eq urls.namespace_project_snippet_url(project2.namespace, project2, snippet)
+      end
 
-      expect(link).not_to match %r(https?://)
-      expect(link).to eq urls.namespace_project_snippet_url(project.namespace, project, snippet, only_path: true)
+      it 'links with adjacent text' do
+        doc = filter("See (#{reference}.)")
+        expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/)
+      end
+
+      it 'ignores invalid snippet IDs on the referenced project' do
+        exp = act = "See #{project2.path_with_namespace}$#{snippet.id + 1}"
+
+        expect(filter(act).to_html).to eq exp
+      end
     end
   end
 end