Fixes certain users from being unable to view an encounter from organizations patients #10688
Conversation
…related navigates to include patient ID
WalkthroughThis pull request introduces a new localization entry for "view_encounter" and updates multiple routes and URL constructions across the application to include a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NavigationComponent
participant Router
participant EncounterShow
User->>NavigationComponent: Trigger action (e.g., view encounter)
NavigationComponent->>Router: Construct URL with facilityId, patientId, encounterId, and tab (subPage optional)
Router->>EncounterShow: Forward request with parameters including patientId
EncounterShow-->>User: Render encounter details in context
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
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
|
| Latest commit: |
0b1b598
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8306ca9f.care-fe.pages.dev |
| Branch Preview URL: | https://rithviknishad-fix-make-encou.care-fe.pages.dev |
CARE
|
||||||||||||||||||||||||||||
| Project |
CARE
|
| Branch Review |
rithviknishad/fix/make-encounter-routes-always-subroute-of-patient
|
| Run status |
|
| Run duration | 06m 46s |
| Commit |
|
| Committer | Rithvik Nishad |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
11
|
| View all changes introduced in this branch ↗︎ | |
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Routers/routes/ConsultationRoutes.tsx (1)
9-86: Well-structured routing changes.The routing changes look comprehensive and consistent. The new structure ensures that patient context is maintained throughout the encounter-related routes, which should fix the reported issue where users couldn't view encounters.
Consider documenting these routing patterns in your team's documentation to maintain consistency in future route additions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Routers/routes/ConsultationRoutes.tsx(1 hunks)src/pages/Encounters/EncounterShow.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/Encounters/EncounterShow.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
🔇 Additional comments (2)
src/Routers/routes/ConsultationRoutes.tsx (2)
43-65:✅ Verification successful
Verify patientId handling in EncounterShow component.
The changes look good and align with the PR objectives. To ensure the fix works as intended, let's verify the EncounterShow component's implementation.
🏁 Script executed:
#!/bin/bash # Check EncounterShow implementation ast-grep --pattern 'interface $_ { $$$ patientId: $_ $$$ }'Length of output: 18915
EncounterShow Component Validated: Patient ID Handling Confirmed
The updated encounter routes correctly pass the patientId prop. Verification of the EncounterShow component (see
src/pages/Encounters/EncounterShow.tsx, lines 44–50) confirms that its Props interface properly includespatientIdas a string. The changes align perfectly with the PR objectives.
- The interface in
EncounterShow.tsxconfirmspatientIdis required and correctly typed.- The updated routes in
ConsultationRoutes.tsxpass the patientId to EncounterShow as expected.
39-42:✅ Verification successful
Verify props passed to QuestionnaireResponseView.
The route provides
facilityIdandencounterIdbut these aren't passed to the component. This might be related to the reported crash in the questionnaire page.Let's check the component's prop requirements:
🏁 Script executed:
#!/bin/bash # Check QuestionnaireResponseView implementation ast-grep --pattern 'interface $_ { $$$ responseId: $_ patientId: $_ $$$ }'Length of output: 84
I'll run a new script to search for the definition of
QuestionnaireResponseViewto check its expected props more directly. This should help confirm whether it actually requiresfacilityIdandencounterId.
🏁 Script executed:
#!/bin/bash # Search for the function definition of QuestionnaireResponseView and show context lines for better insight into its props rg --max-columns=200 -A 5 -B 5 "QuestionnaireResponseView"Length of output: 2704
Props verification successful: QuestionnaireResponseView receives only the required props.
- The component is defined to accept only
responseIdandpatientId, which matches the props passed from the route.- Although the URL includes additional parameters (
facilityIdandencounterId), these are intended for routing and not required by the component, so their omission from the props is correct.- No changes are necessary for this route. If future requirements call for these additional parameters in the component, the props definition should be updated accordingly.
|
LGTM, questionnaire creation via both org and facility both looks good |
| @@ -64,6 +65,7 @@ export const EncounterShow = (props: Props) => { | |||
| pathParams: { id: encounterId }, | |||
| queryParams: { | |||
| facility: facilityId, | |||
| patient: patientId, | |||
There was a problem hiding this comment.
Adding the note here so we don't forget:
- we will need to modify BE as adding patientId here bypasses current permissions.
- currently when patientId is present, it's checking against patient permissions
- if not, it checks if facilityId is present (and against encounter permissions).
|
@rithviknishad 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! 🙌 |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit