Merge branch 'fix_mr_note_label_urls' into 'master'

Fix Merge Request Note Label URLs

See merge request gitlab-org/gitlab!30428

app/models/issue.rb

@@ -344,6 +344,10 @@ class Issue < ApplicationRecord
     previous_changes['updated_at']&.first || updated_at
   end
 
+  def banzai_render_context(field)
+    super.merge(label_url_method: :project_issues_url)
+  end
+
   def design_collection
     @design_collection ||= ::DesignManagement::DesignCollection.new(self)
   end

app/models/merge_request.rb

@@ -1579,6 +1579,10 @@ class MergeRequest < ApplicationRecord
     deployments.visible.includes(:environment).order(id: :desc).limit(10)
   end
 
+  def banzai_render_context(field)
+    super.merge(label_url_method: :project_merge_requests_url)
+  end
+
   private
 
   def with_rebase_lock

app/models/note.rb

@@ -521,7 +521,7 @@ class Note < ApplicationRecord
   end
 
   def banzai_render_context(field)
-    super.merge(noteable: noteable, system_note: system?)
+    super.merge(noteable: noteable, system_note: system?, label_url_method: noteable_label_url_method)
   end
 
   def retrieve_upload(_identifier, paths)
@@ -604,6 +604,10 @@ class Note < ApplicationRecord
 
     errors.add(:base, _('Maximum number of comments exceeded')) if noteable.notes.count >= Noteable::MAX_NOTES_LIMIT
   end
+
+  def noteable_label_url_method
+    for_merge_request? ? :project_merge_requests_url : :project_issues_url
+  end
 end
 
 Note.prepend_if_ee('EE::Note')

changelogs/unreleased/fix_mr_note_label_urls.yml

+---
+title: Fix merge request note label URLs
+merge_request: 30428
+author: Lee Tickett
+type: fixed

lib/banzai/filter/label_reference_filter.rb

@@ -71,13 +71,16 @@ module Banzai
       end
 
       def url_for_object(label, parent)
-        h = Gitlab::Routing.url_helpers
+        label_url_method =
+          if context[:label_url_method]
+            context[:label_url_method]
+          elsif parent.is_a?(Project)
+            :project_issues_url
+          end
 
-        if parent.is_a?(Project)
-          h.project_issues_url(parent, label_name: label.name, only_path: context[:only_path])
-        elsif context[:label_url_method]
-          h.public_send(context[:label_url_method], parent, label_name: label.name, only_path: context[:only_path]) # rubocop:disable GitlabSecurity/PublicSend
-        end
+        return unless label_url_method
+
+        Gitlab::Routing.url_helpers.public_send(label_url_method, parent, label_name: label.name, only_path: context[:only_path]) # rubocop:disable GitlabSecurity/PublicSend
       end
 
       def object_link_text(object, matches)

spec/lib/banzai/filter/label_reference_filter_spec.rb

@@ -47,14 +47,34 @@ describe Banzai::Filter::LabelReferenceFilter do
     expect(link.attr('data-label')).to eq label.id.to_s
   end
 
-  it 'supports an :only_path context' do
+  it 'includes protocol when :only_path not present' do
+    doc = reference_filter("Label #{reference}")
+    link = doc.css('a').first.attr('href')
+
+    expect(link).to match %r(https?://)
+  end
+
+  it 'does not include protocol when :only_path true' do
     doc = reference_filter("Label #{reference}", only_path: true)
     link = doc.css('a').first.attr('href')
 
     expect(link).not_to match %r(https?://)
+  end
+
+  it 'links to issue list when :label_url_method is not present' do
+    doc = reference_filter("Label #{reference}", only_path: true)
+    link = doc.css('a').first.attr('href')
+
     expect(link).to eq urls.project_issues_path(project, label_name: label.name)
   end
 
+  it 'links to merge request list when `label_url_method: :project_merge_requests_url`' do
+    doc = reference_filter("Label #{reference}", { only_path: true, label_url_method: "project_merge_requests_url" })
+    link = doc.css('a').first.attr('href')
+
+    expect(link).to eq urls.project_merge_requests_path(project, label_name: label.name)
+  end
+
   context 'project that does not exist referenced' do
     let(:result) { reference_filter('aaa/bbb~ccc') }
 

spec/models/issue_spec.rb

@@ -1085,4 +1085,15 @@ describe Issue do
       expect(subject).not_to include(labeled_issue)
     end
   end
+
+  describe 'banzai_render_context' do
+    let(:project) { build(:project_empty_repo) }
+    let(:issue) { build :issue, project: project }
+
+    subject(:context) { issue.banzai_render_context(:title) }
+
+    it 'sets the label_url_method in the context' do
+      expect(context[:label_url_method]).to eq(:project_issues_url)
+    end
+  end
 end

spec/models/merge_request_spec.rb

@@ -3926,4 +3926,15 @@ describe MergeRequest do
       expect(count).to eq(0)
     end
   end
+
+  describe 'banzai_render_context' do
+    let(:project) { build(:project_empty_repo) }
+    let(:merge_request) { build :merge_request, target_project: project, source_project: project }
+
+    subject(:context) { merge_request.banzai_render_context(:title) }
+
+    it 'sets the label_url_method in the context' do
+      expect(context[:label_url_method]).to eq(:project_merge_requests_url)
+    end
+  end
 end

spec/models/note_spec.rb

@@ -1353,4 +1353,28 @@ describe Note do
       end
     end
   end
+
+  describe 'banzai_render_context' do
+    let(:project) { build(:project_empty_repo) }
+
+    context 'when noteable is a merge request' do
+      let(:noteable) { build :merge_request, target_project: project, source_project: project }
+
+      subject(:context) { noteable.banzai_render_context(:title) }
+
+      it 'sets the label_url_method in the context' do
+        expect(context[:label_url_method]).to eq(:project_merge_requests_url)
+      end
+    end
+
+    context 'when noteable is an issue' do
+      let(:noteable) { build :issue, project: project }
+
+      subject(:context) { noteable.banzai_render_context(:title) }
+
+      it 'sets the label_url_method in the context' do
+        expect(context[:label_url_method]).to eq(:project_issues_url)
+      end
+    end
+  end
 end