-
Notifications
You must be signed in to change notification settings - Fork 517
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
User creation form bug fix #9404
User creation form bug fix #9404
Conversation
WalkthroughThe changes in this pull request modify the rendering logic of the local body selection field in the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
src/components/Users/UserAddEditForm.tsx (4)
Line range hint
1182-1203
: Consider clearing local body value when hiddenWhile the conditional rendering of the local body field is correct, consider clearing the
local_body
value in the form state whenshowLocalbody
becomes false to prevent any stale values from being submitted.{showLocalbody && includedFields?.includes("local_body") && ( <> {isLocalbodyLoading ? ( <CircularProgress /> ) : ( <SelectFormField {...field("local_body")} label={t("local_body")} required position="above" placeholder={t("choose_localbody")} options={localBodies} optionLabel={(o) => o.name} optionValue={(o) => o.id} onChange={(e) => { handleFieldChange(e, field); + if (!showLocalbody) { + dispatch({ + type: "set_form", + form: { + ...state.form, + local_body: 0, + }, + }); + } }} aria-label={t("local_body")} /> )} </> )}
Line range hint
516-529
: Enhance phone number validationThe current phone number validation could be more robust. Consider adding additional checks for:
- Country code validation
- Minimum and maximum length validation
- Format validation for different phone number types (mobile vs landline)
const validatePhoneNumber = (phoneNumber: string) => { const parsedPhoneNumber = parsePhoneNumber(phoneNumber); if (!parsedPhoneNumber) return false; - return PhoneNumberValidator()(parsedPhoneNumber) === undefined; + const baseValidation = PhoneNumberValidator()(parsedPhoneNumber) === undefined; + if (!baseValidation) return false; + + // Additional validation rules + const phoneNumberWithoutCountryCode = parsedPhoneNumber.replace(/^\+\d{1,3}/, ''); + const isValidLength = phoneNumberWithoutCountryCode.length >= 10 && phoneNumberWithoutCountryCode.length <= 12; + const hasValidFormat = /^\+\d{1,3}\d{10,12}$/.test(parsedPhoneNumber); + + return isValidLength && hasValidFormat; };
Line range hint
1-1203
: Consider breaking down the component for better maintainabilityThe component is quite large and handles multiple responsibilities. Consider:
- Breaking down form sections into separate components
- Moving validation logic to a separate utility file
- Creating a custom hook for form state management
This would improve maintainability, testability, and reusability.
Example structure:
// components/Users/Form/Sections/PersonalInfo.tsx // components/Users/Form/Sections/ContactInfo.tsx // components/Users/Form/Sections/LocationInfo.tsx // components/Users/Form/Sections/Credentials.tsx // hooks/useUserForm.ts // utils/userValidation.ts
Line range hint
533-614
: Improve form error handling UXWhile the validation is thorough, consider enhancing the user experience by:
- Adding inline validation feedback as users type
- Showing a summary of all errors at the top of the form
- Automatically scrolling to the first error field
const validateForm = (formData: UserForm) => { const errors: Partial<Record<keyof UserForm, FieldError>> = {}; const fieldsToValidate = includedFields || Object.keys(formData); + const errorSummary: string[] = []; const facilityError = fieldsToValidate.includes("facilities") ? validateFacility(formData, selectedFacility) : null; if (facilityError) { errors.facilities = facilityError; + errorSummary.push(facilityError); } + + // Scroll to first error + if (Object.keys(errors).length > 0) { + const firstErrorField = document.querySelector(`[name="${Object.keys(errors)[0]}"]`); + firstErrorField?.scrollIntoView({ behavior: 'smooth', block: 'center' }); + } + + return { errors, errorSummary }; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Users/UserAddEditForm.tsx
(1 hunks)src/components/Users/UserFormValidations.tsx
(0 hunks)
💤 Files with no reviewable changes (1)
- src/components/Users/UserFormValidations.tsx
LGTM |
LGTM |
@Jacobjeevan 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
weekly_working_hours
andvideo_connect_link
as required fields for new user creation.Documentation