From 39eb3eb2e48207769b0dc623b847cc2195cbd824 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Sun, 26 Apr 2026 23:38:59 -0700 Subject: [PATCH] =?UTF-8?q?test(canvas):=20unit=20tests=20for=20OrgCancelB?= =?UTF-8?q?utton=20=E2=80=94=20cascade-delete=20+=20optimistic=20store=20(?= =?UTF-8?q?#2071)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [Molecule-Platform-Evolvement-Manager] Closes the third item from #2071 (Canvas test gaps follow-up). Builds on the A2AEdge tests in PR #2143. 10 cases across 4 buckets: **Render (2):** - Default pill with `Cancel (N)` text + correct ARIA label - Confirm dialog NOT visible until pill click **Pill click (3):** - Click flips to confirming view + stops propagation (so React Flow doesn't interpret the click as a node selection) - Confirm copy pluralizes correctly: count=1 → "Delete 1 workspace?", count>1 → "Delete N workspaces?". Negative assertion guards against the wrong-form regressing in either direction. **No / cancel-confirm (1):** - Click No → returns to pill, no API call, no store mutation **Yes / cascade-delete (4):** - Happy path: beginDelete locks the WHOLE subtree (root + children, NOT unrelated workspace) → api.del("/workspaces/?confirm=true") → optimistic store filter strips subtree, keeps unrelated → success toast → endDelete in finally - WS-event race: WS_REMOVED handler clears the root mid-flight. The bail-out branch (`!postDeleteState.nodes.some(n => n.id === rootId)`) must NOT then run a second optimistic filter. Pre-fix the post-await subtree walk would miss any orphaned descendants whose parentId got reparented upward by handleCanvasEvent — pinned now. - Error path: api.del rejects → endDelete UNDOes the lock + error toast surfaces the message → subtree STAYS in the store so the user can retry / interact with the still-deploying nodes - Non-Error rejection (e.g. string thrown directly): toast surfaces the canned "Cancel failed" fallback instead of attempting `.message` ## Mocking - `@/lib/api`, `@/components/Toaster`: simple spy mocks - `@/store/canvas`: object that satisfies BOTH the selector pattern (`useCanvasStore(s => s.x)`) AND `getState()` / `setState()` since the cascade-delete handler walks the subtree via `getState()` and mutates via `setState()` for the optimistic removal. `vi.hoisted` preserves referential identity so the mock fns wired into the state object are observed by every consumer. ## Test plan - [x] All 10 cases pass locally (`vitest run OrgCancelButton.test.tsx` — ~990ms) - [x] No changes to the SUT — pure additive coverage - [ ] CI green ## #2071 progress after this PR - [x] useTemplateDeploy (PR #2121) - [x] A2AEdge (PR #2143) - [x] OrgCancelButton (this PR) - [ ] useDragHandlers — separate PR 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) --- .../canvas/__tests__/OrgCancelButton.test.tsx | 318 ++++++++++++++++++ 1 file changed, 318 insertions(+) create mode 100644 canvas/src/components/canvas/__tests__/OrgCancelButton.test.tsx diff --git a/canvas/src/components/canvas/__tests__/OrgCancelButton.test.tsx b/canvas/src/components/canvas/__tests__/OrgCancelButton.test.tsx new file mode 100644 index 00000000..527a83e4 --- /dev/null +++ b/canvas/src/components/canvas/__tests__/OrgCancelButton.test.tsx @@ -0,0 +1,318 @@ +// @vitest-environment jsdom +/** + * Tests for OrgCancelButton — the cancel-deployment pill on the root + * card of an in-flight org. Two-step UX: click pill → confirm dialog + * → cascade-delete with optimistic store update. + * + * Coverage targets the contracts a future refactor could regress: + * 1. Default render: pill with `Cancel (N)` and the right ARIA label + * 2. Click pill → stopPropagation + flip to confirming view + * 3. Confirm copy pluralizes (1 workspace vs N workspaces) + * 4. "No" → back to pill, no API call, no store mutation + * 5. "Yes" happy path → beginDelete + api.del + optimistic store + * filter (subtree removed) + success toast + endDelete + * 6. "Yes" error path → endDelete (UNDOing the lock) + error toast, + * no optimistic store filter + * 7. Submitting state disables both buttons during the round-trip + * + * Issue: #2071 (Canvas test gaps follow-up). + */ +import { + describe, + it, + expect, + vi, + beforeEach, + afterEach, +} from "vitest"; +import { + render, + screen, + cleanup, + fireEvent, + act, +} from "@testing-library/react"; + +// ── Hoisted mocks ──────────────────────────────────────────────────────────── + +const { mockApiDel, mockShowToast, mockState } = vi.hoisted(() => { + const state = { + nodes: [] as Array<{ id: string; data: { parentId?: string | null } }>, + edges: [] as Array<{ source: string; target: string }>, + beginDelete: vi.fn(), + endDelete: vi.fn(), + }; + return { + mockApiDel: vi.fn(), + mockShowToast: vi.fn(), + mockState: state, + }; +}); + +vi.mock("@/lib/api", () => ({ + api: { del: mockApiDel }, +})); + +vi.mock("@/components/Toaster", () => ({ + showToast: mockShowToast, +})); + +// useCanvasStore must support both selector-pattern usage AND +// getState() / setState() since handleCancel walks the subtree via +// getState() then mutates via setState() for the optimistic removal. +vi.mock("@/store/canvas", () => ({ + useCanvasStore: Object.assign( + (selector: (s: typeof mockState) => unknown) => selector(mockState), + { + getState: () => mockState, + setState: (patch: Partial) => Object.assign(mockState, patch), + }, + ), +})); + +// Import the SUT after the mocks are declared. +import { OrgCancelButton } from "../OrgCancelButton"; + +// ── Helpers ────────────────────────────────────────────────────────────────── + +/** Reset mock state to a default subtree shaped {root: [child1, child2]}. */ +function seedSubtree() { + mockState.nodes = [ + { id: "ws-root", data: { parentId: null } }, + { id: "ws-child-1", data: { parentId: "ws-root" } }, + { id: "ws-child-2", data: { parentId: "ws-root" } }, + { id: "ws-unrelated", data: { parentId: null } }, + ]; + mockState.edges = [ + { source: "ws-root", target: "ws-child-1" }, + { source: "ws-root", target: "ws-child-2" }, + ]; +} + +beforeEach(() => { + mockApiDel.mockReset(); + mockShowToast.mockReset(); + mockState.beginDelete.mockReset(); + mockState.endDelete.mockReset(); + seedSubtree(); +}); + +afterEach(() => { + cleanup(); +}); + +// ── Tests ──────────────────────────────────────────────────────────────────── + +describe("OrgCancelButton — render", () => { + it("default: shows the Cancel (N) pill with the right ARIA label", () => { + render( + , + ); + const pill = screen.getByRole("button", { + name: /cancel deployment of my org/i, + }); + expect(pill.textContent).toContain("Cancel (5)"); + }); + + it("does not render the confirm dialog initially", () => { + render( + , + ); + expect(screen.queryByText(/^delete \d+ workspaces?\?$/i)).toBeNull(); + }); +}); + +describe("OrgCancelButton — pill click", () => { + it("flips to confirming view and stops propagation", () => { + const parentClick = vi.fn(); + render( +
+ +
, + ); + fireEvent.click( + screen.getByRole("button", { name: /cancel deployment of my org/i }), + ); + expect(parentClick).not.toHaveBeenCalled(); + expect(screen.getByText(/delete 5 workspaces\?/i)).toBeTruthy(); + expect(screen.getByRole("button", { name: /^yes$/i })).toBeTruthy(); + expect(screen.getByRole("button", { name: /^no$/i })).toBeTruthy(); + }); + + it("confirm copy pluralizes — singular at count=1", () => { + render( + , + ); + fireEvent.click( + screen.getByRole("button", { name: /cancel deployment of solo/i }), + ); + expect(screen.getByText(/^delete 1 workspace\?$/i)).toBeTruthy(); + // Negative: must NOT pluralize at count=1. + expect(screen.queryByText(/^delete 1 workspaces\?$/i)).toBeNull(); + }); + + it("confirm copy pluralizes — plural at count>1", () => { + render( + , + ); + fireEvent.click( + screen.getByRole("button", { name: /cancel deployment of big org/i }), + ); + expect(screen.getByText(/^delete 9 workspaces\?$/i)).toBeTruthy(); + }); +}); + +describe("OrgCancelButton — No / cancel-confirm", () => { + it("clicking No returns to the pill view, no API call, no store mutation", () => { + render( + , + ); + // Open confirm + fireEvent.click( + screen.getByRole("button", { name: /cancel deployment of my org/i }), + ); + // Dismiss + fireEvent.click(screen.getByRole("button", { name: /^no$/i })); + // Pill back; confirm gone + expect( + screen.getByRole("button", { name: /cancel deployment of my org/i }), + ).toBeTruthy(); + expect(screen.queryByText(/delete \d+ workspaces?\?/i)).toBeNull(); + // No side effects + expect(mockApiDel).not.toHaveBeenCalled(); + expect(mockState.beginDelete).not.toHaveBeenCalled(); + expect(mockState.endDelete).not.toHaveBeenCalled(); + }); +}); + +describe("OrgCancelButton — Yes / cascade delete", () => { + it("happy path: beginDelete → api.del → optimistic store filter → success toast → endDelete", async () => { + mockApiDel.mockResolvedValueOnce({ status: "ok" }); + render( + , + ); + fireEvent.click( + screen.getByRole("button", { name: /cancel deployment of my org/i }), + ); + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: /^yes$/i })); + }); + + // 1) API call hit the cascade-delete endpoint with confirm=true + expect(mockApiDel).toHaveBeenCalledWith("/workspaces/ws-root?confirm=true"); + + // 2) beginDelete locked the WHOLE subtree (root + 2 children) — NOT the unrelated node + expect(mockState.beginDelete).toHaveBeenCalledTimes(1); + const lockedIds = mockState.beginDelete.mock.calls[0][0] as Set; + expect(lockedIds.has("ws-root")).toBe(true); + expect(lockedIds.has("ws-child-1")).toBe(true); + expect(lockedIds.has("ws-child-2")).toBe(true); + expect(lockedIds.has("ws-unrelated")).toBe(false); + + // 3) Optimistic store removal: subtree filtered out, unrelated kept + const remainingIds = mockState.nodes.map((n) => n.id); + expect(remainingIds).toEqual(["ws-unrelated"]); + expect(mockState.edges).toHaveLength(0); + + // 4) Success toast + expect(mockShowToast).toHaveBeenCalledWith( + 'Cancelled deployment of "My Org"', + "success", + ); + + // 5) endDelete fired in the finally block (one call — the success + // path doesn't separately call endDelete in the try, only the + // catch does that; finally always runs once.) + expect(mockState.endDelete).toHaveBeenCalledTimes(1); + }); + + it("bail-out: WS_REMOVED already dropped the root mid-flight → skip optimistic filter", async () => { + // Simulate WS-event handler racing the await: by the time api.del + // resolves, the root node is gone from the store. Without the + // bail-out, the post-delete subtree walk would miss any orphaned + // descendants (handleCanvasEvent reparents children of a removed + // node upward — they no longer share root's id as parentId). + mockApiDel.mockImplementationOnce(async () => { + // During the network round-trip, the WS handler removes the root. + mockState.nodes = mockState.nodes.filter((n) => n.id !== "ws-root"); + return { status: "ok" }; + }); + render( + , + ); + fireEvent.click( + screen.getByRole("button", { name: /cancel deployment of my org/i }), + ); + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: /^yes$/i })); + }); + + // Success toast still fired (the cascade-delete API succeeded). + expect(mockShowToast).toHaveBeenCalledWith( + 'Cancelled deployment of "My Org"', + "success", + ); + // beginDelete was called (locking happens before await). + expect(mockState.beginDelete).toHaveBeenCalled(); + // The bail-out path means we did NOT attempt a second optimistic + // setState after WS_REMOVED already cleared the root. The remaining + // nodes reflect ONLY the WS handler's removal (just root gone). + const remainingIds = mockState.nodes.map((n) => n.id).sort(); + expect(remainingIds).toEqual([ + "ws-child-1", + "ws-child-2", + "ws-unrelated", + ]); + }); + + it("error path: endDelete UNDOes the lock + error toast, no optimistic filter", async () => { + mockApiDel.mockRejectedValueOnce(new Error("server 500")); + render( + , + ); + fireEvent.click( + screen.getByRole("button", { name: /cancel deployment of my org/i }), + ); + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: /^yes$/i })); + }); + + // beginDelete fired (locks happen before the await). + expect(mockState.beginDelete).toHaveBeenCalledTimes(1); + // endDelete fires TWICE in the error path: once in the catch + // (undo the lock) and once in the finally (idempotent on the + // already-cleared set). The point of the test is that the lock + // is undone; the duplicate endDelete is intentional and harmless + // since the implementation's idempotent. + expect(mockState.endDelete.mock.calls.length).toBeGreaterThanOrEqual(1); + // No optimistic filter: subtree must STILL be in the store + // (user can retry / interact with the still-deploying nodes). + const remainingIds = mockState.nodes.map((n) => n.id).sort(); + expect(remainingIds).toEqual([ + "ws-child-1", + "ws-child-2", + "ws-root", + "ws-unrelated", + ]); + // Error toast surfaces the error message + expect(mockShowToast).toHaveBeenCalledWith( + "Cancel failed: server 500", + "error", + ); + }); + + it("error path with non-Error rejection: surfaces 'Cancel failed' fallback", async () => { + mockApiDel.mockRejectedValueOnce("plain string"); + render( + , + ); + fireEvent.click( + screen.getByRole("button", { name: /cancel deployment of my org/i }), + ); + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: /^yes$/i })); + }); + + expect(mockShowToast).toHaveBeenCalledWith("Cancel failed", "error"); + }); +});