Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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,
Expand All @@ -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 (
<ConflictPane
open={open}
onOpenChange={handleOpenChange}
onForceDelete={() => run(true)}
/>
);
}

if (error?.kind === "teardown-failed") {
return (
<TeardownFailedPane
Expand All @@ -61,16 +52,7 @@ export function DashboardSidebarDeleteDialog({
);
}

if (error?.kind === "unknown") {
return (
<UnknownErrorPane
open={open}
onOpenChange={handleOpenChange}
message={error.message}
onRetry={clearError}
/>
);
}
const hasWarnings = hasChanges || hasUnpushedCommits;

return (
<DestroyConfirmPane
Expand All @@ -79,7 +61,10 @@ export function DashboardSidebarDeleteDialog({
workspaceName={workspaceName}
deleteBranch={deleteBranch}
onDeleteBranchChange={setDeleteBranch}
onConfirm={() => run(false)}
hasChanges={hasChanges}
hasUnpushedCommits={hasUnpushedCommits}
isCheckingStatus={isCheckingStatus}
onConfirm={() => run(hasWarnings)}
/>
);
}

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<AlertDialog open={open} onOpenChange={onOpenChange}>
<AlertDialogContent className="max-w-[340px] gap-0 p-0">
Expand All @@ -46,6 +48,17 @@ export function DestroyConfirmPane({
also be removed.
</AlertDialogDescription>
</AlertDialogHeader>
{hasWarnings && (
<div className="px-4 pb-2">
<div className="text-xs text-yellow-700 dark:text-yellow-400 bg-yellow-50 dark:bg-yellow-500/10 border border-yellow-200 dark:border-yellow-500/20 rounded-md px-2.5 py-1.5">
{hasChanges && hasUnpushedCommits
? "Has uncommitted changes and unpushed commits"
: hasChanges
? "Has uncommitted changes"
: "Has unpushed commits"}
</div>
</div>
)}
<div className="px-4 pb-2">
<div className="flex items-center gap-2">
<Checkbox
Expand Down Expand Up @@ -77,6 +90,7 @@ export function DestroyConfirmPane({
size="sm"
className="h-7 px-3 text-xs"
onClick={onConfirm}
disabled={isCheckingStatus}
>
Delete
</Button>
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -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;
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
const hasUnpushedCommits = canDeleteData?.hasUnpushedCommits ?? false;

const [error, setError] = useState<DestroyWorkspaceError | null>(null);
const inFlight = useRef(false);

Expand All @@ -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;

Expand All @@ -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 {
Expand All @@ -107,8 +118,10 @@ export function useDestroyDialogState({
return {
deleteBranch,
setDeleteBranch,
hasChanges,
hasUnpushedCommits,
isCheckingStatus,
error,
clearError,
handleOpenChange,
run,
};
Expand Down
Loading