fix(web): delete workspaces via the non-office route when office is off (#2455)
Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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<typeof vi.fn>;
|
||||
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 });
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -97,8 +97,28 @@ export async function updateWorkspaceAction(
|
||||
});
|
||||
}
|
||||
|
||||
export async function deleteWorkspaceAction(id: string, confirmName: string) {
|
||||
await fetchJson<void>(`${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<void>(`${apiBaseUrl}/api/v1/${path}`, {
|
||||
method: "DELETE",
|
||||
body: JSON.stringify({ confirm_name: confirmName }),
|
||||
});
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user