From c4ea1586563a5e055082c56ff69d7ebbdaa175ee Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Fri, 6 Jan 2023 10:03:02 +1000
Subject: [PATCH] FIX: Improve tags in email subjects and add filter headers
 (#19760)

This commit does a couple of things:

1. Changes the limit of tags to include a subject for a
   notification email to the `max_tags_per_topic` setting
   instead of the arbitrary 3 limit
2. Adds both an X-Discourse-Tags and X-Discourse-Category
   custom header to outbound emails containing the tags
   and category from the subject, so people on mail clients
   that allow advanced filtering (i.e. not Gmail) can filter
   mail by tags and category, which is useful for mailing
   list mode users

c.f. https://meta.discourse.org/t/headers-for-email-notifications-so-that-gmail-users-can-filter-on-tags/249982/17
---
 app/mailers/user_notifications.rb       |  2 +-
 lib/email/message_builder.rb            |  5 +++++
 spec/lib/email/message_builder_spec.rb  | 10 ++++++++++
 spec/mailers/user_notifications_spec.rb | 13 +++++++++++++
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb
index 78bdbd722b..3aca5a9334 100644
--- a/app/mailers/user_notifications.rb
+++ b/app/mailers/user_notifications.rb
@@ -538,7 +538,7 @@ class UserNotifications < ActionMailer::Base
           .visible_tags(Guardian.new(user))
           .joins(:topic_tags)
           .where("topic_tags.topic_id = ?", post.topic_id)
-          .limit(3)
+          .limit(SiteSetting.max_tags_per_topic)
           .pluck(:name)
 
       show_tags_in_subject = tags.any? ? tags.join(" ") : nil
diff --git a/lib/email/message_builder.rb b/lib/email/message_builder.rb
index 9308705cd6..42aefe600a 100644
--- a/lib/email/message_builder.rb
+++ b/lib/email/message_builder.rb
@@ -161,6 +161,11 @@ module Email
       result['X-Discourse-Post-Id']  = @opts[:post_id].to_s  if @opts[:post_id]
       result['X-Discourse-Topic-Id'] = @opts[:topic_id].to_s if @opts[:topic_id]
 
+      # at this point these have been filtered by the recipient's guardian for visibility,
+      # see UserNotifications#send_notification_email
+      result['X-Discourse-Tags'] = @template_args[:show_tags_in_subject] if @opts[:show_tags_in_subject]
+      result['X-Discourse-Category'] = @template_args[:show_category_in_subject] if @opts[:show_category_in_subject]
+
       # please, don't send us automatic responses...
       result['X-Auto-Response-Suppress'] = 'All'
 
diff --git a/spec/lib/email/message_builder_spec.rb b/spec/lib/email/message_builder_spec.rb
index 00487992c0..4a9c1993be 100644
--- a/spec/lib/email/message_builder_spec.rb
+++ b/spec/lib/email/message_builder_spec.rb
@@ -155,6 +155,8 @@ RSpec.describe Email::MessageBuilder do
         body: 'hello world',
         topic_id: 1234,
         post_id: 4567,
+        show_tags_in_subject: "foo bar baz",
+        show_category_in_subject: "random"
         }.merge(additional_opts)
       )
     end
@@ -171,6 +173,14 @@ RSpec.describe Email::MessageBuilder do
       expect(message_with_header_args.header_args['Reply-To']).to eq("\"Discourse\" <#{SiteSetting.notification_email}>")
     end
 
+    it "passes through the topic tags" do
+      expect(message_with_header_args.header_args['X-Discourse-Tags']).to eq('foo bar baz')
+    end
+
+    it "passes through the topic category" do
+      expect(message_with_header_args.header_args['X-Discourse-Category']).to eq('random')
+    end
+
     context "when allow_reply_by_email is enabled " do
       let(:additional_opts) { { allow_reply_by_email: true } }
 
diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb
index 63e4350633..6111bdde8f 100644
--- a/spec/mailers/user_notifications_spec.rb
+++ b/spec/mailers/user_notifications_spec.rb
@@ -400,6 +400,19 @@ RSpec.describe UserNotifications do
       expect(mail_html.scan(/>bobmarley/).count).to eq(1)
     end
 
+    it "the number of tags shown in subject should match max_tags_per_topic" do
+      SiteSetting.email_subject = "[%{site_name}] %{optional_pm}%{optional_cat}%{optional_tags}%{topic_title}"
+      SiteSetting.max_tags_per_topic = 1
+      mail = UserNotifications.user_replied(
+        user,
+        post: response,
+        notification_type: notification.notification_type,
+        notification_data_hash: notification.data_hash
+      )
+      expect(mail.subject).to match(/Taggo/)
+      expect(mail.subject).not_to match(/Taggie/)
+    end
+
     it "doesn't include details when private_email is enabled" do
       SiteSetting.private_email = true
       mail = UserNotifications.user_replied(