[WIKI-181] refactor: invalid file handling#7139
Conversation
WalkthroughThe changes refactor file upload and validation logic by centralizing invalid file handling into a single callback, simplifying parameter usage in hooks and helper functions, and updating type signatures to support undefined upload URLs. Additionally, a string literal for transaction meta is replaced with a constant for consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CustomImageUploader
participant useUploader
participant useDropZone
User->>CustomImageUploader: Select or drop file(s)
CustomImageUploader->>useDropZone: Handle file input
useDropZone->>CustomImageUploader: onFileChange callback
CustomImageUploader->>useUploader: Upload file(s), pass handleInvalidFile
useUploader-->>CustomImageUploader: Calls handleInvalidFile on error
useUploader-->>CustomImageUploader: Returns uploaded file URL(s) or error
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![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
🔭 Outside diff range comments (1)
packages/editor/src/core/hooks/use-file-upload.ts (1)
163-187:⚠️ Potential issueCritical: File validation completely removed from multi-file upload.
The removal of file validation from
uploadFirstFileAndInsertRemainingcould allow invalid files to be processed. While the PR aims to centralize validation, this function now uploads files without any validation checks.This could lead to:
- Invalid file types being uploaded
- Files exceeding size limits being processed
- No user feedback for invalid files in multi-file scenarios
Consider either:
- Ensuring the
uploadercallback performs validation (which it should based on theuseUploaderhook)- Adding a comment to clarify that validation is handled by the uploader
- Pre-validating files before processing to provide immediate feedback
export const uploadFirstFileAndInsertRemaining = async (args: TMultipleFileArgs) => { const { editor, filesList, pos, type, uploader } = args; const filesArray = Array.from(filesList); if (filesArray.length === 0) { console.error("No files found to upload."); return; } + // Note: File validation is performed within the uploader callback // Upload the first file const firstFile = filesArray[0]; uploader(firstFile);
🧹 Nitpick comments (1)
packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx (1)
89-91: Consider improving error notification UX.Using
alert()for error messages provides poor user experience. Consider using a toast notification system or inline error messages for better UX.Example implementation with a toast system:
- const handleInvalidFile = useCallback((_error: EFileError, _file: File, message: string) => { - alert(message); - }, []); + const handleInvalidFile = useCallback((error: EFileError, file: File, message: string) => { + // If a toast system is available in your app + // toast.error(message); + + // Or dispatch an error to a global error handler + console.error(`File upload error for ${file.name}: ${message}`, error); + + // For now, keeping alert as fallback + alert(message); + }, []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx(2 hunks)packages/editor/src/core/hooks/use-file-upload.ts(6 hunks)packages/editor/src/core/plugins/file/delete.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
packages/editor/src/core/plugins/file/delete.ts (1)
3-4: Good refactoring to use constants!Replacing the hardcoded string with
CORE_EDITOR_META.SKIP_FILE_DELETIONimproves maintainability and reduces the risk of typos.Also applies to: 37-37
packages/editor/src/core/hooks/use-file-upload.ts (3)
10-18: Type signature updates align with centralized error handling.The changes to allow
undefinedreturn fromeditorCommandand include theFileobject inonInvalidFilecallback improve error handling capabilities.
33-76: Proper error handling for undefined upload URLs.Good addition of explicit error handling when the upload command returns undefined. The error message is clear and the file object is correctly passed to the onInvalidFile callback.
98-141:Details
✅ Verification successful
Verify file validation is still performed for dropped files.
The removal of
acceptedMimeTypes,maxFileSize, andonInvalidFileparameters fromuseDropZonesuggests that file validation has been centralized. However, ensure that dropped files are still validated during the upload process to prevent invalid files from being processed.Run the following script to verify file validation is performed in the uploader:
🏁 Script executed:
#!/bin/bash # Description: Check if file validation is still performed in the upload flow # Search for isFileValid usage in the upload functions rg -A 10 "isFileValid" packages/editor/src/core/hooks/use-file-upload.ts # Check if the uploader callback passed to useDropZone performs validation ast-grep --pattern 'uploadFile = $$$' packages/editor/src/core/hooks/use-file-upload.tsLength of output: 861
Validation confirmed in uploader; no further changes needed
The
isFileValidcheck inuploadFirstFileAndInsertRemaining(packages/editor/src/core/hooks/use-file-upload.ts) ensures dropped files are still validated againstacceptedMimeTypesandmaxFileSize. Invalid files are halted before upload.packages/editor/src/core/extensions/custom-image/components/image-uploader.tsx (1)
105-110: Hook usage correctly updated to match new interfaces.The removal of validation parameters from
useDropZoneanduploadFirstFileAndInsertRemainingcalls properly aligns with the centralized validation approach in the upload flow.Also applies to: 140-147
aaryan610
changed the title
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
3 participants
Description
This PR refactors the way editor handles invalid files by removing invalid file check from all places except the upload function.
Type of Change
Summary by CodeRabbit
Bug Fixes
Refactor
No changes to the user interface or component props.