Treatment Summary UI #10447
Treatment Summary UI #10447
Conversation
…encounter-overview-design
…encounter-overview-design
…encounter-overview-design
…encounter-overview-design
…encounter-overview-design
…encounter-overview-design
…encounter-overview-design
…work/care_fe into encounter-overview-design
…work/care_fe into treatment-summary-ui
…work/care_fe into treatment-summary-ui
…treatment-summary-ui
…treatment-summary-ui
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
…treatment-summary-ui
…ub.com/ohcnetwork/care_fe into treatment-summary-ui
…ub.com/ohcnetwork/care_fe into treatment-summary-ui
…ub.com/ohcnetwork/care_fe into treatment-summary-ui
…ub.com/ohcnetwork/care_fe into treatment-summary-ui
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/Patient/TreatmentSummary.tsx (1)
30-36: Add error handling for encounter query.The query lacks error handling, which could lead to silent failures.
Apply this diff to add error handling:
const { data: encounter } = useQuery<Encounter>({ + onError: (error) => { + console.error("Failed to fetch encounter:", error); + // Consider showing an error toast or inline error message + }, queryKey: ["encounter", encounterId], queryFn: query(api.encounter.get, {
🧹 Nitpick comments (9)
src/components/Patient/TreatmentSummary.tsx (3)
71-187: Extract repeated grid structure into a reusable component.The patient details section has multiple instances of the same grid structure. Consider extracting this into a reusable component to improve maintainability and reduce code duplication.
Create a new component like this:
+interface DetailRowProps { + label: string; + value: string | null | undefined; +} + +const DetailRow = ({ label, value }: DetailRowProps) => ( + <div className="grid grid-cols-[8rem,0.25rem,1fr] items-center"> + <span className="text-gray-600">{label}</span> + <span className="text-gray-600">:</span> + <span className="font-semibold">{value}</span> + </div> +); {/* Patient Details */} <div className="grid grid-cols-2 gap-x-12 gap-y-6"> <div className="space-y-3"> - <div className="grid grid-cols-[8rem,0.25rem,1fr] items-center"> - <span className="text-gray-600">{t("patient")}</span> - <span className="text-gray-600">:</span> - <span className="font-semibold">{encounter.patient.name}</span> - </div> + <DetailRow label={t("patient")} value={encounter.patient.name} /> {/* Apply similar changes to other grid structures */} </div> </div>
189-224: Add error boundaries and make print preview configurable.The medical information section could benefit from the following improvements:
- Add error boundaries to prevent cascading failures
- Make
isPrintPreviewconfigurable through propsApply these changes:
+interface MedicalInformationProps { + isPrintPreview?: boolean; +} + +class MedicalInformationErrorBoundary extends React.Component { + // Add error boundary implementation +} + -<div className="space-y-6"> +<MedicalInformationErrorBoundary> + <div className="space-y-6"> {/* Allergies */} <AllergyList patientId={encounter.patient.id} encounterId={encounterId} className="border-none shadow-none" - isPrintPreview={true} + isPrintPreview={props.isPrintPreview} /> {/* Apply similar changes to other components */} </div> +</MedicalInformationErrorBoundary>
246-250: Use Tailwind's built-in text size classes.Instead of using a hardcoded text size, consider using Tailwind's built-in text size classes for better responsiveness.
Apply this change:
-<div className="mt-8 space-y-1 pt-2 text-[10px] text-gray-500 flex justify-between"> +<div className="mt-8 space-y-1 pt-2 text-xs text-gray-500 flex justify-between">src/components/Medicine/MedicationsTable.tsx (2)
48-54: Loading state message may be confusing.
Using a “no medications found” message during loading could confuse users. Consider distinguishing between a “Loading…” message and a truly empty state.- if (isLoading) { - return ( - <div> - {t("no_medications_found_for_this_encounter")} - </div> - ); - } + if (isLoading) { + return <div>{t("loading_medications")}</div>; + }
75-75: Consider empty array handling.
Ifmedications?.resultsis empty, no rows will be rendered. Provide a clear “No data” row or fallback if desired.src/Utils/request/query.ts (1)
123-186: Potential performance concern with loading all pages.
For large datasets, storing all pages in memory may affect performance. Consider adding a user-facing limit or streaming approach.while (hasNextPage) { ... + // Optionally, short-circuit if items exceed certain threshold + if (items.length > SOME_REASONABLE_LIMIT) { + break; + } }src/pages/Encounters/PrintPrescription.tsx (2)
64-65: Check disabled logic.
disabled={!medications}may not account for loading or errors distinctly. Consider adding explicit handling for each state.
91-91: Safe optional chaining for patient data.
No issues in referencingencounter?.patient. Consider a placeholder ifnameorphone_numberis absent.Also applies to: 115-115
src/components/Medicine/MedicationRequestTable/index.tsx (1)
67-93: Consider optimizing data fetching strategy.The component makes two separate queries for active and stopped medications. Consider combining these into a single query with a status filter to optimize network requests.
You could refactor the queries like this:
- const { data: activeMedications, isLoading: loadingActive } = useQuery({ - queryKey: ["medication_requests_active", patientId], - queryFn: query(medicationRequestApi.list, { - pathParams: { patientId: patientId }, - queryParams: { - encounter: encounterId, - limit: 100, - status: ["active", "on-hold", "draft", "unknown"].join(","), - }, - }), - enabled: !!patientId, - }); - - const { data: stoppedMedications, isLoading: loadingStopped } = useQuery({ - queryKey: ["medication_requests_stopped", patientId], + const { data: medications, isLoading } = useQuery({ + queryKey: ["medication_requests", patientId, showStopped], queryFn: query(medicationRequestApi.list, { pathParams: { patientId: patientId }, queryParams: { encounter: encounterId, limit: 100, - status: ["ended", "completed", "cancelled", "entered_in_error"].join( - ",", - ), + status: showStopped + ? ["active", "on-hold", "draft", "unknown", "ended", "completed", "cancelled", "entered_in_error"].join(",") + : ["active", "on-hold", "draft", "unknown"].join(","), }, }), enabled: !!patientId, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/Routers/routes/ConsultationRoutes.tsx(1 hunks)src/Utils/request/query.ts(2 hunks)src/Utils/request/types.ts(0 hunks)src/components/Medicine/MedicationRequestTable/index.tsx(2 hunks)src/components/Medicine/MedicationsTable.tsx(4 hunks)src/components/Patient/MedicationStatementList.tsx(4 hunks)src/components/Patient/TreatmentSummary.tsx(1 hunks)src/components/Patient/allergy/list.tsx(8 hunks)src/components/Patient/diagnosis/DiagnosisTable.tsx(2 hunks)src/components/Patient/symptoms/SymptomTable.tsx(2 hunks)src/pages/Encounters/PrintPrescription.tsx(6 hunks)
💤 Files with no reviewable changes (1)
- src/Utils/request/types.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/Patient/diagnosis/DiagnosisTable.tsx
- src/Routers/routes/ConsultationRoutes.tsx
- src/components/Patient/MedicationStatementList.tsx
- src/components/Patient/allergy/list.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (13)
src/components/Patient/symptoms/SymptomTable.tsx (3)
28-31: LGTM!The addition of the optional
isPrintPreviewproperty aligns with the PR objectives for treatment summary UI.
33-36: LGTM!The function signature update maintains backward compatibility with the default value of
falseforisPrintPreview.
107-132: LGTM!The conditional rendering logic for notes based on
isPrintPreviewis well-implemented and follows React best practices.src/components/Medicine/MedicationsTable.tsx (3)
1-1: Imports look good.
No concerns regarding the new imports foruseQuery,query, andmedicationRequestApi.Also applies to: 15-15, 21-21
38-39: Interface refactor acknowledged.
Switching from amedicationsarray topatientIdandencounterIdis consistent with the new data-fetching approach.
42-45: Updated function signature verified.
Ensure all references toMedicationsTablethroughout the codebase properly supplypatientIdandencounterId.src/Utils/request/query.ts (2)
3-4: Constant import acknowledged.
Make sure the defaultRESULTS_PER_PAGE_LIMITsuits typical use cases.
6-11: New types recognized.
AddingPaginatedResponsealigns with the pagination logic below.src/pages/Encounters/PrintPrescription.tsx (3)
20-20: Prop additions appear correct.
patientIdis now required. Confirm no call sites are missing this new prop.Also applies to: 22-22
34-34: Conditional query usage confirmed.
UsingqueryKey: ["medication_requests", patientId]andenabled: !!patientIdprevents unwanted requests ifpatientIdis missing.Also applies to: 36-36, 39-39
125-125: Props to MedicationsTable updated correctly.
PassingpatientIdandencounterIdmatches the revisedMedicationsTablesignature.src/components/Medicine/MedicationRequestTable/index.tsx (2)
175-177: LGTM! URL structure updated to include patient context.The updated URL structure now includes the patient context, maintaining consistency with RESTful patterns.
197-200: Verify MedicationsTable implementation with new props.The component now receives
patientIdandencounterIdinstead of themedicationsarray, suggesting that data fetching has been moved to the MedicationsTable component.Let's verify the MedicationsTable implementation:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if MedicationsTable properly implements the new props # Search for the MedicationsTable component definition ast-grep --pattern 'export const MedicationsTable = ({ patientId, encounterId }: $_) => $_' # Check for useQuery usage within MedicationsTable rg -A 10 'useQuery.*patientId.*encounterId' src/components/Medicine/MedicationsTable.tsxLength of output: 6983
MedicationsTable correctly switches to fetching medications via patientId and encounterId.
- The component accepts the new props as verified at lines 42-46 in
src/components/Medicine/MedicationsTable.tsx.- The
useQuerycall properly uses these props for querying data with a unique query key and fetching the medications list.
…treatment-summary-ui
…ub.com/ohcnetwork/care_fe into treatment-summary-ui
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/Facility/ConsultationDetails/QuickAccess.tsx (1)
48-49: Consider using CARE UI's standard heading component.The heading styles could be standardized using CARE UI's heading component if available, to maintain consistency across the application.
-<h3 className="text-lg font-medium text-gray-950 mb-1"> +<Heading level={3} className="mb-1"> {t("departments_and_teams")} +</Heading>src/Utils/request/query.ts (1)
150-187: Consider implementing memory management for large result sets.For large datasets, consider implementing memory management strategies:
- Stream results instead of accumulating them in memory
- Implement windowing or virtual scrolling in the UI
- Add a warning when result set size exceeds a threshold
Here's an example implementation with memory management:
const paginatedQuery = < Route extends ApiRoute<PaginatedResponse<unknown>, unknown>, >( route: Route, options?: ApiCallOptions<Route> & { pageSize?: number; maxPages?: number; + maxResults?: number; + onProgress?: (progress: { loaded: number; total: number }) => void; }, ) => { return async ({ signal }: { signal: AbortSignal }) => { const items: Route["TRes"]["results"] = []; let hasNextPage = true; let page = 0; let count = 0; const pageSize = options?.pageSize ?? RESULTS_PER_PAGE_LIMIT; + const maxResults = options?.maxResults ?? 1000; while (hasNextPage) { const res = await query(route, { ...options, queryParams: { limit: pageSize, offset: page * pageSize, ...options?.queryParams, }, })({ signal }); count = res.count; items.push(...res.results); + options?.onProgress?.({ + loaded: items.length, + total: count, + }); + + if (items.length >= maxResults) { + console.warn( + `Results truncated to ${maxResults} items to prevent memory issues`, + ); + hasNextPage = false; + } if (options?.maxPages && page >= options.maxPages - 1) { hasNextPage = false; } if (items.length >= res.count) { hasNextPage = false; } page++; } return { count, results: items, }; }; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
public/locale/en.json(6 hunks)src/Utils/request/query.ts(2 hunks)src/components/Facility/ConsultationDetails/QuestionnaireResponsesList.tsx(3 hunks)src/components/Facility/ConsultationDetails/QuickAccess.tsx(1 hunks)src/components/Patient/PatientInfoCard.tsx(1 hunks)src/pages/Encounters/tabs/EncounterUpdatesTab.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Patient/PatientInfoCard.tsx
- src/components/Facility/ConsultationDetails/QuestionnaireResponsesList.tsx
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
- GitHub Check: OSSAR-Scan
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (8)
src/pages/Encounters/tabs/EncounterUpdatesTab.tsx (2)
17-17: LGTM! Mobile-first layout improvement.The flex direction change improves mobile UX by showing the overview section first, while maintaining the side-by-side layout on larger screens.
58-58: LGTM! Optimized layout proportions.Reducing the width of the side overview gives more space to the main content area, which should improve readability and content display.
src/components/Facility/ConsultationDetails/QuickAccess.tsx (1)
15-17: Well-structured component with proper TypeScript and i18n implementation!The component follows best practices with:
- Clear TypeScript interface for props
- Proper use of i18n translations
- Clean functional component implementation
Also applies to: 19-21
public/locale/en.json (2)
336-337: Translation keys added for treatment summary feature.The new translation keys follow consistent naming patterns and provide necessary labels for the treatment summary UI.
Also applies to: 697-698, 917-918, 974-975, 1295-1296
1-2342: Well-organized translation file!The translation file maintains good practices:
- Alphabetically ordered keys for easy lookup
- Consistent snake_case naming
- Clear, descriptive translation values
src/Utils/request/query.ts (3)
3-11: LGTM!The new imports are well-organized and properly support the paginated query functionality.
124-143: LGTM!The documentation is comprehensive and includes clear examples of usage.
144-149: LGTM!The type safety is well-implemented with proper TypeScript generics and constraints.
| return async ({ signal }: { signal: AbortSignal }) => { | ||
| const items: Route["TRes"]["results"] = []; | ||
| let hasNextPage = true; | ||
| let page = 0; | ||
| let count = 0; | ||
|
|
||
| const pageSize = options?.pageSize ?? RESULTS_PER_PAGE_LIMIT; | ||
|
|
||
| while (hasNextPage) { | ||
| const res = await query(route, { | ||
| ...options, | ||
| queryParams: { | ||
| limit: pageSize, | ||
| offset: page * pageSize, | ||
| ...options?.queryParams, | ||
| }, | ||
| })({ signal }); | ||
|
|
||
| count = res.count; | ||
| items.push(...res.results); | ||
|
|
||
| if (options?.maxPages && page >= options.maxPages - 1) { | ||
| hasNextPage = false; | ||
| } | ||
|
|
||
| if (items.length >= res.count) { | ||
| hasNextPage = false; | ||
| } | ||
|
|
||
| page++; | ||
| } | ||
|
|
||
| return { | ||
| count, | ||
| results: items, | ||
| }; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and timeout mechanism.
The implementation should handle failed page requests and implement a timeout mechanism for long-running pagination operations.
Consider implementing these improvements:
const paginatedQuery = <
Route extends ApiRoute<PaginatedResponse<unknown>, unknown>,
>(
route: Route,
- options?: ApiCallOptions<Route> & { pageSize?: number; maxPages?: number },
+ options?: ApiCallOptions<Route> & {
+ pageSize?: number;
+ maxPages?: number;
+ timeoutMs?: number;
+ },
) => {
return async ({ signal }: { signal: AbortSignal }) => {
+ const timeoutId = options?.timeoutMs
+ ? setTimeout(() => signal.abort(), options.timeoutMs)
+ : null;
+
const items: Route["TRes"]["results"] = [];
let hasNextPage = true;
let page = 0;
let count = 0;
const pageSize = options?.pageSize ?? RESULTS_PER_PAGE_LIMIT;
- while (hasNextPage) {
- const res = await query(route, {
- ...options,
- queryParams: {
- limit: pageSize,
- offset: page * pageSize,
- ...options?.queryParams,
- },
- })({ signal });
+ try {
+ while (hasNextPage) {
+ try {
+ const res = await query(route, {
+ ...options,
+ queryParams: {
+ limit: pageSize,
+ offset: page * pageSize,
+ ...options?.queryParams,
+ },
+ })({ signal });
count = res.count;
items.push(...res.results);
if (options?.maxPages && page >= options.maxPages - 1) {
hasNextPage = false;
}
if (items.length >= res.count) {
hasNextPage = false;
}
page++;
+ } catch (error) {
+ if (error instanceof HTTPError) {
+ throw error;
+ }
+ // Retry the current page once before giving up
+ await sleep(1000);
+ continue;
+ }
+ }
+ } finally {
+ if (timeoutId !== null) {
+ clearTimeout(timeoutId);
+ }
+ }
return {
count,
results: items,
};
};
};📝 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.
| return async ({ signal }: { signal: AbortSignal }) => { | |
| const items: Route["TRes"]["results"] = []; | |
| let hasNextPage = true; | |
| let page = 0; | |
| let count = 0; | |
| const pageSize = options?.pageSize ?? RESULTS_PER_PAGE_LIMIT; | |
| while (hasNextPage) { | |
| const res = await query(route, { | |
| ...options, | |
| queryParams: { | |
| limit: pageSize, | |
| offset: page * pageSize, | |
| ...options?.queryParams, | |
| }, | |
| })({ signal }); | |
| count = res.count; | |
| items.push(...res.results); | |
| if (options?.maxPages && page >= options.maxPages - 1) { | |
| hasNextPage = false; | |
| } | |
| if (items.length >= res.count) { | |
| hasNextPage = false; | |
| } | |
| page++; | |
| } | |
| return { | |
| count, | |
| results: items, | |
| }; | |
| }; | |
| }; | |
| const paginatedQuery = < | |
| Route extends ApiRoute<PaginatedResponse<unknown>, unknown>, | |
| >( | |
| route: Route, | |
| options?: ApiCallOptions<Route> & { | |
| pageSize?: number; | |
| maxPages?: number; | |
| timeoutMs?: number; | |
| }, | |
| ) => { | |
| return async ({ signal }: { signal: AbortSignal }) => { | |
| const timeoutId = options?.timeoutMs | |
| ? setTimeout(() => signal.abort(), options.timeoutMs) | |
| : null; | |
| const items: Route["TRes"]["results"] = []; | |
| let hasNextPage = true; | |
| let page = 0; | |
| let count = 0; | |
| const pageSize = options?.pageSize ?? RESULTS_PER_PAGE_LIMIT; | |
| try { | |
| while (hasNextPage) { | |
| try { | |
| const res = await query(route, { | |
| ...options, | |
| queryParams: { | |
| limit: pageSize, | |
| offset: page * pageSize, | |
| ...options?.queryParams, | |
| }, | |
| })({ signal }); | |
| count = res.count; | |
| items.push(...res.results); | |
| if (options?.maxPages && page >= options.maxPages - 1) { | |
| hasNextPage = false; | |
| } | |
| if (items.length >= res.count) { | |
| hasNextPage = false; | |
| } | |
| page++; | |
| } catch (error) { | |
| if (error instanceof HTTPError) { | |
| throw error; | |
| } | |
| // Retry the current page once before giving up | |
| await sleep(1000); | |
| continue; | |
| } | |
| } | |
| } finally { | |
| if (timeoutId !== null) { | |
| clearTimeout(timeoutId); | |
| } | |
| } | |
| return { | |
| count, | |
| results: items, | |
| }; | |
| }; | |
| }; |
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
…treatment-summary-ui
Another issue will be made to simplify and remove as much colours as possible to reduce unnecessary ink usage while printing. Issue - #10480
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements