Delete user functionality for system-admin (PM-3157) #1362
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| const handleConfirm = useCallback(() => { | ||
| if (!ticketUrl.trim()) { | ||
| setError('Delete ticket URL is required') |
There was a problem hiding this comment.
[correctness]
Consider using a more specific validation for the ticketUrl to ensure it is a valid URL format. This will prevent potential issues with invalid URLs being processed.
| ) | ||
|
|
||
| const description | ||
| = `Are you sure you want to DELETE user ${props.userInfo.handle} with email address ${props.userInfo.email}. ` |
There was a problem hiding this comment.
[💡 readability]
The description string concatenation could be simplified using template literals for better readability and maintainability.
| primary | ||
| variant='danger' | ||
| label='Delete' | ||
| onClick={function onClick() { |
There was a problem hiding this comment.
[💡 style]
The 'Delete' button is marked as 'primary' and 'danger'. Consider using only 'danger' to avoid conflicting visual cues, as 'primary' typically implies a positive action.
| setShowDialogDeleteUser(undefined) | ||
| }} | ||
| userInfo={showDialogDeleteUser} | ||
| isLoading={props.deletingUsers?.[showDialogDeleteUser.id]} |
There was a problem hiding this comment.
[correctness]
Ensure that the 'isLoading' prop is correctly handled in the 'DialogDeleteUser' component to prevent potential UI inconsistencies during the delete operation.
|
|
||
| const doDeleteUser = useCallback( | ||
| (userInfo: UserInfo, ticketUrl: string, onSuccess?: () => void) => { | ||
| if (!ticketUrl) { |
There was a problem hiding this comment.
[❗❗ correctness]
The doDeleteUser function checks if ticketUrl is falsy and shows an error toast, but it does not prevent the function from proceeding further. Consider adding a return statement after the toast to prevent further execution when ticketUrl is missing.
| ...previousState.deletingUsers, | ||
| [userId]: false, | ||
| }, | ||
| total: Math.max(previousState.total - 1, 0), |
There was a problem hiding this comment.
[correctness]
In the DELETE_USER_DONE case, the total is decremented by 1. Ensure that this logic aligns with the backend's handling of user deletion to avoid inconsistencies in the user count.
| export const deleteUser = async ( | ||
| handle: string, | ||
| ticketUrl: string, | ||
| ): Promise<void> => xhrRequestAsync<{ ticketUrl: string }, void>({ |
There was a problem hiding this comment.
[correctness]
Consider adding error handling for the xhrRequestAsync call to manage potential failures or network issues. This will improve the robustness of the deleteUser function.
| ): Promise<void> => xhrRequestAsync<{ ticketUrl: string }, void>({ | ||
| data: { ticketUrl }, | ||
| method: 'DELETE', | ||
| url: `${EnvironmentConfig.API.V6}/members/${encodeURIComponent(handle)}`, |
There was a problem hiding this comment.
[security]
Ensure that handle is properly validated before using it in the URL. Although encodeURIComponent is used, additional validation might be necessary to prevent any unexpected behavior or security issues.
1 participant
https://topcoder.atlassian.net/browse/PM-3157