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.
This commit is contained in:
Martin Brennan 2021-10-19 15:26:22 +10:00 committed by GitHub
parent 2364626ded
commit d3678f6930
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 12 additions and 5 deletions

View File

@ -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}")

View File

@ -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

View File

@ -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

View File

@ -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(