Fix cross-project references copy to include the project referencefix/cross-project-ref-path

Also added relevant specs and refactored to_references in a bunch of places to be more consistent.

21 files changed, 73 insertions, 55 deletions

diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb
index eb435cc1783..6d365ea9251 100644
--- a/app/helpers/gitlab_markdown_helper.rb
+++ b/app/helpers/gitlab_markdown_helper.rb
@@ -110,6 +110,28 @@ module GitlabMarkdownHelper
     end
   end
 
+  # Returns the text necessary to reference `entity` across projects
+  #
+  # project - Project to reference
+  # entity  - Object that responds to `to_reference`
+  #
+  # Examples:
+  #
+  #   cross_project_reference(project, project.issues.first)
+  #   # => 'namespace1/project1#123'
+  #
+  #   cross_project_reference(project, project.merge_requests.first)
+  #   # => 'namespace1/project1!345'
+  #
+  # Returns a String
+  def cross_project_reference(project, entity)
+    if entity.respond_to?(:to_reference)
+      entity.to_reference(project, full: true)
+    else
+      ''
+    end
+  end
+
   private
 
   # Return +text+, truncated to +max_chars+ characters, excluding any HTML
@@ -158,28 +180,6 @@ module GitlabMarkdownHelper
     end
   end
 
-  # Returns the text necessary to reference `entity` across projects
-  #
-  # project - Project to reference
-  # entity  - Object that responds to `to_reference`
-  #
-  # Examples:
-  #
-  #   cross_project_reference(project, project.issues.first)
-  #   # => 'namespace1/project1#123'
-  #
-  #   cross_project_reference(project, project.merge_requests.first)
-  #   # => 'namespace1/project1!345'
-  #
-  # Returns a String
-  def cross_project_reference(project, entity)
-    if entity.respond_to?(:to_reference)
-      entity.to_reference(project)
-    else
-      ''
-    end
-  end
-
   def markdown_toolbar_button(options = {})
     data = options[:data].merge({ container: "body" })
     content_tag :button,
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 69cfc47f5bf..0b924b063a4 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -91,8 +91,8 @@ class Commit
     @link_reference_pattern ||= super("commit", /(?<commit>\h{7,40})/)
   end
 
-  def to_reference(from_project = nil)
-    commit_reference(from_project, id)
+  def to_reference(from_project = nil, full: false)
+    commit_reference(from_project, id, full: full)
   end
 
   def reference_link_text(from_project = nil)
@@ -320,8 +320,8 @@ class Commit
 
   private
 
-  def commit_reference(from_project, referable_commit_id)
-    reference = project.to_reference(from_project)
+  def commit_reference(from_project, referable_commit_id, full: false)
+    reference = project.to_reference(from_project, full: full)
 
     if reference.present?
       "#{reference}#{self.class.reference_prefix}#{referable_commit_id}"
diff --git a/app/models/commit_range.rb b/app/models/commit_range.rb
index d9af7f6c139..84e2e8a5dd5 100644
--- a/app/models/commit_range.rb
+++ b/app/models/commit_range.rb
@@ -89,8 +89,8 @@ class CommitRange
 
   alias_method :id, :to_s
 
-  def to_reference(from_project = nil)
-    project_reference = project.to_reference(from_project)
+  def to_reference(from_project = nil, full: false)
+    project_reference = project.to_reference(from_project, full: full)
 
     if project_reference.present?
       project_reference + self.class.reference_prefix + self.id
diff --git a/app/models/concerns/referable.rb b/app/models/concerns/referable.rb
index 8ba009fe04f..da803c7f481 100644
--- a/app/models/concerns/referable.rb
+++ b/app/models/concerns/referable.rb
@@ -17,7 +17,7 @@ module Referable
   #   Issue.last.to_reference(other_project) # => "cross-project#1"
   #
   # Returns a String
-  def to_reference(_from_project = nil)
+  def to_reference(_from_project = nil, full:)
     ''
   end
 
diff --git a/app/models/external_issue.rb b/app/models/external_issue.rb
index 91b508eb325..26712c19b5a 100644
--- a/app/models/external_issue.rb
+++ b/app/models/external_issue.rb
@@ -38,7 +38,7 @@ class ExternalIssue
     @reference_pattern ||= %r{(?<issue>\b([A-Z][A-Z0-9_]+-)\d+)}
   end
 
-  def to_reference(_from_project = nil)
+  def to_reference(_from_project = nil, full: nil)
     id
   end
 
diff --git a/app/models/group.rb b/app/models/group.rb
index 9888b242e98..99675ddb366 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -80,7 +80,7 @@ class Group < Namespace
     end
   end
 
-  def to_reference(_from_project = nil)
+  def to_reference(_from_project = nil, full: nil)
     "#{self.class.reference_prefix}#{name}"
   end
 
diff --git a/app/models/group_label.rb b/app/models/group_label.rb
index 68841ace2e6..92c83b54861 100644
--- a/app/models/group_label.rb
+++ b/app/models/group_label.rb
@@ -8,8 +8,4 @@ class GroupLabel < Label
   def subject_foreign_key
     'group_id'
   end
-
-  def to_reference(source_project = nil, target_project = nil, format: :id)
-    super(source_project, target_project, format: format)
-  end
 end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 6825553512f..65638d9a299 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -97,10 +97,10 @@ class Issue < ActiveRecord::Base
     end
   end
 
-  def to_reference(from_project = nil)
+  def to_reference(from_project = nil, full: false)
     reference = "#{self.class.reference_prefix}#{iid}"
 
-    "#{project.to_reference(from_project)}#{reference}"
+    "#{project.to_reference(from_project, full: full)}#{reference}"
   end
 
   def referenced_merge_requests(current_user = nil)
diff --git a/app/models/label.rb b/app/models/label.rb
index d38c37344c9..5c01c15e5af 100644
--- a/app/models/label.rb
+++ b/app/models/label.rb
@@ -146,17 +146,17 @@ class Label < ActiveRecord::Base
   #
   #   Label.first.to_reference                                     # => "~1"
   #   Label.first.to_reference(format: :name)                      # => "~\"bug\""
-  #   Label.first.to_reference(project, same_namespace_project)    # => "gitlab-ce~1"
-  #   Label.first.to_reference(project, another_namespace_project) # => "gitlab-org/gitlab-ce~1"
+  #   Label.first.to_reference(project, target_project: same_namespace_project)    # => "gitlab-ce~1"
+  #   Label.first.to_reference(project, target_project: another_namespace_project) # => "gitlab-org/gitlab-ce~1"
   #
   # Returns a String
   #
-  def to_reference(source_project = nil, target_project = nil, format: :id)
+  def to_reference(from_project = nil, target_project: nil, format: :id, full: false)
     format_reference = label_format_reference(format)
     reference = "#{self.class.reference_prefix}#{format_reference}"
 
-    if source_project
-      "#{source_project.to_reference(target_project)}#{reference}"
+    if from_project
+      "#{from_project.to_reference(target_project, full: full)}#{reference}"
     else
       reference
     end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 61845bf4036..926944bc3b3 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -175,10 +175,10 @@ class MergeRequest < ActiveRecord::Base
     work_in_progress?(title) ? title : "WIP: #{title}"
   end
 
-  def to_reference(from_project = nil)
+  def to_reference(from_project = nil, full: false)
     reference = "#{self.class.reference_prefix}#{iid}"
 
-    "#{project.to_reference(from_project)}#{reference}"
+    "#{project.to_reference(from_project, full: full)}#{reference}"
   end
 
   def first_commit
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index 0dcfec89f14..8a11f47dd67 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -118,11 +118,11 @@ class Milestone < ActiveRecord::Base
   #   Milestone.first.to_reference(cross_namespace_project)  # => "gitlab-org/gitlab-ce%1"
   #   Milestone.first.to_reference(same_namespace_project)   # => "gitlab-ce%1"
   #
-  def to_reference(from_project = nil, format: :iid)
+  def to_reference(from_project = nil, format: :iid, full: false)
     format_reference = milestone_format_reference(format)
     reference = "#{self.class.reference_prefix}#{format_reference}"
 
-    "#{project.to_reference(from_project)}#{reference}"
+    "#{project.to_reference(from_project, full: full)}#{reference}"
   end
 
   def reference_link_text(from_project = nil)
diff --git a/app/models/project.rb b/app/models/project.rb
index e0ffa7e7af7..11ca09668e0 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -589,8 +589,8 @@ class Project < ActiveRecord::Base
     end
   end
 
-  def to_reference(from_project = nil)
-    if cross_namespace_reference?(from_project)
+  def to_reference(from_project = nil, full: false)
+    if full || cross_namespace_reference?(from_project)
       path_with_namespace
     elsif cross_project_reference?(from_project)
       path
diff --git a/app/models/project_label.rb b/app/models/project_label.rb
index 82f47f0e8fd..313815e5869 100644
--- a/app/models/project_label.rb
+++ b/app/models/project_label.rb
@@ -16,8 +16,8 @@ class ProjectLabel < Label
     'project_id'
   end
 
-  def to_reference(target_project = nil, format: :id)
-    super(project, target_project, format: format)
+  def to_reference(target_project = nil, format: :id, full: false)
+    super(project, target_project: target_project, format: format, full: full)
   end
 
   private
diff --git a/app/models/snippet.rb b/app/models/snippet.rb
index 98ccf5f331f..771a7350556 100644
--- a/app/models/snippet.rb
+++ b/app/models/snippet.rb
@@ -64,11 +64,11 @@ class Snippet < ActiveRecord::Base
     @link_reference_pattern ||= super("snippets", /(?<snippet>\d+)/)
   end
 
-  def to_reference(from_project = nil)
+  def to_reference(from_project = nil, full: false)
     reference = "#{self.class.reference_prefix}#{id}"
 
     if project.present?
-      "#{project.to_reference(from_project)}#{reference}"
+      "#{project.to_reference(from_project, full: full)}#{reference}"
     else
       reference
     end
diff --git a/app/models/user.rb b/app/models/user.rb
index 899a89a4eaa..e719c52836a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -332,7 +332,7 @@ class User < ActiveRecord::Base
     username
   end
 
-  def to_reference(_from_project = nil, _target_project = nil)
+  def to_reference(_from_project = nil, target_project: nil, full: nil)
     "#{self.class.reference_prefix}#{username}"
   end
 
diff --git a/changelogs/unreleased/fix-cross-project-ref-path.yml b/changelogs/unreleased/fix-cross-project-ref-path.yml
new file mode 100644
index 00000000000..91acc1db0a6
--- /dev/null
+++ b/changelogs/unreleased/fix-cross-project-ref-path.yml
@@ -0,0 +1,4 @@
+---
+title: Fix cross-project references copy to include the project reference
+merge_request: 
+author: 
diff --git a/lib/gitlab/gfm/reference_rewriter.rb b/lib/gitlab/gfm/reference_rewriter.rb
index a7c596dced0..b984492d369 100644
--- a/lib/gitlab/gfm/reference_rewriter.rb
+++ b/lib/gitlab/gfm/reference_rewriter.rb
@@ -76,7 +76,7 @@ module Gitlab
         if referable.respond_to?(:project)
           referable.to_reference(target_project)
         else
-          referable.to_reference(@source_project, target_project)
+          referable.to_reference(@source_project, target_project: target_project)
         end
       end
 
diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb
index 1d494edcd3b..810311e2b1a 100644
--- a/spec/helpers/gitlab_markdown_helper_spec.rb
+++ b/spec/helpers/gitlab_markdown_helper_spec.rb
@@ -170,4 +170,14 @@ describe GitlabMarkdownHelper do
       expect(doc.content).to eq "@#{user.username}, can you look at this?..."
     end
   end
+
+  describe '#cross_project_reference' do
+    it 'shows the full MR reference' do
+      expect(helper.cross_project_reference(project, merge_request)).to include(project.path_with_namespace)
+    end
+
+    it 'shows the full issue reference' do
+      expect(helper.cross_project_reference(project, issue)).to include(project.path_with_namespace)
+    end
+  end
 end
diff --git a/spec/models/group_label_spec.rb b/spec/models/group_label_spec.rb
index 668aa6fb357..555a876daeb 100644
--- a/spec/models/group_label_spec.rb
+++ b/spec/models/group_label_spec.rb
@@ -43,7 +43,7 @@ describe GroupLabel, models: true do
       let(:target_project) { build_stubbed(:empty_project, name: 'project-2', namespace: namespace) }
 
       it 'returns a String reference to the object' do
-        expect(label.to_reference(source_project, target_project)).to eq %(project-1~#{label.id})
+        expect(label.to_reference(source_project, target_project: target_project)).to eq %(project-1~#{label.id})
       end
     end
 
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index bb56e44db29..bfa36d92ac3 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -30,6 +30,10 @@ describe Issue, models: true do
       expect(issue.to_reference).to eq "#1"
     end
 
+    it 'returns a String reference with the full path' do
+      expect(issue.to_reference(full: true)).to eq(project.path_with_namespace + '#1')
+    end
+
     it 'supports a cross-project reference' do
       another_project = build(:project, name: 'another-project', namespace: project.namespace)
       expect(issue.to_reference(another_project)).to eq "sample-project#1"
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 646e6c6dbb3..8d1385016fd 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -153,6 +153,10 @@ describe MergeRequest, models: true do
       another_project = build(:project, name: 'another-project', namespace: project.namespace)
       expect(merge_request.to_reference(another_project)).to eq "sample-project!1"
     end
+
+    it 'returns a String reference with the full path' do
+      expect(merge_request.to_reference(full: true)).to eq(project.path_with_namespace + '!1')
+    end
   end
 
   describe '#raw_diffs' do