Discharge Summary #10570
Discharge Summary #10570
Conversation
WalkthroughThe changes integrate discharge summary functionality across both front-end and back-end components. New internationalization keys and updated text improve user messaging for discharge summaries. A new route with a sub-page parameter is added to support detailed encounter views. Additionally, an API endpoint to generate discharge summaries has been introduced, and several components have been updated to handle mutations, dynamic tab filtering, and enhanced dropdown navigation. Changes
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.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/Utils/request/api.tsx (1)
565-569: Consider enhancing the API endpoint definition.The endpoint is well-structured, but consider the following improvements:
- Add a request body type if any parameters are required
- Use a more descriptive response type name instead of the generic
detailfieldPlease verify:
- Are there any required parameters for generating the discharge summary?
- What information should be included in the response besides the status message?
✅ Verification successful
🌐 Web query:
What are the best practices for RESTful API response structures when generating documents?💡 Result:
When designing RESTful API response structures for document generation, key best practices focus on clarity, consistency, and developer usability. These principles ensure APIs are intuitive to consume while maintaining robustness across different scenarios.
HTTP Status Codes
Use semantic HTTP status codes to indicate request outcomes:
- 200 OK for successful GET requests returning data ([3][5])
- 201 Created after successful POST operations ([3][9])
- 400 Bad Request for client-side input errors ([5][7])
- 404 Not Found for missing resources ([7][9])
Data Structure
Adopt these patterns:
// Single resource response { "id": "doc_123", "title": "API Guide", "author": "J. Smith", "created_at": "2025-02-12T10:31:06Z" } // List response with pagination { "data": [ { /* document object */ }, { /* document object */ } ], "pagination": { "total": 100, "page": 5, "limit": 20 } }([3][4][9])
Error Handling
Provide actionable error details:
{ "error": { "code": "VALIDATION_ERROR", "message": "Invalid document format", "details": [ { "field": "title", "issue": "Maximum length exceeded (255 chars)" } ] } }([5][7][9])
Content Negotiation
Support multiple formats through headers:
Accept: application/json Content-Type: application/json; charset=utf-8([1][4][9])
Versioning
Implement through:
- URL path (
/v1/documents)- Custom headers (
X-API-Version: 1.0)
([1][4][5])Documentation Essentials
Include interactive examples using OpenAPI/Swagger with:
- Request/response schemas
- Authentication requirements
- Error code explanations
- Live testing capabilities ([2][8][10])
For document generation APIs specifically:
- Use template references in requests:
{ "template_id": "invoice_template", "data": { "customer_name": "Acme Corp", "items": [/*...*/] } }- Return generation metadata:
{ "document_id": "doc_987", "download_url": "/documents/doc_987.pdf", "generated_at": "2025-02-12T10:35:00Z" }([6][8][10])
Best practices evolve around making responses self-descriptive while maintaining strict adherence to REST principles. Implement consistent formatting across all endpoints and provide comprehensive documentation to ensure smooth integration experiences.
Citations:
- 1: https://daily.dev/blog/restful-api-design-best-practices-guide-2024
- 2: https://www.integrate.io/blog/how-to-document-your-rest-api-like-a-pro/
- 3: https://gist.github.com/igorjs/407ffc3126f6ef2a6fe8f918a0673b59
- 4: https://www.xcubelabs.com/blog/best-practices-for-designing-restful-apis/
- 5: https://www.getambassador.io/blog/7-rest-api-design-best-practices
- 6: https://unidoc.io/post/document-generation-api-guide/
- 7: https://swagger.io/resources/articles/best-practices-in-api-design/
- 8: https://blog.dreamfactory.com/a-complete-guide-to-api-generation
- 9: https://www.freecodecamp.org/news/rest-api-best-practices-rest-endpoint-design-examples/
- 10: https://stoplight.io/api-documentation-guide
Enhance API Endpoint Definition
- API Parameters: The current configuration relies solely on the URL parameter (
encounterId), and there’s no dedicated request body type. Verify if the discharge summary generation requires any additional data (e.g., options to customize the summary) beyond the encounter identifier.- Response Structure: Instead of returning just a generic
detailmessage, consider expanding the response to include specific metadata. Best practices for document-generating endpoints recommend returning details such as:
document_id– a unique identifier for the generated documentdownload_url– a URL from which the document can be retrievedgenerated_at– a timestamp indicating when the document was generatedThese enhancements would align your API with established RESTful best practices, providing clients with clear, actionable information.
src/components/Files/FilesTab.tsx (3)
89-94: Consider using TypeScript enums or const objects for file categories.While the implementation works, using TypeScript enums or const objects would provide better type safety and autocompletion.
- const fileCategories = [ - { value: "all", label: "All" }, - { value: "audio", label: "Audio" }, - { value: "xray", label: "X-Ray" }, - { value: "identity_proof", label: "Identity Proof" }, - { value: "unspecified", label: "Unspecified" }, - { value: "discharge_summary", label: "Discharge Summary" }, - ] as const; + const FILE_CATEGORIES = { + ALL: "all", + AUDIO: "audio", + XRAY: "xray", + IDENTITY_PROOF: "identity_proof", + UNSPECIFIED: "unspecified", + DISCHARGE_SUMMARY: "discharge_summary", + } as const; + + const fileCategories = [ + { value: FILE_CATEGORIES.ALL, label: "All" }, + { value: FILE_CATEGORIES.AUDIO, label: "Audio" }, + { value: FILE_CATEGORIES.XRAY, label: "X-Ray" }, + { value: FILE_CATEGORIES.IDENTITY_PROOF, label: "Identity Proof" }, + { value: FILE_CATEGORIES.UNSPECIFIED, label: "Unspecified" }, + { value: FILE_CATEGORIES.DISCHARGE_SUMMARY, label: "Discharge Summary" }, + ] as const;
653-671: Enhance tab navigation accessibility.While the tab navigation works, consider adding ARIA labels and roles for better screen reader support.
-<TabsList className="grid w-auto grid-cols-2 w-fit"> +<TabsList className="grid w-auto grid-cols-2 w-fit" aria-label="File categories"> <TabsTrigger value="all" asChild> <Link className="text-gray-600" href={`/facility/${encounter?.facility.id}/encounter/${encounter?.id}/files/all`} + role="tab" + aria-selected={subPage === "all"} > {t("all")} </Link> </TabsTrigger> <TabsTrigger value="discharge_summary" asChild> <Link className="text-gray-600" href={`/facility/${encounter?.facility.id}/encounter/${encounter?.id}/files/discharge_summary`} + role="tab" + aria-selected={subPage === "discharge_summary"} > {t("discharge_summary")} </Link> </TabsTrigger> </TabsList>
676-705: Enhance button keyboard accessibility.While the buttons work well, consider adding keyboard shortcuts for common actions.
<Button variant="outline_primary" className="flex flex-row items-center" onClick={() => queryClient.invalidateQueries({ queryKey: ["files"] })} + title={`${t("refresh")} (Ctrl+R)`} + onKeyDown={(e) => { + if (e.ctrlKey && e.key === 'r') { + e.preventDefault(); + queryClient.invalidateQueries({ queryKey: ["files"] }); + } + }} > <CareIcon icon="l-sync" /> <span className="ml-2">{t("refresh")}</span> </Button>public/locale/en.json (1)
954-954: Consider adding email validation message.While the email-related strings are clear, consider adding a validation message for invalid email format.
Add a new key for email validation:
+"invalid_email_format": "Please enter a valid email address format (e.g., example@email.com)",Also applies to: 978-978
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
public/locale/en.json(10 hunks)src/Routers/routes/ConsultationRoutes.tsx(1 hunks)src/Utils/request/api.tsx(1 hunks)src/components/Files/FilesTab.tsx(8 hunks)src/components/Patient/PatientInfoCard.tsx(2 hunks)src/pages/Encounters/EncounterShow.tsx(3 hunks)src/pages/Encounters/tabs/EncounterFilesTab.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (9)
src/pages/Encounters/tabs/EncounterFilesTab.tsx (1)
11-11: LGTM!The
subPageprop is correctly passed through to theFilesTabcomponent.src/Routers/routes/ConsultationRoutes.tsx (1)
34-46: LGTM!The new route is well-structured and correctly passes all required parameters to the
EncounterShowcomponent.src/pages/Encounters/EncounterShow.tsx (1)
29-29: LGTM!The
subPageprop is correctly:
- Typed as optional in both interfaces
- Destructured from props
- Passed down to child components
Also applies to: 48-48, 52-52, 79-79
src/components/Patient/PatientInfoCard.tsx (1)
448-472: LGTM! The discharge summary menu item is well integrated.The implementation correctly adds a discharge summary option to the dropdown menu, with proper internationalization and routing.
src/components/Files/FilesTab.tsx (2)
1-64: LGTM! Props and imports are well structured.The new imports and the optional
subPageprop are properly integrated to support the discharge summary functionality.
113-123: LGTM! Query parameters are properly integrated.The subPage parameter is correctly added to both the query key and filter parameters.
public/locale/en.json (3)
736-740: LGTM! Clear and consistent discharge summary related messages.The new keys provide clear messaging for discharge summary functionality:
- Consistent capitalization in "Discharge Summary" across all messages
- Clear error and status messages
- Informative disclaimer about computer-generated content
Also applies to: 747-747
773-773: LGTM! Improved capitalization consistency.The modification of "download_discharge_summary" value from "Download discharge summary" to "Download Discharge Summary" improves consistency with other discharge summary related strings.
1078-1079: LGTM! Clear action button labels.The action-related strings are clear and follow consistent capitalization:
- "Generate" as a generic action
- "Generate Discharge Summary" and "Regenerate Discharge Summary" as specific actions
Also applies to: 1762-1762
…discharge-summary-ui
Deploying care-fe with
|
| Latest commit: |
0d2c430
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ad691c29.care-fe.pages.dev |
| Branch Preview URL: | https://discharge-summary-ui.care-fe.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Utils/utils.ts (1)
272-287: LGTM! The generic implementation improves reusability.The new
stringifyNestedObjectfunction is well-designed with:
- Type-safe generic constraints
- Customizable separator
- Proper handling of optional parent property
Consider adding JSDoc documentation to describe the function's purpose and parameters:
+/** + * Converts a nested object hierarchy into a string representation. + * @param obj - The object to stringify, must have a name property and optional parent + * @param separator - The separator to use between levels, defaults to ", " + * @returns A string representation of the object hierarchy + */ export const stringifyNestedObject = < T extends { name: string; parent?: Partial<T> }, >( obj: T, separator = ", ", ) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/Utils/utils.ts(1 hunks)src/components/Facility/ConsultationDetails/QuickAccess.tsx(2 hunks)src/components/Location/LocationHistorySheet.tsx(1 hunks)src/components/Location/LocationTree.tsx(1 hunks)src/pages/Appointments/AppointmentDetail.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/components/Location/LocationHistorySheet.tsx
- src/components/Location/LocationTree.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (2)
src/components/Facility/ConsultationDetails/QuickAccess.tsx (1)
89-89: LGTM! The change improves consistency in handling nested objects.The switch to
stringifyNestedObjectaligns with the codebase's standardized approach for displaying hierarchical organization data.src/pages/Appointments/AppointmentDetail.tsx (1)
324-324: LGTM! The change maintains consistency across the codebase.The switch to
stringifyNestedObjectaligns with the standardized approach for displaying hierarchical data.
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
…discharge-summary-ui
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/Utils/utils.ts (1)
236-251: Add safety checks and documentation to the generic utility.The implementation is well-structured and more reusable than the previous organization-specific function. Consider these improvements:
+/** + * Converts a nested object structure into a string representation by traversing its parent chain. + * Each object in the chain must have a name property and may have a parent reference. + * + * @example + * const obj = { name: "Child", parent: { name: "Parent" } }; + * stringifyNestedObject(obj); // "Child, Parent" + * stringifyNestedObject(obj, " > "); // "Child > Parent" + * + * @param obj - The object to stringify + * @param separator - Optional separator between names (default: ", ") + * @returns A string representation of the object hierarchy + */ export const stringifyNestedObject = < T extends { name: string; parent?: Partial<T> }, >( obj: T, separator = ", ", ) => { + if (!obj) return ""; + const levels: string[] = []; + const seen = new Set<Partial<T>>(); let current: Partial<T> | undefined = obj; - while (current?.name) { + while (current?.name && !seen.has(current)) { levels.push(current.name); + seen.add(current); current = current.parent; } return levels.join(separator); };The improvements include:
- Added JSDoc documentation with examples
- Added protection against circular references using a Set
- Added null/undefined check for input object
src/components/Files/FilesTab.tsx (3)
64-64: Enhance type safety for the subPage prop.Consider using a union type of valid page values instead of a string type to prevent potential runtime errors.
- subPage?: string; + subPage?: "all" | "discharge_summary" | "xray" | "identity_proof" | "unspecified";
674-707: Remove unnecessary fragments.The fragments wrapping the refresh and generate buttons are redundant as they each contain only one child component.
- {subPage === "discharge_summary" && ( - <> - <Button - variant="outline_primary" - className="flex flex-row items-center" - onClick={async () => { - await queryClient.invalidateQueries({ - queryKey: ["files"], - }); - toast.success(t("refreshed")); - }} - > - <CareIcon icon="l-sync" /> - <span className="ml-2">{t("refresh")}</span> - </Button> - </> - )} - {subPage === "discharge_summary" && ( - <> - <Button - variant="primary" - className="flex flex-row items-center" - onClick={() => generateDischargeSummary()} - disabled={isGenerating} - > - <CareIcon icon="l-file-medical" className="hidden md:block" /> - <span> - {isGenerating - ? t("generating") - : t("generate_discharge_summary")} - </span> - </Button> - </> - )} + {subPage === "discharge_summary" && ( + <Button + variant="outline_primary" + className="flex flex-row items-center" + onClick={async () => { + await queryClient.invalidateQueries({ + queryKey: ["files"], + }); + toast.success(t("refreshed")); + }} + > + <CareIcon icon="l-sync" /> + <span className="ml-2">{t("refresh")}</span> + </Button> + )} + {subPage === "discharge_summary" && ( + <Button + variant="primary" + className="flex flex-row items-center" + onClick={() => generateDischargeSummary()} + disabled={isGenerating} + > + <CareIcon icon="l-file-medical" className="hidden md:block" /> + <span> + {isGenerating + ? t("generating") + : t("generate_discharge_summary")} + </span> + </Button> + )}🧰 Tools
🪛 Biome (1.9.4)
[error] 675-689: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
[error] 692-706: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
654-671: Consider making tab navigation more dynamic.The current implementation hardcodes tabs for "all" and "discharge_summary". Consider generating tabs dynamically from the fileCategories array to make it more maintainable.
- <TabsList className="grid w-auto grid-cols-2 w-fit"> - <TabsTrigger value="all" asChild> - <Link - className="text-gray-600" - href={`/facility/${encounter?.facility.id}/encounter/${encounter?.id}/files/all`} - > - {t("all")} - </Link> - </TabsTrigger> - <TabsTrigger value="discharge_summary" asChild> - <Link - className="text-gray-600" - href={`/facility/${encounter?.facility.id}/encounter/${encounter?.id}/files/discharge_summary`} - > - {t("discharge_summary")} - </Link> - </TabsTrigger> - </TabsList> + <TabsList className={`grid w-auto grid-cols-${fileCategories.length} w-fit`}> + {fileCategories.map((category) => ( + <TabsTrigger key={category.value} value={category.value} asChild> + <Link + className="text-gray-600" + href={`/facility/${encounter?.facility.id}/encounter/${encounter?.id}/files/${category.value}`} + > + {t(category.value)} + </Link> + </TabsTrigger> + ))} + </TabsList>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
public/locale/en.json(10 hunks)src/Routers/routes/ConsultationRoutes.tsx(1 hunks)src/Utils/request/api.tsx(1 hunks)src/Utils/utils.ts(1 hunks)src/components/Files/FilesTab.tsx(8 hunks)src/components/Patient/PatientInfoCard.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Routers/routes/ConsultationRoutes.tsx
- src/components/Patient/PatientInfoCard.tsx
- src/Utils/request/api.tsx
- public/locale/en.json
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Files/FilesTab.tsx
[error] 675-689: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
[error] 692-706: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Test
- GitHub Check: lint
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
- GitHub Check: OSSAR-Scan
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (3)
src/Utils/utils.ts (1)
229-234: LGTM! Well-structured generic utility function.The function follows the same pattern as
conditionalAttributebut for arrays, with good type safety through generics.src/components/Files/FilesTab.tsx (2)
97-106: Add error handling to the discharge summary mutation.The mutation handles success but lacks error handling. Consider adding an
onErrorhandler to show error messages to users.
113-124: LGTM! Query implementation is well-structured.The query implementation correctly handles the subPage parameter and follows React Query best practices.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/Files/FilesTab.tsx (2)
653-671: Consider adding loading state to tab navigation.The tab navigation could benefit from a loading state to prevent jarring UI updates during navigation.
Consider adding a loading state:
- <Tabs defaultValue={subPage}> + <Tabs defaultValue={subPage} className={cn(filesLoading && "opacity-50 pointer-events-none")}>
674-707: Remove unnecessary fragments.The fragments wrapping single elements are redundant.
Apply this diff to remove unnecessary fragments:
- {subPage === "discharge_summary" && ( - <> <Button variant="outline_primary" className="flex flex-row items-center" onClick={async () => { await queryClient.invalidateQueries({ queryKey: ["files"], }); toast.success(t("refreshed")); }} > <CareIcon icon="l-sync" /> <span className="ml-2">{t("refresh")}</span> </Button> - </> - )} - {subPage === "discharge_summary" && ( - <> <Button variant="primary" className="flex flex-row items-center" onClick={() => generateDischargeSummary()} disabled={isGenerating} > <CareIcon icon="l-file-medical" className="hidden sm:block" /> <span> {isGenerating ? t("generating") : t("generate_discharge_summary")} </span> </Button> - </> - )}🧰 Tools
🪛 Biome (1.9.4)
[error] 675-689: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
[error] 692-706: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Files/FilesTab.tsx(8 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Files/FilesTab.tsx
[error] 675-689: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
[error] 692-706: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: CodeQL-Build
- GitHub Check: OSSAR-Scan
- GitHub Check: OSSAR-Scan
🔇 Additional comments (2)
src/components/Files/FilesTab.tsx (2)
1-65: LGTM! Clean import organization and interface update.The new imports and interface changes are well-structured and properly typed.
97-106: Add error handling to the discharge summary mutation.The mutation lacks error handling which could leave users unaware of failures.
Apply this diff to add error handling:
const { mutate: generateDischargeSummary, isPending: isGenerating } = useMutation<{ detail: string }, HTTPError>({ mutationFn: mutate(routes.encounter.generateDischargeSummary, { pathParams: { encounterId: encounter?.id || "" }, }), onSuccess: (response) => { toast.success(response.detail); refetch(); }, + onError: (error) => { + toast.error(t("error_generating_discharge_summary")); + console.error("Error generating discharge summary:", error); + }, });
|
@amjithtitus09 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
New Features
Refactor