From 62a8904729cdd408e0586bd86fa20ee2b97ab92b Mon Sep 17 00:00:00 2001
From: Jeff Wong <awole20@gmail.com>
Date: Thu, 3 May 2018 15:50:06 -0700
Subject: [PATCH] Feature: Include participants at the bottom of PM emails
 (#5797)

* Feature: Include participants at the bottom of PM emails

... as undecorated links.

https://meta.discourse.org/t/email-notification-recipients-unclear-when-pm-is-sent-to-multiple-users/26934/13?u=featheredtoast

Fix: missing translation for PM mentions

* display membership count as `group (count)`
---
 app/mailers/user_notifications.rb             | 15 ++++++++++++
 app/views/email/notification.html.erb         |  5 ++++
 config/locales/server.en.yml                  | 14 +++++++++++
 lib/email/message_builder.rb                  | 13 +++++++++++
 lib/email/styles.rb                           |  1 +
 spec/components/email/message_builder_spec.rb | 11 +++++++++
 spec/mailers/user_notifications_spec.rb       | 23 +++++++++++++++++++
 7 files changed, 82 insertions(+)

diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb
index 4a021ce808e..cda60d28970 100644
--- a/app/mailers/user_notifications.rb
+++ b/app/mailers/user_notifications.rb
@@ -444,6 +444,20 @@ class UserNotifications < ActionMailer::Base
         else
           I18n.t('subject_pm')
         end
+
+      participants = "#{I18n.t("user_notifications.pm_participants")} "
+      participant_list = []
+      post.topic.allowed_groups.each do |group|
+        participant_list.push "[#{group.name} (#{group.users.count})](#{Discourse.base_url}/groups/#{group.name})"
+      end
+      post.topic.allowed_users.each do |user|
+        if SiteSetting.prioritize_username_in_ux?
+          participant_list.push "[#{user.username}](#{Discourse.base_url}/u/#{user.username_lower})"
+        else
+          participant_list.push "[#{user.name.blank? ? user.username : user.name}](#{Discourse.base_url}/u/#{user.username_lower})"
+        end
+      end
+      participants += participant_list.join(", ")
     end
 
     if SiteSetting.private_email?
@@ -548,6 +562,7 @@ class UserNotifications < ActionMailer::Base
       show_category_in_subject: show_category_in_subject,
       private_reply: post.topic.private_message?,
       subject_pm: subject_pm,
+      participants: participants,
       include_respond_instructions: !(user.suspended? || user.staged?),
       template: template,
       site_description: SiteSetting.site_description,
diff --git a/app/views/email/notification.html.erb b/app/views/email/notification.html.erb
index 5809973d56b..cfd7fa69404 100644
--- a/app/views/email/notification.html.erb
+++ b/app/views/email/notification.html.erb
@@ -31,6 +31,11 @@
   <% end %>
 
   <div class='footer'>%{respond_instructions}</div>
+  <% if post.topic.private_message? %>
+    <div class='undecorated-link-footer footer'>
+      %{participants}
+    </div>
+  <% end %>
   <div class='footer'>%{unsubscribe_instructions}</div>
 
 </div>
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index d14b752625d..41b7fdcab21 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -2614,6 +2614,8 @@ en:
 
     posted_by: "Posted by %{username} on %{post_date}"
 
+    pm_participants: "Participants:"
+
     invited_group_to_private_message_body: |
       %{username} invited @%{group_name} to a message
 
@@ -2747,6 +2749,18 @@ en:
 
         %{respond_instructions}
 
+    user_mentioned_pm:
+      title: "User Mentioned PM"
+      subject_template: "[%{email_prefix}] [PM] %{topic_title}"
+      text_body_template: |
+        %{header_instructions}
+
+        %{message}
+
+        %{context}
+
+        %{respond_instructions}
+
     user_group_mentioned:
       title: "User Group Mentioned"
       subject_template: "[%{email_prefix}] %{topic_title}"
diff --git a/lib/email/message_builder.rb b/lib/email/message_builder.rb
index ac95582586d..e5007cae690 100644
--- a/lib/email/message_builder.rb
+++ b/lib/email/message_builder.rb
@@ -96,6 +96,13 @@ module Email
         html_override.gsub!("%{respond_instructions}", "")
       end
 
+      if @template_args[:participants].present?
+        participants = PrettyText.cook(@template_args[:participants], sanitize: false).html_safe
+        html_override.gsub!("%{participants}", participants)
+      else
+        html_override.gsub!("%{participants}", "")
+      end
+
       styled = Email::Styles.new(html_override, @opts)
       styled.format_basic
       if style = @opts[:style]
@@ -112,6 +119,12 @@ module Email
       body = @opts[:body]
       body = I18n.t("#{@opts[:template]}.text_body_template", template_args).dup if @opts[:template]
 
+      if @template_args[:participants].present?
+        body << "\n"
+        body << @template_args[:participants]
+        body << "\n"
+      end
+
       if @template_args[:unsubscribe_instructions].present?
         body << "\n"
         body << @template_args[:unsubscribe_instructions]
diff --git a/lib/email/styles.rb b/lib/email/styles.rb
index 8fc9677b7ae..7a34bc679ef 100644
--- a/lib/email/styles.rb
+++ b/lib/email/styles.rb
@@ -97,6 +97,7 @@ module Email
       style('.lightbox-wrapper .meta', 'display: none')
       correct_first_body_margin
       correct_footer_style
+      style('div.undecorated-link-footer a', "font-weight: normal;")
       reset_tables
       onebox_styles
       plugin_styles
diff --git a/spec/components/email/message_builder_spec.rb b/spec/components/email/message_builder_spec.rb
index 9283f87262e..9ed9200d519 100644
--- a/spec/components/email/message_builder_spec.rb
+++ b/spec/components/email/message_builder_spec.rb
@@ -247,6 +247,17 @@ describe Email::MessageBuilder do
 
   end
 
+  context "PM multiple participants" do
+    let(:pm_multiple) { Email::MessageBuilder.new(to_address,
+                                                               body: 'hello world',
+                                                               private_reply: true,
+                                                               participants: "user1, user2") }
+
+    it "lists participants out" do
+      expect(pm_multiple.body).to match('hello world\nuser1, user2')
+    end
+  end
+
   context "from field" do
 
     it "has the default from" do
diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb
index 4067b6559b1..322b643cf6f 100644
--- a/spec/mailers/user_notifications_spec.rb
+++ b/spec/mailers/user_notifications_spec.rb
@@ -423,6 +423,29 @@ describe UserNotifications do
       expect(mail.subject).to include("[PM] ")
     end
 
+    it "includes a list of participants, groups first with member lists" do
+      group1 = Fabricate(:group)
+      group2 = Fabricate(:group)
+      group1.name = "group1"
+      group2.name = "group2"
+      user1 = Fabricate(:user)
+      user2 = Fabricate(:user)
+      user1.username = "one"
+      user2.username = "two"
+      user1.groups = [ group1, group2 ]
+      user2.groups = [ group1 ]
+      topic.allowed_users = [ user1, user2 ]
+      topic.allowed_groups = [ group1, group2 ]
+      mail = UserNotifications.user_private_message(
+        response.user,
+        post: response,
+        notification_type: notification.notification_type,
+        notification_data_hash: notification.data_hash
+      )
+
+      expect(mail.body).to include("#{I18n.t("user_notifications.pm_participants")} [group1 (2)](http://test.localhost/groups/group1), [group2 (1)](http://test.localhost/groups/group2), [one](http://test.localhost/u/one), [two](http://test.localhost/u/two)")
+    end
+
     context "when SiteSetting.group_name_in_subject is true" do
       before do
         SiteSetting.group_in_subject = true