From 6b4d0668343b2990e4b1e0fd4b93f4ebdefdff4f Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Tue, 2 Mar 2021 09:18:09 +1000 Subject: [PATCH] FIX: Skip sending PM email for user silence (#12240) We were sending 2 emails for user silencing if a message was provided in the UI. Also always send email for user silence and user suspend with reason regardless of whether message provided. --- app/controllers/admin/users_controller.rb | 16 +++++++--------- app/mailers/user_notifications.rb | 2 -- app/services/user_silencer.rb | 1 + config/locales/server.en.yml | 8 ++------ lib/system_message.rb | 3 ++- spec/services/user_silencer_spec.rb | 7 +++++++ 6 files changed, 19 insertions(+), 18 deletions(-) diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 69dfc97392e..0e5567313c9 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -124,14 +124,12 @@ class Admin::UsersController < Admin::AdminController end @user.logged_out - if message.present? - Jobs.enqueue( - :critical_user_email, - type: :account_suspended, - user_id: @user.id, - user_history_id: user_history.id - ) - end + Jobs.enqueue( + :critical_user_email, + type: :account_suspended, + user_id: @user.id, + user_history_id: user_history.id + ) DiscourseEvent.trigger( :user_suspended, @@ -345,7 +343,7 @@ class Admin::UsersController < Admin::AdminController keep_posts: true, post_id: params[:post_id] ) - if silencer.silence && message.present? + if silencer.silence Jobs.enqueue( :critical_user_email, type: :account_silenced, diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index bd7e6065ab4..6ccb6476da1 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -144,7 +144,6 @@ class UserNotifications < ActionMailer::Base template: "user_notifications.account_silenced", locale: user_locale(user), reason: user_history.details, - message: user_history.context, silenced_till: I18n.l(user.silenced_till, format: :long) ) end @@ -159,7 +158,6 @@ class UserNotifications < ActionMailer::Base template: "user_notifications.account_suspended", locale: user_locale(user), reason: user_history.details, - message: user_history.context, suspended_till: I18n.l(user.suspended_till, format: :long) ) end diff --git a/app/services/user_silencer.rb b/app/services/user_silencer.rb index 51f7d23ce58..fc74af58da9 100644 --- a/app/services/user_silencer.rb +++ b/app/services/user_silencer.rb @@ -61,6 +61,7 @@ class UserSilencer silence_message_params: silence_message_params ) + silence_message_params.merge!(post_alert_options: { skip_send_email: true }) SystemMessage.create(@user, message_type, silence_message_params) true end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 43242b206f9..c705a492428 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -3643,9 +3643,7 @@ en: text_body_template: | You have been suspended from the forum until %{suspended_till}. - %{reason} - - %{message} + Reason - %{reason} account_silenced: title: "Account Silenced" @@ -3653,9 +3651,7 @@ en: text_body_template: | You have been silenced from the forum until %{silenced_till}. - %{reason} - - %{message} + Reason - %{reason} account_exists: title: "Account already exists" diff --git a/lib/system_message.rb b/lib/system_message.rb index 648eede9238..1937483b286 100644 --- a/lib/system_message.rb +++ b/lib/system_message.rb @@ -39,7 +39,8 @@ class SystemMessage target_usernames: @recipient.username, target_group_names: target_group_names, subtype: TopicSubtype.system_message, - skip_validations: true) + skip_validations: true, + post_alert_options: params[:post_alert_options]) post = I18n.with_locale(@recipient.effective_locale) { creator.create } diff --git a/spec/services/user_silencer_spec.rb b/spec/services/user_silencer_spec.rb index 6ed90623538..dfd5df1946c 100644 --- a/spec/services/user_silencer_spec.rb +++ b/spec/services/user_silencer_spec.rb @@ -33,6 +33,13 @@ describe UserSilencer do expect(count).to eq(1) end + it "skips sending the email for the silence PM via post alert" do + NotificationEmailer.enable + Jobs.run_immediately! + UserSilencer.silence(user, admin) + expect(ActionMailer::Base.deliveries.size).to eq(0) + end + it 'does not hide posts for tl1' do user.update!(trust_level: 1)