fix: respect "All Workflows" selection with multiple workflows (#844)
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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<typeof useRouter>) {
|
||||
const [moveError, setMoveError] = useState<MoveTaskError | null>(null);
|
||||
|
||||
|
||||
@@ -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<void> {
|
||||
// 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<void> {
|
||||
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<void> {
|
||||
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,
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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> = {}): 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");
|
||||
});
|
||||
});
|
||||
@@ -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;
|
||||
}
|
||||
@@ -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) => {
|
||||
|
||||
Reference in New Issue
Block a user