From 57584c38c05dbeb7823063c2e98dd79a07f3ed83 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Thu, 23 Nov 2023 15:54:22 +0100 Subject: [PATCH] FIX: correctly uses private_email site setting in chat (#24528) Chat will now check for the state of `SiteSetting.private_email` when sending the summary, when enabled, the mail will not display user information, channel information other than the ID and no message information, only the count of messages. --- .../user_notifications/chat_summary.html.erb | 58 ++++++---- plugins/chat/config/locales/server.en.yml | 2 + .../lib/chat/user_notifications_extension.rb | 9 ++ .../spec/mailers/user_notifications_spec.rb | 108 ++++++++++++++++-- 4 files changed, 143 insertions(+), 34 deletions(-) diff --git a/plugins/chat/app/views/user_notifications/chat_summary.html.erb b/plugins/chat/app/views/user_notifications/chat_summary.html.erb index d3a235d50df..f9c444975d7 100644 --- a/plugins/chat/app/views/user_notifications/chat_summary.html.erb +++ b/plugins/chat/app/views/user_notifications/chat_summary.html.erb @@ -26,37 +26,51 @@ -
<%= chat_channel.title(@user) %>
+
+ <%- if SiteSetting.private_email %> + <%= I18n.t("system_messages.private_channel_title", id: chat_channel.id) %> + <%- else %> + <%= chat_channel.title(@user) %> + <%- end %> +
- <%- messages.take(2).each do |chat_message| %> - <%- sender = chat_message.user %> - <%- sender_name = @display_usernames ? sender.username : sender.name %> - - - <%= sender_name -%> - - <%= sender_name -%> - - - <%= I18n.l(@user_tz.to_local(chat_message.created_at), format: :long) -%> - - - - - - <%= email_excerpt(chat_message.cooked_for_excerpt) %> - - + <%- unless SiteSetting.private_email %> + <%- messages.take(2).each do |chat_message| %> + <%- sender = chat_message.user %> + <%- sender_name = @display_usernames ? sender.username : sender.name %> + + + + <%= sender_name -%> + + <%= sender_name -%> + + + <%= I18n.l(@user_tz.to_local(chat_message.created_at), format: :long) -%> + + + + + + <%= email_excerpt(chat_message.cooked_for_excerpt) %> + + + <%- end %> <%- end %> + - <%- if other_messages_count <= 0 %> + <%- if SiteSetting.private_email %> <%= I18n.t("user_notifications.chat_summary.view_messages", count: messages.size)%> <%- else %> - <%= I18n.t("user_notifications.chat_summary.view_more", count: other_messages_count)%> + <%- if other_messages_count <= 0 %> + <%= I18n.t("user_notifications.chat_summary.view_messages", count: messages.size)%> + <%- else %> + <%= I18n.t("user_notifications.chat_summary.view_more", count: other_messages_count)%> + <%- end %> <%- end %> diff --git a/plugins/chat/config/locales/server.en.yml b/plugins/chat/config/locales/server.en.yml index b2ab31734af..b8d3147f40b 100644 --- a/plugins/chat/config/locales/server.en.yml +++ b/plugins/chat/config/locales/server.en.yml @@ -30,6 +30,7 @@ en: direct_message_enabled_groups_invalid: "You must specify at least one group for this setting. If you do not want anyone except staff to send direct messages, choose the staff group." chat_upload_not_allowed_secure_uploads: "Chat uploads are not allowed when secure uploads site setting is enabled." system_messages: + private_channel_title: "Channel %{id}" chat_channel_archive_complete: title: "Chat Channel Archive Complete" subject_template: "Chat channel archive completed successfully" @@ -233,6 +234,7 @@ en: other: "You have new chat messages" from: "%{site_name}" subject: + private_message: "[%{email_prefix}] New message" 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: diff --git a/plugins/chat/lib/chat/user_notifications_extension.rb b/plugins/chat/lib/chat/user_notifications_extension.rb index 4e06689d700..c36836242cc 100644 --- a/plugins/chat/lib/chat/user_notifications_extension.rb +++ b/plugins/chat/lib/chat/user_notifications_extension.rb @@ -53,6 +53,15 @@ module Chat end def summary_subject(user, grouped_messages) + if SiteSetting.private_email + return( + I18n.t( + "user_notifications.chat_summary.subject.private_message", + email_prefix: @email_prefix, + ) + ) + end + all_channels = grouped_messages.keys grouped_channels = all_channels.partition { |c| !c.direct_message_channel? } channels = grouped_channels.first diff --git a/plugins/chat/spec/mailers/user_notifications_spec.rb b/plugins/chat/spec/mailers/user_notifications_spec.rb index 01f81c9299a..236d15b7d47 100644 --- a/plugins/chat/spec/mailers/user_notifications_spec.rb +++ b/plugins/chat/spec/mailers/user_notifications_spec.rb @@ -33,6 +33,22 @@ describe UserNotifications do end describe "email subject" do + context "when private_email setting is enabled" do + before { SiteSetting.private_email = true } + + it "has a generic subject" do + Fabricate(:chat_message, user: sender, chat_channel: channel) + email = described_class.chat_summary(user, {}) + + expect(email.subject).to eq( + I18n.t( + "user_notifications.chat_summary.subject.private_message", + email_prefix: SiteSetting.email_prefix.presence || SiteSetting.title, + ), + ) + end + end + it "includes the sender username in the subject" do expected_subject = I18n.t( @@ -245,6 +261,21 @@ describe UserNotifications do ) end + context "when private_email setting is enabled" do + before { SiteSetting.private_email = true } + + it "has a generic subject" do + email = described_class.chat_summary(user, {}) + + expect(email.subject).to eq( + I18n.t( + "user_notifications.chat_summary.subject.private_message", + email_prefix: SiteSetting.email_prefix.presence || SiteSetting.title, + ), + ) + end + end + it "includes the sender username in the subject" do expected_subject = I18n.t( @@ -500,6 +531,31 @@ describe UserNotifications do end describe "mail contents" do + context "when private_email setting is enabled" do + before { SiteSetting.private_email = true } + + it "has a generic channel title name" do + email = described_class.chat_summary(user, {}) + + expect(email.html_part.body.to_s).to include( + I18n.t("system_messages.private_channel_title", id: channel.id), + ) + end + + it "doesn’t include message content" do + email = described_class.chat_summary(user, {}) + + expect(email.html_part.body.to_s).to_not include(chat_message.cooked_for_excerpt) + end + + it "doesn’t include user info" do + email = described_class.chat_summary(user, {}) + + expect(email.html_part.body.to_s).to_not include(chat_message.user.small_avatar_url) + expect(email.html_part.body.to_s).to_not include(chat_message.user.username) + end + end + it "returns an email when the user has unread mentions" do email = described_class.chat_summary(user, {}) @@ -581,21 +637,49 @@ describe UserNotifications do expect(user_avatar.attribute("alt").value).to eq(sender.username) end - it "includes a view more link when there are more than two mentions" do - 2.times do - msg = Fabricate(:chat_message, user: sender, chat_channel: channel) - notification = Fabricate(:notification) - Fabricate(:chat_mention, user: user, chat_message: msg, notification: notification) + context "when there are more than two mentions" do + it "includes a view more link " do + 2.times do + msg = Fabricate(:chat_message, user: sender, chat_channel: channel) + notification = Fabricate(:notification) + Fabricate(:chat_mention, user: user, chat_message: msg, notification: notification) + end + + email = described_class.chat_summary(user, {}) + more_messages_channel_link = + Nokogiri::HTML5.fragment(email.html_part.body.to_s).css(".more-messages-link") + + expect(more_messages_channel_link.attribute("href").value).to eq( + chat_message.full_url, + ) + expect(more_messages_channel_link.text).to include( + I18n.t("user_notifications.chat_summary.view_more", count: 1), + ) end - email = described_class.chat_summary(user, {}) - more_messages_channel_link = - Nokogiri::HTML5.fragment(email.html_part.body.to_s).css(".more-messages-link") + context "when private_email setting is enabled" do + before { SiteSetting.private_email = true } - expect(more_messages_channel_link.attribute("href").value).to eq(chat_message.full_url) - expect(more_messages_channel_link.text).to include( - I18n.t("user_notifications.chat_summary.view_more", count: 1), - ) + it "has only a link to view all messages" do + 2.times do + msg = Fabricate(:chat_message, user: sender, chat_channel: channel) + notification = Fabricate(:notification) + Fabricate( + :chat_mention, + user: user, + chat_message: msg, + notification: notification, + ) + end + + email = described_class.chat_summary(user, {}) + more_messages_channel_link = + Nokogiri::HTML5.fragment(email.html_part.body.to_s).css(".more-messages-link") + expect(more_messages_channel_link.text).to include( + I18n.t("user_notifications.chat_summary.view_messages", count: 3), + ) + end + end end it "doesn't repeat mentions we already sent" do