From 1a8c949900993c229038916d6aeb0e319b470c41 Mon Sep 17 00:00:00 2001 From: Andrei Prigorshnev Date: Tue, 20 Jul 2021 14:42:08 +0400 Subject: [PATCH] UX: suspend forever time period messages (#13776) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the Forever option is selected for suspending a user, the user is suspended for 1000 years. Without customizing the site’s text, this time period is displayed to the user in the suspension email that is sent to the user, and if the user attempts to log back into the site. Telling someone that they have been suspended for 1000 years seems likely to come across as a bad attempt at humour. This PR special case messages when a user suspended or silenced forever. --- app/controllers/session_controller.rb | 7 +--- app/mailers/user_notifications.rb | 46 ++++++++++++++++-------- app/models/user.rb | 25 +++++++++++++ config/locales/server.en.yml | 17 +++++++++ lib/auth/result.rb | 3 +- spec/requests/session_controller_spec.rb | 22 +++++++++--- 6 files changed, 94 insertions(+), 26 deletions(-) diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index ffd938165a5..58574161954 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -589,13 +589,8 @@ class SessionController < ApplicationController end def failed_to_login(user) - message = user.suspend_reason ? "login.suspended_with_reason" : "login.suspended" - { - error: I18n.t(message, - date: I18n.l(user.suspended_till, format: :date_only), - reason: Rack::Utils.escape_html(user.suspend_reason) - ), + error: user.suspended_message, reason: 'suspended' } end diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index d5a20e0f073..87703304dd3 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -153,13 +153,22 @@ class UserNotifications < ActionMailer::Base return unless user_history = opts[:user_history] - build_email( - user.email, - template: "user_notifications.account_silenced", - locale: user_locale(user), - reason: user_history.details, - silenced_till: I18n.l(user.silenced_till, format: :long) - ) + if user.silenced_forever? + build_email( + user.email, + template: "user_notifications.account_silenced_forever", + locale: user_locale(user), + reason: user_history.details + ) + else + build_email( + user.email, + template: "user_notifications.account_silenced", + locale: user_locale(user), + reason: user_history.details, + silenced_till: I18n.l(user.silenced_till, format: :long) + ) + end end def account_suspended(user, opts = nil) @@ -167,13 +176,22 @@ class UserNotifications < ActionMailer::Base return unless user_history = opts[:user_history] - build_email( - user.email, - template: "user_notifications.account_suspended", - locale: user_locale(user), - reason: user_history.details, - suspended_till: I18n.l(user.suspended_till, format: :long) - ) + if user.suspended_forever? + build_email( + user.email, + template: "user_notifications.account_suspended_forever", + locale: user_locale(user), + reason: user_history.details + ) + else + build_email( + user.email, + template: "user_notifications.account_suspended", + locale: user_locale(user), + reason: user_history.details, + suspended_till: I18n.l(user.suspended_till, format: :long) + ) + end end def account_exists(user, opts = {}) diff --git a/app/models/user.rb b/app/models/user.rb index da4dbeda2ad..4f807458db6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -958,6 +958,10 @@ class User < ActiveRecord::Base silenced_record.try(:created_at) if silenced? end + def silenced_forever? + silenced_till > 100.years.from_now + end + def suspend_record UserHistory.for(self, :suspend_user).order('id DESC').first end @@ -974,6 +978,27 @@ class User < ActiveRecord::Base nil end + def suspended_message + return nil unless suspended? + + message = "login.suspended" + if suspend_reason + if suspended_forever? + message = "login.suspended_with_reason_forever" + else + message = "login.suspended_with_reason" + end + end + + I18n.t(message, + date: I18n.l(suspended_till, format: :date_only), + reason: Rack::Utils.escape_html(suspend_reason)) + end + + def suspended_forever? + suspended_till > 100.years.from_now + end + # Use this helper to determine if the user has a particular trust level. # Takes into account admin, etc. def has_trust_level?(level) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 3b57d2ae72c..d92c007e50e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2515,6 +2515,7 @@ en: reset_not_allowed_from_ip_address: "You can't request a password reset from that IP address." suspended: "You can't log in until %{date}." suspended_with_reason: "Account suspended until %{date}: %{reason}" + suspended_with_reason_forever: "Account suspended: %{reason}" errors: "%{errors}" not_available: "Not available. Try %{suggestion}?" something_already_taken: "Something went wrong, perhaps the username or email is already registered. Try the forgot password link." @@ -3703,6 +3704,14 @@ en: Reason - %{reason} + account_suspended_forever: + title: "Account Suspended" + subject_template: "[%{email_prefix}] Your account has been suspended" + text_body_template: | + You have been suspended from the forum. + + Reason - %{reason} + account_silenced: title: "Account Silenced" subject_template: "[%{email_prefix}] Your account has been silenced" @@ -3711,6 +3720,14 @@ en: Reason - %{reason} + account_silenced_forever: + title: "Account Silenced" + subject_template: "[%{email_prefix}] Your account has been silenced" + text_body_template: | + You have been silenced from the forum. + + Reason - %{reason} + account_exists: title: "Account already exists" subject_template: "[%{email_prefix}] Account already exists" diff --git a/lib/auth/result.rb b/lib/auth/result.rb index 1b9c1d85b0b..08a12203d2a 100644 --- a/lib/auth/result.rb +++ b/lib/auth/result.rb @@ -111,8 +111,7 @@ class Auth::Result if user&.suspended? return { suspended: true, - suspended_message: I18n.t(user.suspend_reason ? "login.suspended_with_reason" : "login.suspended", - date: I18n.l(user.suspended_till, format: :date_only), reason: user.suspend_reason) + suspended_message: user.suspended_message } end diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index 6bf7b0e05a5..74fce7879a6 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -1401,11 +1401,25 @@ RSpec.describe SessionController do login: user.username, password: 'myawesomepassword' } + expected_message = I18n.t('login.suspended_with_reason', + date: I18n.l(user.suspended_till, format: :date_only), + reason: Rack::Utils.escape_html(user.suspend_reason)) expect(response.status).to eq(200) - expect(response.parsed_body['error']).to eq(I18n.t('login.suspended_with_reason', - date: I18n.l(user.suspended_till, format: :date_only), - reason: Rack::Utils.escape_html(user.suspend_reason) - )) + expect(response.parsed_body['error']).to eq(expected_message) + end + + it 'when suspended forever should return an error without suspended till date' do + user.suspended_till = 101.years.from_now + user.suspended_at = Time.now + user.save! + StaffActionLogger.new(user).log_user_suspend(user, "banned") + + post "/session.json", params: { + login: user.username, password: 'myawesomepassword' + } + + expected_message = I18n.t('login.suspended_with_reason_forever', reason: Rack::Utils.escape_html(user.suspend_reason)) + expect(response.parsed_body['error']).to eq(expected_message) end end