Add project deletion from sidebar context menu#50
Conversation
|
No actionable comments were generated in the recent review. 🎉 WalkthroughAdds a project-level right-click context menu that confirms and invokes project deletion via the Electron API, prevents deleting projects that still have threads, shows toasts for warnings/errors, and dispatches a new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Sidebar
participant "Electron API" as API
participant Store
participant Toasts
User->>Sidebar: Right-click project
Sidebar->>User: Show context menu (Delete)
User->>Sidebar: Select Delete
Sidebar->>Sidebar: Check for project threads
alt project has threads
Sidebar->>Toasts: Show warning toast, abort
else project empty or user confirms
Sidebar->>User: Show confirmation
User->>Sidebar: Confirm
Sidebar->>API: api.projects.remove(projectId, coords)
API-->>Sidebar: Success / Error
alt Error
Sidebar->>Toasts: Show error toast
else Success
Sidebar->>Store: Dispatch DELETE_PROJECT(projectId)
Store->>Store: Remove project, remove threads, update activeThreadId
Store-->>Sidebar: New state
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Add project deletion from the sidebar context menu in
|
Greptile SummaryThis PR adds project deletion functionality with proper safeguards and confirmation dialogs. The implementation prevents deletion of projects with active threads, requiring users to delete all threads first. The reducer properly handles thread cleanup and active thread reassignment when a project is deleted. Key changes:
Code quality:
Confidence Score: 4/5
Important Files Changed
Flowchartflowchart TD
A[User right-clicks project] --> B[Show context menu]
B --> C{User clicks Delete?}
C -->|No| Z[End]
C -->|Yes| D[Find project by ID]
D --> E{Project exists?}
E -->|No| Z
E -->|Yes| F[Check project threads]
F --> G{Has threads?}
G -->|Yes| H[Show warning toast]
H --> I[Prevent deletion]
I --> Z
G -->|No| J[Show confirmation dialog]
J --> K{User confirms?}
K -->|No| Z
K -->|Yes| L{isElectron?}
L -->|Yes| M[Call api.projects.remove]
M --> N{Success?}
N -->|No| O[Show error toast]
O --> Z
N -->|Yes| P[Dispatch DELETE_PROJECT]
L -->|No| P
P --> Q[Filter out project]
Q --> R[Filter out threads]
R --> S[Reassign activeThreadId]
S --> T[Return new state]
T --> Z
Last reviewed commit: 4c49b8a |
| const handleProjectContextMenu = useCallback( | ||
| async (projectId: string, position: { x: number; y: number }) => { | ||
| if (!api) return; | ||
| const clicked = await api.contextMenu.show([{ id: "delete", label: "Delete" }], position); | ||
| if (clicked !== "delete") return; | ||
|
|
||
| const project = state.projects.find((entry) => entry.id === projectId); | ||
| if (!project) return; | ||
|
|
||
| if (isElectron) { | ||
| try { | ||
| await api.projects.remove({ id: projectId }); | ||
| } catch { | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing confirmation dialog before deleting project. Thread deletion (line 278) uses api.dialogs.confirm to warn users - project deletion should too since it's more destructive (deletes all threads)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/src/components/Sidebar.tsx`:
- Around line 334-340: The current deletion path in Sidebar.tsx silently returns
on api.projects.remove failure (inside the isElectron branch) leaving the user
without feedback; update the try/catch around api.projects.remove to catch the
error, log it, and surface a user-visible notification (e.g., show an error
dialog or toast via your existing UI notification utility) that the project
deletion failed and include the error message; ensure you do not proceed with
local UI state changes (e.g., any code that removes the project from state or
navigation) when the backend delete fails so the UI remains consistent with the
backend.
- Around line 325-369: handleProjectContextMenu currently deletes project
threads but omits the worktree cleanup present in handleThreadContextMenu;
before closing terminals/stopping sessions for each thread inside
handleProjectContextMenu, detect any thread.worktree (e.g. thread.worktree?.path
or thread.worktreeId used in your code) and call the existing
removeWorktreeMutation with the appropriate identifier/path to remove the
on-disk worktree, awaiting it and swallowing expected errors like the
thread-level logic does; also add removeWorktreeMutation to the hook dependency
array for handleProjectContextMenu so the linting and behavior remain correct.
🧹 Nitpick comments (1)
apps/web/src/components/Sidebar.tsx (1)
325-332: Consider adding a confirmation dialog before deleting a project.Deleting a project removes all associated threads and their data. Unlike thread deletion (which confirms before removing worktrees), project deletion proceeds immediately after the context menu click. This is a destructive action that could benefit from a confirmation step.
💬 Suggested confirmation dialog
const project = state.projects.find((entry) => entry.id === projectId); if (!project) return; + const threadCount = state.threads.filter((t) => t.projectId === projectId).length; + const confirmed = await api.dialogs.confirm( + `Delete project "${project.name}" and its ${threadCount} thread(s)?` + ); + if (!confirmed) return; + if (isElectron) {
- Add project right-click Delete action in `Sidebar` and clean up sessions/terminals before removal - Add `DELETE_PROJECT` reducer action to remove the project, its threads, and reset active thread - Add reducer test coverage for project deletion behavior
- Prompt before deleting a project with thread and linked worktree counts - Best-effort remove orphaned worktrees when project threads are deleted - Show toast with error details if project deletion fails
- Show warning toast when attempting to delete a non-empty project - Simplify confirmation copy to emphasize irreversible deletion - Remove thread/worktree/session cleanup from project delete flow
32af59b to
4c49b8a
Compare
|
@greptileai review |
Summary
Delete) in the sidebar.DELETE_PROJECTreducer support to remove the project, prune its threads, and reassignactiveThreadIdsafely.Testing
apps/web/src/store.test.ts(deletes a project and all of its threads).Summary by CodeRabbit
New Features
Tests