Doctor notes enhancement #9298
Doctor notes enhancement #9298
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/components/Files/FileUpload.tsx (1)
Line range hint
446-446: Update archivable prop to include PATIENT_NOTESBased on the learnings, PATIENT_NOTES files should be archivable.
-archivable={tab !== "DISCHARGE_SUMMARY"} +archivable={tab === "PATIENT_NOTES" || (tab !== "DISCHARGE_SUMMARY")}
🧹 Nitpick comments (7)
src/Utils/request/types.ts (1)
39-40: LGTM! Consider adding JSDoc comments.The type changes to support both string and number types for
pathParamsand the broaderQueryParamstype are well-designed and maintain type safety while providing necessary flexibility for the new patient notes functionality.Consider adding JSDoc comments to document the purpose and expected usage of these parameters:
export interface QueryOptions<TBody = unknown> { + /** Path parameters for the request URL. Supports both string and numeric values (e.g., IDs) */ pathParams?: Record<string, string | number>; + /** Query string parameters. Supports various types including string, number, boolean, null, undefined */ queryParams?: QueryParams;src/components/Files/FileUpload.tsx (2)
198-217: Improve type safety for queries objectConsider adding explicit TypeScript types for the queries object to improve type safety and maintainability.
+interface QueryState { + data: typeof activeFilesData; + isLoading: boolean; + refetch: () => void; +} +type QueryTypes = 'UNARCHIVED' | 'ARCHIVED' | 'DISCHARGE_SUMMARY' | 'PATIENT_NOTES'; -const queries = { +const queries: Record<QueryTypes, QueryState> = { UNARCHIVED: { data: activeFilesData, isLoading: isLoadingActiveFiles, refetch: refetchActiveFiles, }, // ... rest of the queries };
434-440: Simplify file manager selection logicThe current implementation could be simplified for better maintainability.
-fileManager={ - { - DISCHARGE_SUMMARY: dischargeSummaryFileManager, - PATIENT_NOTES: patientNotesFileManager, - }[tab] || fileManager -} -associating_id={ - tab === "PATIENT_NOTES" ? item.associating_id! : associatedId -} +fileManager={getFileManager(tab)} +associating_id={getAssociatingId(tab, item)} // Add these utility functions: +const getFileManager = (tab: string) => { + const managers = { + DISCHARGE_SUMMARY: dischargeSummaryFileManager, + PATIENT_NOTES: patientNotesFileManager, + }; + return managers[tab as keyof typeof managers] || fileManager; +}; + +const getAssociatingId = (tab: string, item: FileUploadModel) => + tab === "PATIENT_NOTES" ? item.associating_id! : associatedId;src/components/Patient/PatientNotes.tsx (1)
30-40: Consider using useEffect for state updatesThe state updates within the if conditions could lead to unnecessary renders. Consider moving these updates to a useEffect hook.
- if (patientData) { - if (patientData.name && patientData.name !== patientName) { - setPatientName(patientData.name); - } - if ( - patientData.facility_object?.name && - patientData.facility_object.name !== facilityName - ) { - setFacilityName(patientData.facility_object.name); - } - } + useEffect(() => { + if (patientData?.name && patientData.name !== patientName) { + setPatientName(patientData.name); + } + if ( + patientData?.facility_object?.name && + patientData.facility_object.name !== facilityName + ) { + setFacilityName(patientData.facility_object.name); + } + }, [patientData, patientName, facilityName]);src/components/Facility/PatientNotesListComponent.tsx (2)
128-163: Enhance error handling in onAddNote functionThe error handling could be more specific to help users better understand what went wrong.
} catch (error) { - Notification.Error({ msg: "An error occurred while adding the note." }); + Notification.Error({ + msg: error instanceof Error + ? `Failed to add note: ${error.message}` + : "An unexpected error occurred while adding the note." + }); return undefined; }
165-175: Consider debouncing the refetch operationThe real-time update mechanism might trigger too many refetches if multiple updates occur in quick succession.
+ import { debounce } from 'lodash'; + + const debouncedRefetch = debounce(() => { + refetchNotes(); + }, 500); + useMessageListener((data) => { const message = data?.message; if ( (message?.from == "patient/doctor_notes/create" || message?.from == "patient/doctor_notes/edit") && message?.facility_id == facilityId && message?.patient_id == patientId ) { - refetchNotes(); + debouncedRefetch(); } });src/components/Facility/PatientNotesSlideover.tsx (1)
174-177: Add cleanup for local storageConsider cleaning up the local storage when the component unmounts to prevent stale data.
useEffect(() => { localStorage.setItem(localStorageKey, noteField); + return () => { + localStorage.removeItem(localStorageKey); + }; }, [noteField, localStorageKey]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/Utils/request/types.ts(1 hunks)src/components/Common/MentionDropdown.tsx(1 hunks)src/components/Facility/PatientNotesDetailedView.tsx(1 hunks)src/components/Facility/PatientNotesListComponent.tsx(1 hunks)src/components/Facility/PatientNotesSlideover.tsx(7 hunks)src/components/Files/FileUpload.tsx(7 hunks)src/components/Patient/PatientNotes.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Facility/PatientNotesDetailedView.tsx
- src/components/Common/MentionDropdown.tsx
🧰 Additional context used
📓 Learnings (2)
src/components/Patient/PatientNotes.tsx (1)
Learnt from: UdaySagar-Git
PR: ohcnetwork/care_fe#9298
File: src/components/Facility/PatientNotesDetailedView.tsx:147-188
Timestamp: 2024-12-05T22:41:24.173Z
Learning: In the `PatientNotesDetailedView` component, child notes (`state.child_notes`) are fetched along with the parent note in a single query, so a separate loading state for child notes is not required.
src/components/Files/FileUpload.tsx (1)
Learnt from: UdaySagar-Git
PR: ohcnetwork/care_fe#9298
File: src/components/Files/FileUpload.tsx:392-393
Timestamp: 2024-12-17T05:44:33.531Z
Learning: In the `FileUpload` component (`src/components/Files/FileUpload.tsx`), `PATIENT_NOTES` files should be archivable; therefore, the `archivable` prop should include `PATIENT_NOTES`.
🔇 Additional comments (4)
src/components/Files/FileUpload.tsx (2)
235-242: LGTM! Well-implemented tab management
The implementation follows best practices with proper internationalization and conditional rendering based on data availability.
295-298: LGTM! File manager implementation is correct
The implementation correctly follows the pattern and includes proper callback handling for state updates.
src/components/Patient/PatientNotes.tsx (1)
Line range hint 1-61: LGTM! Well-structured component with proper separation of concerns
The component is well-organized with proper type definitions, efficient data fetching using react-query, and good state management practices.
src/components/Facility/PatientNotesListComponent.tsx (1)
97-108:
Fix potential state inconsistency in useEffect
The dependency array is missing state.cPage, which could lead to stale state references.
useEffect(() => {
if (notesData) {
setState((prevState) => ({
...prevState,
notes:
- state.cPage === 1
+ prevState.cPage === 1
? notesData.results
: [...prevState.notes, ...notesData.results],
totalPages: Math.ceil(notesData.count / RESULTS_PER_PAGE_LIMIT),
}));
}
- }, [notesData]);
+ }, [notesData, state.cPage]);Likely invalid or redundant comment.
|
cypress tests regarding fileuploads are passing in my local , can you please look into it (it is passing some times and failing completely sometime in github actions) |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/components/Files/FileUpload.tsx (1)
295-298: Consider adding type annotations for callback handlersWhile the implementation is correct, adding TypeScript type annotations would improve type safety.
const patientNotesFileManager = useFileManager({ type: "PATIENT_NOTES", - onArchive: () => refetchDiscussionNotes(), - onEdit: () => refetchDiscussionNotes(), + onArchive: (): Promise<void> => refetchDiscussionNotes(), + onEdit: (): Promise<void> => refetchDiscussionNotes(), });src/components/Facility/PatientNotesSlideover.tsx (1)
Line range hint
42-47: Consider extracting thread initialization logicThe thread initialization logic could be moved to a separate function or constant to improve readability and maintainability.
+ const getInitialThread = (userType: string) => + userType === "Nurse" + ? PATIENT_NOTES_THREADS.Nurses + : PATIENT_NOTES_THREADS.Doctors; const [thread, setThread] = useState( - authUser.user_type === "Nurse" - ? PATIENT_NOTES_THREADS.Nurses - : PATIENT_NOTES_THREADS.Doctors, + getInitialThread(authUser.user_type) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Facility/PatientNotesSlideover.tsx(7 hunks)src/components/Files/FileUpload.tsx(7 hunks)
🧰 Additional context used
📓 Learnings (1)
src/components/Files/FileUpload.tsx (1)
Learnt from: UdaySagar-Git
PR: ohcnetwork/care_fe#9298
File: src/components/Files/FileUpload.tsx:392-393
Timestamp: 2024-12-17T05:44:33.531Z
Learning: In the `FileUpload` component (`src/components/Files/FileUpload.tsx`), `PATIENT_NOTES` files should be archivable; therefore, the `archivable` prop should include `PATIENT_NOTES`.
🔇 Additional comments (6)
src/components/Files/FileUpload.tsx (3)
213-217: LGTM: Patient notes query integration
The integration of patient notes query follows the established pattern and maintains consistency with other query implementations.
235-242: LGTM: Well-implemented tab management
The patient notes tab implementation follows best practices:
- Uses internationalization
- Conditionally renders based on data availability
- Maintains consistency with existing tab structure
179-194: 🛠️ Refactor suggestion
Add error handling for discussion notes query
The query implementation looks good, but it's missing error handling for API failures.
const {
data: discussionNotesData,
isLoading: isLoadingDiscussionNotes,
+ error: discussionNotesError,
refetch: refetchDiscussionNotes,
} = useQuery({
queryKey: [routes.viewUpload.path, "PATIENT_NOTES", consultationId, offset],
queryFn: query(routes.viewUpload, {
queryParams: {
file_type: "PATIENT_NOTES",
consultation_id: consultationId || "",
is_archived: false,
limit: RESULTS_PER_PAGE_LIMIT,
offset,
},
}),
enabled: !!consultationId,
+ onError: (error) => {
+ console.error('Failed to fetch discussion notes:', error);
+ // TODO: Add user-friendly error notification
+ },
});Likely invalid or redundant comment.
src/components/Facility/PatientNotesSlideover.tsx (3)
1-31: LGTM! Clean import organization
The imports are well-structured and properly organized, with clear separation between external libraries, internal components, and utilities.
237-243: 🛠️ Refactor suggestion
Enhance accessibility for the slideover panel
The slideover panel should have proper ARIA attributes for better screen reader support.
<div
ref={slideoverRef}
+ role="dialog"
+ aria-modal="true"
+ aria-labelledby="discussion-notes-title"
className={classNames(
"fixed bottom-0 z-20 sm:right-8",
show
? "right-0 h-screen w-screen sm:h-fit sm:w-[430px]"
: "right-8 w-[250px]",
)}
>Also add a corresponding ID to the title:
- <span className="font-semibold">Discussion Notes</span>
+ <span id="discussion-notes-title" className="font-semibold">Discussion Notes</span>Likely invalid or redundant comment.
298-307: Verify DiscussionNotesEditor usage across the codebase
Let's ensure consistent usage of the DiscussionNotesEditor component across different features.
✅ Verification successful
DiscussionNotesEditor usage is consistent across the codebase
The component is used in three locations (PatientNotesSlideover, PatientNotesListComponent, and PatientNotesDetailedView) with consistent prop usage:
- All implementations properly provide the required props:
initialNote,onChange,onAddNote - Optional props like
isAuthorized,onRefetch, andmaxRowsare used appropriately - The implementation in PatientNotesSlideover matches the component's interface definition
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all usages of DiscussionNotesEditor to verify consistent prop usage
ast-grep --pattern 'DiscussionNotesEditor $$$'
# Search for any TODO comments related to the editor
rg -i "todo.*editor|editor.*todo"
Length of output: 84
Script:
#!/bin/bash
# Let's try a different approach to find DiscussionNotesEditor usages
# First, find the file containing the component definition
fd "DiscussionNotesEditor" --type f
# Then search for JSX/TSX imports and usage
rg "DiscussionNotesEditor" --type=tsx --type=jsx -B 2 -A 2
Length of output: 170
Script:
#!/bin/bash
# Now that we found the component file, let's try a different ripgrep approach
# Search for imports and usages of DiscussionNotesEditor
rg "DiscussionNotesEditor" -t ts -t tsx -t jsx -B 2 -A 2
# Also check the component definition to verify required props
cat src/components/Common/DiscussionNotesEditor.tsx
Length of output: 9961
Script:
#!/bin/bash
# Let's try one more time with a simpler ripgrep pattern to find usages
rg -l "import.*DiscussionNotesEditor|from.*DiscussionNotesEditor"
# And search for direct component usage
rg "<DiscussionNotesEditor"
Length of output: 653
Script:
#!/bin/bash
# Let's check the props usage in all three files
cat src/components/Facility/PatientNotesDetailedView.tsx | grep -A 5 "<DiscussionNotesEditor"
cat src/components/Facility/PatientNotesListComponent.tsx | grep -A 5 "<DiscussionNotesEditor"
cat src/components/Facility/PatientNotesSlideover.tsx | grep -A 5 "<DiscussionNotesEditor"
Length of output: 1082
rithviknishad
added
changes required
and removed
Deploy-Failed
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
public/locale/en.json (1)
1449-1449: Consider standardizing message input placeholders.For consistency with other input placeholders in the file, consider ending the message placeholder with an ellipsis.
- "type_your_message": "Type your message here...", + "type_your_message": "Type your message...",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
public/locale/en.json(8 hunks)
🔇 Additional comments (1)
public/locale/en.json (1)
358-358: LGTM! New localization entries align with feature requirements.
The added entries support the new doctor notes features including file attachments, user mentions, thread view, and messaging functionality.
Also applies to: 968-968, 1024-1024, 1046-1046, 1123-1123, 1253-1253, 1449-1449, 1515-1515
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/components/Facility/PatientNotesListComponent.tsx (2)
149-155: Enhance note field validation.The current validation only checks for non-whitespace characters. Consider adding more robust validation:
- Maximum length check
- Trim whitespace before validation
Apply this diff:
const onAddNote = async () => { - if (!/\S+/.test(noteField)) { + const trimmedNote = noteField.trim(); + const MAX_NOTE_LENGTH = 1000; // Adjust as needed + + if (!trimmedNote) { Notification.Error({ msg: "Note Should Contain At Least 1 Character", }); return; } + + if (trimmedNote.length > MAX_NOTE_LENGTH) { + Notification.Error({ + msg: `Note exceeds maximum length of ${MAX_NOTE_LENGTH} characters`, + }); + return; + }
61-64: Consider persisting thread view mode.The thread view mode state is reset on component remount. Consider persisting it in localStorage for a better user experience.
Apply this diff:
+ const THREAD_VIEW_MODE_KEY = 'threadViewMode'; + const [mode, setMode] = useState<"thread-view" | "default-view">( - "default-view", + (localStorage.getItem(THREAD_VIEW_MODE_KEY) as "thread-view" | "default-view") || "default-view" ); const [threadViewNote, setThreadViewNote] = useState(""); + useEffect(() => { + localStorage.setItem(THREAD_VIEW_MODE_KEY, mode); + }, [mode]);src/components/Facility/models.tsx (2)
564-565: Consider using a more specific type for ThreadCategory.The current type allows any value from
PATIENT_NOTES_THREADS. Consider using a union type of specific string literals for better type safety.Apply this diff:
- export type ThreadCategory = - (typeof PATIENT_NOTES_THREADS)[keyof typeof PATIENT_NOTES_THREADS]; + export type ThreadCategory = 'DOCTORS' | 'NURSES' | 'SPECIALISTS';
585-589: Add validation constraints to PatientNotesRequest interface.Consider adding validation constraints using TypeScript utility types to enforce data validation at the type level.
Apply this diff:
+ type NonEmptyString = string & { __brand: 'NonEmptyString' }; + export interface PatientNotesRequest { - note: string; + note: NonEmptyString; thread: ThreadCategory; consultation?: string; reply_to?: string; } + + // Validation function + export function createPatientNotesRequest( + note: string, + thread: ThreadCategory, + consultation?: string, + reply_to?: string + ): PatientNotesRequest | null { + if (!note.trim()) return null; + return { + note: note as NonEmptyString, + thread, + consultation, + reply_to, + }; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/Utils/request/api.tsx(3 hunks)src/components/Facility/PatientNotesDetailedView.tsx(1 hunks)src/components/Facility/PatientNotesListComponent.tsx(1 hunks)src/components/Facility/PatientNotesSlideover.tsx(7 hunks)src/components/Facility/models.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Facility/PatientNotesDetailedView.tsx
🧰 Additional context used
📓 Learnings (1)
src/components/Facility/PatientNotesListComponent.tsx (1)
Learnt from: UdaySagar-Git
PR: ohcnetwork/care_fe#9298
File: src/components/Facility/PatientNotesDetailedView.tsx:147-188
Timestamp: 2024-12-05T22:41:24.173Z
Learning: In the `PatientNotesDetailedView` component, child notes (`state.child_notes`) are fetched along with the parent note in a single query, so a separate loading state for child notes is not required.
🔇 Additional comments (3)
src/components/Facility/PatientNotesSlideover.tsx (1)
118-128: 🛠️ Refactor suggestion
Add cleanup for notification subscription effect.
The notification warning effect should be cleaned up when the component unmounts to prevent memory leaks.
Apply this diff:
useEffect(() => {
+ let mounted = true;
if (notificationSubscriptionState === "unsubscribed") {
- Notification.Warn({
+ mounted && Notification.Warn({
msg: "Please subscribe to notifications to get live updates on discussion notes.",
});
} else if (notificationSubscriptionState === "subscribed_on_other_device") {
- Notification.Warn({
+ mounted && Notification.Warn({
msg: "Please subscribe to notifications on this device to get live updates on discussion notes.",
});
}
+ return () => {
+ mounted = false;
+ };
}, [notificationSubscriptionState]);Likely invalid or redundant comment.
src/Utils/request/api.tsx (2)
669-673: LGTM! New endpoint follows RESTful conventions.
The new getPatientNote endpoint is well-structured and maintains consistency with the existing API patterns.
662-662: LGTM! Type safety improvement.
The update to use PatientNotesRequest type enhances type safety for the request body.
|
👋 Hi, @UdaySagar-Git, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
rithviknishad
removed
test failed
changes required
merge conflict
|
@UdaySagar-Git 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
follow up for #7159
backend pr: ohcnetwork/care#2635
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
DiscussionNotesEditorcomponent for editing discussion notes with features like mentions and file uploads.FilePreviewCardcomponent for displaying uploaded file previews.MentionsDropdowncomponent for user mentions in notes.PatientNotesListComponentfor managing and displaying patient notes.Textareacomponent for enhanced text area functionality.PatientNotesDetailedViewcomponent for displaying detailed patient notes and replies.Enhancements
PatientNoteCardwith audio playback functionality.FileUploadcomponent to integrate support for patient notes.Bug Fixes
PatientNotescomponent.Chores