feat: add delete dataset option to Settings dropdown#125
feat: add delete dataset option to Settings dropdown#125balasiddarthan22 wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds owner-only dataset deletion on the dataset page: new sequenceDiagram
participant User
participant SettingsDropdown
participant DatasetPage
participant ConfirmDeleteModal
participant ConvexAPI
participant Router
User->>SettingsDropdown: open settings
User->>SettingsDropdown: click "Delete dataset"
SettingsDropdown->>DatasetPage: invoke onDelete()
DatasetPage->>ConfirmDeleteModal: open modal
User->>ConfirmDeleteModal: confirm delete
ConfirmDeleteModal->>DatasetPage: onConfirm()
DatasetPage->>ConvexAPI: api.datasets.remove(datasetId)
ConvexAPI-->>DatasetPage: success / error
DatasetPage->>Router: router.replace("/dashboard")
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsDropdown
participant DatasetPage
participant ConfirmDeleteModal
participant ConvexAPI
participant Router
User->>SettingsDropdown: open settings
User->>SettingsDropdown: click "Delete dataset"
SettingsDropdown->>DatasetPage: onDelete()
DatasetPage->>ConfirmDeleteModal: set confirmDelete = true
User->>ConfirmDeleteModal: confirm
ConfirmDeleteModal->>DatasetPage: onConfirm()
DatasetPage->>ConvexAPI: removeDataset(dataset._id)
ConvexAPI-->>DatasetPage: result
alt success
DatasetPage->>Router: router.replace("/dashboard")
else error
DatasetPage->>DatasetPage: captureException(error)
end
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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
🤖 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/app/dataset/`[id]/page.tsx:
- Around line 247-256: The delete flow currently closes the confirmation modal
and allows repeated clicks before the async removeDataset completes; update the
confirm action handler and handleDelete to (1) introduce and use an "isDeleting"
boolean (or the mutation's isLoading) to short-circuit duplicate submits, (2) do
NOT close the modal until removeDataset resolves successfully, and (3) only call
router.push("/dashboard") after a successful await removeDataset, while on error
keep the modal open and call captureException(err, { operation:
"dataset_delete", datasetId: dataset._id }) so users see failure and cannot
trigger duplicate destructive requests. Ensure the confirm button is disabled
when isDeleting/isLoading is true.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ec46336c-ff9f-42dc-9222-779de9e08114
📒 Files selected for processing (1)
frontend/app/dataset/[id]/page.tsx
simantak-dabhade
left a comment
There was a problem hiding this comment.
Auth looks sound here: datasets.remove calls loadOwnedDataset, so deletion is enforced server-side and not just hidden in the UI.
The only concern I see is lifecycle-related: delete is available while a dataset is building/updating, but deleting doesn’t abort the active backend run. That can leave the workflow spending work until it notices the dataset is gone. Could we disable delete during active runs, or stop the run before deleting?
I'd agree with @simantak-dabhade here, you may abort an existing building task and delete a building dataset by doing the following:
|
… runs - Add deleting state to prevent repeated clicks while mutation is in flight - Keep modal open until deletion resolves, then navigate with router.replace - Disable Cancel and confirm button while deleting is in progress - Hide delete option when dataset is building or updating to avoid orphaned backend runs
8e4521e to
acc2fa9
Compare
|
Thanks for the feedback! I've disabled the delete option while the dataset is building or updating to avoid orphaned backend runs. I also addressed the CodeRabbit comment by adding a deleting guard to prevent duplicate submits and keeping the modal open until the mutation resolves. |
# Conflicts: # frontend/app/dataset/[id]/page.tsx
|
LGTM! Please review if the blocker can be removed @simantak-dabhade |
Summary
Problem
There was no way to delete a dataset from the UI. If a dataset ended up in a broken state (e.g. created with 0 columns), the user was stuck on a blank page with no recovery path and no way to clean it up.
Test plan