-
Notifications
You must be signed in to change notification settings - Fork 514
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 encounter notes #9617
Patient encounter notes #9617
Conversation
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces enhancements to the notes functionality in a healthcare application. Changes include updates to a localization file, the addition of new API routes for managing patient notes, and the creation of a new React component for displaying and interacting with these notes. A new tab for "EncounterNotes" is integrated into the encounter view, allowing users to manage notes effectively. TypeScript interfaces for messages and threads are also added to ensure type safety. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
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 (
|
Deploying care-fe with Cloudflare Pages
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
f968ac5
to
2765b11
Compare
✅ 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: 0
🧹 Nitpick comments (4)
src/Utils/request/api.tsx (1)
1645-1673
: Add optional error handling callbacks for note-related routes.
While the new note-related routes (listThreads
,createThread
,getMessages
,postMessage
) follow the established pattern, consider adding standardized error handling (e.g., HTTP 4xx or 5xx responses) to help keep consumers of these routes informed about potential request failures.src/pages/Encounters/tabs/EncounterNotesTab.tsx (3)
31-43
: Consider a configurable skeleton item count.
For loading states, make the skeleton item count configurable or dynamic, instead of[1, 2, 3]
, to handle long message lists or different UI states.
67-82
: Add fallback for missing or anonymized user data.
While the avatar logic is correct, consider gracefully handling a scenario whereusername
orprofile_picture_url
might be absent, to avoid runtime errors or broken images.
136-373
: Handle API errors for thread and message creation.
Overall, the component is well-designed with infinite scroll, thread creation, and message posting. For completeness, consider addingonError
callbacks in thecreateThreadMutation
andcreateMessageMutation
to handle failures gracefully, e.g., showing a toast notification on error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
public/locale/en.json
(4 hunks)src/Utils/request/api.tsx
(2 hunks)src/pages/Encounters/EncounterShow.tsx
(2 hunks)src/pages/Encounters/tabs/EncounterNotesTab.tsx
(1 hunks)src/types/notes/messages.ts
(1 hunks)src/types/notes/threads.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/types/notes/messages.ts
- src/types/notes/threads.ts
- public/locale/en.json
- src/pages/Encounters/EncounterShow.tsx
🔇 Additional comments (4)
src/Utils/request/api.tsx (1)
104-105
: Imports are consistent and well-organized.
No issues found with these new imports.
src/pages/Encounters/tabs/EncounterNotesTab.tsx (3)
45-66
: Thread item layout and style are clear.
The code snippet is straightforward. The highlighting for selected threads is intuitive. No issues.
83-127
: Well-structured message rendering.
The usage of conditional classes for the sender vs. recipient is well-conceived. Optionally, you can display timestamps or message statuses here if needed.
129-134
: Generic paginated response interface is a good approach.
This interface is consistent with the rest of the codebase. No issues found.
onClick={handleCreateThread} | ||
disabled={!newThreadTitle} | ||
> | ||
{t("Create")} |
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.
{t("Create")} | |
{t("create")} |
onClick={() => setShowNewThreadForm(true)} | ||
> | ||
<Plus className="mr-2 h-4 w-4" /> | ||
{t("Create New Thread")} |
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.
translation keys missing in en.json, needs to be added as mentioned in Merge Checklist
{t("Create New Thread")} | |
{t("create_new_thread")} |
queryFn: async ({ pageParam = 0 }) => { | ||
const response = await query(routes.notes.patient.getMessages, { | ||
pathParams: { | ||
patientId: encounter.patient.id, | ||
threadId: selectedThread!, | ||
}, | ||
queryParams: { | ||
limit: String(LIMIT), | ||
offset: String(pageParam), | ||
}, | ||
})({ signal: new AbortController().signal }); |
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.
won't we get signal
from the queryFn
's callbacks args itself? 🤔
queryFn: async ({ pageParam = 0 }) => { | |
const response = await query(routes.notes.patient.getMessages, { | |
pathParams: { | |
patientId: encounter.patient.id, | |
threadId: selectedThread!, | |
}, | |
queryParams: { | |
limit: String(LIMIT), | |
offset: String(pageParam), | |
}, | |
})({ signal: new AbortController().signal }); | |
queryFn: async ({ pageParam = 0, signal }) => { | |
const response = await query(routes.notes.patient.getMessages, { | |
pathParams: { | |
patientId: encounter.patient.id, | |
threadId: selectedThread!, | |
}, | |
queryParams: { | |
limit: String(LIMIT), | |
offset: String(pageParam), | |
}, | |
})({ signal }); |
}) => ( | ||
<div | ||
className={`relative p-3 cursor-pointer rounded-lg transition-colors ${ | ||
isSelected ? "bg-primary/10" : "hover:bg-muted" |
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.
we don't have muted
color configured/defined.
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.
Nice find
const UserAvatar = ({ user }: { user: Message["created_by"] }) => { | ||
const initials = user.username.charAt(0).toUpperCase(); | ||
|
||
return user.profile_picture_url ? ( | ||
<img | ||
src={user.profile_picture_url} | ||
alt={user.username} | ||
className="w-8 h-8 rounded-full object-cover" | ||
/> | ||
) : ( | ||
<div className="w-8 h-8 rounded-full bg-primary/10 flex items-center justify-center text-primary font-medium"> | ||
{initials} | ||
</div> | ||
); | ||
}; |
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.
We can reuse the existing Avatar
component right?
interface PaginatedResponse<T> { | ||
count: number; | ||
next: string | null; | ||
previous: string | null; | ||
results: T[]; | ||
} |
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.
This is already defined in src/Utils/request/types.ts
, let's not redefine it
@@ -22,6 +22,8 @@ import { EncounterUpdatesTab } from "@/pages/Encounters/tabs/EncounterUpdatesTab | |||
import { Encounter } from "@/types/emr/encounter"; | |||
import { Patient } from "@/types/emr/newPatient"; | |||
|
|||
import { EncounterNotesTab } from "./tabs/EncounterNotesTab"; |
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.
switch to absolute imports
Reminder: To add the "needs testing" label, comment "ready for testing" on this PR. |
CARE Run #4127
Run Properties:
|
Project |
CARE
|
Branch Review |
encounter-notes
|
Run status |
Failed #4127
|
Run duration | 01m 41s |
Commit |
157307f2df: Patient encounter notes
|
Committer | Bodhisha Thomas |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
1
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
2
|
View all changes introduced in this branch ↗︎ |
Tests for review
cypress/e2e/public_spec/loginpage.cy.ts • 1 failed test
Test | Artifacts | |
---|---|---|
Login Page > should successfully login with admin credentials |
Test Replay
Screenshots
|
Co-authored-by: Rithvik Nishad <[email protected]>
Co-authored-by: Rithvik Nishad <[email protected]>
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: 0
🧹 Nitpick comments (4)
src/types/notes/messages.ts (2)
3-9
: Consider improving type safety for message_historyThe interface looks well-structured, but the
message_history
property usingRecord<string, unknown>
could be more type-safe. Consider defining a specific type for the history entries to better document and enforce the expected structure.+interface MessageHistoryEntry { + timestamp: string; + previous_message: string; + updated_by: UserBase; +} + export interface Message { id: string; message: string; - message_history: Record<string, unknown>; + message_history: Record<string, MessageHistoryEntry>; created_by: UserBase; updated_by: UserBase; }
10-10
: Remove extra newlines at end of fileKeep only one newline at the end of the file to comply with linting rules.
} - - +🧰 Tools
🪛 eslint
[error] 10-11: Delete
⏎
(prettier/prettier)
🪛 GitHub Check: lint
[failure] 10-10:
Delete⏎
src/types/notes/threads.ts (2)
1-4
: Consider adding more thread properties for medical context.The interface structure is clean and follows TypeScript best practices. However, for a medical notes system, consider adding these essential properties:
createdAt
: Timestamp for when the thread was createdauthorId
: ID of the healthcare provider who created the threadpatientId
: Reference to the associated patientstatus
: Thread state (e.g., active, archived)lastUpdated
: Timestamp for the latest modificationHere's a suggested enhancement:
export interface Thread { id: string; title: string; + createdAt: string; + authorId: string; + patientId: string; + status: 'active' | 'archived'; + lastUpdated: string; }
5-5
: Remove extra newlines at the end of file.} - -🧰 Tools
🪛 eslint
[error] 5-6: Delete
⏎
(prettier/prettier)
🪛 GitHub Check: lint
[failure] 5-5:
Delete⏎
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/types/notes/messages.ts
(1 hunks)src/types/notes/threads.ts
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: lint
src/types/notes/threads.ts
[failure] 5-5:
Delete ⏎
src/types/notes/messages.ts
[failure] 10-10:
Delete ⏎
@bodhisha 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! 🙌 |
Co-authored-by: Bodhisha Thomas <[email protected]> Co-authored-by: Bodhish Thomas <[email protected]>
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
Localization
These release notes highlight the addition of a comprehensive notes system for patient encounters, allowing healthcare professionals to create, view, and manage discussion threads and messages directly within the patient's encounter view.