fix: allow uploading multiple same-size files via drag-and-drop#38109
fix: allow uploading multiple same-size files via drag-and-drop#38109dodaa08 wants to merge 3 commits intoRocketChat:developfrom
Conversation
- Changed deduplication key from file.size to file.name-file.size-file.lastModified - Allows uploading multiple files with the same size but different names
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughThe deduplication mechanism for file uploads was updated to identify duplicate files using a composite key of file name, size, and lastModified timestamp instead of size alone. The tracking Set type changed from storing numbers to storing strings accordingly. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![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 (1)
apps/meteor/client/views/room/body/hooks/useFileUploadDropTarget.ts (1)
44-52: LGTM! The composite key fix resolves the duplicate file issue.The deduplication logic now correctly distinguishes files with the same size by including name and lastModified timestamp in the key. This solves the reported bug where same-size files were incorrectly treated as duplicates.
♻️ Optional improvements
1. Eliminate theoretical key collision risk:
While extremely unlikely in practice, the hyphen-separated format could theoretically produce collisions. Consider using
JSON.stringifyfor a collision-proof approach:const getUniqueFiles = () => { const uniqueFiles: File[] = []; - const st: Set<string> = new Set(); + const seenKeys: Set<string> = new Set(); files.forEach((file) => { - const key = `${file.name}-${file.size}-${file.lastModified}`; - if (!st.has(key)) { + const key = JSON.stringify({ name: file.name, size: file.size, lastModified: file.lastModified }); + if (!seenKeys.has(key)) { uniqueFiles.push(file); - st.add(key); + seenKeys.add(key); } }); return uniqueFiles; };2. Improve variable naming:
Rename
sttoseenKeysoruniqueKeysfor better readability (shown in diff above).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/client/views/room/body/hooks/useFileUploadDropTarget.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/room/body/hooks/useFileUploadDropTarget.ts
|
Hi maintainers This change is a small client-side bug fix and shouldn’t require a version bump, so I believe a changeset is not needed here. Please let me know if you’d prefer one to be added. I don’t have permission to add labels or milestones, could you please help by adding the required stat: QA assured label and assigning the appropriate milestone/project if everything looks good? Happy to make any further adjustments if needed. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38109 +/- ##
===========================================
- Coverage 70.77% 70.75% -0.03%
===========================================
Files 3158 3158
Lines 109355 109355
Branches 19648 19671 +23
===========================================
- Hits 77397 77375 -22
- Misses 29934 29947 +13
- Partials 2024 2033 +9
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Closed in favor of #32703 |
3 participants
Proposed changes (including videos or screenshots)
Previously, when multiple files were dragged and dropped into a chat, files with the same size were incorrectly treated as duplicates and only one would upload.
This fix changes the deduplication logic to identify files using their name, size, and modification time together, ensuring that different files with matching sizes are correctly uploaded while still preventing true duplicates.
Changed the key from file.size to ${file.name}-${file.size}-${file.lastModified}.
Local Testing With new key logic
Screencast.From.2026-01-08.23-16-32.mp4
Issue(s)
Closes #38108
Steps to test or reproduce
echo "Hello World 123" > file1.txt
echo "Different Text!" > file2.txt
Further comments
Let me know if there's a better approach or if any changes are needed.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.