Hotfix/uploadv3 duplicate file max reached#226
Conversation
…uploads handleFileCompleted now checks file._asyncProcessing and skips marking complete for async uploads. wrappedOnUploadComplete marks all uploading files complete when polling confirms server processing is done.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 59 minutes and 7 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughVersion bump to beta combined with modifications to file upload completion handling. Dropzone now returns early on max files error. UploadInputV3 prevents async-processing files from immediately completing and changes how completed files reconcile with parent values. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/inputs/upload-input-v3/__tests__/upload-input-v3.test.js (1)
311-350: LGTM — new tests lock in both behavioral changes.The server-rename reconciliation test (L312‑331) guards the root cause of the "max reached" bug, and the
_asyncProcessingtest (L333‑350) pins the HTTP 202 "stay in Loading" contract.One small follow-up worth considering (non-blocking): add a test that calls
onUploadCompleteviadropzoneCallbacks.onUploadComplete(...)and asserts the async row transitions from "Loading" → "Complete" after polling finishes — that's the complementary branch to the_asyncProcessing: trueassertion here and exerciseswrappedOnUploadCompletedirectly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/inputs/upload-input-v3/__tests__/upload-input-v3.test.js` around lines 311 - 350, Add a complementary test that triggers the async-complete branch by calling dropzoneCallbacks.onUploadComplete(...) (which exercises wrappedOnUploadComplete) for an item with _asyncProcessing true, then advance timers / mock polling responses until the upload transitions from "Loading" to "Complete" and assert the UI updates accordingly; specifically, render UploadInputV3 with maxFiles=1 and value=[], simulate onAddedFile and then call dropzoneCallbacks.onUploadComplete({ name: 'video.mp4', size: 5000000, _asyncProcessing: true }), mock the polling endpoint/responses used by wrappedOnUploadComplete, advance timers as needed, and assert that the row initially shows "Loading" and eventually shows "Complete".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/inputs/upload-input-v3/index.js`:
- Around line 193-197: wrappedOnUploadComplete currently marks every entry in
uploadingFiles as complete when any single file's onUploadComplete fires; change
it to only mark the file(s) that actually finished. In the setUploadingFiles
callback used in wrappedOnUploadComplete, update only entries whose identifier
matches the dzId parameter (compare dzId to the per-file id field used in
uploadingFiles, e.g., file.id or file.uploadId) or whose progress is 100,
leaving other files unchanged. Keep the existing onUploadComplete(response,
dzId, dzData) forwarding behavior. Ensure you reference wrappedOnUploadComplete,
setUploadingFiles and the uploadingFiles item id/progress fields when making the
change.
---
Nitpick comments:
In `@src/components/inputs/upload-input-v3/__tests__/upload-input-v3.test.js`:
- Around line 311-350: Add a complementary test that triggers the async-complete
branch by calling dropzoneCallbacks.onUploadComplete(...) (which exercises
wrappedOnUploadComplete) for an item with _asyncProcessing true, then advance
timers / mock polling responses until the upload transitions from "Loading" to
"Complete" and assert the UI updates accordingly; specifically, render
UploadInputV3 with maxFiles=1 and value=[], simulate onAddedFile and then call
dropzoneCallbacks.onUploadComplete({ name: 'video.mp4', size: 5000000,
_asyncProcessing: true }), mock the polling endpoint/responses used by
wrappedOnUploadComplete, advance timers as needed, and assert that the row
initially shows "Loading" and eventually shows "Complete".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e4fee46-8047-49ee-89b3-b5d362700e84
📒 Files selected for processing (4)
package.jsonsrc/components/inputs/dropzone/index.jssrc/components/inputs/upload-input-v3/__tests__/upload-input-v3.test.jssrc/components/inputs/upload-input-v3/index.js
…n-flight files complete Only mark uploading rows as complete when their progress has reached 100%, preventing premature completion of still-uploading files in multi-file (maxFiles > 1) scenarios.
ref: https://app.clickup.com/t/86b9gt7w3
Summary by CodeRabbit
Bug Fixes
Chores