Enhancement of patient details page #8856
Conversation
✅ 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.
- Please make the tabs a seperate page;
Patient profile: .../patient/:id/demographic
Encounters: .../patient/:id/encounters
Lab Test: .../patient/:id/lab_tests
Shifting: .../patient/:id/shifting
Notes: .../patient/:id/notes
- Remove duplicate alerts
- [ ] Not a fan of the right side, we its now a mix of actions and pages so it should probably be refactored to just action. @vinutv will share the design in few days.
- Broken tags
- Left side has a lot of white space. @vinutv can you check and share your comments
https://deploy-preview-8856--care-ohc.netlify.app/facility/0c95c7f0-e1d2-4aff-83fa-933cef60d3a8/patient/7e8fc20f-02da-47f7-8c33-f2d8c7e52dd8
|
👋 Hi, @Mahendar0701, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
Here is the updated design: https://www.figma.com/design/Z93EYKSa1MdBmXndsMBJSQ/Care?node-id=905-901
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
cypress/pageobject/Patient/PatientConsultation.ts (1)
125-128: Consider renaming method and updating selector to reflect new functionalityThe method now performs two actions: navigating to the Encounters tab and clicking the view updates button. Consider the following improvements:
- The method name
clickViewConsultationButtoncould be more descriptive of its dual functionality- The selector
#view_consultation_and_log_updatesshould be updated to match the new "View Updates" terminology- clickViewConsultationButton() { + navigateToEncountersAndViewUpdates() { cy.get("a").contains("Encounters").click(); cy.verifyAndClickElement( - "#view_consultation_and_log_updates", + "#view-updates", "View Updates", ); }src/components/Patient/PatientDetailsTab/Demography.tsx (1)
385-399: Remove or implement ABHA section properlyThe commented-out ABHA section contains hardcoded values. Either implement it properly using actual patient data or remove it completely.
If implementing, consider this approach:
- {/* <div className="mt-4 rounded-md border border-blue-400 bg-blue-50 p-5 grid grid-cols-1 gap-x-4 gap-y-2 md:grid-cols-2 md:gap-y-8 lg:grid-cols-2"> - {[ - { label: t("abha_number"), value: "-" }, - { label: t("abha_address"), value: "-" }, + <div className="mt-4 rounded-md border border-blue-400 bg-blue-50 p-5 grid grid-cols-1 gap-x-4 gap-y-2 md:grid-cols-2 md:gap-y-8 lg:grid-cols-2"> + {[ + { label: t("abha_number"), value: patientData.abha_number || "-" }, + { label: t("abha_address"), value: patientData.abha_address || "-" },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
cypress/pageobject/Patient/PatientConsultation.ts(1 hunks)src/components/Patient/PatientDetailsTab/Demography.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Patient/PatientDetailsTab/Demography.tsx
[error] 204-207: 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] 286-293: 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] 299-299: 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 (5)
cypress/pageobject/Patient/PatientConsultation.ts (1)
125-128: Verify integration with new routing structure
Let's verify that the navigation aligns with the new routing structure mentioned in the AI summary.
src/components/Patient/PatientDetailsTab/Demography.tsx (4)
1-453: LGTM with fixes!
The component is well-structured with good error handling and permissions implementation. Please address the following issues:
- Add error handling for insurance details query
- Implement proper phone number validation for WhatsApp links
- Fix emergency contact name display
- Add missing key props to components in iterables
- Either implement or remove the commented ABHA section
🧰 Tools
🪛 Biome (1.9.4)
[error] 204-207: 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] 286-293: 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] 299-299: 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)
122-122: 🛠️ Refactor suggestion
Add phone number validation for WhatsApp links
The current phone number formatting might produce invalid WhatsApp URLs.
+ const formatWhatsAppNumber = (phoneNumber?: string) => {
+ if (!phoneNumber) return '';
+ // Remove non-digits and handle international format
+ const cleaned = phoneNumber.replace(/\D+/g, '');
+ return cleaned.startsWith('+') ? cleaned.substring(1) : cleaned;
+ };
- href={`https://wa.me/${props.number?.replace(/\D+/g, "")}`}
+ href={`https://wa.me/${formatWhatsAppNumber(props.number)}`}Likely invalid or redundant comment.
63-68: 🛠️ Refactor suggestion
Add error handling for insurance details query
The query lacks error handling which could lead to runtime errors if the API call fails.
- const { data: insuranceDetials } = useQuery(routes.hcx.policies.list, {
+ const { data: insuranceDetails, error, isLoading } = useQuery(routes.hcx.policies.list, {
query: {
patient: id,
limit: 1,
},
});
+ if (isLoading) {
+ return <LoadingSpinner />;
+ }
+
+ if (error) {
+ console.error('Failed to fetch insurance details:', error);
+ return null;
+ }Likely invalid or redundant comment.
286-293: 🛠️ Refactor suggestion
Add missing key prop to EmergencyContact component
React requires a key prop when rendering elements in an array to optimize rendering performance.
<EmergencyContact
+ key="volunteer-contact"
number={patientData.assigned_to_object?.alt_phone_number}
name={
patientData.assigned_to_object
? formatName(patientData.assigned_to_object)
: undefined
}
/>Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 286-293: 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)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
src/components/Patient/PatientDetailsTab/Demography.tsx (1)
384-398: Consider implementing ABHA details sectionThe commented-out ABHA details section suggests incomplete implementation. Consider implementing this section using actual ABHA data from the patient record.
Would you like me to help implement this section with proper data integration? I can help create a new component for displaying ABHA details or integrate it directly into the current component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/Patient/PatientDetailsTab/Demography.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Patient/PatientDetailsTab/Demography.tsx
[error] 203-206: 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] 285-292: 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] 298-298: 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 (2)
src/components/Patient/PatientDetailsTab/Demography.tsx (2)
121-121: 🛠️ Refactor suggestion
Add phone number validation for WhatsApp links
The WhatsApp link is created without proper phone number validation, which could lead to invalid URLs.
+ const formatWhatsAppNumber = (phoneNumber?: string) => {
+ if (!phoneNumber) return "";
+ // Remove all non-digits and ensure proper international format
+ const cleaned = phoneNumber.replace(/\D+/g, "");
+ return cleaned.startsWith("+") ? cleaned.substring(1) : cleaned;
+ };
- href={`https://wa.me/${props.number?.replace(/\D+/g, "")}`}
+ href={`https://wa.me/${formatWhatsAppNumber(props.number)}`}Likely invalid or redundant comment.
285-292: 🛠️ Refactor suggestion
Add key prop to volunteer contact EmergencyContact component
React requires a key prop when rendering elements in an array.
<EmergencyContact
+ key="volunteer-contact"
number={patientData.assigned_to_object?.alt_phone_number}
name={
patientData.assigned_to_object
? formatName(patientData.assigned_to_object)
: undefined
}
/>Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 285-292: 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)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
cypress/pageobject/Patient/PatientMedicalHistory.ts (3)
Line range hint
38-50: Consider enhancing test assertions for better error messagesWhile the current assertions work, they could be more descriptive and maintainable.
Consider this enhancement:
- cy.get("[data-test-id=patient-health-profile]").then(($dashboard) => { - cy.url().should("include", "/facility/"); - expect($dashboard).to.contain(patientPresentHealth); - expect($dashboard).to.contain(patientOngoingMedication); - expect($dashboard).to.contain(patientAllergies); - expect($dashboard).to.contain(patientSymptoms1); + cy.get("[data-testid=patient-health-profile]").within(() => { + cy.url().should("include", "/facility/"); + cy.contains(patientPresentHealth).should("be.visible"); + cy.contains(patientOngoingMedication).should("be.visible"); + cy.contains(patientAllergies).should("be.visible"); + cy.contains(patientSymptoms1).should("be.visible");This approach:
- Provides better error messages
- Ensures elements are actually visible
- Uses Cypress commands instead of jQuery-style assertions
- Scopes all assertions within the health profile section
Line range hint
53-57: Update verifyNoSymptosPresent method for consistencyThe method still uses the old selector which might cause test failures.
Apply this update:
- cy.get("[data-testid=patient-details]").should( + cy.get("[data-testid=patient-health-profile]").should( "not.contain", patientSymptoms1, );
Line range hint
1-1: Consider restructuring tests for new tabbed interfaceBased on the PR objectives introducing a new tabbed interface, consider splitting this page object into smaller, more focused classes (e.g.,
PatientHealthProfile,PatientConsultationHistory, etc.) to align with the new UI structure.This would:
- Better reflect the new UI organization
- Make tests more maintainable
- Allow for parallel test execution
- Improve test isolation
Would you like assistance in planning this restructuring?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
cypress/e2e/patient_spec/PatientRegistration.cy.ts(2 hunks)cypress/pageobject/Patient/PatientMedicalHistory.ts(1 hunks)src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cypress/e2e/patient_spec/PatientRegistration.cy.ts
- src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/components/Patient/InsuranceDetailsCard.tsx (2)
18-18: Remove empty className props.There are multiple instances of empty className props that should be removed.
Apply this cleanup:
-<div className=" "> +<div>Also applies to: 26-26, 34-34, 42-42
23-23: Use null coalescing operator for fallbacks.The
||operator might not handle all falsy values correctly. Consider using??instead.Apply these changes:
-{data.subscriber_id || ""} +{data.subscriber_id ?? ""} -{data.policy_id || ""} +{data.policy_id ?? ""} -{data.insurer_id || ""} +{data.insurer_id ?? ""} -{data.insurer_name || ""} +{data.insurer_name ?? ""}Also applies to: 31-31, 39-39, 47-47
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/Patient/InsuranceDetailsCard.tsx(1 hunks)
🔇 Additional comments (1)
src/components/Patient/InsuranceDetailsCard.tsx (1)
20-20: Verify translation key existence.
Let's verify that all translation keys are defined in the locale files.
Also applies to: 28-28, 36-36, 44-44
✅ Verification successful
Let me verify the actual content of these translation keys to ensure they are properly defined and not just present as keys without values.
Translation keys are properly defined and implemented
All the translation keys used in InsuranceDetailsCard.tsx (insurance__member_id, insurance__policy_name, insurance__insurer_id, and insurance__insurer_name) are properly defined in the English locale file with appropriate translations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if translation keys exist in locale files
# Define the keys to check
KEYS=(
"insurance__member_id"
"insurance__policy_name"
"insurance__insurer_id"
"insurance__insurer_name"
)
# Search for these keys in locale files
echo "Checking translation keys in locale files..."
for key in "${KEYS[@]}"; do
echo "Checking key: $key"
fd -e json -e yaml . public/locale -x grep -l "$key" {} \;
done
Length of output: 1043
Script:
#!/bin/bash
# Check the actual content of the translation keys in en.json
cat public/locale/en.json | jq '. | {
insurance__member_id,
insurance__policy_name,
insurance__insurer_id,
insurance__insurer_name
}'
Length of output: 326
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (10)
src/components/Patient/PatientDetailsTab/ShiftingHistory.tsx (5)
13-15: Add explicit prop validation for required fieldsConsider adding runtime validation for the required
idprop to prevent potential runtime errors if the component is rendered without proper props.+import { z } from "zod"; + +const propsSchema = z.object({ + id: z.string(), +}); + const ShiftingHistory = (props: PatientProps) => { + const { id } = propsSchema.parse(props); - const { id } = props; const { t } = useTranslation();
35-35: Remove unnecessary Fragment wrapperThe Fragment wrapper is redundant as it contains only one child element.
- <>{t("no_results_found")}</> + {t("no_results_found")}🧰 Tools
🪛 Biome (1.9.4)
[error] 35-35: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
37-43: Enhance responsive grid layoutThe current grid layout might have readability issues on medium-sized screens. Consider adding a medium breakpoint for better responsive behavior.
- <PaginatedList.Items<ShiftingModel> className="grid grid-cols-1 lg:grid-cols-2 xl:grid-cols-3 gap-2"> + <PaginatedList.Items<ShiftingModel> className="grid grid-cols-1 md:grid-cols-2 xl:grid-cols-3 gap-4">
24-28: Consider making the page size configurableThe hardcoded page size of 12 might not be optimal for all screen sizes and data loads. Consider making it configurable or adjusting based on viewport size.
+const ITEMS_PER_PAGE = { + sm: 8, + md: 12, + lg: 16, +}; + const ShiftingHistory = (props: PatientProps) => { const { id } = props; const { t } = useTranslation(); + const [pageSize, setPageSize] = useState(ITEMS_PER_PAGE.md); + + useEffect(() => { + const updatePageSize = () => { + const width = window.innerWidth; + if (width < 768) setPageSize(ITEMS_PER_PAGE.sm); + else if (width < 1280) setPageSize(ITEMS_PER_PAGE.md); + else setPageSize(ITEMS_PER_PAGE.lg); + }; + + window.addEventListener('resize', updatePageSize); + updatePageSize(); + return () => window.removeEventListener('resize', updatePageSize); + }, []); return ( // ... existing JSX <PaginatedList route={routes.listShiftRequests} query={{ patient: id }} - perPage={12} + perPage={pageSize} >
18-23: Enhance accessibility of the section headerConsider adding ARIA attributes to improve accessibility and screen reader support.
- <section className="mt-4"> + <section className="mt-4" aria-labelledby="shifting-history-title"> <div className="flex justify-between items-center"> - <h2 className="my-4 ml-0 text-2xl font-semibold leading-tight"> + <h2 id="shifting-history-title" className="my-4 ml-0 text-2xl font-semibold leading-tight"> {t("shifting_history")} </h2> </div>src/components/Patient/PatientDetailsTab/SampleTestHistory.tsx (1)
1-75: Consider implementing proper state management for sample test approvals.While the component effectively displays the sample test history, the incomplete approval functionality suggests a larger architectural consideration. Given that this is part of the patient details page enhancement:
- Consider whether sample test approvals should be handled at this level or moved to a dedicated modal/component
- If approvals are needed, implement proper state management (consider using React Query mutations for API calls)
- Ensure proper error handling and user feedback for approval actions
This will better align with the PR's objective of enhancing the patient details page UI and functionality.
src/components/Patient/PatientDetailsTab/Demography.tsx (1)
387-401: Address commented ABHA sectionThere's a commented section for ABHA details. If this feature is planned for implementation, it should be tracked.
Would you like me to create a GitHub issue to track the implementation of the ABHA section? I can help with implementing this feature, including proper integration with the patient data model.
src/components/Patient/PatientHome.tsx (1)
742-761: Enhance volunteer assignment dialog UXThe volunteer assignment dialog could benefit from the following improvements:
- Add loading state during assignment
- Add confirmation message when unassigning a volunteer
- Improve error handling feedback
Apply this diff to enhance the dialog:
<ConfirmDialog className="w-full justify-between" title={t("assign_a_volunteer_to", { name: patientData.name })} show={openAssignVolunteerDialog} onClose={() => setOpenAssignVolunteerDialog(false)} + loading={isAssigning} description={ <div className="mt-6"> + {assignedVolunteer && ( + <div className="mb-4 text-sm text-gray-600"> + {t("current_volunteer")}: {formatName(assignedVolunteer)} + </div> + )} <UserAutocomplete value={assignedVolunteer as UserBareMinimum} onChange={(user) => setAssignedVolunteer(user.value)} userType={"Volunteer"} name={"assign_volunteer"} error={errors.assignedVolunteer} /> </div> } - action={t("assign")} + action={assignedVolunteer ? t("update") : t("assign")} + confirmationMessage={ + assignedVolunteer + ? t("confirm_unassign_volunteer", { + volunteer: formatName(assignedVolunteer), + patient: patientData.name, + }) + : undefined + } onConfirm={handleAssignedVolunteer} />public/locale/en.json (2)
Line range hint
755-792: Consider adding translations for empty states in health records.While the health profile sections are well defined, consider adding empty state messages for better user experience.
Add these translations:
+"health_profile.empty": "No health profile information available", +"immunisation.empty": "No immunization records available"
Line range hint
1167-1245: Add validation messages for sample test requests.The sample test workflow translations should include validation messages for required fields.
Add these translations:
+"request_sample_test.validation.sample_type_required": "Sample type is required", +"request_sample_test.validation.collection_date_required": "Collection date is required", +"request_sample_test.validation.test_type_required": "Test type is required"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
public/locale/en.json(38 hunks)src/components/Patient/PatientDetailsTab/Demography.tsx(1 hunks)src/components/Patient/PatientDetailsTab/SampleTestHistory.tsx(1 hunks)src/components/Patient/PatientDetailsTab/ShiftingHistory.tsx(1 hunks)src/components/Patient/PatientDetailsTab/index.tsx(1 hunks)src/components/Patient/PatientHome.tsx(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Patient/PatientDetailsTab/index.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Patient/PatientDetailsTab/Demography.tsx
[error] 203-206: 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] 286-293: 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] 301-301: 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)
src/components/Patient/PatientDetailsTab/ShiftingHistory.tsx
[error] 35-35: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (9)
src/components/Patient/PatientDetailsTab/SampleTestHistory.tsx (2)
1-13: LGTM! Well-structured imports with proper type definitions.
The imports are well-organized and the component uses proper type definitions from models, avoiding any usage of 'any' type.
34-35: Verify translation key usage.
The component uses translation keys. Let's verify they exist in the localization files.
Also applies to: 52-53
✅ Verification successful
Translation keys are properly defined and used
The translation key sample_test_history is correctly defined in public/locale/en.json with the value "Sample Test History" and is being used appropriately in the component.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the translation keys exist in the localization files
# Expected results: Both keys should be found in the localization files
# Find all translation files
fd -e json -e yaml public/locales
# Search for the translation keys in the files
rg -l "sample_test_history|no_records_found"
Length of output: 208
Script:
#!/bin/bash
# Let's check the actual content of the translation key in the locale file
rg "sample_test_history" public/locale/en.json -C 2
Length of output: 267
src/components/Patient/PatientDetailsTab/Demography.tsx (3)
203-206:
Fix emergency contact name display
The emergency contact section incorrectly displays the patient's name instead of the emergency contact's name.
<EmergencyContact
+ key="emergency-contact"
number={patientData.emergency_phone_number}
- name={patientData.name}
+ name={patientData.emergency_contact_name}
/>Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 203-206: 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)
121-121: 🛠️ Refactor suggestion
Add phone number validation for WhatsApp links
The WhatsApp URL is created without proper phone number validation, which could lead to invalid URLs.
+ const formatWhatsAppNumber = (phoneNumber?: string) => {
+ if (!phoneNumber) return '';
+ // Remove all non-digit characters and handle international format
+ const cleaned = phoneNumber.replace(/\D+/g, '');
+ return cleaned.startsWith('+') ? cleaned.substring(1) : cleaned;
+ };
- href={`https://wa.me/${props.number?.replace(/\D+/g, "")}`}
+ href={`https://wa.me/${formatWhatsAppNumber(props.number)}`}Likely invalid or redundant comment.
63-67: 🛠️ Refactor suggestion
Add error handling for insurance details query
The query lacks error handling which could lead to runtime errors if the API call fails.
- const { data: insuranceDetials } = useQuery(routes.hcx.policies.list, {
+ const { data: insuranceDetails, isLoading, error } = useQuery(routes.hcx.policies.list, {
query: {
patient: id,
},
});
+ if (isLoading) {
+ return <div className="text-center py-4">{t("loading")}</div>;
+ }
+
+ if (error) {
+ console.error("Failed to fetch insurance details:", error);
+ return <div className="text-center py-4 text-red-500">{t("error_loading_insurance")}</div>;
+ }Likely invalid or redundant comment.
public/locale/en.json (4)
Line range hint 277-309: LGTM: Patient admission and consultation translations are well-structured.
The translations for actions, admission status, and basic patient information are clear and consistent with healthcare terminology.
1029-1033: LGTM: Patient information section translations are comprehensive.
The translations for patient notes, general info, insurance details, social profile, and volunteer contact info are well organized and complete.
1057-1058: LGTM: Profile navigation translations are clear.
The translations for patient profile navigation and creation attribution are concise and clear.
1324-1327: LGTM: Patient transfer status translations are complete.
The translations for transfer status and workflow messages are clear and comprehensive.
|
@bodhish added requested changes. once look into it. Can i do "displaying Shifiting details as a list" in my next PR , as new tab designs are proposed in https://www.figma.com/design/Z93EYKSa1MdBmXndsMBJSQ/Care?node-id=905-901&node-type=canvas&t=cA4CvHwv7gPc5Ij2-0 i will update all tab designs in my next PR. What do you say? I want to close this PR becz its been so long... |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
src/components/Patient/PatientDetailsTab/Demography.tsx (1)
388-402: Use actual ABHA data instead of hardcoded valuesThe commented ABHA section uses hardcoded values. If this section is to be used, it should display actual patient data.
{[ - { label: t("abha_number"), value: "-" }, - { label: t("abha_address"), value: "-" }, + { label: t("abha_number"), value: patientData.abha_number || "-" }, + { label: t("abha_address"), value: patientData.abha_address || "-" }, ].map((info, i) => (src/components/Patient/PatientHome.tsx (1)
76-77: Consider using a typed error state.Instead of using
anytype for errors, consider defining a proper interface for better type safety.-const initErr: any = {}; -const errors = initErr; +interface FormErrors { + assignedVolunteer?: string; + [key: string]: string | undefined; +} +const [errors, setErrors] = useState<FormErrors>({});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
cypress/pageobject/Sample/SampleTestCreate.ts(2 hunks)src/components/Patient/PatientDetailsTab/Demography.tsx(1 hunks)src/components/Patient/PatientHome.tsx(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cypress/pageobject/Sample/SampleTestCreate.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Patient/PatientDetailsTab/Demography.tsx
[error] 203-206: 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] 302-302: 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)
src/components/Patient/PatientDetailsTab/Demography.tsx (3)
26-456: Implementation looks good with minor improvements needed
The component is well-structured with proper TypeScript usage, good error handling, and appropriate permission checks. The suggested improvements will enhance the robustness and maintainability of the code.
🧰 Tools
🪛 Biome (1.9.4)
[error] 203-206: 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] 302-302: 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)
63-67: 🛠️ Refactor suggestion
Add error handling for insurance details query
The insurance details query should handle loading and error states to improve user experience.
- const { data: insuranceDetials } = useQuery(routes.hcx.policies.list, {
+ const { data: insuranceDetials, isLoading, error } = useQuery(routes.hcx.policies.list, {
query: {
patient: id,
},
});
+ if (isLoading) {
+ return <div className="text-center py-4">{t("loading")}</div>;
+ }
+
+ if (error) {
+ console.error("Failed to fetch insurance details:", error);
+ return <div className="text-center py-4 text-red-500">{t("error_loading_insurance")}</div>;
+ }Likely invalid or redundant comment.
121-121: 🛠️ Refactor suggestion
Add phone number validation for WhatsApp links
The current phone number formatting might produce invalid WhatsApp URLs.
+ const formatWhatsAppNumber = (phoneNumber?: string) => {
+ if (!phoneNumber) return '';
+ const cleaned = phoneNumber.replace(/\D+/g, '');
+ return cleaned.startsWith('+') ? cleaned.substring(1) : cleaned;
+ };
- href={`https://wa.me/${props.number?.replace(/\D+/g, "")}`}
+ href={`https://wa.me/${formatWhatsAppNumber(props.number)}`}Likely invalid or redundant comment.
src/components/Patient/PatientHome.tsx (3)
713-715:
Fix incorrect date usage in profile creation timestamp.
The profile creation timestamp is using modified_date instead of created_date.
- {patientData.modified_date
+ {patientData.created_date
- ? relativeDate(patientData.modified_date)
+ ? relativeDate(patientData.created_date)
: "--:--"}Likely invalid or redundant comment.
410-410:
Use a proper phone number formatting library.
Direct manipulation of phone numbers with regex can be unsafe. Consider using a proper phone number formatting library.
-href={`https://wa.me/${patientData.last_consultation.assigned_to_object.alt_phone_number.replace(/\D+/g, "")}`}
+import { parsePhoneNumber } from 'libphonenumber-js';
+
+const phoneNumber = parsePhoneNumber(patientData.last_consultation.assigned_to_object.alt_phone_number, 'IN');
+href={phoneNumber?.isValid() ? `https://wa.me/${phoneNumber.format('E.164').slice(1)}` : '#'}Likely invalid or redundant comment.
174-191:
Add error handling to handlePatientTransfer.
The function should handle API failures gracefully to prevent silent failures.
const handlePatientTransfer = async (value: boolean) => {
+ try {
await request(routes.patchPatient, {
pathParams: {
id: patientData.id as string,
},
body: { allow_transfer: value },
onResponse: ({ res }) => {
if (res?.status === 200) {
setPatientData((prev) => ({
...prev,
allow_transfer: value,
}));
Notification.Success({
msg: t("transfer_status_updated"),
});
+ } else {
+ throw new Error("Failed to update transfer status");
}
},
});
+ } catch (error) {
+ Notification.Error({
+ msg: t("failed_to_update_transfer_status"),
+ });
+ }
};Likely invalid or redundant comment.
| <EmergencyContact | ||
| number={patientData.emergency_phone_number} | ||
| name={patientData.name} | ||
| />, |
There was a problem hiding this comment.
Fix emergency contact name and add key prop
The emergency contact section incorrectly displays the patient's name instead of the emergency contact's name.
<EmergencyContact
+ key="emergency-contact"
number={patientData.emergency_phone_number}
- name={patientData.name}
+ name={patientData.emergency_contact_name}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <EmergencyContact | |
| number={patientData.emergency_phone_number} | |
| name={patientData.name} | |
| />, | |
| <EmergencyContact | |
| key="emergency-contact" | |
| number={patientData.emergency_phone_number} | |
| name={patientData.emergency_contact_name} | |
| />, |
🧰 Tools
🪛 Biome (1.9.4)
[error] 203-206: 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)
| <div | ||
| className="sticky top-0 z-10 mt-4 w-full overflow-x-auto border-b bg-gray-50" | ||
| role="navigation" | ||
| > | ||
| <div className="flex flex-row" role="tablist"> | ||
| {patientTabs.map((tab) => ( | ||
| <Link | ||
| key={tab.route} | ||
| href={`/facility/${facilityId}/patient/${id}/${tab.route}`} | ||
| className={`whitespace-nowrap px-4 py-2 text-sm font-medium ${ | ||
| page === tab.route | ||
| ? "border-b-4 border-green-800 text-green-800 md:border-b-2" | ||
| : "rounded-t-lg text-gray-600 hover:bg-gray-100" | ||
| }`} | ||
| role="tab" | ||
| aria-selected={page === tab.route} | ||
| aria-controls={`${tab.route}-panel`} | ||
| > | ||
| {t(tab.route)} | ||
| </Link> | ||
| ))} | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance tab navigation accessibility.
The tab navigation should support keyboard interactions and follow WAI-ARIA guidelines completely.
+import { useCallback, useState } from 'react';
+
+const [focusedTab, setFocusedTab] = useState(0);
+
+const handleKeyDown = useCallback((event: React.KeyboardEvent, index: number) => {
+ switch (event.key) {
+ case 'ArrowLeft':
+ setFocusedTab(Math.max(0, index - 1));
+ break;
+ case 'ArrowRight':
+ setFocusedTab(Math.min(patientTabs.length - 1, index + 1));
+ break;
+ case 'Home':
+ setFocusedTab(0);
+ break;
+ case 'End':
+ setFocusedTab(patientTabs.length - 1);
+ break;
+ }
+}, [patientTabs.length]);
<div
className="sticky top-0 z-10 mt-4 w-full overflow-x-auto border-b bg-gray-50"
role="navigation"
>
<div className="flex flex-row" role="tablist">
{patientTabs.map((tab, index) => (
<Link
key={tab.route}
href={`/facility/${facilityId}/patient/${id}/${tab.route}`}
className={`whitespace-nowrap px-4 py-2 text-sm font-medium ${
page === tab.route
? "border-b-4 border-green-800 text-green-800 md:border-b-2"
: "rounded-t-lg text-gray-600 hover:bg-gray-100"
}`}
role="tab"
aria-selected={page === tab.route}
aria-controls={`${tab.route}-panel`}
+ tabIndex={focusedTab === index ? 0 : -1}
+ onKeyDown={(e) => handleKeyDown(e, index)}
>
{t(tab.route)}
</Link>
))}
</div>
</div>Committable suggestion skipped: line range outside the PR's diff.
|
@Mahendar0701 consider this closed, @shivankacker will get this merged. |
|
@shivankacker @bodhish is it ready for QA testing, iam seeing a lot of recent commits and conversation, |
|
@nihal467 it is work in progress |
|
Closed, being continued in #9203 |
Proposed Changes
Proposed:
Modified page:
Need further suggestion on implementing ABHA card and Hospital ID Card
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
Improvements
PatientHomecomponent with a new tabbed interface for better organization of patient details.ShiftingBlockcomponent for optional transfer handling.PatientInfoCardandInsuranceDetailscomponents.Chipcomponent's styling for better visual consistency.Bug Fixes