-
Notifications
You must be signed in to change notification settings - Fork 666
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
Display year of birth when age is entered in public patient registration page #10808
Conversation
…ion page Fixes #10807 Add year of birth display when age is entered in public patient registration page * Add a new `div` element to display the year of birth when age is entered * Update the `FormField` for age to include the year of birth display * Calculate the year of birth based on the entered age and display it * Display "Invalid age" message if the calculated year of birth is less than or equal to 0 --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/ohcnetwork/care_fe/issues/10807?shareId=XXXX-XXXX-XXXX-XXXX).
Caution Review failedThe pull request is closed. WalkthroughThe changes modify the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✨ Finishing Touches
🪧 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 (3)
src/pages/PublicAppointments/PatientRegistration.tsx (3)
365-377
: Use translation function for error message textThe error message "Invalid age" is hardcoded in English, while other UI strings in the application use the translation function
t()
. For consistency and proper internationalization, this text should be translated.Consider using the translation function for the error message:
- <span className="text-red-600">Invalid age</span> + <span className="text-red-600">{t("invalid_age")}</span>Make sure to add the "invalid_age" key to your translation files.
367-374
: Clarify the age calculation logicThe calculation is correct, but there might be confusion in the variable naming. In this form,
year_of_birth
is actually used to store the user's age input, while the calculated value represents the actual year of birth.To improve code clarity, you could add a comment explaining this or consider refactoring the variable names in a future update to better reflect their purpose.
365-377
: Enhance form reactivity withwatch
instead ofgetValues
Using
form.getValues()
multiple times doesn't automatically re-render when values change during typing. Usingform.watch()
would make the UI update reactively as the user types.Consider modifying the code to use
watch
for better reactivity:- {form.getValues("year_of_birth") && ( + {form.watch("year_of_birth") && ( <div className="text-sm font-bold"> - {Number(form.getValues("year_of_birth")) <= 0 ? ( + {Number(form.watch("year_of_birth")) <= 0 ? ( <span className="text-red-600">Invalid age</span> ) : ( <span className="text-violet-600"> {t("year_of_birth")}:{" "} {new Date().getFullYear() - - Number(form.getValues("year_of_birth"))} + Number(form.watch("year_of_birth"))} </span> )} </div> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/PublicAppointments/PatientRegistration.tsx
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#10260
File: src/components/Patient/PatientRegistration.tsx:286-289
Timestamp: 2025-01-28T15:50:07.442Z
Learning: For patient registration in the frontend, either year_of_birth or date_of_birth is required for successful registration. If date_of_birth is not available, year_of_birth must be present.
src/pages/PublicAppointments/PatientRegistration.tsx (1)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#10260
File: src/components/Patient/PatientRegistration.tsx:286-289
Timestamp: 2025-01-28T15:50:07.442Z
Learning: For patient registration in the frontend, either year_of_birth or date_of_birth is required for successful registration. If date_of_birth is not available, year_of_birth must be present.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (1)
src/pages/PublicAppointments/PatientRegistration.tsx (1)
365-377
: Implementation successfully meets PR objectivesThe added code successfully implements the PR objective to display the year of birth when age is entered. The UI feedback is clear, with proper error handling for invalid ages and a nicely formatted display for valid ages.
CARE
|
Project |
CARE
|
Branch Review |
bodhish/fix-patient-registration
|
Run status |
|
Run duration | 07m 21s |
Commit |
|
Committer | Bodhish Thomas |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
0
|
|
0
|
|
12
|
View all changes introduced in this branch ↗︎ |
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.
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
{Number(form.getValues("year_of_birth")) <= 0 ? ( | ||
<span className="text-red-600">Invalid age</span> | ||
) : ( | ||
<span className="text-violet-600"> | ||
{t("year_of_birth")}:{" "} | ||
{new Date().getFullYear() - | ||
Number(form.getValues("year_of_birth"))} | ||
</span> | ||
)} |
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.
The validation is checking the raw input value rather than the calculated birth year. Consider computing the birth year as new Date().getFullYear() minus the entered age and then verifying that this value is greater than 0.
{Number(form.getValues("year_of_birth")) <= 0 ? ( | |
<span className="text-red-600">Invalid age</span> | |
) : ( | |
<span className="text-violet-600"> | |
{t("year_of_birth")}:{" "} | |
{new Date().getFullYear() - | |
Number(form.getValues("year_of_birth"))} | |
</span> | |
)} | |
{(() => { | |
const currentYear = new Date().getFullYear(); | |
const birthYear = currentYear - Number(form.getValues("year_of_birth")); | |
return birthYear <= 0 ? ( | |
<span className="text-red-600">Invalid age</span> | |
) : ( | |
<span className="text-violet-600"> | |
{t("year_of_birth")}: {birthYear} | |
</span> | |
); | |
})()} |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
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/pages/Patient/Utils.tsx (1)
1-10
: Consider adding validation constraints for the age field.Since age is now being used instead of year_of_birth, it's important to enforce appropriate validation constraints. This should include:
- Ensuring the age is a positive number
- Setting a reasonable maximum age limit (e.g., 120 years)
- Handling validation errors with appropriate user feedback
If the validation is handled elsewhere, consider adding JSDoc comments to this type to document the expected constraints:
export type AppointmentPatientRegister = { name: string; gender: string; phone_number: string; address: string; date_of_birth?: Date | string; + /** + * Patient's age in years. Must be a positive number. + * Used to calculate and display year of birth on the registration form. + */ age?: string; geo_organization?: string; pincode?: string; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/Patient/Utils.tsx
(1 hunks)src/pages/PublicAppointments/PatientRegistration.tsx
(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/PublicAppointments/PatientRegistration.tsx
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#10260
File: src/components/Patient/PatientRegistration.tsx:286-289
Timestamp: 2025-01-28T15:50:07.442Z
Learning: For patient registration in the frontend, either year_of_birth or date_of_birth is required for successful registration. If date_of_birth is not available, year_of_birth must be present.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (1)
src/pages/Patient/Utils.tsx (1)
7-7
:❓ Verification inconclusive
Verify backend compatibility with the type change from
year_of_birth
toage
.This change replaces
year_of_birth
withage
in theAppointmentPatientRegister
type. According to previous learnings, eitheryear_of_birth
ordate_of_birth
is required for successful patient registration. Please ensure one of the following:
- The backend has been updated to accept
age
instead ofyear_of_birth
- The frontend calculates
year_of_birth
fromage
and includes it in the data sent to the backend- The
date_of_birth
field is always provided when usingage
🏁 Script executed:
#!/bin/bash # Check how the registration API is called to verify the data being sent echo "Checking API calls for patient registration..." rg -A 10 "registerPatient|createPatient|patientRegister" --glob "*.ts*" # Look for any backend validation related to year_of_birth or age echo "Checking backend validation requirements..." rg "year_of_birth|dateOfBirth" --glob "*.ts*" | grep -i "required|validation|error"Length of output: 4944
Action: Confirm API field compatibility for patient registration
The new change now defines the patient’s birth-related information with an optional
age
in the type (insrc/pages/Patient/Utils.tsx
). However, our investigation shows that the registration flow (e.g., insrc/components/Patient/PatientRegistration.tsx
) still references and usesyear_of_birth
(both when sending data and when handling responses).Please verify that one of the following approaches has been fully implemented before merging:
- Backend Update: The backend now accepts
age
instead ofyear_of_birth
.- Frontend Transformation: The frontend calculates or maps
year_of_birth
fromage
(or vice versa) before sending data to the backend.- Alternative Field: The process always provides
date_of_birth
to satisfy registration requirements when usingage
.Manually review the API contract and related validations—especially in files such as
src/Utils/request/api.tsx
andsrc/components/Patient/PatientRegistration.tsx
—to ensure these changes won’t break the patient registration process.
LGTM |
@bodhish 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! 🙌 |
Fixes #10807
Add year of birth display when age is entered in public patient registration page
div
element to display the year of birth when age is enteredFormField
for age to include the year of birth displayFor more details, open the Copilot Workspace session.
Summary by CodeRabbit
New Features
Bug Fixes