Clean up encounter routes #11084
Clean up encounter routes #11084
Conversation
WalkthroughThis pull request reverses the sidebar rendering logic in the AppRouter and refactors several route definitions to consolidate similar routes. It adjusts prop interfaces across various components to either introduce new properties (such as an optional Changes
Sequence Diagram(s)sequenceDiagram
participant AR as AppRouter
participant Admin as AdminPages
participant Fac as FacilityPages
AR->>Admin: Check for adminPages availability
alt Admin pages exist
AR->>AR: Set sidebarFor = ADMIN
else
AR->>Fac: Default to facility pages
AR->>AR: Set sidebarFor = FACILITY
end
sequenceDiagram
participant AP as Appointments Component
participant API as Schedule API
AP->>AP: Check if facilityId is present
alt facilityId exists
AP->>API: Call appointments.list endpoint
else
AP->>API: Call appointments.getAppointments endpoint
end
API-->>AP: Return appointment data
AP->>AP: Render appointments list or loading state
alt facilityId exists
AP->>AP: Display "Schedule Appointment" button
end
Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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
|
| Latest commit: |
e13b79b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://49c3f6e3.care-fe.pages.dev |
| Branch Preview URL: | https://clenup-encounter-routes.care-fe.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/Routers/routes/ConsultationRoutes.tsx (1)
48-70: Well-implemented route consolidation for different contextsThe refactoring elegantly handles both facility and organization contexts using a reduce pattern, which:
- Reduces code duplication
- Makes maintenance easier
- Provides consistent handling of both contexts
The conditional setting of
facilityIdensures the component works correctly in either context.Consider adding a comment explaining this pattern for future developers, especially since the dynamic route generation approach is more advanced than the typical route definitions:
+ // Dynamically generate similar routes for both facility and organization contexts + // When identifier is "facility", pass the id as facilityId + // When identifier is "organization", facilityId remains undefined ...["facility", "organization"].reduce((acc: AppRoutes, identifier) => { acc[`/${identifier}/:id/patient/:patientId/encounter/:encounterId/:tab`] = ({ id, encounterId, tab, patientId }) => (src/components/Patient/PatientDetailsTab/Appointments.tsx (1)
99-105: Use optional chaining for better code safety.The loading state is now correctly derived from the
isLoadingflag rather than inferring it from the absence of data. However, there's a potential issue with the condition check.Apply this change for better null safety:
- ) : appointments && appointments.length ? ( + ) : appointments?.length ? (🧰 Tools
🪛 Biome (1.9.4)
[error] 105-105: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/Routers/AppRouter.tsx(1 hunks)src/Routers/routes/ConsultationRoutes.tsx(1 hunks)src/Routers/routes/PatientRoutes.tsx(2 hunks)src/components/Facility/EncounterCard.tsx(2 hunks)src/components/Medicine/MedicationRequestTable/index.tsx(2 hunks)src/components/Patient/PatientDetailsTab/Appointments.tsx(3 hunks)src/components/Patient/PatientDetailsTab/EncounterHistory.tsx(1 hunks)src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx(1 hunks)src/components/Patient/PatientDetailsTab/index.tsx(1 hunks)src/components/Patient/PatientDetailsTab/patientUpdates.tsx(1 hunks)src/components/Patient/PatientHome.tsx(2 hunks)src/components/Patient/allergy/list.tsx(6 hunks)src/components/Patient/diagnosis/list.tsx(5 hunks)src/components/Patient/symptoms/list.tsx(5 hunks)src/pages/Encounters/EncounterShow.tsx(3 hunks)src/pages/Encounters/tabs/EncounterMedicinesTab.tsx(0 hunks)src/pages/Encounters/tabs/EncounterUpdatesTab.tsx(1 hunks)src/types/scheduling/scheduleApi.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/pages/Encounters/tabs/EncounterMedicinesTab.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Patient/PatientDetailsTab/Appointments.tsx
[error] 105-105: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (29)
src/components/Patient/symptoms/list.tsx (2)
23-23: Great refactoring: UsingreadOnlyinstead offacilityIdfor conditional rendering.The change from using
facilityIdto a more explicitreadOnlyprop improves the component's API by making its behavior more predictable and intention-revealing. This is a positive pattern that separates the component's display behavior from its data dependencies.Also applies to: 30-30, 47-47, 70-70, 84-84, 121-121, 128-128, 134-136
136-137:❓ Verification inconclusive
Simplified URL - verify routing logic still works.
The link URL has been simplified from what was likely a more complex path that included facility/patient context. Ensure the router correctly handles this simplified path and that navigation still works as expected throughout the application.
🏁 Script executed:
#!/bin/bash # Check if there are any references to the old URL pattern with facilityId rg -A 1 "questionnaire/symptom" --glob "*.{tsx,jsx,ts,js}"Length of output: 273
Routing Simplification Verification Needed
The URL has been simplified to
questionnaire/symptomin the component. Please verify that the router configuration and navigation logic now correctly handle this simplified path throughout the application. In particular, ensure that any functionality previously dependent on the facility/patient context is either updated or no longer required.
- Verify that the router’s route definitions and any redirects are correctly configured for the simplified URL.
- Confirm through manual or automated tests that navigation behaves as expected across the app.
src/Routers/AppRouter.tsx (1)
89-89: Reversed sidebar priority to favor admin pages.This change corrects the sidebar selection logic to prioritize admin pages over facility pages, which aligns with the PR objective. This ensures that when both admin routes and app routes are available, the admin sidebar is displayed.
src/pages/Encounters/tabs/EncounterUpdatesTab.tsx (1)
30-30: Simplified component props by removing facilityId.Removing the
facilityIdfromSymptomsListandDiagnosisListcalls is consistent with the refactoring pattern seen in other files. This change helps streamline the component API and aligns with the project's goal of simplifying routing and context management.Also applies to: 35-35
src/components/Medicine/MedicationRequestTable/index.tsx (2)
53-57: Props interface simplified by removing facilityId.The removal of
facilityIdfrom thePropsinterface is consistent with the pattern of simplification throughout this PR. This helps create more focused and less context-dependent components.
162-163:❓ Verification inconclusive
Simplified link URLs - verify routing still works.
The links to questionnaire and print pages have been simplified to use direct paths rather than constructed URLs with facilityId context. Ensure that the router correctly handles these simplified paths and that navigation still works as expected throughout the application.
Also applies to: 173-174
🏁 Script executed:
#!/bin/bash # Check for any references to the old URL patterns and the new simplified routes echo "Checking questionnaire/medication_request route usage:" rg -A 1 "questionnaire/medication_request" --glob "*.{tsx,jsx,ts,js}" echo "Checking prescriptions/print route usage:" rg -A 1 "prescriptions/print" --glob "*.{tsx,jsx,ts,js}" echo "Checking router definitions for these routes:" rg -A 5 "medication_request|prescriptions/print" --glob "*Routes.tsx"Length of output: 1731
Action: Verify navigation with simplified link URLs
The simplified URLs in
src/components/Medicine/MedicationRequestTable/index.tsx(lines 162–163 and 173–174) now use direct paths (questionnaire/medication_requestandprescriptions/print). However, our router definition for the print route (insrc/Routers/routes/ConsultationRoutes.tsx) still expects a path with dynamic parameters (i.e./facility/:facilityId/patient/:patientId/encounter/:encounterId/prescriptions/print). Please ensure that navigation to the print page works as intended—confirm that any necessary context (such as facility, patient, and encounter IDs) is supplied through state or other means when using the simplified link. Also, verify that the questionnaire route is similarly supported in the routing configuration.src/components/Patient/PatientDetailsTab/EncounterHistory.tsx (1)
107-111: Good addition of the facilityId prop!This enhancement properly passes the facility context down to the EncounterCard component, enabling it to generate facility-specific URLs.
src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx (3)
29-29: LGTM: Consistent use of readOnly prop.Adding the readOnly prop to AllergyList supports view-only functionality.
33-33: LGTM: Consistent use of readOnly prop.Adding the readOnly prop to SymptomsList supports view-only functionality.
37-37: LGTM: Consistent use of readOnly prop.Adding the readOnly prop to DiagnosisList supports view-only functionality.
src/Routers/routes/PatientRoutes.tsx (2)
1-1: Good consolidation of imports.Removing the separate import for
facilityPatientTabsand relying solely onpatientTabssimplifies the code.
34-40: Code simplification approved.Using
patientTabsfor both patient and facility routes reduces duplication and improves maintainability.src/components/Facility/EncounterCard.tsx (2)
19-19: Good interface extension.Making
facilityIdoptional allows the component to be used in both facility-specific and general contexts.
23-23: Consistent prop destructuring.Properly destructuring the new
facilityIdprop alongside the existingencounterprop.src/types/scheduling/scheduleApi.ts (1)
160-167: LGTM: New API endpoint to retrieve appointments across facilitiesThe new endpoint is well-defined with appropriate path parameters and return type. This addition aligns with the PR objective of improving handling for both facility and organization contexts.
src/components/Patient/PatientHome.tsx (2)
13-13: Streamlined tab import patternThis change supports the consolidation of
facilityPatientTabsandpatientTabsinto a single import, simplifying the component's tab management.
24-24: Updated type definition to match new tab structureThe type definition has been correctly updated to reflect the consolidated tab structure, maintaining type safety.
src/components/Patient/PatientDetailsTab/index.tsx (1)
38-38:✅ Verification successful
Component update for the "updates" route
The component for the "updates" route has been changed to
Updates. This change aligns with the PR objectives of improving consistency in the patient details tabs.
🏁 Script executed:
#!/bin/bash # Verify the Updates component exists and its implementation fd "Updates.tsx|Updates.jsx" --type fLength of output: 99
Updates Component Update Confirmed
The file search reveals that the updates component is defined in
src/components/Patient/PatientDetailsTab/patientUpdates.tsxand is correctly imported asUpdatesin the index file. This aligns with the PR objectives of maintaining consistency within the patient details tabs.src/components/Patient/PatientDetailsTab/Appointments.tsx (2)
29-42: Good enhancement to handle both facility and organization contexts.The updated query function now intelligently selects the appropriate API endpoint based on the presence of a
facilityId. This makes the component more versatile, allowing it to work in both facility-specific and organization-wide contexts, which aligns with the PR's objective.
74-84: Smart UI enhancement for context-aware action buttons.Showing the "Schedule Appointment" button only when a facility is available prevents users from attempting actions that would fail. This improves the user experience by adapting the UI based on available contexts.
src/components/Patient/diagnosis/list.tsx (2)
24-32: Good pattern for component flexibility.Adding the
readOnlyprop with a default value enhances the component's reusability across different contexts. This allows the same component to be used in both interactive and display-only scenarios.
126-134: Context-aware UI controls with simplified routing.Conditionally rendering the edit link based on the
readOnlystate improves UX by hiding edit options in read-only contexts. The simplified URL structure also makes the routes more maintainable and consistent.src/components/Patient/PatientDetailsTab/patientUpdates.tsx (1)
3-16: Excellent code simplification through component reuse.Replacing the custom implementation with the
QuestionnaireResponsesListcomponent significantly reduces code duplication and improves maintainability. This change showcases good adherence to DRY principles and creates a more consistent user experience across the application.src/components/Patient/allergy/list.tsx (2)
53-75: Consistent implementation of the readOnly pattern.Adding the
readOnlyprop follows the same pattern applied to other components, showing a systematic approach to component enhancement. This consistency across components makes the codebase more predictable and easier to maintain.
285-293: Context-aware UI with simplified routing.The conditional rendering of the edit link based on the
readOnlystate shows good attention to UX details. The simplified URL structure also aligns with the PR's objective of cleaning up routes.src/pages/Encounters/EncounterShow.tsx (4)
46-46: Good update making facilityId optionalMaking the
facilityIdprop optional aligns with the PR objectives of supporting both facility and organization contexts. This allows the component to be used in contexts where a facility ID might not be available.
65-71: Good implementation of conditional query parametersThis is a clean implementation that conditionally includes either facility or patient query parameters based on the presence of a facilityId. This change supports the PR's goal of improving handling of facility and organization contexts.
80-84: Verified removal of facilityId from encounterTabPropsThe facilityId has been correctly removed from the encounterTabProps object, which is consistent with the removal of this property from the EncounterTabProps interface mentioned in the AI summary.
175-175:❓ Verification inconclusive
Route simplification looks good
Simplifying the href to just use the tab value is a cleaner approach. This appears to be part of the broader routing cleanup mentioned in the PR objectives.
🏁 Script executed:
#!/bin/bash # Let's check how the simplified route is used elsewhere in the codebase rg -C 3 'href={`\${tab}`}' --type tsx --type jsxLength of output: 155
I've updated the review comment to note that the route simplification is sound but that the original regex search failed due to a syntax issue. To double‑check usage across the project, please run a fixed-string search (e.g., using
rg -F) to confirm consistent use of the simplified route. In the meantime, the approved change insrc/pages/Encounters/EncounterShow.tsx(line 175) appears to follow the intended routing cleanup.Action: Verify that the simplified reference
href={`${tab}`}is used consistently in similar contexts.
| href={ | ||
| facilityId | ||
| ? `/facility/${facilityId}/patient/${encounter.patient.id}/encounter/${encounter.id}/updates` | ||
| : `/organization/organizationId/patient/${encounter.patient.id}/encounter/${encounter.id}/updates` | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Check the fallback URL logic.
The conditional URL generation is a good enhancement, but there's a potential issue with the fallback case.
When facilityId is not provided, the URL contains a hardcoded string "organizationId" which appears to be a placeholder. Is this intentional, or should it be dynamically populated with the actual organization ID?
🏁 Script executed:
#!/bin/bash
# Check if "organizationId" is used as a placeholder in other components
rg "organizationId" --type tsx --type jsxLength of output: 69
Please run the above command to check if "organizationId" is used elsewhere in the codebase as a placeholder. Once confirmed, verify if the fallback URL in src/components/Facility/EncounterCard.tsx (lines 122-126) is meant to use a static string or if it should dynamically include the actual organization ID.
Action Required: Verify Fallback URL Logic
- In the fallback case (when
facilityIdis not provided), the URL currently uses the literal"organizationId". - Confirm whether this is an intentional placeholder or if it requires dynamic population with the actual organization ID.
- Please review the usage (if any) of
"organizationId"in the codebase to ensure consistency and correctness in URL generation.
🏁 Script executed:
#!/bin/bash
# Searching for "organizationId" in all files to verify if it's used as a placeholder elsewhere
rg "organizationId"Length of output: 6866
Action Required: Update Fallback URL Logic in EncounterCard
The current fallback URL in src/components/Facility/EncounterCard.tsx (lines 122–126) is using the literal "organizationId" instead of a dynamic value. Given that other parts of the codebase (for example, in src/Utils/request/api.tsx and various components) make use of a dynamically provided organizationId, this appears to be an oversight.
- Update Required: Replace the hardcoded
"organizationId"string in the fallback case with the appropriate dynamic organization ID. - Verification: Ensure that the dynamic organization ID is correctly obtained from the relevant context or props, so the generated URL reflects the actual organization rather than a placeholder.
|
@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! 🙌 |
This pull request includes multiple changes to enhance the routing and patient details functionality in the application. The key updates involve modifications to the routing logic, adjustments to patient details tabs, and improvements to the handling of facility and organization contexts.
Routing and Navigation Updates:
src/Routers/AppRouter.tsx: Corrected the logic to determine thesidebarForvalue, prioritizingadminPagesoverappPages.src/Routers/routes/ConsultationRoutes.tsx: Refactored the route definitions to dynamically handle bothfacilityandorganizationcontexts.Patient Details Tabs:
src/Routers/routes/PatientRoutes.tsx: ConsolidatedfacilityPatientTabsandpatientTabsinto a singlepatientTabsimport and updated the route definitions accordingly [1] [2].src/components/Patient/PatientDetailsTab/index.tsx: RemovedfacilityPatientTabsand adjusted thepatientTabsto include all necessary components [1] [2].Component Enhancements:
src/components/Facility/EncounterCard.tsx: Added an optionalfacilityIdprop to handle facility-specific URLs [1] [2].src/components/Patient/PatientDetailsTab/Appointments.tsx: Enhanced the appointments query to handle both facility and organization contexts and added a loading state [1] [2] [3] [4].src/components/Patient/PatientDetailsTab/EncounterHistory.tsx: UpdatedEncounterCardusage to include thefacilityIdprop.Other Changes:
src/components/Patient/PatientDetailsTab/patientUpdates.tsx: Replaced the custom updates list with theQuestionnaireResponsesListcomponent for consistency.src/components/Patient/PatientHome.tsx: Simplified the tab selection logic by using a singlepatientTabsarray [1] [2] [3].src/components/Patient/allergy/list.tsx: Added areadOnlyprop to theAllergyListcomponent to support read-only views [1] [2].@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Refactor