Patient files #9787
Patient files #9787
Conversation
WalkthroughThis pull request introduces significant enhancements to file management and localization across multiple components. Key changes include the creation of a reusable Changes
Assessment against linked issues
Possibly related PRs
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. |
![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
🧹 Nitpick comments (5)
src/components/Users/models.tsx (1)
61-64: Consider adding field-level documentation for better clarity.Defining a dedicated
UserPermissionstype is an excellent step towards more explicit data modeling. To further enhance clarity, consider adding short comments or JSDoc lines indicating the intended use of each field (name,slug,context). This will help future contributors quickly understand these properties.src/components/Files/FilesTab.tsx (3)
107-108: Remove or clarify commented-out code.These lines referencing
file_categoryare commented out. If they're no longer needed, removing them would keep the code cleaner. Otherwise, consider explaining why it's temporarily commented out.97 queryFn: query(routes.viewUpload, { 98 queryParams: { 99 ... 104 ...(qParams.is_archived !== undefined && { 105 is_archived: qParams.is_archived, 106 }), 107- //file_category: qParams.file_category, 108 },
147-151: Duplicate 'pdf' extension.You're appending "pdf" twice to the allowed extensions. Consider removing the duplicate entry to keep the list consistent.
147 "pdf", 148 "xls", 149 "xlsx", 150 "ods", -151 "pdf",
530-540: Remove or justify the commented-out<TabsList>block.This entire block is commented out. If it's in development, add a TODO or comment explaining its purpose. If obsolete, remove it to maintain readability.
src/pages/Facility/FacilityDetailsPage.tsx (1)
107-108: Use optional chaining for cleaner checks.Static analysis suggests using optional chaining on
tokenDataandtokenData.createdAt. This slightly reduces complexity and prevents potential undefined access.- if (tokenData && tokenData.createdAt && dayjs(tokenData.createdAt).isAfter(dayjs().subtract(14, "minutes"))) { + if (tokenData?.createdAt && dayjs(tokenData.createdAt).isAfter(dayjs().subtract(14, "minutes"))) {🧰 Tools
🪛 Biome (1.9.4)
[error] 107-108: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
public/locale/en.json(17 hunks)src/Routers/AppRouter.tsx(1 hunks)src/components/Files/FilesTab.tsx(1 hunks)src/components/Patient/PatientDetailsTab/PatientFiles.tsx(1 hunks)src/components/Patient/PatientDetailsTab/index.tsx(2 hunks)src/components/Patient/PatientHome.tsx(1 hunks)src/components/Users/CreateUserForm.tsx(17 hunks)src/components/Users/models.tsx(1 hunks)src/hooks/useFileManager.tsx(1 hunks)src/pages/Encounters/tabs/EncounterFilesTab.tsx(1 hunks)src/pages/Facility/FacilityDetailsPage.tsx(1 hunks)src/pages/Patients/VerifyPatient.tsx(9 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/pages/Encounters/tabs/EncounterFilesTab.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/pages/Facility/FacilityDetailsPage.tsx
[error] 107-108: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (23)
src/Routers/AppRouter.tsx (1)
92-92: Mapping permission objects toslugis a clean approach.Providing only the
slugproperty to thePermissionProviderhelps keep the provider data minimal and focused on permission checks. This is a solid approach, but ensure no other part of the application requires the additional context provided byUserPermissions(likenameorcontext). If needed, you could pass the entire object instead, or maintain a separate store tracking all permission info.src/components/Files/FilesTab.tsx (1)
1-672: Overall file structure and functionality.The file organization and approach for handling file uploads, audio playback, and data fetching look comprehensive. The usage of the
Tabs,useQuery, and local hooks is well-structured, and the addition of tooltips for long file names is a nice usability touch.src/components/Patient/PatientDetailsTab/PatientFiles.tsx (1)
5-13: VerifypatientDataavailability.Ensure that
patientDatais always defined when rendering this component. If there's a case wherepatientData.idmight be undefined, consider adding a safety check or a loading state.✅ Verification successful
No additional safety checks needed
The
patientDataprop is properly typed as required in thePatientPropsinterface, and the parent component (PatientHome) already implements a null check that prevents rendering whenpatientDatais undefined. The current implementation is safe.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # This script searches for references to `PatientFilesTab` to confirm that # `patientData` is always populated before rendering. rg -A 5 $'PatientFilesTab'Length of output: 1264
Script:
#!/bin/bash # Check PatientProps interface and component usage ast-grep --pattern 'interface PatientProps { $$$ }' # Check parent component rendering rg -B 5 -A 10 '<PatientFilesTab' # Check for loading states rg -A 3 'patientData.*undefined|!patientData|patientData\?'Length of output: 5255
src/components/Patient/PatientDetailsTab/index.tsx (1)
9-9: New route for patient files looks good.The addition of
PatientFilesTabtopatientTabsis straightforward and appears to be implemented properly. No issues found.Also applies to: 48-51
src/pages/Patients/VerifyPatient.tsx (1)
5-5: Internationalization usage is correct.Using
useTranslationand replacing hardcoded strings witht()calls is well done. This improves maintainability and supports multiple languages seamlessly.Also applies to: 74-74, 100-100, 103-103, 114-114, 116-116, 136-136, 139-139, 170-170, 173-173, 190-190, 192-192, 211-211, 214-214
src/components/Patient/PatientHome.tsx (1)
111-111: Good use of i18n for "patient not found" message.
This localized approach ensures that messages are consistent with the rest of the application’s translation framework.src/components/Users/CreateUserForm.tsx (15)
78-80: Dynamic gender enum looks good.
Switching from a static enum toGENDER_TYPES.map(gender => gender.id)enables more flexibility and consistency across the app.
160-164: User type field label and placeholder.
Your i18n usage for user type selection is correct. This improves multilingual support.
185-187: Consistent translations for first and last name fields.
Applyingt("first_name")andt("last_name")ensures consistency across the UI.Also applies to: 199-201
214-216: Username label and placeholder localized.
The integration witht("username")is consistent.
229-235: Password field placeholders updated.
The distinct placeholders for password fields enhance clarity for multilingual users.
247-251: Confirm password field label and placeholder.
Good job ensuring both password fields have translated labels and placeholders.
266-266: Email label updated.
This aligns with the overall internationalization approach.
281-281: Phone number fields localized.
The labels for primary and alternative phone numbers reflect i18n best practices.Also applies to: 295-295
322-324: WhatsApp number label localized.
Resolves the need for multiple languages in phone/WhatsApp settings.
336-336: Date of birth label improvements.
Ensures multilingual support for date fields.
350-350: Gender label and placeholder.
The approach is consistent with the dynamic enum shift above.Also applies to: 357-357
380-382: Qualification label.
Localizing professional details ensures better user experience across languages.
398-402: Years of experience.
The translation usage is appropriate, verifying consistent user-facing text.
416-419: Medical council registration label.
Completes the i18n coverage for key user form fields.
449-449: Create user button label translated.
Great finishing touch on the form submission button for multilingual support.src/hooks/useFileManager.tsx (1)
94-94: Verify consistency of updated query key.
Renaming from${fileType}-filesto${fileType}-filecan affect caching. Confirm all references in the codebase are updated to maintain consistent data retrieval and cache invalidation.✅ Verification successful
Query key change is correct and intentional
The different suffixes (${fileType}-filesvs${fileType}-file) are properly used to distinguish between list and single-item queries, which is a good practice for cache management in React Query.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search references to the old query key and the new one rg --json '\\$\\{fileType\\}-files' -A 5 rg --json '\\$\\{fileType\\}-file' -A 5Length of output: 307
Script:
#!/bin/bash # Search for both singular and plural forms with proper escaping echo "=== Searching for -files pattern ===" rg "fileType.*-files" -A 5 echo -e "\n=== Searching for -file pattern ===" rg "fileType.*-file[^s]" -A 5Length of output: 1618
public/locale/en.json (1)
313-313: New localized strings added.
These keys (e.g.,"active_encounters","alternative_phone_number","book_a_new_appointment","create_encounter","create_user","encounter_class","quick_actions","quick_actions_description","start_a_new_clinical_encounter","start_date","view_and_manage_patient_encounters","whatsapp_number_is_same_as_phone_number") reflect expanded internationalization coverage. Make sure to cross-check usage across components and confirm that new strings are displayed correctly.Also applies to: 366-366, 472-472, 639-639, 653-653, 800-800, 1490-1490, 1491-1491, 1697-1697, 1699-1699, 1883-1883, 1916-1916
|
@Jacobjeevan When i checked deploy preview i saw that the File upload Button is missing here...Or is there any other route for file uploading ?? After:
|
It's related to perms; Backend would address that later today :) That should take care of it. |
Jacobjeevan
added
waiting for backend
and removed
question
|
👋 Hi, @Jacobjeevan, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
public/locale/en.json (3)
1512-1513: Consider consolidating duplicate translations.There appears to be duplicate translations for the same concept:
"quick_actions_description": "Schedule an appointment or create a new encounter""schedule_an_appointment_or_create_a_new_encounter": "Schedule an appointment or create a new encounter"Consider using a single key to maintain consistency and ease maintenance.
{ - "quick_actions_description": "Schedule an appointment or create a new encounter", + "quick_actions_description": "@:schedule_an_appointment_or_create_a_new_encounter", }Also applies to: 1617-1617
640-640: Consider optimizing encounter-related translations.There are several related translations that could be consolidated using parameters:
"create_a_new_encounter_to_get_started""create_encounter""start_a_new_clinical_encounter"Consider using a parameterized approach to reduce duplication and maintain consistency.
{ + "encounter_create": { + "default": "Create Encounter", + "clinical": "Start a new clinical encounter", + "get_started": "Create a new encounter to get started" + } }Also applies to: 645-645, 1720-1720
1939-1939: Consider improving the WhatsApp message formatting.The current translation could be more professionally formatted.
- "whatsapp_number_is_same_as_phone_number": "WhatsApp number is same as phone number", + "whatsapp_number_is_same_as_phone_number": "WhatsApp number is the same as phone number",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json(17 hunks)src/components/Patient/PatientHome.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Patient/PatientHome.tsx
🔇 Additional comments (4)
public/locale/en.json (4)
314-314: LGTM! Core translations for encounters and files.The translations for "active_encounters" and "files" are clear and concise.
Also applies to: 967-967
803-803: LGTM! Encounter management translations.The translations for encounter classification and priority are appropriately concise and use standard medical terminology.
Also applies to: 1491-1491
1388-1388: LGTM! Patient management translations.The translations for patient-related messages are clear and user-friendly:
- Error message for when a patient is not found
- Action description for viewing and managing patient encounters
Also applies to: 1906-1906
967-967: Verify completeness of file-related translations.Since functionality is being moved from EncounterFiles to Files component, please verify that all necessary file-related translations have been added, such as:
- File upload/download actions
- File type descriptions
- File-related error messages
✅ Verification successful
All necessary file-related translations are present ✓
The codebase has comprehensive coverage of file-related translations including:
- File operations (upload, download, archive)
- Error messages
- File categories and states
- UI labels and descriptions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any hardcoded strings related to files in the codebase echo "Searching for potential missing file-related translations..." rg -i "file" --type ts --type tsx | grep -i "string\|message\|label\|title\|description"Length of output: 247
Script:
#!/bin/bash # Check existing file-related translations echo "Existing file-related translations:" rg -i "file" public/locale/ # Search for potential hardcoded strings in TypeScript files echo -e "\nChecking TypeScript files for potential untranslated strings:" rg -i "['\"](.*file.*)['\"]" --type ts -g '!*.test.ts' -g '!*.spec.ts' # Search in TSX files using a different approach echo -e "\nChecking React components for potential untranslated strings:" find . -name "*.tsx" -type f -exec rg -i "['\"](.*file.*)['\"]" {} \;Length of output: 35407
|
Resolved cors issue ✅ |
|
👋 Hi, @Jacobjeevan, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
@Jacobjeevan can you clear the merge conflict |
|
👋 Hi, @Jacobjeevan, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
👋 Hi, @Jacobjeevan, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/components/Files/FilesTab.tsx (4)
130-153: Remove duplicate "pdf" extension.The "pdf" extension appears twice in the
allowedExtensionsarray.allowedExtensions: [ "jpg", "jpeg", "png", "gif", "bmp", "tiff", "mp4", "mov", "avi", "wmv", "mp3", "wav", "ogg", "txt", "csv", "rtf", "doc", "odt", "pdf", "xls", "xlsx", "ods", - "pdf", ],
531-541: Remove commented-out code.The commented-out
TabsListcode should be either removed or implemented if needed. Commented-out code can lead to confusion and maintenance issues.
178-185: Consider moving icons mapping to constants.The
iconsmapping could be moved to a constants file or defined outside the component to prevent recreation on each render.+// In src/common/constants.ts +export const FILE_TYPE_ICONS: Record<keyof typeof FILE_EXTENSIONS | "UNKNOWN", IconName> = { + AUDIO: "l-volume", + IMAGE: "l-image", + PRESENTATION: "l-presentation-play", + VIDEO: "l-video", + UNKNOWN: "l-file-medical", + DOCUMENT: "l-file-medical", +}; -const icons: Record<keyof typeof FILE_EXTENSIONS | "UNKNOWN", IconName> = { - AUDIO: "l-volume", - IMAGE: "l-image", - PRESENTATION: "l-presentation-play", - VIDEO: "l-video", - UNKNOWN: "l-file-medical", - DOCUMENT: "l-file-medical", -}; +const icons = FILE_TYPE_ICONS;
611-613: Improve error handling for multiple file uploads.Currently, the error is only shown for the first file (
index === 0). Consider showing errors for all files that fail validation.-error={ - index === 0 && fileUpload.error ? fileUpload.error : undefined -} +error={fileUpload.errors?.[index]}This change requires updating the
useFileUploadhook to support multiple error messages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Files/FilesTab.tsx(1 hunks)
🔇 Additional comments (2)
src/components/Files/FilesTab.tsx (2)
646-691: Add loading state handling for audio files.The component should show a loading state while fetching the audio file URL.
62-556: Add unit tests for the FilesTab component.Consider adding unit tests to verify:
- File upload functionality
- Permission-based rendering
- Filter and pagination behavior
- File type detection and icon mapping
Would you like me to help create unit tests for these components?
|
👋 Hi, @Jacobjeevan, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
LGTM |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/hooks/useFileUpload.tsx (1)
201-209: Consider adding retry logic for failed markUploadComplete.When markUploadComplete fails, the file is lost even though it was successfully uploaded. Consider adding retry logic or storing the file data for later retry.
if (markUploadCompleteError) { + const retryCount = 3; + let retryAttempt = 0; + while (retryAttempt < retryCount) { + try { + await markUploadComplete({ + data, + associating_id: associating_id, + }); + resolve(); + return; + } catch (error) { + retryAttempt++; + if (retryAttempt === retryCount) { toast.error(t("file_error__mark_complete_failed")); reject(); + } + await new Promise(r => setTimeout(r, 1000 * retryAttempt)); + } + } }src/components/Files/FilesTab.tsx (3)
533-543: Remove commented code.Remove the commented TabsList code block if it's no longer needed. If it's for future use, add a TODO comment explaining why it's kept.
178-185: Move file type constants to a separate file.Consider moving the icons mapping to a separate constants file for better maintainability.
- const icons: Record<keyof typeof FILE_EXTENSIONS | "UNKNOWN", IconName> = { - AUDIO: "l-volume", - IMAGE: "l-image", - PRESENTATION: "l-presentation-play", - VIDEO: "l-video", - UNKNOWN: "l-file-medical", - DOCUMENT: "l-file-medical", - };Create a new file
src/constants/fileIcons.ts:import { IconName } from "@/CAREUI/icons/CareIcon"; import { FILE_EXTENSIONS } from "@/common/constants"; export const FILE_TYPE_ICONS: Record<keyof typeof FILE_EXTENSIONS | "UNKNOWN", IconName> = { AUDIO: "l-volume", IMAGE: "l-image", PRESENTATION: "l-presentation-play", VIDEO: "l-video", UNKNOWN: "l-file-medical", DOCUMENT: "l-file-medical", };
573-644: Enhance dialog accessibility.The FileUploadDialog should have improved accessibility:
- Add aria-label to buttons
- Add role="dialog" to the dialog
- Add descriptive aria-label to the progress bar
<Dialog open={open} onOpenChange={onOpenChange} aria-labelledby="file-upload-dialog" + role="dialog" > <DialogContent className="mb-8 rounded-lg p-5" aria-describedby="file-upload" > // ... other code ... <Button variant="outline_primary" onClick={() => fileUpload.handleFileUpload(associatingId)} disabled={fileUpload.uploading} className="w-full" id="upload_file_button" + aria-label={t("upload")} > <CareIcon icon="l-check" className="mr-1" /> {t("upload")} </Button> // ... other code ... {!!fileUpload.progress && ( <Progress value={fileUpload.progress} className="mt-4" + aria-label={`Upload progress: ${fileUpload.progress}%`} /> )} </DialogContent> </Dialog>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
public/locale/en.json(14 hunks)src/components/Files/FilesTab.tsx(1 hunks)src/components/Patient/PatientHome.tsx(1 hunks)src/hooks/useFileManager.tsx(3 hunks)src/hooks/useFileUpload.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Patient/PatientHome.tsx
- src/hooks/useFileManager.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/hooks/useFileUpload.tsx
[error] 179-179: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (4)
src/hooks/useFileUpload.tsx (3)
162-181: LGTM! Robust error handling for upload completion.The implementation properly handles upload completion with error cases and success notifications.
🧰 Tools
🪛 Biome (1.9.4)
[error] 179-179: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Line range hint
254-288: LGTM! Improved error handling for batch uploads.The error handling allows the upload process to continue even when some files fail, providing a better user experience.
293-293: LGTM! Error state reset in clearFiles.Properly resets error state when clearing files to prevent stale error messages.
public/locale/en.json (1)
1019-1019: LGTM! Comprehensive localization coverage.All required translation keys for file management and encounters are properly added and organized.
Also applies to: 316-316, 673-673, 634-635, 1754-1754
|
@Jacobjeevan 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
Summary by CodeRabbit
Localization
New Features
FilesTabcomponent for file management.Improvements