fix(web): confirm before archiving from PR merged/closed banners (#1619)
* fix(web): require confirmation before archiving from PR merged/closed banners The Archive link in the PR merged/closed status-bar banners archived the task immediately, unlike every other archive surface. Route it through the shared TaskArchiveConfirmDialog (cleanup summary + subtask cascade), and move the banners into pr-archive-banners.tsx since chat-input-area.tsx was over the file-size lint limit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(web): cover cascade passthrough in banner archive confirmation Review follow-up: the subtask cascade checkbox is an acceptance criterion for the banner archive flow, but tests only asserted cascade: false. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(e2e): verify cancel path of PR banner archive confirmation QA follow-up: exercise the dialog cancel path in a real browser — cancel must close the dialog, keep the banner, and archive nothing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(web): expose pending state on banner archive confirmation Thread isPending from useBannerArchiveConfirm into TaskArchiveConfirmDialog as isArchiving, matching the other archive surfaces. Guards double-submit when the dialog is reopened while an archive is still in flight on a slow connection. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(e2e): scope archive-cancel click to the alertdialog Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
João Salavisa
@@ -1,32 +1,16 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
act,
|
||||
cleanup,
|
||||
fireEvent,
|
||||
render,
|
||||
renderHook,
|
||||
screen,
|
||||
waitFor,
|
||||
} from "@testing-library/react";
|
||||
import { act, cleanup, renderHook } from "@testing-library/react";
|
||||
|
||||
const archiveAndSwitchMock = vi.fn();
|
||||
const toastMock = vi.fn();
|
||||
const handleSendMessageMock = vi.fn();
|
||||
const taskPRsByTaskId = vi.hoisted(() => ({
|
||||
value: {
|
||||
"task-1": [{ pr_number: 42, state: "merged" }],
|
||||
} as Record<string, Array<{ pr_number: number; state: string }>>,
|
||||
}));
|
||||
|
||||
const mockState = {};
|
||||
|
||||
vi.mock("@/components/state-provider", () => ({
|
||||
useAppStore: (selector: (state: typeof mockState) => unknown) => selector(mockState),
|
||||
useAppStoreApi: () => ({ getState: () => mockState }),
|
||||
}));
|
||||
|
||||
vi.mock("@/hooks/use-task-actions", () => ({
|
||||
useArchiveAndSwitchTask: () => archiveAndSwitchMock,
|
||||
}));
|
||||
|
||||
vi.mock("@/components/toast-provider", () => ({
|
||||
useToast: () => ({ toast: toastMock }),
|
||||
}));
|
||||
@@ -48,6 +32,11 @@ vi.mock("@/components/task/chat/todo-indicator", () => ({
|
||||
TodoIndicator: () => null,
|
||||
}));
|
||||
|
||||
vi.mock("./pr-archive-banners", () => ({
|
||||
PRMergedBanner: () => null,
|
||||
PRClosedBanner: () => null,
|
||||
}));
|
||||
|
||||
vi.mock("@/hooks/use-keyboard-shortcut", () => ({
|
||||
useKeyboardShortcut: () => undefined,
|
||||
}));
|
||||
@@ -77,30 +66,10 @@ vi.mock("@/lib/ws/connection", () => ({
|
||||
getWebSocketClient: () => ({ send: vi.fn() }),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/local-storage", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("@/lib/local-storage")>()),
|
||||
markPRClosedBannerDismissed: vi.fn(),
|
||||
markPRMergedBannerDismissed: vi.fn(),
|
||||
wasPRClosedBannerDismissed: () => false,
|
||||
wasPRMergedBannerDismissed: () => false,
|
||||
}));
|
||||
|
||||
const mockState = {
|
||||
taskPRs: {
|
||||
get byTaskId() {
|
||||
return taskPRsByTaskId.value;
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
import { PRMergedBanner, useSubmitHandler } from "./chat-input-area";
|
||||
import { useSubmitHandler } from "./chat-input-area";
|
||||
|
||||
beforeEach(() => {
|
||||
archiveAndSwitchMock.mockResolvedValue(undefined);
|
||||
handleSendMessageMock.mockResolvedValue(undefined);
|
||||
taskPRsByTaskId.value = {
|
||||
"task-1": [{ pr_number: 42, state: "merged" }],
|
||||
};
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -109,31 +78,6 @@ afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe("PRMergedBanner", () => {
|
||||
it("archives without showing a success toast", async () => {
|
||||
render(<PRMergedBanner taskId="task-1" />);
|
||||
|
||||
fireEvent.click(screen.getByTestId("pr-merged-archive-button"));
|
||||
|
||||
await waitFor(() => expect(archiveAndSwitchMock).toHaveBeenCalledWith("task-1"));
|
||||
expect(toastMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps the failure toast when archive fails", async () => {
|
||||
archiveAndSwitchMock.mockRejectedValueOnce(new Error("archive failed"));
|
||||
render(<PRMergedBanner taskId="task-1" />);
|
||||
|
||||
fireEvent.click(screen.getByTestId("pr-merged-archive-button"));
|
||||
|
||||
await waitFor(() =>
|
||||
expect(toastMock).toHaveBeenCalledWith({
|
||||
description: "Failed to archive task",
|
||||
variant: "error",
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("useSubmitHandler", () => {
|
||||
function panelState(overrides = {}) {
|
||||
return {
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback, useState, type ReactNode } from "react";
|
||||
import { IconArrowRight, IconGitMerge, IconGitPullRequestClosed, IconX } from "@tabler/icons-react";
|
||||
import { IconArrowRight } from "@tabler/icons-react";
|
||||
import { Button } from "@kandev/ui/button";
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
|
||||
import { TodoIndicator } from "./todo-indicator";
|
||||
import { PRMergedBanner, PRClosedBanner } from "./pr-archive-banners";
|
||||
import { PRStatusChip } from "@/components/github/pr-status-chip";
|
||||
import { ShareButton, shareableSessionStateClient } from "@/components/task/share/share-button";
|
||||
import { getWebSocketClient } from "@/lib/ws/connection";
|
||||
@@ -28,14 +29,7 @@ import {
|
||||
} from "@/lib/state/slices/comments/format";
|
||||
import { usePlanActions } from "@/hooks/domains/kanban/use-plan-actions";
|
||||
import { useExecutorEnvironmentAvailability } from "@/hooks/domains/session/use-executor-environment-availability";
|
||||
import { useArchiveAndSwitchTask } from "@/hooks/use-task-actions";
|
||||
import { useToast } from "@/components/toast-provider";
|
||||
import {
|
||||
markPRClosedBannerDismissed,
|
||||
markPRMergedBannerDismissed,
|
||||
wasPRClosedBannerDismissed,
|
||||
wasPRMergedBannerDismissed,
|
||||
} from "@/lib/local-storage";
|
||||
import type { DiffComment } from "@/lib/diff/types";
|
||||
import type { useChatPanelState } from "./use-chat-panel-state";
|
||||
|
||||
@@ -267,135 +261,6 @@ export function useChatPanelHandlers(
|
||||
return { handleCancelTurn };
|
||||
}
|
||||
|
||||
// Shared archive-task action for the terminal-state banners: archives the task,
|
||||
// switches to the next one, and only toasts on failure.
|
||||
function useArchiveTaskAction(taskId: string) {
|
||||
const archiveAndSwitch = useArchiveAndSwitchTask();
|
||||
const { toast } = useToast();
|
||||
return useCallback(async () => {
|
||||
try {
|
||||
await archiveAndSwitch(taskId);
|
||||
} catch {
|
||||
toast({ description: "Failed to archive task", variant: "error" });
|
||||
}
|
||||
}, [taskId, archiveAndSwitch, toast]);
|
||||
}
|
||||
|
||||
// Presentational banner shared by PRMergedBanner / PRClosedBanner — an icon, a
|
||||
// message, and Archive + Dismiss controls. Colors/icon/testIds are supplied by
|
||||
// the caller so the two variants stay visually distinct.
|
||||
function ArchiveDismissBanner({
|
||||
testIdPrefix,
|
||||
icon,
|
||||
text,
|
||||
containerClass,
|
||||
archiveClass,
|
||||
dismissClass,
|
||||
onArchive,
|
||||
onDismiss,
|
||||
}: {
|
||||
testIdPrefix: string;
|
||||
icon: ReactNode;
|
||||
text: string;
|
||||
containerClass: string;
|
||||
archiveClass: string;
|
||||
dismissClass: string;
|
||||
onArchive: () => void;
|
||||
onDismiss: () => void;
|
||||
}) {
|
||||
return (
|
||||
<div data-testid={`${testIdPrefix}-banner`} className={containerClass}>
|
||||
{icon}
|
||||
<span className="flex-1">{text}</span>
|
||||
<button
|
||||
type="button"
|
||||
data-testid={`${testIdPrefix}-archive-button`}
|
||||
onClick={onArchive}
|
||||
className={archiveClass}
|
||||
>
|
||||
Archive
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
aria-label="Dismiss"
|
||||
data-testid={`${testIdPrefix}-dismiss-button`}
|
||||
onClick={onDismiss}
|
||||
className={dismissClass}
|
||||
>
|
||||
<IconX className="h-3 w-3" />
|
||||
</button>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
export function PRMergedBanner({ taskId }: { taskId: string }) {
|
||||
const taskPRs = useAppStore((state) => state.taskPRs.byTaskId[taskId]);
|
||||
const [dismissed, setDismissed] = useState(() => wasPRMergedBannerDismissed(taskId));
|
||||
const handleArchive = useArchiveTaskAction(taskId);
|
||||
|
||||
const handleDismiss = useCallback(() => {
|
||||
markPRMergedBannerDismissed(taskId);
|
||||
setDismissed(true);
|
||||
}, [taskId]);
|
||||
|
||||
// Multi-repo: only show "ready to archive" once every PR is merged. A
|
||||
// single merged repo with others still open means the task isn't done yet.
|
||||
const allMerged = !!taskPRs && taskPRs.length > 0 && taskPRs.every((pr) => pr.state === "merged");
|
||||
if (!allMerged || dismissed) return null;
|
||||
|
||||
const bannerText =
|
||||
taskPRs.length === 1
|
||||
? `PR #${taskPRs[0].pr_number} has been merged. You can archive this task.`
|
||||
: `All ${taskPRs.length} PRs have been merged. You can archive this task.`;
|
||||
|
||||
return (
|
||||
<ArchiveDismissBanner
|
||||
testIdPrefix="pr-merged"
|
||||
icon={<IconGitMerge className="h-3.5 w-3.5 shrink-0" />}
|
||||
text={bannerText}
|
||||
containerClass="flex flex-1 items-center gap-2 rounded-md bg-purple-500/10 px-2 py-1 text-purple-600 dark:text-purple-400"
|
||||
archiveClass="underline underline-offset-2 hover:text-purple-700 dark:hover:text-purple-300 cursor-pointer"
|
||||
dismissClass="p-0.5 hover:bg-purple-500/10 rounded cursor-pointer"
|
||||
onArchive={handleArchive}
|
||||
onDismiss={handleDismiss}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
export function PRClosedBanner({ taskId }: { taskId: string }) {
|
||||
const taskPRs = useAppStore((state) => state.taskPRs.byTaskId[taskId]);
|
||||
const [dismissed, setDismissed] = useState(() => wasPRClosedBannerDismissed(taskId));
|
||||
const handleArchive = useArchiveTaskAction(taskId);
|
||||
|
||||
const handleDismiss = useCallback(() => {
|
||||
markPRClosedBannerDismissed(taskId);
|
||||
setDismissed(true);
|
||||
}, [taskId]);
|
||||
|
||||
// Mirror the merged banner's all-or-nothing rule: show only once every PR is
|
||||
// closed-without-merging. A mix of merged + closed shows neither banner.
|
||||
const allClosed = !!taskPRs && taskPRs.length > 0 && taskPRs.every((pr) => pr.state === "closed");
|
||||
if (!allClosed || dismissed) return null;
|
||||
|
||||
const bannerText =
|
||||
taskPRs.length === 1
|
||||
? `PR #${taskPRs[0].pr_number} was closed without merging. You can archive this task.`
|
||||
: `All ${taskPRs.length} PRs were closed without merging. You can archive this task.`;
|
||||
|
||||
return (
|
||||
<ArchiveDismissBanner
|
||||
testIdPrefix="pr-closed"
|
||||
icon={<IconGitPullRequestClosed className="h-3.5 w-3.5 shrink-0" />}
|
||||
text={bannerText}
|
||||
containerClass="flex flex-1 items-center gap-2 rounded-md bg-red-500/10 px-2 py-1 text-red-600 dark:text-red-400"
|
||||
archiveClass="underline underline-offset-2 hover:text-red-700 dark:hover:text-red-300 cursor-pointer"
|
||||
dismissClass="p-0.5 hover:bg-red-500/10 rounded cursor-pointer"
|
||||
onArchive={handleArchive}
|
||||
onDismiss={handleDismiss}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
type TodoDisplayItem = {
|
||||
text: string;
|
||||
done?: boolean;
|
||||
|
||||
@@ -0,0 +1,183 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react";
|
||||
|
||||
const archiveAndSwitchMock = vi.fn();
|
||||
const toastMock = vi.fn();
|
||||
const mockGetSubtaskCount = vi.fn();
|
||||
const taskPRsByTaskId = vi.hoisted(() => ({
|
||||
value: {
|
||||
"task-1": [{ pr_number: 42, state: "merged" }],
|
||||
} as Record<string, Array<{ pr_number: number; state: string }>>,
|
||||
}));
|
||||
|
||||
const mockState = {
|
||||
taskPRs: {
|
||||
get byTaskId() {
|
||||
return taskPRsByTaskId.value;
|
||||
},
|
||||
},
|
||||
kanban: {
|
||||
tasks: [{ id: "task-1", title: "Task One", primaryExecutorType: "worktree" }],
|
||||
},
|
||||
kanbanMulti: { snapshots: {} },
|
||||
};
|
||||
|
||||
vi.mock("@/components/state-provider", () => ({
|
||||
useAppStore: (selector: (state: typeof mockState) => unknown) => selector(mockState),
|
||||
useAppStoreApi: () => ({ getState: () => mockState }),
|
||||
}));
|
||||
|
||||
vi.mock("@/hooks/use-task-actions", () => ({
|
||||
useArchiveAndSwitchTask: () => archiveAndSwitchMock,
|
||||
}));
|
||||
|
||||
vi.mock("@/components/toast-provider", () => ({
|
||||
useToast: () => ({ toast: toastMock }),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/api", () => ({
|
||||
getSubtaskCount: (...args: unknown[]) => mockGetSubtaskCount(...args),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/local-storage", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("@/lib/local-storage")>()),
|
||||
markPRClosedBannerDismissed: vi.fn(),
|
||||
markPRMergedBannerDismissed: vi.fn(),
|
||||
wasPRClosedBannerDismissed: () => false,
|
||||
wasPRMergedBannerDismissed: () => false,
|
||||
}));
|
||||
|
||||
import { PRClosedBanner, PRMergedBanner } from "./pr-archive-banners";
|
||||
|
||||
const MERGED_ARCHIVE_BUTTON = "pr-merged-archive-button";
|
||||
const MERGED_ARCHIVE_CONFIRM = "pr-merged-archive-confirm";
|
||||
|
||||
beforeEach(() => {
|
||||
archiveAndSwitchMock.mockResolvedValue(undefined);
|
||||
mockGetSubtaskCount.mockResolvedValue({ count: 0 });
|
||||
taskPRsByTaskId.value = {
|
||||
"task-1": [{ pr_number: 42, state: "merged" }],
|
||||
};
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.restoreAllMocks();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe("PRMergedBanner", () => {
|
||||
it("opens the confirmation dialog instead of archiving directly", async () => {
|
||||
render(<PRMergedBanner taskId="task-1" />);
|
||||
|
||||
fireEvent.click(screen.getByTestId(MERGED_ARCHIVE_BUTTON));
|
||||
|
||||
expect(await screen.findByTestId(MERGED_ARCHIVE_CONFIRM)).toBeTruthy();
|
||||
expect(screen.getByText(/Are you sure you want to archive "Task One"\?/)).toBeTruthy();
|
||||
expect(archiveAndSwitchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("archives after confirming, without a success toast", async () => {
|
||||
render(<PRMergedBanner taskId="task-1" />);
|
||||
|
||||
fireEvent.click(screen.getByTestId(MERGED_ARCHIVE_BUTTON));
|
||||
fireEvent.click(await screen.findByTestId(MERGED_ARCHIVE_CONFIRM));
|
||||
|
||||
await waitFor(() =>
|
||||
expect(archiveAndSwitchMock).toHaveBeenCalledWith("task-1", { cascade: false }),
|
||||
);
|
||||
expect(toastMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not archive when the dialog is cancelled", async () => {
|
||||
render(<PRMergedBanner taskId="task-1" />);
|
||||
|
||||
fireEvent.click(screen.getByTestId(MERGED_ARCHIVE_BUTTON));
|
||||
fireEvent.click(await screen.findByText("Cancel"));
|
||||
|
||||
await waitFor(() => expect(screen.queryByTestId(MERGED_ARCHIVE_CONFIRM)).toBeNull());
|
||||
expect(archiveAndSwitchMock).not.toHaveBeenCalled();
|
||||
expect(screen.getByTestId("pr-merged-banner")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("keeps the failure toast when archive fails", async () => {
|
||||
archiveAndSwitchMock.mockRejectedValueOnce(new Error("archive failed"));
|
||||
render(<PRMergedBanner taskId="task-1" />);
|
||||
|
||||
fireEvent.click(screen.getByTestId(MERGED_ARCHIVE_BUTTON));
|
||||
fireEvent.click(await screen.findByTestId(MERGED_ARCHIVE_CONFIRM));
|
||||
|
||||
await waitFor(() =>
|
||||
expect(toastMock).toHaveBeenCalledWith({
|
||||
description: "Failed to archive task",
|
||||
variant: "error",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("passes cascade=true through when the subtask checkbox is ticked", async () => {
|
||||
mockGetSubtaskCount.mockResolvedValue({ count: 2 });
|
||||
render(<PRMergedBanner taskId="task-1" />);
|
||||
|
||||
fireEvent.click(screen.getByTestId(MERGED_ARCHIVE_BUTTON));
|
||||
fireEvent.click(await screen.findByTestId("archive-cascade-checkbox"));
|
||||
fireEvent.click(screen.getByTestId(MERGED_ARCHIVE_CONFIRM));
|
||||
|
||||
await waitFor(() =>
|
||||
expect(archiveAndSwitchMock).toHaveBeenCalledWith("task-1", { cascade: true }),
|
||||
);
|
||||
});
|
||||
|
||||
it("disables the confirm button while an archive is in flight", async () => {
|
||||
let resolveArchive: () => void = () => {};
|
||||
archiveAndSwitchMock.mockImplementation(
|
||||
() => new Promise<void>((resolve) => (resolveArchive = resolve)),
|
||||
);
|
||||
render(<PRMergedBanner taskId="task-1" />);
|
||||
|
||||
fireEvent.click(screen.getByTestId(MERGED_ARCHIVE_BUTTON));
|
||||
fireEvent.click(await screen.findByTestId(MERGED_ARCHIVE_CONFIRM));
|
||||
await waitFor(() => expect(archiveAndSwitchMock).toHaveBeenCalledTimes(1));
|
||||
|
||||
// Reopen while the async archive is still pending: confirm must be disabled.
|
||||
fireEvent.click(screen.getByTestId(MERGED_ARCHIVE_BUTTON));
|
||||
const confirm = await screen.findByTestId<HTMLButtonElement>(MERGED_ARCHIVE_CONFIRM);
|
||||
expect(confirm.disabled).toBe(true);
|
||||
fireEvent.click(confirm);
|
||||
expect(archiveAndSwitchMock).toHaveBeenCalledTimes(1);
|
||||
|
||||
resolveArchive();
|
||||
await waitFor(() =>
|
||||
expect(screen.getByTestId<HTMLButtonElement>(MERGED_ARCHIVE_CONFIRM).disabled).toBe(false),
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to a generic title when the task is not in the store", async () => {
|
||||
taskPRsByTaskId.value = {
|
||||
"task-2": [{ pr_number: 7, state: "merged" }],
|
||||
};
|
||||
render(<PRMergedBanner taskId="task-2" />);
|
||||
|
||||
fireEvent.click(screen.getByTestId(MERGED_ARCHIVE_BUTTON));
|
||||
|
||||
expect(await screen.findByTestId(MERGED_ARCHIVE_CONFIRM)).toBeTruthy();
|
||||
expect(screen.getByText(/Are you sure you want to archive "this task"\?/)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
describe("PRClosedBanner", () => {
|
||||
it("archives through the confirmation dialog", async () => {
|
||||
taskPRsByTaskId.value = {
|
||||
"task-1": [{ pr_number: 42, state: "closed" }],
|
||||
};
|
||||
render(<PRClosedBanner taskId="task-1" />);
|
||||
|
||||
fireEvent.click(screen.getByTestId("pr-closed-archive-button"));
|
||||
expect(archiveAndSwitchMock).not.toHaveBeenCalled();
|
||||
fireEvent.click(await screen.findByTestId("pr-closed-archive-confirm"));
|
||||
|
||||
await waitFor(() =>
|
||||
expect(archiveAndSwitchMock).toHaveBeenCalledWith("task-1", { cascade: false }),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,182 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback, useState, type ReactNode } from "react";
|
||||
import { IconGitMerge, IconGitPullRequestClosed, IconX } from "@tabler/icons-react";
|
||||
import { TaskArchiveConfirmDialog } from "@/components/task/task-archive-confirm-dialog";
|
||||
import { useAppStore, useAppStoreApi } from "@/components/state-provider";
|
||||
import { useArchiveAndSwitchTask } from "@/hooks/use-task-actions";
|
||||
import { useToast } from "@/components/toast-provider";
|
||||
import { findTaskInSnapshots } from "@/lib/kanban/find-task";
|
||||
import {
|
||||
markPRClosedBannerDismissed,
|
||||
markPRMergedBannerDismissed,
|
||||
wasPRClosedBannerDismissed,
|
||||
wasPRMergedBannerDismissed,
|
||||
} from "@/lib/local-storage";
|
||||
|
||||
type ArchiveTarget = { title: string; executorType?: string | null };
|
||||
|
||||
// Archiving from the terminal-state banners goes through the same confirmation
|
||||
// dialog as every other archive surface. Only failures toast; on success the
|
||||
// archive-and-switch flow moves the user to the next task.
|
||||
function useBannerArchiveConfirm(taskId: string) {
|
||||
const store = useAppStoreApi();
|
||||
const archiveAndSwitch = useArchiveAndSwitchTask();
|
||||
const { toast } = useToast();
|
||||
const [target, setTarget] = useState<ArchiveTarget | null>(null);
|
||||
const [isPending, setIsPending] = useState(false);
|
||||
|
||||
const requestArchive = useCallback(() => {
|
||||
const state = store.getState();
|
||||
const task = findTaskInSnapshots(taskId, state.kanbanMulti.snapshots, state.kanban.tasks);
|
||||
setTarget({ title: task?.title ?? "this task", executorType: task?.primaryExecutorType });
|
||||
}, [store, taskId]);
|
||||
|
||||
const closeConfirm = useCallback(() => setTarget(null), []);
|
||||
|
||||
const confirmArchive = useCallback(
|
||||
async ({ cascade }: { cascade: boolean }) => {
|
||||
setIsPending(true);
|
||||
try {
|
||||
await archiveAndSwitch(taskId, { cascade });
|
||||
} catch {
|
||||
toast({ description: "Failed to archive task", variant: "error" });
|
||||
} finally {
|
||||
setIsPending(false);
|
||||
}
|
||||
},
|
||||
[archiveAndSwitch, taskId, toast],
|
||||
);
|
||||
|
||||
return { target, requestArchive, closeConfirm, confirmArchive, isPending };
|
||||
}
|
||||
|
||||
// Presentational banner shared by PRMergedBanner / PRClosedBanner — an icon, a
|
||||
// message, and Archive + Dismiss controls. Colors/icon/testIds are supplied by
|
||||
// the caller so the two variants stay visually distinct. The Archive control
|
||||
// opens the shared archive confirmation dialog rather than archiving directly.
|
||||
function ArchiveDismissBanner({
|
||||
testIdPrefix,
|
||||
icon,
|
||||
text,
|
||||
containerClass,
|
||||
archiveClass,
|
||||
dismissClass,
|
||||
taskId,
|
||||
onDismiss,
|
||||
}: {
|
||||
testIdPrefix: string;
|
||||
icon: ReactNode;
|
||||
text: string;
|
||||
containerClass: string;
|
||||
archiveClass: string;
|
||||
dismissClass: string;
|
||||
taskId: string;
|
||||
onDismiss: () => void;
|
||||
}) {
|
||||
const { target, requestArchive, closeConfirm, confirmArchive, isPending } =
|
||||
useBannerArchiveConfirm(taskId);
|
||||
return (
|
||||
<>
|
||||
<div data-testid={`${testIdPrefix}-banner`} className={containerClass}>
|
||||
{icon}
|
||||
<span className="flex-1">{text}</span>
|
||||
<button
|
||||
type="button"
|
||||
data-testid={`${testIdPrefix}-archive-button`}
|
||||
onClick={requestArchive}
|
||||
className={archiveClass}
|
||||
>
|
||||
Archive
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
aria-label="Dismiss"
|
||||
data-testid={`${testIdPrefix}-dismiss-button`}
|
||||
onClick={onDismiss}
|
||||
className={dismissClass}
|
||||
>
|
||||
<IconX className="h-3 w-3" />
|
||||
</button>
|
||||
</div>
|
||||
<TaskArchiveConfirmDialog
|
||||
open={target !== null}
|
||||
onOpenChange={(open) => {
|
||||
if (!open) closeConfirm();
|
||||
}}
|
||||
taskTitle={target?.title ?? ""}
|
||||
taskId={taskId}
|
||||
executorType={target?.executorType}
|
||||
isArchiving={isPending}
|
||||
onConfirm={confirmArchive}
|
||||
confirmTestId={`${testIdPrefix}-archive-confirm`}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
export function PRMergedBanner({ taskId }: { taskId: string }) {
|
||||
const taskPRs = useAppStore((state) => state.taskPRs.byTaskId[taskId]);
|
||||
const [dismissed, setDismissed] = useState(() => wasPRMergedBannerDismissed(taskId));
|
||||
|
||||
const handleDismiss = useCallback(() => {
|
||||
markPRMergedBannerDismissed(taskId);
|
||||
setDismissed(true);
|
||||
}, [taskId]);
|
||||
|
||||
// Multi-repo: only show "ready to archive" once every PR is merged. A
|
||||
// single merged repo with others still open means the task isn't done yet.
|
||||
const allMerged = !!taskPRs && taskPRs.length > 0 && taskPRs.every((pr) => pr.state === "merged");
|
||||
if (!allMerged || dismissed) return null;
|
||||
|
||||
const bannerText =
|
||||
taskPRs.length === 1
|
||||
? `PR #${taskPRs[0].pr_number} has been merged. You can archive this task.`
|
||||
: `All ${taskPRs.length} PRs have been merged. You can archive this task.`;
|
||||
|
||||
return (
|
||||
<ArchiveDismissBanner
|
||||
testIdPrefix="pr-merged"
|
||||
icon={<IconGitMerge className="h-3.5 w-3.5 shrink-0" />}
|
||||
text={bannerText}
|
||||
containerClass="flex flex-1 items-center gap-2 rounded-md bg-purple-500/10 px-2 py-1 text-purple-600 dark:text-purple-400"
|
||||
archiveClass="underline underline-offset-2 hover:text-purple-700 dark:hover:text-purple-300 cursor-pointer"
|
||||
dismissClass="p-0.5 hover:bg-purple-500/10 rounded cursor-pointer"
|
||||
taskId={taskId}
|
||||
onDismiss={handleDismiss}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
export function PRClosedBanner({ taskId }: { taskId: string }) {
|
||||
const taskPRs = useAppStore((state) => state.taskPRs.byTaskId[taskId]);
|
||||
const [dismissed, setDismissed] = useState(() => wasPRClosedBannerDismissed(taskId));
|
||||
|
||||
const handleDismiss = useCallback(() => {
|
||||
markPRClosedBannerDismissed(taskId);
|
||||
setDismissed(true);
|
||||
}, [taskId]);
|
||||
|
||||
// Mirror the merged banner's all-or-nothing rule: show only once every PR is
|
||||
// closed-without-merging. A mix of merged + closed shows neither banner.
|
||||
const allClosed = !!taskPRs && taskPRs.length > 0 && taskPRs.every((pr) => pr.state === "closed");
|
||||
if (!allClosed || dismissed) return null;
|
||||
|
||||
const bannerText =
|
||||
taskPRs.length === 1
|
||||
? `PR #${taskPRs[0].pr_number} was closed without merging. You can archive this task.`
|
||||
: `All ${taskPRs.length} PRs were closed without merging. You can archive this task.`;
|
||||
|
||||
return (
|
||||
<ArchiveDismissBanner
|
||||
testIdPrefix="pr-closed"
|
||||
icon={<IconGitPullRequestClosed className="h-3.5 w-3.5 shrink-0" />}
|
||||
text={bannerText}
|
||||
containerClass="flex flex-1 items-center gap-2 rounded-md bg-red-500/10 px-2 py-1 text-red-600 dark:text-red-400"
|
||||
archiveClass="underline underline-offset-2 hover:text-red-700 dark:hover:text-red-300 cursor-pointer"
|
||||
dismissClass="p-0.5 hover:bg-red-500/10 rounded cursor-pointer"
|
||||
taskId={taskId}
|
||||
onDismiss={handleDismiss}
|
||||
/>
|
||||
);
|
||||
}
|
||||
@@ -111,7 +111,7 @@ vi.mock("@/components/github/pr-status-chip", () => ({
|
||||
PRStatusChip: () => null,
|
||||
}));
|
||||
|
||||
vi.mock("./chat/chat-input-area", () => ({
|
||||
vi.mock("./chat/pr-archive-banners", () => ({
|
||||
PRMergedBanner: () => null,
|
||||
}));
|
||||
|
||||
|
||||
@@ -21,7 +21,7 @@ import { Button } from "@kandev/ui/button";
|
||||
import { Textarea } from "@kandev/ui/textarea";
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
|
||||
import { PRStatusChip } from "@/components/github/pr-status-chip";
|
||||
import { PRMergedBanner } from "./chat/chat-input-area";
|
||||
import { PRMergedBanner } from "./chat/pr-archive-banners";
|
||||
import { type ChatInputContainerHandle } from "./chat/chat-input-container";
|
||||
import { useChatPanelState } from "./chat/use-chat-panel-state";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
|
||||
@@ -58,6 +58,9 @@ export class SessionPage {
|
||||
prMergedArchiveButton() {
|
||||
return this.page.getByTestId("pr-merged-archive-button");
|
||||
}
|
||||
prMergedArchiveConfirmButton() {
|
||||
return this.page.getByTestId("pr-merged-archive-confirm");
|
||||
}
|
||||
prMergedDismissButton() {
|
||||
return this.page.getByTestId("pr-merged-dismiss-button");
|
||||
}
|
||||
@@ -67,6 +70,9 @@ export class SessionPage {
|
||||
prClosedArchiveButton() {
|
||||
return this.page.getByTestId("pr-closed-archive-button");
|
||||
}
|
||||
prClosedArchiveConfirmButton() {
|
||||
return this.page.getByTestId("pr-closed-archive-confirm");
|
||||
}
|
||||
prClosedDismissButton() {
|
||||
return this.page.getByTestId("pr-closed-dismiss-button");
|
||||
}
|
||||
|
||||
@@ -454,8 +454,18 @@ test.describe("Chat status bar", () => {
|
||||
|
||||
const urlBeforeArchive = testPage.url();
|
||||
|
||||
// Click archive in the PR merged banner
|
||||
// Cancelling the confirmation dialog must not archive anything
|
||||
await session.prMergedArchiveButton().click();
|
||||
await expect(session.prMergedArchiveConfirmButton()).toBeVisible({ timeout: 10_000 });
|
||||
await testPage.getByRole("alertdialog").getByRole("button", { name: "Cancel" }).click();
|
||||
await expect(session.prMergedArchiveConfirmButton()).not.toBeVisible();
|
||||
await expect(session.prMergedBanner()).toBeVisible();
|
||||
expect(testPage.url()).toBe(urlBeforeArchive);
|
||||
|
||||
// Click archive in the PR merged banner, then confirm in the dialog
|
||||
await session.prMergedArchiveButton().click();
|
||||
await expect(session.prMergedArchiveConfirmButton()).toBeVisible({ timeout: 10_000 });
|
||||
await session.prMergedArchiveConfirmButton().click();
|
||||
|
||||
// Should switch to task B
|
||||
await expect(session.taskInSidebar("Archive Banner Task A")).not.toBeVisible({
|
||||
|
||||
Reference in New Issue
Block a user