From 986545eee36cfcf820a163decd0461804d2aefe0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Sun, 9 Aug 2026 00:56:15 +0200 Subject: [PATCH] fix(web): delete workspaces via the non-office route when office is off (#2455) Co-authored-by: Kandev Agent Co-authored-by: Claude Opus 5 (1M context) --- apps/web/app/actions/workspaces.test.ts | 33 +++++++++--- apps/web/app/actions/workspaces.ts | 24 ++++++++- .../workspace/workspace-edit-client.tsx | 6 ++- .../workspace-delete-regular-mode.spec.ts | 51 +++++++++++++++++++ 4 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 apps/web/e2e/tests/settings/workspace-delete-regular-mode.spec.ts diff --git a/apps/web/app/actions/workspaces.test.ts b/apps/web/app/actions/workspaces.test.ts index 71ff6f0272..937847baef 100644 --- a/apps/web/app/actions/workspaces.test.ts +++ b/apps/web/app/actions/workspaces.test.ts @@ -14,6 +14,7 @@ import { const REVIEW_STEP_NAME = "Review"; const STEP_COLOR = "bg-blue-500"; +const WORKSPACE_NAME = "My Workspace"; describe("exportAllWorkflowsAction", () => { beforeEach(() => { @@ -65,16 +66,36 @@ describe("deleteWorkspaceAction", () => { vi.restoreAllMocks(); }); - it("sends the workspace name as confirm_name in the DELETE body", async () => { - await deleteWorkspaceAction("ws-1", "My Workspace"); - + const deleteCall = () => { const fetchMock = fetch as unknown as ReturnType; expect(fetchMock).toHaveBeenCalledTimes(1); + return fetchMock.mock.calls[0]; + }; - const [url, init] = fetchMock.mock.calls[0]; - expect(url).toBe("http://backend.test/api/v1/office/workspaces/ws-1"); + it("sends the workspace name as confirm_name in the DELETE body", async () => { + await deleteWorkspaceAction("ws-1", WORKSPACE_NAME, true); + + const [, init] = deleteCall(); expect(init.method).toBe("DELETE"); - expect(JSON.parse(init.body as string)).toEqual({ confirm_name: "My Workspace" }); + expect(JSON.parse(init.body as string)).toEqual({ confirm_name: WORKSPACE_NAME }); + }); + + it("uses the office route when the office feature is enabled", async () => { + await deleteWorkspaceAction("ws-1", WORKSPACE_NAME, true); + + expect(deleteCall()[0]).toBe("http://backend.test/api/v1/office/workspaces/ws-1"); + }); + + // The `/api/v1/office` router group is not mounted when `features.office` is + // off, so the office route 404s there. Deletion has to go to the generic + // endpoint, which owns the same cascade and confirm-name guard. + it("uses the generic route when the office feature is disabled", async () => { + await deleteWorkspaceAction("ws-1", WORKSPACE_NAME, false); + + const [url, init] = deleteCall(); + expect(url).toBe("http://backend.test/api/v1/workspaces/ws-1"); + expect(init.method).toBe("DELETE"); + expect(JSON.parse(init.body as string)).toEqual({ confirm_name: WORKSPACE_NAME }); }); }); diff --git a/apps/web/app/actions/workspaces.ts b/apps/web/app/actions/workspaces.ts index 330a2f5925..35692fba0b 100644 --- a/apps/web/app/actions/workspaces.ts +++ b/apps/web/app/actions/workspaces.ts @@ -97,8 +97,28 @@ export async function updateWorkspaceAction( }); } -export async function deleteWorkspaceAction(id: string, confirmName: string) { - await fetchJson(`${apiBaseUrl}/api/v1/office/workspaces/${id}`, { +// Workspace deletion has two backend routes and the caller must pick by flag. +// +// The office route (`/api/v1/office/workspaces/:id`) additionally purges +// office-owned rows, which have no FK cascade to `workspaces` and are removed by +// an explicit statement list in +// `internal/office/repository/sqlite/workspace_deletion.go`. But the whole +// `/api/v1/office` router group is only mounted when the office services exist, +// and those are skipped when `features.office` is off — the production default. +// Calling it there 404s, which is how Settings deletion broke for every +// non-office install. +// +// So: office on -> office route (cascade plus office cleanup); office off -> +// generic route, which owns the same task/workflow cascade and confirm-name +// guard and is the only one mounted. `officeEnabled` is required rather than +// defaulted so a new call site has to make the choice explicitly. +export async function deleteWorkspaceAction( + id: string, + confirmName: string, + officeEnabled: boolean, +) { + const path = officeEnabled ? `office/workspaces/${id}` : `workspaces/${id}`; + await fetchJson(`${apiBaseUrl}/api/v1/${path}`, { method: "DELETE", body: JSON.stringify({ confirm_name: confirmName }), }); diff --git a/apps/web/app/settings/workspace/workspace-edit-client.tsx b/apps/web/app/settings/workspace/workspace-edit-client.tsx index 62ec7c1f11..5e1e1456ba 100644 --- a/apps/web/app/settings/workspace/workspace-edit-client.tsx +++ b/apps/web/app/settings/workspace/workspace-edit-client.tsx @@ -29,6 +29,7 @@ type Workspace = WorkspaceState["items"][number]; import { useRequest } from "@/lib/http/use-request"; import { useToast } from "@/components/toast-provider"; import { useAppStore } from "@/components/state-provider"; +import { useFeature } from "@/hooks/domains/features/use-feature"; import { SettingsCard } from "@/components/settings/settings-card"; import { useSettingsSaveContributor } from "@/components/settings/settings-save-provider"; import { SettingsTarget } from "@/components/settings/settings-target"; @@ -452,6 +453,9 @@ function useWorkspaceEditForm(workspace: Workspace) { const saveWorkspaceRequest = useRequest(updateWorkspaceAction); const deleteWorkspaceRequest = useRequest(deleteWorkspaceAction); + // Selects the delete route: the office endpoint only exists while the feature + // is on. See `deleteWorkspaceAction`. + const officeEnabled = useFeature("office"); const activeExecutors = executors.filter((executor: Executor) => executor.status === "active"); const isDirty = @@ -476,7 +480,7 @@ function useWorkspaceEditForm(workspace: Workspace) { const handleDeleteWorkspace = async () => { if (deleteConfirmText !== currentWorkspace.name) return; try { - await deleteWorkspaceRequest.run(currentWorkspace.id, currentWorkspace.name); + await deleteWorkspaceRequest.run(currentWorkspace.id, currentWorkspace.name, officeEnabled); setWorkspaces(workspaces.filter((ws: Workspace) => ws.id !== currentWorkspace.id)); runWithNavigationBlockerBypassed(() => router.push("/settings/workspace")); } catch (error) { diff --git a/apps/web/e2e/tests/settings/workspace-delete-regular-mode.spec.ts b/apps/web/e2e/tests/settings/workspace-delete-regular-mode.spec.ts new file mode 100644 index 0000000000..0c9f3b5072 --- /dev/null +++ b/apps/web/e2e/tests/settings/workspace-delete-regular-mode.spec.ts @@ -0,0 +1,51 @@ +import { test, expect } from "../../fixtures/test-base"; +import { useRegularMode } from "../../helpers/regular-mode"; + +// Regression guard for workspace deletion on a non-office install. Settings +// deletion used to always call `/api/v1/office/workspaces/:id`, but the whole +// `/api/v1/office` router group is only mounted when the office services exist, +// and those are skipped when `features.office` is off — the production default. +// The rest of the suite runs with office ON (see profiles.yaml), which is why +// `workspace-delete.spec.ts` never caught this. Keep this file separate: the +// office-off backend restart is file-scoped. +useRegularMode(); + +test.describe("Workspace settings without the office feature", () => { + test("deletes a workspace from the settings edit page", async ({ testPage, apiClient }) => { + const suffix = Date.now().toString(36); + const workspaceName = `Regular Delete ${suffix}`; + const workspace = await apiClient.createWorkspace(workspaceName); + + await testPage.goto(`/settings/workspace/${workspace.id}`); + await expect(testPage.getByRole("heading", { name: workspaceName })).toBeVisible({ + timeout: 15_000, + }); + + await testPage.getByTestId("workspace-settings-delete-button").click(); + + const confirmInput = testPage.getByTestId("workspace-settings-delete-confirm-input"); + const confirmButton = testPage.getByTestId("workspace-settings-delete-confirm-button"); + await expect(confirmInput).toBeVisible(); + + await confirmInput.fill(workspaceName); + await expect(confirmButton).toBeEnabled(); + + // Assert the request target, not just the outcome: hitting the office route + // here is the exact bug, and a 404 would otherwise surface only as a toast. + const deleteRequest = testPage.waitForResponse( + (response) => + response.url().endsWith(`/api/v1/workspaces/${workspace.id}`) && + response.request().method() === "DELETE", + // Below the 60s spec timeout so a regression reports this wait as the + // failure rather than timing out the whole test. + { timeout: 15_000 }, + ); + await confirmButton.click(); + expect((await deleteRequest).ok()).toBe(true); + + await expect(testPage).toHaveURL(/\/settings\/workspace$/, { timeout: 10_000 }); + + const { workspaces } = await apiClient.listWorkspaces(); + expect(workspaces.some((item) => item.id === workspace.id)).toBe(false); + }); +});