FIX: Do not show recipient user in email participants list (#14642)

FIX: Do not show recipient user in email participants list (#14642)

This commit removes the recipient’s username from the respond to / participants list that is shown at the bottom of user notification emails. For example if the recipient’s username was jsmith, and there were participants ljones and bmiller, we currently show this:

“reply to this email to respond to jsmith, ljones, bmiller”

or

“Participants: jsmith, ljones, bmiller”

However this is a bit redundant, as you are not replying to yourself here if you are the recipient user. So we omit the recipient user’s username from this list, which is only used in the text of the email and not elsewhere.

diff --git a/app/mailers/group_smtp_mailer.rb b/app/mailers/group_smtp_mailer.rb
index cc9d17f..ce12e4b 100644
--- a/app/mailers/group_smtp_mailer.rb
+++ b/app/mailers/group_smtp_mailer.rb
@@ -9,6 +9,7 @@ class GroupSmtpMailer < ActionMailer::Base
     raise 'SMTP is disabled' if !SiteSetting.enable_smtp
 
     op_incoming_email = post.topic.first_post.incoming_email
+    recipient_user = User.find_by_email(to_address, primary: true)
 
     delivery_options = {
       address: from_group.smtp_server,
@@ -39,7 +40,7 @@ class GroupSmtpMailer < ActionMailer::Base
       only_reply_by_email: true,
       use_from_address_for_reply_to: SiteSetting.enable_smtp && from_group.smtp_enabled?,
       private_reply: post.topic.private_message?,
-      participants: participants(post),
+      participants: participants(post, recipient_user),
       include_respond_instructions: true,
       template: 'user_notifications.user_posted_pm',
       use_topic_title_subject: true,
@@ -72,7 +73,7 @@ class GroupSmtpMailer < ActionMailer::Base
     )
   end
 
-  def participants(post)
+  def participants(post, recipient_user)
     list = []
 
     post.topic.allowed_groups.each do |g|
@@ -80,6 +81,8 @@ class GroupSmtpMailer < ActionMailer::Base
     end
 
     post.topic.allowed_users.each do |u|
+      next if u.id == recipient_user.id
+
       if SiteSetting.prioritize_username_in_ux?
         if u.staged?
           list.push("#{u.email}")
diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb
index 8770330..5cca253 100644
--- a/app/mailers/user_notifications.rb
+++ b/app/mailers/user_notifications.rb
@@ -560,6 +560,8 @@ class UserNotifications < ActionMailer::Base
       end
 
       post.topic.allowed_users.each do |u|
+        next if u.id == user.id
+
         if SiteSetting.prioritize_username_in_ux?
           participant_list.push "[#{u.username}](#{Discourse.base_url}/u/#{u.username_lower})"
         else
diff --git a/spec/jobs/regular/group_smtp_email_spec.rb b/spec/jobs/regular/group_smtp_email_spec.rb
index 59fc8d0..8923ab4 100644
--- a/spec/jobs/regular/group_smtp_email_spec.rb
+++ b/spec/jobs/regular/group_smtp_email_spec.rb
@@ -30,6 +30,7 @@ RSpec.describe Jobs::GroupSmtpEmail do
     SiteSetting.reply_by_email_address = "test+%{reply_key}@test.com"
     SiteSetting.reply_by_email_enabled = true
     TopicAllowedGroup.create(group: group, topic: topic)
+    TopicAllowedUser.create(user: recipient_user, topic: topic)
     TopicAllowedUser.create(user: staged1, topic: topic)
     TopicAllowedUser.create(user: staged2, topic: topic)
     TopicAllowedUser.create(user: normaluser, topic: topic)
@@ -64,7 +65,7 @@ RSpec.describe Jobs::GroupSmtpEmail do
     expect(email_log.as_mail_message.html_part.to_s).not_to include(I18n.t("user_notifications.in_reply_to"))
   end
 
-  it "includes the participants in the correct format, and does not have links for the staged users" do
+  it "includes the participants in the correct format (but not the recipient user), and does not have links for the staged users" do
     subject.execute(args)
     email_log = EmailLog.find_by(post_id: post.id, topic_id: post.topic_id, user_id: recipient_user.id)
     email_text = email_log.as_mail_message.text_part.to_s
@@ -74,6 +75,7 @@ RSpec.describe Jobs::GroupSmtpEmail do
     expect(email_text).to include("cormac@lit.com")
     expect(email_text).not_to include("[cormac@lit.com]")
     expect(email_text).to include("normaluser")
+    expect(email_text).not_to include(recipient_user.username)
   end
 
   it "creates an EmailLog record with the correct details" do
diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb
index 3a1bc52..e186b8d 100644
--- a/spec/mailers/user_notifications_spec.rb
+++ b/spec/mailers/user_notifications_spec.rb
@@ -591,14 +591,14 @@ describe UserNotifications do
       expect(mail.subject).to include("[PM] ")
     end
 
-    it "includes a list of participants, groups first with member lists" do
+    it "includes a list of participants (except for the destination user), groups first with member lists" do
       group1 = Fabricate(:group, name: "group1")
       group2 = Fabricate(:group, name: "group2")
 
       user1 = Fabricate(:user, username: "one", groups: [group1, group2])
       user2 = Fabricate(:user, username: "two", groups: [group1])
 
-      topic.allowed_users = [user1, user2]
+      topic.allowed_users = [user, user1, user2]
       topic.allowed_groups = [group1, group2]
 
       mail = UserNotifications.user_private_message(

GitHub sha: d3678f69301497eb232287ed9fd513c549c6146d

This commit appears in #14642 which was approved by techAPJ. It was merged by martin.