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
This commit is contained in:
Martin Brennan 2023-01-06 10:03:02 +10:00 committed by GitHub
parent 12f843068d
commit c4ea158656
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 29 additions and 1 deletions

View File

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

View File

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

View File

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

View File

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