-
Notifications
You must be signed in to change notification settings - Fork 514
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
Patient Registration form refactor #9854
base: develop
Are you sure you want to change the base?
Conversation
* forgot password mutation * updated
Co-authored-by: Rithvik Nishad <[email protected]>
…updating related components. Refactor organization level retrieval to use metadata for improved localization. Remove deprecated organization levels constant and clean up unused code in various components.
- Introduced a new PermissionContext to manage user permissions and super admin status across the application. - Updated AppRouter to utilize PermissionProvider for managing permissions. - Changed UserModel to store permissions as strings instead of objects. - Refactored OrganizationFacilities and OrganizationPatients components to use updated permission checks and organization identifiers. - Enhanced OrganizationLayout to conditionally render navigation items based on user permissions. This commit improves the permission management system and ensures consistent handling of user permissions throughout the application.
- Introduced a new `searchPostFix` prop in `ValueSetSelect` to allow appending a suffix to the search query. - Updated `MedicationRequestQuestion` and `MedicationStatementQuestion` components to utilize the `searchPostFix` prop with a default value of " clinical drug". - This change enhances search functionality by providing more context in search queries.
Bumps [@tanstack/react-query-devtools](https://github.com/TanStack/query/tree/HEAD/packages/react-query-devtools) from 5.62.11 to 5.62.15. - [Release notes](https://github.com/TanStack/query/releases) - [Commits](https://github.com/TanStack/query/commits/HEAD/packages/react-query-devtools) --- updated-dependencies: - dependency-name: "@tanstack/react-query-devtools" dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Bodhisha Thomas <[email protected]> Co-authored-by: Bodhish Thomas <[email protected]>
…tiple components (#9793)
* Enhance encounter data handling by adding encounterId prop across multiple components - Added `encounterId` prop to `ObservationChart`, `ObservationHistoryTable`, `ObservationsList`, `QuestionnaireResponsesList`, and `StructuredResponseView` components to improve data fetching and display related to specific encounters. - Updated query parameters in API calls to include `encounterId` for better data context. - Refactored `EncounterPlotsTab` and `EncounterUpdatesTab` to pass the new `encounterId` prop, ensuring consistent data handling across the application. This change improves the overall functionality and user experience by ensuring that encounter-specific data is accurately retrieved and displayed. * fix: disable encounter create button during save to prevent multiple submissions #9794
…anizationSelector labels - Removed unused state and commented-out code in PatientHome for improved readability. - Enhanced patient data display by updating the last updated and created by fields to use `updated_by` instead of `modified_by`. - Updated date formatting functions to ensure consistent display of patient and encounter dates. - Changed labels in FacilityOrganizationSelector from "Organization" to "Select Department" and adjusted related text for clarity. These changes streamline the codebase and improve user interface clarity.
…components directly. (#9847)
WalkthroughThis pull request introduces several changes across multiple files, focusing on enhancing localization, modifying the plugin architecture, and refactoring patient registration forms. Key modifications include adding new entries to the localization JSON file for improved user prompts and validation messages, integrating Changes
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. |
Deploying care-fe with Cloudflare Pages
|
CARE Run #4225
Run Properties:
|
Project |
CARE
|
Branch Review |
abdm_micro_fe
|
Run status |
Passed #4225
|
Run duration | 01m 33s |
Commit |
e18e44805d: Patient Registration form refactor
|
Committer | Khavin Shankar |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
6
|
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.
Actionable comments posted: 8
🧹 Nitpick comments (2)
src/components/Patient/PatientRegistration.tsx (1)
229-242
: Unnecessary use ofuseMemo
for duplicatePatientsThe
duplicatePatients
variable does not benefit significantly from memoization here, as the computation is minimal. UsinguseMemo
might introduce unnecessary complexity.You may simplify the code by removing
useMemo
:-const duplicatePatients = useMemo(() => { - return patientPhoneSearch.data?.results.filter((p) => p.id !== patientId); -}, [patientPhoneSearch.data, patientId]); +const duplicatePatients = patientPhoneSearch.data?.results.filter((p) => p.id !== patientId);src/pluginTypes.ts (1)
27-28
: UpdatePatientRegistrationFormComponentType
to use specific form typesCurrently,
form
is typed asUseFormReturn<any>
, which may reduce type safety. Consider specifying the exact form schema type to enhance type checking and prevent potential bugs.For example:
-import { UseFormReturn } from "react-hook-form"; +import { UseFormReturn, FieldValues } from "react-hook-form"; +import { z } from "zod"; +import { formSchema } from "@/components/Patient/PatientRegistration"; export type PatientRegistrationFormComponentType = React.FC<{ - form: UseFormReturn<any>; + form: UseFormReturn<z.infer<typeof formSchema>>; patientId?: string; }>;Ensure that the
formSchema
is exported fromPatientRegistration
and imported here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
public/locale/en.json
(1 hunks)scripts/setup-care-apps.ts
(1 hunks)src/components/Facility/ConsultationDetails/EncounterContext.tsx
(0 hunks)src/components/Patient/PatientHome.tsx
(3 hunks)src/components/Patient/PatientRegistration.tsx
(7 hunks)src/pluginTypes.ts
(2 hunks)vite.config.mts
(2 hunks)
💤 Files with no reviewable changes (1)
- src/components/Facility/ConsultationDetails/EncounterContext.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Patient/PatientRegistration.tsx
[error] 418-418: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 457-457: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (6)
scripts/setup-care-apps.ts (1)
44-51
: Ensure proper formatting of generatedpluginMap.ts
contentThe added line breaks in the template string may affect the formatting of the generated
pluginMap.ts
. Verify that the output is as intended and the code remains syntactically correct.Run the script and check the contents of
pluginMap.ts
to ensure it is correctly formatted and imports are resolved properly.src/pluginTypes.ts (1)
22-24
: EnsurePatientHomeActionsComponentType
props are correctly typedThe new
PatientHomeActionsComponentType
expects apatient
prop of typePatient
. Verify that this aligns with the actual data being passed and that thePatient
type correctly represents the patient data structure.If necessary, update the
Patient
type import to match the correct patient model used in your application.vite.config.mts (1)
198-203
: LGTM: Shared dependencies updateGood addition of
react-i18next
and@tanstack/react-query
to the shared dependencies. This will help avoid duplicate package loading across federated modules.src/components/Patient/PatientHome.tsx (2)
14-14
: LGTM: Clean plugin integrationThe
PLUGIN_Component
integration is well-implemented with proper props passing for patient data and styling.Also applies to: 155-159
138-139
: LGTM: Improved markup structureGood cleanup of the div structure while maintaining the styling consistency.
public/locale/en.json (1)
1433-1433
: LGTM: Added blood group selection messageThe translation key follows the established naming pattern and provides clear user guidance.
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.
i18n for schema errors missing
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.
quick pass, lgtm
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 (3)
src/components/Patient/PatientRegistration.tsx (3)
169-172
: Simplify year of birth calculationThe year of birth calculation in the success handler is complex and could be simplified.
Consider extracting this logic into a helper function:
+ const getYearOfBirth = (data: PatientModel) => { + if (data.date_of_birth) return new Date(data.date_of_birth).getFullYear(); + return data.age ? new Date().getFullYear() - Number(data.age) : undefined; + }; + navigate(`/facility/${facilityId}/patients/verify`, { query: { phone_number: resp.phone_number, - year_of_birth: - form.getValues("yob_or_dob") === "dob" - ? new Date(resp.date_of_birth!).getFullYear() - : new Date().getFullYear() - Number(resp.age!), + year_of_birth: getYearOfBirth(resp), partial_id: resp?.id?.slice(0, 5), }, });
273-279
: Potential race condition in debounced phone number searchThe current implementation might lead to race conditions where an older search result could override a newer one if the responses arrive out of order.
Consider using a request identifier or AbortController to handle this:
useEffect(() => { + const abortController = new AbortController(); const handler = setTimeout(() => { const phoneNumber = form.getValues("phone_number"); if (!patientId || patientQuery.data?.phone_number !== phoneNumber) { setSuppressDuplicateWarning(false); } setDebouncedNumber(phoneNumber); }, 500); return () => { clearTimeout(handler); + abortController.abort(); }; }, [form.watch("phone_number")]); // eslint-disable-line react-hooks/exhaustive-deps
437-457
: Add aria-labels for better accessibilityThe blood group select component lacks proper aria-labels for screen readers.
Add appropriate aria labels:
<Select {...field} onValueChange={field.onChange} defaultValue={field.value} + aria-label={t("blood_group")} > <FormControl> - <SelectTrigger> + <SelectTrigger aria-label={t("select_blood_group")}> <SelectValue placeholder={t("please_select_blood_group")} /> </SelectTrigger> </FormControl> - <SelectContent> + <SelectContent aria-label={t("blood_group_options")}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json
(9 hunks)src/components/Patient/PatientRegistration.tsx
(8 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 (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (2)
src/components/Patient/PatientRegistration.tsx (2)
488-495
:⚠️ Potential issueHandle undefined values in date fields
The date field splitting logic doesn't handle undefined values, which could lead to runtime errors.
Add null checks and default values:
- value={ - form.watch("date_of_birth")?.split("-")[2] - } + value={form.watch("date_of_birth")?.split("-")[2] || ""} onChange={(e) => form.setValue( "date_of_birth", - `${form.watch("date_of_birth")?.split("-")[0]}-${form.watch("date_of_birth")?.split("-")[1]}-${e.target.value}`, + `${form.watch("date_of_birth")?.split("-")[0] || "YYYY"}-${form.watch("date_of_birth")?.split("-")[1] || "MM"}-${e.target.value}`, ) }Likely invalid or redundant comment.
562-574
: 🛠️ Refactor suggestionSimplify age calculation logic
The age calculation logic is complex and could be simplified. Also, the current implementation might not handle edge cases correctly.
Consider extracting this logic into a helper function:
+const calculateAge = (yearOfBirth: string) => { + const currentYear = new Date().getFullYear(); + const parsedYear = Number(yearOfBirth); + return isNaN(parsedYear) ? '' : String(currentYear - parsedYear); +}; + +const calculateYearOfBirth = (age: string) => { + const currentYear = new Date().getFullYear(); + const parsedAge = Number(age); + return isNaN(parsedAge) ? '' : String(currentYear - parsedAge); +}; - value={ - new Date().getFullYear() - - Number(form.watch("year_of_birth")) - } + value={calculateAge(form.watch("year_of_birth") || '')} onChange={(e) => form.setValue( "year_of_birth", - String( - new Date().getFullYear() - - Number(e.target.value), - ), + calculateYearOfBirth(e.target.value), ) }Likely invalid or redundant comment.
👋 Hi, @khavinshankar, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
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: 2
🧹 Nitpick comments (1)
public/locale/en.json (1)
1450-1450
: Fix typo in validation messageThe word "Permanant" is misspelled in the validation message key and value.
Apply this diff to fix the typo:
- "permanant_address_is_required": "Permanant address is required", + "permanent_address_is_required": "Permanent address is required",Remember to update any references to this key in the code as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json
(9 hunks)src/components/Patient/PatientRegistration.tsx
(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test
- GitHub Check: OSSAR-Scan
- GitHub Check: OSSAR-Scan
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (3)
src/components/Patient/PatientRegistration.tsx (3)
248-263
: Add missing dependency in useEffectThe
useEffect
hook depends onform.reset
butform
is not included in the dependency array. This could lead to stale closures if the form reference changes.
555-566
: Fix age calculation logicThe age calculation in the form appears to be incorrect. The current implementation may lead to wrong age values being displayed and stored.
404-416
: Add key props to list itemsMissing key props in list rendering can cause React performance issues and warnings.
Also applies to: 443-447
phone_number: z | ||
.string() | ||
.regex(/^\+\d{12}$/, t("phone_number_must_be_10_digits")), |
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.
Fix inconsistency in phone number validation
The regex pattern /^\+\d{12}$/
requires exactly 12 digits after the '+' sign, but the error message says "must be a 10-digit mobile number". This mismatch will confuse users.
Apply this diff to fix the validation:
- .regex(/^\+\d{12}$/, t("phone_number_must_be_10_digits")),
+ .regex(/^\+\d{10}$/, t("phone_number_must_be_10_digits")),
Make the same change for emergency_phone_number
validation as well.
Also applies to: 93-95
<Input | ||
type="number" | ||
placeholder="DD" | ||
{...field} | ||
value={ | ||
form.watch("date_of_birth")?.split("-")[2] | ||
} | ||
onChange={(e) => | ||
form.setValue( | ||
"date_of_birth", | ||
`${form.watch("date_of_birth")?.split("-")[0]}-${form.watch("date_of_birth")?.split("-")[1]}-${e.target.value}`, | ||
) | ||
} | ||
/> |
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.
Add validation for undefined date of birth values
When splitting form.watch("date_of_birth")
, if the value is undefined or in an unexpected format, it could cause runtime errors.
Apply this diff to add validation:
value={
- form.watch("date_of_birth")?.split("-")[2]
+ form.watch("date_of_birth")?.split("-")?.[2] || ""
}
onChange={(e) =>
form.setValue(
"date_of_birth",
- `${form.watch("date_of_birth")?.split("-")[0]}-${form.watch("date_of_birth")?.split("-")[1]}-${e.target.value}`,
+ `${form.watch("date_of_birth")?.split("-")?.[0] || ""}-${form.watch("date_of_birth")?.split("-")?.[1] || ""}-${e.target.value}`,
)
}
Apply similar changes to the month and year input handlers.
Also applies to: 495-508, 514-527
Summary by CodeRabbit
Release Notes
Localization
Patient Registration
Plugin System
Configuration