FIX: Allow admins to change user ignore list (#16129)

Previously, if an admin user tried to add/remove
users to another user's ignored list, it would
be added to their own ignore list because the
controller used current_user. Now for admins only
a source_user_id parameter can be passed through,
which will be used to ignore the target user for
that source user.

app/assets/javascripts/discourse/app/components/ignored-user-list.js

@@ -11,7 +11,10 @@ export default Component.extend({
       this.items.removeObject(item);
       User.findByUsername(item).then((user) => {
         user
-          .updateNotificationLevel("normal")
+          .updateNotificationLevel({
+            level: "normal",
+            actingUser: this.model,
+          })
           .catch(popupAjaxError)
           .finally(() => this.set("saved", true));
       });

app/assets/javascripts/discourse/app/controllers/ignore-duration-with-username.js

@@ -20,7 +20,11 @@ export default Controller.extend(ModalFunctionality, {
       this.set("loading", true);
       User.findByUsername(this.ignoredUsername).then((user) => {
         user
-          .updateNotificationLevel("ignore", this.ignoredUntil)
+          .updateNotificationLevel({
+            level: "ignore",
+            expiringAt: this.ignoredUntil,
+            actingUser: this.model,
+          })
           .then(() => {
             this.onUserIgnored(this.ignoredUsername);
             this.send("closeModal");

app/assets/javascripts/discourse/app/controllers/ignore-duration.js

@@ -17,7 +17,10 @@ export default Controller.extend(ModalFunctionality, {
       }
       this.set("loading", true);
       this.model
-        .updateNotificationLevel("ignore", this.ignoredUntil)
+        .updateNotificationLevel({
+          level: "ignored",
+          expiringAt: this.ignoredUntil,
+        })
         .then(() => {
           this.set("model.ignored", true);
           this.set("model.muted", false);

app/assets/javascripts/discourse/app/controllers/preferences/users.js

@@ -1,5 +1,5 @@
 import { action, computed } from "@ember/object";
-import { alias, and, or } from "@ember/object/computed";
+import { alias, and } from "@ember/object/computed";
 import Controller from "@ember/controller";
 import discourseComputed from "discourse-common/utils/decorators";
 import { makeArray } from "discourse-common/lib/helpers";
@@ -13,7 +13,10 @@ export default Controller.extend({
     return trustLevel >= this.siteSettings.min_trust_level_to_allow_ignore;
   },
 
-  ignoredEnabled: or("userCanIgnore", "model.staff"),
+  @discourseComputed("userCanIgnore", "model.staff")
+  ignoredEnabled(userCanIgnore, userIsStaff) {
+    return this.currentUser.staff || userCanIgnore || userIsStaff;
+  },
 
   allowPmUsersEnabled: and(
     "model.user_option.enable_allowed_pm_users",

app/assets/javascripts/discourse/app/controllers/user.js

@@ -250,8 +250,7 @@ export default Controller.extend(CanCheckEmails, {
     },
 
     updateNotificationLevel(level) {
-      const user = this.model;
+      return this.model.updateNotificationLevel({ level });
-      return user.updateNotificationLevel(level);
     },
   },
 });

app/assets/javascripts/discourse/app/models/user.js

@@ -780,18 +780,25 @@ const User = RestModel.extend({
     }
   },
 
-  updateNotificationLevel(level, expiringAt) {
+  updateNotificationLevel({ level, expiringAt = null, actingUser = null }) {
+    if (!actingUser) {
+      actingUser = User.current();
+    }
     return ajax(`${userPath(this.username)}/notification_level.json`, {
       type: "PUT",
-      data: { notification_level: level, expiring_at: expiringAt },
+      data: {
+        notification_level: level,
+        expiring_at: expiringAt,
+        acting_user_id: actingUser.id,
+      },
     }).then(() => {
-      const currentUser = User.current();
+      if (!actingUser.ignored_users) {
-      if (currentUser) {
+        actingUser.ignored_users = [];
-        if (level === "normal" || level === "mute") {
+      }
-          currentUser.ignored_users.removeObject(this.username);
+      if (level === "normal" || level === "mute") {
-        } else if (level === "ignore") {
+        actingUser.ignored_users.removeObject(this.username);
-          currentUser.ignored_users.addObject(this.username);
+      } else if (level === "ignore") {
-        }
+        actingUser.ignored_users.addObject(this.username);
       }
     });
   },

app/assets/javascripts/select-kit/addon/components/user-notifications-dropdown.js

@@ -49,10 +49,10 @@ export default DropdownSelectBox.extend({
   }),
 
   changeToNormal() {
-    this.updateNotificationLevel("normal").catch(popupAjaxError);
+    this.updateNotificationLevel({ level: "normal" }).catch(popupAjaxError);
   },
   changeToMuted() {
-    this.updateNotificationLevel("mute").catch(popupAjaxError);
+    this.updateNotificationLevel({ level: "mute" }).catch(popupAjaxError);
   },
   changeToIgnored() {
     showModal("ignore-duration", {

app/controllers/users_controller.rb

@@ -1329,26 +1329,42 @@ class UsersController < ApplicationController
   end
 
   def notification_level
-    user = fetch_user_from_params
+    target_user = fetch_user_from_params
+    acting_user = current_user
+
+    # the admin should be able to change notification levels
+    # on behalf of other users, so we cannot rely on current_user
+    # for this case
+    if params[:acting_user_id].present? &&
+        params[:acting_user_id].to_i != current_user.id
+      if current_user.staff?
+        acting_user = User.find(params[:acting_user_id])
+      else
+        @error_message = "error"
+        raise Discourse::InvalidAccess
+      end
+    end
 
     if params[:notification_level] == "ignore"
       @error_message = "ignore_error"
-      guardian.ensure_can_ignore_user!(user)
+      guardian.ensure_can_ignore_user!(target_user)
-      MutedUser.where(user: current_user, muted_user: user).delete_all
+      MutedUser.where(user: acting_user, muted_user: target_user).delete_all
-      ignored_user = IgnoredUser.find_by(user: current_user, ignored_user: user)
+      ignored_user = IgnoredUser.find_by(user: acting_user, ignored_user: target_user)
       if ignored_user.present?
         ignored_user.update(expiring_at: DateTime.parse(params[:expiring_at]))
       else
-        IgnoredUser.create!(user: current_user, ignored_user: user, expiring_at: Time.parse(params[:expiring_at]))
+        IgnoredUser.create!(user: acting_user, ignored_user: target_user, expiring_at: Time.parse(params[:expiring_at]))
       end
+
     elsif params[:notification_level] == "mute"
       @error_message = "mute_error"
-      guardian.ensure_can_mute_user!(user)
+      guardian.ensure_can_mute_user!(target_user)
-      IgnoredUser.where(user: current_user, ignored_user: user).delete_all
+      IgnoredUser.where(user: acting_user, ignored_user: target_user).delete_all
-      MutedUser.find_or_create_by!(user: current_user, muted_user: user)
+      MutedUser.find_or_create_by!(user: acting_user, muted_user: target_user)
+
     elsif params[:notification_level] == "normal"
-      MutedUser.where(user: current_user, muted_user: user).delete_all
+      MutedUser.where(user: acting_user, muted_user: target_user).delete_all
-      IgnoredUser.where(user: current_user, ignored_user: user).delete_all
+      IgnoredUser.where(user: acting_user, ignored_user: target_user).delete_all
     end
 
     render json: success_json

config/locales/server.en.yml

@@ -5153,6 +5153,7 @@ en:
   notification_level:
     ignore_error: "Sorry, you can't ignore that user."
     mute_error: "Sorry, you can't mute that user."
+    error: "Sorry, you cannot change the notification level for that user."
 
   discord:
     not_in_allowed_guild: "Authentication failed. You are not a member of a permitted Discord guild."

spec/requests/users_controller_spec.rb

@@ -2925,11 +2925,46 @@ describe UsersController do
 
       context 'when changing notification level to ignore' do
         it 'changes notification level to ignore' do
-          put "/u/#{another_user.username}/notification_level.json", params: { notification_level: "ignore" }
+          put "/u/#{another_user.username}/notification_level.json", params: {
+            notification_level: "ignore",
+            expiring_at: 3.days.from_now
+          }
+          expect(response.status).to eq(200)
           expect(MutedUser.count).to eq(0)
           expect(IgnoredUser.find_by(user_id: user.id, ignored_user_id: another_user.id)).to be_present
         end
 
+        it "allows admin to change the ignore status for a source user" do
+          ignored_user.destroy!
+          sign_in(Fabricate(:user, admin: true))
+          put "/u/#{another_user.username}/notification_level.json", params: {
+            notification_level: "ignore",
+            acting_user_id: user.id,
+            expiring_at: 3.days.from_now
+          }
+          expect(response.status).to eq(200)
+          expect(IgnoredUser.find_by(user_id: user.id, ignored_user_id: another_user.id)).to be_present
+        end
+
+        it "does not allow a regular user to change the ignore status for anyone but themself" do
+          ignored_user.destroy!
+          acting_user = Fabricate(:user)
+          put "/u/#{another_user.username}/notification_level.json", params: {
+            notification_level: "ignore",
+            acting_user_id: acting_user.id,
+            expiring_at: 3.days.from_now
+          }
+          expect(response.status).to eq(422)
+          expect(IgnoredUser.find_by(user_id: acting_user.id, ignored_user_id: another_user.id)).to eq(nil)
+
+          put "/u/#{another_user.username}/notification_level.json", params: {
+            notification_level: "ignore",
+            expiring_at: 3.days.from_now
+          }
+          expect(response.status).to eq(200)
+          expect(IgnoredUser.find_by(user_id: user.id, ignored_user_id: another_user.id)).to be_present
+        end
+
         context 'when expiring_at param is set' do
           it 'changes notification level to ignore' do
             freeze_time(Time.now) do