From d2116f0029ca268c9f19be79bd3b8349f25a9700 Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nbianca@users.noreply.github.com>
Date: Tue, 3 Nov 2020 17:38:56 +0200
Subject: [PATCH] FIX: Show error message if user is already silenced or
 suspended (#10988)

Users could be silenced or suspended by two staff members at the same time and
would not be aware of it. This commit shows an error message if another penalty
has been applied.
---
 .../admin/addon/mixins/penalty-controller.js  |  8 +++-
 .../admin/addon/models/admin-user.js          | 13 +-----
 .../templates/modal/admin-silence-user.hbs    |  4 ++
 .../templates/modal/admin-suspend-user.hbs    |  4 ++
 app/controllers/admin/users_controller.rb     | 19 +++++++++
 config/locales/server.en.yml                  |  2 +
 spec/requests/admin/users_controller_spec.rb  | 42 +++++++++++++++++++
 7 files changed, 78 insertions(+), 14 deletions(-)

diff --git a/app/assets/javascripts/admin/addon/mixins/penalty-controller.js b/app/assets/javascripts/admin/addon/mixins/penalty-controller.js
index caf3aee5596..250d60664f6 100644
--- a/app/assets/javascripts/admin/addon/mixins/penalty-controller.js
+++ b/app/assets/javascripts/admin/addon/mixins/penalty-controller.js
@@ -1,12 +1,13 @@
 import I18n from "I18n";
 import ModalFunctionality from "discourse/mixins/modal-functionality";
-import { popupAjaxError } from "discourse/lib/ajax-error";
+import { extractError } from "discourse/lib/ajax-error";
 import Mixin from "@ember/object/mixin";
 import { next } from "@ember/runloop";
 import { Promise } from "rsvp";
 import bootbox from "bootbox";
 
 export default Mixin.create(ModalFunctionality, {
+  errorMessage: null,
   reason: null,
   message: null,
   postEdit: null,
@@ -18,6 +19,7 @@ export default Mixin.create(ModalFunctionality, {
 
   resetModal() {
     this.setProperties({
+      errorMessage: null,
       reason: null,
       message: null,
       loadingUser: true,
@@ -66,6 +68,8 @@ export default Mixin.create(ModalFunctionality, {
           callback(result);
         }
       })
-      .catch(popupAjaxError);
+      .catch((error) => {
+        this.set("errorMessage", extractError(error));
+      });
   },
 });
diff --git a/app/assets/javascripts/admin/addon/models/admin-user.js b/app/assets/javascripts/admin/addon/models/admin-user.js
index 4a58d212733..0bc1132623b 100644
--- a/app/assets/javascripts/admin/addon/models/admin-user.js
+++ b/app/assets/javascripts/admin/addon/models/admin-user.js
@@ -352,28 +352,17 @@ const AdminUser = User.extend({
       type: "PUT",
     })
       .then((result) => this.setProperties(result.unsilence))
-      .catch((e) => {
-        const error = I18n.t("admin.user.unsilence_failed", {
-          error: this._formatError(e),
-        });
-        bootbox.alert(error);
-      })
       .finally(() => this.set("silencingUser", false));
   },
 
   silence(data) {
     this.set("silencingUser", true);
+
     return ajax(`/admin/users/${this.id}/silence`, {
       type: "PUT",
       data,
     })
       .then((result) => this.setProperties(result.silence))
-      .catch((e) => {
-        const error = I18n.t("admin.user.silence_failed", {
-          error: this._formatError(e),
-        });
-        bootbox.alert(error);
-      })
       .finally(() => this.set("silencingUser", false));
   },
 
diff --git a/app/assets/javascripts/admin/addon/templates/modal/admin-silence-user.hbs b/app/assets/javascripts/admin/addon/templates/modal/admin-silence-user.hbs
index 5e18b4cfb40..db3a184437b 100644
--- a/app/assets/javascripts/admin/addon/templates/modal/admin-silence-user.hbs
+++ b/app/assets/javascripts/admin/addon/templates/modal/admin-silence-user.hbs
@@ -1,6 +1,10 @@
 {{#d-modal-body title="admin.user.silence_modal_title"}}
   {{#conditional-loading-spinner condition=loadingUser}}
 
+    {{#if errorMessage}}
+      <div class="alert alert-error">{{errorMessage}}</div>
+    {{/if}}
+
     <div class="until-controls">
       <label>
         {{future-date-input
diff --git a/app/assets/javascripts/admin/addon/templates/modal/admin-suspend-user.hbs b/app/assets/javascripts/admin/addon/templates/modal/admin-suspend-user.hbs
index 24e837efead..34c2685696f 100644
--- a/app/assets/javascripts/admin/addon/templates/modal/admin-suspend-user.hbs
+++ b/app/assets/javascripts/admin/addon/templates/modal/admin-suspend-user.hbs
@@ -1,6 +1,10 @@
 {{#d-modal-body title="admin.user.suspend_modal_title"}}
   {{#conditional-loading-spinner condition=loadingUser}}
 
+    {{#if errorMessage}}
+      <div class="alert alert-error">{{errorMessage}}</div>
+    {{/if}}
+
     {{#if user.canSuspend}}
       <div class="until-controls">
         <label>
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index 48b292cc1c5..5815b0c34cc 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -93,6 +93,16 @@ class Admin::UsersController < Admin::AdminController
 
   def suspend
     guardian.ensure_can_suspend!(@user)
+
+    if @user.suspended?
+      suspend_record = @user.suspend_record
+      message = I18n.t("user.already_suspended",
+        staff: suspend_record.acting_user.username,
+        time_ago: FreedomPatches::Rails4.time_ago_in_words(suspend_record.created_at, true, scope: :'datetime.distance_in_words_verbose')
+      )
+      return render json: failed_json.merge(message: message), status: 409
+    end
+
     params.require([:suspend_until, :reason])
 
     @user.suspended_till = params[:suspend_until]
@@ -315,6 +325,15 @@ class Admin::UsersController < Admin::AdminController
   def silence
     guardian.ensure_can_silence_user! @user
 
+    if @user.silenced?
+      silenced_record = @user.silenced_record
+      message = I18n.t("user.already_silenced",
+        staff: silenced_record.acting_user.username,
+        time_ago: FreedomPatches::Rails4.time_ago_in_words(silenced_record.created_at, true, scope: :'datetime.distance_in_words_verbose')
+      )
+      return render json: failed_json.merge(message: message), status: 409
+    end
+
     message = params[:message]
 
     silencer = UserSilencer.new(
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 5133993cd7e..773c6272de7 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -2507,6 +2507,8 @@ en:
       same_ip_address: "Same IP address (%{ip_address}) as other users"
       inactive_user: "Inactive user"
     email_in_spam_header: "User's first email was flagged as spam"
+    already_silenced: "User was already silenced by %{staff} %{time_ago}."
+    already_suspended: "User was already suspended by %{staff} %{time_ago}."
 
   reviewables_reminder:
     submitted:
diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb
index 83bd456dfbe..60404b6a960 100644
--- a/spec/requests/admin/users_controller_spec.rb
+++ b/spec/requests/admin/users_controller_spec.rb
@@ -149,6 +149,27 @@ RSpec.describe Admin::UsersController do
       expect(log.details).to match(/because I said so/)
     end
 
+    it "checks if user is suspended" do
+      put "/admin/users/#{user.id}/suspend.json", params: {
+        suspend_until: 5.hours.from_now,
+        reason: "because I said so"
+      }
+
+      put "/admin/users/#{user.id}/suspend.json", params: {
+        suspend_until: 5.hours.from_now,
+        reason: "because I said so too"
+      }
+
+      expect(response.status).to eq(409)
+      expect(response.parsed_body["message"]).to eq(
+        I18n.t(
+          "user.already_suspended",
+          staff: admin.username,
+          time_ago: FreedomPatches::Rails4.time_ago_in_words(user.suspend_record.created_at, true, scope: :'datetime.distance_in_words_verbose')
+        )
+      )
+    end
+
     it "requires suspend_until and reason" do
       expect(user).not_to be_suspended
       put "/admin/users/#{user.id}/suspend.json", params: {}
@@ -749,6 +770,27 @@ RSpec.describe Admin::UsersController do
       reg_user.reload
       expect(reg_user).to be_silenced
     end
+
+    it "checks if user is silenced" do
+      put "/admin/users/#{user.id}/silence.json", params: {
+        silenced_till: 5.hours.from_now,
+        reason: "because I said so"
+      }
+
+      put "/admin/users/#{user.id}/silence.json", params: {
+        silenced_till: 5.hours.from_now,
+        reason: "because I said so too"
+      }
+
+      expect(response.status).to eq(409)
+      expect(response.parsed_body["message"]).to eq(
+        I18n.t(
+          "user.already_silenced",
+          staff: admin.username,
+          time_ago: FreedomPatches::Rails4.time_ago_in_words(user.silenced_record.created_at, true, scope: :'datetime.distance_in_words_verbose')
+        )
+      )
+    end
   end
 
   describe '#unsilence' do