UX: Include group name in email when group is invited to a PM.

https://meta.discourse.org/t/xyz-invited-you-to-a-message-but-really-invited-a-group-im-in/65996
This commit is contained in:
Guo Xiang Tan 2017-07-26 15:51:44 +09:00
parent d3a975e99a
commit b59dfb86f4
5 changed files with 99 additions and 13 deletions

View File

@ -27,6 +27,7 @@ class Admin::EmailTemplatesController < Admin::AdminController
"user_notifications.set_password", "user_notifications.signup", "user_notifications.set_password", "user_notifications.signup",
"user_notifications.signup_after_approval", "user_notifications.signup_after_approval",
"user_notifications.user_invited_to_private_message_pm", "user_notifications.user_invited_to_private_message_pm",
"user_notifications.user_invited_to_private_message_pm_group",
"user_notifications.user_invited_to_topic", "user_notifications.user_mentioned", "user_notifications.user_invited_to_topic", "user_notifications.user_mentioned",
"user_notifications.user_posted", "user_notifications.user_posted_pm", "user_notifications.user_posted", "user_notifications.user_posted_pm",
"user_notifications.user_quoted", "user_notifications.user_replied", "user_notifications.user_quoted", "user_notifications.user_replied",

View File

@ -311,7 +311,7 @@ class UserNotifications < ActionMailer::Base
allow_reply_by_email = opts[:allow_reply_by_email] unless user.suspended? allow_reply_by_email = opts[:allow_reply_by_email] unless user.suspended?
original_username = notification_data[:original_username] || notification_data[:display_username] original_username = notification_data[:original_username] || notification_data[:display_username]
send_notification_email( email_options = {
title: notification_data[:topic_title], title: notification_data[:topic_title],
post: post, post: post,
username: original_username, username: original_username,
@ -323,7 +323,13 @@ class UserNotifications < ActionMailer::Base
notification_type: notification_type, notification_type: notification_type,
use_invite_template: opts[:use_invite_template], use_invite_template: opts[:use_invite_template],
user: user user: user
) }
if group_id = notification_data[:group_id]
email_options[:group_name] = Group.find_by(id: group_id)&.name
end
send_notification_email(email_options)
end end
def send_notification_email(opts) def send_notification_email(opts)
@ -337,12 +343,18 @@ class UserNotifications < ActionMailer::Base
from_alias = opts[:from_alias] from_alias = opts[:from_alias]
notification_type = opts[:notification_type] notification_type = opts[:notification_type]
user = opts[:user] user = opts[:user]
group_name = opts[:group_name]
locale = user_locale(user) locale = user_locale(user)
template = "user_notifications.user_#{notification_type}" template = "user_notifications.user_#{notification_type}"
if post.topic.private_message? if post.topic.private_message?
template << "_pm" template << "_pm"
template << "_staged" if user.staged?
if group_name
template << "_group"
elsif user.staged
template << "_staged"
end
end end
# category name # category name
@ -376,17 +388,33 @@ class UserNotifications < ActionMailer::Base
end end
end end
translation_override_exists = TranslationOverride.where(locale: SiteSetting.default_locale, translation_key: "#{template}.text_body_template").exists? translation_override_exists = TranslationOverride.where(
locale: SiteSetting.default_locale,
translation_key: "#{template}.text_body_template"
).exists?
if opts[:use_invite_template] if opts[:use_invite_template]
if post.topic.private_message? invite_template = "user_notifications.invited"
invite_template = "user_notifications.invited_to_private_message_body" invite_template << "_group" if group_name
else
invite_template = "user_notifications.invited_to_topic_body" invite_template <<
end if post.topic.private_message?
"_to_private_message_body"
else
"_to_topic_body"
end
topic_excerpt = post.excerpt.tr("\n", " ") if post.is_first_post? && post.excerpt topic_excerpt = post.excerpt.tr("\n", " ") if post.is_first_post? && post.excerpt
topic_excerpt = "" if SiteSetting.private_email? topic_excerpt = "" if SiteSetting.private_email?
message = I18n.t(invite_template, username: username, topic_title: gsub_emoji_to_unicode(title), topic_excerpt: topic_excerpt, site_title: SiteSetting.title, site_description: SiteSetting.site_description)
message = I18n.t(invite_template,
username: username,
group_name: group_name,
topic_title: gsub_emoji_to_unicode(title),
topic_excerpt: topic_excerpt,
site_title: SiteSetting.title,
site_description: SiteSetting.site_description
)
unless translation_override_exists unless translation_override_exists
html = UserNotificationRenderer.new(Rails.configuration.paths["app/views"]).render( html = UserNotificationRenderer.new(Rails.configuration.paths["app/views"]).render(
@ -434,6 +462,7 @@ class UserNotifications < ActionMailer::Base
topic_id: post.topic_id, topic_id: post.topic_id,
context: context, context: context,
username: username, username: username,
group_name: group_name,
add_unsubscribe_link: !user.staged, add_unsubscribe_link: !user.staged,
mailing_list_mode: user.user_option.mailing_list_mode, mailing_list_mode: user.user_option.mailing_list_mode,
unsubscribe_url: post.unsubscribe_url(user), unsubscribe_url: post.unsubscribe_url(user),

View File

@ -723,6 +723,8 @@ SQL
PostAlerter.new.after_save_post(last_post) PostAlerter.new.after_save_post(last_post)
add_small_action(user, "invited_group", group.name) add_small_action(user, "invited_group", group.name)
group_id = group.id
group.users.where( group.users.where(
"group_users.notification_level > ?", NotificationLevels.all[:muted] "group_users.notification_level > ?", NotificationLevels.all[:muted]
).find_each do |u| ).find_each do |u|
@ -733,7 +735,8 @@ SQL
post_number: 1, post_number: 1,
data: { data: {
topic_title: self.title, topic_title: self.title,
display_username: user.username display_username: user.username,
group_id: group_id
}.to_json }.to_json
) )
end end

View File

@ -2424,6 +2424,17 @@ en:
posted_by: "Posted by %{username} on %{post_date}" posted_by: "Posted by %{username} on %{post_date}"
invited_group_to_private_message_body: |
%{username} invited @%{group_name} to a message
> #### %{topic_title}
>
> %{topic_excerpt}
at
> %{site_title} -- %{site_description}
invited_to_private_message_body: | invited_to_private_message_body: |
%{username} invited you to a message %{username} invited you to a message
@ -2446,6 +2457,16 @@ en:
> %{site_title} -- %{site_description} > %{site_title} -- %{site_description}
user_invited_to_private_message_pm_group:
title: "User Invited Group to PM"
subject_template: "[%{email_prefix}] %{username} invited @%{group_name} to a message '%{topic_title}'"
text_body_template: |
%{header_instructions}
%{message}
%{respond_instructions}
user_invited_to_private_message_pm: user_invited_to_private_message_pm:
title: "User Invited to PM" title: "User Invited to PM"
subject_template: "[%{email_prefix}] %{username} invited you to a message '%{topic_title}'" subject_template: "[%{email_prefix}] %{username} invited you to a message '%{topic_title}'"

View File

@ -468,6 +468,7 @@ describe UserNotifications do
shared_examples "notification email building" do shared_examples "notification email building" do
let(:post) { Fabricate(:post, user: user) } let(:post) { Fabricate(:post, user: user) }
let(:mail_type) { "user_#{notification_type}"} let(:mail_type) { "user_#{notification_type}"}
let(:mail_template) { "user_notifications.#{mail_type}" }
let(:username) { "walterwhite"} let(:username) { "walterwhite"}
let(:notification) do let(:notification) do
Fabricate(:notification, Fabricate(:notification,
@ -488,7 +489,7 @@ describe UserNotifications do
end end
it "has a template" do it "has a template" do
expects_build_with(has_entry(:template, "user_notifications.#{mail_type}")) expects_build_with(has_entry(:template, mail_template))
end end
it "overrides the html part" do it "overrides the html part" do
@ -549,7 +550,11 @@ describe UserNotifications do
end end
before do before do
TranslationOverride.upsert!("en", "user_notifications.user_#{notification_type}.text_body_template", custom_body) TranslationOverride.upsert!(
"en",
"#{mail_template}.text_body_template",
custom_body
)
end end
it "shouldn't use the default html_override" do it "shouldn't use the default html_override" do
@ -598,6 +603,33 @@ describe UserNotifications do
describe "user invited to a private message" do describe "user invited to a private message" do
include_examples "notification email building" do include_examples "notification email building" do
let(:notification_type) { :invited_to_private_message } let(:notification_type) { :invited_to_private_message }
let(:post) { Fabricate(:private_message_post) }
let(:user) { post.user }
let(:mail_template) { "user_notifications.user_#{notification_type}_pm" }
include_examples "respect for private_email"
include_examples "no reply by email"
include_examples "sets user locale"
end
end
describe "group invited to a private message" do
include_examples "notification email building" do
let(:notification_type) { :invited_to_private_message }
let(:post) { Fabricate(:private_message_post) }
let(:user) { post.user }
let(:group) { Fabricate(:group) }
let(:mail_template) { "user_notifications.user_#{notification_type}_pm_group" }
before do
notification.data_hash[:group_id] = group.id
notification.save!
end
it "should include the group name" do
expects_build_with(has_entry(:group_name, group.name))
end
include_examples "respect for private_email" include_examples "respect for private_email"
include_examples "no reply by email" include_examples "no reply by email"
include_examples "sets user locale" include_examples "sets user locale"