From 63ac2c3cd6b9fb6db9ff36edc73c4d4a0d55cd90 Mon Sep 17 00:00:00 2001 From: jamesgeorge007 Date: Wed, 27 Mar 2024 17:42:20 +0530 Subject: [PATCH] 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 --- .../src/helpers/userManagement.ts | 147 ++++++++++-------- .../src/pages/users/_id.vue | 5 +- .../src/pages/users/index.vue | 19 +-- 3 files changed, 86 insertions(+), 85 deletions(-) diff --git a/packages/hoppscotch-sh-admin/src/helpers/userManagement.ts b/packages/hoppscotch-sh-admin/src/helpers/userManagement.ts index 662c32cd44a..9dd6bf2f19e 100644 --- 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; - 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 index 1375ff6cdb4..6151b766f3e 100644 --- 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 index 1db77d937b4..c18e48df2ad 100644 --- 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([]); // Ensure this variable is declared outside the debounce function let debounceTimeout: ReturnType | null = null; -let toastTimeout: ReturnType | 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) );