From 800d71f3d150d4dca39fbfd61b01869f28a55655 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Thu, 11 Jun 2026 12:02:25 +0100 Subject: [PATCH] fix: correct sidebar workspace switcher routing and selection persistence (#1329) --- apps/web/app/page.tsx | 26 ++-- .../app-sidebar-workspace-picker.test.tsx | 127 ++++++++++++++++++ .../app-sidebar-workspace-picker.tsx | 2 +- apps/web/lib/ssr/resolve-active-id.test.ts | 32 +++++ apps/web/lib/ssr/resolve-active-id.ts | 13 ++ 5 files changed, 185 insertions(+), 15 deletions(-) create mode 100644 apps/web/components/app-sidebar/app-sidebar-workspace-picker.test.tsx create mode 100644 apps/web/lib/ssr/resolve-active-id.test.ts create mode 100644 apps/web/lib/ssr/resolve-active-id.ts diff --git a/apps/web/app/page.tsx b/apps/web/app/page.tsx index 56bbdcb749..4a0ca41727 100644 --- a/apps/web/app/page.tsx +++ b/apps/web/app/page.tsx @@ -1,3 +1,4 @@ +import { cookies } from "next/headers"; import { PageClient } from "@/app/page-client"; import { StateHydrator } from "@/components/state-hydrator"; import { @@ -13,6 +14,7 @@ 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 { resolveActiveId } from "@/lib/ssr/resolve-active-id"; import type { AppState } from "@/lib/state/store"; import type { ListWorkspacesResponse, UserSettingsResponse } from "@/lib/types/http"; @@ -56,19 +58,6 @@ function readAgentProfileId( return typeof value === "string" ? value : undefined; } -function resolveActiveId( - items: T[], - preferredId?: string, - fallbackId?: string | null, -): string | null { - return ( - items.find((i) => i.id === preferredId)?.id ?? - items.find((i) => i.id === fallbackId)?.id ?? - items[0]?.id ?? - null - ); -} - function buildBaseState( workspaces: ListWorkspacesResponse, userSettingsResponse: UserSettingsResponse | null, @@ -124,15 +113,24 @@ export default async function Page({ searchParams }: PageProps) { const taskId = resolveParam(resolvedParams.taskId); const sessionId = resolveParam(resolvedParams.sessionId); - const [workspaces, userSettingsResponse] = await Promise.all([ + const [workspaces, userSettingsResponse, cookieStore] = await Promise.all([ listWorkspaces({ cache: "no-store" }), fetchUserSettings({ cache: "no-store" }).catch(() => null), + cookies().catch((error) => { + console.error("Failed to read cookies on Kanban page:", error); + return null; + }), ]); const settingsWorkspaceId = userSettingsResponse?.settings?.workspace_id || null; const settingsWorkflowId = userSettingsResponse?.settings?.workflow_filter_id || null; + // The sidebar picker writes the selected workspace to this cookie so the + // choice survives a refresh even when office is disabled (userSettings is + // not updated on select). Priority: URL param > cookie > saved setting. + const cookieWorkspaceId = cookieStore?.get("office-active-workspace")?.value ?? null; const activeWorkspaceId = resolveActiveId( workspaces.workspaces, workspaceId, + cookieWorkspaceId, settingsWorkspaceId, ); diff --git a/apps/web/components/app-sidebar/app-sidebar-workspace-picker.test.tsx b/apps/web/components/app-sidebar/app-sidebar-workspace-picker.test.tsx new file mode 100644 index 0000000000..eb1bb451cd --- /dev/null +++ b/apps/web/components/app-sidebar/app-sidebar-workspace-picker.test.tsx @@ -0,0 +1,127 @@ +import { describe, it, expect, beforeEach, vi, afterEach } from "vitest"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; + +const navigationMock = vi.hoisted(() => ({ push: vi.fn() })); + +vi.mock("next/navigation", () => ({ + useRouter: () => ({ push: navigationMock.push }), +})); + +// Radix dropdown primitives rely on pointer/portal behaviour that jsdom doesn't +// model well. Render them as plain elements so the focus stays on the picker's +// routing logic: `onSelect` fires on click of the item. +vi.mock("@kandev/ui/dropdown-menu", () => ({ + DropdownMenu: ({ children }: { children: React.ReactNode }) =>
{children}
, + DropdownMenuTrigger: ({ children }: { children: React.ReactNode }) =>
{children}
, + DropdownMenuContent: ({ children }: { children: React.ReactNode }) =>
{children}
, + DropdownMenuItem: ({ + children, + onSelect, + disabled, + "data-testid": testId, + }: { + children: React.ReactNode; + onSelect?: () => void; + disabled?: boolean; + "data-testid"?: string; + }) => ( + + ), + DropdownMenuSeparator: () =>
, +})); + +const storeState = { + features: { office: false }, + workspaces: { + items: [{ id: "w1", name: "Default Workspace" }], + activeId: "w1", + }, + setActiveWorkspace: vi.fn(), +}; + +vi.mock("@/components/state-provider", () => ({ + useAppStore: (selector: (state: typeof storeState) => unknown) => selector(storeState), +})); + +import { AppSidebarWorkspacePicker } from "./app-sidebar-workspace-picker"; + +describe("AppSidebarWorkspacePicker — Add workspace routing", () => { + beforeEach(() => { + navigationMock.push = vi.fn(); + storeState.features.office = false; + storeState.setActiveWorkspace = vi.fn(); + }); + + afterEach(() => { + cleanup(); + }); + + it("routes to the office setup wizard when the office feature is enabled", () => { + storeState.features.office = true; + render(); + + fireEvent.click(screen.getByText("Add workspace")); + + expect(navigationMock.push).toHaveBeenCalledWith("/office/setup?mode=new"); + }); + + it("routes to the settings workspaces page when the office feature is disabled", () => { + storeState.features.office = false; + render(); + + fireEvent.click(screen.getByText("Add workspace")); + + expect(navigationMock.push).toHaveBeenCalledWith("/settings/workspace"); + }); +}); + +describe("AppSidebarWorkspacePicker — workspace select", () => { + // jsdom over http drops `secure` cookies, so intercept the setter to capture + // the write directly rather than reading `document.cookie` back. + let cookieWrites: string[] = []; + let cookieDescriptor: PropertyDescriptor | undefined; + + beforeEach(() => { + navigationMock.push = vi.fn(); + storeState.setActiveWorkspace = vi.fn(); + cookieWrites = []; + cookieDescriptor = Object.getOwnPropertyDescriptor(Document.prototype, "cookie"); + Object.defineProperty(document, "cookie", { + configurable: true, + get: () => cookieWrites.join("; "), + set: (value: string) => { + cookieWrites.push(value); + }, + }); + }); + + afterEach(() => { + if (cookieDescriptor) { + Object.defineProperty(document, "cookie", cookieDescriptor); + } + cleanup(); + }); + + it("writes the active-workspace cookie and updates the store on select", () => { + storeState.features.office = false; + render(); + + fireEvent.click(screen.getByTestId("sidebar-workspace-item-w1")); + + expect(cookieWrites.some((c) => c.startsWith("office-active-workspace=w1"))).toBe(true); + expect(storeState.setActiveWorkspace).toHaveBeenCalledWith("w1"); + expect(navigationMock.push).not.toHaveBeenCalled(); + }); + + it("navigates to the office workspace when the office feature is enabled", () => { + storeState.features.office = true; + render(); + + fireEvent.click(screen.getByTestId("sidebar-workspace-item-w1")); + + expect(storeState.setActiveWorkspace).toHaveBeenCalledWith("w1"); + expect(navigationMock.push).toHaveBeenCalledWith("/office?workspaceId=w1"); + }); +}); diff --git a/apps/web/components/app-sidebar/app-sidebar-workspace-picker.tsx b/apps/web/components/app-sidebar/app-sidebar-workspace-picker.tsx index e193cd25be..f2b80e4d39 100644 --- a/apps/web/components/app-sidebar/app-sidebar-workspace-picker.tsx +++ b/apps/web/components/app-sidebar/app-sidebar-workspace-picker.tsx @@ -93,7 +93,7 @@ export function AppSidebarWorkspacePicker() { { - router.push("/office/setup?mode=new"); + router.push(officeEnabled ? "/office/setup?mode=new" : "/settings/workspace"); }} > diff --git a/apps/web/lib/ssr/resolve-active-id.test.ts b/apps/web/lib/ssr/resolve-active-id.test.ts new file mode 100644 index 0000000000..3b4bec18e8 --- /dev/null +++ b/apps/web/lib/ssr/resolve-active-id.test.ts @@ -0,0 +1,32 @@ +import { describe, it, expect } from "vitest"; +import { resolveActiveId } from "./resolve-active-id"; + +const items = [{ id: "sky" }, { id: "test" }]; + +describe("resolveActiveId", () => { + it("returns the first candidate that exists in items", () => { + expect(resolveActiveId(items, "test")).toBe("test"); + }); + + it("honours candidate priority order (earlier wins)", () => { + // URL param > cookie > setting: the cookie ("test") should win over the + // saved setting ("sky") when no URL param is given. + expect(resolveActiveId(items, undefined, "test", "sky")).toBe("test"); + }); + + it("falls through to the next candidate when an earlier one is missing", () => { + expect(resolveActiveId(items, "ghost", "test")).toBe("test"); + }); + + it("skips null/undefined candidates", () => { + expect(resolveActiveId(items, null, undefined, "sky")).toBe("sky"); + }); + + it("falls back to the first item when no candidate matches", () => { + expect(resolveActiveId(items, "ghost", null)).toBe("sky"); + }); + + it("returns null when there are no items", () => { + expect(resolveActiveId([], "test")).toBeNull(); + }); +}); diff --git a/apps/web/lib/ssr/resolve-active-id.ts b/apps/web/lib/ssr/resolve-active-id.ts new file mode 100644 index 0000000000..9b4e5e2886 --- /dev/null +++ b/apps/web/lib/ssr/resolve-active-id.ts @@ -0,0 +1,13 @@ +// Returns the first candidate id present in `items` (checked in order, so callers +// pass highest-priority first), falling back to the first item's id. +export function resolveActiveId( + items: T[], + ...preferredIds: (string | null | undefined)[] +): string | null { + for (const id of preferredIds) { + if (id == null) continue; + const match = items.find((i) => i.id === id); + if (match) return match.id; + } + return items[0]?.id ?? null; +}