From f82115885d07e4eefec3594bdfc4bcd0210a1277 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@gitlab.com>
Date: Wed, 8 Mar 2017 16:39:20 +0100
Subject: [PATCH] Build link to discussions to resolve in a helper

---
 app/helpers/issues_helper.rb              | 14 ++++++++++
 app/views/shared/issuable/_form.html.haml |  5 ++--
 spec/helpers/issues_helper_spec.rb        | 32 +++++++++++++++++++++++
 3 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index 4bdf07fe1ad..63b6fde60c3 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -134,6 +134,20 @@ module IssuesHelper
     options_from_collection_for_select(options, 'name', 'title', params[:due_date])
   end
 
+  def link_to_discussions_to_resolve(merge_request, single_discussion)
+    link_text = merge_request.to_reference
+    link_text += " (discussion #{single_discussion.first_note.id})" if single_discussion
+
+    path = if single_discussion
+             Gitlab::UrlBuilder.build(single_discussion.first_note)
+           else
+             project = merge_request.project
+             namespace_project_merge_request_path(project.namespace, project, merge_request)
+           end
+
+    link_to link_text, path
+  end
+
   # Required for Banzai::Filter::IssueReferenceFilter
   module_function :url_for_issue
 end
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 907eb399c61..50b4bcff3bd 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -54,13 +54,12 @@
         - if @discussion_to_resolve
           = hidden_field_tag 'discussion_to_resolve', @discussion_to_resolve.id
           Creating this issue will resolve the discussion in
-          = link_to "#{@merge_request_for_resolving_discussions.to_reference} (discussion #{@discussion_to_resolve.first_note.id})", Gitlab::UrlBuilder.build(@discussion_to_resolve.first_note)
         - else
           Creating this issue will resolve all discussions in
-          = link_to @merge_request_for_resolving_discussions.to_reference, merge_request_path(@merge_request_for_resolving_discussions)
+        = link_to_discussions_to_resolve(@merge_request_for_resolving_discussions, @discussion_to_resolve)
       - else
         The discussion at
-        = link_to @merge_request_for_resolving_discussions.to_reference, merge_request_path(@merge_request_for_resolving_discussions)
+        = link_to_discussions_to_resolve(@merge_request_for_resolving_discussions, @discussion_to_resolve)
         will stay unresolved. Ask someone with permission to resolve it.
 
 - is_footer = !(issuable.is_a?(MergeRequest) && issuable.new_record?)
diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb
index 88d853935c7..f0554cc068d 100644
--- a/spec/helpers/issues_helper_spec.rb
+++ b/spec/helpers/issues_helper_spec.rb
@@ -131,4 +131,36 @@ describe IssuesHelper do
       expect(options).to have_selector('option', text: milestone2.title)
     end
   end
+
+  describe "#link_to_discussions_to_resolve" do
+    describe "passing only a merge request" do
+      let(:merge_request) { create(:merge_request) }
+
+      it "links just the merge request" do
+        expected_path = namespace_project_merge_request_path(merge_request.project.namespace, merge_request.project, merge_request)
+
+        expect(link_to_discussions_to_resolve(merge_request, nil)).to include(expected_path)
+      end
+
+      it "containst the reference to the merge request" do
+        expect(link_to_discussions_to_resolve(merge_request, nil)).to include(merge_request.to_reference)
+      end
+    end
+
+    describe "when passing a discussion" do
+      let(:diff_note) {  create(:diff_note_on_merge_request) }
+      let(:merge_request) { diff_note.noteable }
+      let(:discussion) { Discussion.new([diff_note]) }
+
+      it "links to the merge request with first note if a single discussion was passed" do
+        expected_path = Gitlab::UrlBuilder.build(diff_note)
+
+        expect(link_to_discussions_to_resolve(merge_request, discussion)).to include(expected_path)
+      end
+
+      it "contains both the reference to the merge request and a mention of the discussion" do
+        expect(link_to_discussions_to_resolve(merge_request, discussion)).to include("#{merge_request.to_reference} (discussion #{diff_note.id})")
+      end
+    end
+  end
 end