From 76c49408cd392593ebdc3f29557045b1f68288ef Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 19:14:53 +0000 Subject: [PATCH] fix(app-shell): name the field and the Formula (CEL) editor in the retired `formula` 422 diagnostic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Card context: objectui#6526, adjudicated option B — keep the objectui#6043 migration path, make the blocked state actionable. A stored object can carry the retired `formula` alias inside a formula field; `FieldSchema` refuses it by name and that hard 422 blocks every later save. The client gate's diagnostic now appends a pointer to the spec's own rejection: it names the field carrying the key and points at the field inspector's Formula (CEL) editor, where one edit commits `expression` and clears the alias. Presentation only — verdict, issue set and paths unchanged; fires only for formula-type fields, where the inspector renders that editor (objectui#4306). The spec message is appended to, never replaced, so other retired keys riding the same `unrecognized_keys` issue keep their own rename prescriptions. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- .changeset/6526-formula-422-diagnostic.md | 17 +++ ...clientValidation.retiredFormulaKey.test.ts | 143 ++++++++++++++++++ .../views/metadata-admin/clientValidation.ts | 79 +++++++++- 3 files changed, 235 insertions(+), 4 deletions(-) create mode 100644 .changeset/6526-formula-422-diagnostic.md create mode 100644 packages/app-shell/src/views/metadata-admin/clientValidation.retiredFormulaKey.test.ts diff --git a/.changeset/6526-formula-422-diagnostic.md b/.changeset/6526-formula-422-diagnostic.md new file mode 100644 index 0000000000..96da1b8ae5 --- /dev/null +++ b/.changeset/6526-formula-422-diagnostic.md @@ -0,0 +1,17 @@ +--- +'@object-ui/app-shell': patch +--- + +The object designer's client-side 422 on a draft carrying the retired `formula` +field key is now actionable (objectui#6526, adjudicated option B). The spec's +rejection at `fields.` gains an appended pointer that names the field and +names the destination: select the field and make one edit in its Formula (CEL) +editor, which commits the value to `expression` and clears the retired alias. + +Presentation only — the verdict, issue set and paths are unchanged, and nothing +about what the gate accepts changes. The migration path itself is untouched: +`RETIRED_FIELD_KEYS` still does not strip `formula` (objectui#6043's ruling), +and the object stays unsaveable until the author makes that one edit — the +ruling's accepted cost, now with a signposted way out. The pointer fires only +for `formula`-type fields, where the inspector actually renders that editor +(objectui#4306); any other field type keeps the bare spec message. diff --git a/packages/app-shell/src/views/metadata-admin/clientValidation.retiredFormulaKey.test.ts b/packages/app-shell/src/views/metadata-admin/clientValidation.retiredFormulaKey.test.ts new file mode 100644 index 0000000000..f1db674fcb --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/clientValidation.retiredFormulaKey.test.ts @@ -0,0 +1,143 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The retired `formula` alias diagnostic (objectui#6526, adjudicated option B). + * + * A stored object can carry a legacy `formula` key inside a formula field — + * the Field Designer's textarea wrote it until objectui#6043 retired the + * control. `FieldSchema` refuses the key by name, and that hard 422 blocks + * every later save of the object. The ruling keeps the migration path (never + * strip; see `object-fields-io`'s `RETIRED_FIELD_KEYS` note) and makes the + * diagnostic actionable instead: it must NAME the field carrying the key and + * POINT at the "Formula (CEL)" editor in the field inspector, where one edit + * commits `expression` and clears the alias. + * + * The pins here assert the message names BOTH. A pin that only asserts "a + * rejection occurred" passes on the pre-objectui#6526 behaviour — the bare + * spec message with no destination — and proves nothing. + */ + +import { describe, it, expect } from 'vitest'; +import { validateMetadataDraft } from './clientValidation'; + +const draftWith = (fields: unknown) => ({ name: 'invoice', label: 'Invoice', fields }); + +/** The destination the pointer must name — the inspector's editor label. */ +const POINTER = /Formula \(CEL\)/; + +describe('validateMetadataDraft — retired `formula` alias on an object draft (objectui#6526)', () => { + it('names the field and points at the Formula (CEL) editor', async () => { + const res = await validateMetadataDraft( + 'object', + draftWith({ + amount: { type: 'formula', label: 'Amount', formula: 'record.price * 2' }, + price: { type: 'number', label: 'Price' }, + }), + ); + expect(res.ok).toBe(false); + const issue = res.issues.find((i) => i.path === 'fields.amount'); + expect(issue).toBeTruthy(); + // Names the FIELD in the message itself, not only in the path. + expect(issue!.message).toMatch(/Field `amount` carries the retired `formula` key/); + // Names the DESTINATION: the field inspector's Formula (CEL) editor. + expect(issue!.message).toMatch(POINTER); + // Says what the one edit does — commits `expression`, clears the alias. + expect(issue!.message).toMatch(/commits the value to `expression`/); + }); + + it('gives the same pointer on the edit door, where a stored body arrives', async () => { + const res = await validateMetadataDraft( + 'object', + draftWith({ amount: { type: 'formula', label: 'Amount', formula: '1 + 1' } }), + undefined, + { mode: 'edit' }, + ); + expect(res.ok).toBe(false); + const issue = res.issues.find((i) => i.path === 'fields.amount'); + expect(issue).toBeTruthy(); + expect(issue!.message).toMatch(/Field `amount`/); + expect(issue!.message).toMatch(POINTER); + }); + + it('appends to the spec message — the contract voice survives (AGENTS.md #0.1)', async () => { + const res = await validateMetadataDraft( + 'object', + draftWith({ amount: { type: 'formula', label: 'Amount', formula: '1 + 1' } }), + ); + const issue = res.issues.find((i) => i.path === 'fields.amount'); + expect(issue).toBeTruthy(); + // The spec's own rejection text still opens the message. + expect(issue!.message).toMatch(/^Unrecognized key\(s\) on this field: `formula`/); + expect(issue!.message).toMatch(POINTER); + }); + + it('keeps the disclosure for other retired keys riding the same issue', async () => { + // A pre-objectui#6041 stored body can carry `referenceTo` alongside + // `formula` in ONE `keys` array; its rename prescription must survive. + const res = await validateMetadataDraft( + 'object', + draftWith({ + amount: { type: 'formula', label: 'Amount', formula: '1 + 1', referenceTo: 'account' }, + }), + ); + const issue = res.issues.find((i) => i.path === 'fields.amount'); + expect(issue).toBeTruthy(); + expect(issue!.message).toMatch(/referenceTo/); + expect(issue!.message).toMatch(POINTER); + }); + + it('does not point a non-formula field at an editor that will not render', async () => { + // The inspector renders the Formula (CEL) editor only while the field IS + // a formula (objectui#4306 ruling) — a pointer here would name a + // destination that does not exist. The spec's rejection stands alone. + const res = await validateMetadataDraft( + 'object', + draftWith({ note: { type: 'text', label: 'Note', formula: 'record.x' } }), + ); + expect(res.ok).toBe(false); + const issue = res.issues.find((i) => i.path === 'fields.note'); + // Positive control in the same query shape: the rejection is still there… + expect(issue).toBeTruthy(); + expect(issue!.message).toMatch(/Unrecognized key\(s\) on this field: `formula`/); + // …and it is the bare spec message, no pointer. + expect(issue!.message).not.toMatch(POINTER); + }); + + it('does not fire on an unrecognized key that is not `formula`', async () => { + const res = await validateMetadataDraft( + 'object', + draftWith({ amount: { type: 'formula', label: 'Amount', expression: '1 + 1', bogusKey: true } }), + ); + expect(res.ok).toBe(false); + const issue = res.issues.find((i) => i.path === 'fields.amount'); + // Positive control in the same query shape: the rejection is still there… + expect(issue).toBeTruthy(); + expect(issue!.message).toMatch(/Unrecognized key\(s\) on this field: `bogusKey`/); + // …and no formula pointer rides on it. + expect(issue!.message).not.toMatch(POINTER); + }); + + it('is presentation only: same verdict, same paths as the raw spec parse', async () => { + const draft = draftWith({ + amount: { type: 'formula', label: 'Amount', formula: '1 + 1' }, + price: { type: 'number', label: 'Price', bogusKey: true }, + }); + const { ObjectSchema } = await import('@objectstack/spec/data'); + const raw = (ObjectSchema as { safeParse: (v: unknown) => { success: boolean; error?: { issues: Array<{ path: Array }> } } }).safeParse(draft); + expect(raw.success).toBe(false); + const rawPaths = raw.error!.issues.map((i) => i.path.join('.')).sort(); + expect(rawPaths.length).toBeGreaterThan(0); + const res = await validateMetadataDraft('object', draft); + expect(res.ok).toBe(false); + expect(res.issues.map((i) => i.path).sort()).toEqual(rawPaths); + }); + + it('stays green once the field is migrated (expression, no alias)', async () => { + const res = await validateMetadataDraft( + 'object', + draftWith({ amount: { type: 'formula', label: 'Amount', expression: '1 + 1' } }), + ); + expect(res.ok).toBe(true); + expect(res.issues).toEqual([]); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/clientValidation.ts b/packages/app-shell/src/views/metadata-admin/clientValidation.ts index add1fa93fa..2963641584 100644 --- a/packages/app-shell/src/views/metadata-admin/clientValidation.ts +++ b/packages/app-shell/src/views/metadata-admin/clientValidation.ts @@ -42,6 +42,12 @@ type ZodLikeIssue = { * issue and buries every member's real diagnostics here. */ errors?: ZodLikeIssue[][]; + /** + * Present only on `unrecognized_keys`: the offending key names, verbatim. + * One issue carries EVERY unrecognized key of its object, so a legacy body + * can list several retired aliases in a single `keys` array. + */ + keys?: string[]; }; type ZodLikeSchema = { @@ -797,6 +803,64 @@ async function validateObjectFieldRules(draft: unknown): Promise`), the same hard `422 + * INVALID_METADATA` that blocks EVERY later save of that object. The + * adjudicated way out (objectui#6526, upholding objectui#6043) is NOT to + * strip the key — `object-fields-io`'s `RETIRED_FIELD_KEYS` note records how + * a strip destroys the authored expression — but to make the blocked state + * actionable: NAME the field, and POINT at the one surface that migrates the + * value properly. That surface is `ObjectFieldInspector`'s "Formula (CEL)" + * editor (`designer.field.formula`): the legacy value seeds it + * (`def.expression ?? def.formula`) and the first edit commits `expression` + * and clears the alias. The object staying unsaveable until that edit is the + * ruling's accepted cost; this pointer is what makes the cost payable. + * + * The pointer is APPENDED, never substituted: the spec's message is the + * contract's voice (AGENTS.md #0.1), and it carries the disclosure for any + * OTHER unrecognized key riding the same issue — a pre-objectui#6041 body can + * list `referenceTo` alongside `formula` in one `keys` array, and that key's + * own rename prescription must survive. + * + * Fires only when the field IS a `formula` field: the inspector renders the + * editor only for that type (the objectui#4306 ruling — a verdict with no + * on-screen editor to fix it wedges the author), so for any other type the + * pointer would name a destination that does not render, and the spec's + * message stands alone. + */ +const RETIRED_FORMULA_KEY = 'formula'; + +function retiredFormulaKeyPointer(fieldName: string): string { + return ( + `Field \`${fieldName}\` carries the retired \`${RETIRED_FORMULA_KEY}\` key. ` + + `To migrate: select the field in the object designer and make one edit in its ` + + `Formula (CEL) editor — that commits the value to \`expression\`, clears the ` + + `retired key, and the object saves again.` + ); +} + +function annotateRetiredFormulaKeyIssues(issues: ZodLikeIssue[], draft: unknown): ZodLikeIssue[] { + return issues.map((issue) => { + if (issue.code !== 'unrecognized_keys') return issue; + const path = issue.path ?? []; + if (path.length !== 2 || path[0] !== 'fields' || typeof path[1] !== 'string') return issue; + if (!issue.keys?.includes(RETIRED_FORMULA_KEY)) return issue; + const def = valueAtPath(draft, path) as { type?: unknown } | null | undefined; + if (!def || def.type !== 'formula') return issue; + return { ...issue, message: `${issue.message} ${retiredFormulaKeyPointer(path[1])}` }; + }); +} + // Keyed by mode AND type — `view` resolves to a different schema per mode, so // caching by type alone would hand the create gate whichever mode asked first. const SCHEMA_CACHE = new Map(); @@ -924,11 +988,18 @@ export async function validateMetadataDraft( } if (rawIssues.length === 0 && celIssues.length === 0) return { ok: true, issues: [] }; - // Presentation only — see `expandViewIssues`. `ok` is already `false` here - // and `rawIssues` is already final; this only decides what gets RENDERED for + // Presentation only — see `expandViewIssues` and + // `annotateRetiredFormulaKeyIssues`. `ok` is already `false` here and + // `rawIssues` is already final; this only decides what gets RENDERED for // each of them, so the verdict cannot move (pinned by the parity test in - // `clientValidation.viewDiagnostics.test.ts`). - const renderable = type === 'view' ? expandViewIssues(rawIssues, [], draft) : rawIssues; + // `clientValidation.viewDiagnostics.test.ts`, and for `object` by the one + // in `clientValidation.retiredFormulaKey.test.ts`). + const renderable = + type === 'view' + ? expandViewIssues(rawIssues, [], draft) + : type === 'object' + ? annotateRetiredFormulaKeyIssues(rawIssues, draft) + : rawIssues; const issues: SchemaFormIssue[] = [ ...renderable.map((issue) => ({ path: (issue.path ?? []).map((seg) => String(seg)).join('.'),