From 7fe2affd6d7fcdc308146b65fd93b9e46ec12cc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Thu, 7 May 2026 23:20:30 +0100 Subject: [PATCH] fix: respect "All Workflows" selection with multiple workflows (#844) --- apps/web/app/page.tsx | 13 +- apps/web/components/kanban-board.tsx | 23 +--- .../e2e/tests/kanban/workflow-filter.spec.ts | 121 ++++++++++++++++++ apps/web/lib/kanban/resolve-workflow.test.ts | 95 ++++++++++++++ apps/web/lib/kanban/resolve-workflow.ts | 34 +++++ .../lib/state/slices/kanban/kanban-slice.ts | 3 - 6 files changed, 258 insertions(+), 31 deletions(-) create mode 100644 apps/web/e2e/tests/kanban/workflow-filter.spec.ts create mode 100644 apps/web/lib/kanban/resolve-workflow.test.ts create mode 100644 apps/web/lib/kanban/resolve-workflow.ts diff --git a/apps/web/app/page.tsx b/apps/web/app/page.tsx index eab6748863..56bbdcb749 100644 --- a/apps/web/app/page.tsx +++ b/apps/web/app/page.tsx @@ -12,6 +12,7 @@ import { import { listWorkspaceTaskPRs } from "@/lib/api/domains/github-api"; import { snapshotToState } from "@/lib/ssr/mapper"; import { mapUserSettingsResponse } from "@/lib/ssr/user-settings"; +import { resolveDesiredWorkflowId } from "@/lib/kanban/resolve-workflow"; import type { AppState } from "@/lib/state/store"; import type { ListWorkspacesResponse, UserSettingsResponse } from "@/lib/types/http"; @@ -158,13 +159,13 @@ export default async function Page({ searchParams }: PageProps) { listQuickChatSessions(activeWorkspaceId, { cache: "no-store" }).catch(() => ({ tasks: [] })), ]); - // Active workflow defaults to the first non-hidden workflow when no preference is set, - // so hidden workflows (e.g., improve-kandev) never auto-select on load. - const workflowId = resolveActiveId( - workflowList.workflows.filter((w) => !w.hidden), - workflowIdParam, + // null preserves the user's "All Workflows" choice when more than one + // workflow is visible — only auto-pick when there's exactly one. + const workflowId = resolveDesiredWorkflowId({ + activeWorkflowId: workflowIdParam ?? null, settingsWorkflowId, - ); + workspaceWorkflows: workflowList.workflows, + }); // Map quick chat tasks to sessions const quickChatSessions = quickChatResponse.tasks diff --git a/apps/web/components/kanban-board.tsx b/apps/web/components/kanban-board.tsx index 2e658d1dd7..fc7d2ae317 100644 --- a/apps/web/components/kanban-board.tsx +++ b/apps/web/components/kanban-board.tsx @@ -16,6 +16,7 @@ import { MobileTaskSheet } from "./kanban/mobile-task-sheet"; import { TaskMultiSelectToolbar } from "./kanban/task-multi-select-toolbar"; import { useKanbanData, useKanbanActions, useKanbanNavigation } from "@/hooks/domains/kanban"; import { useAllWorkflowSnapshots } from "@/hooks/domains/kanban/use-all-workflow-snapshots"; +import { resolveDesiredWorkflowId } from "@/lib/kanban/resolve-workflow"; import { useWorkspacePRs } from "@/hooks/domains/github/use-task-pr"; import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint"; import { useTaskMultiSelect } from "@/hooks/use-task-multi-select"; @@ -91,28 +92,6 @@ function useWorkflowSelection({ ]); } -function resolveDesiredWorkflowId({ - activeWorkflowId, - settingsWorkflowId, - workspaceWorkflows, -}: { - activeWorkflowId?: string | null; - settingsWorkflowId?: string | null; - workspaceWorkflows: WorkflowsState["items"]; -}): string | null { - const visibleWorkflows = workspaceWorkflows.filter((workflow) => !workflow.hidden); - if (activeWorkflowId && visibleWorkflows.some((workflow) => workflow.id === activeWorkflowId)) { - return activeWorkflowId; - } - if ( - settingsWorkflowId && - visibleWorkflows.some((workflow) => workflow.id === settingsWorkflowId) - ) { - return settingsWorkflowId; - } - return visibleWorkflows[0]?.id ?? null; -} - function useMoveErrorState(router: ReturnType) { const [moveError, setMoveError] = useState(null); diff --git a/apps/web/e2e/tests/kanban/workflow-filter.spec.ts b/apps/web/e2e/tests/kanban/workflow-filter.spec.ts new file mode 100644 index 0000000000..db74ca63fe --- /dev/null +++ b/apps/web/e2e/tests/kanban/workflow-filter.spec.ts @@ -0,0 +1,121 @@ +import { test, expect } from "../../fixtures/test-base"; +import { KanbanPage } from "../../pages/kanban-page"; +import type { Page } from "@playwright/test"; + +const TASK_VISIBLE_TIMEOUT = 10_000; +const ALPHA_TASK = "Alpha task"; +const BETA_TASK = "Beta task"; + +async function pickListboxOption(page: Page, optionLabel: string): Promise { + // Radix Select renders the currently-selected item twice (once in the trigger + // for SelectValue display, once in the listbox). Scope to the listbox so we + // click the real option. + const listbox = page.getByRole("listbox"); + await listbox.getByRole("option", { name: optionLabel, exact: true }).click(); + await expect(listbox).toHaveCount(0); +} + +async function closeDisplayDropdown(page: Page): Promise { + const trigger = page.getByTestId("display-button"); + if ((await trigger.getAttribute("data-state")) === "open") { + await trigger.click({ force: true }); + } + await expect(trigger).not.toHaveAttribute("data-state", "open"); + await expect(page.getByRole("menu")).toHaveCount(0); +} + +async function selectWorkflowFilter(page: Page, optionLabel: string): Promise { + await page.getByTestId("display-button").click(); + await page.getByTestId("display-workflow-filter").click(); + await pickListboxOption(page, optionLabel); + await closeDisplayDropdown(page); +} + +test.describe("Kanban workflow filter", () => { + let workflowBId: string | null = null; + + // Pull `testPage` so its fixture (which runs `e2eReset` and resets user + // settings) is set up before this hook seeds workflows/tasks — otherwise + // the reset wipes the seed data the moment a test first reads testPage. + // eslint-disable-next-line @typescript-eslint/no-unused-vars + test.beforeEach(async ({ apiClient, seedData, testPage }) => { + const workflowB = await apiClient.createWorkflow(seedData.workspaceId, "Workflow B", "simple"); + workflowBId = workflowB.id; + const stepsB = (await apiClient.listWorkflowSteps(workflowB.id)).steps; + const startB = stepsB.find((s) => s.is_start_step) ?? stepsB[0]; + + await apiClient.createTask(seedData.workspaceId, ALPHA_TASK, { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + await apiClient.createTask(seedData.workspaceId, BETA_TASK, { + workflow_id: workflowB.id, + workflow_step_id: startB.id, + }); + }); + + test.afterEach(async ({ apiClient, seedData }) => { + if (workflowBId) { + await apiClient.deleteWorkflow(workflowBId).catch(() => {}); + workflowBId = null; + } + await apiClient.saveUserSettings({ + workspace_id: seedData.workspaceId, + workflow_filter_id: seedData.workflowId, + repository_ids: [], + }); + }); + + // Regression: c64e835 made resolveDesiredWorkflowId fall back to the first + // visible workflow whenever both the active id and persisted setting were + // null. The kanban page's useWorkflowSelection effect then silently + // overwrote a freshly-picked "All Workflows" choice on the next render. + // The /tasks list page does not run that effect, so the existing + // task-list-filters spec missed this — pin the kanban path explicitly. + test("'All Workflows' selection persists on the kanban board", async ({ testPage }) => { + const kanban = new KanbanPage(testPage); + await kanban.goto(); + + await expect(kanban.taskCardByTitle(ALPHA_TASK)).toBeVisible({ + timeout: TASK_VISIBLE_TIMEOUT, + }); + await expect(kanban.taskCardByTitle(BETA_TASK)).not.toBeVisible(); + + await selectWorkflowFilter(testPage, "All Workflows"); + + // Both tasks visible — useWorkflowSelection must not overwrite the null choice. + await expect(kanban.taskCardByTitle(ALPHA_TASK)).toBeVisible({ + timeout: TASK_VISIBLE_TIMEOUT, + }); + await expect(kanban.taskCardByTitle(BETA_TASK)).toBeVisible({ + timeout: TASK_VISIBLE_TIMEOUT, + }); + }); + + // Regression: SSR resolveActiveId in app/page.tsx fell back to the first + // visible workflow when settings.workflow_filter_id was empty, so a saved + // "All Workflows" preference reverted on hard refresh. + test("'All Workflows' selection survives a hard refresh", async ({ + testPage, + apiClient, + seedData, + }) => { + // Persist "All Workflows" directly via the API to mimic the post-selection + // state, then load the kanban page from scratch (no in-flight client state). + await apiClient.saveUserSettings({ + workspace_id: seedData.workspaceId, + workflow_filter_id: "", + repository_ids: [], + }); + + const kanban = new KanbanPage(testPage); + await kanban.goto(); + + await expect(kanban.taskCardByTitle(ALPHA_TASK)).toBeVisible({ + timeout: TASK_VISIBLE_TIMEOUT, + }); + await expect(kanban.taskCardByTitle(BETA_TASK)).toBeVisible({ + timeout: TASK_VISIBLE_TIMEOUT, + }); + }); +}); diff --git a/apps/web/lib/kanban/resolve-workflow.test.ts b/apps/web/lib/kanban/resolve-workflow.test.ts new file mode 100644 index 0000000000..cfcf8f08d7 --- /dev/null +++ b/apps/web/lib/kanban/resolve-workflow.test.ts @@ -0,0 +1,95 @@ +import { describe, it, expect } from "vitest"; +import { resolveDesiredWorkflowId } from "./resolve-workflow"; +import type { WorkflowsState } from "@/lib/state/slices"; + +type Workflow = WorkflowsState["items"][number]; + +function workflow(id: string, overrides: Partial = {}): Workflow { + return { id, workspaceId: "ws-1", name: id, ...overrides }; +} + +describe("resolveDesiredWorkflowId", () => { + it("keeps the currently active workflow when it is still visible", () => { + const result = resolveDesiredWorkflowId({ + activeWorkflowId: "wf-2", + settingsWorkflowId: "wf-1", + workspaceWorkflows: [workflow("wf-1"), workflow("wf-2")], + }); + expect(result).toBe("wf-2"); + }); + + it("falls back to the persisted settings workflow when active is missing", () => { + const result = resolveDesiredWorkflowId({ + activeWorkflowId: null, + settingsWorkflowId: "wf-1", + workspaceWorkflows: [workflow("wf-1"), workflow("wf-2")], + }); + expect(result).toBe("wf-1"); + }); + + // Regression: c64e835 forced fallback to the first visible workflow when + // both active and settings ids were null, making "All Workflows" impossible + // to select with multiple workflows present. + it("returns null when the user has cleared the filter and multiple workflows exist", () => { + const result = resolveDesiredWorkflowId({ + activeWorkflowId: null, + settingsWorkflowId: null, + workspaceWorkflows: [workflow("wf-1"), workflow("wf-2"), workflow("wf-3")], + }); + expect(result).toBeNull(); + }); + + it("auto-selects the only visible workflow when exactly one exists", () => { + const result = resolveDesiredWorkflowId({ + activeWorkflowId: null, + settingsWorkflowId: null, + workspaceWorkflows: [workflow("wf-only")], + }); + expect(result).toBe("wf-only"); + }); + + it("ignores hidden workflows when resolving the fallback", () => { + const result = resolveDesiredWorkflowId({ + activeWorkflowId: null, + settingsWorkflowId: null, + workspaceWorkflows: [workflow("wf-1", { hidden: true }), workflow("wf-2")], + }); + expect(result).toBe("wf-2"); + }); + + it("does not honor an active id that is no longer visible", () => { + const result = resolveDesiredWorkflowId({ + activeWorkflowId: "wf-stale", + settingsWorkflowId: "wf-1", + workspaceWorkflows: [workflow("wf-1"), workflow("wf-2")], + }); + expect(result).toBe("wf-1"); + }); + + it("returns null when no workflows are visible", () => { + const result = resolveDesiredWorkflowId({ + activeWorkflowId: null, + settingsWorkflowId: null, + workspaceWorkflows: [], + }); + expect(result).toBeNull(); + }); + + it("returns null when active id is stale and settings id is also null", () => { + const result = resolveDesiredWorkflowId({ + activeWorkflowId: "wf-stale", + settingsWorkflowId: null, + workspaceWorkflows: [workflow("wf-1"), workflow("wf-2")], + }); + expect(result).toBeNull(); + }); + + it("does not fall back to a hidden settings workflow", () => { + const result = resolveDesiredWorkflowId({ + activeWorkflowId: null, + settingsWorkflowId: "wf-hidden", + workspaceWorkflows: [workflow("wf-hidden", { hidden: true }), workflow("wf-visible")], + }); + expect(result).toBe("wf-visible"); + }); +}); diff --git a/apps/web/lib/kanban/resolve-workflow.ts b/apps/web/lib/kanban/resolve-workflow.ts new file mode 100644 index 0000000000..27d5d2affd --- /dev/null +++ b/apps/web/lib/kanban/resolve-workflow.ts @@ -0,0 +1,34 @@ +import type { WorkflowsState } from "@/lib/state/slices"; + +type WorkflowLike = { id: string; hidden?: boolean }; + +/** + * Resolve the workflow id that should be active given the current store state + * and persisted user settings. + * + * `null` is a valid "All Workflows" selection: when the user has explicitly + * cleared the filter, we must not silently fall back to the first workflow. + * Auto-selecting only happens when there is exactly one visible workflow — + * otherwise the user would never be able to keep "All Workflows" picked. + */ +export function resolveDesiredWorkflowId({ + activeWorkflowId, + settingsWorkflowId, + workspaceWorkflows, +}: { + activeWorkflowId?: string | null; + settingsWorkflowId?: string | null; + workspaceWorkflows: WorkflowsState["items"] | WorkflowLike[]; +}): string | null { + const visibleWorkflows = workspaceWorkflows.filter((workflow) => !workflow.hidden); + if (activeWorkflowId && visibleWorkflows.some((workflow) => workflow.id === activeWorkflowId)) { + return activeWorkflowId; + } + if ( + settingsWorkflowId && + visibleWorkflows.some((workflow) => workflow.id === settingsWorkflowId) + ) { + return settingsWorkflowId; + } + return visibleWorkflows.length === 1 ? visibleWorkflows[0].id : null; +} diff --git a/apps/web/lib/state/slices/kanban/kanban-slice.ts b/apps/web/lib/state/slices/kanban/kanban-slice.ts index 02bc40c301..8a3d594e52 100644 --- a/apps/web/lib/state/slices/kanban/kanban-slice.ts +++ b/apps/web/lib/state/slices/kanban/kanban-slice.ts @@ -26,9 +26,6 @@ export const createKanbanSlice: StateCreator< setWorkflows: (workflows) => set((draft) => { draft.workflows.items = workflows; - if (!draft.workflows.activeId && workflows.length) { - draft.workflows.activeId = workflows[0].id; - } }), reorderWorkflowItems: (workflowIds) => set((draft) => {