fix(office): persist workspace name and description edits on save (#2859)
Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
@@ -449,8 +449,6 @@ type CreateCommentRequest struct {
|
||||
|
||||
// UpdateWorkspaceSettingsRequest is the request body for updating workspace settings.
|
||||
type UpdateWorkspaceSettingsRequest struct {
|
||||
Name *string `json:"name"`
|
||||
Description *string `json:"description"`
|
||||
PermissionHandlingMode *string `json:"permission_handling_mode"`
|
||||
RecoveryLookbackHours *int `json:"recovery_lookback_hours"`
|
||||
RequireApprovalForNewAgents *bool `json:"require_approval_for_new_agents"`
|
||||
|
||||
@@ -0,0 +1,314 @@
|
||||
import { act, cleanup, fireEvent, renderHook, screen } from "@testing-library/react";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { useSettingsState } from "./settings-content";
|
||||
import { updateWorkspaceAction } from "@/app/actions/workspaces";
|
||||
import { getWorkspaceSettings } from "@/lib/api/domains/office-api";
|
||||
import type { WorkspaceState } from "@/lib/state/slices/workspace/types";
|
||||
import {
|
||||
SettingsSaveProvider,
|
||||
useSettingsSaveCoordinator,
|
||||
} from "@/components/settings/settings-save-provider";
|
||||
|
||||
vi.mock("@/app/actions/workspaces", () => ({
|
||||
updateWorkspaceAction: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/api/domains/office-api", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("@/lib/api/domains/office-api")>();
|
||||
return { ...actual, updateWorkspaceSettings: vi.fn(), getWorkspaceSettings: vi.fn() };
|
||||
});
|
||||
|
||||
const mockUpdateWorkspaceAction = vi.mocked(updateWorkspaceAction);
|
||||
const mockGetWorkspaceSettings = vi.mocked(getWorkspaceSettings);
|
||||
|
||||
type Workspace = WorkspaceState["items"][number];
|
||||
|
||||
const WORKSPACE_ID = "ws-1";
|
||||
const NEW_DESCRIPTION = "New description";
|
||||
const REMOTE_NAME = "Remote Name";
|
||||
const REMOTE_DESCRIPTION = "Remote description";
|
||||
|
||||
function workspace(overrides: Partial<Workspace> = {}): Workspace {
|
||||
return {
|
||||
id: WORKSPACE_ID,
|
||||
name: "Original Name",
|
||||
description: "Original description",
|
||||
owner_id: "user-1",
|
||||
created_at: "2026-01-01T00:00:00Z",
|
||||
updated_at: "2026-01-01T00:00:00Z",
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
// Mirrors the slice of `StoreApi<AppState>` the appearance-save handler
|
||||
// reads: a live `getState()` accessor, not a snapshot passed at render time.
|
||||
// This is what lets the test below prove a concurrent store write survives.
|
||||
function makeStoreApi(initialItems: Workspace[], onSetWorkspaces?: (items: Workspace[]) => void) {
|
||||
let items = initialItems;
|
||||
const setWorkspaces = vi.fn((next: Workspace[]) => {
|
||||
items = next;
|
||||
onSetWorkspaces?.(next);
|
||||
});
|
||||
return {
|
||||
storeApi: {
|
||||
getState: () => ({ workspaces: { items, activeId: items[0]?.id ?? null }, setWorkspaces }),
|
||||
},
|
||||
setWorkspaces,
|
||||
getItems: () => items,
|
||||
setItems: (next: Workspace[]) => {
|
||||
items = next;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function renderSettings(
|
||||
activeWorkspace: Workspace,
|
||||
storeApi: ReturnType<typeof makeStoreApi>["storeApi"],
|
||||
) {
|
||||
return renderHook(({ ws }: { ws: Workspace }) => useSettingsState(ws, storeApi), {
|
||||
initialProps: { ws: activeWorkspace },
|
||||
wrapper: SettingsSaveProvider,
|
||||
});
|
||||
}
|
||||
|
||||
function renderCoordinatedSettings(
|
||||
activeWorkspace: Workspace,
|
||||
storeApi: ReturnType<typeof makeStoreApi>["storeApi"],
|
||||
) {
|
||||
return renderHook(
|
||||
({ ws }: { ws: Workspace }) => {
|
||||
const settings = useSettingsState(ws, storeApi);
|
||||
const coordinator = useSettingsSaveCoordinator();
|
||||
return { settings, coordinator };
|
||||
},
|
||||
{ initialProps: { ws: activeWorkspace }, wrapper: SettingsSaveProvider },
|
||||
);
|
||||
}
|
||||
|
||||
// Combines the two setup calls every test needs: a store double plus the
|
||||
// hook rendered against it.
|
||||
function renderWithStore(items: Workspace[], onSetWorkspaces?: (items: Workspace[]) => void) {
|
||||
const store = makeStoreApi(items, onSetWorkspaces);
|
||||
return { ...store, ...renderSettings(items[0], store.storeApi) };
|
||||
}
|
||||
|
||||
async function saveAppearance(result: ReturnType<typeof renderSettings>["result"]) {
|
||||
await act(async () => {
|
||||
await result.current.handleSaveAppearance();
|
||||
});
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
mockGetWorkspaceSettings.mockResolvedValue({ settings: {} } as never);
|
||||
});
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
describe("useSettingsState appearance save", () => {
|
||||
it("saves the trimmed name and description through updateWorkspaceAction and syncs the store", async () => {
|
||||
const ws = workspace();
|
||||
const { setWorkspaces, result, rerender } = renderWithStore([ws]);
|
||||
mockUpdateWorkspaceAction.mockResolvedValue({
|
||||
...ws,
|
||||
name: "Renamed",
|
||||
description: NEW_DESCRIPTION,
|
||||
} as never);
|
||||
|
||||
act(() => {
|
||||
result.current.setName(" Renamed ");
|
||||
result.current.setDescription(NEW_DESCRIPTION);
|
||||
});
|
||||
|
||||
await saveAppearance(result);
|
||||
|
||||
expect(mockUpdateWorkspaceAction).toHaveBeenCalledWith(WORKSPACE_ID, {
|
||||
name: "Renamed",
|
||||
description: NEW_DESCRIPTION,
|
||||
});
|
||||
expect(setWorkspaces).toHaveBeenCalledWith([
|
||||
{ ...ws, name: "Renamed", description: NEW_DESCRIPTION },
|
||||
]);
|
||||
|
||||
// The submitted draft had leading/trailing whitespace; the echo guard
|
||||
// must compare against that raw draft, not its trimmed form, or the
|
||||
// input keeps the whitespace forever and the dirty flag never clears.
|
||||
rerender({ ws: { ...ws, name: "Renamed", description: NEW_DESCRIPTION } });
|
||||
expect(result.current.name).toBe("Renamed");
|
||||
expect(result.current.appearanceDirty).toBe(false);
|
||||
});
|
||||
|
||||
it("clears the dirty flag once the store reflects the saved workspace", async () => {
|
||||
const { getItems, result, rerender } = renderWithStore([workspace()]);
|
||||
mockUpdateWorkspaceAction.mockResolvedValue({ ...workspace(), name: "Renamed" } as never);
|
||||
|
||||
act(() => result.current.setName("Renamed"));
|
||||
expect(result.current.appearanceDirty).toBe(true);
|
||||
|
||||
await saveAppearance(result);
|
||||
|
||||
// Simulate the Zustand store propagating the update back into
|
||||
// `activeWorkspace`, the way SettingsContent's subscription would on the
|
||||
// next render.
|
||||
rerender({ ws: getItems()[0] });
|
||||
|
||||
expect(result.current.appearanceDirty).toBe(false);
|
||||
});
|
||||
|
||||
it("refuses to save an all-whitespace name", async () => {
|
||||
const { setWorkspaces, result } = renderWithStore([workspace()]);
|
||||
|
||||
act(() => result.current.setName(" "));
|
||||
|
||||
await saveAppearance(result);
|
||||
|
||||
expect(mockUpdateWorkspaceAction).not.toHaveBeenCalled();
|
||||
expect(setWorkspaces).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// A newer keystroke landing before the PATCH from an older one resolves
|
||||
// must survive that response, not get overwritten by the stale echo.
|
||||
it("keeps a draft typed during an in-flight save instead of reverting it", async () => {
|
||||
const ws = workspace();
|
||||
let resolveUpdate!: (value: Workspace) => void;
|
||||
mockUpdateWorkspaceAction.mockImplementation(
|
||||
() => new Promise<Workspace>((resolve) => (resolveUpdate = resolve)) as never,
|
||||
);
|
||||
|
||||
const { result } = renderWithStore([ws]);
|
||||
act(() => result.current.setName("Alpha"));
|
||||
let savePromise!: Promise<void>;
|
||||
act(() => {
|
||||
savePromise = result.current.handleSaveAppearance();
|
||||
});
|
||||
act(() => result.current.setName("Alpha2"));
|
||||
|
||||
await act(async () => {
|
||||
resolveUpdate({ ...ws, name: "Alpha" });
|
||||
await savePromise;
|
||||
});
|
||||
|
||||
expect(result.current.name).toBe("Alpha2");
|
||||
});
|
||||
|
||||
// A workspace.deleted WS event elsewhere can reassign the active workspace
|
||||
// without this page remounting; the draft must reset to the new one's values.
|
||||
it("resets the draft when the active workspace's identity changes", async () => {
|
||||
const { result, rerender } = renderWithStore([workspace()]);
|
||||
act(() => result.current.setName("Unsaved Draft"));
|
||||
expect(result.current.appearanceDirty).toBe(true);
|
||||
rerender({ ws: workspace({ id: "ws-2", name: "Other Workspace", description: "Other desc" }) });
|
||||
expect(result.current.name).toBe("Other Workspace");
|
||||
expect(result.current.description).toBe("Other desc");
|
||||
expect(result.current.appearanceDirty).toBe(false);
|
||||
});
|
||||
|
||||
it("does not clobber a workspace added to the store while the save was in flight", async () => {
|
||||
const ws = workspace();
|
||||
const concurrent = workspace({ id: "ws-2", name: "Concurrent Workspace" });
|
||||
const { setWorkspaces, setItems, result } = renderWithStore([ws]);
|
||||
|
||||
// Simulate a `workspace.created` WS event landing mid-PATCH: the store
|
||||
// gains a second workspace between render (when the old code captured
|
||||
// its snapshot) and the moment the save handler actually reads state.
|
||||
mockUpdateWorkspaceAction.mockImplementation(async () => {
|
||||
setItems([ws, concurrent]);
|
||||
return { ...ws, name: "Renamed", description: NEW_DESCRIPTION } as never;
|
||||
});
|
||||
|
||||
act(() => {
|
||||
result.current.setName("Renamed");
|
||||
result.current.setDescription(NEW_DESCRIPTION);
|
||||
});
|
||||
|
||||
await saveAppearance(result);
|
||||
|
||||
expect(setWorkspaces).toHaveBeenCalledWith([
|
||||
{ ...ws, name: "Renamed", description: NEW_DESCRIPTION },
|
||||
concurrent,
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("coordinated appearance save", () => {
|
||||
it("registers appearance drafts with the shared save coordinator", async () => {
|
||||
const ws = workspace();
|
||||
const store = makeStoreApi([ws]);
|
||||
const { result } = renderCoordinatedSettings(ws, store.storeApi);
|
||||
mockUpdateWorkspaceAction.mockResolvedValue({ ...ws, name: "Renamed" } as never);
|
||||
|
||||
act(() => result.current.settings.setName("Renamed"));
|
||||
|
||||
expect(result.current.coordinator.hasDirty).toBe(true);
|
||||
await act(async () => {
|
||||
await expect(result.current.coordinator.saveAll()).resolves.toMatchObject({
|
||||
canLeave: true,
|
||||
});
|
||||
});
|
||||
|
||||
expect(mockUpdateWorkspaceAction).toHaveBeenCalledWith(WORKSPACE_ID, {
|
||||
name: "Renamed",
|
||||
description: ws.description,
|
||||
});
|
||||
expect(store.setWorkspaces).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("propagates save failures after clearing saving state", async () => {
|
||||
mockUpdateWorkspaceAction.mockRejectedValue(new Error("network error"));
|
||||
const { setWorkspaces, result } = renderWithStore([workspace()]);
|
||||
|
||||
act(() => result.current.setName("Changed"));
|
||||
await expect(
|
||||
act(async () => {
|
||||
await result.current.handleSaveAppearance();
|
||||
}),
|
||||
).rejects.toThrow("network error");
|
||||
|
||||
expect(setWorkspaces).not.toHaveBeenCalled();
|
||||
expect(result.current.savingAppearance).toBe(false);
|
||||
});
|
||||
|
||||
it("resets the appearance draft to the authoritative workspace baseline", async () => {
|
||||
const ws = workspace();
|
||||
const { result } = renderCoordinatedSettings(ws, makeStoreApi([ws]).storeApi);
|
||||
|
||||
act(() => {
|
||||
result.current.settings.setName("Unsaved name");
|
||||
result.current.settings.setDescription("Unsaved description");
|
||||
});
|
||||
|
||||
expect(screen.getByTestId("settings-floating-save")).toBeTruthy();
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole("button", { name: /reset/i }));
|
||||
});
|
||||
|
||||
expect(result.current.settings.name).toBe(ws.name);
|
||||
expect(result.current.settings.description).toBe(ws.description);
|
||||
expect(result.current.settings.appearanceDirty).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("same-workspace appearance updates", () => {
|
||||
it("syncs an untouched draft when the same workspace receives a remote update", () => {
|
||||
const ws = workspace();
|
||||
const { result, rerender } = renderWithStore([ws]);
|
||||
|
||||
rerender({ ws: { ...ws, name: REMOTE_NAME, description: REMOTE_DESCRIPTION } });
|
||||
|
||||
expect(result.current.name).toBe(REMOTE_NAME);
|
||||
expect(result.current.description).toBe(REMOTE_DESCRIPTION);
|
||||
expect(result.current.appearanceDirty).toBe(false);
|
||||
});
|
||||
|
||||
it("preserves edited fields while syncing untouched fields from a remote update", () => {
|
||||
const ws = workspace();
|
||||
const { result, rerender } = renderWithStore([ws]);
|
||||
act(() => result.current.setName("Local Draft"));
|
||||
|
||||
rerender({ ws: { ...ws, name: REMOTE_NAME, description: REMOTE_DESCRIPTION } });
|
||||
|
||||
expect(result.current.name).toBe("Local Draft");
|
||||
expect(result.current.description).toBe(REMOTE_DESCRIPTION);
|
||||
expect(result.current.appearanceDirty).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -1,14 +1,25 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback, useEffect, useRef, useState } from "react";
|
||||
import {
|
||||
useCallback,
|
||||
useEffect,
|
||||
useRef,
|
||||
useState,
|
||||
type Dispatch,
|
||||
type SetStateAction,
|
||||
} from "react";
|
||||
import type { TFunction } from "i18next";
|
||||
import Image from "@/components/routing/app-image";
|
||||
import { IconUpload, IconDeviceFloppy } from "@tabler/icons-react";
|
||||
import { toast } from "@/lib/toast/sonner";
|
||||
import { Input } from "@kandev/ui/input";
|
||||
import { Switch } from "@kandev/ui/switch";
|
||||
import { Button } from "@kandev/ui/button";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import { useAppStore, useAppStoreApi } from "@/components/state-provider";
|
||||
import { useSettingsSaveContributor } from "@/components/settings/settings-save-provider";
|
||||
import { updateWorkspaceSettings, getWorkspaceSettings } from "@/lib/api/domains/office-api";
|
||||
import { updateWorkspaceAction } from "@/app/actions/workspaces";
|
||||
import type { AppState } from "@/lib/state/store";
|
||||
import type { WorkspaceState } from "@/lib/state/slices/workspace/types";
|
||||
import { ConfigSection } from "./config-section";
|
||||
import { DangerZoneSection } from "./danger-zone-section";
|
||||
@@ -58,24 +69,18 @@ function AppearanceSection({
|
||||
logoPreview,
|
||||
initial,
|
||||
fileInputRef,
|
||||
dirty,
|
||||
saving,
|
||||
onNameChange,
|
||||
onDescriptionChange,
|
||||
onLogoChange,
|
||||
onSave,
|
||||
}: {
|
||||
name: string;
|
||||
description: string;
|
||||
logoPreview: string | null;
|
||||
initial: string;
|
||||
fileInputRef: React.RefObject<HTMLInputElement | null>;
|
||||
dirty: boolean;
|
||||
saving: boolean;
|
||||
onNameChange: (v: string) => void;
|
||||
onDescriptionChange: (v: string) => void;
|
||||
onLogoChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
|
||||
onSave: () => void;
|
||||
}) {
|
||||
const { t } = useTranslation();
|
||||
return (
|
||||
@@ -133,14 +138,6 @@ function AppearanceSection({
|
||||
className="mt-1"
|
||||
/>
|
||||
</div>
|
||||
{dirty && (
|
||||
<div className="flex justify-end pt-2">
|
||||
<Button size="sm" onClick={onSave} disabled={saving} className="cursor-pointer">
|
||||
<IconDeviceFloppy className="h-4 w-4 mr-1.5" />
|
||||
{saving ? t("office:saving") : t("common:save")}
|
||||
</Button>
|
||||
</div>
|
||||
)}
|
||||
</SettingCard>
|
||||
);
|
||||
}
|
||||
@@ -249,6 +246,147 @@ function RecoverySection({
|
||||
|
||||
type Workspace = WorkspaceState["items"][number];
|
||||
|
||||
type WorkspaceStoreApi = {
|
||||
getState: () => Pick<AppState, "workspaces" | "setWorkspaces">;
|
||||
};
|
||||
|
||||
type SaveAppearanceOptions = {
|
||||
activeWorkspace: Workspace | undefined;
|
||||
name: string;
|
||||
description: string;
|
||||
storeApi: WorkspaceStoreApi;
|
||||
nameRef: React.RefObject<string>;
|
||||
descriptionRef: React.RefObject<string>;
|
||||
setName: (v: string) => void;
|
||||
setDescription: (v: string) => void;
|
||||
setSaving: (v: boolean) => void;
|
||||
t: TFunction;
|
||||
};
|
||||
|
||||
function buildSaveAppearanceHandler({
|
||||
activeWorkspace,
|
||||
name,
|
||||
description,
|
||||
storeApi,
|
||||
nameRef,
|
||||
descriptionRef,
|
||||
setName,
|
||||
setDescription,
|
||||
setSaving,
|
||||
t,
|
||||
}: SaveAppearanceOptions) {
|
||||
return async () => {
|
||||
if (!activeWorkspace) return;
|
||||
const trimmedName = name.trim();
|
||||
if (!trimmedName) {
|
||||
toast.error(t("workspaces:workspaceNameIsRequired"));
|
||||
return;
|
||||
}
|
||||
setSaving(true);
|
||||
try {
|
||||
const updated = await updateWorkspaceAction(activeWorkspace.id, {
|
||||
name: trimmedName,
|
||||
description,
|
||||
});
|
||||
// Only echo the server response back into the draft fields if the user
|
||||
// hasn't kept typing since this save started: the PATCH round trip can
|
||||
// take long enough for a newer keystroke to land before the response
|
||||
// does, and unconditionally overwriting the draft here would silently
|
||||
// discard it (refs, not the closed-over name/description, hold the
|
||||
// latest value across that round trip).
|
||||
if (nameRef.current === name) setName(updated.name);
|
||||
if (descriptionRef.current === description) setDescription(updated.description ?? "");
|
||||
// Read the store's current list at save time, not the array captured at
|
||||
// render/handler-build time: an in-flight workspace.created/updated/deleted
|
||||
// WS event can land during the PATCH round trip, and a stale snapshot here
|
||||
// would clobber it with a whole-array replace (setWorkspaces has no merge
|
||||
// semantics). Same idiom as config-chat-agent-section.tsx.
|
||||
const { workspaces: current, setWorkspaces } = storeApi.getState();
|
||||
setWorkspaces(
|
||||
current.items.map((ws) =>
|
||||
ws.id === updated.id
|
||||
? { ...ws, name: updated.name, description: updated.description }
|
||||
: ws,
|
||||
),
|
||||
);
|
||||
toast.success(t("office:appearanceSettingsSaved"));
|
||||
} catch (err) {
|
||||
toast.error(err instanceof Error ? err.message : t("office:failedToSaveSettings"));
|
||||
throw err;
|
||||
} finally {
|
||||
setSaving(false);
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
function usePermissionsState(activeWorkspace: Workspace | undefined) {
|
||||
const { t } = useTranslation();
|
||||
const [approvalNewAgents, setApprovalNewAgents] = useState(true);
|
||||
const [approvalTaskCompletion, setApprovalTaskCompletion] = useState(false);
|
||||
const [approvalSkillChanges, setApprovalSkillChanges] = useState(true);
|
||||
const [origApprovalNewAgents, setOrigApprovalNewAgents] = useState(true);
|
||||
const [origApprovalTaskCompletion, setOrigApprovalTaskCompletion] = useState(false);
|
||||
const [origApprovalSkillChanges, setOrigApprovalSkillChanges] = useState(true);
|
||||
const [savingPermissions, setSavingPermissions] = useState(false);
|
||||
const activeWorkspaceId = activeWorkspace?.id;
|
||||
|
||||
useEffect(() => {
|
||||
if (!activeWorkspaceId) return;
|
||||
void getWorkspaceSettings(activeWorkspaceId)
|
||||
.then((res) => {
|
||||
const s = res.settings;
|
||||
if (s.require_approval_for_new_agents !== undefined) {
|
||||
setApprovalNewAgents(s.require_approval_for_new_agents);
|
||||
setOrigApprovalNewAgents(s.require_approval_for_new_agents);
|
||||
}
|
||||
if (s.require_approval_for_task_completion !== undefined) {
|
||||
setApprovalTaskCompletion(s.require_approval_for_task_completion);
|
||||
setOrigApprovalTaskCompletion(s.require_approval_for_task_completion);
|
||||
}
|
||||
if (s.require_approval_for_skill_changes !== undefined) {
|
||||
setApprovalSkillChanges(s.require_approval_for_skill_changes);
|
||||
setOrigApprovalSkillChanges(s.require_approval_for_skill_changes);
|
||||
}
|
||||
})
|
||||
.catch(() => {});
|
||||
}, [activeWorkspaceId]);
|
||||
|
||||
const handleSavePermissions = useCallback(async () => {
|
||||
if (!activeWorkspace) return;
|
||||
setSavingPermissions(true);
|
||||
try {
|
||||
await updateWorkspaceSettings(activeWorkspace.id, {
|
||||
require_approval_for_new_agents: approvalNewAgents,
|
||||
require_approval_for_task_completion: approvalTaskCompletion,
|
||||
require_approval_for_skill_changes: approvalSkillChanges,
|
||||
});
|
||||
setOrigApprovalNewAgents(approvalNewAgents);
|
||||
setOrigApprovalTaskCompletion(approvalTaskCompletion);
|
||||
setOrigApprovalSkillChanges(approvalSkillChanges);
|
||||
toast.success(t("office:permissionSettingsSaved"));
|
||||
} catch (err) {
|
||||
toast.error(err instanceof Error ? err.message : t("office:failedToSaveSettings"));
|
||||
} finally {
|
||||
setSavingPermissions(false);
|
||||
}
|
||||
}, [activeWorkspace, approvalNewAgents, approvalTaskCompletion, approvalSkillChanges]);
|
||||
|
||||
return {
|
||||
approvalNewAgents,
|
||||
setApprovalNewAgents,
|
||||
approvalTaskCompletion,
|
||||
setApprovalTaskCompletion,
|
||||
approvalSkillChanges,
|
||||
setApprovalSkillChanges,
|
||||
savingPermissions,
|
||||
permissionsDirty:
|
||||
approvalNewAgents !== origApprovalNewAgents ||
|
||||
approvalTaskCompletion !== origApprovalTaskCompletion ||
|
||||
approvalSkillChanges !== origApprovalSkillChanges,
|
||||
handleSavePermissions,
|
||||
};
|
||||
}
|
||||
|
||||
function useRecoveryState(activeWorkspace: Workspace | undefined) {
|
||||
const { t } = useTranslation();
|
||||
const [lookbackHours, setLookbackHours] = useState(24);
|
||||
@@ -294,85 +432,131 @@ function useRecoveryState(activeWorkspace: Workspace | undefined) {
|
||||
};
|
||||
}
|
||||
|
||||
function useSettingsState(activeWorkspace: Workspace | undefined) {
|
||||
type AppearanceBaseline = {
|
||||
id: string | undefined;
|
||||
name: string;
|
||||
description: string;
|
||||
};
|
||||
|
||||
function getAppearanceBaseline(workspace: Workspace | undefined): AppearanceBaseline {
|
||||
return {
|
||||
id: workspace?.id,
|
||||
name: workspace?.name ?? "",
|
||||
description: workspace?.description ?? "",
|
||||
};
|
||||
}
|
||||
|
||||
function reconcileAppearanceDraft(
|
||||
previous: AppearanceBaseline,
|
||||
next: AppearanceBaseline,
|
||||
setName: Dispatch<SetStateAction<string>>,
|
||||
setDescription: Dispatch<SetStateAction<string>>,
|
||||
) {
|
||||
if (next.id !== previous.id) {
|
||||
setName(next.name);
|
||||
setDescription(next.description);
|
||||
return;
|
||||
}
|
||||
|
||||
setName((current) => (current === previous.name ? next.name : current));
|
||||
setDescription((current) => (current === previous.description ? next.description : current));
|
||||
}
|
||||
|
||||
export function useSettingsState(
|
||||
activeWorkspace: Workspace | undefined,
|
||||
storeApi: WorkspaceStoreApi,
|
||||
) {
|
||||
const { t } = useTranslation();
|
||||
const [name, setName] = useState(activeWorkspace?.name ?? "");
|
||||
const [description, setDescription] = useState(activeWorkspace?.description ?? "");
|
||||
const appearance = getAppearanceBaseline(activeWorkspace);
|
||||
const [name, setName] = useState(appearance.name);
|
||||
const [description, setDescription] = useState(appearance.description);
|
||||
const [logoPreview, setLogoPreview] = useState<string | null>(null);
|
||||
const fileInputRef = useRef<HTMLInputElement>(null);
|
||||
const [approvalNewAgents, setApprovalNewAgents] = useState(true);
|
||||
const [approvalTaskCompletion, setApprovalTaskCompletion] = useState(false);
|
||||
const [approvalSkillChanges, setApprovalSkillChanges] = useState(true);
|
||||
const [origApprovalNewAgents, setOrigApprovalNewAgents] = useState(true);
|
||||
const [origApprovalTaskCompletion, setOrigApprovalTaskCompletion] = useState(false);
|
||||
const [origApprovalSkillChanges, setOrigApprovalSkillChanges] = useState(true);
|
||||
const [savingAppearance, setSavingAppearance] = useState(false);
|
||||
const [savingPermissions, setSavingPermissions] = useState(false);
|
||||
const recovery = useRecoveryState(activeWorkspace);
|
||||
const permissions = usePermissionsState(activeWorkspace);
|
||||
const { id: appearanceId, name: appearanceName, description: appearanceDescription } = appearance;
|
||||
const appearanceBaselineRef = useRef(appearance);
|
||||
|
||||
const activeWorkspaceId = activeWorkspace?.id;
|
||||
|
||||
// If the active workspace's identity changes without this page
|
||||
// remounting - e.g. a workspace.deleted WS event elsewhere reassigns
|
||||
// workspaces.activeId while this page stays mounted - the draft must
|
||||
// reset to the new workspace's values. Otherwise the stale draft from
|
||||
// the old workspace looks "dirty" against the new one's name, and Save
|
||||
// would persist the old workspace's data onto the new, unrelated one.
|
||||
// For same-workspace updates, only fields that still match the previous
|
||||
// baseline are refreshed. This keeps remote changes visible without
|
||||
// overwriting a local draft.
|
||||
useEffect(() => {
|
||||
if (!activeWorkspaceId) return;
|
||||
void getWorkspaceSettings(activeWorkspaceId)
|
||||
.then((res) => {
|
||||
const s = res.settings;
|
||||
if (s.require_approval_for_new_agents !== undefined) {
|
||||
setApprovalNewAgents(s.require_approval_for_new_agents);
|
||||
setOrigApprovalNewAgents(s.require_approval_for_new_agents);
|
||||
}
|
||||
if (s.require_approval_for_task_completion !== undefined) {
|
||||
setApprovalTaskCompletion(s.require_approval_for_task_completion);
|
||||
setOrigApprovalTaskCompletion(s.require_approval_for_task_completion);
|
||||
}
|
||||
if (s.require_approval_for_skill_changes !== undefined) {
|
||||
setApprovalSkillChanges(s.require_approval_for_skill_changes);
|
||||
setOrigApprovalSkillChanges(s.require_approval_for_skill_changes);
|
||||
}
|
||||
})
|
||||
.catch(() => {});
|
||||
}, [activeWorkspaceId]);
|
||||
const next = {
|
||||
id: appearanceId,
|
||||
name: appearanceName,
|
||||
description: appearanceDescription,
|
||||
};
|
||||
const previous = appearanceBaselineRef.current;
|
||||
|
||||
reconcileAppearanceDraft(previous, next, setName, setDescription);
|
||||
appearanceBaselineRef.current = next;
|
||||
}, [appearanceDescription, appearanceId, appearanceName]);
|
||||
|
||||
// Latest-value refs so the in-flight save handler (a closure fixed at the
|
||||
// moment Save was clicked) can tell whether the draft has moved on since,
|
||||
// instead of blindly trusting the name/description it captured at submit
|
||||
// time.
|
||||
const nameRef = useRef(name);
|
||||
const descriptionRef = useRef(description);
|
||||
useEffect(() => {
|
||||
nameRef.current = name;
|
||||
}, [name]);
|
||||
useEffect(() => {
|
||||
descriptionRef.current = description;
|
||||
}, [description]);
|
||||
|
||||
const handleLogoChange = (e: React.ChangeEvent<HTMLInputElement>) => {
|
||||
const file = e.target.files?.[0];
|
||||
if (file) setLogoPreview(URL.createObjectURL(file));
|
||||
};
|
||||
|
||||
const handleSaveAppearance = useCallback(async () => {
|
||||
if (!activeWorkspace) return;
|
||||
setSavingAppearance(true);
|
||||
try {
|
||||
await updateWorkspaceSettings(activeWorkspace.id, { name, description });
|
||||
toast.success(t("office:appearanceSettingsSaved"));
|
||||
} catch (err) {
|
||||
toast.error(err instanceof Error ? err.message : t("office:failedToSaveSettings"));
|
||||
} finally {
|
||||
setSavingAppearance(false);
|
||||
}
|
||||
}, [activeWorkspace, name, description]);
|
||||
// useCallback (matching handleSavePermissions/handleSaveRecovery) so the
|
||||
// handler only gets a new identity when an input it actually reads changes.
|
||||
const handleSaveAppearance = useCallback(
|
||||
buildSaveAppearanceHandler({
|
||||
activeWorkspace,
|
||||
name,
|
||||
description,
|
||||
storeApi,
|
||||
nameRef,
|
||||
descriptionRef,
|
||||
setName,
|
||||
setDescription,
|
||||
setSaving: setSavingAppearance,
|
||||
t,
|
||||
}),
|
||||
[activeWorkspace, name, description, storeApi, nameRef, descriptionRef, t],
|
||||
);
|
||||
|
||||
const handleSavePermissions = useCallback(async () => {
|
||||
if (!activeWorkspace) return;
|
||||
setSavingPermissions(true);
|
||||
try {
|
||||
await updateWorkspaceSettings(activeWorkspace.id, {
|
||||
require_approval_for_new_agents: approvalNewAgents,
|
||||
require_approval_for_task_completion: approvalTaskCompletion,
|
||||
require_approval_for_skill_changes: approvalSkillChanges,
|
||||
});
|
||||
setOrigApprovalNewAgents(approvalNewAgents);
|
||||
setOrigApprovalTaskCompletion(approvalTaskCompletion);
|
||||
setOrigApprovalSkillChanges(approvalSkillChanges);
|
||||
toast.success(t("office:permissionSettingsSaved"));
|
||||
} catch (err) {
|
||||
toast.error(err instanceof Error ? err.message : t("office:failedToSaveSettings"));
|
||||
} finally {
|
||||
setSavingPermissions(false);
|
||||
}
|
||||
}, [activeWorkspace, approvalNewAgents, approvalTaskCompletion, approvalSkillChanges]);
|
||||
const appearanceDirty =
|
||||
Boolean(appearanceId) && (name !== appearanceName || description !== appearanceDescription);
|
||||
const appearanceRevision = JSON.stringify({
|
||||
id: appearanceId,
|
||||
name,
|
||||
description,
|
||||
});
|
||||
|
||||
const origName = activeWorkspace?.name ?? "";
|
||||
const origDescription = activeWorkspace?.description ?? "";
|
||||
useSettingsSaveContributor({
|
||||
id: "office-workspace-appearance",
|
||||
order: 10,
|
||||
revision: appearanceRevision,
|
||||
isDirty: appearanceDirty,
|
||||
canSave: Boolean(name.trim()),
|
||||
invalidReason: name.trim() ? undefined : t("workspaces:workspaceNameIsRequired"),
|
||||
save: handleSaveAppearance,
|
||||
discard: (revision) => {
|
||||
if (!Object.is(revision, appearanceRevision)) return;
|
||||
setName(appearanceName);
|
||||
setDescription(appearanceDescription);
|
||||
},
|
||||
});
|
||||
|
||||
return {
|
||||
name,
|
||||
@@ -381,33 +565,23 @@ function useSettingsState(activeWorkspace: Workspace | undefined) {
|
||||
setDescription,
|
||||
logoPreview,
|
||||
fileInputRef,
|
||||
approvalNewAgents,
|
||||
setApprovalNewAgents,
|
||||
approvalTaskCompletion,
|
||||
setApprovalTaskCompletion,
|
||||
approvalSkillChanges,
|
||||
setApprovalSkillChanges,
|
||||
...permissions,
|
||||
...recovery,
|
||||
appearanceDirty: name !== origName || description !== origDescription,
|
||||
permissionsDirty:
|
||||
approvalNewAgents !== origApprovalNewAgents ||
|
||||
approvalTaskCompletion !== origApprovalTaskCompletion ||
|
||||
approvalSkillChanges !== origApprovalSkillChanges,
|
||||
appearanceDirty,
|
||||
savingAppearance,
|
||||
savingPermissions,
|
||||
handleLogoChange,
|
||||
handleSaveAppearance,
|
||||
handleSavePermissions,
|
||||
};
|
||||
}
|
||||
|
||||
export function SettingsContent() {
|
||||
const { t } = useTranslation();
|
||||
const storeApi = useAppStoreApi();
|
||||
const workspaces = useAppStore((s) => s.workspaces);
|
||||
const setWorkspaces = useAppStore((s) => s.setWorkspaces);
|
||||
const setActiveWorkspace = useAppStore((s) => s.setActiveWorkspace);
|
||||
const activeWorkspace = workspaces.items.find((w) => w.id === workspaces.activeId);
|
||||
const s = useSettingsState(activeWorkspace);
|
||||
const s = useSettingsState(activeWorkspace, storeApi);
|
||||
const initial = (s.name || "W").charAt(0).toUpperCase();
|
||||
|
||||
return (
|
||||
@@ -420,12 +594,9 @@ export function SettingsContent() {
|
||||
logoPreview={s.logoPreview}
|
||||
initial={initial}
|
||||
fileInputRef={s.fileInputRef}
|
||||
dirty={s.appearanceDirty}
|
||||
saving={s.savingAppearance}
|
||||
onNameChange={s.setName}
|
||||
onDescriptionChange={s.setDescription}
|
||||
onLogoChange={s.handleLogoChange}
|
||||
onSave={s.handleSaveAppearance}
|
||||
/>
|
||||
</div>
|
||||
|
||||
|
||||
@@ -65,12 +65,24 @@ test.describe("Dashboard", () => {
|
||||
});
|
||||
|
||||
test("update workspace settings", async ({ officeApi, officeSeed }) => {
|
||||
const updated = await officeApi.updateWorkspaceSettings(officeSeed.workspaceId, {
|
||||
name: "E2E Workspace Updated",
|
||||
});
|
||||
expect(updated).toBeDefined();
|
||||
const { settings: originalSettings } = await officeApi.getWorkspaceSettings(
|
||||
officeSeed.workspaceId,
|
||||
);
|
||||
const originalLookbackHours = (originalSettings as { recovery_lookback_hours: number })
|
||||
.recovery_lookback_hours;
|
||||
|
||||
const settings = await officeApi.getWorkspaceSettings(officeSeed.workspaceId);
|
||||
expect(settings).toBeDefined();
|
||||
try {
|
||||
const updated = await officeApi.updateWorkspaceSettings(officeSeed.workspaceId, {
|
||||
recovery_lookback_hours: 42,
|
||||
});
|
||||
expect(updated).toEqual({ ok: true });
|
||||
|
||||
const { settings } = await officeApi.getWorkspaceSettings(officeSeed.workspaceId);
|
||||
expect((settings as { recovery_lookback_hours: number }).recovery_lookback_hours).toBe(42);
|
||||
} finally {
|
||||
await officeApi.updateWorkspaceSettings(officeSeed.workspaceId, {
|
||||
recovery_lookback_hours: originalLookbackHours,
|
||||
});
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,8 +1,46 @@
|
||||
import { test, expect } from "../../fixtures/office-fixture";
|
||||
import { waitForHttp } from "../../helpers/causal-waits";
|
||||
|
||||
test.describe("Workspace settings UI", () => {
|
||||
test("settings page shows danger zone", async ({ testPage, officeSeed: _ }) => {
|
||||
await testPage.goto("/office/workspace/settings");
|
||||
await expect(testPage.getByText(/danger zone/i)).toBeVisible({ timeout: 10_000 });
|
||||
});
|
||||
|
||||
test("renaming the workspace persists and updates the sidebar", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
officeSeed,
|
||||
}) => {
|
||||
const renamed = "E2E Workspace Renamed";
|
||||
let originalName: string | undefined;
|
||||
try {
|
||||
await testPage.goto("/office/workspace/settings");
|
||||
const nameInput = testPage.getByPlaceholder("Workspace name");
|
||||
await expect(nameInput).toBeVisible({ timeout: 10_000 });
|
||||
originalName = await nameInput.inputValue();
|
||||
await nameInput.fill(renamed);
|
||||
const saveBar = testPage.getByTestId("settings-floating-save");
|
||||
const saveButton = saveBar.getByRole("button", { name: "Save changes" });
|
||||
await expect(saveBar).toBeVisible();
|
||||
|
||||
const workspaceSaved = waitForHttp(
|
||||
testPage,
|
||||
"PATCH",
|
||||
new RegExp(`/api/v1/workspaces/${officeSeed.workspaceId}$`),
|
||||
);
|
||||
await saveButton.click();
|
||||
await workspaceSaved;
|
||||
|
||||
await expect(saveBar).toHaveAttribute("data-status", "saved");
|
||||
await expect(testPage.getByTestId("sidebar-workspace-trigger")).toContainText(renamed);
|
||||
|
||||
await testPage.reload();
|
||||
await expect(nameInput).toHaveValue(renamed);
|
||||
} finally {
|
||||
await apiClient.updateWorkspace(officeSeed.workspaceId, {
|
||||
name: originalName ?? "E2E Workspace",
|
||||
});
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -625,8 +625,6 @@ export function getWorkspaceSettings(workspaceId: string, options?: ApiRequestOp
|
||||
export function updateWorkspaceSettings(
|
||||
workspaceId: string,
|
||||
data: {
|
||||
name?: string;
|
||||
description?: string;
|
||||
require_approval_for_new_agents?: boolean;
|
||||
require_approval_for_task_completion?: boolean;
|
||||
require_approval_for_skill_changes?: boolean;
|
||||
|
||||
@@ -48,6 +48,7 @@ import {
|
||||
import { RoutineDetailRoute } from "./office-routine-client-routes";
|
||||
import { TooltipProvider } from "@kandev/ui/tooltip";
|
||||
import { Trans, useTranslation } from "react-i18next";
|
||||
import { SettingsSaveProvider } from "@/components/settings/settings-save-provider";
|
||||
|
||||
type RouteRenderer = () => React.ReactNode;
|
||||
|
||||
@@ -62,7 +63,11 @@ const OFFICE_ROUTES: Record<string, RouteRenderer> = {
|
||||
"/office/workspace/costs": () => <CostsPageClient initialCostSummary={null} />,
|
||||
"/office/workspace/skills": () => <SkillsPageClient initialSkills={[]} />,
|
||||
"/office/workspace/routing": () => <ProviderRoutingPage />,
|
||||
"/office/workspace/settings": () => <SettingsPage />,
|
||||
"/office/workspace/settings": () => (
|
||||
<SettingsSaveProvider placement="content">
|
||||
<SettingsPage />
|
||||
</SettingsSaveProvider>
|
||||
),
|
||||
"/office/workspace/settings/sync": () => <SyncPage />,
|
||||
"/office/workspace/org": () => <OrgPage />,
|
||||
};
|
||||
@@ -103,7 +108,7 @@ export function OfficeRoutes({ pathname }: { pathname: string }) {
|
||||
|
||||
return (
|
||||
<TooltipProvider>
|
||||
<div className="flex h-full min-h-0 flex-col">
|
||||
<div className="relative flex h-full min-h-0 flex-col">
|
||||
<OfficeShell routePath={normalizedPathname}>
|
||||
{renderOfficeRoute(normalizedPathname)}
|
||||
</OfficeShell>
|
||||
|
||||
Reference in New Issue
Block a user