From d31cd517cd3f1d64cc408a99d17ef98ccabdfee3 Mon Sep 17 00:00:00 2001 From: Satya Patel Date: Thu, 23 Apr 2026 13:01:05 -0700 Subject: [PATCH] fix(desktop): show v1 uncommitted-changes banner instead of second delete prompt The v2 workspace delete flow showed two warnings: a generic confirm pane, then a "Uncommitted changes in worktree" pane after destroy hit a conflict. Surface the v1 yellow banner inline on the confirm pane via the existing canDelete preflight, force when warnings are shown, and silently retry on the rare race so the user only ever sees one prompt. --- .../DashboardSidebarDeleteDialog.tsx | 41 ++++--------- .../components/ConflictPane/ConflictPane.tsx | 57 ----------------- .../components/ConflictPane/index.ts | 1 - .../DestroyConfirmPane/DestroyConfirmPane.tsx | 24 ++++++-- .../UnknownErrorPane/UnknownErrorPane.tsx | 56 ----------------- .../components/UnknownErrorPane/index.ts | 1 - .../useDestroyDialogState.ts | 61 +++++++++++-------- 7 files changed, 69 insertions(+), 172 deletions(-) delete mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/ConflictPane/ConflictPane.tsx delete mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/ConflictPane/index.ts delete mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/UnknownErrorPane/UnknownErrorPane.tsx delete mode 100644 apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/UnknownErrorPane/index.ts diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/DashboardSidebarDeleteDialog.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/DashboardSidebarDeleteDialog.tsx index 2dea5a47ecc..8f10b423181 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/DashboardSidebarDeleteDialog.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/DashboardSidebarDeleteDialog.tsx @@ -1,7 +1,5 @@ -import { ConflictPane } from "./components/ConflictPane"; import { DestroyConfirmPane } from "./components/DestroyConfirmPane"; import { TeardownFailedPane } from "./components/TeardownFailedPane"; -import { UnknownErrorPane } from "./components/UnknownErrorPane"; import { useDestroyDialogState } from "./hooks/useDestroyDialogState"; interface DashboardSidebarDeleteDialogProps { @@ -14,10 +12,10 @@ interface DashboardSidebarDeleteDialogProps { } /** - * Dispatches between confirm / conflict / teardown-failed / unknown-error - * panes based on the error returned by `workspaceCleanup.destroy`. The - * destroy itself runs in the background under a toast — this dialog is - * only on screen when the user has a decision to make. + * Dispatches between confirm and teardown-failed panes based on the error + * returned by `workspaceCleanup.destroy`. Dirty-worktree state is surfaced + * inline as a banner on the confirm pane so the user only sees one warning + * before the destroy runs. */ export function DashboardSidebarDeleteDialog({ workspaceId, @@ -29,27 +27,20 @@ export function DashboardSidebarDeleteDialog({ const { deleteBranch, setDeleteBranch, + hasChanges, + hasUnpushedCommits, + isCheckingStatus, error, - clearError, handleOpenChange, run, } = useDestroyDialogState({ workspaceId, workspaceName, + open, onOpenChange, onDeleted, }); - if (error?.kind === "conflict") { - return ( - run(true)} - /> - ); - } - if (error?.kind === "teardown-failed") { return ( - ); - } + const hasWarnings = hasChanges || hasUnpushedCommits; return ( run(false)} + hasChanges={hasChanges} + hasUnpushedCommits={hasUnpushedCommits} + isCheckingStatus={isCheckingStatus} + onConfirm={() => run(hasWarnings)} /> ); } diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/ConflictPane/ConflictPane.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/ConflictPane/ConflictPane.tsx deleted file mode 100644 index 3a96e54dca6..00000000000 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/ConflictPane/ConflictPane.tsx +++ /dev/null @@ -1,57 +0,0 @@ -import { - AlertDialog, - AlertDialogContent, - AlertDialogDescription, - AlertDialogFooter, - AlertDialogHeader, - AlertDialogTitle, -} from "@superset/ui/alert-dialog"; -import { Button } from "@superset/ui/button"; - -interface ConflictPaneProps { - open: boolean; - onOpenChange: (open: boolean) => void; - /** Re-runs destroy with `force: true`. */ - onForceDelete: () => void; -} - -/** Shown when the preflight dirty-worktree check blocks destroy. */ -export function ConflictPane({ - open, - onOpenChange, - onForceDelete, -}: ConflictPaneProps) { - return ( - - - - - Uncommitted changes in worktree - - - The worktree has uncommitted changes. Delete anyway will discard - them. - - - - - - - - - ); -} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/ConflictPane/index.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/ConflictPane/index.ts deleted file mode 100644 index 76a14754291..00000000000 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/ConflictPane/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { ConflictPane } from "./ConflictPane"; diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/DestroyConfirmPane/DestroyConfirmPane.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/DestroyConfirmPane/DestroyConfirmPane.tsx index 441816c2ba2..b54ede75067 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/DestroyConfirmPane/DestroyConfirmPane.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/DestroyConfirmPane/DestroyConfirmPane.tsx @@ -17,23 +17,25 @@ interface DestroyConfirmPaneProps { workspaceName: string; deleteBranch: boolean; onDeleteBranchChange: (next: boolean) => void; + hasChanges: boolean; + hasUnpushedCommits: boolean; + isCheckingStatus: boolean; onConfirm: () => void; } -/** - * Default pane: the first click on "Delete". Offers the branch opt-in. - * Confirm hands off to the parent which closes the dialog and runs the - * destroy under a toast — no in-dialog pending state. - */ export function DestroyConfirmPane({ open, onOpenChange, workspaceName, deleteBranch, onDeleteBranchChange, + hasChanges, + hasUnpushedCommits, + isCheckingStatus, onConfirm, }: DestroyConfirmPaneProps) { const checkboxId = useId(); + const hasWarnings = hasChanges || hasUnpushedCommits; return ( @@ -46,6 +48,17 @@ export function DestroyConfirmPane({ also be removed. + {hasWarnings && ( +
+
+ {hasChanges && hasUnpushedCommits + ? "Has uncommitted changes and unpushed commits" + : hasChanges + ? "Has uncommitted changes" + : "Has unpushed commits"} +
+
+ )}
Delete diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/UnknownErrorPane/UnknownErrorPane.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/UnknownErrorPane/UnknownErrorPane.tsx deleted file mode 100644 index 60c3e9ae216..00000000000 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/UnknownErrorPane/UnknownErrorPane.tsx +++ /dev/null @@ -1,56 +0,0 @@ -import { - AlertDialog, - AlertDialogContent, - AlertDialogDescription, - AlertDialogFooter, - AlertDialogHeader, - AlertDialogTitle, -} from "@superset/ui/alert-dialog"; -import { Button } from "@superset/ui/button"; - -interface UnknownErrorPaneProps { - open: boolean; - onOpenChange: (open: boolean) => void; - message: string; - /** Clears the error and returns to the default confirm pane. */ - onRetry: () => void; -} - -/** Fallback for TRPC/network errors that aren't CONFLICT or TEARDOWN_FAILED. */ -export function UnknownErrorPane({ - open, - onOpenChange, - message, - onRetry, -}: UnknownErrorPaneProps) { - return ( - - - - - Delete failed - - {message} - - - - - - - - ); -} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/UnknownErrorPane/index.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/UnknownErrorPane/index.ts deleted file mode 100644 index 967bb538ef0..00000000000 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/UnknownErrorPane/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { UnknownErrorPane } from "./UnknownErrorPane"; diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/useDestroyDialogState.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/useDestroyDialogState.ts index 1480d5cdc0e..4f916cc1922 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/useDestroyDialogState.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/useDestroyDialogState.ts @@ -1,39 +1,29 @@ import { toast } from "@superset/ui/sonner"; import { useCallback, useRef, useState } from "react"; +import type { DestroyWorkspaceSuccess } from "renderer/hooks/host-service/useDestroyWorkspace"; import { type DestroyWorkspaceError, useDestroyWorkspace, } from "renderer/hooks/host-service/useDestroyWorkspace"; import { useV2UserPreferences } from "renderer/hooks/useV2UserPreferences/useV2UserPreferences"; +import { electronTrpc } from "renderer/lib/electron-trpc"; import { useNavigateAwayFromWorkspace } from "renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useNavigateAwayFromWorkspace"; import { useDeletingWorkspaces } from "renderer/routes/_authenticated/providers/DeletingWorkspacesProvider"; +const STATUS_STALE_TIME_MS = 5_000; + interface UseDestroyDialogStateOptions { workspaceId: string; workspaceName: string; + open: boolean; onOpenChange: (open: boolean) => void; onDeleted?: () => void; } -/** - * Drives the delete flow for `DashboardSidebarDeleteDialog`. - * - * UX pattern: - * - On confirm, navigate off the workspace first (if viewing it), - * close the dialog, mark the workspace deleting (row hides - * optimistically), fire a one-shot "Deleting..." toast, and let - * destroy run in the background. A loading toast across the 10–20s - * teardown feels worse than fire-and-forget + hidden row. - * - On success, `onDeleted` removes the row from sidebar state. - * - On error, `clearDeleting` runs in the `finally` block so the row - * reappears. For decision-required errors (CONFLICT, TEARDOWN_FAILED) - * we reopen the dialog in the matching error pane so the user can - * force-retry with full context. - * - For unknown errors we just toast.error — no reopen. - */ export function useDestroyDialogState({ workspaceId, workspaceName, + open, onOpenChange, onDeleted, }: UseDestroyDialogStateOptions) { @@ -44,6 +34,19 @@ export function useDestroyDialogState({ const { preferences, setDeleteLocalBranch: setDeleteBranch } = useV2UserPreferences(); const deleteBranch = preferences.deleteLocalBranch; + + const { data: canDeleteData, isPending: isCheckingStatus } = + electronTrpc.workspaces.canDelete.useQuery( + { id: workspaceId }, + { + enabled: open, + staleTime: STATUS_STALE_TIME_MS, + refetchOnWindowFocus: false, + }, + ); + const hasChanges = canDeleteData?.hasChanges ?? false; + const hasUnpushedCommits = canDeleteData?.hasUnpushedCommits ?? false; + const [error, setError] = useState(null); const inFlight = useRef(false); @@ -55,13 +58,8 @@ export function useDestroyDialogState({ [onOpenChange], ); - const clearError = useCallback(() => setError(null), []); - const run = useCallback( async (force: boolean) => { - // Guard against double-submit: optimistic close + async mutate means - // a rapid second click (from the same pane or a re-opened error pane) - // could fire destroy twice before the first resolves. if (inFlight.current) return; inFlight.current = true; @@ -75,12 +73,25 @@ export function useDestroyDialogState({ toast(`Deleting "${workspaceName}"...`); try { - const result = await destroy({ deleteBranch, force }); + let result: DestroyWorkspaceSuccess; + try { + result = await destroy({ deleteBranch, force }); + } catch (firstErr) { + const e = firstErr as DestroyWorkspaceError; + // Race: preflight said clean but worktree was dirty by the time + // destroy ran. The user already confirmed once — don't make them + // confirm a second "uncommitted changes" warning, just force. + if (e.kind === "conflict" && !force) { + result = await destroy({ deleteBranch, force: true }); + } else { + throw firstErr; + } + } for (const warning of result.warnings) toast.warning(warning); onDeleted?.(); } catch (err) { const e = err as DestroyWorkspaceError; - if (e.kind === "conflict" || e.kind === "teardown-failed") { + if (e.kind === "teardown-failed") { setError(e); onOpenChange(true); } else { @@ -107,8 +118,10 @@ export function useDestroyDialogState({ return { deleteBranch, setDeleteBranch, + hasChanges, + hasUnpushedCommits, + isCheckingStatus, error, - clearError, handleOpenChange, run, };