diff --git a/app/assets/javascripts/admin/models/admin-user.js.es6 b/app/assets/javascripts/admin/models/admin-user.js.es6 index 59c3c3ee1d3..4bbe46f4464 100644 --- a/app/assets/javascripts/admin/models/admin-user.js.es6 +++ b/app/assets/javascripts/admin/models/admin-user.js.es6 @@ -244,17 +244,13 @@ const AdminUser = Discourse.User.extend({ return ajax(`/admin/users/${this.id}/suspend`, { type: 'PUT', data - }).then(result => { - this.setProperties(result.suspension); - }); + }).then(result => this.setProperties(result.suspension)); }, unsuspend() { return ajax(`/admin/users/${this.id}/unsuspend`, { type: 'PUT' - }).then(result => { - this.setProperties(result.suspension); - }); + }).then(result => this.setProperties(result.suspension)); }, logOut() { diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 07dd472db9e..d8b21aa0b61 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -56,11 +56,31 @@ class Admin::UsersController < Admin::AdminController @user.suspended_till = params[:duration].to_i.days.from_now @user.suspended_at = DateTime.now - @user.save! - @user.revoke_api_key - StaffActionLogger.new(current_user).log_user_suspend(@user, params[:reason]) + message = params[:message] + + user_history = nil + + User.transaction do + @user.save! + @user.revoke_api_key + + user_history = StaffActionLogger.new(current_user).log_user_suspend( + @user, + params[:reason], + context: message + ) + 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 + render_json_dump( suspension: { suspended: true, diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb index ce9909da386..0eeea9ecbb5 100644 --- a/app/jobs/regular/user_email.rb +++ b/app/jobs/regular/user_email.rb @@ -29,14 +29,13 @@ module Jobs notification = Notification.find_by(id: args[:notification_id]) end - message, skip_reason = message_for_email(user, - post, - type, - notification, - args[:notification_type], - args[:notification_data_hash], - args[:email_token], - args[:to_address]) + message, skip_reason = message_for_email( + user, + post, + type, + notification, + args + ) if message Email::Sender.new(message, type, user).send @@ -57,11 +56,21 @@ module Jobs quoted } - def message_for_email(user, post, type, notification, notification_type = nil, notification_data_hash = nil, email_token = nil, to_address = nil) + def message_for_email(user, post, type, notification, args = nil) + args ||= {} + + notification_type = args[:notification_type] + notification_data_hash = args[:notification_data_hash] + email_token = args[:email_token] + to_address = args[:to_address] + set_skip_context(type, user.id, to_address || user.email, post.try(:id)) return skip_message(I18n.t("email_log.anonymous_user")) if user.anonymous? - return skip_message(I18n.t("email_log.suspended_not_pm")) if user.suspended? && type.to_s != "user_private_message" + + if user.suspended? && !["user_private_message", "account_suspended"].include?(type.to_s) + return skip_message(I18n.t("email_log.suspended_not_pm")) + end return if user.staged && type.to_s == "digest" @@ -108,8 +117,8 @@ module Jobs # Make sure that mailer exists raise Discourse::InvalidParameters.new("type=#{type}") unless UserNotifications.respond_to?(type) - email_args[:email_token] = email_token if email_token.present? - email_args[:new_email] = user.email if type.to_s == "notify_old_email" + email_args[:email_token] = email_token if email_token.present? + email_args[:new_email] = user.email if type.to_s == "notify_old_email" if EmailLog.reached_max_emails?(user, type.to_s) return skip_message(I18n.t('email_log.exceeded_emails_limit')) @@ -119,6 +128,10 @@ module Jobs return skip_message(I18n.t('email_log.exceeded_bounces_limit')) end + if args[:user_history_id] + email_args[:user_history] = UserHistory.where(id: args[:user_history_id]).first + end + message = EmailLog.unique_email_per_post(post, user) do UserNotifications.send(type, user, email_args) end diff --git a/app/jobs/scheduled/pending_flags_reminder.rb b/app/jobs/scheduled/pending_flags_reminder.rb index 128a52187c6..817b735b1b2 100644 --- a/app/jobs/scheduled/pending_flags_reminder.rb +++ b/app/jobs/scheduled/pending_flags_reminder.rb @@ -35,7 +35,7 @@ module Jobs end def pending_flag_ids - FlagQuery.flagged_post_actions('active') + FlagQuery.flagged_post_actions(filter: 'active') .where('post_actions.created_at < ?', SiteSetting.notify_about_flags_after.to_i.hours.ago) .pluck(:id) end diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index c4ba2c5532c..83ba60ad085 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -68,6 +68,21 @@ class UserNotifications < ActionMailer::Base email_token: opts[:email_token]) end + def account_suspended(user, opts = nil) + opts ||= {} + + return unless user_history = opts[:user_history] + + build_email( + user.email, + 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 + def short_date(dt) if dt.year == Time.now.year I18n.l(dt, format: :short_no_year) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index e2d912f049d..9f9f8ec0019 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2630,6 +2630,17 @@ en: %{message} + account_suspended: + title: "Account Suspended" + subject_template: "[%{email_prefix}] Your account has been suspended" + text_body_template: | + You have been suspended from the forum until %{suspended_till}. + + %{reason} + + %{message} + + digest: why: "A brief summary of %{site_link} since your last visit on %{last_seen_at}" since_last_visit: "Since your last visit" diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index ad209931c3a..8a714f29721 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -121,8 +121,53 @@ describe Admin::UsersController do end context '.suspend' do + let(:user) { Fabricate(:evil_trout) } + let!(:api_key) { Fabricate(:api_key, user: user) } - let(:evil_trout) { Fabricate(:evil_trout) } + it "works properly" do + put( + :suspend, + user_id: user.id, + duration: 10, + reason: "because I said so", + format: :json + ) + expect(response).to be_success + + user.reload + expect(user.suspended_at).to be_present + expect(user.suspended_till).to be_present + expect(ApiKey.where(user_id: user.id).count).to eq(0) + + log = UserHistory.where(target_user_id: user.id).order('id desc').first + expect(log).to be_present + expect(log.details).to match(/because I said so/) + end + + it "can send a message to the user" do + Jobs.expects(:enqueue).with( + :critical_user_email, + has_entries( + type: :account_suspended, + user_id: user.id, + message: "long reason" + ) + ) + + put( + :suspend, + user_id: user.id, + duration: 10, + reason: "short reason", + message: "long reason", + format: :json + ) + expect(response).to be_success + + log = UserHistory.where(target_user_id: user.id).order('id desc').first + expect(log).to be_present + expect(log.details).to match(/short reason/) + expect(log.details).to match(/long reason/) it "also revoke any api keys" do User.any_instance.expects(:revoke_api_key)