Merge branch 'jk-group-mentions-fix' into 'master'

Fix link text from group context

Closes gitlab-ee#4100

See merge request gitlab-org/gitlab-ce!15565

app/controllers/concerns/preview_markdown.rb

@@ -8,6 +8,7 @@ module PreviewMarkdown
       case controller_name
       when 'wikis'    then { pipeline: :wiki, project_wiki: @project_wiki, page_slug: params[:id] }
       when 'snippets' then { skip_project_check: true }
+      when 'groups'   then { group: group }
       else {}
       end
 

app/models/commit.rb

@@ -109,12 +109,12 @@ class Commit
     @link_reference_pattern ||= super("commit", /(?<commit>#{COMMIT_SHA_PATTERN})/)
   end
 
-  def to_reference(from_project = nil, full: false)
-    commit_reference(from_project, id, full: full)
+  def to_reference(from = nil, full: false)
+    commit_reference(from, id, full: full)
   end
 
-  def reference_link_text(from_project = nil, full: false)
-    commit_reference(from_project, short_id, full: full)
+  def reference_link_text(from = nil, full: false)
+    commit_reference(from, short_id, full: full)
   end
 
   def diff_line_count
@@ -381,8 +381,8 @@ class Commit
 
   private
 
-  def commit_reference(from_project, referable_commit_id, full: false)
-    reference = project.to_reference(from_project, full: full)
+  def commit_reference(from, referable_commit_id, full: false)
+    reference = project.to_reference(from, full: full)
 
     if reference.present?
       "#{reference}#{self.class.reference_prefix}#{referable_commit_id}"

app/models/commit_range.rb

@@ -89,8 +89,8 @@ class CommitRange
 
   alias_method :id, :to_s
 
-  def to_reference(from_project = nil, full: false)
-    project_reference = project.to_reference(from_project, full: full)
+  def to_reference(from = nil, full: false)
+    project_reference = project.to_reference(from, full: full)
 
     if project_reference.present?
       project_reference + self.class.reference_prefix + self.id
@@ -99,8 +99,8 @@ class CommitRange
     end
   end
 
-  def reference_link_text(from_project = nil)
-    project_reference = project.to_reference(from_project)
+  def reference_link_text(from = nil)
+    project_reference = project.to_reference(from)
     reference         = ref_from + notation + ref_to
 
     if project_reference.present?

app/models/concerns/mentionable.rb

@@ -31,11 +31,11 @@ module Mentionable
   #
   # By default this will be the class name and the result of calling
   # `to_reference` on the object.
-  def gfm_reference(from_project = nil)
+  def gfm_reference(from = nil)
     # "MergeRequest" > "merge_request" > "Merge request" > "merge request"
     friendly_name = self.class.to_s.underscore.humanize.downcase
 
-    "#{friendly_name} #{to_reference(from_project)}"
+    "#{friendly_name} #{to_reference(from)}"
   end
 
   # The GFM reference to this Mentionable, which shouldn't be included in its #references.

app/models/concerns/referable.rb

@@ -7,7 +7,7 @@ module Referable
 
   # Returns the String necessary to reference this object in Markdown
   #
-  # from_project - Refering Project object
+  # from - Referring parent object
   #
   # This should be overridden by the including class.
   #
@@ -17,12 +17,12 @@ module Referable
   #   Issue.last.to_reference(other_project) # => "cross-project#1"
   #
   # Returns a String
-  def to_reference(_from_project = nil, full:)
+  def to_reference(_from = nil, full:)
     ''
   end
 
-  def reference_link_text(from_project = nil)
-    to_reference(from_project)
+  def reference_link_text(from = nil)
+    to_reference(from)
   end
 
   included do

app/models/external_issue.rb

@@ -38,11 +38,11 @@ class ExternalIssue
     @project.id
   end
 
-  def to_reference(_from_project = nil, full: nil)
+  def to_reference(_from = nil, full: nil)
     id
   end
 
-  def reference_link_text(from_project = nil)
+  def reference_link_text(from = nil)
     return "##{id}" if id =~ /^\d+$/
 
     id

app/models/group.rb

@@ -97,7 +97,7 @@ class Group < Namespace
     end
   end
 
-  def to_reference(_from_project = nil, full: nil)
+  def to_reference(_from = nil, full: nil)
     "#{self.class.reference_prefix}#{full_path}"
   end
 

app/models/label.rb

@@ -165,12 +165,12 @@ class Label < ActiveRecord::Base
   #
   # Returns a String
   #
-  def to_reference(from_project = nil, target_project: nil, format: :id, full: false)
+  def to_reference(from = nil, target_project: nil, format: :id, full: false)
     format_reference = label_format_reference(format)
     reference = "#{self.class.reference_prefix}#{format_reference}"
 
-    if from_project
-      "#{from_project.to_reference(target_project, full: full)}#{reference}"
+    if from
+      "#{from.to_reference(target_project, full: full)}#{reference}"
     else
       reference
     end

app/models/milestone.rb

@@ -162,18 +162,18 @@ 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: :name, full: false)
+  def to_reference(from = nil, format: :name, full: false)
     format_reference = milestone_format_reference(format)
     reference = "#{self.class.reference_prefix}#{format_reference}"
 
     if project
-      "#{project.to_reference(from_project, full: full)}#{reference}"
+      "#{project.to_reference(from, full: full)}#{reference}"
     else
       reference
     end
   end
 
-  def reference_link_text(from_project = nil)
+  def reference_link_text(from = nil)
     self.title
   end
 

app/models/project.rb

@@ -760,10 +760,10 @@ class Project < ActiveRecord::Base
     end
   end
 
-  def to_human_reference(from_project = nil)
-    if cross_namespace_reference?(from_project)
+  def to_human_reference(from = nil)
+    if cross_namespace_reference?(from)
       name_with_namespace
-    elsif cross_project_reference?(from_project)
+    elsif cross_project_reference?(from)
       name
     end
   end

app/models/snippet.rb

@@ -75,11 +75,11 @@ class Snippet < ActiveRecord::Base
     @link_reference_pattern ||= super("snippets", /(?<snippet>\d+)/)
   end
 
-  def to_reference(from_project = nil, full: false)
+  def to_reference(from = nil, full: false)
     reference = "#{self.class.reference_prefix}#{id}"
 
     if project.present?
-      "#{project.to_reference(from_project, full: full)}#{reference}"
+      "#{project.to_reference(from, full: full)}#{reference}"
     else
       reference
     end

app/models/user.rb

@@ -437,7 +437,7 @@ class User < ActiveRecord::Base
     username
   end
 
-  def to_reference(_from_project = nil, target_project: nil, full: nil)
+  def to_reference(_from = nil, target_project: nil, full: nil)
     "#{self.class.reference_prefix}#{username}"
   end
 

changelogs/unreleased/jk-group-mentions-fix.yml

@@ -0,0 +1,5 @@
+---
+title: Fix link text from group context
+merge_request:
+author:
+type: fixed

lib/banzai/filter/abstract_reference_filter.rb

@@ -213,7 +213,8 @@ module Banzai
       end
 
       def object_link_text(object, matches)
-        text = object.reference_link_text(context[:project])
+        parent = context[:project] || context[:group]
+        text = object.reference_link_text(parent)
 
         extras = object_link_text_extras(object, matches)
         text += " (#{extras.join(", ")})" if extras.any?

spec/lib/banzai/filter/issue_reference_filter_spec.rb

@@ -343,7 +343,9 @@ describe Banzai::Filter::IssueReferenceFilter do
       reference = "#{project.full_path}##{issue.iid}"
       doc = reference_filter("See #{reference}", context)
 
-      expect(doc.css('a').first.attr('href')).to eq helper.url_for_issue(issue.iid, project)
+      link = doc.css('a').first
+      expect(link.attr('href')).to eq(helper.url_for_issue(issue.iid, project))
+      expect(link.text).to include("#{project.full_path}##{issue.iid}")
     end
 
     it 'ignores reference for shorthand cross-reference' do
@@ -358,7 +360,9 @@ describe Banzai::Filter::IssueReferenceFilter do
 
       doc = reference_filter("See #{reference}", context)
 
-      expect(doc.css('a').first.attr('href')).to eq(helper.url_for_issue(issue.iid, project) + "#note_123")
+      link = doc.css('a').first
+      expect(link.attr('href')).to eq(helper.url_for_issue(issue.iid, project) + "#note_123")
+      expect(link.text).to include("#{project.full_path}##{issue.iid}")
     end
 
     it 'links to a valid reference for cross-reference in link href' do
@@ -367,7 +371,9 @@ describe Banzai::Filter::IssueReferenceFilter do
 
       doc = reference_filter("See #{reference_link}", context)
 
-      expect(doc.css('a').first.attr('href')).to eq helper.url_for_issue(issue.iid, project) + "#note_123"
+      link = doc.css('a').first
+      expect(link.attr('href')).to eq(helper.url_for_issue(issue.iid, project) + "#note_123")
+      expect(link.text).to include('Reference')
     end
 
     it 'links to a valid reference for issue reference in the link href' do
@@ -375,7 +381,9 @@ describe Banzai::Filter::IssueReferenceFilter do
       reference_link = %{<a href="#{reference}">Reference</a>}
       doc = reference_filter("See #{reference_link}", context)
 
-      expect(doc.css('a').first.attr('href')).to eq helper.url_for_issue(issue.iid, project)
+      link = doc.css('a').first
+      expect(link.attr('href')).to eq(helper.url_for_issue(issue.iid, project))
+      expect(link.text).to include('Reference')
     end
   end