Initial change handle functionality #1434
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| gap: 6px; | ||
| } | ||
|
|
||
| .dialogLoadingSpinnerContainer { |
There was a problem hiding this comment.
[maintainability]
Using position: absolute; for .dialogLoadingSpinnerContainer without specifying a top, right, or bottom value other than bottom: 0; can lead to layout issues if the parent container's size changes. Consider ensuring the parent container has a defined size or constraints to prevent unexpected behavior.
| props.setOpen(false) | ||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [isLoading]) |
There was a problem hiding this comment.
[correctness]
The handleClose function is missing props.setOpen in its dependency array. This could lead to stale closures if setOpen changes. Consider adding props.setOpen to the dependency array.
| const onSubmit = useCallback(() => { | ||
| setShowConfirm(true) | ||
| }, []) | ||
| const currentHandle = props.userInfo.handle |
There was a problem hiding this comment.
[correctness]
The newHandle value is derived from getValues() outside of the form submission or confirmation logic. This could lead to stale data if the form state changes before submission. Consider moving this logic inside the onSubmit or onConfirm functions.
| forceUpdateValue | ||
| onChange={_.noop} | ||
| error={_.get(errors, 'newHandle.message')} | ||
| inputControl={register('newHandle')} |
There was a problem hiding this comment.
[💡 maintainability]
The inputControl prop is not a standard prop for InputText. If this is a custom implementation, ensure it is correctly handled within the InputText component. Otherwise, consider removing or renaming it to avoid confusion.
| .then(result => { | ||
| setIsLoading(false) | ||
| toast.success('Handle updated successfully') | ||
| props.userInfo.handle = result?.handle ?? nextHandle |
There was a problem hiding this comment.
[design]
Directly mutating props.userInfo.handle could lead to unexpected side effects, especially if userInfo is used elsewhere. Consider using a state update function or a context provider to manage this state change.
| {showDialogEditUserHandle && ( | ||
| <DialogEditUserHandle | ||
| open | ||
| setOpen={function setOpen() { |
There was a problem hiding this comment.
[💡 maintainability]
Consider using a consistent naming convention for the setOpen function across all dialog components. Currently, the setOpen function is defined inline with a different style compared to other similar functions in the file. This could improve maintainability by ensuring consistency.
| * Model for edit user handle form | ||
| */ | ||
| export interface FormEditUserHandle { | ||
| newHandle: string |
There was a problem hiding this comment.
[correctness]
Consider adding validation logic to ensure that newHandle meets expected constraints (e.g., length, allowed characters) to prevent potential issues with invalid user handles.
| newHandle: string, | ||
| ): Promise<MemberInfo> => { | ||
| const payload = { | ||
| newHandle: newHandle.trim(), |
There was a problem hiding this comment.
[correctness]
Consider validating newHandle before trimming and sending it in the payload. This ensures that the handle meets any necessary criteria (e.g., length, character restrictions) before making the API call.
| } | ||
|
|
||
| return xhrPatchAsync<typeof payload, MemberInfo>( | ||
| `${EnvironmentConfig.API.V6}/members/${encodeURIComponent(handle)}/change_handle`, |
There was a problem hiding this comment.
[❗❗ security]
Ensure that handle is validated before encoding and using it in the URL. This prevents potential issues with invalid or malicious input.
| */ | ||
| export const formEditUserHandleSchema: Yup.ObjectSchema<FormEditUserHandle> | ||
| = Yup.object({ | ||
| newHandle: Yup.string() |
There was a problem hiding this comment.
[correctness]
Consider adding a .min(3, 'Handle must be at least 3 characters long.') constraint to ensure the handle has a minimum length. This can help prevent invalid or too short handles from being accepted.
topcoder.atlassian.net/browse/PS-513