From b7f0de1d8d9dde4e41de52ca06e6f0534c4d5aca Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 07:55:23 +0000 Subject: [PATCH] Lock banner titles every state that opens it; lock vocabulary declared once (#5024) --- .changeset/metadata-lock-state-5024.md | 25 +++ .../AuditPanel.lockState.test.tsx | 11 +- .../ResourceEditPage.lockBanner.test.tsx | 162 ++++++++++++++++++ .../views/metadata-admin/ResourceEditPage.tsx | 63 ++++++- .../src/views/metadata-admin/i18n.ts | 25 ++- packages/data-objectstack/src/index.ts | 1 + .../data-objectstack/src/metadata-client.ts | 33 +++- 7 files changed, 303 insertions(+), 17 deletions(-) create mode 100644 .changeset/metadata-lock-state-5024.md create mode 100644 packages/app-shell/src/views/metadata-admin/ResourceEditPage.lockBanner.test.tsx diff --git a/.changeset/metadata-lock-state-5024.md b/.changeset/metadata-lock-state-5024.md new file mode 100644 index 000000000..785458cbf --- /dev/null +++ b/.changeset/metadata-lock-state-5024.md @@ -0,0 +1,25 @@ +--- +'@object-ui/data-objectstack': patch +'@object-ui/app-shell': patch +--- + +The metadata lock banner can no longer render an amber, padlocked box with no +title, and the ADR-0010 §3.6 lock vocabulary is declared once instead of three +times (objectui#5024). + +`MetadataLayered.lock` and `MetadataAuditEntry.lockState` each spelled the four +states out by hand, 42 lines apart in one file, compared by no gate. They are now +one exported `MetadataLockState` — derived from `GetMetaItemLayeredResponseSchema`'s +`z.enum` in `@objectstack/spec`, which already owns this vocabulary, so the copies +were restating a schema rather than filling a gap. + +The user-visible half is the banner. Its title was three independent `&&` branches +with no fallback, while the switch that opens the banner is true for any non-`none` +value — so a lock state outside the four opened the box and left the headline +empty. That is reachable without a fifth state ever being added here: +`MetadataClient.layered()` casts the wire value through unchecked, so a newer +server reaches this banner as-is. Measured, not assumed — feeding `no-publish` +through the page rendered the padlock, the border and an empty title. The title is +now a keyed lookup with a loud fallback that names the unrecognised token, so a +fifth state fails `type-check` here and, if one arrives from a server anyway, the +operator reads a sentence instead of a blank box. diff --git a/packages/app-shell/src/views/metadata-admin/AuditPanel.lockState.test.tsx b/packages/app-shell/src/views/metadata-admin/AuditPanel.lockState.test.tsx index 390ce1d98..aa47fd02b 100644 --- a/packages/app-shell/src/views/metadata-admin/AuditPanel.lockState.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/AuditPanel.lockState.test.tsx @@ -19,10 +19,13 @@ * * The state list below is bound to `MetadataAuditEntry['lockState']` itself * rather than spelled freely, so a fifth lock state is covered by these cases - * the moment the union gains it. Unlike objectui#4982's overlay scopes there is - * no `@objectstack/spec` enum to read at runtime — this union is hand-written in - * `packages/data-objectstack` — hence the `satisfies`-checked key trick instead - * of a `.options` array. (The compile-time half of the coverage is + * the moment the vocabulary gains it. That field is `MetadataLockState` since + * objectui#5024, derived from `GetMetaItemLayeredResponseSchema`'s `z.enum` — + * so unlike what this comment first claimed, objectui#4982's overlay scopes are + * NOT the odd one out: the spec declares this enum too (17.1.0). The + * `satisfies`-checked key trick is kept over a runtime `.options` array because + * it fails at `type-check` rather than at run time, which is the earlier of the + * two; nothing about the derivation forces the choice either way. (The compile-time half of the coverage is * `LOCK_STATE_ZH` in `./i18n`, whose key type is that union: a new state with no * zh-CN label fails `type-check` before it can reach the column.) */ diff --git a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.lockBanner.test.tsx b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.lockBanner.test.tsx new file mode 100644 index 000000000..54a2b8ffb --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.lockBanner.test.tsx @@ -0,0 +1,162 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The lock banner must never render a headline-less amber box — objectui#5024. + * + * ## The defect + * + * The banner's title was three independent `&&` branches with no `else` and no + * fallback: + * + * {layered?.lock === 'full' && t('engine.edit.lockFull', locale)} + * {layered?.lock === 'no-overlay' && t('engine.edit.lockNoOverlay', locale)} + * {layered?.lock === 'no-delete' && t('engine.edit.lockNoDelete', locale)} + * + * while the switch that OPENS the banner is `layered?.lock && lock !== 'none'` + * — true for *any* non-`none` value. So a lock state outside the ADR-0010 §3.6 + * four opens the amber box, draws the padlock and the border, and leaves the + * title `
` empty: a locked-looking banner that never says what is locked + * or why. + * + * ## Why this is reachable today, not only "the day a fifth state lands" + * + * The card framed it as dormant, on the reading that both hand-written unions + * list the same four values. The unions are not the gate. `MetadataClient. + * layered()` passes the wire value through with an unchecked cast — + * + * ...(body.lock !== undefined ? { lock: body.lock as MetadataLayered['lock'] } : {}), + * + * — over a `res.json()` body. There is no Zod parse, no allowlist, no default + * on this path. The union constrains what this repo may *write*; it constrains + * nothing about what a server may *send*. A backend that grows a fifth state + * reaches this banner with zero code change here, which is why the fix has to + * be a runtime fallback and not only a compile-time exhaustiveness check: a + * `satisfies` assertion is satisfied by the current four either way and would + * have left the blank banner exactly as it was. + * + * ## What this suite pins + * + * The four known states keep their existing sentences (this half passes before + * the fix — it is the control that says a red fifth-value case is the defect + * and not a broken harness), and an out-of-vocabulary state renders a loud, + * non-empty fallback that also surfaces the raw token so the state a server + * actually sent is diagnosable from the screen. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; + +const objectDef = { + name: 'showcase_account', + label: 'Account', + fields: [{ name: 'title', label: 'Title', type: 'text' }], +}; + +const layeredImpl = { current: vi.fn() }; + +const mockClient = { + list: vi.fn(async () => []), + listDrafts: vi.fn(async () => []), + get: vi.fn(async () => null), + getDraft: vi.fn(async () => null), + references: vi.fn(async () => []), + layered: vi.fn(async (...args: unknown[]) => layeredImpl.current(...args)), +}; + +vi.mock('./useMetadata', async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + useMetadataClient: () => mockClient, + useMetadataTypes: () => ({ + loading: false, + error: null, + entries: [ + { + type: 'object', + name: 'object', + label: 'Object', + allowOrgOverride: false, + allowRuntimeCreate: true, + schema: { + type: 'object', + properties: { name: { type: 'string' }, label: { type: 'string' } }, + }, + }, + ], + }), + }; +}); + +import { MetadataResourceEditPage } from './ResourceEditPage'; + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +/** + * Mount the editor over a layered envelope carrying `lock`. `provenance: 'org'` + * keeps the separate installed-package notice out of the way so the assertions + * below read the LOCK banner and nothing else. + */ +async function renderWithLock(lock: unknown) { + layeredImpl.current = vi.fn(async () => ({ + code: { ...objectDef, _packageId: 'com.example.base', _provenance: 'org' }, + overlay: null, + overlayScope: null, + effective: objectDef, + provenance: 'org', + packageId: 'com.example.base', + editable: true, + lock, + })); + render( + + + , + ); + await waitFor(() => expect(mockClient.layered).toHaveBeenCalled()); + return waitFor(() => { + const el = screen.queryByTestId('lock-banner-title'); + expect(el).not.toBeNull(); + return el as HTMLElement; + }); +} + +describe('ResourceEditPage lock banner — every state that opens the banner also titles it (#5024)', () => { + // The control half: these pass before the fix as well as after. If the + // out-of-vocabulary case below goes red while these stay green, the red is + // the defect rather than a harness that cannot mount the page. + it.each([ + ['full', 'This item is locked and cannot be edited or deleted.'], + ['no-overlay', 'This item is locked and cannot be edited.'], + ['no-delete', 'This item is locked and cannot be deleted.'], + ])('known state %s keeps its existing sentence', async (lock, sentence) => { + const el = await renderWithLock(lock); + expect(el.textContent?.trim()).toBe(sentence); + }); + + it('an out-of-vocabulary state from the wire still gets a title', async () => { + // The whole defect in one assertion: pre-fix this element exists (the + // banner opened) and is EMPTY. + const el = await renderWithLock('no-publish'); + expect(el.textContent?.trim()).not.toBe(''); + }); + + it('an out-of-vocabulary state names the raw token it could not read', async () => { + // "Loudly", per the triage ruling: a generic "this is locked" that hides + // WHICH state arrived would leave an operator with nothing to report. + const el = await renderWithLock('no-publish'); + expect(el.textContent).toContain('no-publish'); + }); + + it('a non-string lock value cannot crash or blank the banner', async () => { + // `layered()` casts whatever JSON held; a malformed body is the same class + // of unchecked input as an unknown state name. + const el = await renderWithLock(42); + expect(el.textContent?.trim()).not.toBe(''); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx index 48b5bb08a..542e3993e 100644 --- a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx +++ b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx @@ -82,6 +82,7 @@ import { import { Empty, EmptyTitle, EmptyDescription } from '@object-ui/components'; import type { MetadataLayered, + MetadataLockState, MetadataReference, } from '@object-ui/data-objectstack'; import { PageShell } from './PageShell.js'; @@ -127,6 +128,62 @@ import { describeIssuePath } from './issuePath.js'; import { buildCreateModeBody } from './createBody.js'; import { errorCodeIs, errorCodeIsAnyOf } from '@object-ui/types'; +/** + * ADR-0010 §3.6 lock state -> the lock banner's headline sentence. + * + * Keyed on `MetadataLockState` — which derives from `packages/spec`'s own + * `z.enum` — so a fifth state added upstream fails `type-check` HERE, naming + * the label that is missing. That is the strictness the audit panel's + * `LOCK_STATE_ZH` has had since objectui#5004 and this banner did not: its + * title used to be three independent `&&` branches with no `else`. + * + * `Exclude<…, 'none'>` because `none` never banners — `isLocked` gates the + * whole box on `lock && lock !== 'none'`. Typing the record over exactly the + * states that CAN reach the screen keeps the two rules from drifting apart, + * and still fails on a state added to the union. + */ +const LOCK_BANNER_TITLE_KEY: Record, string> = { + 'no-overlay': 'engine.edit.lockNoOverlay', + 'no-delete': 'engine.edit.lockNoDelete', + full: 'engine.edit.lockFull', +}; + +/** + * The banner's headline for whatever `lock` ACTUALLY arrived — including a + * value the lookup above has never heard of (objectui#5024). + * + * The compile-time half cannot be the whole fix. `MetadataLockState` types what + * this repo may WRITE; it constrains nothing about what a server may SEND, + * because `MetadataClient.layered()` casts the wire value in unchecked: + * + * ...(body.lock !== undefined ? { lock: body.lock as MetadataLayered['lock'] } : {}), + * + * over a raw `res.json()` body — no parse, no allowlist, no default. So a + * back end that grows a fifth state reaches this banner with no code change + * here at all. Measured rather than assumed: feeding `no-publish` through this + * page opened the amber box, drew the padlock and the border, and left the + * title `
` empty. An exhaustive `satisfies` alone would have type-checked + * green over that exact render. + * + * Hence a sentence for the unrecognised value, carrying the raw token: the + * operator who meets this is the only person able to report which state their + * server actually sent, and a generic "this is locked" would take that away. + * `String(lock)` rather than a cast — the same unchecked path can hand us a + * number or an object, and this must not throw on the way to explaining itself. + */ +function lockBannerTitle( + lock: MetadataLayered['lock'], + locale: string | undefined, +): string { + if ( + typeof lock === 'string' && + Object.prototype.hasOwnProperty.call(LOCK_BANNER_TITLE_KEY, lock) + ) { + return t(LOCK_BANNER_TITLE_KEY[lock as Exclude], locale); + } + return tFormat('engine.edit.lockUnknown', locale, { state: String(lock) }); +} + /** * Metadata types whose canvas IS the primary create-time authoring * surface, so we render the preview/inspector split during create @@ -1990,10 +2047,8 @@ function MetadataResourceEditPageImpl({
-
- {layered?.lock === 'full' && t('engine.edit.lockFull', locale)} - {layered?.lock === 'no-overlay' && t('engine.edit.lockNoOverlay', locale)} - {layered?.lock === 'no-delete' && t('engine.edit.lockNoDelete', locale)} +
+ {lockBannerTitle(layered?.lock, locale)}
{lockReason &&
{lockReason}
} {layered?.lockDocsUrl && ( diff --git a/packages/app-shell/src/views/metadata-admin/i18n.ts b/packages/app-shell/src/views/metadata-admin/i18n.ts index fe14f40c1..a6f983a42 100644 --- a/packages/app-shell/src/views/metadata-admin/i18n.ts +++ b/packages/app-shell/src/views/metadata-admin/i18n.ts @@ -282,6 +282,13 @@ const ENGINE_STRINGS_EN: Record = { 'engine.edit.lockFull': 'This item is locked and cannot be edited or deleted.', 'engine.edit.lockNoOverlay': 'This item is locked and cannot be edited.', 'engine.edit.lockNoDelete': 'This item is locked and cannot be deleted.', + // objectui#5024 — the headline for a lock state this console has no + // sentence for. Reachable without any change here: `layered()` casts the + // wire value through unchecked, so a newer server can send a fifth state + // today. Names the raw token because the operator who sees it is the only + // one who can report which state their server sent. + 'engine.edit.lockUnknown': + 'This item is locked, but this console does not recognise the lock state \u2018{state}\u2019 — it may come from a newer server. Some operations will be blocked.', 'engine.edit.history': 'History', 'engine.edit.auditTab': 'Audit log', 'engine.edit.auditCount': 'events', @@ -2101,6 +2108,8 @@ const ENGINE_STRINGS_ZH: Record = { 'engine.edit.lockFull': '该元数据已锁定，不可编辑或删除。', 'engine.edit.lockNoOverlay': '该元数据已锁定，不可编辑。', 'engine.edit.lockNoDelete': '该元数据已锁定，不可删除。', + 'engine.edit.lockUnknown': + '该元数据已锁定，但当前控制台无法识别锁状态“{state}”——它可能来自更新版本的服务端。部分操作将被阻止。', 'engine.edit.history': '历史', 'engine.edit.auditTab': '审计日志', 'engine.edit.auditCount': '条记录', @@ -4244,13 +4253,15 @@ const LAYER_SCOPE_ZH: Record, string> = { * zh-CN labels for the ADR-0010 §3.6 four-state metadata lock, keyed by the * producer field the only consumer actually renders. * - * The key type is `MetadataAuditEntry['lockState']` — the hand-written union in - * `packages/data-objectstack/src/metadata-client.ts`, not a `@objectstack/spec` - * enum, because this repo owns that union today. (Whether the spec should own - * the lock vocabulary is a separate question; it is deliberately not answered - * here.) Binding the keys means a fifth lock state added to the union stops this - * record from compiling and names the label that is missing, instead of the - * column silently shipping a raw English token. + * The key type is `MetadataAuditEntry['lockState']`, which since objectui#5024 + * is `MetadataLockState` — derived from `GetMetaItemLayeredResponseSchema`'s + * `z.enum` in `@objectstack/spec`. It was a hand-written union in + * `packages/data-objectstack/src/metadata-client.ts` when this comment was + * first written, on the belief that the spec had no enum to derive from; the + * spec does declare one (17.1.0), so the question of ownership this comment + * used to leave open is answered upstream, not here. Binding the keys means a + * fifth lock state stops this record from compiling and names the label that is + * missing, instead of the column silently shipping a raw English token. * * That silent path is objectui#5004, and it was total rather than partial: the * table used to hold `draft` / `locked` / `published` / `none` — a draft-status diff --git a/packages/data-objectstack/src/index.ts b/packages/data-objectstack/src/index.ts index ff036cc22..9f23a64ab 100644 --- a/packages/data-objectstack/src/index.ts +++ b/packages/data-objectstack/src/index.ts @@ -4687,6 +4687,7 @@ export type { MetadataError, MetadataValidationIssue, MetadataLayered, + MetadataLockState, MetadataOverlayScope, MetadataReference, MetadataDiagnostics, diff --git a/packages/data-objectstack/src/metadata-client.ts b/packages/data-objectstack/src/metadata-client.ts index 64e866fb3..06c96afd2 100644 --- a/packages/data-objectstack/src/metadata-client.ts +++ b/packages/data-objectstack/src/metadata-client.ts @@ -265,6 +265,35 @@ export interface MetadataDeleteOptions extends MetadataClientSaveOptions { */ export type MetadataOverlayScope = GetMetaItemLayeredResponse['overlayScope']; +/** + * ADR-0010 §3.6 — the four-state metadata protection lock + * (`none` / `no-overlay` / `no-delete` / `full`), read from + * `GetMetaItemLayeredResponseSchema`'s own `z.enum` rather than spelled out + * again here (objectui#5024). + * + * It used to be written out twice in this file, 42 lines apart: once as + * {@link MetadataLayered.lock} (optional) and once as + * {@link MetadataAuditEntry.lockState} (nullable), identical in every other + * respect and compared by no gate. A fifth state added to one would have left + * the other compiling — the same "declared N times, diffed by nothing" failure + * objectui#4972 and objectui#4984 record for other vocabularies. + * + * Deriving beats a local alias the two merely share. The producer of these + * values is the framework, and `packages/spec` already declares the vocabulary, + * so the copies were restating a schema that existed rather than filling a gap. + * The card that reported this recorded the opposite — "`@objectstack/spec` 也没 + * 有对应的 `z.enum` 可派生" — and so did the audit panel's neighbouring comment; + * both predate the enum, which ships in `@objectstack/spec` 17.1.0. This is the + * same treatment {@link MetadataOverlayScope} above already gets, and it closes + * the cross-repo half of the drift, not just the in-repo half. + * + * ⚠️ This types what this repo may WRITE. It does NOT constrain what a server + * may SEND: {@link MetadataClient.layered} casts the wire value through + * unchecked, so every reader must still handle a value outside these four. The + * lock banner in `ResourceEditPage` is the worked example. + */ +export type MetadataLockState = GetMetaItemLayeredResponse['lock']; + /** * Layered view of a metadata item — the body of * `GET /meta/:type/:name/layers` (`GetMetaItemLayeredResponseSchema`). @@ -295,7 +324,7 @@ export interface MetadataLayered { _diagnostics?: MetadataDiagnostics; // ── ADR-0010 Phase 1 — protection envelope ── /** 4-state lock: `none` / `no-overlay` / `no-delete` / `full`. */ - lock?: 'none' | 'no-overlay' | 'no-delete' | 'full'; + lock?: MetadataLockState; /** Human-readable reason for the lock (tooltip text). */ lockReason?: string; /** Which layer set the lock: artifact / package / overlay / env-forced. */ @@ -337,7 +366,7 @@ export interface MetadataAuditEntry { /** Machine-readable reason code (`item_locked`, `ok`, …). */ code: string; /** Effective lock at the moment of the attempt. */ - lockState: 'none' | 'no-overlay' | 'no-delete' | 'full' | null; + lockState: MetadataLockState | null; /** True when admin forced the write through despite the lock. */ lockOverridden: boolean; /** Request-id for trace correlation (if propagated). */