format merge request references properly

CHANGELOG

@@ -74,6 +74,7 @@ v 8.7.0 (unreleased)
   - Selected diff rows highlight
   - Fix emoji categories in the emoji picker
   - Add encrypted credentials for imported projects and migrate old ones
+  - Properly format all merge request references with ! rather than # !3740 (Ben Bodenmiller)
   - Author and participants are displayed first on users autocompletion
 
 v 8.6.6

app/mailers/emails/merge_requests.rb

@@ -56,7 +56,7 @@ module Emails
       {
         from: sender(sender_id),
         to: recipient(recipient_id),
-        subject: subject("#{@merge_request.title} (##{@merge_request.iid})")
+        subject: subject("#{@merge_request.title} (#{@merge_request.to_reference})")
       }
     end
   end

app/mailers/emails/notes.rb

@@ -38,7 +38,7 @@ module Emails
       {
         from: sender(@note.author_id),
         to: recipient(recipient_id),
-        subject: subject("#{@note.noteable.title} (##{@note.noteable.iid})")
+        subject: subject("#{@note.noteable.title} (#{@note.noteable.to_reference})")
       }
     end
 

app/models/project_services/hipchat_service.rb

@@ -183,7 +183,7 @@ class HipchatService < Service
     title = obj_attr[:title]
 
     merge_request_url = "#{project_url}/merge_requests/#{merge_request_id}"
-    merge_request_link = "<a href=\"#{merge_request_url}\">merge request ##{merge_request_id}</a>"
+    merge_request_link = "<a href=\"#{merge_request_url}\">merge request !#{merge_request_id}</a>"
     message = "#{user_name} #{state} #{merge_request_link} in " \
       "#{project_link}: <b>#{title}</b>"
 
@@ -224,7 +224,7 @@ class HipchatService < Service
     when "MergeRequest"
       subj_attr = HashWithIndifferentAccess.new(data[:merge_request])
       subject_id = subj_attr[:iid]
-      subject_desc = "##{subject_id}"
+      subject_desc = "!#{subject_id}"
       subject_type = "merge request"
       title = format_title(subj_attr[:title])
     when "Snippet"

app/models/project_services/slack_service/merge_message.rb

@@ -50,7 +50,7 @@ class SlackService
     end
 
     def merge_request_link
-      "[merge request ##{merge_request_id}](#{merge_request_url})"
+      "[merge request !#{merge_request_id}](#{merge_request_url})"
     end
 
     def merge_request_url

app/models/project_services/slack_service/note_message.rb

@@ -58,7 +58,7 @@ class SlackService
 
     def create_merge_note(merge_request)
       commented_on_message(
-        "[merge request ##{merge_request[:iid]}](#{@note_url})",
+        "[merge request !#{merge_request[:iid]}](#{@note_url})",
         format_title(merge_request[:title]))
     end
 

app/views/notify/closed_merge_request_email.html.haml

 %p
-  = "Merge Request ##{@merge_request.iid} was closed by #{@updated_by.name}"
+  = "Merge Request #{@merge_request.to_reference} was closed by #{@updated_by.name}"

app/views/notify/closed_merge_request_email.text.haml

-= "Merge Request ##{@merge_request.iid} was closed by #{@updated_by.name}"
+= "Merge Request #{@merge_request.to_reference} was closed by #{@updated_by.name}"
 
 Merge Request url: #{namespace_project_merge_request_url(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request)}
 

app/views/notify/merge_request_status_email.html.haml

 %p
-  = "Merge Request ##{@merge_request.iid} was #{@mr_status} by #{@updated_by.name}"
+  = "Merge Request #{@merge_request.to_reference} was #{@mr_status} by #{@updated_by.name}"

app/views/notify/merge_request_status_email.text.haml

-= "Merge Request ##{@merge_request.iid} was #{@mr_status} by #{@updated_by.name}"
+= "Merge Request #{@merge_request.to_reference} was #{@mr_status} by #{@updated_by.name}"
 
 Merge Request url: #{namespace_project_merge_request_url(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request)}
 

app/views/notify/merged_merge_request_email.html.haml

 %p
-  = "Merge Request ##{@merge_request.iid} was merged"
+  = "Merge Request #{@merge_request.to_reference} was merged"

app/views/notify/merged_merge_request_email.text.haml

-= "Merge Request ##{@merge_request.iid} was merged"
+= "Merge Request #{@merge_request.to_reference} was merged"
 
 Merge Request url: #{namespace_project_merge_request_url(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request)}
 

app/views/notify/new_merge_request_email.text.erb

-New Merge Request #<%= @merge_request.iid %>
+New Merge Request <%= @merge_request.to_reference %>
 
 <%= url_for(namespace_project_merge_request_url(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request)) %>
 

app/views/notify/note_merge_request_email.text.erb

-New comment for Merge Request <%= @merge_request.iid %>
+New comment for Merge Request <%= @merge_request.to_reference %>
 
 <%= url_for(namespace_project_merge_request_url(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, anchor: "note_#{@note.id}")) %>
 

app/views/projects/builds/show.html.haml

@@ -10,7 +10,7 @@
     - merge_request = @build.merge_request
     - if merge_request
       via
-      = link_to "merge request ##{merge_request.iid}", merge_request_path(merge_request)
+      = link_to "merge request #{merge_request.to_reference}", merge_request_path(merge_request)
 
   #up-build-trace
   - builds = @build.commit.matrix_builds(@build)

app/views/projects/merge_requests/edit.html.haml

-- page_title "Edit", "#{@merge_request.title} (##{@merge_request.iid})", "Merge Requests"
+- page_title "Edit", "#{@merge_request.title} (#{@merge_request.to_reference}", "Merge Requests"
 = render "header_title"
 
 %h3.page-title
-  Edit Merge Request ##{@merge_request.iid}
+  Edit Merge Request #{@merge_request.to_reference}
 %hr
 = render 'form'

app/views/projects/merge_requests/invalid.html.haml

-- page_title "#{@merge_request.title} (##{@merge_request.iid})", "Merge Requests"
+- page_title "#{@merge_request.title} (#{merge_request.to_reference}", "Merge Requests"
 = render "header_title"
 
 .merge-request

app/views/search/results/_merge_request.html.haml

@@ -2,7 +2,7 @@
   %h4
     = link_to [merge_request.target_project.namespace.becomes(Namespace), merge_request.target_project, merge_request] do
       %span.term.str-truncated= merge_request.title
-    .pull-right ##{merge_request.iid}
+    .pull-right #{merge_request.to_reference}
   - if merge_request.description.present?
     .description.term
       = preserve do

features/steps/dashboard/todos.rb

@@ -31,7 +31,7 @@ class Spinach::Features::DashboardTodos < Spinach::FeatureSteps
     expect(page).to have_content 'Done 0'
 
     expect(page).to have_link project.name_with_namespace
-    should_see_todo(1, "John Doe assigned you merge request !#{merge_request.iid}", merge_request.title)
+    should_see_todo(1, "John Doe assigned you merge request #{merge_request.to_reference}", merge_request.title)
     should_see_todo(2, "John Doe mentioned you on issue ##{issue.iid}", "#{current_user.to_reference} Wdyt?")
     should_see_todo(3, "John Doe assigned you issue ##{issue.iid}", issue.title)
     should_see_todo(4, "Mary Jane mentioned you on issue ##{issue.iid}", issue.title)
@@ -45,7 +45,7 @@ class Spinach::Features::DashboardTodos < Spinach::FeatureSteps
     page.within('.nav-sidebar') { expect(page).to have_content 'Todos 3' }
     expect(page).to have_content 'To do 3'
     expect(page).to have_content 'Done 1'
-    should_not_see_todo "John Doe assigned you merge request !#{merge_request.iid}"
+    should_not_see_todo "John Doe assigned you merge request #{merge_request.to_reference}"
   end
 
   step 'I click on the "Done" tab' do
@@ -54,7 +54,7 @@ class Spinach::Features::DashboardTodos < Spinach::FeatureSteps
 
   step 'I should see all todos marked as done' do
     expect(page).to have_link project.name_with_namespace
-    should_see_todo(1, "John Doe assigned you merge request !#{merge_request.iid}", merge_request.title, false)
+    should_see_todo(1, "John Doe assigned you merge request #{merge_request.to_reference}", merge_request.title, false)
   end
 
   step 'I filter by "Enterprise"' do
@@ -82,11 +82,11 @@ class Spinach::Features::DashboardTodos < Spinach::FeatureSteps
   end
 
   step 'I should not see todos related to "Merge Requests" in the list' do
-    should_not_see_todo "John Doe assigned you merge request !#{merge_request.iid}"
+    should_not_see_todo "John Doe assigned you merge request #{merge_request.to_reference}"
   end
 
   step 'I should not see todos related to "Assignments" in the list' do
-    should_not_see_todo "John Doe assigned you merge request !#{merge_request.iid}"
+    should_not_see_todo "John Doe assigned you merge request #{merge_request.to_reference}"
     should_not_see_todo "John Doe assigned you issue ##{issue.iid}"
   end
 

features/steps/project/forked_merge_requests.rb

@@ -114,7 +114,7 @@ class Spinach::Features::ProjectForkedMergeRequests < Spinach::FeatureSteps
 
   step 'I see the edit page prefilled for "Merge Request On Forked Project"' do
     expect(current_path).to eq edit_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)
-    expect(page).to have_content "Edit merge request ##{@merge_request.id}"
+    expect(page).to have_content "Edit merge request #{@merge_request.to_reference}"
     expect(find("#merge_request_title").value).to eq "Merge Request On Forked Project"
   end
 

spec/mailers/notify_spec.rb

@@ -213,7 +213,7 @@ describe Notify do
           it_behaves_like 'an unsubscribeable thread'
 
           it 'has the correct subject' do
-            is_expected.to have_subject /#{merge_request.title} \(##{merge_request.iid}\)/
+            is_expected.to have_subject /#{merge_request.title} \(#{merge_request.to_reference}\)/
           end
 
           it 'contains a link to the new merge request' do
@@ -268,7 +268,7 @@ describe Notify do
           end
 
           it 'has the correct subject' do
-            is_expected.to have_subject /#{merge_request.title} \(##{merge_request.iid}\)/
+            is_expected.to have_subject /#{merge_request.title} \(#{merge_request.to_reference}\)/
           end
 
           it 'contains the name of the previous assignee' do
@@ -302,7 +302,7 @@ describe Notify do
           end
 
           it 'has the correct subject' do
-            is_expected.to have_subject /#{merge_request.title} \(##{merge_request.iid}\)/
+            is_expected.to have_subject /#{merge_request.title} \(#{merge_request.to_reference}\)/
           end
 
           it 'contains the names of the added labels' do
@@ -331,7 +331,7 @@ describe Notify do
           end
 
           it 'has the correct subject' do
-            is_expected.to have_subject /#{merge_request.title} \(##{merge_request.iid}\)/i
+            is_expected.to have_subject /#{merge_request.title} \(#{merge_request.to_reference}\)/i
           end
 
           it 'contains the new status' do
@@ -364,7 +364,7 @@ describe Notify do
           end
 
           it 'has the correct subject' do
-            is_expected.to have_subject /#{merge_request.title} \(##{merge_request.iid}\)/
+            is_expected.to have_subject /#{merge_request.title} \(#{merge_request.to_reference}\)/
           end
 
           it 'contains the new status' do
@@ -502,7 +502,7 @@ describe Notify do
         it_behaves_like 'an unsubscribeable thread'
 
         it 'has the correct subject' do
-          is_expected.to have_subject /#{merge_request.title} \(##{merge_request.iid}\)/
+          is_expected.to have_subject /#{merge_request.title} \(#{merge_request.to_reference}\)/
         end
 
         it 'contains a link to the merge request note' do

spec/models/project_services/hipchat_service_spec.rb

@@ -152,7 +152,7 @@ describe HipchatService, models: true do
 
         obj_attr = merge_sample_data[:object_attributes]
         expect(message).to eq("#{user.name} opened " \
-            "<a href=\"#{obj_attr[:url]}\">merge request ##{obj_attr["iid"]}</a> in " \
+            "<a href=\"#{obj_attr[:url]}\">merge request !#{obj_attr["iid"]}</a> in " \
             "<a href=\"#{project.web_url}\">#{project_name}</a>: " \
             "<b>Awesome merge request</b>" \
             "<pre>please fix</pre>")
@@ -202,7 +202,7 @@ describe HipchatService, models: true do
         title = data[:merge_request]['title']
 
         expect(message).to eq("#{user.name} commented on " \
-            "<a href=\"#{obj_attr[:url]}\">merge request ##{merge_id}</a> in " \
+            "<a href=\"#{obj_attr[:url]}\">merge request !#{merge_id}</a> in " \
             "<a href=\"#{project.web_url}\">#{project_name}</a>: " \
             "<b>#{title}</b>" \
             "<pre>merge request note</pre>")

spec/models/project_services/slack_service/merge_message_spec.rb

@@ -31,7 +31,7 @@ describe SlackService::MergeMessage, models: true do
   context 'open' do
     it 'returns a message regarding opening of merge requests' do
       expect(subject.pretext).to eq(
-        'Test User opened <somewhere.com/merge_requests/100|merge request #100> '\
+        'Test User opened <somewhere.com/merge_requests/100|merge request !100> '\
         'in <somewhere.com|project_name>: *Issue title*')
       expect(subject.attachments).to be_empty
     end
@@ -43,7 +43,7 @@ describe SlackService::MergeMessage, models: true do
     end
     it 'returns a message regarding closing of merge requests' do
       expect(subject.pretext).to eq(
-        'Test User closed <somewhere.com/merge_requests/100|merge request #100> '\
+        'Test User closed <somewhere.com/merge_requests/100|merge request !100> '\
         'in <somewhere.com|project_name>: *Issue title*')
       expect(subject.attachments).to be_empty
     end

spec/models/project_services/slack_service/note_message_spec.rb

@@ -63,7 +63,7 @@ describe SlackService::NoteMessage, models: true do
     it 'returns a message regarding notes on a merge request' do
       message = SlackService::NoteMessage.new(@args)
       expect(message.pretext).to eq("Test User commented on " \
-      "<url|merge request #30> in <somewhere.com|project_name>: " \
+      "<url|merge request !30> in <somewhere.com|project_name>: " \
       "*merge request title*")
       expected_attachments =  [
           {