Add Username to Email From Header in Notifications

When replying to a notification from GitLab, it can be non-obvious as
to how to ping the person I am responding to without looking for their
username in Gitlab.

By adding the username to the From header in the form of From: User
Name (@user), it is much more clear as to how to ping a person in the
response.

app/mailers/notify.rb

@@ -70,7 +70,7 @@ class Notify < ApplicationMailer
     return unless sender = User.find(sender_id)
 
     address = default_sender_address
-    address.display_name = sender_name.presence || sender.name
+    address.display_name = sender_name.presence || "#{sender.name} (#{sender.to_reference})"
 
     if send_from_user_email && can_send_from_user_email?(sender)
       address.address = sender.email

changelogs/unreleased/afontaine-add-username-to-email-from.yml

+---
+title: Add Username to Email From Header in Notifications
+merge_request: 56588
+author:
+type: changed

ee/spec/mailers/notify_spec.rb

@@ -106,9 +106,7 @@ RSpec.describe Notify do
         it_behaves_like 'an unsubscribeable thread'
 
         it 'is sent as the last approver' do
-          sender = subject.header[:from].addrs[0]
-          expect(sender.display_name).to eq(last_approver.name)
-          expect(sender.address).to eq(gitlab_sender)
+          expect_sender(last_approver)
         end
 
         it 'has the correct subject' do
@@ -170,9 +168,7 @@ RSpec.describe Notify do
         it_behaves_like 'an unsubscribeable thread'
 
         it 'is sent as the last unapprover' do
-          sender = subject.header[:from].addrs[0]
-          expect(sender.display_name).to eq(last_unapprover.name)
-          expect(sender.address).to eq(gitlab_sender)
+          expect_sender(last_unapprover)
         end
 
         it 'has the correct subject' do
@@ -397,4 +393,10 @@ RSpec.describe Notify do
       is_expected.to have_body_text recipient.confirmation_token
     end
   end
+
+  def expect_sender(user)
+    sender = subject.header[:from].addrs[0]
+    expect(sender.display_name).to eq("#{user.name} (@#{user.username})")
+    expect(sender.address).to eq(gitlab_sender)
+  end
 end

spec/mailers/emails/merge_requests_spec.rb

@@ -55,9 +55,7 @@ RSpec.describe Emails::MergeRequests do
     it_behaves_like 'appearance header and footer not enabled'
 
     it 'is sent as the merge request author' do
-      sender = subject.header[:from].addrs[0]
-      expect(sender.display_name).to eq(merge_request.author.name)
-      expect(sender.address).to eq(gitlab_sender)
+      expect_sender(merge_request.author)
     end
 
     it 'has the correct subject and body' do
@@ -85,9 +83,7 @@ RSpec.describe Emails::MergeRequests do
     it_behaves_like 'appearance header and footer not enabled'
 
     it 'is sent as the author' do
-      sender = subject.header[:from].addrs[0]
-      expect(sender.display_name).to eq(current_user.name)
-      expect(sender.address).to eq(gitlab_sender)
+      expect_sender(current_user)
     end
 
     it 'has the correct subject and body' do
@@ -120,9 +116,7 @@ RSpec.describe Emails::MergeRequests do
     it_behaves_like 'appearance header and footer not enabled'
 
     it 'is sent as the merge author' do
-      sender = subject.header[:from].addrs[0]
-      expect(sender.display_name).to eq(merge_author.name)
-      expect(sender.address).to eq(gitlab_sender)
+      expect_sender(merge_author)
     end
 
     it 'has the correct subject and body' do
@@ -153,9 +147,7 @@ RSpec.describe Emails::MergeRequests do
     it_behaves_like 'appearance header and footer not enabled'
 
     it 'is sent as the author' do
-      sender = subject.header[:from].addrs[0]
-      expect(sender.display_name).to eq(current_user.name)
-      expect(sender.address).to eq(gitlab_sender)
+      expect_sender(current_user)
     end
 
     it 'has the correct subject and body' do
@@ -229,4 +221,10 @@ RSpec.describe Emails::MergeRequests do
       it { expect(subject).to have_content('attachment has been truncated to avoid exceeding the maximum allowed attachment size of 15 MB.') }
     end
   end
+
+  def expect_sender(user)
+    sender = subject.header[:from].addrs[0]
+    expect(sender.display_name).to eq("#{user.name} (@#{user.username})")
+    expect(sender.address).to eq(gitlab_sender)
+  end
 end

spec/mailers/notify_spec.rb

@@ -69,11 +69,8 @@ RSpec.describe Notify do
       it_behaves_like 'an email sent to a user'
 
       it 'is sent to the assignee as the author' do
-        sender = subject.header[:from].addrs.first
-
         aggregate_failures do
-          expect(sender.display_name).to eq(current_user.name)
-          expect(sender.address).to eq(gitlab_sender)
+          expect_sender(current_user)
           expect(subject).to deliver_to(recipient.notification_email)
         end
       end
@@ -146,9 +143,7 @@ RSpec.describe Notify do
         it_behaves_like 'appearance header and footer not enabled'
 
         it 'is sent as the author' do
-          sender = subject.header[:from].addrs[0]
-          expect(sender.display_name).to eq(current_user.name)
-          expect(sender.address).to eq(gitlab_sender)
+          expect_sender(current_user)
         end
 
         it 'has the correct subject and body' do
@@ -187,9 +182,7 @@ RSpec.describe Notify do
         it_behaves_like 'appearance header and footer not enabled'
 
         it 'is sent as the author' do
-          sender = subject.header[:from].addrs[0]
-          expect(sender.display_name).to eq(current_user.name)
-          expect(sender.address).to eq(gitlab_sender)
+          expect_sender(current_user)
         end
 
         it 'has the correct subject and body' do
@@ -251,9 +244,7 @@ RSpec.describe Notify do
         it_behaves_like 'appearance header and footer not enabled'
 
         it 'is sent as the author' do
-          sender = subject.header[:from].addrs[0]
-          expect(sender.display_name).to eq(current_user.name)
-          expect(sender.address).to eq(gitlab_sender)
+          expect_sender(current_user)
         end
 
         it 'has the correct subject and body' do
@@ -389,9 +380,7 @@ RSpec.describe Notify do
         it_behaves_like 'appearance header and footer not enabled'
 
         it 'is sent as the author' do
-          sender = subject.header[:from].addrs[0]
-          expect(sender.display_name).to eq(current_user.name)
-          expect(sender.address).to eq(gitlab_sender)
+          expect_sender(current_user)
         end
 
         it 'has the correct subject and body' do
@@ -456,9 +445,7 @@ RSpec.describe Notify do
         it_behaves_like 'appearance header and footer not enabled'
 
         it 'is sent as the author' do
-          sender = subject.header[:from].addrs[0]
-          expect(sender.display_name).to eq(current_user.name)
-          expect(sender.address).to eq(gitlab_sender)
+          expect_sender(current_user)
         end
 
         it 'has the correct subject and body' do
@@ -486,10 +473,7 @@ RSpec.describe Notify do
         it_behaves_like 'appearance header and footer not enabled'
 
         it 'is sent as the push user' do
-          sender = subject.header[:from].addrs[0]
-
-          expect(sender.display_name).to eq(push_user.name)
-          expect(sender.address).to eq(gitlab_sender)
+          expect_sender(push_user)
         end
 
         it 'has the correct subject and body' do
@@ -1002,11 +986,8 @@ RSpec.describe Notify do
         it_behaves_like 'it should have Gmail Actions links'
 
         it 'is sent to the given recipient as the author' do
-          sender = subject.header[:from].addrs[0]
-
           aggregate_failures do
-            expect(sender.display_name).to eq(note_author.name)
-            expect(sender.address).to eq(gitlab_sender)
+            expect_sender(note_author)
             expect(subject).to deliver_to(recipient.notification_email)
           end
         end
@@ -1162,11 +1143,8 @@ RSpec.describe Notify do
         it_behaves_like 'it should have Gmail Actions links'
 
         it 'is sent to the given recipient as the author' do
-          sender = subject.header[:from].addrs[0]
-
           aggregate_failures do
-            expect(sender.display_name).to eq(note_author.name)
-            expect(sender.address).to eq(gitlab_sender)
+            expect_sender(note_author)
             expect(subject).to deliver_to(recipient.notification_email)
           end
         end
@@ -1221,12 +1199,6 @@ RSpec.describe Notify do
         issue.issue_email_participants.create!(email: 'service.desk@example.com')
       end
 
-      def expect_sender(username)
-        sender = subject.header[:from].addrs[0]
-        expect(sender.display_name).to eq(username)
-        expect(sender.address).to eq(gitlab_sender)
-      end
-
       describe 'thank you email' do
         subject { described_class.service_desk_thank_you_email(issue.id) }
 
@@ -1244,14 +1216,16 @@ RSpec.describe Notify do
         end
 
         it 'uses service bot name by default' do
-          expect_sender(User.support_bot.name)
+          expect_sender(User.support_bot)
         end
 
         context 'when custom outgoing name is set' do
           let_it_be(:settings) { create(:service_desk_setting, project: project, outgoing_name: 'some custom name') }
 
           it 'uses custom name in "from" header' do
-            expect_sender('some custom name')
+            sender = subject.header[:from].addrs[0]
+            expect(sender.display_name).to eq('some custom name')
+            expect(sender.address).to eq(gitlab_sender)
           end
         end
 
@@ -1259,7 +1233,7 @@ RSpec.describe Notify do
           let_it_be(:settings) { create(:service_desk_setting, project: project, outgoing_name: '') }
 
           it 'uses service bot name' do
-            expect_sender(User.support_bot.name)
+            expect_sender(User.support_bot)
           end
         end
       end
@@ -1276,7 +1250,7 @@ RSpec.describe Notify do
         end
 
         it 'uses author\'s name in "from" header' do
-          expect_sender(first_note.author.name)
+          expect_sender(first_note.author)
         end
 
         it 'has the correct subject and body' do
@@ -1672,9 +1646,7 @@ RSpec.describe Notify do
     it_behaves_like 'appearance header and footer not enabled'
 
     it 'is sent as the author' do
-      sender = subject.header[:from].addrs[0]
-      expect(sender.display_name).to eq(user.name)
-      expect(sender.address).to eq(gitlab_sender)
+      expect_sender(user)
     end
 
     it 'has the correct subject and body' do
@@ -1699,9 +1671,7 @@ RSpec.describe Notify do
     it_behaves_like 'appearance header and footer not enabled'
 
     it 'is sent as the author' do
-      sender = subject.header[:from].addrs[0]
-      expect(sender.display_name).to eq(user.name)
-      expect(sender.address).to eq(gitlab_sender)
+      expect_sender(user)
     end
 
     it 'has the correct subject and body' do
@@ -1725,9 +1695,7 @@ RSpec.describe Notify do
     it_behaves_like 'appearance header and footer not enabled'
 
     it 'is sent as the author' do
-      sender = subject.header[:from].addrs[0]
-      expect(sender.display_name).to eq(user.name)
-      expect(sender.address).to eq(gitlab_sender)
+      expect_sender(user)
     end
 
     it 'has the correct subject' do
@@ -1748,9 +1716,7 @@ RSpec.describe Notify do
     it_behaves_like 'appearance header and footer not enabled'
 
     it 'is sent as the author' do
-      sender = subject.header[:from].addrs[0]
-      expect(sender.display_name).to eq(user.name)
-      expect(sender.address).to eq(gitlab_sender)
+      expect_sender(user)
     end
 
     it 'has the correct subject' do
@@ -1777,9 +1743,7 @@ RSpec.describe Notify do
     it_behaves_like 'appearance header and footer not enabled'
 
     it 'is sent as the author' do
-      sender = subject.header[:from].addrs[0]
-      expect(sender.display_name).to eq(user.name)
-      expect(sender.address).to eq(gitlab_sender)
+      expect_sender(user)
     end
 
     it 'has the correct subject and body' do
@@ -1870,9 +1834,7 @@ RSpec.describe Notify do
     it_behaves_like 'appearance header and footer not enabled'
 
     it 'is sent as the author' do
-      sender = subject.header[:from].addrs[0]
-      expect(sender.display_name).to eq(user.name)
-      expect(sender.address).to eq(gitlab_sender)
+      expect_sender(user)
     end
 
     it 'has the correct subject and body' do
@@ -1962,12 +1924,8 @@ RSpec.describe Notify do
     it_behaves_like 'an unsubscribeable thread'
 
     it 'is sent to the given recipient as the author' do
-      sender = subject.header[:from].addrs[0]
-
       aggregate_failures do
-        expect(sender.display_name).to eq(review.author_name)
-        expect(sender.address).to eq(gitlab_sender)
-        expect(subject).to deliver_to(recipient.notification_email)
+        expect_sender(review.author)
       end
     end
 
@@ -2002,4 +1960,10 @@ RSpec.describe Notify do
       end
     end
   end
+
+  def expect_sender(user)
+    sender = subject.header[:from].addrs[0]
+    expect(sender.display_name).to eq("#{user.name} (@#{user.username})")
+    expect(sender.address).to eq(gitlab_sender)
+  end
 end

spec/services/notification_service_spec.rb

@@ -1662,7 +1662,7 @@ RSpec.describe NotificationService, :mailer do
         notification.issue_due(issue)
         email = find_email_for(@subscriber)
 
-        expect(email.header[:from].display_names).to eq([issue.author.name])
+        expect(email.header[:from].display_names).to eq(["#{issue.author.name} (@#{issue.author.username})"])
       end
 
       it_behaves_like 'participating notifications' do

spec/support/shared_examples/mailers/notify_shared_examples.rb

@@ -225,7 +225,7 @@ RSpec.shared_examples 'a note email' do
     sender = subject.header[:from].addrs[0]
 
     aggregate_failures do
-      expect(sender.display_name).to eq(note_author.name)
+      expect(sender.display_name).to eq("#{note_author.name} (@#{note_author.username})")
       expect(sender.address).to eq(gitlab_sender)
       expect(subject).to deliver_to(recipient.notification_email)
     end