From e57db86a5cc2f949810fbff923c317b762575240 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Mon, 8 Jun 2026 11:55:35 -0700 Subject: [PATCH] FN-6029: restore board drop-check regression coverage Restore Board drag-drop pre-check coverage by extracting the decision logic into a reusable helper. - extract Board canDropTask rejection logic into a shared helper used by the component - replace Lane-mocking coverage with direct helper tests for workflow, column, and capacity branches - add regression cases for missing workflows, default-workflow fallback, same-column WIP drops, and cross-workflow occupancy handling Files changed: packages/dashboard/app/components/Board.tsx | 37 +-- .../__tests__/Board.canDropTask.test.tsx | 261 ++++++++++++--------- .../dashboard/app/components/boardCanDropTask.ts | 56 +++++ 3 files changed, 218 insertions(+), 136 deletions(-) Fusion-Task-Id: FN-6029 Fusion-Task-Lineage: cd71bbc5-0f97-49c3-9eef-0dd8221341c0 --- packages/dashboard/app/components/Board.tsx | 37 +-- .../__tests__/Board.canDropTask.test.tsx | 263 ++++++++++-------- .../app/components/boardCanDropTask.ts | 56 ++++ 3 files changed, 219 insertions(+), 137 deletions(-) create mode 100644 packages/dashboard/app/components/boardCanDropTask.ts diff --git a/packages/dashboard/app/components/Board.tsx b/packages/dashboard/app/components/Board.tsx index 9a310bb4b0..4b30ed7e4e 100644 --- a/packages/dashboard/app/components/Board.tsx +++ b/packages/dashboard/app/components/Board.tsx @@ -11,6 +11,7 @@ import { useBlockerFanout } from "../hooks/useBlockerFanout"; import { MOBILE_MEDIA_QUERY } from "../hooks/useViewportMode"; import { recordResumeEvent } from "../utils/resumeInstrumentation"; import { subscribeSse } from "../sse-bus"; +import { getBoardCanDropTaskRejection } from "./boardCanDropTask"; interface BoardProps { tasks: Task[]; @@ -418,32 +419,16 @@ export function Board({ tasks, projectId, maxConcurrent, onMoveTask, onPauseTask // Drag pre-check (R17): adjacency + capacity from the lane's column metadata. // Cross-lane drag → workflow-mismatch. Deterministic rejections return a // messageKey (no-move); null = allowed. - const canDropTask = useCallback((taskId: string, targetColumnId: string, laneWorkflowId: string): string | null => { - if (!boardWorkflows) return null; - const sourceTask = tasks.find((t) => t.id === taskId); - if (!sourceTask) return null; - const sourceWorkflowId = boardWorkflows.taskWorkflowIds[taskId] ?? boardWorkflows.defaultWorkflowId; - // Cross-lane drag never switches workflows (R17). - if (sourceWorkflowId !== laneWorkflowId) { - return "board.rejection.workflowMismatch"; - } - const workflow = boardWorkflows.workflows.find((w) => w.id === laneWorkflowId); - if (!workflow) return null; - const targetCol = workflow.columns.find((c) => c.id === targetColumnId); - if (!targetCol) return "board.rejection.unknownColumn"; - // Capacity pre-check: a wip-flagged column that is already full rejects. - if (targetCol.flags.countsTowardWip) { - const occupants = tasks.filter( - (t) => t.column === targetColumnId && (boardWorkflows.taskWorkflowIds[t.id] ?? boardWorkflows.defaultWorkflowId) === laneWorkflowId, - ).length; - // The default workflow's in-progress limit is maxConcurrent; custom limits - // are enforced authoritatively server-side (the 409 fallback still snaps back). - if (Number.isFinite(maxConcurrent) && maxConcurrent > 0 && sourceTask.column !== targetColumnId && occupants >= maxConcurrent) { - return "board.rejection.capacityExhausted"; - } - } - return null; - }, [boardWorkflows, tasks, maxConcurrent]); + const canDropTask = useCallback((taskId: string, targetColumnId: string, laneWorkflowId: string): string | null => ( + getBoardCanDropTaskRejection({ + boardWorkflows, + tasks, + maxConcurrent, + taskId, + targetColumnId, + laneWorkflowId, + }) + ), [boardWorkflows, tasks, maxConcurrent]); // FN-4380: GitHub badge state comes from persisted task fields (`task.prInfo`, // `task.issueInfo`, `task.githubTracking.issue`) and live WebSocket `badge:updated` diff --git a/packages/dashboard/app/components/__tests__/Board.canDropTask.test.tsx b/packages/dashboard/app/components/__tests__/Board.canDropTask.test.tsx index 4a106b8ebf..bbe19f8f03 100644 --- a/packages/dashboard/app/components/__tests__/Board.canDropTask.test.tsx +++ b/packages/dashboard/app/components/__tests__/Board.canDropTask.test.tsx @@ -1,58 +1,18 @@ -// FN-1416: Board-level coverage of the canDropTask drag pre-check (R17). +// FN-1416/FN-6029: Board-level coverage of the canDropTask drag pre-check (R17). // -// canDropTask is an internal Board closure passed down to . Board.tsx is -// being edited by another agent, so rather than touch it (or its existing -// test), this file mocks to CAPTURE the real canDropTask closure Board -// constructs, then drives the three rejection branches plus the allowed case: -// - cross-workflow drag → "board.rejection.workflowMismatch" -// - unknown target column in the lane → "board.rejection.unknownColumn" -// - full wip column (>= maxConcurrent) → "board.rejection.capacityExhausted" -// - valid same-lane, under-capacity drop → null (allowed) -// -// This exercises the production closure (not a copy), so a regression in any -// branch fails here. +// Board now passes one-argument per-column wrappers directly to in the +// selected-workflow rendering path, while still adapts the canonical +// three-argument decision for multi-lane rendering. These tests exercise the +// pure Board decision seam directly so unrendered-column branches (especially +// unknownColumn) remain covered without stale Lane mocking. -import React from "react"; -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { render, act } from "@testing-library/react"; +import { describe, it, expect } from "vitest"; import type { Task } from "@fusion/core"; -import { Board } from "../Board"; +import type { BoardWorkflowsPayload } from "../../api"; +import { getBoardCanDropTaskRejection } from "../boardCanDropTask"; -vi.mock("../../hooks/useBatchBadgeFetch", () => ({ - useBatchBadgeFetch: vi.fn(() => ({ - fetchBatch: vi.fn(), - isLoading: false, - lastFetchTime: null, - getBatchData: vi.fn(), - })), -})); - -const fetchBoardWorkflowsMock = vi.fn(); -vi.mock("../../api", () => ({ - fetchWorkflowSteps: vi.fn().mockResolvedValue([]), - fetchBoardWorkflows: (...args: unknown[]) => fetchBoardWorkflowsMock(...args), - promoteTask: vi.fn().mockResolvedValue({}), -})); - -vi.mock("../../sse-bus", () => ({ - subscribeSse: vi.fn(() => () => {}), -})); - -// Don't pull in the full Column tree from the mocked Lane. -vi.mock("../Column", () => ({ Column: () =>
})); - -// Capture the canDropTask closure Board passes to each Lane. -type CanDrop = (taskId: string, targetColumnId: string, workflowId: string) => string | null; -let capturedCanDropTask: CanDrop | null = null; -vi.mock("../Lane", () => ({ - Lane: (props: { canDropTask: CanDrop }) => { - capturedCanDropTask = props.canDropTask; - return
; - }, -})); - -const DEFAULT_LANE = "builtin:coding"; -const CUSTOM_LANE = "WF-001"; +const DEFAULT_WORKFLOW = "builtin:coding"; +const CUSTOM_WORKFLOW = "WF-001"; // builtin:coding columns (in-progress counts toward wip; todo does not). const defaultColumns = [ @@ -86,86 +46,167 @@ function makeTask(id: string, column: string): Task { } as unknown as Task; } -function boardProps(overrides: Record = {}) { +function boardWorkflows(taskWorkflowIds: Record = {}): BoardWorkflowsPayload { return { - tasks: [] as Task[], - maxConcurrent: 2, - onMoveTask: () => Promise.resolve({} as never), - onOpenDetail: () => {}, - addToast: () => {}, - onQuickCreate: () => Promise.resolve({} as never), - onNewTask: () => {}, - autoMerge: true, - onToggleAutoMerge: () => {}, - globalPaused: false, - ...overrides, + flagEnabled: true, + defaultWorkflowId: DEFAULT_WORKFLOW, + workflows: [ + { id: DEFAULT_WORKFLOW, name: "Coding", columns: defaultColumns }, + { id: CUSTOM_WORKFLOW, name: "Custom", columns: customColumns }, + ], + taskWorkflowIds, }; } -/** Render Board flag-ON with the given tasks and wait for canDropTask capture. */ -async function renderAndCapture(tasks: Task[], taskWorkflowIds: Record) { - fetchBoardWorkflowsMock.mockResolvedValue({ - flagEnabled: true, - defaultWorkflowId: DEFAULT_LANE, - workflows: [ - { id: DEFAULT_LANE, name: "Coding", columns: defaultColumns }, - { id: CUSTOM_LANE, name: "Custom", columns: customColumns }, - ], - taskWorkflowIds, +function canDrop({ + workflows = boardWorkflows(), + tasks, + maxConcurrent = 2, + taskId = "FN-1", + targetColumnId, + laneWorkflowId = DEFAULT_WORKFLOW, +}: { + workflows?: BoardWorkflowsPayload | null | undefined; + tasks: Task[]; + maxConcurrent?: number; + taskId?: string; + targetColumnId: string; + laneWorkflowId?: string; +}) { + return getBoardCanDropTaskRejection({ + boardWorkflows: workflows, + tasks, + maxConcurrent, + taskId, + targetColumnId, + laneWorkflowId, }); - await act(async () => { - const props = boardProps({ tasks }) as unknown as React.ComponentProps; - render(); - await Promise.resolve(); - }); - expect(capturedCanDropTask).toBeTypeOf("function"); - return capturedCanDropTask!; } -describe("Board canDropTask pre-check (FN-1416)", () => { - beforeEach(() => { - capturedCanDropTask = null; - fetchBoardWorkflowsMock.mockReset(); - try { window.localStorage.clear(); } catch { /* jsdom */ } - }); - - it("cross-workflow drag → workflowMismatch", async () => { - // FN-1 lives in the default lane; dragging it into the custom lane crosses - // workflows (R17 never switches a card's workflow via drag). +describe("Board canDropTask pre-check (FN-1416/FN-6029)", () => { + it("cross-workflow drag returns workflowMismatch", () => { const tasks = [makeTask("FN-1", "todo")]; - const canDrop = await renderAndCapture(tasks, { "FN-1": DEFAULT_LANE }); - expect(canDrop("FN-1", "c-run", CUSTOM_LANE)).toBe("board.rejection.workflowMismatch"); + + expect(canDrop({ + workflows: boardWorkflows({ "FN-1": DEFAULT_WORKFLOW }), + tasks, + targetColumnId: "c-run", + laneWorkflowId: CUSTOM_WORKFLOW, + })).toBe("board.rejection.workflowMismatch"); }); - it("unknown target column in the lane → unknownColumn", async () => { + it("unknown target column in the source workflow returns unknownColumn", () => { const tasks = [makeTask("FN-1", "todo")]; - const canDrop = await renderAndCapture(tasks, { "FN-1": DEFAULT_LANE }); - expect(canDrop("FN-1", "does-not-exist", DEFAULT_LANE)).toBe("board.rejection.unknownColumn"); + + expect(canDrop({ + workflows: boardWorkflows({ "FN-1": DEFAULT_WORKFLOW }), + tasks, + targetColumnId: "does-not-exist", + })).toBe("board.rejection.unknownColumn"); }); - it("full wip column (occupants >= maxConcurrent) → capacityExhausted", async () => { - // maxConcurrent: 2; two cards already occupy in-progress in the default lane. - // Dragging a third (from todo) into in-progress must reject on capacity. + it("full wip column returns capacityExhausted", () => { + // maxConcurrent: 2; two cards already occupy in-progress in the default + // workflow. Dragging a third from todo into in-progress must reject. const tasks = [ makeTask("FN-1", "todo"), makeTask("FN-2", "in-progress"), makeTask("FN-3", "in-progress"), ]; - const canDrop = await renderAndCapture(tasks, { - "FN-1": DEFAULT_LANE, - "FN-2": DEFAULT_LANE, - "FN-3": DEFAULT_LANE, - }); - expect(canDrop("FN-1", "in-progress", DEFAULT_LANE)).toBe("board.rejection.capacityExhausted"); + + expect(canDrop({ + workflows: boardWorkflows({ + "FN-1": DEFAULT_WORKFLOW, + "FN-2": DEFAULT_WORKFLOW, + "FN-3": DEFAULT_WORKFLOW, + }), + tasks, + targetColumnId: "in-progress", + })).toBe("board.rejection.capacityExhausted"); }); - it("valid same-lane drop under capacity → allowed (null)", async () => { - // One free in-progress slot (maxConcurrent 2, one occupant); moving FN-1 from - // todo into in-progress in its own lane is permitted. + it("valid same-workflow drops under capacity return null", () => { const tasks = [makeTask("FN-1", "todo"), makeTask("FN-2", "in-progress")]; - const canDrop = await renderAndCapture(tasks, { "FN-1": DEFAULT_LANE, "FN-2": DEFAULT_LANE }); - expect(canDrop("FN-1", "in-progress", DEFAULT_LANE)).toBeNull(); - // Dropping into a non-wip column (todo → in-review) is also allowed. - expect(canDrop("FN-1", "in-review", DEFAULT_LANE)).toBeNull(); + const workflows = boardWorkflows({ "FN-1": DEFAULT_WORKFLOW, "FN-2": DEFAULT_WORKFLOW }); + + expect(canDrop({ workflows, tasks, targetColumnId: "in-progress" })).toBeNull(); + expect(canDrop({ workflows, tasks, targetColumnId: "in-review" })).toBeNull(); + }); + + it("returns null when boardWorkflows is undefined", () => { + expect(canDrop({ + workflows: undefined, + tasks: [makeTask("FN-1", "todo")], + targetColumnId: "in-progress", + })).toBeNull(); + }); + + it("returns null when the source task is missing", () => { + expect(canDrop({ + workflows: boardWorkflows({ "FN-1": DEFAULT_WORKFLOW }), + tasks: [makeTask("FN-2", "todo")], + taskId: "FN-1", + targetColumnId: "in-progress", + })).toBeNull(); + }); + + it("returns null when the source workflow is missing from boardWorkflows", () => { + expect(canDrop({ + workflows: { + flagEnabled: true, + defaultWorkflowId: DEFAULT_WORKFLOW, + workflows: [{ id: CUSTOM_WORKFLOW, name: "Custom", columns: customColumns }], + taskWorkflowIds: { "FN-1": DEFAULT_WORKFLOW }, + }, + tasks: [makeTask("FN-1", "todo")], + targetColumnId: "in-progress", + })).toBeNull(); + }); + + it("falls back to the default workflow when a task has no workflow id", () => { + expect(canDrop({ + workflows: boardWorkflows({}), + tasks: [makeTask("FN-1", "todo")], + targetColumnId: "does-not-exist", + laneWorkflowId: DEFAULT_WORKFLOW, + })).toBe("board.rejection.unknownColumn"); + }); + + it("allows same-column wip drops even when the column is at capacity", () => { + const tasks = [ + makeTask("FN-1", "in-progress"), + makeTask("FN-2", "in-progress"), + makeTask("FN-3", "in-progress"), + ]; + + expect(canDrop({ + workflows: boardWorkflows({ + "FN-1": DEFAULT_WORKFLOW, + "FN-2": DEFAULT_WORKFLOW, + "FN-3": DEFAULT_WORKFLOW, + }), + tasks, + targetColumnId: "in-progress", + })).toBeNull(); + }); + + it("does not count occupants from other workflows toward this workflow's capacity", () => { + const tasks = [ + makeTask("FN-1", "todo"), + makeTask("FN-2", "in-progress"), + makeTask("FN-3", "in-progress"), + makeTask("FN-4", "in-progress"), + ]; + + expect(canDrop({ + workflows: boardWorkflows({ + "FN-1": DEFAULT_WORKFLOW, + "FN-2": DEFAULT_WORKFLOW, + "FN-3": CUSTOM_WORKFLOW, + "FN-4": CUSTOM_WORKFLOW, + }), + tasks, + targetColumnId: "in-progress", + })).toBeNull(); }); }); diff --git a/packages/dashboard/app/components/boardCanDropTask.ts b/packages/dashboard/app/components/boardCanDropTask.ts new file mode 100644 index 0000000000..3d3b05c9a5 --- /dev/null +++ b/packages/dashboard/app/components/boardCanDropTask.ts @@ -0,0 +1,56 @@ +import type { Task } from "@fusion/core"; +import type { BoardWorkflowsPayload } from "../api"; + +export interface BoardCanDropTaskInput { + boardWorkflows: BoardWorkflowsPayload | null | undefined; + tasks: Task[]; + maxConcurrent: number; + taskId: string; + targetColumnId: string; + laneWorkflowId: string; +} + +/** + * Canonical Board drag pre-check (R17). Deterministic rejections return a stable + * i18n message key; `null` means the pre-check allows the drop or cannot decide. + */ +export function getBoardCanDropTaskRejection({ + boardWorkflows, + tasks, + maxConcurrent, + taskId, + targetColumnId, + laneWorkflowId, +}: BoardCanDropTaskInput): string | null { + if (!boardWorkflows) return null; + + const sourceTask = tasks.find((task) => task.id === taskId); + if (!sourceTask) return null; + + const sourceWorkflowId = boardWorkflows.taskWorkflowIds[taskId] ?? boardWorkflows.defaultWorkflowId; + // Cross-lane drag never switches workflows (R17). + if (sourceWorkflowId !== laneWorkflowId) { + return "board.rejection.workflowMismatch"; + } + + const workflow = boardWorkflows.workflows.find((candidate) => candidate.id === laneWorkflowId); + if (!workflow) return null; + + const targetColumn = workflow.columns.find((column) => column.id === targetColumnId); + if (!targetColumn) return "board.rejection.unknownColumn"; + + // Capacity pre-check: a wip-flagged column that is already full rejects. + if (targetColumn.flags.countsTowardWip) { + const occupants = tasks.filter( + (task) => task.column === targetColumnId + && (boardWorkflows.taskWorkflowIds[task.id] ?? boardWorkflows.defaultWorkflowId) === laneWorkflowId, + ).length; + // The default workflow's in-progress limit is maxConcurrent; custom limits + // are enforced authoritatively server-side (the 409 fallback still snaps back). + if (Number.isFinite(maxConcurrent) && maxConcurrent > 0 && sourceTask.column !== targetColumnId && occupants >= maxConcurrent) { + return "board.rejection.capacityExhausted"; + } + } + + return null; +}