Fix broken validation in delete-project-dialog#780
Fix broken validation in delete-project-dialog#780delta6626 wants to merge 3 commits intoOpenCut-app:mainfrom
Conversation
|
@delta6626 is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe delete-project dialog now requires the user to type an exact confirmation string before enabling deletion. A ChangesDelete Confirmation Validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/project/components/delete-project-dialog.tsx`:
- Line 84: Replace the loose inequality with a strict one: in the JSX where the
Button's disabled prop compares confirmationInput to REQUIRED_CONFIRMATION_TEXT
(the expression inside the Button with onClick={onConfirm}), change the operator
from != to !== so the check uses strict inequality and avoids type-coercion.
- Around line 67-77: The Label and Input are not programmatically associated;
add a unique id (e.g., "confirmation-input") to the Input component and set the
Label's htmlFor to that id so screen readers and click-to-focus work properly;
update the JSX around Label and Input (the components rendering the confirmation
field using REQUIRED_CONFIRMATION_TEXT, confirmationInput, and
setConfirmationInput) to include the matching id/htmlFor attributes.
- Around line 32-35: The dialog's confirmationInput state (confirmationInput,
setConfirmationInput) is not reset when the Dialog (Dialog component using
isOpen and onOpenChange) closes, so reopening bypasses the "DELETE" guard; fix
by wrapping the passed onOpenChange with a local handler (e.g.,
handleOpenChange) that calls setConfirmationInput("") whenever open becomes
false and then forwards the event to the original onOpenChange prop, and pass
that handler into Dialog's onOpenChange.
🪄 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: 981b72af-980c-484c-9f38-007b79c130b7
📒 Files selected for processing (1)
apps/web/src/project/components/delete-project-dialog.tsx
| const [confirmationInput, setConfirmationInput] = useState<string>(""); | ||
|
|
||
| return ( | ||
| <Dialog open={isOpen} onOpenChange={onOpenChange}> |
There was a problem hiding this comment.
confirmationInput is never reset when the dialog closes — the confirmation guard is bypassed on re-use.
When the dialog is closed (via Cancel, backdrop click, or Escape) and then reopened, confirmationInput retains its previous value. If the user previously typed "DELETE" and confirmed a deletion, the state still holds "DELETE" when the dialog opens for the next project, causing the delete button to be immediately enabled and completely bypassing the guard.
The fix is to intercept onOpenChange locally and reset the state whenever the dialog transitions to closed:
🛡️ Proposed fix
const [confirmationInput, setConfirmationInput] = useState<string>("");
+
+ const handleOpenChange = (open: boolean) => {
+ if (!open) setConfirmationInput("");
+ onOpenChange(open);
+ };
return (
- <Dialog open={isOpen} onOpenChange={onOpenChange}>
+ <Dialog open={isOpen} onOpenChange={handleOpenChange}>
...
- <Button variant="outline" onClick={() => onOpenChange(false)}>
+ <Button variant="outline" onClick={() => handleOpenChange(false)}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/project/components/delete-project-dialog.tsx` around lines 32 -
35, The dialog's confirmationInput state (confirmationInput,
setConfirmationInput) is not reset when the Dialog (Dialog component using
isOpen and onOpenChange) closes, so reopening bypasses the "DELETE" guard; fix
by wrapping the passed onOpenChange with a local handler (e.g.,
handleOpenChange) that calls setConfirmationInput("") whenever open becomes
false and then forwards the event to the original onOpenChange prop, and pass
that handler into Dialog's onOpenChange.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/web/src/project/components/delete-project-dialog.tsx (1)
32-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
confirmationInputis still not reset when the dialog closes — delete guard is bypassed on re-use.The previously flagged fix has not been applied.
onOpenChangeis forwarded directly to<Dialog>(line 35) and the Cancel button (line 82), soconfirmationInputretains"DELETE"after a successful deletion. The next time the dialog opens the button is immediately enabled, completely defeating the guard.🛡️ Proposed fix
const [confirmationInput, setConfirmationInput] = useState<string>(""); + const handleOpenChange = (open: boolean) => { + if (!open) setConfirmationInput(""); + onOpenChange(open); + }; return ( - <Dialog open={isOpen} onOpenChange={onOpenChange}> + <Dialog open={isOpen} onOpenChange={handleOpenChange}> ... - <Button variant="outline" onClick={() => onOpenChange(false)}> + <Button variant="outline" onClick={() => handleOpenChange(false)}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/project/components/delete-project-dialog.tsx` around lines 32 - 35, confirmationInput is not cleared when the dialog closes so the delete guard is bypassed on reopen; fix it by resetting confirmationInput whenever the dialog is closed — either add a useEffect that watches isOpen and calls setConfirmationInput("") when isOpen becomes false, or wrap the onOpenChange prop passed to <Dialog> with a handler that calls setConfirmationInput("") before delegating to the original onOpenChange; also ensure the Cancel button’s handler similarly clears confirmationInput (reference confirmationInput, setConfirmationInput, onOpenChange, <Dialog>, and the Cancel button handler).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/web/src/project/components/delete-project-dialog.tsx`:
- Around line 32-35: confirmationInput is not cleared when the dialog closes so
the delete guard is bypassed on reopen; fix it by resetting confirmationInput
whenever the dialog is closed — either add a useEffect that watches isOpen and
calls setConfirmationInput("") when isOpen becomes false, or wrap the
onOpenChange prop passed to <Dialog> with a handler that calls
setConfirmationInput("") before delegating to the original onOpenChange; also
ensure the Cancel button’s handler similarly clears confirmationInput (reference
confirmationInput, setConfirmationInput, onOpenChange, <Dialog>, and the Cancel
button handler).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8336beab-a863-492f-89c0-e61f7d905c38
📒 Files selected for processing (1)
apps/web/src/project/components/delete-project-dialog.tsx
1 participant
Fixes #724
Changes made
Summary by CodeRabbit