From d1cddea685901b76a6a36ffa59299762665e0b2e Mon Sep 17 00:00:00 2001
From: Gerhard Schlager <gerhard.schlager@discourse.org>
Date: Wed, 7 Dec 2022 15:45:02 +0100
Subject: [PATCH] REFACTOR: Make chat summary email notifications easier to
translate (#19354)
---
plugins/chat/config/locales/server.en.yml | 21 ++--
.../user_notifications_extension.rb | 113 ++++++++++--------
.../spec/mailers/user_notifications_spec.rb | 90 +++++++++-----
3 files changed, 130 insertions(+), 94 deletions(-)
diff --git a/plugins/chat/config/locales/server.en.yml b/plugins/chat/config/locales/server.en.yml
index ce7ada209d8..62c2229208b 100644
--- a/plugins/chat/config/locales/server.en.yml
+++ b/plugins/chat/config/locales/server.en.yml
@@ -162,7 +162,7 @@ en:
notify_moderators:
chat_pm_title: 'A chat message in "%{channel_name}" requires staff attention'
chat_pm_body: "%{link}\n\n%{message}"
-
+
reviewables:
reasons:
chat_message_queued_by_staff: "A staff member thinks this chat message needs review."
@@ -174,14 +174,17 @@ en:
other: "You have new chat messages"
from: "%{site_name}"
subject:
- direct_message:
- one: "[%{email_prefix}] New message from %{message_title}"
- other: "[%{email_prefix}] New messages from %{message_title} and %{others}"
- chat_channel:
- one: "[%{email_prefix}] New message in %{message_title}"
- other: "[%{email_prefix}] New messages in %{message_title} and %{others}"
- other_direct_message: "from %{dm_title}"
- others: "%{count} others"
+ direct_message_from_1: "[%{email_prefix}] New message from %{username}"
+ direct_message_from_2: "[%{email_prefix}] New message from %{username1} and %{username2}"
+ direct_message_from_more:
+ one: "[%{email_prefix}] New message from %{username} and %{count} other"
+ other: "[%{email_prefix}] New message from %{username} and %{count} others"
+ chat_channel_1: "[%{email_prefix}] New message in %{channel}"
+ chat_channel_2: "[%{email_prefix}] New message in %{channel1} and %{channel2}"
+ chat_channel_more:
+ one: "[%{email_prefix}] New message in %{channel} and %{count} other"
+ other: "[%{email_prefix}] New message in %{channel} and %{count} others"
+ chat_channel_and_direct_message: "[%{email_prefix}] New message in %{channel} and from %{username}"
unsubscribe: "This chat summary is sent from %{site_link} when you are away. Change your %{email_preferences_link}, or %{unsubscribe_link} to unsubscribe."
unsubscribe_no_link: "This chat summary is sent from %{site_link} when you are away. Change your %{email_preferences_link}."
view_messages:
diff --git a/plugins/chat/lib/extensions/user_notifications_extension.rb b/plugins/chat/lib/extensions/user_notifications_extension.rb
index e86d88f7d90..55fd73470f0 100644
--- a/plugins/chat/lib/extensions/user_notifications_extension.rb
+++ b/plugins/chat/lib/extensions/user_notifications_extension.rb
@@ -60,67 +60,76 @@ module Chat::UserNotificationsExtension
end
def summary_subject(user, grouped_messages)
- channels = grouped_messages.keys
- grouped_channels = channels.partition { |c| !c.direct_message_channel? }
- non_dm_channels = grouped_channels.first
+ all_channels = grouped_messages.keys
+ grouped_channels = all_channels.partition { |c| !c.direct_message_channel? }
+ channels = grouped_channels.first
dm_users = grouped_channels.last.flat_map { |c| grouped_messages[c].map(&:user) }.uniq
- total_count_for_subject = non_dm_channels.size + dm_users.size
-
- # Prioritize messages from regular channels.
- first_message_from = non_dm_channels.pop
- if first_message_from
- first_message_title = first_message_from.title(user)
- subject_key = "chat_channel"
+ # Prioritize messages from regular channels over direct messages
+ if channels.any?
+ channel_notification_text(channels, dm_users)
else
- subject_key = "direct_message"
- first_message_from = dm_users.pop
- first_message_title = first_message_from.username
+ direct_message_notification_text(dm_users)
end
-
- subject_opts = {
- email_prefix: @email_prefix,
- count: total_count_for_subject,
- message_title: first_message_title,
- others:
- other_channels_text(
- user,
- total_count_for_subject,
- first_message_from,
- non_dm_channels,
- dm_users,
- ),
- }
-
- I18n.t(with_subject_prefix(subject_key), **subject_opts)
end
- def with_subject_prefix(key)
- "user_notifications.chat_summary.subject.#{key}"
+ private
+
+ def channel_notification_text(channels, dm_users)
+ total_count = channels.size + dm_users.size
+
+ if total_count > 2
+ I18n.t(
+ "user_notifications.chat_summary.subject.chat_channel_more",
+ email_prefix: @email_prefix,
+ channel: channels.first.title,
+ count: total_count - 1
+ )
+ elsif channels.size == 1 && dm_users.size == 0
+ I18n.t(
+ "user_notifications.chat_summary.subject.chat_channel_1",
+ email_prefix: @email_prefix,
+ channel: channels.first.title
+ )
+ elsif channels.size == 1 && dm_users.size == 1
+ I18n.t(
+ "user_notifications.chat_summary.subject.chat_channel_and_direct_message",
+ email_prefix: @email_prefix,
+ channel: channels.first.title,
+ username: dm_users.first.username
+ )
+ elsif channels.size == 2
+ I18n.t(
+ "user_notifications.chat_summary.subject.chat_channel_2",
+ email_prefix: @email_prefix,
+ channel1: channels.first.title,
+ channel2: channels.second.title
+ )
+ end
end
- def other_channels_text(
- user,
- total_count,
- first_message_from,
- other_non_dm_channels,
- other_dm_users
- )
- return if total_count <= 1
- return I18n.t(with_subject_prefix("others"), count: total_count - 1) if total_count > 2
-
- # The summary contains exactly two messages.
- if other_non_dm_channels.empty?
- second_message_from = other_dm_users.first
- second_message_title = second_message_from.username
+ def direct_message_notification_text(dm_users)
+ case dm_users.size
+ when 1
+ I18n.t(
+ "user_notifications.chat_summary.subject.direct_message_from_1",
+ email_prefix: @email_prefix,
+ username: dm_users.first.username
+ )
+ when 2
+ I18n.t(
+ "user_notifications.chat_summary.subject.direct_message_from_2",
+ email_prefix: @email_prefix,
+ username1: dm_users.first.username,
+ username2: dm_users.second.username
+ )
else
- second_message_from = other_non_dm_channels.first
- second_message_title = second_message_from.title(user)
+ I18n.t(
+ "user_notifications.chat_summary.subject.direct_message_from_more",
+ email_prefix: @email_prefix,
+ username: dm_users.first.username,
+ count: dm_users.size - 1
+ )
end
-
- second_message_is_from_channel = first_message_from.class == second_message_from.class
- return second_message_title if second_message_is_from_channel
-
- I18n.t(with_subject_prefix("other_direct_message"), dm_title: second_message_title)
end
end
diff --git a/plugins/chat/spec/mailers/user_notifications_spec.rb b/plugins/chat/spec/mailers/user_notifications_spec.rb
index 11d35065d52..f825c5400c3 100644
--- a/plugins/chat/spec/mailers/user_notifications_spec.rb
+++ b/plugins/chat/spec/mailers/user_notifications_spec.rb
@@ -27,13 +27,11 @@ describe UserNotifications do
describe "email subject" do
it "includes the sender username in the subject" do
- expected_subject =
- I18n.t(
- "user_notifications.chat_summary.subject.direct_message",
- count: 1,
- email_prefix: SiteSetting.title,
- message_title: sender.username,
- )
+ expected_subject = I18n.t(
+ "user_notifications.chat_summary.subject.direct_message_from_1",
+ email_prefix: SiteSetting.title,
+ username: sender.username
+ )
Fabricate(:chat_message, user: sender, chat_channel: channel)
email = described_class.chat_summary(user, {})
@@ -49,13 +47,11 @@ describe UserNotifications do
chat_channel: channel,
)
DirectMessageUser.create!(direct_message: channel.chatable, user: another_participant)
- expected_subject =
- I18n.t(
- "user_notifications.chat_summary.subject.direct_message",
- count: 1,
- email_prefix: SiteSetting.title,
- message_title: sender.username,
- )
+ expected_subject = I18n.t(
+ "user_notifications.chat_summary.subject.direct_message_from_1",
+ email_prefix: SiteSetting.title,
+ username: sender.username
+ )
Fabricate(:chat_message, user: sender, chat_channel: channel)
email = described_class.chat_summary(user, {})
@@ -64,7 +60,7 @@ describe UserNotifications do
expect(email.subject).not_to include(another_participant.username)
end
- it "includes both channel titles when there are exactly two with unread messages" do
+ it "includes both usernames when there are exactly two DMs with unread messages" do
another_dm_user = Fabricate(:user, group_ids: [chatters_group.id])
refresh_auto_groups
another_dm_user.reload
@@ -77,17 +73,27 @@ describe UserNotifications do
Fabricate(:chat_message, user: sender, chat_channel: channel)
email = described_class.chat_summary(user, {})
+ expected_subject = I18n.t(
+ "user_notifications.chat_summary.subject.direct_message_from_2",
+ email_prefix: SiteSetting.title,
+ username1: another_dm_user.username,
+ username2: sender.username
+ )
+
+ expect(email.subject).to eq(expected_subject)
expect(email.subject).to include(sender.username)
expect(email.subject).to include(another_dm_user.username)
end
it "displays a count when there are more than two DMs with unread messages" do
user = Fabricate(:user, group_ids: [chatters_group.id])
+ senders = []
3.times do
sender = Fabricate(:user, group_ids: [chatters_group.id])
refresh_auto_groups
sender.reload
+ senders << sender
channel =
Chat::DirectMessageChannelCreator.create!(
acting_user: sender,
@@ -101,11 +107,16 @@ describe UserNotifications do
Fabricate(:chat_message, user: sender, chat_channel: channel)
end
- expected_count_text = I18n.t("user_notifications.chat_summary.subject.others", count: 2)
-
email = described_class.chat_summary(user, {})
- expect(email.subject).to include(expected_count_text)
+ expected_subject = I18n.t(
+ "user_notifications.chat_summary.subject.direct_message_from_more",
+ email_prefix: SiteSetting.title,
+ username: senders.first.username,
+ count: 2
+ )
+
+ expect(email.subject).to eq(expected_subject)
end
it "returns an email if the user is not following the direct channel" do
@@ -144,13 +155,11 @@ describe UserNotifications do
before { Fabricate(:chat_mention, user: user, chat_message: chat_message) }
it "includes the sender username in the subject" do
- expected_subject =
- I18n.t(
- "user_notifications.chat_summary.subject.chat_channel",
- count: 1,
- email_prefix: SiteSetting.title,
- message_title: channel.title(user),
- )
+ expected_subject = I18n.t(
+ "user_notifications.chat_summary.subject.chat_channel_1",
+ email_prefix: SiteSetting.title,
+ channel: channel.title(user)
+ )
email = described_class.chat_summary(user, {})
@@ -177,6 +186,14 @@ describe UserNotifications do
email = described_class.chat_summary(user, {})
+ expected_subject = I18n.t(
+ "user_notifications.chat_summary.subject.chat_channel_2",
+ email_prefix: SiteSetting.title,
+ channel1: channel.title(user),
+ channel2: another_chat_channel.title(user)
+ )
+
+ expect(email.subject).to eq(expected_subject)
expect(email.subject).to include(channel.title(user))
expect(email.subject).to include(another_chat_channel.title(user))
end
@@ -199,11 +216,17 @@ describe UserNotifications do
)
Fabricate(:chat_mention, user: user, chat_message: another_chat_message)
end
- expected_count_text = I18n.t("user_notifications.chat_summary.subject.others", count: 2)
+
+ expected_subject = I18n.t(
+ "user_notifications.chat_summary.subject.chat_channel_more",
+ email_prefix: SiteSetting.title,
+ channel: channel.title(user),
+ count: 2
+ )
email = described_class.chat_summary(user, {})
- expect(email.subject).to include(expected_count_text)
+ expect(email.subject).to eq(expected_subject)
end
end
@@ -220,15 +243,16 @@ describe UserNotifications do
end
it "always includes the DM second" do
- expected_other_text =
- I18n.t(
- "user_notifications.chat_summary.subject.other_direct_message",
- dm_title: sender.username,
- )
+ expected_subject = I18n.t(
+ "user_notifications.chat_summary.subject.chat_channel_and_direct_message",
+ email_prefix: SiteSetting.title,
+ channel: channel.title(user),
+ username: sender.username
+ )
email = described_class.chat_summary(user, {})
- expect(email.subject).to include(expected_other_text)
+ expect(email.subject).to eq(expected_subject)
end
end
end