diff --git a/.changeset/settings-locked-key-tolerant-read.md b/.changeset/settings-locked-key-tolerant-read.md new file mode 100644 index 0000000000..aeb379fca9 --- /dev/null +++ b/.changeset/settings-locked-key-tolerant-read.md @@ -0,0 +1,26 @@ +--- +"@object-ui/console": patch +--- + +fix(settings): read the locked key from `error.details`, tolerating both wire shapes — objectstack#4224 + +`SettingsView.onSave` rendered the `SETTINGS_LOCKED` toast from +`err.payload.error.key`. That key was a SIBLING of `code`/`message` inside +`error`, a position `ApiErrorSchema` never declared — it reached the console only +because the schema is a plain `z.object` and silently strips what it does not +declare, so nothing ever failed to flag it. objectstack#4224 moves it into +`error.details`, the slot the contract does declare. + +This is the console's half, and it ships **first**: the read is now +`error.details?.key ?? error.key`, so the toast keeps naming the locked key +against servers on either side of that change rather than degrading to +`Locked by environment: undefined` during the window where the two repos are on +different versions. The fallback can go once the oldest supported server carries +the fix. + +Also stops interpolating a missing key: when neither position carries one the +toast now reads `Locked by environment` rather than appending `undefined`. + +This was the only in-console reader of the four keys objectstack#4224 relocated +(`namespace`, `key`, `reason`, `fields`) — a repo-wide grep for the other three +finds no consumer. diff --git a/apps/console/src/pages/settings/SettingsView.tsx b/apps/console/src/pages/settings/SettingsView.tsx index 16116dcbd6..5280f95f6a 100644 --- a/apps/console/src/pages/settings/SettingsView.tsx +++ b/apps/console/src/pages/settings/SettingsView.tsx @@ -15,6 +15,7 @@ import { getIcon } from '../../utils/getIcon'; import { SettingsField } from './SettingsField'; import { getSettingsNamespace, + lockedKeyOf, runSettingsAction, saveSettingsNamespace, } from './api'; @@ -128,8 +129,11 @@ export function SettingsView() { setDraft({}); toast.success('Settings saved'); } catch (err: any) { - if (err?.payload?.error?.code === 'SETTINGS_LOCKED') { - toast.error(`Locked by environment: ${err.payload.error.key}`); + const apiError = err?.payload?.error; + if (apiError?.code === 'SETTINGS_LOCKED') { + // `lockedKeyOf` reads both wire positions — see its note (objectstack#4224). + const key = lockedKeyOf(apiError); + toast.error(key ? `Locked by environment: ${key}` : 'Locked by environment'); } else { toast.error(err?.message ?? 'Save failed'); } diff --git a/apps/console/src/pages/settings/api.test.ts b/apps/console/src/pages/settings/api.test.ts new file mode 100644 index 0000000000..fa8e7ebd59 --- /dev/null +++ b/apps/console/src/pages/settings/api.test.ts @@ -0,0 +1,64 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * `lockedKeyOf` — the two wire positions of a `SETTINGS_LOCKED` key + * (objectstack#4224). + * + * Why this is pinned rather than left as an inline `??`: it is a COMPAT read, + * and the failure mode of a compat read is that someone deletes the half they + * did not know was load-bearing. Both branches are asserted here, so removing + * either one is a red test that names the server version it breaks — which is + * also the signal for when the old branch may legitimately go. + * + * The old position (`error.key`) was never declared by `ApiErrorSchema`; it + * reached the console only because the schema is a plain `z.object` and strips + * undeclared keys instead of rejecting them. That is exactly why no test on + * either side of the wire ever caught it. + */ + +import { describe, it, expect } from 'vitest'; +import { lockedKeyOf } from './api'; + +describe('lockedKeyOf', () => { + it('reads the declared slot — a server carrying objectstack#4224', () => { + expect( + lockedKeyOf({ + code: 'SETTINGS_LOCKED', + message: "Setting 'branding.workspace_name' is locked (locked-by-env).", + details: { namespace: 'branding', key: 'workspace_name', reason: 'locked-by-env' }, + }), + ).toBe('workspace_name'); + }); + + it('falls back to the pre-#4224 sibling — a server without it', () => { + expect( + lockedKeyOf({ + code: 'SETTINGS_LOCKED', + message: "Setting 'branding.workspace_name' is locked (locked-by-env).", + namespace: 'branding', + key: 'workspace_name', + reason: 'locked-by-env', + }), + ).toBe('workspace_name'); + }); + + it('prefers the declared slot when a body somehow carries both', () => { + expect(lockedKeyOf({ key: 'old_key', details: { key: 'new_key' } })).toBe('new_key'); + }); + + it('yields undefined rather than a string when no position carries a key', () => { + // The toast branches on this: no key means "Locked by environment" rather + // than "Locked by environment: undefined". + expect(lockedKeyOf({ code: 'SETTINGS_LOCKED', message: 'Locked.' })).toBeUndefined(); + expect(lockedKeyOf({ details: {} })).toBeUndefined(); + expect(lockedKeyOf({ details: null })).toBeUndefined(); + expect(lockedKeyOf({ key: '' })).toBeUndefined(); + expect(lockedKeyOf({ key: 42 })).toBeUndefined(); + expect(lockedKeyOf(undefined)).toBeUndefined(); + expect(lockedKeyOf('not an object')).toBeUndefined(); + }); +}); diff --git a/apps/console/src/pages/settings/api.ts b/apps/console/src/pages/settings/api.ts index 90a4af98f4..82783928ba 100644 --- a/apps/console/src/pages/settings/api.ts +++ b/apps/console/src/pages/settings/api.ts @@ -28,6 +28,35 @@ const jsonHeaders = (): HeadersInit => ({ Accept: 'application/json', }); +/** + * The env-locked key named by a `SETTINGS_LOCKED` error, read from whichever + * position the serving version puts it in (objectstack#4224). + * + * It used to arrive as `error.key` — a SIBLING of `code`/`message` that + * `ApiErrorSchema` never declared. That body was accepted only because the + * schema is a plain `z.object` and strips undeclared keys rather than rejecting + * them, so nothing on either side ever flagged it. objectstack#4224 moves it to + * `error.details.key`, the slot the contract does declare. + * + * Declared home first, old position second, so the console names the key against + * servers on either side of that change instead of rendering `undefined` for the + * duration of the window. Drop the second read once the oldest supported server + * carries the fix. + * + * Lives here rather than inline in the view because this module already owns + * what the server's error body looks like (`jsonOrThrow` is what builds + * `err.payload`), and because a compat shim that no test exercises is a compat + * shim that gets deleted by the next person who reads only one of the two + * shapes. + */ +export function lockedKeyOf(apiError: unknown): string | undefined { + if (!apiError || typeof apiError !== 'object') return undefined; + const e = apiError as { key?: unknown; details?: { key?: unknown } | null }; + const declared = e.details && typeof e.details === 'object' ? e.details.key : undefined; + const found = declared ?? e.key; + return typeof found === 'string' && found.length > 0 ? found : undefined; +} + export async function listSettingsManifests(): Promise { const res = await fetch(BASE, { credentials: 'include', headers: jsonHeaders() }); return jsonOrThrow(res);