fix: show error dialog for duplicate folder addition - #1427
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough
ChangesMutation behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
![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 (1)
frontend/src/hooks/useFolder.ts (1)
32-44: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd or verify regression coverage for mutation feedback.
Cover successful folder addition and a duplicate-folder
409 Conflict, asserting thatshowInfoDialogreceives the backend error message.As per path instructions, ensure critical functionality is covered by automated, comprehensive tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/hooks/useFolder.ts` around lines 32 - 44, Add regression tests for the mutation feedback flow in useFolder, covering both successful folder addition and duplicate-folder responses with HTTP 409 Conflict. Assert that success feedback is shown for additions and that showInfoDialog receives the backend error message for the conflict, reusing the existing mutation and feedback test patterns.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/hooks/useFolder.ts`:
- Line 4: Update the import in useFolder.ts to use the named useMutationFeedback
export from useMutationFeedback.tsx instead of a default import, while
preserving its existing usage.
---
Nitpick comments:
In `@frontend/src/hooks/useFolder.ts`:
- Around line 32-44: Add regression tests for the mutation feedback flow in
useFolder, covering both successful folder addition and duplicate-folder
responses with HTTP 409 Conflict. Assert that success feedback is shown for
additions and that showInfoDialog receives the backend error message for the
conflict, reusing the existing mutation and feedback test patterns.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 300960f0-c601-45b5-87b3-4dd317d9dcb8
📒 Files selected for processing (1)
frontend/src/hooks/useFolder.ts
|
@Prince-ES Is the long "Processing" state really necessary? Is there no better/optimized way, so it gets quicker? If not, then could we avoid blocking the entire screen while this processing is happening? It would be nice if we could use a less intrusive loading state instead of dispatching a full-screen overlay. One more question: What happens if I add a non-duplicate folder? Does the full-screen processing overlay still appear in that case? |
The add-folder mutation is actually configured with retry:2 and retryDelay: 500 so, a 409 conflict is retried twice before surfacing the error. Meaning it takes 3x time + delays currently (3 requests per adding a duplicate folder). 2026-07-29.16-08-46.mp4
Since addition of Non-duplicate folders gives a status code 200(successful request), no retries are made hence short state. So instead of retry:2 the mutation for adding a folder probably should not retry when the backend returns 409 Conflict. shall i add a commit for this? |
True, there's no need to retry if it's a conflict error. You can proceed with the commit if required. |
|
Once done, ping me in the discord server for a quicker response 😉 |
|
this is the result:- 2026-07-30.07-35-10.mp4 |
|
@rohan-pandeyy I updated my branch with the latest main, fixed the failing frontend tests, and all tests pass locally (31/31 suites, 288/288 tests). The remaining failing "Lint Check / Linting (pull_request_target)" workflow appears to fail during actions/checkout with: Refusing to check out fork pull request code from a 'pull_request_target' workflow It doesn't reach the linting step, so this looks like a workflow configuration issue rather than a lint error in the PR. |
|
@Prince-ES Pls pull the latest changes, it should fix the lint error |
2 participants
Addressed Issues:
Fixes #1416
Screenshots/Recordings:
Before:
2026-07-26.06-45-23.mp4
After:
2026-07-27.21-23-27.mp4
Additional Notes:
useMutationFeedbackandshowInfoDialogflow to display backend error messages consistently with the application UI.AI Usage Disclosure:
I have used the following AI models and tools:
Checklist
Summary by CodeRabbit