Username validation added in Create user form #9835
Conversation
WalkthroughThis pull request introduces enhancements to the user creation process by adding username availability validation and expanding localization support. The changes focus on improving the user experience during account creation by implementing real-time username availability checks and adding more specific localization strings for user roles and contact information. The modifications span across the localization JSON file, user creation form component, and user API type definitions. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/Users/CreateUserForm.tsx (3)
131-158: Consider adding error handling for network failures.The username availability check should handle network errors explicitly rather than showing a generic error message.
const checkUsername = async (username: string) => { setUsernameExists(userExistsEnums.checking); - const { res: usernameCheck } = await request(UserApi.checkUsername, { - pathParams: { username }, - silent: true, - }); - if (usernameCheck === undefined || usernameCheck.status === 409) { - setUsernameExists(userExistsEnums.exists); - } else if (usernameCheck.status === 200) { - setUsernameExists(userExistsEnums.available); - } else { + try { + const { res: usernameCheck } = await request(UserApi.checkUsername, { + pathParams: { username }, + silent: true, + }); + if (usernameCheck === undefined || usernameCheck.status === 409) { + setUsernameExists(userExistsEnums.exists); + } else if (usernameCheck.status === 200) { + setUsernameExists(userExistsEnums.available); + } else { + throw new Error('Unexpected response status'); + } + } catch (error) { Notification.Error({ - msg: "Some error occurred while checking username availability. Please try again later.", + msg: error instanceof Error ? error.message : "Network error while checking username availability. Please try again later.", }); - } + setUsernameExists(userExistsEnums.idle); + } };
188-196: Consider debouncing the username validation.The 500ms delay might not be sufficient for fast typists. Consider increasing it to reduce unnecessary API calls.
useEffect(() => { setUsernameExists(userExistsEnums.idle); const timeout = setTimeout(() => { if (validateUsername(usernameInput)) { checkUsername(usernameInput); } - }, 500); + }, 800); return () => clearTimeout(timeout); }, [usernameInput]);
314-321: Consider adding aria-label for accessibility.The username input field should have an aria-label for better screen reader support.
<Input placeholder={t("username")} + aria-label={t("username")} {...field} value={usernameInput} onChange={(e) => setUsernameInput(e.target.value)} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
public/locale/en.json(10 hunks)src/components/Users/CreateUserForm.tsx(18 hunks)src/types/user/userApi.ts(1 hunks)
🔇 Additional comments (3)
src/types/user/userApi.ts (1)
24-28: LGTM! The username validation endpoint is well-defined.The endpoint follows RESTful conventions and uses appropriate HTTP status codes for availability checks.
src/components/Users/CreateUserForm.tsx (1)
99-126: Well-structured validation rule component.The
validateRulefunction provides consistent visual feedback for validation rules with proper icon states.public/locale/en.json (1)
32-32: LGTM! Translation keys are well-organized.The new keys follow the existing naming conventions and provide comprehensive coverage for the user creation flow.
Also applies to: 75-78, 371-371, 655-655, 736-736, 781-781, 1310-1310, 1712-1712, 1918-1918, 1930-1930
Rishith25
changed the title
|
PR's that are using the old deprecated methods, will be closed without review. |
|
@Jacobjeevan @rithviknishad I have done the changes and I have remove deprecated methods |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/components/Users/CreateUserForm.tsx (5)
103-130: Consider memoizing the validateRule function to optimize performance.The
validateRulefunction is recreated on every render. Since it's used multiple times in the component, it should be memoized usinguseCallback.-export const validateRule = ( +export const validateRule = useCallback(( condition: boolean, content: JSX.Element | string, isInitialState: boolean = false, -) => { +) => { return ( <div> {isInitialState ? ( <CareIcon icon="l-circle" className="text-xl text-gray-500" /> ) : condition ? ( <CareIcon icon="l-check-circle" className="text-xl text-primary-500" /> ) : ( <CareIcon icon="l-times-circle" className="text-xl text-red-500" /> )}{" "} <span className={classNames( isInitialState ? "text-black" : condition ? "text-primary-500" : "text-red-500", )} > {content} </span> </div> ); -}; +}, []);
222-226: Debounce the username input handler to prevent excessive API calls.The
handleInputChangefunction triggers on every keystroke. To optimize performance and reduce API calls, consider debouncing the input handler.+const debouncedSetUsername = useCallback( + debounce((value: string) => { + setUsernameInput(value); + setUsernameExists(userExistsEnums.idle); + }, 300), + [] +); -const handleInputChange = (e: React.ChangeEvent<HTMLInputElement>) => { - const value = e.target.value; - setUsernameInput(value); - setUsernameExists(userExistsEnums.idle); -}; +const handleInputChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => { + debouncedSetUsername(e.target.value); +}, [debouncedSetUsername]);
Line range hint
43-93: Consider making phone number validation more flexible.The current phone number validation is strictly tied to the Indian format (+91). Consider making it more flexible to support international phone numbers.
-phone_number: z - .string() - .regex( - /^\+91[0-9]{10}$/, - "Phone number must start with +91 followed by 10 digits", - ), +phone_number: z + .string() + .regex( + /^\+[1-9]\d{1,14}$/, + "Phone number must be in international format (e.g., +91XXXXXXXXXX)", + ),Also, consider extracting the password validation schema to a constant for reusability:
const passwordSchema = z .string() .min(8, "Password must be at least 8 characters") .regex(/[a-z]/, "Password must contain at least one lowercase letter") .regex(/[A-Z]/, "Password must contain at least one uppercase letter") .regex(/[0-9]/, "Password must contain at least one number"); const userFormSchema = z .object({ // ...other fields password: passwordSchema, c_password: z.string(), }) .refine((data) => data.password === data.c_password, { message: "Passwords don't match", path: ["c_password"], });
343-350: Enhance accessibility for the username input field.Add aria-labels and aria-describedby attributes to improve accessibility.
<div className="relative"> <Input placeholder={t("username")} + aria-label={t("username")} + aria-describedby="username-feedback" {...field} value={usernameInput} onChange={handleInputChange} /> </div> +<div id="username-feedback" className="sr-only"> + {usernameExists === userExistsEnums.exists && t("username_not_available")} + {usernameExists === userExistsEnums.available && t("username_available")} +</div>
164-181: Improve error handling with more specific error messages.The current error handling uses a generic message for all non-409 errors. Consider providing more specific error messages based on the error type.
-if (error instanceof Error && "status" in error) { - const status = (error as any).status; - - if (status === 409) { - setUsernameExists(userExistsEnums.exists); - } else if (status === 404) { - toast.error( - "Some error occurred while checking username availability. Please try again later.", - ); - setUsernameExists(userExistsEnums.idle); - } -} else { - toast.error( - "Some error occurred while checking username availability. Please try again later.", - ); - setUsernameExists(userExistsEnums.idle); -} +if (error instanceof Error && "status" in error) { + const status = (error as any).status; + switch (status) { + case 409: + setUsernameExists(userExistsEnums.exists); + break; + case 404: + toast.error(t("username_check_service_unavailable")); + setUsernameExists(userExistsEnums.idle); + break; + case 500: + toast.error(t("username_check_server_error")); + setUsernameExists(userExistsEnums.idle); + break; + default: + toast.error(t("username_check_unknown_error")); + setUsernameExists(userExistsEnums.idle); + } +} else { + toast.error(t("username_check_network_error")); + setUsernameExists(userExistsEnums.idle); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json(8 hunks)src/components/Users/CreateUserForm.tsx(17 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (2)
src/components/Users/CreateUserForm.tsx (1)
147-154: LGTM! Well-implemented API integration.The use of react-query for API integration is well done, with proper handling of loading states and error scenarios.
public/locale/en.json (1)
367-367: LGTM! Well-structured user role translations.The translations for user roles (doctor, nurse, staff, volunteer) and contact information are properly defined.
Also applies to: 733-733, 1310-1310, 1713-1713, 1919-1919
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/Users/CreateUserForm.tsx (3)
139-162: Consider optimizing the feedback rendering logic.The feedback UI implementation can be simplified by using optional chaining and removing unnecessary Fragment wrappers.
- if ( - form.formState.errors.username && - form.formState.errors.username?.message - ) { + if (form.formState.errors.username?.message) { return validateRule(false, form.formState.errors.username.message); } ... - return validateRule(false, <>{t("username_not_available")}</>); + return validateRule(false, t("username_not_available")); ... - return validateRule(true, <>{t("username_available")}</>); + return validateRule(true, t("username_available"));🧰 Tools
🪛 Biome (1.9.4)
[error] 141-142: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 158-158: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
[error] 160-160: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
273-279: Ensure consistent error message styling across form fields.The password field's error message styling should match other fields for consistency.
<FormControl> <Input type="password" placeholder={t("password")} {...field} + className="error-input" />
Line range hint
164-189: Enhance error handling in form submission.Consider adding specific error messages for different failure scenarios and handle network errors explicitly.
} catch (error) { + const errorMessage = error instanceof Error ? error.message : t("user_add_error"); Notification.Error({ - msg: t("user_add_error"), + msg: errorMessage, }); }🧰 Tools
🪛 Biome (1.9.4)
[error] 141-142: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 158-158: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
[error] 160-160: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json(9 hunks)src/components/Users/CreateUserForm.tsx(18 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Users/CreateUserForm.tsx
[error] 141-142: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 158-158: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
[error] 160-160: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (3)
src/components/Users/CreateUserForm.tsx (2)
Line range hint
44-57: LGTM! Comprehensive username validation schema.The validation rules are well-defined and cover important aspects:
- Length restrictions (4-16 characters)
- Allowed characters (lowercase, numbers, ._-)
- Start/end character requirements
- Consecutive special character prevention
130-137: LGTM! Clean implementation of username availability check.The useQuery implementation is efficient with:
- Proper query key for caching
- Silent error handling
- Minimum length check before triggering API call
public/locale/en.json (1)
1932-1932:⚠️ Potential issueFix typo in translation key.
The key "whataapp_number_same_as_phone_number" has a typo in "whataapp".
- "whataapp_number_same_as_phone_number": "WhatsApp number is same as phone number", + "whatsapp_number_same_as_phone_number": "WhatsApp number is same as phone number",Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/Users/CreateUserForm.tsx (2)
117-128: Consider debouncing the username validation.The current implementation triggers validation on every character input, which could lead to unnecessary API calls. Consider adding debounce to optimize performance.
const usernameInput = form.watch("username"); const phoneNumber = form.watch("phone_number"); const isWhatsApp = form.watch("phone_number_is_whatsapp"); + const debouncedValidation = useCallback( + debounce((value: string) => { + if (value && value.length > 0) { + form.trigger("username"); + } + }, 300), + [form] + ); useEffect(() => { if (isWhatsApp) { form.setValue("alt_phone_number", phoneNumber); } - if (usernameInput && usernameInput.length > 0) { - form.trigger("username"); - } + debouncedValidation(usernameInput); }, [phoneNumber, isWhatsApp, form, usernameInput]);
139-162: Simplify the feedback rendering logic.The feedback rendering logic can be improved by:
- Removing unnecessary Fragment usage
- Simplifying the conditional logic
const renderUsernameFeedback = (usernameInput: string) => { - if ( - form.formState.errors.username && - form.formState.errors.username?.message - ) { + const { errors: { username } } = form.formState; + + if (username?.message) { - return validateRule(false, form.formState.errors.username.message); + return validateRule(false, username.message); } else if (isLoading) { return ( <div className="flex items-center gap-1"> <CareIcon icon="l-spinner" className="text-xl text-gray-500 animate-spin" /> <span className="text-gray-500 text-sm"> {t("checking_availability")} </span> </div> ); } else if (error) { - return validateRule(false, <>{t("username_not_available")}</>); + return validateRule(false, t("username_not_available")); } else if (usernameInput) { - return validateRule(true, <>{t("username_available")}</>); + return validateRule(true, t("username_available")); } };🧰 Tools
🪛 Biome (1.9.4)
[error] 141-142: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 158-158: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
[error] 160-160: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json(9 hunks)src/components/Users/CreateUserForm.tsx(18 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Users/CreateUserForm.tsx
[error] 141-142: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 158-158: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
[error] 160-160: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (3)
src/components/Users/CreateUserForm.tsx (2)
130-137: LGTM! Well-implemented username availability check.The implementation follows best practices with proper error handling, silent API calls, and conditional enabling based on form validation state.
Line range hint
203-492: LGTM! Comprehensive internationalization implementation.All user-facing text is properly internationalized using the translation function, following consistent naming conventions.
public/locale/en.json (1)
367-367: LGTM! Translation keys are well-organized and complete.All necessary translations for the user creation form have been added with clear and consistent naming.
Also applies to: 507-507, 652-652, 734-734, 779-779, 1311-1311, 1714-1714, 1920-1920, 1933-1933
@Jacobjeevan ✅Done |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/Users/UserFormValidations.tsx (1)
60-66: LGTM! Consider extracting icon classes for consistency.The styling changes look good. However, to maintain consistency across the application, consider extracting the icon color classes into constants or a theme configuration.
+ // Add to a theme configuration file + const ICON_COLORS = { + initial: "text-gray-500", + success: "text-green-500", + error: "text-red-500", + } as const; - <CareIcon icon="l-circle" className="text-gray-500" /> + <CareIcon icon="l-circle" className={ICON_COLORS.initial} /> - <CareIcon icon="l-check-circle" className="text-green-500" /> + <CareIcon icon="l-check-circle" className={ICON_COLORS.success} /> - <CareIcon icon="l-times-circle" className="text-red-500" /> + <CareIcon icon="l-times-circle" className={ICON_COLORS.error} />src/components/Users/CreateUserForm.tsx (2)
139-162: Consider error state handling improvements.The username feedback implementation looks good, but there are a few potential improvements:
- The error state could be more specific (distinguish between network errors and username taken)
- The loading state could reuse the validateRule function for consistency
} else if (isLoading) { - return ( - <div className="flex items-center gap-1"> - <CareIcon - icon="l-spinner" - className="text-sm text-gray-500 animate-spin" - /> - <span className="text-gray-500 text-sm"> - {t("checking_availability")} - </span> - </div> - ); + return validateRule( + true, + <span className="flex items-center gap-1"> + <CareIcon icon="l-spinner" className="animate-spin" /> + {t("checking_availability")} + </span>, + true + ); } else if (error) { - return validateRule(false, t("username_not_available")); + return validateRule( + false, + error.status === 409 + ? t("username_not_available") + : t("username_check_failed") + ); }
258-260: Consider adding aria-label for accessibility.The username input field should have an aria-label to improve accessibility.
<div className="relative"> - <Input placeholder={t("username")} {...field} /> + <Input + placeholder={t("username")} + aria-label={t("username")} + {...field} + /> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
public/locale/en.json(9 hunks)src/components/Users/CreateUserForm.tsx(18 hunks)src/components/Users/UserFormValidations.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- public/locale/en.json
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
🔇 Additional comments (3)
src/components/Users/CreateUserForm.tsx (3)
130-137: LGTM! Clean implementation of username availability check.The useQuery implementation for username validation is clean and efficient. Good job on:
- Using the silent flag to suppress notifications
- Enabling the query only when the username is valid
- Using proper query key for caching
203-214: LGTM! Comprehensive internationalization implementation.Good job on implementing translations for all user-facing text. The translations are consistently applied across:
- Form labels
- Input placeholders
- Button text
- Validation messages
Also applies to: 228-230, 242-244, 256-260, 273-279, 291-295, 310-312, 325-325, 339-339, 366-368, 380-380, 394-394, 424-426, 442-446, 460-463, 492-492
125-128: Verify the necessity of manual username validation trigger.The manual trigger of username validation might be redundant as react-hook-form should handle this automatically when the field changes.
✅ Verification successful
Manual username validation trigger is necessary and correct.
The trigger is required as part of a complex validation system that includes:
- Synchronous zod validation rules
- Asynchronous username availability check
- Custom UI feedback system that shows loading states and availability
The manual trigger ensures consistent validation state when related fields change, particularly for the custom username feedback UI.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar patterns in the codebase to verify if this is a common practice rg "form\.trigger\(" -A 2Length of output: 239
Script:
#!/bin/bash # Check the form setup and validation rules ast-grep --pattern 'useForm<$_>({$$$})' src/components/Users/CreateUserForm.tsx # Look for username validation rules rg "username.*validation|validation.*username" -i src/components/Users/CreateUserForm.tsx # Check the complete component structure cat src/components/Users/CreateUserForm.tsxLength of output: 16249
nihal467
added
tested
reviewed
|
LGTM |
|
@Rishith25 Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Documentation