fix: correct sidebar workspace switcher routing and selection persistence (#1329)

2026-06-11 12:02:25 +01:00
parent 00500cca05
commit 800d71f3d1
5 changed files with 185 additions and 15 deletions
@@ -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<T extends { id: string }>(
-  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,
    );

@@ -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 }) => <div>{children}</div>,
+  DropdownMenuTrigger: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
+  DropdownMenuContent: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
+  DropdownMenuItem: ({
+    children,
+    onSelect,
+    disabled,
+    "data-testid": testId,
+  }: {
+    children: React.ReactNode;
+    onSelect?: () => void;
+    disabled?: boolean;
+    "data-testid"?: string;
+  }) => (
+    <button type="button" disabled={disabled} data-testid={testId} onClick={() => onSelect?.()}>
+      {children}
+    </button>
+  ),
+  DropdownMenuSeparator: () => <hr />,
+}));
+
+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(<AppSidebarWorkspacePicker />);
+
+    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(<AppSidebarWorkspacePicker />);
+
+    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(<AppSidebarWorkspacePicker />);
+
+    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(<AppSidebarWorkspacePicker />);
+
+    fireEvent.click(screen.getByTestId("sidebar-workspace-item-w1"));
+
+    expect(storeState.setActiveWorkspace).toHaveBeenCalledWith("w1");
+    expect(navigationMock.push).toHaveBeenCalledWith("/office?workspaceId=w1");
+  });
+});
@@ -93,7 +93,7 @@ export function AppSidebarWorkspacePicker() {
        <DropdownMenuItem
          className="cursor-pointer gap-2"
          onSelect={() => {
-            router.push("/office/setup?mode=new");
+            router.push(officeEnabled ? "/office/setup?mode=new" : "/settings/workspace");
          }}
        >
          <IconPlus className="h-3.5 w-3.5" />
@@ -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();
+  });
+});
@@ -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<T extends { id: string }>(
+  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;
+}