[ENG-485] feat: refactor ReportViewer and associated routes to use associatingId and reportType for improved report handling#16440
[ENG-485] feat: refactor ReportViewer and associated routes to use associatingId and reportType for improved report handling#16440abhimanyurajeesh wants to merge 8 commits into
Conversation
…d and reportType for improved report handling
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (2)
WalkthroughReportViewer is refactored to generalize from encounter-specific to context-generic by accepting ChangesReport Viewer Generalization and Context-Aware Routes
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR generalises
Confidence Score: 5/5Safe to merge — the generalisation is mechanically consistent across all three report contexts, URL construction matches the new routes, and the loading-state guard fix is correct. The rename from encounterId to associatingId is applied uniformly in routes, components, query keys, and API call bodies. The three report-type paths (encounter, patient, account) each have matching route definitions and correct URL construction in getReportBasePath. The only open items are a dev-time diagnostic gap when basePath resolution silently discards templates and a pre-existing blob-URL cleanup pattern now duplicated in ReportViewer — neither affects correctness in production. ReportSubTab.tsx — the GenerateReportDropdown silently shows nothing if getReportBasePath returns null for every template (e.g., missing patientId at a PATIENT_SUMMARY call site), which could be confusing during integration of future report types. Important Files Changed
Reviews (3): Last reviewed commit: "refactor: remove unused encounterId and ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Refactors report viewing/generation flows to key off a generic associatingId plus explicit ReportType, enabling consistent routing and filtering for encounter, patient, and billing-account reports.
Changes:
- Updated ReportViewer API to use
associatingIdandreportType, and updated encounter/patient/account report routes accordingly. - Replaced the template sheet-based “generate report” UI with a template-driven dropdown in report listings and the account action menu.
- Removed the PDF print action from the generic file preview dialog.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Routers/routes/PatientRoutes.tsx | Adds patient report routes that render ReportViewer with associatingId and ReportType.PATIENT_SUMMARY. |
| src/Routers/routes/FacilityRoutes.tsx | Adds billing account report routes that render ReportViewer with ReportType.ACCOUNT_REPORT. |
| src/Routers/routes/ConsultationRoutes.tsx | Updates encounter report routes to pass associatingId into ReportViewer. |
| src/pages/Facility/billing/account/AccountShow.tsx | Adds template-driven report links to the account actions dropdown; wires ReportSubTab with billing context. |
| src/pages/Encounters/TemplateBuilder/TemplateReportSheet.tsx | Removes the old template sheet UI for report generation. |
| src/pages/Encounters/ReportViewer.tsx | Switches to associatingId/reportType for listing/creating reports and refines loading behavior. |
| src/components/ui/multi-filter/utils/navigation-helper.tsx | Adds hideRightArrow option for the navigation helper UI. |
| src/components/Files/ReportSubTab.tsx | Adds route-aware navigation for report preview and a template dropdown for “Generate report”. |
| src/components/Common/FilePreviewDialog.tsx | Removes the PDF print button from the generic file preview header actions. |
Comments suppressed due to low confidence (1)
src/components/Common/FilePreviewDialog.tsx:252
- This removes the PDF print action from the generic file preview dialog (added previously for PDF previews). That’s a functional regression for users previewing PDFs outside of the dedicated ReportViewer, and it’s not mentioned in this PR’s scope/description.
{file_state.extension === "pdf" && fileUrl && (
<Button
variant="outline"
onClick={() => {
window.open(fileUrl, "_blank", "noopener,noreferrer");
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pages/Facility/billing/account/AccountShow.tsx`:
- Around line 173-183: Gate the templates query and the "Reports" dropdown UI
behind a template-read permission check: add a permission boolean (e.g.,
canReadTemplates via your existing permission hook) and change the useQuery
enabled flag to enabled: dropdownOpen && canReadTemplates so
query(templateApi.listTemplates, ...) only runs when allowed, and hide or
disable the "Reports" dropdown section (the UI that uses templatesData and
isLoadingTemplates) when canReadTemplates is false; apply the same pattern for
the other occurrence around ReportType.ACCOUNT_REPORT and the block at lines
~489-506.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d7a80a59-5216-4b5e-812b-5cb4e7414066
📒 Files selected for processing (9)
src/Routers/routes/ConsultationRoutes.tsxsrc/Routers/routes/FacilityRoutes.tsxsrc/Routers/routes/PatientRoutes.tsxsrc/components/Common/FilePreviewDialog.tsxsrc/components/Files/ReportSubTab.tsxsrc/components/ui/multi-filter/utils/navigation-helper.tsxsrc/pages/Encounters/ReportViewer.tsxsrc/pages/Encounters/TemplateBuilder/TemplateReportSheet.tsxsrc/pages/Facility/billing/account/AccountShow.tsx
💤 Files with no reviewable changes (2)
- src/pages/Encounters/TemplateBuilder/TemplateReportSheet.tsx
- src/components/Common/FilePreviewDialog.tsx
🎭 Playwright Test ResultsStatus: ✅ Passed
📊 Detailed results are available in the playwright-final-report artifact. Run: #9621 |
Deploying care-preview with
|
| Latest commit: |
35b2dc2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://49489c55.care-preview-a7w.pages.dev |
| Branch Preview URL: | https://eng-485.care-preview-a7w.pages.dev |
| `/facility/${facilityId}/billing/account/${billingAccountId}/report/${report.id}`, | ||
| ); | ||
| } else if (canNavigateToPatientPreview) { | ||
| const effectiveFacilityId = facilityId ?? facility?.id; |
There was a problem hiding this comment.
we can move defining effectiveFacilityId to 116 and use it in other blocks as well; allows to remove the else block in 130.
There was a problem hiding this comment.
replacing with switch
| facilityId?: string; | ||
| patientId?: string; | ||
| encounterId?: string; | ||
| billingAccountId?: string; |
There was a problem hiding this comment.
Associating Id is encounterId, patientId or accountId based on report type, so we can derive the url based on that.
Remove patientId, encounterId and billingAccountId props.
There was a problem hiding this comment.
cant remove patient id, can remove the encounter id and billingAccountID
| </DropdownMenuItem> | ||
| {canListTemplates && isLoadingTemplates && ( | ||
| <DropdownMenuItem disabled> | ||
| <Loader className="size-3 animate-spin" /> |
There was a problem hiding this comment.
this loading state is a bit weird since we are displaying it after the other dropdown items @nihal467
Ideally we should be moving this up.
There was a problem hiding this comment.
it looks okay to me, we shouldn't block the other for this to load
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…ilesTab and AccountShow components
yash-learner
left a comment
yash-learner
left a comment
There was a problem hiding this comment.
This is outdated with new approach, I think we can cleanup
care_fe/src/types/emr/report/reportApi.ts
Line 22 in 35b2dc2
Proposed Changes
Fixes ENG-485
This is a follow up PR #16418 (review)
All report view is moved to this
Replaced the generate sheet with new dropdown
A new dropdown has been added for generating reports in account
@nihal467 Need to test all the report view and generate flow
Tagging: @ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Refactor