-
Notifications
You must be signed in to change notification settings - Fork 559
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
Fix Translations Not Loading for Global Zod Validators Error Messages and Improve Patient Registration Error Messages #10581
Fix Translations Not Loading for Global Zod Validators Error Messages and Improve Patient Registration Error Messages #10581
Conversation
WalkthroughThis pull request modifies the localization file by removing a specific validation message related to phone number requirements. Additionally, it transitions the validation logic across several components from static object exports to dynamic function calls. The phone number validation now includes a minimum length check and updates the error messages accordingly. Various form components have been updated to utilize the new validation schema function, enhancing the flexibility of the validation logic. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (2)
✨ 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. |
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
🧹 Nitpick comments (1)
src/components/Patient/PatientRegistration.tsx (1)
557-557
: Verify the type casting approach.The type casting
(null as unknown as number)
is a workaround to handle empty values. Consider using a more type-safe approach:- : (null as unknown as number), // intentionally setting to undefined, when the value is empty to avoid 0 in the input field + : undefined, // intentionally setting to undefined, when the value is empty to avoid 0 in the input field
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
public/locale/en.json
(1 hunks)src/Utils/validators.ts
(1 hunks)src/components/Facility/FacilityForm.tsx
(2 hunks)src/components/Patient/PatientRegistration.tsx
(3 hunks)src/components/Resource/ResourceCreate.tsx
(2 hunks)src/components/Users/UserForm.tsx
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (8)
src/Utils/validators.ts (2)
5-5
: Improved i18n initialization by making validators dynamic.The change from a static object to a function ensures that translations are properly initialized when the validation schema is created.
14-19
: Enhanced phone number validation with required field check.The validation now properly enforces both format and presence requirements:
.min(1)
ensures the field is not empty.refine()
validates the phone number formatsrc/components/Resource/ResourceCreate.tsx (1)
49-64
: LGTM! Validation schema properly initialized.The component correctly adapts to the new validation schema initialization pattern.
src/components/Facility/FacilityForm.tsx (1)
41-63
: LGTM! Validation schema properly initialized.The component correctly adapts to the new validation schema initialization pattern and maintains both phone number and coordinates validation.
src/components/Users/UserForm.tsx (1)
41-63
: LGTM! Validation schema properly initialized.The component correctly adapts to the new validation schema initialization pattern.
src/components/Patient/PatientRegistration.tsx (2)
59-59
: LGTM! Import change aligns with translation fix.The change from static
validators
import to dynamicgetValidationSchema
function should help resolve the translation loading issues.
569-577
: LGTM! Improved age validation feedback.Good addition of validation feedback for invalid age and clear display of calculated year of birth.
public/locale/en.json (1)
1618-1618
: LGTM! Clear and consistent error message.The added translation for phone number validation is clear and follows the existing message style in the file.
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: 1
🧹 Nitpick comments (1)
src/components/Patient/PatientRegistration.tsx (1)
560-560
: Consider using undefined instead of null for age reset.Using
null
asunknown as number
is a type assertion that could be avoided. Since the age field is optional in the schema,undefined
would be more appropriate.- : (null as unknown as number), + : undefined,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Patient/PatientRegistration.tsx
(5 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/Patient/PatientRegistration.tsx (2)
59-59
: LGTM! Dynamic validation schema initialization.The change from static import to dynamic initialization using
getValidationSchema()
aligns with the PR objective of fixing translation loading issues.Also applies to: 85-86
569-578
: LGTM! Enhanced age validation feedback.The improved display logic for year of birth calculation with proper error handling for invalid age values provides better user feedback.
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
🧹 Nitpick comments (2)
src/components/Patient/PatientRegistration.tsx (2)
560-560
: Consider improving type safety for age handling.The current type casting
(null as unknown as number)
is a workaround that could be improved for better type safety.Consider updating the form schema to properly handle nullable numbers:
-age: z +age: z.union([ + z .number() .int() .positive() .min(1, t("age_must_be_positive")) .max(120, t("age_must_be_below_120")) - .optional(), + z.null() +]).optional(),
569-578
: Consider enhancing accessibility for error states.While the color-coded messages provide good visual feedback, they could be more accessible.
Consider adding ARIA attributes for better screen reader support:
- <span className="text-red-600">Invalid age</span> + <span className="text-red-600" role="alert" aria-label={t("invalid_age_error")}>Invalid age</span> - <span className="text-violet-600"> + <span className="text-violet-600" role="status" aria-label={t("year_of_birth_calculation")}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json
(2 hunks)src/components/Patient/PatientRegistration.tsx
(5 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 (5)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (2)
src/components/Patient/PatientRegistration.tsx (2)
59-59
: LGTM! Good improvement in validation initialization.The change from static import to dynamic validation schema generation improves the handling of internationalized validation messages.
Also applies to: 85-85
125-128
: LGTM! Well-structured validation for geo_organization.The validation properly checks for UUID format and uses internationalized error messages.
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.
overlooked this thing earlier, other than that LGTM
@rajku-dev 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
Before
After
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor