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
This commit is contained in:
@@ -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`
|
||||
|
||||
@@ -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 <Lane>. Board.tsx is
|
||||
// being edited by another agent, so rather than touch it (or its existing
|
||||
// test), this file mocks <Lane> 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 <Column> in the
|
||||
// selected-workflow rendering path, while <Lane> 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: () => <div /> }));
|
||||
|
||||
// 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 <section data-testid="lane" />;
|
||||
},
|
||||
}));
|
||||
|
||||
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<string, unknown> = {}) {
|
||||
function boardWorkflows(taskWorkflowIds: Record<string, string> = {}): 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<string, string>) {
|
||||
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<typeof Board>;
|
||||
render(<Board {...props} />);
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
56
packages/dashboard/app/components/boardCanDropTask.ts
Normal file
56
packages/dashboard/app/components/boardCanDropTask.ts
Normal file
@@ -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;
|
||||
}
|
||||
Reference in New Issue
Block a user