-
Notifications
You must be signed in to change notification settings - Fork 522
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: Year Of Birth Field Validation Condition in Patient transfer form #9133
Fix: Year Of Birth Field Validation Condition in Patient transfer form #9133
Conversation
WalkthroughThe changes in Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
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: 1
🧹 Outside diff range and nitpick comments (2)
src/components/Facility/TransferPatientDialog.tsx (2)
133-137
: Consider extracting validation logic to reduce duplicationThe validation logic for year of birth is duplicated between
handleOnBlur
andvalidateForm
. Consider extracting this into a shared utility function.+const MIN_BIRTH_YEAR = 1900; + +const validateYearOfBirth = (year: number | null): string => { + if (!year) return t("field_required"); + if (year > maxYear) return `Cannot be greater than ${maxYear}`; + if (year < MIN_BIRTH_YEAR) return `Cannot be smaller than ${MIN_BIRTH_YEAR}`; + return ""; +}; case "year_of_birth": - if (!state.form[field]) { - errors[field] = t("field_required"); - invalidForm = true; - } - - if (parseInt(state.form[field] || "0") > maxYear) { - errors[field] = `Cannot be greater than ${maxYear}`; - invalidForm = true; - } - - if (parseInt(state.form[field] || "0") < 1900) { - errors[field] = `Cannot be smaller than 1900`; - invalidForm = true; - } + const errorMessage = validateYearOfBirth(Number(state.form[field])); + if (errorMessage) { + errors[field] = errorMessage; + invalidForm = true; + } return;
Line range hint
78-217
: Consider implementing form validation using a schema validatorWhile the current implementation meets the requirements, consider using a form validation library like Yup or Zod for more maintainable and type-safe form validation. This would:
- Centralize validation rules
- Provide better type safety
- Reduce code duplication
- Make it easier to add new validations in the future
Example implementation with Yup:
import * as yup from 'yup'; const validationSchema = yup.object().shape({ patient: yup.string().required('Please select the suspect/patient'), year_of_birth: yup.number() .required(t("field_required")) .min(1900, 'Cannot be smaller than 1900') .max(maxYear, `Cannot be greater than ${maxYear}`) });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/Facility/TransferPatientDialog.tsx
(3 hunks)
🔇 Additional comments (2)
src/components/Facility/TransferPatientDialog.tsx (2)
78-82
: LGTM! Good separation of concerns
The simplified handleChange function now focuses solely on updating form state, which aligns well with the PR objective of avoiding validation on every keystroke.
217-217
: LGTM! Validation timing improved
The addition of the onBlur handler successfully addresses the UX issue by validating only after the user has finished entering data.
|
…_in_Patient_transfer
…_in_Patient_transfer
LGTM |
@AdityaJ2305 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
onBlur
which executeshandleOnBlur
validateForm
@ohcnetwork/care-fe-code-reviewers
@nihal467
Screen Recording
Screen.Recording.2024-11-16.at.12.04.38.AM.mov
Merge Checklist
Summary by CodeRabbit
New Features
year_of_birth
input, including checks for acceptable range and improved error messaging.Bug Fixes