consolidate error msgs; allow re-bans

src/core/client/admin/components/BanModal.tsx

@@ -1,4 +1,5 @@
 import { Localized } from "@fluent/react/compat";
+import { FORM_ERROR } from "final-form";
 import React, {
   FunctionComponent,
   useCallback,
@@ -221,27 +222,39 @@ const BanModal: FunctionComponent<Props> = ({
   const onFormSubmit = useCallback(async () => {
     switch (updateType) {
       case UpdateType.ALL_SITES:
-        await banUser({
-          userID, // Should be defined because the modal shouldn't open if author is null
-          message: customizeMessage ? emailMessage : getDefaultMessage,
-          rejectExistingComments,
-          siteIDs: viewerIsScoped
-            ? viewer?.moderationScopes?.sites?.map(({ id }) => id)
-            : [],
-        });
+        try {
+          await banUser({
+            userID, // Should be defined because the modal shouldn't open if author is null
+            message: customizeMessage ? emailMessage : getDefaultMessage,
+            rejectExistingComments,
+            siteIDs: viewerIsScoped
+              ? viewer?.moderationScopes?.sites?.map(({ id }) => id)
+              : [],
+          });
+        } catch (err) {
+          return { [FORM_ERROR]: err.message };
+        }
         break;
       case UpdateType.SPECIFIC_SITES:
-        await updateUserBan({
-          userID,
-          message: customizeMessage ? emailMessage : getDefaultMessage,
-          banSiteIDs,
-          unbanSiteIDs,
-        });
+        try {
+          await updateUserBan({
+            userID,
+            message: customizeMessage ? emailMessage : getDefaultMessage,
+            banSiteIDs,
+            unbanSiteIDs,
+          });
+        } catch (err) {
+          return { [FORM_ERROR]: err.message };
+        }
         break;
       case UpdateType.NO_SITES:
-        await removeUserBan({
-          userID,
-        });
+        try {
+          await removeUserBan({
+            userID,
+          });
+        } catch (err) {
+          return { [FORM_ERROR]: err.message };
+        }
     }
     if (banDomain) {
       void createDomainBan({

src/core/client/stream/tabs/Comments/Comment/UserBanPopover/UserBanPopoverContainer.tsx

@@ -124,13 +124,8 @@ const UserBanPopoverContainer: FunctionComponent<Props> = ({
         });
       }
     } catch (e) {
-      if (e.message === "CANNOT_BAN_ACCOUNT_WITH_MOD_PRIVILEGES") {
-        const errorMessage = getMessage(
-          localeBundles,
-          "comments-userBanPopover-moderator-ban-error",
-          "Cannot ban accounts with moderator privileges"
-        );
-        setBanError(errorMessage);
+      if (e.message) {
+        setBanError(e.message);
       }
     }
     if (siteBan) {

src/core/client/stream/test/comments/stream/moderation.spec.tsx

@@ -579,7 +579,7 @@ it("site moderator cannot ban another moderator with site privileges", async ()
                 siteIDs: ["site-id"],
               });
               throw new InvalidRequestError({
-                code: ERROR_CODES.CANNOT_BAN_ACCOUNT_WITH_MOD_PRIVILEGES,
+                code: ERROR_CODES.MODERATOR_CANNOT_BE_BANNED_ON_SITE,
                 param: "input.body",
                 traceID: "traceID",
               });

src/core/common/errors.ts

@@ -267,12 +267,6 @@ export enum ERROR_CODES {
    */
   USER_ALREADY_BANNED = "USER_ALREADY_BANNED",
 
-  /**
-   * CANNOT_BAN_ACCOUNT_WITH_MOD_PRIVILEGES is returned when a moderator attempts
-   * to ban another moderator with site privileges.
-   */
-  CANNOT_BAN_ACCOUNT_WITH_MOD_PRIVILEGES = "CANNOT_BAN_ACCOUNT_WITH_MOD_PRIVILEGES",
-
   /**
    * USER_SUSPENDED is returned when the user attempts to perform an action that
    * is not permitted if they are suspended.

src/core/server/errors/index.ts

@@ -783,14 +783,6 @@ export class UserAlreadyBannedError extends CoralError {
   }
 }
 
-export class CannotBanAccountWithModPrivilegesError extends CoralError {
-  constructor() {
-    super({
-      code: ERROR_CODES.CANNOT_BAN_ACCOUNT_WITH_MOD_PRIVILEGES,
-    });
-  }
-}
-
 export class UserBanned extends CoralError {
   constructor(userID: string, resource?: string, operation?: string) {
     super({

src/core/server/errors/translations.ts

@@ -47,9 +47,7 @@ export const ERROR_TRANSLATIONS: Record<ERROR_CODES, string> = {
   USER_ALREADY_BANNED: "error-userAlreadyBanned",
   USER_BANNED: "error-userBanned",
   USER_SITE_BANNED: "error-userSiteBanned",
-  MODERATOR_CANNOT_BE_BANNED_ON_SITE: "error-moderatorCannotBeBannedOnSite",
-  CANNOT_BAN_ACCOUNT_WITH_MOD_PRIVILEGES:
-    "error-cannotBanAccountWithModPrivileges",
+  MODERATOR_CANNOT_BE_BANNED_ON_SITE: "error-cannotBanAccountWithModPrivileges",
   USER_SUSPENDED: "error-userSuspended",
   USER_WARNED: "error-userWarned",
   INTEGRATION_DISABLED: "error-integrationDisabled",

src/core/server/locales/en-US/errors.ftl

@@ -16,6 +16,7 @@ error-tokenNotFound = Specified token does not exist.
 error-emailAlreadySet = Email address has already been set.
 error-emailNotSet = Email address has not been set yet.
 error-emailDomainProtected = Email domain cannot be moderated.
+error-cannotBanAccountWithModPrivileges = Cannot ban accounts with moderator privileges
 error-duplicateUser =
   Specified user already exists with a different login method.
 error-duplicateEmail = Specified email address is already in use.

src/core/server/models/user/user.ts

@@ -16,7 +16,6 @@ import {
   PasswordResetTokenExpired,
   SSOProfileNotSetError,
   TokenNotFoundError,
-  UserAlreadyBannedError,
   UserAlreadyPremoderated,
   UserAlreadySuspendedError,
   UsernameAlreadySetError,
@@ -1992,12 +1991,6 @@ export async function siteBanUser(
       throw new UserNotFoundError(id);
     }
 
-    // Check to see if the user is already banned.
-    const ban = consolidateUserBanStatus(user.status.ban);
-    if (ban.active) {
-      throw new UserAlreadyBannedError();
-    }
-
     throw new Error("an unexpected error occurred");
   }
 
@@ -2037,9 +2030,6 @@ export async function banUser(
     {
       id,
       tenantID,
-      "status.ban.active": {
-        $ne: true,
-      },
     },
     {
       $set: {
@@ -2063,12 +2053,6 @@ export async function banUser(
       throw new UserNotFoundError(id);
     }
 
-    // Check to see if the user is already banned.
-    const ban = consolidateUserBanStatus(user.status.ban);
-    if (ban.active) {
-      throw new UserAlreadyBannedError();
-    }
-
     throw new Error("an unexpected error occurred");
   }
 

src/core/server/services/users/users.ts

@@ -18,7 +18,6 @@ import { Config } from "coral-server/config";
 import { DataCache } from "coral-server/data/cache/dataCache";
 import { MongoContext } from "coral-server/data/context";
 import {
-  CannotBanAccountWithModPrivilegesError,
   DuplicateEmailError,
   DuplicateUserError,
   EmailAlreadySetError,
@@ -29,7 +28,6 @@ import {
   ModeratorCannotBeBannedOnSiteError,
   PasswordIncorrect,
   TokenNotFoundError,
-  // UserAlreadyBannedError,
   UserAlreadyPremoderated,
   UserAlreadySuspendedError,
   UserBioTooLongError,
@@ -1405,28 +1403,60 @@ export async function ban(
     banner.moderationScopes.siteIDs &&
     banner.moderationScopes.siteIDs.length !== 0;
 
+  // used to determine whether to send another ban email notification or not
+  let alreadyBanned = false;
+
+  // Check to see if the User is currently banned.
+  const banStatus = consolidateUserBanStatus(targetUser.status.ban);
+  if (banStatus.active) {
+    alreadyBanned = true;
+  }
+
+  // check if user is already banned on all of the siteIDs
+  if (
+    targetUser.status.ban.siteIDs &&
+    targetUser.status.ban.siteIDs.length > 0
+  ) {
+    const siteIDsAlreadyBanned = targetUser.status.ban.siteIDs?.filter(
+      (siteID) => {
+        return siteIDs?.includes(siteID);
+      }
+    );
+    if (siteIDsAlreadyBanned.length === siteIDs?.length) {
+      alreadyBanned = true;
+    }
+  }
+
   if (bannerIsSiteMod) {
     // ensure they've provided at least one site ID
     if (!siteIDs || siteIDs.length === 0) {
       throw new Error(
         "site moderators must provide at least one site ID to ban"
       );
     }
+    // make sure org moderators aren't site banned
+    if (
+      targetUser.role === GQLUSER_ROLE.MODERATOR &&
+      !targetUser.moderationScopes?.scoped
+    ) {
+      throw new ModeratorCannotBeBannedOnSiteError();
+    }
+
+    // a site moderator cannot ban another site moderator with privileges for one of
+    // the site ids they are trying to ban against.
     if (
       targetUser.role === GQLUSER_ROLE.MODERATOR &&
       targetUser.moderationScopes &&
       targetUser.moderationScopes.siteIDs &&
       targetUser.moderationScopes.siteIDs.length !== 0
     ) {
-      // a site moderator cannot ban another site moderator with privilegs for one of
-      // the site ids they are trying to ban against.
       const siteIDInModScopes = targetUser.moderationScopes.siteIDs.some(
         (site) => {
           return siteIDs.includes(site);
         }
       );
       if (siteIDInModScopes) {
-        throw new CannotBanAccountWithModPrivilegesError();
+        throw new ModeratorCannotBeBannedOnSiteError();
       }
     }
   }
@@ -1459,20 +1489,10 @@ export async function ban(
   }
   // Otherwise, perform a regular ban
   else {
-    // Check to see if the User is currently banned.
-    const banStatus = consolidateUserBanStatus(targetUser.status.ban);
-    if (banStatus.active) {
-      if (rejectExistingComments) {
-        await rejector.add({
-          tenantID: tenant.id,
-          authorID: userID,
-          moderatorID: banner.id,
-        });
-      }
-      return targetUser;
-      // throw new UserAlreadyBannedError();
+    // moderators can't be generally banned
+    if (targetUser.role === GQLUSER_ROLE.MODERATOR) {
+      throw new ModeratorCannotBeBannedOnSiteError();
     }
-
     // Ban the user.
     user = await banUser(mongo, tenant.id, userID, banner.id, message, now);
 
@@ -1521,27 +1541,29 @@ export async function ban(
     }
   }
 
-  // If the user has an email address associated with their account, send them
-  // a ban notification email.
-  if (user?.email) {
-    // Send the ban user email.
-    await mailer.add({
-      tenantID: tenant.id,
-      message: {
-        to: user.email,
-      },
-      template: {
-        name: "account-notification/ban",
-        context: {
-          // TODO: (wyattjoh) possibly reevaluate the use of a required username.
-          username: user.username!,
-          organizationName: tenant.organization.name,
-          organizationURL: tenant.organization.url,
-          organizationContactEmail: tenant.organization.contactEmail,
-          customMessage: (message || "").replace(/\n/g, "<br />"),
+  if (!alreadyBanned) {
+    // If the user has an email address associated with their account, send them
+    // a ban notification email.
+    if (user?.email) {
+      // Send the ban user email.
+      await mailer.add({
+        tenantID: tenant.id,
+        message: {
+          to: user.email,
         },
-      },
-    });
+        template: {
+          name: "account-notification/ban",
+          context: {
+            // TODO: (wyattjoh) possibly reevaluate the use of a required username.
+            username: user.username!,
+            organizationName: tenant.organization.name,
+            organizationURL: tenant.organization.url,
+            organizationContactEmail: tenant.organization.contactEmail,
+            customMessage: (message || "").replace(/\n/g, "<br />"),
+          },
+        },
+      });
+    }
   }
 
   return user;