diff --git a/apps/backend/internal/office/dashboard/service_tasks.go b/apps/backend/internal/office/dashboard/service_tasks.go index 6073951839..6dc275092d 100644 --- a/apps/backend/internal/office/dashboard/service_tasks.go +++ b/apps/backend/internal/office/dashboard/service_tasks.go @@ -111,6 +111,7 @@ func (s *DashboardService) UpdateTaskProjectID(ctx context.Context, taskID, proj } } s.publishTaskUpdated(ctx, taskID, []string{"project_id"}) + s.publishCanonicalTaskUpdated(ctx, taskID) return nil } diff --git a/apps/backend/internal/office/dashboard/task_updated_events_test.go b/apps/backend/internal/office/dashboard/task_updated_events_test.go index dff1605a73..0292887759 100644 --- a/apps/backend/internal/office/dashboard/task_updated_events_test.go +++ b/apps/backend/internal/office/dashboard/task_updated_events_test.go @@ -93,3 +93,23 @@ func TestUpdateTaskStatus_NoPublisherWiredIsSafe(t *testing.T) { t.Fatalf("update task status: %v", err) } } + +func TestUpdateTaskProjectIDPublishesCanonicalTaskUpdatedForAssignAndClear(t *testing.T) { + deps := newTestDeps(t) + insertTestTask(t, deps.db, "project-transition", "ws-project", "Project transition", "todo", 0) + insertTestProject(t, deps, "project-1", "ws-project") + pub := &recordingTaskLifecyclePublisher{} + deps.svc.SetTaskLifecyclePublisher(pub) + + if err := deps.svc.UpdateTaskProjectID(context.Background(), "project-transition", "project-1"); err != nil { + t.Fatalf("assign project: %v", err) + } + if err := deps.svc.UpdateTaskProjectID(context.Background(), "project-transition", ""); err != nil { + t.Fatalf("clear project: %v", err) + } + + want := []string{"project-transition", "project-transition"} + if len(pub.published) != len(want) || pub.published[0] != want[0] || pub.published[1] != want[1] { + t.Fatalf("published = %v, want %v", pub.published, want) + } +} diff --git a/apps/backend/internal/task/service/service_events.go b/apps/backend/internal/task/service/service_events.go index 6d3dff3804..7631ed3dd0 100644 --- a/apps/backend/internal/task/service/service_events.go +++ b/apps/backend/internal/task/service/service_events.go @@ -426,6 +426,9 @@ func (s *Service) publishTaskEventNow(ctx context.Context, eventType string, tas "updated_at": task.UpdatedAt.Format(time.RFC3339Nano), "is_ephemeral": task.IsEphemeral, "autopilot": task.Autopilot, + // Always explicit because a project reassignment can change Office + // ownership in either direction while a client has the task cached. + "is_from_office": task.IsFromOffice, // Consumers that restore quick-chat tabs filter on origin, so it has to // travel with the event and not just the HTTP DTO. "origin": task.Origin, diff --git a/apps/backend/internal/task/service/service_events_test.go b/apps/backend/internal/task/service/service_events_test.go index 2ffaf04615..61ff5c5860 100644 --- a/apps/backend/internal/task/service/service_events_test.go +++ b/apps/backend/internal/task/service/service_events_test.go @@ -717,6 +717,30 @@ func TestPublishTaskUpdated_EmitsAutopilot(t *testing.T) { } } +func TestPublishTaskUpdatedEmitsOfficeIdentityExplicitly(t *testing.T) { + for _, tc := range []struct { + name string + isFromOffice bool + }{ + {name: "office", isFromOffice: true}, + {name: "kanban", isFromOffice: false}, + } { + t.Run(tc.name, func(t *testing.T) { + svc, eventBus, _ := createTestService(t) + svc.PublishTaskUpdated(context.Background(), &models.Task{ + ID: "task-office-identity", WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", + IsFromOffice: tc.isFromOffice, + }) + + data := singlePublishedEventData(t, eventBus) + got, ok := data["is_from_office"].(bool) + if !ok || got != tc.isFromOffice { + t.Fatalf("is_from_office payload = %#v, want %t", data["is_from_office"], tc.isFromOffice) + } + }) + } +} + func TestPublishTaskUpdatedRedactsDeferredLaunchAttribution(t *testing.T) { svc, eventBus, _ := createTestService(t) task := &models.Task{ diff --git a/apps/web/components/task/mobile/session-task-switcher-sheet-item.test.ts b/apps/web/components/task/mobile/session-task-switcher-sheet-item.test.ts index 8398285455..5f99aa594d 100644 --- a/apps/web/components/task/mobile/session-task-switcher-sheet-item.test.ts +++ b/apps/web/components/task/mobile/session-task-switcher-sheet-item.test.ts @@ -75,3 +75,12 @@ describe("toSheetItem remote executor projection", () => { expect(item.primarySessionId).toBe("session-1"); }); }); + +describe("toSheetItem Office identity projection", () => { + // @covers AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.4 + it("carries Office identity into the phone task drawer row", () => { + const item = toSheetItem(task({ isFromOffice: true }), emptyCtx()); + + expect(item.isFromOffice).toBe(true); + }); +}); diff --git a/apps/web/components/task/mobile/session-task-switcher-sheet-item.ts b/apps/web/components/task/mobile/session-task-switcher-sheet-item.ts index 01c85260af..936aa8a642 100644 --- a/apps/web/components/task/mobile/session-task-switcher-sheet-item.ts +++ b/apps/web/components/task/mobile/session-task-switcher-sheet-item.ts @@ -114,6 +114,7 @@ export function toSheetItem( primaryExecutorProfileId: task.primaryExecutorProfileId ?? undefined, workflowStepColor: facts.workflowStepColor, isArchived: task.isArchived === true, + isFromOffice: task.isFromOffice, isRemoteExecutor: task.isRemoteExecutor, remoteExecutorId: task.primaryExecutorId ?? undefined, remoteExecutorType: task.primaryExecutorType ?? undefined, diff --git a/apps/web/components/task/mobile/session-task-switcher-sheet-props.ts b/apps/web/components/task/mobile/session-task-switcher-sheet-props.ts index 961679b36a..589e122f40 100644 --- a/apps/web/components/task/mobile/session-task-switcher-sheet-props.ts +++ b/apps/web/components/task/mobile/session-task-switcher-sheet-props.ts @@ -20,6 +20,7 @@ export function buildMobileTaskSwitcherProps( ): TaskSwitcherProps { return { grouped: helpers.grouped, + nestHierarchyTasks: props.tasks, workflows: props.workflows, stepsByWorkflowId: props.stepsByWorkflowId, activeTaskId: props.activeTaskId, diff --git a/apps/web/components/task/task-nest-context-menu.tsx b/apps/web/components/task/task-nest-context-menu.tsx index cfdfd0dd52..14c8e66ddd 100644 --- a/apps/web/components/task/task-nest-context-menu.tsx +++ b/apps/web/components/task/task-nest-context-menu.tsx @@ -8,7 +8,6 @@ import { ContextMenuSubContent, ContextMenuSubTrigger, } from "@kandev/ui/context-menu"; -import { useAppStore } from "@/components/state-provider"; import { useNestTask } from "@/hooks/use-nest-task"; import { computeNestCandidates } from "@/lib/sidebar/nest-candidates"; import type { TaskSwitcherItem } from "./task-switcher"; @@ -16,6 +15,8 @@ import { useTranslation } from "react-i18next"; type TaskNestContextMenuItemsProps = { task: TaskSwitcherItem; + nestCandidateTasks?: TaskSwitcherItem[]; + nestHierarchyTasks?: TaskSwitcherItem[]; disabled?: boolean; }; @@ -25,25 +26,23 @@ type TaskNestContextMenuItemsProps = { * other tasks in the same workflow, excluding the task's own descendants * (which would create a cycle) and its current parent. */ -export function TaskNestContextMenuItems({ task, disabled }: TaskNestContextMenuItemsProps) { +export function TaskNestContextMenuItems({ + task, + nestCandidateTasks = [], + nestHierarchyTasks, + disabled, +}: TaskNestContextMenuItemsProps) { const { t } = useTranslation(); const workflowId = task.workflowId; - // Prefer the all-workflows snapshot; fall back to the active kanban tasks, - // which the sidebar also renders from before the multi-snapshot fetch - // resolves (e.g. initial /t/:id load). Without the fallback the menu would - // show "No other tasks" even though rows are visible. - const tasks = useAppStore((s) => { - if (!workflowId) return undefined; - return ( - s.kanbanMulti?.snapshots?.[workflowId]?.tasks ?? - (s.kanban?.workflowId === workflowId ? s.kanban?.tasks : undefined) - ); - }); const nestTask = useNestTask(); if (!workflowId || task.isArchived) return null; - const candidates = computeNestCandidates(tasks ?? [], task.id); + const candidates = computeNestCandidates( + nestCandidateTasks.filter((candidate) => candidate.workflowId === workflowId), + task.id, + nestHierarchyTasks ?? nestCandidateTasks, + ); const hasParent = Boolean(task.parentTaskId); return ( diff --git a/apps/web/components/task/task-session-sidebar-aggregate.test.ts b/apps/web/components/task/task-session-sidebar-aggregate.test.ts index 86b003c3d6..526e42571e 100644 --- a/apps/web/components/task/task-session-sidebar-aggregate.test.ts +++ b/apps/web/components/task/task-session-sidebar-aggregate.test.ts @@ -233,6 +233,26 @@ describe("aggregateSidebarTasks active precedence", () => { expect(result.allTasks[0].autopilot).toBe(true); }); + + // @covers AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.4 + it("preserves projected Office identity through a newer partial active task", () => { + const projected = makeTask("t1", "s1", { + isFromOffice: true, + updatedAt: "2026-08-03T00:00:00Z", + }); + const active = makeTask("t1", "s1", { + updatedAt: "2026-08-03T00:00:01Z", + }); + + const result = aggregateSidebarTasks( + { "wf-1": makeSnapshot([makeStep("s1", 0)], [projected]) }, + "wf-1", + [active], + [makeStep("s1", 0)], + ); + + expect(result.allTasks[0].isFromOffice).toBe(true); + }); }); describe("aggregateSidebarTasks task lifecycle freshness", () => { diff --git a/apps/web/components/task/task-session-sidebar-aggregate.ts b/apps/web/components/task/task-session-sidebar-aggregate.ts index 6c7790d0bc..5bb097a272 100644 --- a/apps/web/components/task/task-session-sidebar-aggregate.ts +++ b/apps/web/components/task/task-session-sidebar-aggregate.ts @@ -114,6 +114,7 @@ function reconcileTaskProjection( // a partial WS event, so do not let it erase the value from the full // workflow projection. autopilot: active.autopilot ?? projected.autopilot, + isFromOffice: active.isFromOffice ?? projected.isFromOffice, _workflowId: activeWorkflowId, }; const task = activeTaskIsNewer(active, projected) ? activeTask : projected; diff --git a/apps/web/components/task/task-session-sidebar-item.test.ts b/apps/web/components/task/task-session-sidebar-item.test.ts index 564915a8ad..eb5090da3a 100644 --- a/apps/web/components/task/task-session-sidebar-item.test.ts +++ b/apps/web/components/task/task-session-sidebar-item.test.ts @@ -168,6 +168,15 @@ describe("buildSidebarItem", () => { }); }); +describe("buildSidebarItem Office identity", () => { + // @covers AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.4 + it("preserves isFromOffice for candidate filtering", () => { + const item = buildSidebarItem(task({ isFromOffice: true }), emptyContext()); + + expect(item.isFromOffice).toBe(true); + }); +}); + describe("buildSidebarItem — parked on background work", () => { it("carries the parked-on-background-work projection through to the sidebar row", () => { const item = buildSidebarItem(task({ parkedOnBackgroundWork: true }), emptyContext()); diff --git a/apps/web/components/task/task-session-sidebar-item.ts b/apps/web/components/task/task-session-sidebar-item.ts index 3ece9e0d3f..52dd0ea00c 100644 --- a/apps/web/components/task/task-session-sidebar-item.ts +++ b/apps/web/components/task/task-session-sidebar-item.ts @@ -159,6 +159,7 @@ export function buildSidebarItem( remoteExecutorName: task.primaryExecutorName ?? undefined, createdAt: task.createdAt, isArchived: task.isArchived === true, + isFromOffice: task.isFromOffice, parentTaskTitle: task.parentTaskId ? context.titleById.get(task.parentTaskId) : undefined, parentTaskId: task.parentTaskId ?? undefined, workspaceMode: task.workspaceMode, diff --git a/apps/web/components/task/task-session-sidebar-switcher-props.ts b/apps/web/components/task/task-session-sidebar-switcher-props.ts index e3afc193ff..00aabac358 100644 --- a/apps/web/components/task/task-session-sidebar-switcher-props.ts +++ b/apps/web/components/task/task-session-sidebar-switcher-props.ts @@ -14,6 +14,7 @@ type TaskSwitcherComponentProps = ComponentProps; */ export function buildTaskSwitcherProps(args: { grouped: TaskSwitcherComponentProps["grouped"]; + nestHierarchyTasks: TaskSwitcherComponentProps["nestHierarchyTasks"]; workflows: TaskSwitcherComponentProps["workflows"]; stepsByWorkflowId: TaskSwitcherComponentProps["stepsByWorkflowId"]; highlightedTaskId: string | null; @@ -49,6 +50,7 @@ export function buildTaskSwitcherProps(args: { }): TaskSwitcherComponentProps { return { grouped: args.grouped, + nestHierarchyTasks: args.nestHierarchyTasks, workflows: args.workflows, stepsByWorkflowId: args.stepsByWorkflowId, activeTaskId: args.highlightedTaskId, diff --git a/apps/web/components/task/task-session-sidebar.tsx b/apps/web/components/task/task-session-sidebar.tsx index 79e9c2c64f..1fa8a27926 100644 --- a/apps/web/components/task/task-session-sidebar.tsx +++ b/apps/web/components/task/task-session-sidebar.tsx @@ -501,6 +501,7 @@ export const TaskSessionSidebar = memo(function TaskSessionSidebar({ ); const switcherProps = buildTaskSwitcherProps({ grouped, + nestHierarchyTasks: displayTasks, workflows, stepsByWorkflowId, highlightedTaskId, diff --git a/apps/web/components/task/task-switcher-context-menu-items.tsx b/apps/web/components/task/task-switcher-context-menu-items.tsx index ac1068094f..d182f0de25 100644 --- a/apps/web/components/task/task-switcher-context-menu-items.tsx +++ b/apps/web/components/task/task-switcher-context-menu-items.tsx @@ -47,6 +47,8 @@ type LinkActions = ReturnType; type SingleGroupProps = Pick< SingleSelectionMenuProps, | "task" + | "nestCandidateTasks" + | "nestHierarchyTasks" | "isDeleting" | "isArchiving" | "isPinned" @@ -282,6 +284,8 @@ function SingleEditGroup({ function SingleRelationshipsGroup({ task, + nestCandidateTasks, + nestHierarchyTasks, isDeleting, onCreateSubtask, onDetachTask, @@ -292,6 +296,8 @@ function SingleRelationshipsGroup({ }: Pick< SingleGroupProps, | "task" + | "nestCandidateTasks" + | "nestHierarchyTasks" | "isDeleting" | "onCreateSubtask" | "onDetachTask" @@ -303,7 +309,14 @@ function SingleRelationshipsGroup({ return ( <> - {!task.isArchived && } + {!task.isArchived && ( + + )} {hasLinkGroup && linkActions && pluginLinkActions && ( { parentTaskId: "parent-1", })} workflows={[ - { id: WORKFLOW_ID, name: "Workflow 1" }, + { id: WORKFLOW_ID, name: WORKFLOW_1_NAME }, { id: "workflow-2", name: "Workflow 2" }, ]} stepsByWorkflowId={{ @@ -246,7 +247,7 @@ describe("TaskItemWithContextMenu — grouped bulk actions", () => { task={task({ workflowId: WORKFLOW_ID, workflowStepId: STEP_ID })} selectedTaskIds={new Set(["task-1", "task-2"])} workflows={[ - { id: WORKFLOW_ID, name: "Workflow 1" }, + { id: WORKFLOW_ID, name: WORKFLOW_1_NAME }, { id: "workflow-2", name: "Workflow 2" }, ]} stepsByWorkflowId={{ @@ -309,7 +310,7 @@ function renderWorkflowMoveMenu({ workflowId: WORKFLOW_1_ID, workflowStepId: "step-1", })} - workflows={[{ id: WORKFLOW_1_ID, name: "Workflow 1" }]} + workflows={[{ id: WORKFLOW_1_ID, name: WORKFLOW_1_NAME }]} stepsByWorkflowId={{ [WORKFLOW_1_ID]: [ { id: "step-1", title: "Todo" }, diff --git a/apps/web/components/task/task-switcher-context-menu.tsx b/apps/web/components/task/task-switcher-context-menu.tsx index d940ba032b..4a13017f4c 100644 --- a/apps/web/components/task/task-switcher-context-menu.tsx +++ b/apps/web/components/task/task-switcher-context-menu.tsx @@ -26,6 +26,10 @@ export { createTaskLinkSelectAction } from "./task-switcher-link-menu"; type ContextMenuProps = TaskLinkHandlers & { task: TaskSwitcherItem; + nestCandidateTasks?: TaskSwitcherItem[]; + nestHierarchyTasks?: TaskSwitcherItem[]; + getNestCandidateTasks?: () => TaskSwitcherItem[]; + getNestHierarchyTasks?: () => TaskSwitcherItem[] | undefined; workflows?: TaskMoveWorkflow[]; stepsByWorkflowId?: Record; steps?: StepDef[]; @@ -55,19 +59,40 @@ type ContextMenuProps = TaskLinkHandlers & { isMixedWorkflowSelection?: boolean; }; +type OpenedNestSources = { + candidates?: TaskSwitcherItem[]; + hierarchy?: TaskSwitcherItem[]; +}; + +function useContextMenuOpenState(props: ContextMenuProps) { + const [contextOpen, setContextOpen] = useState(false); + const [openedNestSources, setOpenedNestSources] = useState({}); + const handleContextOpenChange = (open: boolean) => { + if (open) { + setOpenedNestSources({ + candidates: props.getNestCandidateTasks?.() ?? props.nestCandidateTasks, + hierarchy: props.getNestHierarchyTasks?.() ?? props.nestHierarchyTasks, + }); + } + setContextOpen(open); + }; + return { contextOpen, setContextOpen, openedNestSources, handleContextOpenChange }; +} + // This component coordinates the context menu and drag cancellation. Archive // state lives in its focused adapter so unavailable actions stay unavailable. export function TaskItemWithContextMenu(props: ContextMenuProps) { const { children, ...menuProps } = props; const { task, stepsByWorkflowId, steps, onRequestMoveOptions, onBeforeMoveOptionsOpen } = props; - const [contextOpen, setContextOpen] = useState(false); + const { contextOpen, setContextOpen, openedNestSources, handleContextOpenChange } = + useContextMenuOpenState(props); const [menuKey, setMenuKey] = useState(0); const moveTasks = useTaskWorkflowMove(); const closeMenu = () => { setContextOpen(false); setMenuKey((k) => k + 1); }; - const { handleOpenChange, triggerProps } = useMenuTouchDragCancel(setContextOpen); + const { handleOpenChange, triggerProps } = useMenuTouchDragCancel(handleContextOpenChange); const { isFinePointer, isMobile } = useResponsiveBreakpoint(); const usesTouchDrawer = useTouchDrawer(); const archive = useTaskSwitcherArchiveConfirmation({ @@ -107,11 +132,17 @@ export function TaskItemWithContextMenu(props: ContextMenuProps) { // owns its own drawer, so the inline path stays off there too. const inlineSubmitWithOptions = !usesTouchDrawer && !onRequestMoveOptions ? submitMoveOptionsForStep : undefined; - const contextMenuProps = buildTaskContextMenuItemsProps(props, closeMenu, moveTasks, { - onMoveToStepWithOptions: handleMoveToStepWithOptions, - onSubmitWithOptions: inlineSubmitWithOptions, - isMoving, - }); + const contextMenuProps = buildTaskContextMenuItemsProps( + props, + openedNestSources, + closeMenu, + moveTasks, + { + onMoveToStepWithOptions: handleMoveToStepWithOptions, + onSubmitWithOptions: inlineSubmitWithOptions, + isMoving, + }, + ); return ( <> @@ -157,7 +188,11 @@ export function TaskItemWithContextMenu(props: ContextMenuProps) { export type TaskContextMenuItemsProps = Omit< ContextMenuProps, - "children" | "onBeforeMoveOptionsOpen" | "onRequestMoveOptions" + | "children" + | "onBeforeMoveOptionsOpen" + | "onRequestMoveOptions" + | "getNestCandidateTasks" + | "getNestHierarchyTasks" > & { closeMenu: () => void; moveTasks: ReturnType; @@ -171,6 +206,7 @@ export type TaskContextMenuItemsProps = Omit< function buildTaskContextMenuItemsProps( props: ContextMenuProps, + nestSources: OpenedNestSources, closeMenu: () => void, moveTasks: ReturnType, moveOptions: { @@ -186,10 +222,14 @@ function buildTaskContextMenuItemsProps( children: _children, onRequestMoveOptions: _onRequestMoveOptions, onBeforeMoveOptionsOpen: _onBeforeMoveOptionsOpen, + getNestCandidateTasks: _getNestCandidateTasks, + getNestHierarchyTasks: _getNestHierarchyTasks, ...rest } = props; return { ...rest, + nestCandidateTasks: nestSources.candidates ?? rest.nestCandidateTasks, + nestHierarchyTasks: nestSources.hierarchy ?? rest.nestHierarchyTasks, onMoveToStepWithOptions: moveOptions.onMoveToStepWithOptions, onSubmitWithOptions: moveOptions.onSubmitWithOptions, moveOptionsBusy: moveOptions.isMoving, diff --git a/apps/web/components/task/task-switcher-nest-context-menu.test.tsx b/apps/web/components/task/task-switcher-nest-context-menu.test.tsx new file mode 100644 index 0000000000..a753682484 --- /dev/null +++ b/apps/web/components/task/task-switcher-nest-context-menu.test.tsx @@ -0,0 +1,112 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { StateProvider } from "@/components/state-provider"; +import { ToastProvider } from "@/components/toast-provider"; +import { TaskItemWithContextMenu } from "./task-switcher-context-menu"; +import type { TaskSwitcherItem } from "./task-switcher-types"; + +const WORKFLOW_ID = "workflow-1"; +const STEP_ID = "step-1"; + +function task(overrides: Partial = {}): TaskSwitcherItem { + return { id: "task-1", title: "Task 1", state: "IN_PROGRESS", ...overrides }; +} + +function storeTask(item: TaskSwitcherItem) { + return { + id: item.id, + title: item.title, + workflowId: item.workflowId, + workflowStepId: item.workflowStepId, + position: 0, + }; +} + +afterEach(cleanup); + +describe("TaskItemWithContextMenu Nest under candidates", () => { + // @covers AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.1 + // @covers AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.2 + it("uses rendered candidates when the workflow snapshot is partial", async () => { + const current = task({ workflowId: WORKFLOW_ID, workflowStepId: STEP_ID }); + const target = task({ + id: "visible-target", + title: "Visible target", + workflowId: WORKFLOW_ID, + workflowStepId: STEP_ID, + }); + + render( + + + +
Task 1
+
+
+
, + ); + + fireEvent.contextMenu(screen.getByTestId("task-row")); + const nestUnder = await screen.findByRole("menuitem", { name: "Nest under" }); + fireEvent.pointerMove(nestUnder, { pointerType: "mouse" }); + + expect(await screen.findByRole("menuitem", { name: "Visible target" })).not.toBeNull(); + }); + + it("captures the latest candidates when the menu opens", async () => { + const current = task({ workflowId: WORKFLOW_ID, workflowStepId: STEP_ID }); + const staleTarget = task({ + id: "stale-target", + title: "Stale target", + workflowId: WORKFLOW_ID, + workflowStepId: STEP_ID, + }); + const latestTarget = task({ + id: "latest-target", + title: "Latest target", + workflowId: WORKFLOW_ID, + workflowStepId: STEP_ID, + }); + let candidates = [current, staleTarget]; + + render( + + + candidates}> +
Task 1
+
+
+
, + ); + candidates = [current, latestTarget]; + + fireEvent.contextMenu(screen.getByTestId("fresh-task-row")); + const nestUnder = await screen.findByRole("menuitem", { name: "Nest under" }); + fireEvent.pointerMove(nestUnder, { pointerType: "mouse" }); + + expect(await screen.findByRole("menuitem", { name: "Latest target" })).not.toBeNull(); + expect(screen.queryByRole("menuitem", { name: "Stale target" })).toBeNull(); + }); +}); diff --git a/apps/web/components/task/task-switcher-render-stability.test.tsx b/apps/web/components/task/task-switcher-render-stability.test.tsx index f31590f286..72ac56efa5 100644 --- a/apps/web/components/task/task-switcher-render-stability.test.tsx +++ b/apps/web/components/task/task-switcher-render-stability.test.tsx @@ -8,6 +8,7 @@ const renderCounts = vi.hoisted(() => ({ groups: new Map(), nodes: new Map(), rows: new Map(), + nestCandidateGetters: new Map TaskSwitcherItem[]) | undefined>(), })); function increment(counts: Map, key: string) { @@ -22,7 +23,18 @@ vi.mock("./task-item", () => ({ })); vi.mock("./task-switcher-context-menu", () => ({ - TaskItemWithContextMenu: ({ children }: { children: ReactElement }) => children, + TaskItemWithContextMenu: ({ + task, + children, + getNestCandidateTasks, + }: { + task: TaskSwitcherItem; + children: ReactElement; + getNestCandidateTasks?: () => TaskSwitcherItem[]; + }) => { + renderCounts.nestCandidateGetters.set(task.id, getNestCandidateTasks); + return children; + }, })); vi.mock("./task-switcher-subtask-dnd", () => ({ @@ -131,6 +143,7 @@ beforeEach(() => { renderCounts.groups.clear(); renderCounts.nodes.clear(); renderCounts.rows.clear(); + renderCounts.nestCandidateGetters.clear(); }); afterEach(() => cleanup()); @@ -189,6 +202,20 @@ describe("TaskSwitcher render stability", () => { expect(latestHandler).toHaveBeenCalledWith(TASK_B); }); + + it("gives a skipped row the latest nest candidates", () => { + const taskA = task(TASK_A, WORKFLOW_A); + const taskB = task(TASK_B, WORKFLOW_A); + const view = render(switcher(groupedTogether([taskA, taskB]))); + + view.rerender(switcher(groupedTogether([{ ...taskA, title: TASK_A_UPDATED }, { ...taskB }]))); + + expect( + renderCounts.nestCandidateGetters + .get(TASK_B)?.() + .map((item) => item.title), + ).toEqual([TASK_A_UPDATED, TASK_B]); + }); }); describe("TaskSwitcher nested render stability", () => { diff --git a/apps/web/components/task/task-switcher-row.tsx b/apps/web/components/task/task-switcher-row.tsx index 9693185d0f..4ad84f70c2 100644 --- a/apps/web/components/task/task-switcher-row.tsx +++ b/apps/web/components/task/task-switcher-row.tsx @@ -16,6 +16,9 @@ export type SubtaskToggleInfo = { export type TaskRowProps = { task: TaskSwitcherItem; + nestCandidateTasks?: TaskSwitcherItem[]; + getNestCandidateTasks?: () => TaskSwitcherItem[]; + getNestHierarchyTasks?: () => TaskSwitcherItem[] | undefined; isSubTask?: boolean; depth?: number; subtaskToggle?: SubtaskToggleInfo; @@ -66,6 +69,9 @@ function archiveAware(value: T | undefined, isArchived: boolean): T | undefin function getContextMenuProps(props: TaskRowProps, isArchived: boolean) { return { + nestCandidateTasks: props.nestCandidateTasks, + getNestCandidateTasks: props.getNestCandidateTasks, + getNestHierarchyTasks: props.getNestHierarchyTasks, onEditTask: archiveAware(props.onEditTask, isArchived), onRenameTask: archiveAware(props.onRenameTask, isArchived), onArchiveTask: archiveAware(props.onArchiveTask, isArchived), diff --git a/apps/web/components/task/task-switcher-subtask-dnd.test.ts b/apps/web/components/task/task-switcher-subtask-dnd.test.ts index 2755231701..212995b5dd 100644 --- a/apps/web/components/task/task-switcher-subtask-dnd.test.ts +++ b/apps/web/components/task/task-switcher-subtask-dnd.test.ts @@ -17,13 +17,24 @@ const WORKFLOW = "wf-1"; const OTHER_WORKFLOW = "wf-2"; const PWC = "parent-with-child"; // root that already has a child const PWC_CHILD = "child-of-pwc"; +const OFFICE_SUBJECT = "office-subject"; +const OFFICE_TARGET_ROOT = "office-target-root"; +const HIDDEN_CHILD = "hidden-child"; -function task(id: string, overrides: { workflowId?: string; parentTaskId?: string | null } = {}) { +function task( + id: string, + overrides: { + workflowId?: string; + parentTaskId?: string | null; + isFromOffice?: boolean; + } = {}, +) { return { id, title: id, workflowId: overrides.workflowId ?? WORKFLOW, parentTaskId: overrides.parentTaskId ?? undefined, + isFromOffice: overrides.isFromOffice, }; } @@ -154,4 +165,39 @@ describe("computeNestTargets", () => { expect(targets.has("other-wf")).toBe(false); expect(targets.has(SUB_C)).toBe(false); }); + + // @covers AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.1 + // @covers AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.4 + it("matches the shared Office candidates for a subject with children", () => { + const officeTasks = [ + task(OFFICE_SUBJECT, { isFromOffice: true }), + task("office-descendant", { + parentTaskId: OFFICE_SUBJECT, + isFromOffice: true, + }), + task(OFFICE_TARGET_ROOT, { isFromOffice: true }), + task("office-target-child", { + parentTaskId: OFFICE_TARGET_ROOT, + isFromOffice: true, + }), + ]; + + expect(computeNestTargets(officeTasks[0], officeTasks)).toEqual( + new Set([OFFICE_TARGET_ROOT, "office-target-child"]), + ); + }); + + it("excludes a descendant whose intermediate ancestor is hidden", () => { + const hierarchy = [ + task(OFFICE_SUBJECT, { isFromOffice: true }), + task(HIDDEN_CHILD, { parentTaskId: OFFICE_SUBJECT, isFromOffice: true }), + task("visible-grandchild", { parentTaskId: HIDDEN_CHILD, isFromOffice: true }), + task("valid-target", { isFromOffice: true }), + ]; + const visibleTasks = hierarchy.filter((candidate) => candidate.id !== HIDDEN_CHILD); + + expect(computeNestTargets(hierarchy[0], visibleTasks, hierarchy)).toEqual( + new Set(["valid-target"]), + ); + }); }); diff --git a/apps/web/components/task/task-switcher-subtask-dnd.tsx b/apps/web/components/task/task-switcher-subtask-dnd.tsx index edcdec6785..f1c5819062 100644 --- a/apps/web/components/task/task-switcher-subtask-dnd.tsx +++ b/apps/web/components/task/task-switcher-subtask-dnd.tsx @@ -122,16 +122,20 @@ const GROUP_ROOT_LEVEL = "__group_root__"; /** * Computes the rows a drag may nest the active task under, mirroring the * context menu's candidate rules (computeNestCandidates) scoped to the - * active task's workflow: same-workflow roots, excluding the task itself, - * its current parent, and any subtask (which also excludes its descendants). + * active task's workflow. Every mode excludes the task, its current parent, + * and descendants; Kanban keeps the root-only depth limit while Office may + * target tasks at any depth. */ export function computeNestTargets( activeTask: Pick | undefined, groupTasks: TaskSwitcherItem[], + hierarchyTasks: readonly TaskSwitcherItem[] = groupTasks, ): Set { if (!activeTask) return new Set(); const sameWorkflow = groupTasks.filter((t) => t.workflowId === activeTask.workflowId); - return new Set(computeNestCandidates(sameWorkflow, activeTask.id).map((t) => t.id)); + return new Set( + computeNestCandidates(sameWorkflow, activeTask.id, hierarchyTasks).map((task) => task.id), + ); } /** @@ -328,6 +332,47 @@ export function SortableTaskLevel({ ); } +function useActiveNestDrag( + groupTasks: TaskSwitcherItem[], + tasksById: ReadonlyMap, + getNestHierarchyTasks?: () => TaskSwitcherItem[] | undefined, +) { + const [activeDrag, setActiveDrag] = useState<{ + id: string; + hierarchyTasks?: TaskSwitcherItem[]; + } | null>(null); + const activeDragId = activeDrag?.id ?? null; + const nestTargetIds = useMemo( + () => + computeNestTargets( + activeDragId ? tasksById.get(activeDragId) : undefined, + groupTasks, + activeDrag?.hierarchyTasks ?? groupTasks, + ), + [activeDrag, activeDragId, tasksById, groupTasks], + ); + const handleDragStart = useCallback( + (event: DragStartEvent) => { + setActiveDrag({ + id: String(event.active.id), + hierarchyTasks: getNestHierarchyTasks?.(), + }); + }, + [getNestHierarchyTasks], + ); + const clearActiveDrag = useCallback(() => setActiveDrag(null), []); + return { activeDragId, nestTargetIds, handleDragStart, clearActiveDrag }; +} + +type TaskTreeDndGroupProps = { + groupTasks: TaskSwitcherItem[]; + getNestHierarchyTasks?: () => TaskSwitcherItem[] | undefined; + onReorderGroup?: (groupTaskIds: string[]) => void; + onReorderSubtasks?: (parentTaskId: string, orderedSubtaskIds: string[]) => void; + onNestTask?: (activeTaskId: string, parentTaskId: string) => void; + children: (nestTargetIds: Set, activeDragId: string | null) => React.ReactNode; +}; + /** * One group-spanning DndContext for the sidebar task tree. Replaces the * per-level contexts so a drag can target rows outside its own sibling level: @@ -339,18 +384,12 @@ export function SortableTaskLevel({ */ export function TaskTreeDndGroup({ groupTasks, + getNestHierarchyTasks, onReorderGroup, onReorderSubtasks, onNestTask, children, -}: { - groupTasks: TaskSwitcherItem[]; - onReorderGroup?: (groupTaskIds: string[]) => void; - onReorderSubtasks?: (parentTaskId: string, orderedSubtaskIds: string[]) => void; - onNestTask?: (activeTaskId: string, parentTaskId: string) => void; - children: (nestTargetIds: Set, activeDragId: string | null) => React.ReactNode; -}) { - const [activeDragId, setActiveDragId] = useState(null); +}: TaskTreeDndGroupProps) { const sensors = useSensors( useSensor(MouseSensor, { activationConstraint: TASK_SWITCHER_DRAG_ACTIVATION_CONSTRAINTS.pointer, @@ -378,18 +417,14 @@ export function TaskTreeDndGroup({ childrenByParent: byParent, }; }, [groupTasks]); - const nestTargetIds = useMemo( - () => computeNestTargets(activeDragId ? tasksById.get(activeDragId) : undefined, groupTasks), - [activeDragId, tasksById, groupTasks], + const { activeDragId, nestTargetIds, handleDragStart, clearActiveDrag } = useActiveNestDrag( + groupTasks, + tasksById, + getNestHierarchyTasks, ); - - const handleDragStart = useCallback((event: DragStartEvent) => { - setActiveDragId(String(event.active.id)); - }, []); - const handleDragCancel = useCallback(() => setActiveDragId(null), []); const handleDragEnd = useCallback( (event: DragEndEvent) => { - setActiveDragId(null); + clearActiveDrag(); const { active, over } = event; if (!over) return; const decision = resolveSidebarDrop({ @@ -411,7 +446,14 @@ export function TaskTreeDndGroup({ break; } }, - [groupRootIds, childrenByParent, onReorderGroup, onReorderSubtasks, onNestTask], + [ + clearActiveDrag, + groupRootIds, + childrenByParent, + onReorderGroup, + onReorderSubtasks, + onNestTask, + ], ); const collisionDetection = useCallback((args) => { @@ -432,7 +474,7 @@ export function TaskTreeDndGroup({ collisionDetection={collisionDetection} onDragStart={handleDragStart} onDragEnd={handleDragEnd} - onDragCancel={handleDragCancel} + onDragCancel={clearActiveDrag} > {children(nestTargetIds, activeDragId)} diff --git a/apps/web/components/task/task-switcher-tree.tsx b/apps/web/components/task/task-switcher-tree.tsx index abc6eb521e..dc197fe0e7 100644 --- a/apps/web/components/task/task-switcher-tree.tsx +++ b/apps/web/components/task/task-switcher-tree.tsx @@ -1,6 +1,6 @@ "use client"; -import { memo, useCallback, useMemo } from "react"; +import { memo, useCallback, useLayoutEffect, useMemo, useRef } from "react"; import { countGroupTasks, type SidebarGroup } from "@/lib/sidebar/apply-view"; import { SortableTaskLevel, @@ -14,7 +14,14 @@ import type { TaskSwitcherItem } from "./task-switcher-types"; export type TaskRowBaseProps = Omit< TaskRowProps, - "task" | "subtaskToggle" | "isPinned" | "isSubTask" | "depth" + | "task" + | "nestCandidateTasks" + | "getNestCandidateTasks" + | "getNestHierarchyTasks" + | "subtaskToggle" + | "isPinned" + | "isSubTask" + | "depth" >; type TaskTreeContext = { @@ -23,6 +30,8 @@ type TaskTreeContext = { onToggleSubtasks?: (parentTaskId: string) => void; pinnedSet: Set; rowProps: TaskRowBaseProps; + getNestCandidateTasks: () => TaskSwitcherItem[]; + getNestHierarchyTasks?: () => TaskSwitcherItem[] | undefined; onReorderGroup?: (groupTaskIds: string[]) => void; onReorderSubtasks?: (parentTaskId: string, orderedSubtaskIds: string[]) => void; onNestTask?: (taskId: string, parentTaskId: string) => void; @@ -74,6 +83,8 @@ function taskTreeNodeEqual(previous: TaskTreeNodeProps, next: TaskTreeNodeProps) previous.depth !== next.depth || previous.isDraggable !== next.isDraggable || previous.ctx.rowProps !== next.ctx.rowProps || + previous.ctx.getNestCandidateTasks !== next.ctx.getNestCandidateTasks || + previous.ctx.getNestHierarchyTasks !== next.ctx.getNestHierarchyTasks || previous.ctx.onToggleSubtasks !== next.ctx.onToggleSubtasks || previous.ctx.onReorderGroup !== next.ctx.onReorderGroup || previous.ctx.onReorderSubtasks !== next.ctx.onReorderSubtasks || @@ -125,6 +136,8 @@ const TaskTreeNode = memo(function TaskTreeNode({ subtaskToggle={toggleInfo} isPinned={isRoot && ctx.pinnedSet.has(task.id)} {...ctx.rowProps} + getNestCandidateTasks={ctx.getNestCandidateTasks} + getNestHierarchyTasks={ctx.getNestHierarchyTasks} onTogglePin={isRoot ? ctx.rowProps.onTogglePin : undefined} /> {isNestTarget && } @@ -184,6 +197,7 @@ function getReorderHandler(parentTaskId: string | null, ctx: TaskTreeContext) { export type GroupSectionProps = { group: SidebarGroup; subTasksByParentId: Map; + getNestHierarchyTasks?: () => TaskSwitcherItem[] | undefined; rowProps: TaskRowBaseProps; pinnedSet: Set; isCollapsed: boolean; @@ -230,6 +244,7 @@ function groupSectionEqual(previous: GroupSectionProps, next: GroupSectionProps) if ( previous.group !== next.group || previous.rowProps !== next.rowProps || + previous.getNestHierarchyTasks !== next.getNestHierarchyTasks || previous.pinnedSet !== next.pinnedSet || previous.isCollapsed !== next.isCollapsed || previous.onToggleGroup !== next.onToggleGroup || @@ -286,6 +301,7 @@ function flattenGroupTasks( export const GroupSection = memo(function GroupSection({ group, subTasksByParentId, + getNestHierarchyTasks, rowProps, pinnedSet, isCollapsed, @@ -302,6 +318,11 @@ export const GroupSection = memo(function GroupSection({ () => flattenGroupTasks(group.tasks, subTasksByParentId), [group.tasks, subTasksByParentId], ); + const groupTasksRef = useRef(groupTasks); + useLayoutEffect(() => { + groupTasksRef.current = groupTasks; + }, [groupTasks]); + const getNestCandidateTasks = useCallback(() => groupTasksRef.current, []); const renderTree = (nestTargetIds: Set) => { if (isCollapsed) return null; @@ -311,6 +332,8 @@ export const GroupSection = memo(function GroupSection({ onToggleSubtasks, pinnedSet, rowProps, + getNestCandidateTasks, + getNestHierarchyTasks, onReorderGroup, onReorderSubtasks, onNestTask, @@ -333,6 +356,7 @@ export const GroupSection = memo(function GroupSection({ )} ; activeTaskId: string | null; diff --git a/apps/web/components/task/task-switcher.tsx b/apps/web/components/task/task-switcher.tsx index 793d33e447..e16537ebbb 100644 --- a/apps/web/components/task/task-switcher.tsx +++ b/apps/web/components/task/task-switcher.tsx @@ -158,12 +158,14 @@ function buildGroupSectionProps( pinnedSet: Set; collapsedSet: Set; showHeader: boolean; + getNestHierarchyTasks: () => TaskSwitcherProps["nestHierarchyTasks"]; }, ): GroupSectionProps { - const { group, rowProps, pinnedSet, collapsedSet, showHeader } = options; + const { group, rowProps, pinnedSet, collapsedSet, showHeader, getNestHierarchyTasks } = options; return { group, subTasksByParentId: grouped.subTasksByParentId, + getNestHierarchyTasks, rowProps, pinnedSet, isCollapsed: collapsedSet.has(group.key), @@ -218,6 +220,11 @@ export const TaskSwitcher = memo(function TaskSwitcher(props: TaskSwitcherProps) () => new Set(props.collapsedGroupKeys ?? []), [props.collapsedGroupKeys], ); + const nestHierarchyTasksRef = useRef(props.nestHierarchyTasks); + useLayoutEffect(() => { + nestHierarchyTasksRef.current = props.nestHierarchyTasks; + }, [props.nestHierarchyTasks]); + const getNestHierarchyTasks = useMemo(() => () => nestHierarchyTasksRef.current, []); if (isLoading) return ; @@ -258,6 +265,7 @@ export const TaskSwitcher = memo(function TaskSwitcher(props: TaskSwitcherProps) pinnedSet, collapsedSet, showHeader: showHeaders, + getNestHierarchyTasks, })} /> ))} diff --git a/apps/web/e2e/tests/task/mobile-office-sidebar-nest-task.spec.ts b/apps/web/e2e/tests/task/mobile-office-sidebar-nest-task.spec.ts new file mode 100644 index 0000000000..ff4c508ee6 --- /dev/null +++ b/apps/web/e2e/tests/task/mobile-office-sidebar-nest-task.spec.ts @@ -0,0 +1,89 @@ +import { test, expect } from "../../fixtures/office-fixture"; +import { SessionPage } from "../../pages/session-page"; + +function taskId(task: Record): string { + return task.id as string; +} + +test.describe("Mobile Office sidebar Nest under", () => { + // @covers AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.1 + // @covers AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.4 + // @covers AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.5 + test("nests an Office parent under another Office task", async ({ + testPage, + apiClient, + officeApi, + seedData, + }) => { + const project = await officeApi.createProject( + seedData.workspaceId, + "Mobile Office nesting project", + ); + const projectId = taskId(project); + const subject = await apiClient.createTask( + seedData.workspaceId, + "Mobile Office nesting subject", + { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + const descendant = await apiClient.createTask( + seedData.workspaceId, + "Mobile Office nesting descendant", + { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + parent_id: subject.id, + }, + ); + const target = await apiClient.createTask( + seedData.workspaceId, + "Mobile Office nesting target", + { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + await Promise.all([ + officeApi.updateTaskProjectId(subject.id, projectId), + officeApi.updateTaskProjectId(descendant.id, projectId), + officeApi.updateTaskProjectId(target.id, projectId), + ]); + await apiClient.saveUserSettings({ + workspace_id: seedData.workspaceId, + workflow_filter_id: seedData.workflowId, + keyboard_shortcuts: {}, + enable_preview_on_click: false, + sidebar_views: [], + }); + + await testPage.goto(`/t/${subject.id}`); + await new SessionPage(testPage).waitForLoad(); + await testPage.getByTestId("mobile-session-menu").tap(); + + const taskSheet = testPage.getByRole("dialog", { name: "Tasks" }); + const subjectRow = taskSheet + .getByTestId("sidebar-task-item") + .filter({ hasText: "Mobile Office nesting subject" }); + await expect(subjectRow).toBeVisible(); + await subjectRow.getByRole("button", { name: "Task actions" }).tap(); + await testPage.getByRole("menuitem", { name: "Nest under" }).tap(); + await expect( + testPage.getByRole("menuitem", { name: "Mobile Office nesting descendant" }), + ).toHaveCount(0); + await testPage.getByRole("menuitem", { name: "Mobile Office nesting target" }).tap(); + + const taskBlock = (id: string) => + taskSheet.locator(`[data-testid="sortable-task-block"][data-task-id="${id}"]`); + await expect(taskBlock(subject.id)).toHaveAttribute("data-depth", "1", { timeout: 10_000 }); + await expect(taskBlock(descendant.id)).toHaveAttribute("data-depth", "2"); + await expect + .poll(async () => (await apiClient.getTask(subject.id)).parent_id ?? "") + .toBe(target.id); + }); +}); diff --git a/apps/web/e2e/tests/task/office-sidebar-nest-task.spec.ts b/apps/web/e2e/tests/task/office-sidebar-nest-task.spec.ts new file mode 100644 index 0000000000..829523d004 --- /dev/null +++ b/apps/web/e2e/tests/task/office-sidebar-nest-task.spec.ts @@ -0,0 +1,96 @@ +import { test, expect } from "../../fixtures/office-fixture"; +import { SessionPage } from "../../pages/session-page"; + +function taskId(task: Record): string { + return task.id as string; +} + +test.describe("Office sidebar Nest under", () => { + // @covers AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.1 + // @covers AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.4 + test("nests an Office parent under another Office task", async ({ + testPage, + apiClient, + officeApi, + seedData, + }) => { + const project = await officeApi.createProject(seedData.workspaceId, "Office nesting project"); + const projectId = taskId(project); + const subject = await apiClient.createTask(seedData.workspaceId, "Office nesting subject", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + const subjectId = subject.id; + const descendant = await apiClient.createTask( + seedData.workspaceId, + "Office nesting descendant", + { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + parent_id: subjectId, + }, + ); + const descendantId = descendant.id; + const target = await apiClient.createTask(seedData.workspaceId, "Office nesting target", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + const targetId = target.id; + + await Promise.all([ + officeApi.updateTaskProjectId(subjectId, projectId), + officeApi.updateTaskProjectId(descendantId, projectId), + officeApi.updateTaskProjectId(targetId, projectId), + ]); + await apiClient.saveUserSettings({ + workspace_id: seedData.workspaceId, + workflow_filter_id: seedData.workflowId, + keyboard_shortcuts: {}, + enable_preview_on_click: false, + sidebar_views: [], + }); + + await testPage.goto(`/t/${subjectId}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await expect(session.sidebar).toBeVisible({ timeout: 10_000 }); + + const taskBlock = (id: string) => + session.sidebar.locator(`[data-testid="sortable-task-block"][data-task-id="${id}"]`); + await expect(taskBlock(subjectId)).toHaveAttribute("data-depth", "0"); + await expect(taskBlock(descendantId)).toHaveAttribute("data-depth", "1"); + await expect(taskBlock(targetId)).toHaveAttribute("data-depth", "0"); + + const subjectRow = session.sidebar + .locator("[data-testid='sidebar-task-item']") + .filter({ hasText: "Office nesting subject" }); + await subjectRow.hover(); + await subjectRow.getByRole("button", { name: "Task actions" }).click(); + await testPage.getByRole("menuitem", { name: "Nest under" }).hover(); + + await expect(testPage.getByRole("menuitem", { name: "Office nesting descendant" })).toHaveCount( + 0, + ); + await testPage.getByRole("menuitem", { name: "Office nesting target" }).click(); + + await expect(taskBlock(subjectId)).toHaveAttribute("data-depth", "1", { timeout: 10_000 }); + await expect(taskBlock(descendantId)).toHaveAttribute("data-depth", "2"); + await expect( + taskBlock(targetId).locator( + `[data-testid="sortable-task-block"][data-task-id="${subjectId}"]`, + ), + ).toHaveCount(1); + + await expect + .poll(async () => (await apiClient.getTask(subjectId)).parent_id ?? "") + .toBe(targetId); + + await testPage.reload(); + await session.waitForLoad(); + await expect(taskBlock(subjectId)).toHaveAttribute("data-depth", "1", { timeout: 10_000 }); + await expect(taskBlock(descendantId)).toHaveAttribute("data-depth", "2"); + }); +}); diff --git a/apps/web/e2e/tests/workflow/workflow-step-move-overrides.spec.ts b/apps/web/e2e/tests/workflow/workflow-step-move-overrides.spec.ts index 226eb66428..84baee9700 100644 --- a/apps/web/e2e/tests/workflow/workflow-step-move-overrides.spec.ts +++ b/apps/web/e2e/tests/workflow/workflow-step-move-overrides.spec.ts @@ -172,6 +172,7 @@ test("keeps the desktop next-step form open when clicking a checkbox label", asy const form = testPage.getByTestId("proceed-next-step-options"); await expect(form).toBeVisible(); + await form.hover(); await form.getByText("Skip the step prompt", { exact: true }).click(); await expect(form).toBeVisible(); diff --git a/apps/web/lib/kanban/map-task.test.ts b/apps/web/lib/kanban/map-task.test.ts index 58973ce421..dc2809d12d 100644 --- a/apps/web/lib/kanban/map-task.test.ts +++ b/apps/web/lib/kanban/map-task.test.ts @@ -67,6 +67,16 @@ function wsPayload(overrides: Partial = {}): TaskLike { } describe("toKanbanTask — HTTP DTO / WS payload parity", () => { + // @covers AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.4 + it("maps is_from_office to isFromOffice", () => { + const officeProjection: Partial = { is_from_office: true }; + const http = toKanbanTask(httpDTO(officeProjection)); + const ws = toKanbanTask(wsPayload(officeProjection)); + + expect(http.isFromOffice).toBe(true); + expect(ws.isFromOffice).toBe(true); + }); + it("carries workspace identity and archived state through both task shapes", () => { const archivedAt = "2026-08-04T10:00:00Z"; const http = toKanbanTask(httpDTO({ archived_at: archivedAt } as Partial)); diff --git a/apps/web/lib/kanban/map-task.ts b/apps/web/lib/kanban/map-task.ts index c71d6a2d4f..b0847ec97b 100644 --- a/apps/web/lib/kanban/map-task.ts +++ b/apps/web/lib/kanban/map-task.ts @@ -84,6 +84,7 @@ export type TaskLike = { primary_agent_profile_id?: string | null; labels?: string | string[] | null; is_remote_executor?: boolean; + is_from_office?: boolean; parent_id?: string | null; assignee_user_id?: string; updated_at?: string; @@ -305,6 +306,7 @@ export function toKanbanTask(source: TaskLike): KanbanTask { primaryAgentName: source.primary_agent_name ?? undefined, labels: pickLabels(source), assigneeUserId: pickAssignee(source.assignee_user_id), + isFromOffice: source.is_from_office, parentTaskId: source.parent_id ?? undefined, workspaceMode: workspaceModeFromMetadata(source.metadata), updatedAt: source.updated_at, diff --git a/apps/web/lib/sidebar/nest-candidates.test.ts b/apps/web/lib/sidebar/nest-candidates.test.ts index 6bb3e43ec7..c5638e7610 100644 --- a/apps/web/lib/sidebar/nest-candidates.test.ts +++ b/apps/web/lib/sidebar/nest-candidates.test.ts @@ -1,7 +1,12 @@ import { describe, expect, it } from "vitest"; import { computeNestCandidates } from "./nest-candidates"; -type T = { id: string; title: string; parentTaskId?: string | null }; +type T = { + id: string; + title: string; + parentTaskId?: string | null; + isFromOffice?: boolean; +}; const tasks: T[] = [ { id: "a", title: "A" }, // root with a child @@ -9,6 +14,8 @@ const tasks: T[] = [ { id: "d", title: "D" }, // root, leaf { id: "e", title: "E" }, // root, leaf ]; +const OFFICE_ROOT = "office-root"; +const OFFICE_CHILD = "office-child"; describe("computeNestCandidates", () => { it("excludes the task itself", () => { @@ -52,4 +59,94 @@ describe("computeNestCandidates", () => { it("returns empty when the task is the only one", () => { expect(computeNestCandidates([{ id: "solo", title: "Solo" }], "solo")).toEqual([]); }); + + it("returns empty when the subject is missing", () => { + expect(computeNestCandidates(tasks, "missing")).toEqual([]); + }); +}); + +describe("computeNestCandidates for Office hierarchies", () => { + // @covers AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.4 + it("allows deep Office targets and excludes descendants", () => { + const officeTasks: T[] = [ + { id: OFFICE_ROOT, title: "Office root", isFromOffice: true }, + { + id: OFFICE_CHILD, + title: "Office child", + parentTaskId: OFFICE_ROOT, + isFromOffice: true, + }, + { + id: "office-grandchild", + title: "Office grandchild", + parentTaskId: OFFICE_CHILD, + isFromOffice: true, + }, + { id: "office-target-root", title: "Target root", isFromOffice: true }, + { + id: "office-target-child", + title: "Target child", + parentTaskId: "office-target-root", + isFromOffice: true, + }, + ]; + + expect(computeNestCandidates(officeTasks, "office-root").map((task) => task.id)).toEqual([ + "office-target-root", + "office-target-child", + ]); + }); + + it("uses the complete hierarchy to exclude descendants hidden by the current view", () => { + const hierarchy: T[] = [ + { id: "subject", title: "Subject", isFromOffice: true }, + { + id: "hidden-child", + title: "Hidden child", + parentTaskId: "subject", + isFromOffice: true, + }, + { + id: "visible-grandchild", + title: "Visible grandchild", + parentTaskId: "hidden-child", + isFromOffice: true, + }, + { id: "valid-target", title: "Valid target", isFromOffice: true }, + ]; + const visibleTasks = hierarchy.filter((task) => task.id !== "hidden-child"); + + expect( + computeNestCandidates(visibleTasks, "subject", hierarchy).map((task) => task.id), + ).toEqual(["valid-target"]); + }); + + it("allows an Office subject to nest under a non-Office subtask", () => { + const mixedTasks: T[] = [ + { id: "office-subject", title: "Office subject", isFromOffice: true }, + { id: "kanban-root", title: "Kanban root" }, + { id: "kanban-child", title: "Kanban child", parentTaskId: "kanban-root" }, + ]; + + expect(computeNestCandidates(mixedTasks, "office-subject").map((task) => task.id)).toContain( + "kanban-child", + ); + }); + + it("allows a non-Office subject to nest under an Office subtask", () => { + const mixedTasks: T[] = [ + { id: "kanban-subject", title: "Kanban subject" }, + { id: OFFICE_ROOT, title: "Office root", isFromOffice: true }, + { + id: OFFICE_CHILD, + title: "Office child", + parentTaskId: OFFICE_ROOT, + isFromOffice: true, + }, + ]; + + expect(computeNestCandidates(mixedTasks, "kanban-subject").map((task) => task.id)).toContain( + OFFICE_CHILD, + ); + }); }); diff --git a/apps/web/lib/sidebar/nest-candidates.ts b/apps/web/lib/sidebar/nest-candidates.ts index d470d1ddec..2a640397ba 100644 --- a/apps/web/lib/sidebar/nest-candidates.ts +++ b/apps/web/lib/sidebar/nest-candidates.ts @@ -6,29 +6,54 @@ export type NestCandidate = { id: string; title: string; parentTaskId?: string | null; + isFromOffice?: boolean; }; /** * computeNestCandidates returns the tasks that `taskId` may be nested under, - * enforcing the one-level kanban subtask limit. + * enforcing the one-level Kanban limit while allowing arbitrary-depth Office + * hierarchies. * - * When the task itself already has children, nesting it under any parent would - * push those children to depth 2, so no candidate is offered. Otherwise a - * candidate is any task that is NOT: - * - the task itself, - * - the task's current parent (selecting it would be a no-op), or - * - already a subtask — nesting under a subtask would create a grandchild - * (this also structurally excludes the task's own descendants, so no cycle - * can be introduced). + * Every hierarchy excludes the task, its current parent, and descendants. + * Kanban additionally requires a childless subject and root candidate. Office + * uses the backend's endpoint rule: either endpoint being Office lifts those + * two depth restrictions. * * Order is preserved from the input list. */ -export function computeNestCandidates(tasks: T[], taskId: string): T[] { - const task = tasks.find((t) => t.id === taskId); +export function computeNestCandidates( + tasks: T[], + taskId: string, + hierarchyTasks: readonly NestCandidate[] = tasks, +): T[] { + const tasksById = new Map(hierarchyTasks.map((candidate) => [candidate.id, candidate])); + const task = tasksById.get(taskId) ?? tasks.find((candidate) => candidate.id === taskId); + if (!task) return []; - // A task with children cannot be nested without exceeding the one-level limit. - if (tasks.some((t) => t.parentTaskId === taskId)) return []; + const taskHasChildren = hierarchyTasks.some((candidate) => candidate.parentTaskId === taskId); + const currentParent = task.parentTaskId ?? undefined; - const currentParent = task?.parentTaskId ?? undefined; - return tasks.filter((t) => t.id !== taskId && t.id !== currentParent && !t.parentTaskId); + return tasks.filter((candidate) => { + const hierarchyCandidate = tasksById.get(candidate.id) ?? candidate; + if (hierarchyCandidate.id === taskId || hierarchyCandidate.id === currentParent) return false; + if (hasAncestor(hierarchyCandidate, taskId, tasksById)) return false; + if (task.isFromOffice || hierarchyCandidate.isFromOffice) return true; + return !taskHasChildren && !hierarchyCandidate.parentTaskId; + }); +} + +function hasAncestor( + candidate: NestCandidate, + ancestorId: string, + tasksById: ReadonlyMap, +): boolean { + const visited = new Set(); + let parentId = candidate.parentTaskId; + while (parentId) { + if (parentId === ancestorId) return true; + if (visited.has(parentId)) return true; + visited.add(parentId); + parentId = tasksById.get(parentId)?.parentTaskId; + } + return false; } diff --git a/apps/web/lib/state/slices/kanban/types.ts b/apps/web/lib/state/slices/kanban/types.ts index 4156e71b99..b0c5beb88a 100644 --- a/apps/web/lib/state/slices/kanban/types.ts +++ b/apps/web/lib/state/slices/kanban/types.ts @@ -164,6 +164,8 @@ export type KanbanState = { primaryAgentName?: string | null; labels?: string[]; isRemoteExecutor?: boolean; + /** Backend-owned discriminator for task hierarchy rules. Absent means non-Office. */ + isFromOffice?: boolean; /** Human assignee (user id). Independent of any agent assignment. */ assigneeUserId?: string; parentTaskId?: string | null; diff --git a/apps/web/lib/ws/handlers/task-merge.ts b/apps/web/lib/ws/handlers/task-merge.ts index 155a80e98e..3a8b85ce05 100644 --- a/apps/web/lib/ws/handlers/task-merge.ts +++ b/apps/web/lib/ws/handlers/task-merge.ts @@ -134,6 +134,10 @@ export function mergeTaskUpdate( if (!hasPayloadField(payload, "metadata")) merged.metadata = existing.metadata; if (!hasPayloadField(payload, "labels")) merged.labels = existing.labels; if (!hasPayloadField(payload, "origin")) merged.origin = existing.origin; + preserveOmittedField(existing, merged, payload, nextTask, { + payloadKey: "is_from_office", + taskField: "isFromOffice", + }); preserveOmittedField(existing, merged, payload, nextTask, { payloadKey: "task_pending_action", taskField: "taskPendingAction", diff --git a/apps/web/lib/ws/handlers/tasks-office-identity.test.ts b/apps/web/lib/ws/handlers/tasks-office-identity.test.ts new file mode 100644 index 0000000000..afdca34607 --- /dev/null +++ b/apps/web/lib/ws/handlers/tasks-office-identity.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it } from "vitest"; +import { registerTasksHandlers } from "./tasks"; +import { makeMessage, makeStore } from "./tasks.test-helpers"; + +const TASK_ID = "office-task"; +const WORKFLOW_ID = "office-workflow"; +const STEP_ID = "office-step"; + +function cachedOfficeTask() { + return { + id: TASK_ID, + title: "Office task", + workflowId: WORKFLOW_ID, + workflowStepId: STEP_ID, + position: 0, + isFromOffice: true, + }; +} + +function storeWithCachedOfficeTask() { + return makeStore({ + kanban: { + workflowId: WORKFLOW_ID, + steps: [], + tasks: [cachedOfficeTask()], + }, + kanbanMulti: { + isLoading: false, + snapshots: { + [WORKFLOW_ID]: { + workflowId: WORKFLOW_ID, + workflowName: "Office", + steps: [], + tasks: [cachedOfficeTask()], + }, + }, + }, + } as never); +} + +describe("task.updated Office identity preservation", () => { + // @covers AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.4 + it("preserves Office identity when a lightweight update omits it", () => { + const store = storeWithCachedOfficeTask(); + + registerTasksHandlers(store)["task.updated"]!( + makeMessage({ + task_id: TASK_ID, + workflow_id: WORKFLOW_ID, + workflow_step_id: STEP_ID, + title: "Renamed Office task", + state: "IN_PROGRESS", + }), + ); + + expect(store.getState().kanban.tasks[0]?.isFromOffice).toBe(true); + expect(store.getState().kanbanMulti.snapshots[WORKFLOW_ID]?.tasks[0]?.isFromOffice).toBe(true); + }); + + it("clears Office identity when task.updated explicitly sets it to false", () => { + const store = storeWithCachedOfficeTask(); + + registerTasksHandlers(store)["task.updated"]!( + makeMessage({ + task_id: TASK_ID, + workflow_id: WORKFLOW_ID, + workflow_step_id: STEP_ID, + title: "Project removed", + state: "IN_PROGRESS", + is_from_office: false, + }), + ); + + expect(store.getState().kanban.tasks[0]?.isFromOffice).toBe(false); + expect(store.getState().kanbanMulti.snapshots[WORKFLOW_ID]?.tasks[0]?.isFromOffice).toBe(false); + }); +}); diff --git a/docs/plans/fix-nest-under-candidates/plan.md b/docs/plans/fix-nest-under-candidates/plan.md new file mode 100644 index 0000000000..aa4f98f6f2 --- /dev/null +++ b/docs/plans/fix-nest-under-candidates/plan.md @@ -0,0 +1,164 @@ +--- +created: 2026-09-16 +status: implemented +requirements: + - REQ-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001 +system_design: + - ../../specs/tasks/system-design/subtask-reparenting-drag-drop.md +legacy_specs: [] +--- + +# Implementation Plan: Fix Nest under candidates + +## Overview + +The sidebar can incorrectly disable `Nest under` for two independent reasons. Its context menu +prefers any present multi-workflow snapshot even when that snapshot is a partial placeholder, and +the shared eligibility helper applies the one-level Kanban rule to Office tasks despite the +backend's explicit Office depth exemption. The correction preserves Office identity through the +frontend projection, uses the rendered task group for menu and drag candidates, and makes their +shared pure filter match the backend depth contract. Review validation also requires the complete +unfiltered hierarchy for cycle checks, stable candidate access that preserves row memoization, and +an explicit Office discriminator on canonical task updates after project reassignment. + +No endpoint, persistence schema, localization, or layout change is required. + +The smallest reproductions are focused frontend tests. An Office subject with a child and an +eligible Office root produces an empty candidate array, while a context menu rendered beside an +eligible task cannot find that task when the raw multi-workflow snapshot contains only the subject. +Before the correction, the focused command reports two failures and 28 passes: + +```bash +(cd apps/web && pnpm test -- lib/sidebar/nest-candidates.test.ts components/task/task-switcher-context-menu.test.tsx) +``` + +The permanent regressions are `allows deep Office targets and excludes descendants` in +`lib/sidebar/nest-candidates.test.ts` and `uses rendered candidates when the workflow snapshot is +partial` in `components/task/task-switcher-nest-context-menu.test.tsx`. + +## Scope + +### In scope + +- Preserve the existing backend Office discriminator through the sidebar task projection. +- Publish the canonical task update after Office project reassignment and serialize its Office + discriminator explicitly in both directions. +- Make menu and drag candidates share the rendered task group and eligibility helper. +- Validate descendants and child depth against the complete pre-filter hierarchy without adding + hidden tasks to the candidate list. +- Preserve memoized row stability while menus and drags read the latest committed candidate data. +- Match the backend's existing Office depth exemption and retain Kanban's one-level rule. +- Prove the correction with focused tests and an Office sidebar browser flow. + +### Out of scope + +- Backend validation, API shape, persistence, and workspace materialization. +- Kanban card drag, sibling reorder, and the separate Office list parent picker. +- New controls, menu copy, layout, touch gestures, or navigation. + +## Technical approach + +1. Preserve the DTO's optional `is_from_office` value through `toKanbanTask`, the Kanban snapshot + type, desktop and phone sidebar projections, and `TaskSwitcherItem` as `isFromOffice`. Preserve + the cached value when a lightweight `task.updated` event omits the field and when a partial + active-board task is reconciled with a full multi-workflow projection. +2. Extend `computeNestCandidates` with explicit descendant detection against a separate complete + hierarchy. Preserve the current Kanban root-only and childless-subject rules, while allowing + arbitrary-depth Office candidates when either endpoint is Office. +3. Pass the flattened rendered `groupTasks` and complete pre-filter hierarchy through stable getters + in the task-tree composition. Remove the menu's direct raw-snapshot selection. Keep drag target + discovery on the same collections and helper without rerendering unaffected rows. +4. Add `is_from_office` explicitly to canonical task events and publish one after Office project + assignment or removal so live caches observe both transitions. +5. Add focused projection, event, helper, menu, drag, and render-stability tests. Add an Office E2E + that reproduces the disabled submenu with a parent task and proves successful deeper nesting. + +## ASCII UI preview + +### UI-01: `Nest under` submenu with a visible eligible Office target + +Current behavior established by the focused component reproduction: + +```text +Nest under > +--------------------+ + | No other tasks | disabled + +--------------------+ +``` + +Required behavior: + +```text +Nest under > +--------------------+ + | Office target | selectable + +--------------------+ +``` + +For a Kanban subject that already has a child, the disabled `No other tasks` state remains correct. +Desktop retains the anchored submenu shown above. On phone, the existing inset menu sheet renders +the same candidate row and retains its existing touch target, scroll, dismissal, and safe-area +behavior. Spacing is illustrative; candidate presence and eligibility are required by +AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.1 through .5. + +## Tests + +- AC `AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.1` and + `AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.2`: + `task-switcher-nest-context-menu.test.tsx`, `uses rendered candidates when the workflow snapshot +is partial`; `task-switcher-subtask-dnd.test.ts`, `matches menu candidates for Office and Kanban +tasks`. +- AC `AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.3`: `nest-candidates.test.ts`, `keeps the +one-level Kanban limit`. +- AC `AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.4`: `map-task.test.ts`, `maps is_from_office to +isFromOffice`; + `tasks-office-identity.test.ts`, `preserves Office identity when task.updated omits it`; + `task-session-sidebar-aggregate.test.ts`, `preserves projected Office identity through a newer +partial active task`; + `task-session-sidebar-item.test.ts`, `preserves isFromOffice`; + `session-task-switcher-sheet-item.test.ts`, `carries Office identity into the phone task drawer +row`; + `nest-candidates.test.ts`, `allows deep Office targets and excludes descendants`. +- Filtered hierarchy and stable composition: `nest-candidates.test.ts`, `uses the complete hierarchy +to exclude descendants hidden by the current view`; `task-switcher-render-stability.test.tsx`, + `gives a skipped row the latest nest candidates`. +- Live Office identity: backend dashboard and task-service event tests cover canonical publication + for project assignment/removal and explicit true/false serialization; the frontend handler test + covers clearing the cached identity. +- AC `AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.5`: the shared component tests render the + existing menu and drag paths without separate desktop and phone candidate logic. + +## E2E tests + +- AC `AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.1` through + `AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.4`: add `office-sidebar-nest-task.spec.ts`, Chromium + scenario `nests an Office parent under another Office task`, asserting the target row, persisted + parent, and deeper rendered hierarchy. +- AC `AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.3`: rerun `sidebar-nest-task.spec.ts` in + Chromium as the shallow Kanban regression. +- AC `AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.5`: rerun + `mobile-subtask-reparent-drag-drop.spec.ts` and add `mobile-office-sidebar-nest-task.spec.ts` in + `mobile-chrome` to preserve the shared touch path and prove the Office candidate reaches the + phone task sheet. + +## Work orders + +- [x] [Task 01: Correct Nest under candidates](task-01-correct-nest-under-candidates.md) + +One sequential work order owns the projection, shared algorithm, both interaction paths, and browser +proof because each layer is required for one observable result. A wave does not authorize +delegation. + +## Verification results + +Implemented in one work order. The focused Vitest suite passes 107 tests across nine files, and the +full frontend suite passes 18,454 tests with four skipped. Typecheck, full frontend lint, the i18n +ratchet, and the complete Office dashboard and task service Go package tests pass. Chromium passes +the existing Kanban nesting flow and the new Office depth-two nesting and reload flow. The +`mobile-chrome` project passes the existing touch re-parenting cases and the new Office phone-sheet +flow. Specification validation and the scoped diff check also pass. + +## Risks + +- The backend remains the final validator for races and hierarchy data absent from the complete + client projection; rejected mutations must retain the existing rollback and error path. +- Reading a raw snapshot in either consumer would recreate the menu/drag divergence. +- Missing Office metadata must default to non-Office so the correction cannot relax Kanban rules. diff --git a/docs/plans/fix-nest-under-candidates/task-01-correct-nest-under-candidates.md b/docs/plans/fix-nest-under-candidates/task-01-correct-nest-under-candidates.md new file mode 100644 index 0000000000..d3cdf85028 --- /dev/null +++ b/docs/plans/fix-nest-under-candidates/task-01-correct-nest-under-candidates.md @@ -0,0 +1,171 @@ +--- +id: "01-correct-nest-under-candidates" +title: "Correct Nest under candidates" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001 +acceptance_criteria: + - AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.1 + - AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.2 + - AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.3 + - AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.4 + - AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.5 +system_design: + - ../../specs/tasks/system-design/subtask-reparenting-drag-drop.md +--- + +# Task 01: Correct Nest under candidates + +## Summary + +The sidebar context menu and drag interaction offer the same eligible tasks from the rendered group, +including arbitrary-depth Office targets, while Kanban keeps its one-level hierarchy. + +## In scope + +- Preserve the backend Office discriminator through HTTP mapping, lightweight WebSocket merges, + active/snapshot reconciliation, and the sidebar projection. +- Make canonical task updates serialize Office identity explicitly and publish after project + assignment or removal. +- Make `computeNestCandidates` distinguish Office from Kanban and reject descendants through the + complete pre-filter hierarchy. +- Supply the rendered group and complete hierarchy through stable getters, preserving row + memoization, and remove the context menu's independent raw-snapshot lookup. +- Keep drag target discovery on the same group and helper. +- Add focused unit, component, and Office E2E regression coverage. + +## Out of scope + +- Backend validation, task PATCH semantics, and persistence. +- Kanban card drag, sibling reorder, Office list parent picker, menu copy, and menu geometry. +- Lifting the one-level Kanban hierarchy limit. + +## Acceptance + +- A visible eligible task remains selectable from `Nest under` when the corresponding raw + multi-workflow snapshot is partial, and drag exposes exactly the same candidate identifiers. +- Office subjects with children and Office targets at any depth are eligible except for self, + current parent, and descendants, including a descendant behind a filtered intermediate ancestor; + the existing Kanban root-only and childless-subject behavior remains unchanged. +- Unaffected rows retain their memoization while a subsequently opened menu sees current candidates, + and project assignment/removal updates cached Office identity without a reload. +- The focused frontend suites, Office browser scenario, typecheck, lint, i18n ratchet, and spec + validation pass. + +## ASCII UI preview + +### UI-01: `Nest under` submenu with a visible eligible Office target + +See the combined preview in +[plan.md](plan.md#ui-01-nest-under-submenu-with-a-visible-eligible-office-target). + +```text +Before After ++--------------------+ +--------------------+ +| No other tasks | | Office target | +| disabled | | selectable | ++--------------------+ +--------------------+ +``` + +Desktop keeps its anchored submenu. Phone uses the same candidate row in the existing inset menu +sheet. No spacing, control, or navigation change is part of this work order. + +## Verification + +```bash +(cd apps && pnpm install --frozen-lockfile) +(cd apps/web && pnpm test -- lib/kanban/map-task.test.ts lib/ws/handlers/tasks-office-identity.test.ts lib/sidebar/nest-candidates.test.ts components/task/task-session-sidebar-aggregate.test.ts components/task/task-session-sidebar-item.test.ts components/task/mobile/session-task-switcher-sheet-item.test.ts components/task/task-switcher-subtask-dnd.test.ts components/task/task-switcher-context-menu.test.tsx components/task/task-switcher-nest-context-menu.test.tsx) +(cd apps/web && pnpm run typecheck) +(cd apps/web && pnpm run lint) +(cd apps/web && pnpm run i18n:ratchet) +(cd apps/backend && go test ./internal/office/dashboard ./internal/task/service) +(cd apps/web && pnpm e2e:run tests/task/sidebar-nest-task.spec.ts tests/task/office-sidebar-nest-task.spec.ts) +(cd apps/web && pnpm e2e:run --project mobile-chrome tests/task/mobile-subtask-reparent-drag-drop.spec.ts tests/task/mobile-office-sidebar-nest-task.spec.ts) +python3 scripts/list-docs.py validate +python3 scripts/lint-spec-files.py --all +git diff --check -- docs/specs/tasks docs/plans/fix-nest-under-candidates apps/web +``` + +Run the complete block from the repository root. + +## Files likely touched + +- `apps/web/lib/kanban/map-task.ts` +- `apps/web/lib/kanban/map-task.test.ts` +- `apps/web/lib/state/slices/kanban/types.ts` +- `apps/web/lib/ws/handlers/task-merge.ts` +- `apps/web/lib/ws/handlers/tasks-office-identity.test.ts` +- `apps/web/lib/sidebar/nest-candidates.ts` +- `apps/web/lib/sidebar/nest-candidates.test.ts` +- `apps/web/components/task/task-switcher-types.ts` +- `apps/web/components/task/task-session-sidebar-aggregate.ts` +- `apps/web/components/task/task-session-sidebar-aggregate.test.ts` +- `apps/web/components/task/task-session-sidebar-item.ts` +- `apps/web/components/task/task-session-sidebar-item.test.ts` +- `apps/web/components/task/mobile/session-task-switcher-sheet-item.ts` +- `apps/web/components/task/mobile/session-task-switcher-sheet-item.test.ts` +- `apps/web/components/task/task-switcher-tree.tsx` +- `apps/web/components/task/task-switcher.tsx` +- `apps/web/components/task/task-switcher-render-stability.test.tsx` +- `apps/web/components/task/task-session-sidebar-switcher-props.ts` +- `apps/web/components/task/mobile/session-task-switcher-sheet-props.ts` +- `apps/web/components/task/task-switcher-row.tsx` +- `apps/web/components/task/task-switcher-context-menu.tsx` +- `apps/web/components/task/task-switcher-context-menu-items.tsx` +- `apps/web/components/task/task-nest-context-menu.tsx` +- `apps/web/components/task/task-switcher-context-menu.test.tsx` +- `apps/web/components/task/task-switcher-nest-context-menu.test.tsx` +- `apps/web/components/task/task-switcher-subtask-dnd.tsx` +- `apps/web/components/task/task-switcher-subtask-dnd.test.ts` +- `apps/web/e2e/tests/task/office-sidebar-nest-task.spec.ts` +- `apps/web/e2e/tests/task/mobile-office-sidebar-nest-task.spec.ts` +- `apps/backend/internal/office/dashboard/service_tasks.go` +- `apps/backend/internal/office/dashboard/task_updated_events_test.go` +- `apps/backend/internal/task/service/service_events.go` +- `apps/backend/internal/task/service/service_events_test.go` + +Use fewer files when the rendered-group value can be threaded through an existing context or prop. +Do not introduce a general task registry for this local composition. + +## Dependencies + +None. + +## Parallelism + +`sequential` + +## Inputs + +- Requirement acceptance criteria and scenarios in + `docs/specs/tasks/requirements/subtask-reparenting-drag-drop.md`. +- Candidate-source and eligibility contracts in + `docs/specs/tasks/system-design/subtask-reparenting-drag-drop.md`. +- Existing canonical mutation path in `apps/web/hooks/use-nest-task.ts` and backend validation in + `apps/backend/internal/task/service/service_tasks.go`. + +## Risks + +- A cycle filter based only on visible tasks would expose deeper descendants when a view hides an + intermediate ancestor; follow ancestor chains in the complete hierarchy with a visited set. +- Defaulting a missing Office discriminator to true would relax Kanban rules; absent values remain + non-Office. +- A menu-local store selector would let menu and drag diverge again; both consume the rendered + group supplied by their common tree composition. +- Passing changing candidate arrays directly to every row would defeat memoization; stable getters + must resolve the latest committed arrays at interaction time. + +## Results + +Implemented the shared rendered-group candidate path, Office-aware hierarchy filtering, Office +identity projection and partial-update preservation, and descendant-cycle prevention. The new +browser scenario proves an Office parent with a child can be nested under another Office task, +persists the relationship, renders depth two, and survives reload. + +Verification passed: frozen install; 107 focused Vitest tests across nine files; the full frontend +suite with 18,454 passing tests and four skipped; TypeScript typecheck; full frontend lint; i18n +ratchet; complete Office dashboard and task service Go package tests; desktop Chromium and +`mobile-chrome` nesting flows; both specification validators; and the scoped diff check. diff --git a/docs/specs/tasks/README.md b/docs/specs/tasks/README.md index a547b05308..3102237fc7 100644 --- a/docs/specs/tasks/README.md +++ b/docs/specs/tasks/README.md @@ -171,6 +171,7 @@ signals, and task-scoped scheduling contracts. - [Prompt attachments](system-design/prompt-attachments.md) - [Task Actions Menu on Preview and Detail Surfaces](system-design/task-actions-menu.md) - [Task menu grouping](system-design/task-menu-grouping.md) +- [Subtask re-parenting by drag and drop](system-design/subtask-reparenting-drag-drop.md) - [Task Archive Confirmation](system-design/archive-confirmation.md) - [Task removal navigation](system-design/removal-navigation.md) - [Task plan content size limit](system-design/plan-content-size-limit.md) diff --git a/docs/specs/tasks/requirements/subtask-reparenting-drag-drop.md b/docs/specs/tasks/requirements/subtask-reparenting-drag-drop.md index bf5d950d93..733ee8cde4 100644 --- a/docs/specs/tasks/requirements/subtask-reparenting-drag-drop.md +++ b/docs/specs/tasks/requirements/subtask-reparenting-drag-drop.md @@ -5,6 +5,7 @@ created: 2026-08-04 owners: - kandev --- + # Subtask re-parenting by drag and drop Requirements ## Overview @@ -15,11 +16,30 @@ This document is the migrated task-system source for the capability. The source ### REQ-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001: Subtask re-parenting by drag and drop -**Intent:** Preserve the observable task or workflow behavior recorded by the legacy specification. +**Intent:** Let users re-parent tasks from the sidebar menu or drag gesture while presenting every +eligible target that is already visible in the task group. #### Acceptance criteria -- **AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.1:** When a consumer uses this capability, the system shall provide the observable behavior and exclusions documented below. +- **AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.1:** The sidebar `Nest under` submenu and the + drag nest zones shall derive their targets from the same rendered task group and the same + eligibility rules. +- **AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.2:** When an eligible same-workflow target is + visible in the rendered task group, the target shall remain available while workflow snapshots + are loading, refreshing, or temporarily incomplete; the submenu shall show `No other tasks` only + when that rendered group has no eligible target. +- **AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.3:** For non-Office tasks, an eligible target shall + be a root task, and a task that already has children shall have no nest target, preserving the + one-level Kanban hierarchy. +- **AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.4:** When the subject or target is an Office task, + the UI shall allow arbitrary-depth re-parenting while excluding the subject, its current parent, + and its descendants, including descendants whose intermediate ancestors are hidden by the active + sidebar view. The backend remains authoritative for self, cycle, archive, workspace, and + concurrency validation. Live project assignment and removal shall update the cached Office + identity in both directions. +- **AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.5:** Desktop sidebar and mobile task-switcher + presentations shall use the same menu and drag eligibility without changing their existing + pointer, touch, focus, dismissal, or scrolling behavior. ## Migrated source detail @@ -31,7 +51,13 @@ Users can detach a subtask or nest a task under another via context-menu actions - The sidebar task tree (desktop sidebar and the mobile task switcher sheet) lets a user re-parent a task by dragging its row onto another row's **nest drop zone**. - The result is strictly equivalent to choosing `Un-nest (remove parent)` then `Nest under ` from the task's context menu: the task's parent becomes the target, and a task whose workspace mode is `inherit_parent` ends with mode `shared_group` — its materialized workspace and workspace-group membership are unchanged. -- Drop targets are exactly the candidates the context menu offers (the `computeNestCandidates` rules): same-workflow root tasks in the visible group, excluding the dragged task, its current parent, and any subtask (which also excludes the task's own descendants). A task that already has children offers no nest targets. +- Drop targets are exactly the candidates the context menu offers (the `computeNestCandidates` + rules) from the same-workflow tasks in the rendered group. The complete unfiltered task + hierarchy is used only to validate ancestry and depth, so filtering an intermediate ancestor + cannot expose a descendant as a target. Every mode excludes the subject and its current parent. + Kanban candidates must be roots and a Kanban subject with children has no targets. Office + candidates may be at any depth, and an Office subject may keep its descendants, but its + descendants are excluded as targets so the UI cannot offer a known cycle. - While a drag with valid targets is active, candidate rows show a nest drop zone (a left-edge strip) with a `Nest under ` affordance. Dropping on a zone re-parents; dropping between rows keeps the existing sibling-reorder behavior; any other drop is a no-op. - Re-parenting is a single API call on the existing canonical path (`PATCH /api/v1/tasks/:id` with `parent_id`), which already rejects self-parenting, missing/archived/cross-workspace targets, descendant cycles, and one-level-depth violations for kanban tasks. - The sidebar `Nest under` menu, the Office parent picker, the WS task-update path, and the Office dashboard PATCH all share the same composite semantics: any effective parent change normalizes an `inherit_parent` workspace mode to `shared_group`. @@ -56,11 +82,18 @@ No new endpoint. Reuses and extends existing contracts: - `PATCH /api/v1/tasks/:id` with `parent_id` (non-empty nests; `""` un-nests) — already validated by `Service.resolveParentID` (self, existence, archived, same workspace, descendant cycle) and `validateReparentDepth` (one-level kanban limit, Office trees exempt). **Behavior addition:** when the effective parent changes, `inherit_parent` workspace mode is normalized to `shared_group` (mirroring the detach operation). Success returns the updated task DTO; invalid targets map to `400`; missing task to `404`. - `PATCH /api/v1/office/tasks/:id` with non-empty `parent_id` — same normalization added for parity; empty parent continues to route through the canonical detach operation. -- WS `task.updated` payload unchanged: `parent_id` is always present (nil when cleared), and `metadata` carries the normalized workspace mode. +- WS `task.updated` includes an explicit `is_from_office` boolean so either Office-identity + transition clears or establishes the cached value. `parent_id` is always present (nil when + cleared), and `metadata` carries the normalized workspace mode. Office project reassignment + publishes this canonical event after the write. ## Failure modes -- **Invalid target** (self, descendant, subtask, archived, missing, cross-workspace, or depth violation): the UI filters these targets out before a drop can land, so a drop outside every nest zone is a no-op. If a request is nevertheless rejected by the backend (e.g. a valid-zone target became invalid between render and drop), the UI keeps the task in its original tree position, rolls back the optimistic update, and shows a request-error toast. +- **Invalid target** (self, descendant, archived, missing, cross-workspace, or a Kanban depth + violation): the UI filters known-invalid targets before a drop can land, so a drop outside every + nest zone is a no-op. If a request is nevertheless rejected by the backend (for example, a + valid-zone target became invalid between render and drop), the UI keeps the task in its original + tree position, rolls back the optimistic update, and shows a request-error toast. - **Persistence failure**: no successful response is returned; the UI rolls back to the original tree. - **Concurrent submissions** are safe: setting the same parent twice is idempotent, and the optimistic update is reconciled by the authoritative `task.updated` event. - **No valid targets**: the drag offers no nest zones; only reorder remains possible. @@ -70,8 +103,22 @@ No new endpoint. Reuses and extends existing contracts: - **GIVEN** a subtask `C` under parent `A` and a root task `B` in the same workflow, **WHEN** the user drags `C` onto `B`'s nest drop zone, **THEN** `C`'s parent becomes `B`, the sidebar shows `C` nested under `B`, and no reload occurs. - **GIVEN** an `inherit_parent` subtask `C` under `A`, **WHEN** `C` is dragged onto root `B`'s nest zone, **THEN** `C`'s persisted workspace mode is `shared_group` and its workspace-group membership is unchanged. - **GIVEN** a root task with no children, **WHEN** it is dragged onto another root's nest zone, **THEN** it becomes that root's subtask. -- **GIVEN** a task that already has children, **WHEN** it is dragged, **THEN** no row offers a nest drop zone (reorder only). -- **GIVEN** a drag over a subtask row or over a task in a different workflow, **WHEN** the pointer rests on the row, **THEN** no nest drop zone is offered. +- **GIVEN** a Kanban task that already has children, **WHEN** it is dragged, **THEN** no row offers a nest drop zone (reorder only). +- **GIVEN** a Kanban drag over a subtask row or a drag over a task in a different workflow, **WHEN** the pointer rests on the row, **THEN** no nest drop zone is offered. +- **GIVEN** an Office task that already has a child and another eligible Office task in the same + rendered group, **WHEN** the user opens `Nest under` or starts a re-parent drag, **THEN** the + other task is offered and the resulting Office tree may be deeper than one level. +- **GIVEN** an eligible target is visible while the stored multi-workflow snapshot is temporarily + incomplete, **WHEN** the user opens `Nest under`, **THEN** the visible target is offered instead + of a disabled `No other tasks` row. +- **GIVEN** an Office task and one of its descendants, **WHEN** the menu or drag targets are shown, + **THEN** that descendant is not offered as a parent. +- **GIVEN** a sidebar filter hides the intermediate task between an Office subject and a visible + descendant, **WHEN** the menu or drag targets are shown, **THEN** the visible descendant is still + excluded as a parent. +- **GIVEN** a Kanban task is assigned to or removed from an Office project, **WHEN** the canonical + `task.updated` event arrives, **THEN** open sidebar caches immediately adopt the explicit new + Office identity without a reload. - **GIVEN** a subtask dragged toward its current parent, **WHEN** the pointer rests on that parent row, **THEN** no nest drop zone is offered. - **GIVEN** a drag dropped between two sibling rows, **WHEN** the drop lands outside every nest zone, **THEN** the siblings reorder as before. - **GIVEN** a drop that lands outside every nest zone, **WHEN** the drop completes, **THEN** the task keeps its original parent and no request is sent (a plain no-op); a request-error toast appears only when a valid-zone drop's request is rejected by the backend. @@ -91,4 +138,5 @@ No new endpoint. Reuses and extends existing contracts: ## Implementation plan -See [the implementation plan](../../../plans/subtask-reparenting-drag-drop/plan.md). \ No newline at end of file +See the original [drag-and-drop implementation plan](../../../plans/subtask-reparenting-drag-drop/plan.md) +and the [Nest under candidate correction plan](../../../plans/fix-nest-under-candidates/plan.md). diff --git a/docs/specs/tasks/system-design/subtask-reparenting-drag-drop.md b/docs/specs/tasks/system-design/subtask-reparenting-drag-drop.md new file mode 100644 index 0000000000..002cd36743 --- /dev/null +++ b/docs/specs/tasks/system-design/subtask-reparenting-drag-drop.md @@ -0,0 +1,115 @@ +--- +status: current +system: tasks +requirements: + - REQ-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001 +--- + +# Subtask re-parenting by drag and drop system design + +## Purpose and boundaries + +This design owns sidebar candidate discovery for `Nest under` and drag-to-re-parent. It makes the +rendered task group the shared candidate source and carries the existing Office discriminator into +that view model so the UI can apply the same depth boundary as the backend. The complete unfiltered +task collection supplies hierarchy metadata for cycle and depth validation without adding hidden +tasks to the candidate list. + +The canonical parent mutation, workspace-mode normalization, optimistic update, rollback, and +WebSocket reconciliation continues to use the canonical task event. Office project reassignment +now invokes that publisher, and the event carries the authoritative Office identity explicitly. +The persistence boundaries are described by [Detached Workspace Continuity](detached-workspace-continuity.md). +No endpoint, database column, feature flag, or user-facing copy is added. + +## Requirement mapping + +| Criteria | Design section | +| --------------------------------------------------- | --------------------------------------- | +| AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.1 and .2 | Candidate source and propagation | +| AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.3 and .4 | Eligibility algorithm | +| AC-TASKS-SUBTASK-REPARENTING-DRAG-DROP-001.5 | Responsive interaction and verification | + +## Candidate source and propagation + +The task HTTP DTO already exposes `is_from_office`. `toKanbanTask` preserves it as +`isFromOffice`, the Kanban snapshot task type owns that property, and both desktop and phone +sidebar projections preserve it on `TaskSwitcherItem`. This is a data projection only; Office +identity continues to come from the backend. + +The mapped property remains optional on partial inputs: candidate checks treat an absent value as +non-Office, and `mergeTaskUpdate` retains the cached value when a lightweight event omits +`is_from_office`. Canonical `task.updated` events always carry an explicit true or false value, so +project assignment and removal can establish or clear the cached identity. The Office project +mutation publishes that canonical event after its task-row write. Active-board and multi-workflow +reconciliation uses the same absent-value fallback so an unrelated partial event cannot erase +Office identity from a full snapshot. + +`GroupSection` already flattens the roots and descendants that it renders into `groupTasks` for +drag candidate calculation. That collection is the candidate source for both drag and context-menu +composition. Desktop and phone also pass their complete pre-view task list as hierarchy metadata. +The menu must not reread `kanbanMulti.snapshots[workflowId].tasks`: a present placeholder or +partially refreshed snapshot can contain fewer tasks than the reconciled group visible beside the +menu. + +Rows receive stable getter functions for both collections. The owning switcher and group update +their refs after each committed render, so opening a menu or starting a drag reads current data +without making every memoized sibling row rerender when one task changes. + +Candidate order follows rendered group order. Candidates remain scoped to the subject's workflow, +so mixed-workflow sidebar groups cannot create an invalid target. + +## Eligibility algorithm + +`computeNestCandidates` remains the shared pure function. Its input includes `id`, `parentTaskId`, +and `isFromOffice`. It applies these rules in order: + +1. Resolve the subject from the complete hierarchy, falling back to the rendered group. With no + subject, return no candidates. +2. Exclude the subject and its current parent in every mode. +3. Exclude every rendered task whose ancestor chain reaches the subject. Follow parent links in the + complete hierarchy, including ancestors hidden by filtering, and track visited identifiers so + corrupt hierarchy data cannot loop in the browser. +4. When neither the subject nor candidate is an Office task, require the candidate to be a root and + return no candidates if the subject has a child. This preserves the one-level Kanban boundary. +5. When either endpoint is an Office task, allow a candidate at any depth and allow a subject that + has children. This mirrors `validateReparentDepth`, which exempts the mutation when either + endpoint is Office. + +The browser filter is an affordance, not an authorization boundary. `PATCH /api/v1/tasks/:id` +continues to reject stale, archived, missing, cross-workspace, self, and cyclic targets. + +## Mutation and failure flow + +The context-menu selection and the drag nest zone call the existing `useNestTask` path with the +subject workflow and target identifier. The request uses the canonical task PATCH and the existing +optimistic snapshot update. A rejected request restores the prior tree and shows the existing +request error. A successful `task.updated` event reconciles the sidebar, board, and task detail. + +A temporary multi-workflow placeholder cannot suppress a target that is already present in the +rendered group. A filtered-out task never becomes a candidate, but it remains available as ancestry +metadata. If the rendered group itself contains no eligible task, the menu shows its existing +disabled `No other tasks` row and drag shows no nest zone. + +## Responsive interaction + +Desktop and phone continue to use the same task-row context-menu and drag components. Desktop keeps +the anchored submenu; the phone task switcher keeps its inset sheet, touch drag sensor, menu event +containment, internal scroll owner, safe-area spacing, and current touch targets. Candidate data +changes do not add controls or alter geometry. + +## Verification + +Pure tests cover Office and Kanban eligibility, mixed Office/Kanban endpoints, filtered intermediate +ancestors, descendant exclusion, input order, and missing subjects. Projection and event tests prove +`is_from_office` reaches `TaskSwitcherItem`, survives omitted fields, and changes explicitly in both +directions. Component tests prove the context menu uses current rendered-group candidates without +rerendering unaffected rows, and that menu and drag return the same candidates. + +The existing Kanban sidebar E2E remains a shallow-hierarchy regression. Focused desktop and phone +Office sidebar E2Es create a parent with a child and another target, select that target from `Nest +under`, and assert the deeper persisted and rendered hierarchy. The existing mobile re-parenting +E2E continues to cover the shared touch drag path. + +## Implementation Plans + +- [Fix Nest under candidates](../../../plans/fix-nest-under-candidates/plan.md)