feat(FN-5439): add overlap blocker clear action
- Add overlap blocker clear validation patch in task workflow routes\n- Type updateTask fields for overlap blocker clear support\n- Add overlap blocker clear UI action in TaskDetailModal\n- Wire legacy API for overlap clear mutation\n- Add dashboard and modal rendering tests for overlap clear flows
This commit is contained in:
committed by
gsxdsm
parent
9c2b61eec6
commit
6289d4a60b
@@ -427,6 +427,8 @@ export function updateTask(
|
||||
prompt?: string;
|
||||
dependencies?: string[];
|
||||
enabledWorkflowSteps?: string[];
|
||||
overlapBlockedBy?: string | null;
|
||||
status?: null;
|
||||
modelProvider?: string | null;
|
||||
modelId?: string | null;
|
||||
validatorModelProvider?: string | null;
|
||||
|
||||
@@ -1953,6 +1953,46 @@ export function TaskDetailContent({
|
||||
}
|
||||
}, [task.id, dependencies, addToast]);
|
||||
|
||||
const handleClearOverlapBlocker = useCallback(async () => {
|
||||
if (!workingTask.overlapBlockedBy) return;
|
||||
|
||||
const requestTaskId = task.id;
|
||||
const previousOverlapBlockedBy = workingTask.overlapBlockedBy;
|
||||
const previousStatus = workingTask.status;
|
||||
|
||||
setFullDetail((prev) => prev
|
||||
? {
|
||||
...prev,
|
||||
overlapBlockedBy: undefined,
|
||||
...(previousStatus === "queued" ? { status: undefined } : {}),
|
||||
}
|
||||
: prev);
|
||||
|
||||
try {
|
||||
const updatedTask = await updateTask(task.id, {
|
||||
overlapBlockedBy: null,
|
||||
status: previousStatus === "queued" ? null : undefined,
|
||||
}, projectId);
|
||||
if (activeTaskIdRef.current !== requestTaskId) {
|
||||
return;
|
||||
}
|
||||
setFullDetail((prev) => prev ? ({ ...prev, ...updatedTask } as TaskDetail) : (updatedTask as TaskDetail));
|
||||
onTaskUpdated?.(updatedTask);
|
||||
} catch (err) {
|
||||
if (activeTaskIdRef.current !== requestTaskId) {
|
||||
return;
|
||||
}
|
||||
setFullDetail((prev) => prev
|
||||
? {
|
||||
...prev,
|
||||
overlapBlockedBy: previousOverlapBlockedBy,
|
||||
...(previousStatus === "queued" ? { status: previousStatus } : {}),
|
||||
}
|
||||
: prev);
|
||||
addToast(getErrorMessage(err), "error");
|
||||
}
|
||||
}, [activeTaskIdRef, addToast, onTaskUpdated, projectId, task.id, workingTask.overlapBlockedBy, workingTask.status]);
|
||||
|
||||
const handleDepClick = useCallback(async (depId: string) => {
|
||||
try {
|
||||
const detail = await fetchTaskDetail(depId, projectId);
|
||||
@@ -3245,8 +3285,18 @@ export function TaskDetailContent({
|
||||
)}
|
||||
{workingTask.overlapBlockedBy && (
|
||||
<div className="detail-empty-inline">
|
||||
File scope overlap blocker: {workingTask.overlapBlockedBy}
|
||||
{!overlapBlockerActive && " (stale)"}
|
||||
<span>
|
||||
File scope overlap blocker: {workingTask.overlapBlockedBy}
|
||||
{!overlapBlockerActive && " (stale)"}
|
||||
</span>
|
||||
<button
|
||||
type="button"
|
||||
className="btn btn-sm"
|
||||
onClick={() => void handleClearOverlapBlocker()}
|
||||
title={`Clear overlap blocker ${workingTask.overlapBlockedBy}`}
|
||||
>
|
||||
Clear
|
||||
</button>
|
||||
</div>
|
||||
)}
|
||||
<div className="dep-trigger-wrap">
|
||||
|
||||
@@ -333,6 +333,116 @@ describe("TaskDetailModal", () => {
|
||||
expect(screen.queryByText("File scope overlap blocker: FN-OVER (stale)")).toBeNull();
|
||||
});
|
||||
|
||||
it("renders clear overlap blocker button only when overlapBlockedBy is present", () => {
|
||||
const { rerender } = render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({ id: "FN-T", column: "todo", overlapBlockedBy: "FN-OVER" })}
|
||||
onClose={noop}
|
||||
onMoveTask={noopMove}
|
||||
onDeleteTask={noopDelete}
|
||||
onMergeTask={noopMerge}
|
||||
onOpenDetail={noopOpenDetail}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByRole("button", { name: "Clear" })).toBeInTheDocument();
|
||||
|
||||
rerender(
|
||||
<TaskDetailModal
|
||||
task={makeTask({ id: "FN-T", column: "todo", overlapBlockedBy: undefined })}
|
||||
onClose={noop}
|
||||
onMoveTask={noopMove}
|
||||
onDeleteTask={noopDelete}
|
||||
onMergeTask={noopMerge}
|
||||
onOpenDetail={noopOpenDetail}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.queryByRole("button", { name: "Clear" })).toBeNull();
|
||||
});
|
||||
|
||||
it("clears overlap blocker and queued status when clicking Clear", async () => {
|
||||
vi.mocked(dashboardApi.updateTask).mockResolvedValueOnce(
|
||||
makeTask({ id: "FN-T", column: "todo", overlapBlockedBy: undefined, status: undefined }),
|
||||
);
|
||||
|
||||
render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({ id: "FN-T", column: "todo", overlapBlockedBy: "FN-OVER", status: "queued" })}
|
||||
onClose={noop}
|
||||
onMoveTask={noopMove}
|
||||
onDeleteTask={noopDelete}
|
||||
onMergeTask={noopMerge}
|
||||
onOpenDetail={noopOpenDetail}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
await userEvent.click(screen.getByRole("button", { name: "Clear" }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(dashboardApi.updateTask).toHaveBeenCalledWith(
|
||||
"FN-T",
|
||||
{ overlapBlockedBy: null, status: null },
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it("clears overlap blocker without status clear when task is not queued", async () => {
|
||||
vi.mocked(dashboardApi.updateTask).mockResolvedValueOnce(
|
||||
makeTask({ id: "FN-T", column: "todo", overlapBlockedBy: undefined }),
|
||||
);
|
||||
|
||||
render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({ id: "FN-T", column: "todo", overlapBlockedBy: "FN-OVER", status: "planning" })}
|
||||
onClose={noop}
|
||||
onMoveTask={noopMove}
|
||||
onDeleteTask={noopDelete}
|
||||
onMergeTask={noopMerge}
|
||||
onOpenDetail={noopOpenDetail}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
await userEvent.click(screen.getByRole("button", { name: "Clear" }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(dashboardApi.updateTask).toHaveBeenCalledWith(
|
||||
"FN-T",
|
||||
{ overlapBlockedBy: null, status: undefined },
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it("shows toast and restores overlap blocker when clear fails", async () => {
|
||||
const addToast = vi.fn();
|
||||
vi.mocked(dashboardApi.updateTask).mockRejectedValueOnce(new Error("boom"));
|
||||
|
||||
render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({ id: "FN-T", column: "todo", overlapBlockedBy: "FN-OVER", status: "queued" })}
|
||||
onClose={noop}
|
||||
onMoveTask={noopMove}
|
||||
onDeleteTask={noopDelete}
|
||||
onMergeTask={noopMerge}
|
||||
onOpenDetail={noopOpenDetail}
|
||||
addToast={addToast}
|
||||
/>,
|
||||
);
|
||||
|
||||
await userEvent.click(screen.getByRole("button", { name: "Clear" }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(addToast).toHaveBeenCalledWith("boom", "error");
|
||||
});
|
||||
expect(screen.getByText("File scope overlap blocker: FN-OVER (stale)")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows overlap blockedBy summary in Blocking section", () => {
|
||||
render(
|
||||
<TaskDetailModal
|
||||
|
||||
@@ -1917,6 +1917,75 @@ describe("PATCH /tasks/:id", () => {
|
||||
expect(res.body.dependencies).toEqual(["FN-002"]);
|
||||
});
|
||||
|
||||
it("clears overlapBlockedBy when null is provided", async () => {
|
||||
const updatedTask = { ...FAKE_TASK_DETAIL, overlapBlockedBy: undefined };
|
||||
(store.updateTask as ReturnType<typeof vi.fn>).mockResolvedValue(updatedTask);
|
||||
|
||||
const res = await REQUEST(buildApp(), "PATCH", "/api/tasks/KB-001", JSON.stringify({ overlapBlockedBy: null }), {
|
||||
"Content-Type": "application/json",
|
||||
});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-001", {
|
||||
overlapBlockedBy: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("clears status when null is provided", async () => {
|
||||
const updatedTask = { ...FAKE_TASK_DETAIL, status: undefined };
|
||||
(store.updateTask as ReturnType<typeof vi.fn>).mockResolvedValue(updatedTask);
|
||||
|
||||
const res = await REQUEST(buildApp(), "PATCH", "/api/tasks/KB-001", JSON.stringify({ status: null }), {
|
||||
"Content-Type": "application/json",
|
||||
});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-001", {
|
||||
status: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("accepts overlapBlockedBy clear and status clear together", async () => {
|
||||
const updatedTask = { ...FAKE_TASK_DETAIL, overlapBlockedBy: undefined, status: undefined };
|
||||
(store.updateTask as ReturnType<typeof vi.fn>).mockResolvedValue(updatedTask);
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"PATCH",
|
||||
"/api/tasks/KB-001",
|
||||
JSON.stringify({ overlapBlockedBy: null, status: null }),
|
||||
{
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-001", {
|
||||
overlapBlockedBy: null,
|
||||
status: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects non-null status values", async () => {
|
||||
const res = await REQUEST(buildApp(), "PATCH", "/api/tasks/KB-001", JSON.stringify({ status: "queued" }), {
|
||||
"Content-Type": "application/json",
|
||||
});
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toBe("status may only be cleared via this endpoint (must be null)");
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects non-string non-null overlapBlockedBy values", async () => {
|
||||
const res = await REQUEST(buildApp(), "PATCH", "/api/tasks/KB-001", JSON.stringify({ overlapBlockedBy: 123 }), {
|
||||
"Content-Type": "application/json",
|
||||
});
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toBe("overlapBlockedBy must be a string or null");
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("forwards priority to store.updateTask without changing task column", async () => {
|
||||
const triageTask = { ...FAKE_TASK_DETAIL, column: "triage" as const, status: "awaiting-approval" as const };
|
||||
const updatedTask = { ...triageTask, priority: "high" as const };
|
||||
|
||||
@@ -2377,7 +2377,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
router.patch("/tasks/:id", async (req, res) => {
|
||||
try {
|
||||
const { store: scopedStore } = await getProjectContext(req);
|
||||
const { title, description, prompt, priority, dependencies, enabledWorkflowSteps, modelProvider, modelId, validatorModelProvider, validatorModelId, planningModelProvider, planningModelId, thinkingLevel, assigneeUserId, reviewLevel, executionMode, sourceIssue, nodeId, branch, baseBranch, githubTracking, noCommitsExpected } = req.body;
|
||||
const { title, description, prompt, priority, dependencies, enabledWorkflowSteps, modelProvider, modelId, validatorModelProvider, validatorModelId, planningModelProvider, planningModelId, thinkingLevel, assigneeUserId, reviewLevel, executionMode, sourceIssue, nodeId, branch, baseBranch, githubTracking, noCommitsExpected, overlapBlockedBy, status } = req.body;
|
||||
const hasBodyField = (field: string) => Object.prototype.hasOwnProperty.call(req.body, field);
|
||||
|
||||
// Validate model fields are strings or undefined/null
|
||||
@@ -2539,6 +2539,29 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
}
|
||||
}
|
||||
|
||||
let validatedOverlapBlockedBy: string | null | undefined;
|
||||
if (hasBodyField("overlapBlockedBy")) {
|
||||
if (overlapBlockedBy === null || overlapBlockedBy === undefined) {
|
||||
validatedOverlapBlockedBy = overlapBlockedBy;
|
||||
} else if (typeof overlapBlockedBy === "string") {
|
||||
const trimmed = overlapBlockedBy.trim();
|
||||
if (trimmed.length === 0) {
|
||||
throw new Error("overlapBlockedBy must be a string or null");
|
||||
}
|
||||
validatedOverlapBlockedBy = trimmed;
|
||||
} else {
|
||||
throw new Error("overlapBlockedBy must be a string or null");
|
||||
}
|
||||
}
|
||||
|
||||
let validatedStatus: null | undefined;
|
||||
if (hasBodyField("status")) {
|
||||
if (status !== null) {
|
||||
throw new Error("status may only be cleared via this endpoint (must be null)");
|
||||
}
|
||||
validatedStatus = null;
|
||||
}
|
||||
|
||||
const updates: Parameters<typeof scopedStore.updateTask>[1] = {};
|
||||
if (title !== undefined) updates.title = title;
|
||||
if (description !== undefined) updates.description = description;
|
||||
@@ -2564,6 +2587,8 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
if (hasBodyField("githubTracking")) {
|
||||
(updates as Record<string, unknown>).githubTracking = validatedGithubTracking;
|
||||
}
|
||||
if (hasBodyField("overlapBlockedBy")) updates.overlapBlockedBy = validatedOverlapBlockedBy;
|
||||
if (hasBodyField("status")) updates.status = validatedStatus;
|
||||
|
||||
if (hasBodyField("nodeId") && validatedNodeId !== undefined) {
|
||||
const currentTask = await scopedStore.getTask(req.params.id);
|
||||
@@ -2608,7 +2633,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
if (err instanceof ApiError) {
|
||||
throw err;
|
||||
}
|
||||
const status = (err instanceof Error ? err.message : String(err)).includes("must be a string") || (err instanceof Error ? err.message : String(err)).includes("must be a non-empty string") || (err instanceof Error ? err.message : String(err)).includes("must be a string or null") || (err instanceof Error ? err.message : String(err)).includes("must be an array of strings") || (err instanceof Error ? err.message : String(err)).includes("must be a boolean") || (err instanceof Error ? err.message : String(err)).includes("thinkingLevel must be one of") || (err instanceof Error ? err.message : String(err)).includes("reviewLevel must be an integer") || (err instanceof Error ? err.message : String(err)).includes("executionMode must be one of") || (err instanceof Error ? err.message : String(err)).includes("priority must be one of") || (err instanceof Error ? err.message : String(err)).includes("sourceIssue") ? 400 : 500;
|
||||
const status = (err instanceof Error ? err.message : String(err)).includes("must be a string") || (err instanceof Error ? err.message : String(err)).includes("must be a non-empty string") || (err instanceof Error ? err.message : String(err)).includes("must be a string or null") || (err instanceof Error ? err.message : String(err)).includes("must be an array of strings") || (err instanceof Error ? err.message : String(err)).includes("must be a boolean") || (err instanceof Error ? err.message : String(err)).includes("thinkingLevel must be one of") || (err instanceof Error ? err.message : String(err)).includes("reviewLevel must be an integer") || (err instanceof Error ? err.message : String(err)).includes("executionMode must be one of") || (err instanceof Error ? err.message : String(err)).includes("priority must be one of") || (err instanceof Error ? err.message : String(err)).includes("sourceIssue") || (err instanceof Error ? err.message : String(err)).includes("status may only be cleared") ? 400 : 500;
|
||||
throw new ApiError(status, err instanceof Error ? err.message : String(err));
|
||||
}
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user