File Archiving, Perms, Card Layout #10384
Conversation
WalkthroughThis update introduces several new keys in the localization file to support archiving details. Multiple new dialog components are added, including those for displaying archived file details, playing audio files, and managing file uploads. The FilesTab component is updated to pass complete encounter and patient objects instead of simple IDs, and it now includes permission checks and dynamic UI renderings. Updates in associated components propagate the changes to ensure proper data flow and user interactions across the file management features. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant FT as FilesTab
participant AFD as ArchivedFileDialog
U->>FT: Click "View Archived File" button
FT->>AFD: Open dialog with file details
AFD->>AFD: Format archive date (using dayjs) and fetch translations
AFD->>U: Display archived info (reason, archived_by, archived_at)
U->>AFD: Close dialog
AFD->>FT: Trigger onOpenChange callback to close dialog
sequenceDiagram
participant U as User
participant FT as FilesTab
participant APD as AudioPlayerDialog
participant Q as Query (react-query)
participant AP as AudioPlayer
U->>FT: Click "Play Audio" button
FT->>APD: Open AudioPlayerDialog with file info
APD->>Q: Fetch signed audio URL (if file is valid)
Q->>APD: Return signed URL
APD->>AP: Render audio player with URL
U->>APD: Close dialog, stopping playback
sequenceDiagram
participant U as User
participant FT as FilesTab
participant FUD as FileUploadDialog
participant FU as FileUpload Logic
U->>FT: Click "Upload File" button
FT->>FUD: Open FileUploadDialog with file upload object
FUD->>U: Display file list, input fields, and progress indicators
U->>FUD: Modify file name / remove file / start upload
FUD->>FU: Execute corresponding file upload operations
FU->>FUD: Update progress, error, or completion status
U->>FUD: Close dialog
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
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: 0
🧹 Nitpick comments (11)
src/components/Files/FilesTab.tsx (5)
56-57: New optional props enhance flexibility.
Addingencounter?: Encounterandpatient?: Patientensures this component can handle multiple data contexts. Consider adding inline documentation for these props if usage grows more complex.
213-229: Permission checks look correct and comprehensive.
Logic to disable actions based on encounter status or user permissions is sound. Consider extracting the list of blocked statuses into a shared constant to make it easier to maintain.
234-302: Detailed control buttons with good user cues.
The conditionals allow granular actions (play, view, download, archive, rename). Overall structure is fine, but if the list of actions grows, consider refactoring into smaller components or utilities to reduce complexity.
417-478: Responsive card layout is well-implemented.
Effectively displays file data for smaller screens. The duplication with table layout is understandable for responsiveness, but consider extracting shared logic (e.g., file info display) into a helper for maintainability.
479-591: Tabular view is user-friendly on larger devices.
Repeats some logic seen in the card layout. Creating a shared subcomponent might help unify file representation and reduce duplication going forward.src/components/Files/AudioPlayerDialog.tsx (2)
27-34: Add loading and error states for data fetching.The query should handle loading and error states to provide better user feedback.
- const { data: fileData } = useQuery({ + const { data: fileData, isLoading, error } = useQuery({ queryKey: [routes.retrieveUpload, type, file?.id], queryFn: query(routes.retrieveUpload, { queryParams: { file_type: type, associating_id: associatingId }, pathParams: { id: file?.id || "" }, }), enabled: !!file?.id, }); + if (isLoading) return <div>Loading...</div>; + if (error) return <div>Error loading audio file</div>;
39-46: Enhance dialog accessibility.The dialog should have a more descriptive aria-label.
<Dialog open={open} onOpenChange={() => { stopPlayback(); onOpenChange(false); }} - aria-labelledby="audio-player-dialog" + aria-labelledby="audio-player-dialog-title" + aria-describedby="audio-player-dialog-description" >src/components/Files/ArchivedFileDialog.tsx (2)
22-22: Improve null handling for file name construction.The current implementation might result in "undefined" being displayed if both name and extension are null.
- const fileName = file?.name ? file.name + file.extension : ""; + const fileName = file?.name ? `${file.name}${file.extension || ""}` : "";
50-52: Add date format localization support.The date format should respect the user's locale settings.
- {dayjs(file?.archived_datetime).format("DD MMM YYYY, hh:mm A")} + {dayjs(file?.archived_datetime).locale(i18next.language).format("L LT")}src/components/Files/FileUploadDialog.tsx (2)
45-92: Enhance accessibility for file list items.The file list items should be more accessible with proper ARIA attributes and keyboard navigation.
- <div key={index} className="space-y-2"> + <div + key={index} + className="space-y-2" + role="listitem" + aria-labelledby={`file-name-${index}`} + > <div className="flex items-center justify-between gap-2 rounded-md bg-secondary-300 px-4 py-2"> - <span className="flex items-center truncate"> + <span + className="flex items-center truncate" + id={`file-name-${index}`} + >
115-117: Add determinate progress indicator.The progress bar should include textual representation of progress.
{!!fileUpload.progress && ( - <Progress value={fileUpload.progress} className="mt-4" /> + <div className="mt-4"> + <Progress + value={fileUpload.progress} + aria-label="Upload progress" + aria-valuemin={0} + aria-valuemax={100} + aria-valuenow={fileUpload.progress} + /> + <span className="text-sm text-gray-500 mt-1"> + {Math.round(fileUpload.progress)}% uploaded + </span> + </div> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
public/locale/en.json(1 hunks)src/components/Files/ArchivedFileDialog.tsx(1 hunks)src/components/Files/AudioPlayerDialog.tsx(1 hunks)src/components/Files/FileUploadDialog.tsx(1 hunks)src/components/Files/FilesTab.tsx(11 hunks)src/components/Patient/PatientDetailsTab/PatientFiles.tsx(1 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 (12)
src/components/Files/FilesTab.tsx (9)
3-3: Good introduction of i18n.
Importingtfromi18nextis appropriate and consistent with other i18n usage in the codebase.
34-36: All dialog imports appear valid and properly utilized.
These imports forArchivedFileDialog,AudioPlayerDialog, andFileUploadDialogalign well with the respective components seen later in the code.
40-40: Proper hook usage for file uploads.
ImportinguseFileUploadseems consistent with the rest of the file’s functionality.
49-50: Well-structured data imports.
PullingEncounterandPatienttypes from separate modules improves clarity and modularity.
61-61: Destructured assignment is clean and readable.
Nicely captures incoming props, ensuring clarity on the data being used.
66-68: State variables for handling archived files.
Properly sets local component state to manage dialog openings and selected archived files. No immediate issues.
191-208: Clear UI feedback for archived files.
ThegetArchivedMessagefunction and included button elegantly convey archived file status. It’s a nice usability feature that helps differentiate active vs. archived files.
363-363: Straightforward shortcut for hidden upload buttons.
Early return ensures no UI is rendered ifeditPermissionis false. This is concise and clear.
639-639: Ensures card layout is rendered within TabsContent.
Linking<RenderCard />in this tab context appropriately completes the responsive behavior.src/components/Patient/PatientDetailsTab/PatientFiles.tsx (1)
11-11: Passing the full patient object is appropriate.
Provides richer data context and aligns with the updatedFilesTabsignature.src/pages/Encounters/tabs/EncounterFilesTab.tsx (1)
10-10: Prop change aligns with new FilesTab interface.
RemovingencounterIdin favor of the entireencounterobject aids in more detailed context sharing.public/locale/en.json (1)
384-388: LGTM! New localization keys are consistent.The added keys for archiving functionality match their usage in the
ArchivedFileDialogcomponent.
Jacobjeevan
added
needs testing
needs review
work-in-progress
and removed
needs testing
needs review
labels
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/components/Files/FilesTab.tsx (1)
92-110: Consider implementing error handling for the query.The useQuery hook should include error handling to gracefully handle API failures.
const { data: files, isLoading: filesLoading, + error, refetch, } = useQuery({ queryKey: ["files", type, associatingId, qParams], queryFn: query(routes.viewUpload, { queryParams: { // ... existing params }, }), }); +if (error) { + return <div className="text-red-500">{t("error_loading_files")}</div>; +}
🧹 Nitpick comments (4)
src/components/Files/FilesTab.tsx (4)
214-230: Consider extracting status check to a constant.The status check array could be moved to a constant to improve maintainability and reusability.
+const READONLY_ENCOUNTER_STATUSES = [ + "completed", + "cancelled", + "entered_in_error", + "discontinued", +] as const; const editPermission = () => { if (type === "encounter") { return ( encounter && - ![ - "completed", - "cancelled", - "entered_in_error", - "discontinued", - ].includes(encounter.status) && + !READONLY_ENCOUNTER_STATUSES.includes(encounter.status) && hasPermission("can_write_encounter") ); } else if (type === "patient") { return hasPermission("can_write_patient"); } return false; };
480-592: Consider extracting common file rendering logic.The
RenderCardandRenderTablecomponents share similar logic for rendering file information. Consider extracting the common parts to reduce code duplication.+const FileInfo = ({ file, filetype }: { file: FileUploadModel; filetype: string }) => { + const fileName = file.name ? file.name + file.extension : ""; + return ( + <> + <span className="p-2 rounded-full bg-gray-100 shrink-0"> + <CareIcon icon={icons[filetype]} className="text-xl" /> + </span> + <div className="min-w-0 flex-1"> + <div className="font-medium text-gray-900 truncate">{fileName}</div> + <div className="mt-1 text-sm text-gray-500">{filetype}</div> + </div> + </> + ); +};
107-107: Uncomment or remove commented code.There's a commented line for file_category filtering. Either remove it or uncomment if it's needed.
- //file_category: qParams.file_category, + file_category: qParams.file_category,
235-305: Consider breaking down DetailButtons into smaller components.The DetailButtons component is quite large and handles multiple responsibilities. Consider breaking it down into smaller, more focused components for better maintainability.
+const FileActions = ({ file, associatingId }: { file: FileUploadModel; associatingId: string }) => ( + <DropdownMenu> + <DropdownMenuTrigger asChild> + <Button variant="secondary"> + <CareIcon icon="l-ellipsis-h" /> + </Button> + </DropdownMenuTrigger> + <DropdownMenuContent align="start"> + {/* ... existing dropdown items ... */} + </DropdownMenuContent> + </DropdownMenu> +); const DetailButtons = ({ file }: { file: FileUploadModel }) => { const filetype = getFileType(file); return ( <> {editPermission() && ( <div className="flex flex-row gap-2 justify-end"> {filetype === "AUDIO" && !file.is_archived && <AudioButton file={file} />} {fileManager.isPreviewable(file) && <PreviewButton file={file} />} <FileActions file={file} associatingId={associatingId} /> </div> )} </> ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Files/FilesTab.tsx(12 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
src/components/Files/FilesTab.tsx (2)
52-58: Props interface update aligns with best practices.The change from passing IDs to complete objects (
EncounterandPatient) improves type safety and reduces the need for additional data fetching.
418-478: Responsive design implementation looks good.The card layout for smaller screens (xl:hidden) is well-implemented with appropriate spacing and information hierarchy. The implementation aligns with the PR objective of improving the UI for smaller screens.
| <ArchivedFileDialog | ||
| open={openArchivedFileDialog} | ||
| onOpenChange={setOpenArchivedFileDialog} | ||
| file={selectedArchivedFile} | ||
| /> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify archived file dialog implementation.
The new archived file dialog component has been added, but we should verify its implementation to ensure it handles all edge cases properly.
🏁 Script executed:
#!/bin/bash
# Search for ArchivedFileDialog implementation
ast-grep --pattern 'export default function ArchivedFileDialog({ $$ }) {
$$$
}'Length of output: 83
🏁 Script executed:
#!/bin/bash
# First, try to locate files with "ArchivedFileDialog" in their name.
echo "Searching for files with 'ArchivedFileDialog' in filename:"
fd "ArchivedFileDialog" --type f
echo -e "\nSearching for the definition of ArchivedFileDialog in TypeScript files:"
rg "export default .*ArchivedFileDialog" --type ts
# If a file is found, display its contents to verify its implementation details.
files=$(fd "ArchivedFileDialog" --type f)
if [ -z "$files" ]; then
echo "No file found with name containing 'ArchivedFileDialog'."
else
for file in $files; do
echo -e "\n=== Contents of: $file ==="
cat "$file"
echo "============================"
done
fiLength of output: 2554
ArchivedFileDialog implementation issues detected.
- The component does not guard against a
nullfile value. Whenfileisnull, most fields (e.g.,fileName, archive reason, archived by) correctly render as empty, but the call todayjs(file?.archived_datetime)falls back todayjs(undefined), which returns the current date. This unintended behavior may mislead users into seeing an incorrect archived timestamp.
Jacobjeevan
added
waiting for related PR
Proposed Changes
Archived
Switched to card layout for screens <= 1080px.
Hide upload and edit buttons
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit