Merge branch 'sh-show-all-slack-names' into 'master'

Include GitLab full name with username in Slack messages

Closes #38865

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

app/models/project_services/chat_message/base_message.rb

@@ -3,6 +3,7 @@ require 'slack-notifier'
 module ChatMessage
   class BaseMessage
     attr_reader :markdown
+    attr_reader :user_full_name
     attr_reader :user_name
     attr_reader :user_avatar
     attr_reader :project_name
@@ -12,10 +13,19 @@ module ChatMessage
       @markdown = params[:markdown] || false
       @project_name = params.dig(:project, :path_with_namespace) || params[:project_name]
       @project_url = params.dig(:project, :web_url) || params[:project_url]
+      @user_full_name = params.dig(:user, :name) || params[:user_full_name]
       @user_name = params.dig(:user, :username) || params[:user_name]
       @user_avatar = params.dig(:user, :avatar_url) || params[:user_avatar]
     end
 
+    def user_combined_name
+      if user_full_name.present?
+        "#{user_full_name} (#{user_name})"
+      else
+        user_name
+      end
+    end
+
     def pretext
       return message if markdown
 

app/models/project_services/chat_message/issue_message.rb

@@ -29,7 +29,7 @@ module ChatMessage
 
     def activity
       {
-        title: "Issue #{state} by #{user_name}",
+        title: "Issue #{state} by #{user_combined_name}",
         subtitle: "in #{project_link}",
         text: issue_link,
         image: user_avatar
@@ -40,9 +40,9 @@ module ChatMessage
 
     def message
       if state == 'opened'
-        "[#{project_link}] Issue #{state} by #{user_name}"
+        "[#{project_link}] Issue #{state} by #{user_combined_name}"
       else
-        "[#{project_link}] Issue #{issue_link} #{state} by #{user_name}"
+        "[#{project_link}] Issue #{issue_link} #{state} by #{user_combined_name}"
       end
     end
 

app/models/project_services/chat_message/merge_message.rb

@@ -24,7 +24,7 @@ module ChatMessage
 
     def activity
       {
-        title: "Merge Request #{state} by #{user_name}",
+        title: "Merge Request #{state} by #{user_combined_name}",
         subtitle: "in #{project_link}",
         text: merge_request_link,
         image: user_avatar
@@ -46,7 +46,7 @@ module ChatMessage
     end
 
     def merge_request_message
-      "#{user_name} #{state} #{merge_request_link} in #{project_link}: #{title}"
+      "#{user_combined_name} #{state} #{merge_request_link} in #{project_link}: #{title}"
     end
 
     def merge_request_link

app/models/project_services/chat_message/note_message.rb

@@ -32,7 +32,7 @@ module ChatMessage
 
     def activity
       {
-        title: "#{user_name} #{link('commented on ' + target, note_url)}",
+        title: "#{user_combined_name} #{link('commented on ' + target, note_url)}",
         subtitle: "in #{project_link}",
         text: formatted_title,
         image: user_avatar
@@ -42,7 +42,7 @@ module ChatMessage
     private
 
     def message
-      "#{user_name} #{link('commented on ' + target, note_url)} in #{project_link}: *#{formatted_title}*"
+      "#{user_combined_name} #{link('commented on ' + target, note_url)} in #{project_link}: *#{formatted_title}*"
     end
 
     def format_title(title)

app/models/project_services/chat_message/pipeline_message.rb

@@ -9,7 +9,7 @@ module ChatMessage
     def initialize(data)
       super
 
-      @user_name = data.dig(:user, :name) || 'API'
+      @user_name = data.dig(:user, :username) || 'API'
 
       pipeline_attributes = data[:object_attributes]
       @ref_type = pipeline_attributes[:tag] ? 'tag' : 'branch'
@@ -35,7 +35,7 @@ module ChatMessage
 
     def activity
       {
-        title: "Pipeline #{pipeline_link} of #{ref_type} #{branch_link} by #{user_name} #{humanized_status}",
+        title: "Pipeline #{pipeline_link} of #{ref_type} #{branch_link} by #{user_combined_name} #{humanized_status}",
         subtitle: "in #{project_link}",
         text: "in #{pretty_duration(duration)}",
         image: user_avatar || ''
@@ -45,7 +45,7 @@ module ChatMessage
     private
 
     def message
-      "#{project_link}: Pipeline #{pipeline_link} of #{ref_type} #{branch_link} by #{user_name} #{humanized_status} in #{pretty_duration(duration)}"
+      "#{project_link}: Pipeline #{pipeline_link} of #{ref_type} #{branch_link} by #{user_combined_name} #{humanized_status} in #{pretty_duration(duration)}"
     end
 
     def humanized_status

app/models/project_services/chat_message/push_message.rb

@@ -33,7 +33,7 @@ module ChatMessage
                end
 
       {
-        title: "#{user_name} #{action} #{ref_type}",
+        title: "#{user_combined_name} #{action} #{ref_type}",
         subtitle: "in #{project_link}",
         text: compare_link,
         image: user_avatar
@@ -57,15 +57,15 @@ module ChatMessage
     end
 
     def new_branch_message
-      "#{user_name} pushed new #{ref_type} #{branch_link} to #{project_link}"
+      "#{user_combined_name} pushed new #{ref_type} #{branch_link} to #{project_link}"
     end
 
     def removed_branch_message
-      "#{user_name} removed #{ref_type} #{ref} from #{project_link}"
+      "#{user_combined_name} removed #{ref_type} #{ref} from #{project_link}"
     end
 
     def push_message
-      "#{user_name} pushed to #{ref_type} #{branch_link} of #{project_link} (#{compare_link})"
+      "#{user_combined_name} pushed to #{ref_type} #{branch_link} of #{project_link} (#{compare_link})"
     end
 
     def commit_messages

app/models/project_services/chat_message/wiki_page_message.rb

@@ -31,7 +31,7 @@ module ChatMessage
 
     def activity
       {
-        title: "#{user_name} #{action} #{wiki_page_link}",
+        title: "#{user_combined_name} #{action} #{wiki_page_link}",
         subtitle: "in #{project_link}",
         text: title,
         image: user_avatar
@@ -41,7 +41,7 @@ module ChatMessage
     private
 
     def message
-      "#{user_name} #{action} #{wiki_page_link} in #{project_link}: *#{title}*"
+      "#{user_combined_name} #{action} #{wiki_page_link} in #{project_link}: *#{title}*"
     end
 
     def description_message

changelogs/unreleased/sh-show-all-slack-names.yml

+---
+title: Include GitLab full name in Slack messages
+merge_request:
+author:
+type: changed

spec/models/project_services/chat_message/issue_message_spec.rb

@@ -42,7 +42,7 @@ describe ChatMessage::IssueMessage do
     context 'open' do
       it 'returns a message regarding opening of issues' do
         expect(subject.pretext).to eq(
-          '[<http://somewhere.com|project_name>] Issue opened by test.user')
+          '[<http://somewhere.com|project_name>] Issue opened by Test User (test.user)')
         expect(subject.attachments).to eq([
           {
             title: "#100 Issue title",
@@ -62,7 +62,7 @@ describe ChatMessage::IssueMessage do
 
       it 'returns a message regarding closing of issues' do
         expect(subject.pretext). to eq(
-          '[<http://somewhere.com|project_name>] Issue <http://url.com|#100 Issue title> closed by test.user')
+          '[<http://somewhere.com|project_name>] Issue <http://url.com|#100 Issue title> closed by Test User (test.user)')
         expect(subject.attachments).to be_empty
       end
     end
@@ -76,10 +76,10 @@ describe ChatMessage::IssueMessage do
     context 'open' do
       it 'returns a message regarding opening of issues' do
         expect(subject.pretext).to eq(
-          '[[project_name](http://somewhere.com)] Issue opened by test.user')
+          '[[project_name](http://somewhere.com)] Issue opened by Test User (test.user)')
         expect(subject.attachments).to eq('issue description')
         expect(subject.activity).to eq({
-          title: 'Issue opened by test.user',
+          title: 'Issue opened by Test User (test.user)',
           subtitle: 'in [project_name](http://somewhere.com)',
           text: '[#100 Issue title](http://url.com)',
           image: 'http://someavatar.com'
@@ -95,10 +95,10 @@ describe ChatMessage::IssueMessage do
 
       it 'returns a message regarding closing of issues' do
         expect(subject.pretext). to eq(
-          '[[project_name](http://somewhere.com)] Issue [#100 Issue title](http://url.com) closed by test.user')
+          '[[project_name](http://somewhere.com)] Issue [#100 Issue title](http://url.com) closed by Test User (test.user)')
         expect(subject.attachments).to be_empty
         expect(subject.activity).to eq({
-          title: 'Issue closed by test.user',
+          title: 'Issue closed by Test User (test.user)',
           subtitle: 'in [project_name](http://somewhere.com)',
           text: '[#100 Issue title](http://url.com)',
           image: 'http://someavatar.com'

spec/models/project_services/chat_message/merge_message_spec.rb

@@ -33,7 +33,7 @@ describe ChatMessage::MergeMessage do
     context 'open' do
       it 'returns a message regarding opening of merge requests' do
         expect(subject.pretext).to eq(
-          'test.user opened <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>: *Merge Request title*')
+          'Test User (test.user) opened <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>: *Merge Request title*')
         expect(subject.attachments).to be_empty
       end
     end
@@ -44,7 +44,7 @@ describe ChatMessage::MergeMessage do
       end
       it 'returns a message regarding closing of merge requests' do
         expect(subject.pretext).to eq(
-          'test.user closed <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>: *Merge Request title*')
+          'Test User (test.user) closed <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>: *Merge Request title*')
         expect(subject.attachments).to be_empty
       end
     end
@@ -58,10 +58,10 @@ describe ChatMessage::MergeMessage do
     context 'open' do
       it 'returns a message regarding opening of merge requests' do
         expect(subject.pretext).to eq(
-          'test.user opened [!100 *Merge Request title*](http://somewhere.com/merge_requests/100) in [project_name](http://somewhere.com): *Merge Request title*')
+          'Test User (test.user) opened [!100 *Merge Request title*](http://somewhere.com/merge_requests/100) in [project_name](http://somewhere.com): *Merge Request title*')
         expect(subject.attachments).to be_empty
         expect(subject.activity).to eq({
-          title: 'Merge Request opened by test.user',
+          title: 'Merge Request opened by Test User (test.user)',
           subtitle: 'in [project_name](http://somewhere.com)',
           text: '[!100 *Merge Request title*](http://somewhere.com/merge_requests/100)',
           image: 'http://someavatar.com'
@@ -76,10 +76,10 @@ describe ChatMessage::MergeMessage do
 
       it 'returns a message regarding closing of merge requests' do
         expect(subject.pretext).to eq(
-          'test.user closed [!100 *Merge Request title*](http://somewhere.com/merge_requests/100) in [project_name](http://somewhere.com): *Merge Request title*')
+          'Test User (test.user) closed [!100 *Merge Request title*](http://somewhere.com/merge_requests/100) in [project_name](http://somewhere.com): *Merge Request title*')
         expect(subject.attachments).to be_empty
         expect(subject.activity).to eq({
-          title: 'Merge Request closed by test.user',
+          title: 'Merge Request closed by Test User (test.user)',
           subtitle: 'in [project_name](http://somewhere.com)',
           text: '[!100 *Merge Request title*](http://somewhere.com/merge_requests/100)',
           image: 'http://someavatar.com'

spec/models/project_services/chat_message/note_message_spec.rb

@@ -38,7 +38,7 @@ describe ChatMessage::NoteMessage do
 
     context 'without markdown' do
       it 'returns a message regarding notes on commits' do
-        expect(subject.pretext).to eq("test.user <http://url.com|commented on " \
+        expect(subject.pretext).to eq("Test User (test.user) <http://url.com|commented on " \
           "commit 5f163b2b> in <http://somewhere.com|project_name>: " \
           "*Added a commit message*")
         expect(subject.attachments).to eq([{
@@ -55,11 +55,11 @@ describe ChatMessage::NoteMessage do
 
       it 'returns a message regarding notes on commits' do
         expect(subject.pretext).to eq(
-          'test.user [commented on commit 5f163b2b](http://url.com) in [project_name](http://somewhere.com): *Added a commit message*'
+          'Test User (test.user) [commented on commit 5f163b2b](http://url.com) in [project_name](http://somewhere.com): *Added a commit message*'
         )
         expect(subject.attachments).to eq('comment on a commit')
         expect(subject.activity).to eq({
-          title: 'test.user [commented on commit 5f163b2b](http://url.com)',
+          title: 'Test User (test.user) [commented on commit 5f163b2b](http://url.com)',
           subtitle: 'in [project_name](http://somewhere.com)',
           text: 'Added a commit message',
           image: 'http://fakeavatar'
@@ -81,7 +81,7 @@ describe ChatMessage::NoteMessage do
 
     context 'without markdown' do
       it 'returns a message regarding notes on a merge request' do
-        expect(subject.pretext).to eq("test.user <http://url.com|commented on " \
+        expect(subject.pretext).to eq("Test User (test.user) <http://url.com|commented on " \
           "merge request !30> in <http://somewhere.com|project_name>: " \
           "*merge request title*")
         expect(subject.attachments).to eq([{
@@ -98,10 +98,10 @@ describe ChatMessage::NoteMessage do
 
       it 'returns a message regarding notes on a merge request' do
         expect(subject.pretext).to eq(
-          'test.user [commented on merge request !30](http://url.com) in [project_name](http://somewhere.com): *merge request title*')
+          'Test User (test.user) [commented on merge request !30](http://url.com) in [project_name](http://somewhere.com): *merge request title*')
         expect(subject.attachments).to eq('comment on a merge request')
         expect(subject.activity).to eq({
-          title: 'test.user [commented on merge request !30](http://url.com)',
+          title: 'Test User (test.user) [commented on merge request !30](http://url.com)',
           subtitle: 'in [project_name](http://somewhere.com)',
           text: 'merge request title',
           image: 'http://fakeavatar'
@@ -124,7 +124,7 @@ describe ChatMessage::NoteMessage do
     context 'without markdown' do
       it 'returns a message regarding notes on an issue' do
         expect(subject.pretext).to eq(
-          "test.user <http://url.com|commented on " \
+          "Test User (test.user) <http://url.com|commented on " \
             "issue #20> in <http://somewhere.com|project_name>: " \
             "*issue title*")
         expect(subject.attachments).to eq([{
@@ -141,10 +141,10 @@ describe ChatMessage::NoteMessage do
 
       it 'returns a message regarding notes on an issue' do
         expect(subject.pretext).to eq(
-          'test.user [commented on issue #20](http://url.com) in [project_name](http://somewhere.com): *issue title*')
+          'Test User (test.user) [commented on issue #20](http://url.com) in [project_name](http://somewhere.com): *issue title*')
         expect(subject.attachments).to eq('comment on an issue')
         expect(subject.activity).to eq({
-          title: 'test.user [commented on issue #20](http://url.com)',
+          title: 'Test User (test.user) [commented on issue #20](http://url.com)',
           subtitle: 'in [project_name](http://somewhere.com)',
           text: 'issue title',
           image: 'http://fakeavatar'
@@ -165,7 +165,7 @@ describe ChatMessage::NoteMessage do
 
     context 'without markdown' do
       it 'returns a message regarding notes on a project snippet' do
-        expect(subject.pretext).to eq("test.user <http://url.com|commented on " \
+        expect(subject.pretext).to eq("Test User (test.user) <http://url.com|commented on " \
           "snippet $5> in <http://somewhere.com|project_name>: " \
           "*snippet title*")
         expect(subject.attachments).to eq([{
@@ -182,7 +182,7 @@ describe ChatMessage::NoteMessage do
 
       it 'returns a message regarding notes on a project snippet' do
         expect(subject.pretext).to eq(
-          'test.user [commented on snippet $5](http://url.com) in [project_name](http://somewhere.com): *snippet title*')
+          'Test User (test.user) [commented on snippet $5](http://url.com) in [project_name](http://somewhere.com): *snippet title*')
         expect(subject.attachments).to eq('comment on a snippet')
       end
     end

spec/models/project_services/chat_message/pipeline_message_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 describe ChatMessage::PipelineMessage do
   subject { described_class.new(args) }
 
-  let(:user) { { name: 'hacker' } }
+  let(:user) { { name: "The Hacker", username: 'hacker' } }
   let(:duration) { 7210 }
   let(:args) do
     {
@@ -22,12 +22,13 @@ describe ChatMessage::PipelineMessage do
       user: user
     }
   end
+  let(:combined_name) { "The Hacker (hacker)" }
 
   context 'without markdown' do
     context 'pipeline succeeded' do
       let(:status) { 'success' }
       let(:color) { 'good' }
-      let(:message) { build_message('passed') }
+      let(:message) { build_message('passed', combined_name) }
 
       it 'returns a message with information about succeeded build' do
         expect(subject.pretext).to be_empty
@@ -39,7 +40,7 @@ describe ChatMessage::PipelineMessage do
     context 'pipeline failed' do
       let(:status) { 'failed' }
       let(:color) { 'danger' }
-      let(:message) { build_message }
+      let(:message) { build_message(status, combined_name) }
 
       it 'returns a message with information about failed build' do
         expect(subject.pretext).to be_empty
@@ -75,13 +76,13 @@ describe ChatMessage::PipelineMessage do
     context 'pipeline succeeded' do
       let(:status) { 'success' }
       let(:color) { 'good' }
-      let(:message) { build_markdown_message('passed') }
+      let(:message) { build_markdown_message('passed', combined_name) }
 
       it 'returns a message with information about succeeded build' do
         expect(subject.pretext).to be_empty
         expect(subject.attachments).to eq(message)
         expect(subject.activity).to eq({
-          title: 'Pipeline [#123](http://example.gitlab.com/pipelines/123) of branch [develop](http://example.gitlab.com/commits/develop) by hacker passed',
+          title: 'Pipeline [#123](http://example.gitlab.com/pipelines/123) of branch [develop](http://example.gitlab.com/commits/develop) by The Hacker (hacker) passed',
           subtitle: 'in [project_name](http://example.gitlab.com)',
           text: 'in 02:00:10',
           image: ''
@@ -92,13 +93,13 @@ describe ChatMessage::PipelineMessage do
     context 'pipeline failed' do
       let(:status) { 'failed' }
       let(:color) { 'danger' }
-      let(:message) { build_markdown_message }
+      let(:message) { build_markdown_message(status, combined_name) }
 
       it 'returns a message with information about failed build' do
         expect(subject.pretext).to be_empty
         expect(subject.attachments).to eq(message)
         expect(subject.activity).to eq({
-          title: 'Pipeline [#123](http://example.gitlab.com/pipelines/123) of branch [develop](http://example.gitlab.com/commits/develop) by hacker failed',
+          title: 'Pipeline [#123](http://example.gitlab.com/pipelines/123) of branch [develop](http://example.gitlab.com/commits/develop) by The Hacker (hacker) failed',
           subtitle: 'in [project_name](http://example.gitlab.com)',
           text: 'in 02:00:10',
           image: ''

spec/models/project_services/chat_message/wiki_page_message_spec.rb

@@ -29,7 +29,7 @@ describe ChatMessage::WikiPageMessage do
 
         it 'returns a message that a new wiki page was created' do
           expect(subject.pretext).to eq(
-            'test.user created <http://url.com|wiki page> in <http://somewhere.com|project_name>: '\
+            'Test User (test.user) created <http://url.com|wiki page> in <http://somewhere.com|project_name>: '\
               '*Wiki page title*')
         end
       end
@@ -41,7 +41,7 @@ describe ChatMessage::WikiPageMessage do
 
         it 'returns a message that a wiki page was updated' do
           expect(subject.pretext).to eq(
-            'test.user edited <http://url.com|wiki page> in <http://somewhere.com|project_name>: '\
+            'Test User (test.user) edited <http://url.com|wiki page> in <http://somewhere.com|project_name>: '\
               '*Wiki page title*')
         end
       end
@@ -95,7 +95,7 @@ describe ChatMessage::WikiPageMessage do
 
         it 'returns a message that a new wiki page was created' do
           expect(subject.pretext).to eq(
-            'test.user created [wiki page](http://url.com) in [project_name](http://somewhere.com): *Wiki page title*')
+            'Test User (test.user) created [wiki page](http://url.com) in [project_name](http://somewhere.com): *Wiki page title*')
         end
       end
 
@@ -106,7 +106,7 @@ describe ChatMessage::WikiPageMessage do
 
         it 'returns a message that a wiki page was updated' do
           expect(subject.pretext).to eq(
-            'test.user edited [wiki page](http://url.com) in [project_name](http://somewhere.com): *Wiki page title*')
+            'Test User (test.user) edited [wiki page](http://url.com) in [project_name](http://somewhere.com): *Wiki page title*')
         end
       end
     end
@@ -141,7 +141,7 @@ describe ChatMessage::WikiPageMessage do
 
         it 'returns the attachment for a new wiki page' do
           expect(subject.activity).to eq({
-            title: 'test.user created [wiki page](http://url.com)',
+            title: 'Test User (test.user) created [wiki page](http://url.com)',
             subtitle: 'in [project_name](http://somewhere.com)',
             text: 'Wiki page title',
             image: 'http://someavatar.com'
@@ -156,7 +156,7 @@ describe ChatMessage::WikiPageMessage do
 
         it 'returns the attachment for an updated wiki page' do
           expect(subject.activity).to eq({
-            title: 'test.user edited [wiki page](http://url.com)',
+            title: 'Test User (test.user) edited [wiki page](http://url.com)',
             subtitle: 'in [project_name](http://somewhere.com)',
             text: 'Wiki page title',
             image: 'http://someavatar.com'