chore: address CR comments

- Implicitly infer the action type (bulk/individual) from the supplied deleted users list. - Display toast messages one after the other by relying on the native toast APIs refraining from the need to maintain timeouts separately. - Ensure the toast message about user deletion success/failure with the count is displayed only when above `0`. Co-authored-by: amk-dev <akash.k.mohan98@gmail.com>
hoppscotch · Mar 27, 2024 · 63ac2c3 · 63ac2c3
1 parent 8a8cdcf
commit 63ac2c3
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 85 deletions.
diff --git a/packages/hoppscotch-sh-admin/src/helpers/userManagement.ts b/packages/hoppscotch-sh-admin/src/helpers/userManagement.ts
@@ -3,106 +3,117 @@ import { getI18n } from '~/modules/i18n';
 import { UserDeletionResult } from './backend/graphql';
 import { ADMIN_CANNOT_BE_DELETED, USER_IS_OWNER } from './errors';
 
-type IndividualActionInput = {
-  type: 'individual';
-  metadata: null;
-};
-type BulkActionInput = {
-  type: 'bulk';
-  metadata: {
-    areMultipleUsersSelected: boolean;
-    deletedIDs: string[];
-  };
+type IndividualActionMetadata = null;
+type BulkActionMetadata = {
+  areMultipleUsersSelected: boolean;
+  deletedIDs: string[];
 };
+type ActionMetadata = IndividualActionMetadata | BulkActionMetadata;
 
-type IndividualActionResult = {
-  data: null;
-};
-type BulkActionResult = {
-  data: { timeoutID: NodeJS.Timeout | null };
+type HandleUserDeletion = {
+  (deletedUsersList: UserDeletionResult[], metadata: ActionMetadata): void;
 };
 
-type HandleUserDeletion = {
-  (
-    deletedUsersList: UserDeletionResult[],
-    action: IndividualActionInput | BulkActionInput
-  ): IndividualActionResult | BulkActionResult;
+type ToastMessage = {
+  message: string;
+  state: 'success' | 'error';
 };
 
 const t = getI18n();
 const toast = useToast();
 
+const displayToastMessages = (
+  toastMessages: ToastMessage[],
+  currentIndex: number,
+  isBulkAction: boolean,
+  metadata: ActionMetadata
+) => {
+  const { message, state } = toastMessages[currentIndex];
+
+  toast[state](message, {
+    duration: 2000,
+    onComplete: () => {
+      if (currentIndex < toastMessages.length - 1) {
+        displayToastMessages(
+          toastMessages,
+          currentIndex + 1,
+          isBulkAction,
+          metadata
+        );
+      }
+    },
+  });
+};
+
 export const handleUserDeletion: HandleUserDeletion = (
   deletedUsersList,
-  action
+  metadata
 ) => {
-  let timeoutID: NodeJS.Timeout | null = null;
-
   const uniqueErrorMessages = new Set(
     deletedUsersList.map(({ errorMessage }) => errorMessage).filter(Boolean)
   ) as Set<string>;
 
-  const { type, metadata } = action;
+  const isBulkAction = deletedUsersList.length > 1;
 
   // Show the success toast based on the action type if there are no errors
   if (uniqueErrorMessages.size === 0) {
-    if (type === 'bulk') {
+    if (isBulkAction) {
       toast.success(
-        metadata.areMultipleUsersSelected
+        (metadata as BulkActionMetadata).areMultipleUsersSelected
           ? t('state.delete_user_success')
           : t('state.delete_users_success')
       );
 
-      return { type, data: { timeoutID } };
+      return;
     }
 
     toast.success(t('state.delete_user_success'));
-    return { type, data: null };
+    return;
   }
 
   const errMsgMap = {
-    [ADMIN_CANNOT_BE_DELETED]:
-      type === 'bulk'
-        ? t('state.remove_admin_for_deletion')
-        : t('state.remove_admin_to_delete_user'),
-
-    [USER_IS_OWNER]:
-      type === 'bulk'
-        ? t('state.remove_owner_for_deletion')
-        : t('state.remove_owner_to_delete_user'),
+    [ADMIN_CANNOT_BE_DELETED]: isBulkAction
+      ? t('state.remove_admin_for_deletion')
+      : t('state.remove_admin_to_delete_user'),
+
+    [USER_IS_OWNER]: isBulkAction
+      ? t('state.remove_owner_for_deletion')
+      : t('state.remove_owner_to_delete_user'),
   };
   const errMsgMapKeys = Object.keys(errMsgMap);
 
-  if (type === 'bulk') {
-    const { areMultipleUsersSelected, deletedIDs } = metadata;
+  const toastMessages: ToastMessage[] = [];
 
-    // Show toast messages with the count of users deleted only if multiple users are selected
-    if (areMultipleUsersSelected) {
-      toast.success(
-        t('state.delete_some_users_success', { count: deletedIDs.length })
-      );
-      toast.error(
-        t('state.delete_some_users_failure', {
-          count: deletedUsersList.length - deletedIDs.length,
-        })
-      );
+  if (isBulkAction) {
+    const { areMultipleUsersSelected, deletedIDs } =
+      metadata as BulkActionMetadata;
+
+    if (areMultipleUsersSelected && deletedIDs.length > 0) {
+      toastMessages.push({
+        message: t('state.delete_some_users_success', {
+          count: deletedIDs.length,
+        }),
+        state: 'success',
+      });
+    }
+
+    const remainingDeletionsCount = deletedUsersList.length - deletedIDs.length;
+    if (remainingDeletionsCount > 0) {
+      toastMessages.push({
+        message: t('state.delete_some_users_failure', {
+          count: remainingDeletionsCount,
+        }),
+        state: 'error',
+      });
     }
   }
 
   uniqueErrorMessages.forEach((errorMessage) => {
     if (errMsgMapKeys.includes(errorMessage)) {
-      if (type === 'bulk') {
-        timeoutID = setTimeout(
-          () => {
-            toast.error(errMsgMap[errorMessage as keyof typeof errMsgMap]);
-          },
-          metadata.areMultipleUsersSelected ? 2000 : 0
-        );
-
-        return;
-      }
-
-      toast.error(errMsgMap[errorMessage as keyof typeof errMsgMap]);
+      toastMessages.push({
+        message: errMsgMap[errorMessage as keyof typeof errMsgMap],
+        state: 'error',
+      });
     }
   });
 
@@ -112,10 +123,16 @@ export const handleUserDeletion: HandleUserDeletion = (
       (key) => !((key as string) in errMsgMap)
     )
   ) {
-    type === 'bulk' && metadata.areMultipleUsersSelected
-      ? t('state.delete_users_failure')
-      : t('state.delete_user_failure');
+    const fallbackErrMsg =
+      isBulkAction && (metadata as BulkActionMetadata).areMultipleUsersSelected
+        ? t('state.delete_users_failure')
+        : t('state.delete_user_failure');
+
+    toastMessages.push({
+      message: fallbackErrMsg,
+      state: 'error',
+    });
   }
 
-  return { data: type === 'bulk' ? { timeoutID } : null };
+  displayToastMessages(toastMessages, 0, isBulkAction, metadata);
 };
diff --git a/packages/hoppscotch-sh-admin/src/pages/users/_id.vue b/packages/hoppscotch-sh-admin/src/pages/users/_id.vue
@@ -210,10 +210,7 @@ const deleteUserMutation = async (id: string | null) => {
   } else {
     const deletedUsers = result.data?.removeUsersByAdmin || [];
 
-    handleUserDeletion(deletedUsers, {
-      type: 'individual',
-      metadata: null,
-    });
+    handleUserDeletion(deletedUsers, null);
   }
   confirmDeletion.value = false;
   deleteUserUID.value = null;

diff --git a/packages/hoppscotch-sh-admin/src/pages/users/index.vue b/packages/hoppscotch-sh-admin/src/pages/users/index.vue
@@ -309,16 +309,10 @@ const selectedRows = ref<UsersListQuery['infra']['allUsers']>([]);
 // Ensure this variable is declared outside the debounce function
 let debounceTimeout: ReturnType<typeof setTimeout> | null = null;
 
-let toastTimeout: ReturnType<typeof setTimeout> | null = null;
-
 onUnmounted(() => {
   if (debounceTimeout) {
     clearTimeout(debounceTimeout);
   }
-
-  if (toastTimeout) {
-    clearTimeout(toastTimeout);
-  }
 });
 
 // Debounce Function
@@ -590,18 +584,11 @@ const deleteUsers = async (id: string | null) => {
       .filter((user) => user.isDeleted)
       .map((user) => user.userUID);
 
-    const { data } = handleUserDeletion(deletedUsers, {
-      type: 'bulk',
-      metadata: {
-        areMultipleUsersSelected: areMultipleUsersSelected.value,
-        deletedIDs,
-      },
+    handleUserDeletion(deletedUsers, {
+      areMultipleUsersSelected: areMultipleUsersSelected.value,
+      deletedIDs,
     });
 
-    if (data?.timeoutID) {
-      toastTimeout = data.timeoutID;
-    }
-
     usersList.value = usersList.value.filter(
       (user) => !deletedIDs.includes(user.uid)
     );