-
Notifications
You must be signed in to change notification settings - Fork 532
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 Detail Tabs: Org List Access #10121
base: develop
Are you sure you want to change the base?
Patient Detail Tabs: Org List Access #10121
Conversation
WalkthroughThis pull request introduces changes across multiple files to enhance patient routing and tab management. The modifications include updating localization files, adjusting patient routes, and modifying components to support more flexible patient and facility-specific navigation. The changes primarily focus on improving the handling of patient-related routes and tabs, with a particular emphasis on conditionally managing routes based on facility context. Changes
Sequence DiagramsequenceDiagram
participant User
participant PatientRoutes
participant PatientHome
participant PatientRegistration
User->>PatientRoutes: Navigate to patient page
PatientRoutes->>PatientHome: Render with tabs
alt Facility Context Exists
PatientHome->>PatientHome: Use facilityPatientTabs
else No Facility Context
PatientHome->>PatientHome: Use patientTabs
end
User->>PatientHome: Select tab/action
alt Edit Patient
PatientHome->>PatientRegistration: Open registration with optional facilityId
end
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. |
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
public/locale/en.json
(1 hunks)src/Routers/routes/PatientRoutes.tsx
(2 hunks)src/components/Patient/PatientDetailsTab/Demography.tsx
(1 hunks)src/components/Patient/PatientDetailsTab/index.tsx
(2 hunks)src/components/Patient/PatientHome.tsx
(5 hunks)src/components/Patient/PatientRegistration.tsx
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (15)
src/components/Patient/PatientDetailsTab/index.tsx (3)
Line range hint
19-49
: LGTM! Well-structured common tabs configuration.The separation of common tabs into a dedicated array improves code organization and maintainability.
59-59
: LGTM! Clean separation of patient-only tabs.The spread operator effectively creates a separate array for patient-only context.
50-58
: Verify tab order in facilityPatientTabs.The implementation correctly places the appointments tab after demography. However, let's verify this is the intended order for facility context.
✅ Verification successful
Tab order in facilityPatientTabs is logically structured
The current order (demography → appointments → encounters → health-profile) follows a natural progression from basic patient information to facility-specific interactions to clinical data. This structure aligns with good UX principles and maintains a logical flow of information.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if there are any UI tests or documentation specifying the expected tab order rg -A 5 "tab.*order" --type=md rg -A 5 "tab.*order" --type=testLength of output: 96
Script:
#!/bin/bash # Find commonTabs definition and related test/story files echo "=== Searching for commonTabs definition ===" rg -p "const commonTabs.*=" --type=typescript -A 10 echo -e "\n=== Looking for PatientDetailsTab related files ===" fd -e tsx -e ts "PatientDetailsTab.*\.(test|spec|stories)\." echo -e "\n=== Searching for tab-related patterns in tsx files ===" rg "tabs.*=.*\[" --type=typescript -A 5Length of output: 571
Script:
#!/bin/bash # Search with correct file extensions and broader patterns echo "=== Searching for commonTabs definition ===" rg -p "const commonTabs" -g "*.{ts,tsx}" -A 10 echo -e "\n=== Looking for tab configurations ===" rg "tabs.*=.*\[" -g "*.{ts,tsx}" -A 5 echo -e "\n=== Looking for PatientDetailsTab files ===" fd . -e ts -e tsx | rg -i "patientdetailstab"Length of output: 2152
src/Routers/routes/PatientRoutes.tsx (4)
2-5
: LGTM! Clean import organization.The imports correctly include both tab configurations needed for routing.
25-25
: LGTM! New route supports direct patient access.The new update route correctly handles the patient-only context.
26-31
: LGTM! Patient routes use appropriate tabs.The route generation correctly uses patientTabs for direct patient access.
Line range hint
38-44
: LGTM! Facility routes use facility-specific tabs.The route generation correctly uses facilityPatientTabs for facility context.
src/components/Patient/PatientHome.tsx (5)
12-15
: LGTM! Clean import organization.The imports correctly include both tab configurations needed for rendering.
26-26
: LGTM! Type safety for page prop.The union type correctly supports routes from both tab configurations.
46-46
: LGTM! Clean conditional tab selection.The ternary operator effectively selects the appropriate tab set based on context.
59-67
: LGTM! Appointment scheduling only in facility context.The conditional rendering aligns with PR objectives by showing appointment scheduling only in facility context.
109-116
: LGTM! Context-aware link generation.The conditional href generation correctly handles both facility and direct patient contexts.
src/components/Patient/PatientDetailsTab/Demography.tsx (1)
40-46
: LGTM! Context-aware navigation in edit handler.The conditional navigation correctly handles both facility and direct patient contexts.
src/components/Patient/PatientRegistration.tsx (1)
63-63
: LGTM! Interface change aligns with PR objectives.Making
facilityId
optional allows the component to handle direct patient access routes without facility context.public/locale/en.json (1)
1455-1455
: LGTM! Translation key addition is clear and follows conventions.The new translation key "no_updates_found" with its value is well-named and provides clear user feedback.
if (facilityId) { | ||
createPatient({ | ||
...values, | ||
facility: facilityId, | ||
ward_old: undefined, | ||
}); | ||
} |
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.
🛠️ Refactor suggestion
Add error handling for missing facilityId case.
The code correctly checks for facilityId before creating a patient, but doesn't handle the case when facilityId is undefined. Consider adding error feedback for users.
if (facilityId) {
createPatient({
...values,
facility: facilityId,
ward_old: undefined,
});
+} else {
+ toast.error(t("facility_required_for_patient_creation"));
}
Committable suggestion skipped: line range outside the PR's diff.
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes