Merge branch 'rymai/gitlab-ce-3007-fix-mr-label-links' into 'master'

Fix label links for a merge request pointing to issues list

_Originally opened at !2150._

---

I have added a type parameter to the `link_to_label` helper method that is set to `'merge_request'` when in the context of a merge request.

I have also improved the specs to actually test the output of the method instead of its implementation.

Fixes #3007.

See merge request !2700

5 files changed, 29 insertions, 24 deletions

diff --git a/CHANGELOG b/CHANGELOG
index b7e8822fdd6..d43b8f69063 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -13,6 +13,7 @@ v 8.5.0 (unreleased)
     set it up
   - Fix diff comments loaded by AJAX to load comment with diff in discussion tab
   - Whitelist raw "abbr" elements when parsing Markdown (Benedict Etzel)
+  - Fix label links for a merge request pointing to issues list
   - Don't vendor minified JS
   - Display 404 error on group not found
   - Track project import failure
diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb
index 92eac0560bd..1c7fcc13b42 100644
--- a/app/helpers/labels_helper.rb
+++ b/app/helpers/labels_helper.rb
@@ -7,6 +7,8 @@ module LabelsHelper
   # project - Project object which will be used as the context for the label's
   #           link. If omitted, defaults to `@project`, or the label's own
   #           project.
+  # type    - The type of item the link will point to (:issue or
+  #           :merge_request). If omitted, defaults to :issue.
   # block   - An optional block that will be passed to `link_to`, forming the
   #           body of the link element. If omitted, defaults to
   #           `render_colored_label`.
@@ -23,14 +25,19 @@ module LabelsHelper
   #   # Force the generated link to use a provided project
   #   link_to_label(label, project: Project.last)
   #
+  #   # Force the generated link to point to merge requests instead of issues
+  #   link_to_label(label, type: :merge_request)
+  #
   #   # Customize link body with a block
   #   link_to_label(label) { "My Custom Label Text" }
   #
   # Returns a String
-  def link_to_label(label, project: nil, &block)
+  def link_to_label(label, project: nil, type: :issue, &block)
     project ||= @project || label.project
-    link = namespace_project_issues_path(project.namespace, project,
-                                         label_name: label.name)
+    link = send("namespace_project_#{type.to_s.pluralize}_path",
+                project.namespace,
+                project,
+                label_name: label.name)
 
     if block_given?
       link_to link, &block
diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml
index a051729dc32..e25bf917b43 100644
--- a/app/views/projects/merge_requests/_merge_request.html.haml
+++ b/app/views/projects/merge_requests/_merge_request.html.haml
@@ -53,7 +53,7 @@
     - if merge_request.labels.any?
        
       - merge_request.labels.each do |label|
-        = link_to_label(label, project: merge_request.project)
+        = link_to_label(label, project: merge_request.project, type: 'merge_request')
     - if merge_request.tasks?
        
       %span.task-status
diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml
index cab500d7244..75a7d9be2c1 100644
--- a/app/views/shared/issuable/_sidebar.html.haml
+++ b/app/views/shared/issuable/_sidebar.html.haml
@@ -90,7 +90,7 @@
           .value.issuable-show-labels
             - if issuable.labels.any?
               - issuable.labels.each do |label|
-                = link_to_label(label)
+                = link_to_label(label, type: issuable.to_ability_name)
             - else
               .light None
           .selectbox
@@ -129,4 +129,4 @@
 
     :javascript
       new Subscription("#{toggle_subscription_path(issuable)}");
-      new IssuableContext();
\ No newline at end of file
+      new IssuableContext();
diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb
index 0b9176357bc..4f129eca183 100644
--- a/spec/helpers/labels_helper_spec.rb
+++ b/spec/helpers/labels_helper_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 describe LabelsHelper do
   describe 'link_to_label' do
     let(:project) { create(:empty_project) }
-    let(:label)   { create(:label, project: project) }
+    let(:label) { create(:label, project: project) }
 
     context 'with @project set' do
       before do
@@ -11,34 +11,31 @@ describe LabelsHelper do
       end
 
       it 'uses the instance variable' do
-        expect(label).not_to receive(:project)
-        link_to_label(label)
+        expect(link_to_label(label)).to match %r{<a href="/#{@project.to_reference}/issues\?label_name=#{label.name}">.*</a>}
       end
     end
 
     context 'without @project set' do
       it "uses the label's project" do
-        expect(label).to receive(:project).and_return(project)
-        link_to_label(label)
+        expect(link_to_label(label)).to match %r{<a href="/#{label.project.to_reference}/issues\?label_name=#{label.name}">.*</a>}
       end
     end
 
-    context 'with a named project argument' do
-      it 'uses the provided project' do
-        arg = double('project')
-        expect(arg).to receive(:namespace).and_return('foo')
-        expect(arg).to receive(:to_param).and_return('foo')
+    context 'with a project argument' do
+      let(:another_project) { double('project', namespace: 'foo3', to_param: 'bar3') }
 
-        link_to_label(label, project: arg)
+      it 'links to merge requests page' do
+        expect(link_to_label(label, project: another_project)).to match %r{<a href="/foo3/bar3/issues\?label_name=#{label.name}">.*</a>}
       end
+    end
 
-      it 'takes precedence over other types' do
-        @project = project
-        expect(@project).not_to receive(:namespace)
-        expect(label).not_to receive(:project)
-
-        arg = double('project', namespace: 'foo', to_param: 'foo')
-        link_to_label(label, project: arg)
+    context 'with a type argument' do
+      ['issue', :issue, 'merge_request', :merge_request].each do |type|
+        context "set to #{type}" do
+          it 'links to correct page' do
+            expect(link_to_label(label, type: type)).to match %r{<a href="/#{label.project.to_reference}/#{type.to_s.pluralize}\?label_name=#{label.name}">.*</a>}
+          end
+        end
       end
     end