Include GitLab full name with username in Slack messages

We used to include the first and last name of the user, but !6624 modified
this to include only the username. Let's compromise and add both in the form
of `First Last (username)`.

Closes #38865

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

@@ -0,0 +1,5 @@
+---
+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'