diff --git a/.changeset/referenceto-carryover-8896.md b/.changeset/referenceto-carryover-8896.md new file mode 100644 index 0000000000..9d38a7b931 --- /dev/null +++ b/.changeset/referenceto-carryover-8896.md @@ -0,0 +1,26 @@ +--- +"@object-ui/plugin-designer": patch +"@object-ui/app-shell": patch +"@object-ui/types": patch +--- + +Keep a relationship target stored only as the retired `referenceTo` spelling (objectui#8896) + +Two field-IO sites deleted the retired `referenceTo` key without keeping its +value, so an object whose lookup or master-detail target survived only under the +pre-objectui#6041 spelling lost the target on the way through: + +- The Field Designer's carried-through half (fields whose stored type the + designer cannot author, objectui#8060) re-emits the stored document verbatim + with no read door in front of it. The strip took the target and the relationship + guard then refused the whole object's save — including a save the author + triggered by editing an entirely different field, on a page that renders the + offending field read-only, so its "Pick the target object" advice named a + control that does not exist there. +- The object designer's single read door for `draft.fields` deleted the target on + load, leaving the target editor empty and committing the loss on the next save. + +Both sites now lift the value onto the spec spelling `reference` before dropping +the retired key. The retired key still never reaches the wire, a live `reference` +is never overwritten by a stale legacy value, and a field with no usable target +under either spelling is still refused. diff --git a/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.referenceCarryover-8896.test.ts b/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.referenceCarryover-8896.test.ts new file mode 100644 index 0000000000..dc31b2b953 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.referenceCarryover-8896.test.ts @@ -0,0 +1,165 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#8896 — the metadata-admin read door loses a relationship target + * stored only under the retired `referenceTo` spelling. + * + * ## The claim this file falsifies, and where it is written + * + * `RETIRED_FIELD_KEYS`' own docblock in `object-fields-io.ts`: + * + * > Nothing is lost on the way out: where the spec has a spelling for the + * > concept it is a SEPARATE key (`reference`, `system`) that is NOT stripped + * > and rides through untouched, which is what lets the designer read it back. + * + * and the tombstone registry's entry for the key + * (`types/src/internal/retired-field-keys.ts`): + * + * > The strip loses nothing — every write path re-emits the designer's target + * > under `reference`, and the read door's writers never emit the retired + * > spelling. + * + * Both hold only when the draft ALSO carries the spec spelling. A draft holding + * the target ONLY as `referenceTo` has it deleted on load with nothing left + * behind: `readFields` is the single read door for `draft.fields` across the + * whole object designer, and `writeFields` writes each def back verbatim, so + * the loss is committed on the next save. `ObjectFieldInspector`'s target + * editor reads `def.reference` and renders EMPTY, and every reader downstream + * of this door reads the canonical key alone — `resolveReferenceTo` was + * deliberately reduced to `def.reference` by objectui#6837's ruling that + * protocol normalization belongs on the server and the readers just execute + * the protocol. + * + * ## What the repair is, and the two things it is NOT + * + * The retired KEY is still stripped — it must be: `FieldSchema` refuses + * `referenceTo` by name, so carrying it out is the hard 422 the strip exists + * to prevent. What changes is that the VALUE is recovered into the spec + * spelling first, which is the same "recover at the door, read canonically + * everywhere else" shape objectui#6837 settled on. + * + * ⛔ NOT a per-reader fallback arm. objectui#6837 deleted those on a maintainer + * ruling and this file does not bring one back — the pins below assert on what + * comes OUT of the door, and `resolveReferenceTo` keeps reading `reference` + * alone. + * + * ⛔ NOT a generic `specEquivalent` migration driven off the registry. The + * registry says so itself ("Documentation for the reader, NEVER an instruction + * to migrate a value mechanically"), because objectui#6043 refused exactly that + * for `formula`, whose value is a LANGUAGE and not a name. + */ + +import { describe, it, expect } from 'vitest'; +import { FieldSchema } from '@objectstack/spec/data'; +import { readFields, writeFields, RETIRED_FIELD_KEYS } from './object-fields-io'; +import { resolveReferenceTo } from '../inspectors/useDatasetFields'; + +/** Round-trip a `draft.fields` value the way every designer write path does. */ +function roundTrip(fields: unknown) { + return writeFields(readFields(fields)); +} + +const unrecognizedKeys = (result: ReturnType): string[] => + result.success + ? [] + : result.error.issues + .filter((i) => i.code === 'unrecognized_keys') + .flatMap((i) => (i as unknown as { keys: string[] }).keys); + +describe('the instrument', () => { + it('the installed `FieldSchema` refuses `referenceTo` by name and accepts `reference`', () => { + // Without this the recovery below could be "fixed" by simply carrying the + // retired key through, which is the 422 this door exists to prevent. + expect(unrecognizedKeys(FieldSchema.safeParse({ type: 'lookup', label: 'L', referenceTo: 'account' }))) + .toContain('referenceTo'); + expect(FieldSchema.safeParse({ type: 'lookup', label: 'L', reference: 'account' }).success).toBe(true); + }); + + it('`referenceTo` is one of the keys this door strips', () => { + expect([...RETIRED_FIELD_KEYS]).toContain('referenceTo'); + }); +}); + +describe('objectui#8896 · a target stored only as `referenceTo` survives the read door', () => { + it('record-shaped draft: the value arrives under the spec spelling, the retired key does not', () => { + const out = roundTrip({ + owner_id: { type: 'lookup', label: 'Owner', referenceTo: 'account' }, + }) as Record>; + + expect(out.owner_id).toEqual({ type: 'lookup', label: 'Owner', reference: 'account' }); + expect('referenceTo' in out.owner_id).toBe(false); + }); + + it('array-shaped draft: the same, through the other branch of the door', () => { + const out = roundTrip([ + { name: 'owner_id', type: 'lookup', label: 'Owner', referenceTo: 'account' }, + ]) as Array>; + + expect(out[0]).toEqual({ name: 'owner_id', type: 'lookup', label: 'Owner', reference: 'account' }); + expect('referenceTo' in out[0]).toBe(false); + }); + + it('the def the designer reads carries the target, so the inspector renders it', () => { + // The consumer-side statement of the same fact: every reader downstream of + // this door reads `reference` alone, by objectui#6837's ruling. + const view = readFields({ owner_id: { type: 'master_detail', label: 'Parent', referenceTo: 'invoice' } }); + expect(view.entries[0].def.reference).toBe('invoice'); + expect(resolveReferenceTo(view.entries[0].def)).toBe('invoice'); + }); + + it('what comes out of the door parses through the real FieldSchema', () => { + const out = roundTrip({ + owner_id: { type: 'lookup', label: 'Owner', referenceTo: 'account' }, + }) as Record>; + + const result = FieldSchema.safeParse(out.owner_id); + expect(unrecognizedKeys(result)).toEqual([]); + expect(result.success).toBe(true); + }); +}); + +/** + * ⭐ The firing controls. Each one fails on a build that "fixed" this by + * recovering unconditionally, and the first two also fail on a build that + * simply stopped stripping the key. + */ +describe('objectui#8896 · firing controls', () => { + it('the spec spelling WINS — a stale legacy value never overwrites a live target', () => { + // A pre-objectui#6041 designer wrote both keys; the canonical one is the + // one the author has been editing ever since. + const out = roundTrip({ + owner_id: { type: 'lookup', label: 'Owner', reference: 'account', referenceTo: 'stale_legacy' }, + }) as Record>; + + expect(out.owner_id.reference).toBe('account'); + expect('referenceTo' in out.owner_id).toBe(false); + }); + + it('a field with NO usable target gains no invented one', () => { + // `unrecognized_keys` fires on the key's PRESENCE, so an empty retired key + // is real and stored. Recovering it would smuggle a target that names no + // object past every gate downstream. + const out = roundTrip({ + a: { type: 'lookup', label: 'A', referenceTo: '' }, + b: { type: 'lookup', label: 'B', referenceTo: ' ' }, + c: { type: 'text', label: 'C' }, + }) as Record>; + + expect('reference' in out.a).toBe(false); + expect('reference' in out.b).toBe(false); + expect('reference' in out.c).toBe(false); + expect(RETIRED_FIELD_KEYS.filter((k) => k in out.a)).toEqual([]); + expect(RETIRED_FIELD_KEYS.filter((k) => k in out.b)).toEqual([]); + }); + + it('the OTHER retired keys are still dropped with nothing left behind', () => { + // The recovery is keyed to ONE tombstone, not to `specEquivalent` in + // general: `isSystem` has a spec equivalent too and is deliberately not + // recovered — its strip IS the whole write half of objectui#6044. + const out = roundTrip({ + code: { type: 'text', label: 'Code', indexed: true, isSystem: true }, + }) as Record>; + + expect(out.code).toEqual({ type: 'text', label: 'Code' }); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.retiredKeys.test.ts b/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.retiredKeys.test.ts index a685914511..142b3dd04f 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.retiredKeys.test.ts +++ b/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.retiredKeys.test.ts @@ -51,6 +51,32 @@ const SAMPLE: Record<(typeof RETIRED_FIELD_KEYS)[number], unknown> = { isSystem: true, }; +/** + * What is left on `{ type: 'lookup', label: 'Owner' }` after the door has seen + * that key — the falsification half of each case below, and the one place the + * three keys stop being interchangeable (objectui#8896). + * + * `indexed` and `isSystem` leave NOTHING behind, for two different recorded + * reasons: the field-level index flag built no index at all (objectui#4644 — + * the concept moved to the object's `indexes[]`, so there is no field-level + * value to keep), and the system flag's spec spelling `system` is a separate + * key the draft either carries or does not, so the strip IS the whole write + * half of objectui#6044 and re-stamping it would invent a flag the author + * never set. + * + * `referenceTo` is the one whose VALUE the draft may hold nowhere else. It is + * the same CONCEPT as `reference` and the same KIND of value — a bare object + * name — so deleting it destroyed the only copy of a relationship target and + * left the designer's target editor empty. The door keeps that value under the + * spec spelling now; the retired key is dropped exactly as the other two are, + * which is what every `key in out` assertion below is stating. + */ +const RESIDUE: Record<(typeof RETIRED_FIELD_KEYS)[number], Record> = { + indexed: {}, + referenceTo: { reference: 'account' }, + isSystem: {}, +}; + describe('object-fields-io · retired FieldSchema keys (objectui#4644, objectui#6519)', () => { it('names exactly the three keys this door strips', () => { // The list is derived from the tombstone registry (objectui#6527: @@ -77,8 +103,10 @@ describe('object-fields-io · retired FieldSchema keys (objectui#4644, objectui# }) as Record>; expect(key in out.owner_id).toBe(false); - // Falsification: keyed to the tombstone, not a blanket unknown-key purge. - expect(out.owner_id).toEqual({ type: 'lookup', label: 'Owner' }); + // Falsification: keyed to the tombstone, not a blanket unknown-key purge + // — and, for the one key whose value has nowhere else to live, the value + // survives under the spec spelling (see {@link RESIDUE}). + expect(out.owner_id).toEqual({ type: 'lookup', label: 'Owner', ...RESIDUE[key] }); }); it(`drops \`${key}\` from an array-shaped draft on round-trip`, () => { @@ -87,7 +115,7 @@ describe('object-fields-io · retired FieldSchema keys (objectui#4644, objectui# ]) as Array>; expect(key in out[0]).toBe(false); - expect(out[0]).toEqual({ name: 'owner_id', type: 'lookup', label: 'Owner' }); + expect(out[0]).toEqual({ name: 'owner_id', type: 'lookup', label: 'Owner', ...RESIDUE[key] }); }); } @@ -102,7 +130,9 @@ describe('object-fields-io · retired FieldSchema keys (objectui#4644, objectui# >; expect(RETIRED_FIELD_KEYS.filter((k) => k in out.owner_id)).toEqual([]); - expect(out.owner_id).toEqual({ type: 'lookup', label: 'Owner' }); + // Every retired key is gone; the lookup's target is not, because it is a + // VALUE and not a key — the asymmetry {@link RESIDUE} records. + expect(out.owner_id).toEqual({ type: 'lookup', label: 'Owner', reference: 'account' }); }); it('drops falsy values too — the key itself is what the parse rejects', () => { diff --git a/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.ts b/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.ts index 6fd3399e24..680d326598 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.ts +++ b/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.ts @@ -52,6 +52,14 @@ export type Shape = 'array' | 'record'; * concept it is a SEPARATE key (`reference`, `system`) that is NOT stripped * and rides through untouched, which is what lets the designer read it back. * + * ⚠️ That sentence was UNCONDITIONAL and, for one key, measurably false + * (objectui#8896): it holds only where the draft ALSO carries the spec + * spelling. A draft holding a relationship target ONLY as `referenceTo` had it + * deleted here with nothing left behind. {@link stripRetiredFieldKeys} now + * keeps that one value under `reference` before dropping the key, so the + * sentence is true as written — see there for why the repair is keyed to this + * key alone and is NOT a `specEquivalent` migration. + * * ── The two registry keys this door deliberately does NOT strip ── * * `formula` — RULED, objectui#6526 option B (2026-08-27): this door reads @@ -87,12 +95,75 @@ export type Shape = 'array' | 'record'; */ export const RETIRED_FIELD_KEYS = retiredFieldKeysFor('metadataAdminFieldsReadDoor'); -/** Drop {@link RETIRED_FIELD_KEYS} from one field definition. */ +/** + * The pre-objectui#6041 spelling of a relationship target, and the spec key it + * was renamed to. + * + * Named as a PAIR here rather than derived from the tombstone's + * `specEquivalent`: the registry is explicit that `specEquivalent` is + * "Documentation for the reader, NEVER an instruction to migrate a value + * mechanically", because objectui#6043 refused exactly that for `formula` — + * whose value is a LANGUAGE, so a blind rename launders non-CEL text into a + * formula that parses green and evaluates to null. A relationship target is a + * bare object NAME under both spellings, which is what makes reading it under + * either one a read rather than a migration, and that fact is specific to this + * one key. + */ +const RETIRED_REFERENCE_KEY = 'referenceTo'; +const SPEC_REFERENCE_KEY = 'reference'; + +/** Does this value NAME a target object? Blank and non-string name none. */ +function isUsableTarget(value: unknown): value is string { + return typeof value === 'string' && value.trim() !== ''; +} + +/** + * Drop {@link RETIRED_FIELD_KEYS} from one field definition, keeping a + * relationship target the draft holds ONLY under the retired spelling + * (objectui#8896). + * + * ## The claim this repairs — it is this module's own, one paragraph up + * + * {@link RETIRED_FIELD_KEYS}' note said "Nothing is lost on the way out: where + * the spec has a spelling for the concept it is a SEPARATE key (`reference`, + * `system`) that is NOT stripped and rides through untouched". That holds only + * when the draft ALSO carries the spec spelling. A draft holding the target + * only as `referenceTo` had it deleted here with nothing left behind — and + * this is the SINGLE read door for `draft.fields` across the whole object + * designer, with `writeFields` writing each def back verbatim, so the loss + * committed on the next save. `ObjectFieldInspector`'s target editor reads + * `def.reference` and rendered empty; so did every other reader, because + * objectui#6837's ruling ("protocol normalization belongs on the server, the + * front end just executes the protocol") deleted the per-reader legacy arms. + * + * Recovering HERE is that ruling's own shape rather than an exception to it: + * the door normalizes, the readers stay canonical. `reference_to` is the + * ingestion choke points' business (`normalizeSchemaReferenceKeys`, which also + * stamps a second key `FieldSchema` refuses) and is deliberately not touched by + * this door, which serves a WRITE path. + * + * ## Three things this does not do + * + * ⛔ The retired key still never survives — `FieldSchema` refuses it by name, + * which is the whole reason this door strips. + * ⛔ Never overwrites a live `reference`: the spec spelling is what the author + * has been editing, and a stale legacy value beside it is the older truth. + * ⛔ Never invents a target. `unrecognized_keys` fires on the key's PRESENCE, + * so `referenceTo: ''` is a real stored state; recovering it would hand every + * gate downstream a target that names no object. + */ function stripRetiredFieldKeys(def: Record): Record { const present = RETIRED_FIELD_KEYS.filter((k) => k in def); if (present.length === 0) return def; const next = { ...def }; for (const k of present) delete next[k]; + if ( + (present as readonly string[]).includes(RETIRED_REFERENCE_KEY) + && !isUsableTarget(next[SPEC_REFERENCE_KEY]) + && isUsableTarget(def[RETIRED_REFERENCE_KEY]) + ) { + next[SPEC_REFERENCE_KEY] = def[RETIRED_REFERENCE_KEY]; + } return next; } diff --git a/packages/plugin-designer/src/MetadataFieldsPage.carriedThroughReference-8896.test.tsx b/packages/plugin-designer/src/MetadataFieldsPage.carriedThroughReference-8896.test.tsx new file mode 100644 index 0000000000..b2875bcd17 --- /dev/null +++ b/packages/plugin-designer/src/MetadataFieldsPage.carriedThroughReference-8896.test.tsx @@ -0,0 +1,260 @@ +/** + * 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. + */ + +/** + * objectui#8896 — the CARRIED-THROUGH half loses a target stored only as + * `referenceTo`, and the loss refuses the whole save. + * + * ## The path, and why no read door stands in front of it + * + * objectui#8060 split the stored `fields` map: a field whose stored `type` + * this designer cannot author (`master_detail`, `vector`, the other 20) is + * PRESERVED and re-emitted straight from the stored document as + * `carryOver(keep.raw)`. `toDesignerField` — the read door — is on the + * DESIGNABLE branch only, so nothing on this path ever looks at the stored + * field's keys before `carryOver` strips them. + * + * `carryOver` strips `referenceTo` (the registry's `metadataFieldsPageCarryOver` + * site, objectui#6041/#6527), because `FieldSchema` refuses that spelling BY + * NAME and re-emitting it is the hard 422 that blocks every later save. So for + * a stored `master_detail` whose target survives ONLY under the retired + * spelling, the strip takes the target with it, and objectui#7714's + * `assertRelationshipTargetPresent` then refuses the ENTIRE save: + * + * stored { parent_id: { type: 'master_detail', label: 'Parent', + * referenceTo: 'invoice' } } + * author relabels an UNRELATED `name` field + * => puts = [] (no PUT at all) + * => "[MetadataFieldsPage] cannot save the field `parent_id`: a + * `master_detail` field needs a `reference` naming the object it links + * to, … Pick the target object, or change the field to a + * non-relationship type." + * + * ⭐ The consequence, which is what makes this worth a card: the author edited + * a DIFFERENT field. A preserved field is rendered READ-ONLY on this page by + * design (objectui#8060), so "Pick the target object" names a control that does + * not exist here — every later save of that object is refused from this page + * with no way out of it from this page. + * + * ## The repair, and the two things it deliberately does NOT do + * + * ⛔ NOT loosening `assertRelationshipTargetPresent`. The gate is right: a + * `master_detail` really does need a target, and a genuinely target-less one + * must stay refused — pinned below as this file's firing control, so a build + * that simply removed the gate reds instead of passing. + * + * ⛔ NOT emitting `referenceTo`. That spelling is `unrecognized_keys` against + * the installed `FieldSchema`, so carrying the KEY out to the wire re-creates + * the 422 the strip exists to prevent. The VALUE is recovered into the spec + * spelling `reference`; the retired key still never reaches the wire. + * + * This is the same shape the designable half already uses — read the target + * wherever the stored document put it, write it under the one spelling the spec + * declares — and it is the SAME reader: both halves now go through + * `storedRelationshipTarget`, the function objectui#8058 added and argued. One + * spelling rule, stated once, for the two branches of one writer. + * + * ⚠️ Scope: this file drives the PRESERVED branch only, with a stored type + * (`master_detail`) `DESIGNER_FIELD_TYPES` does not carry. The designable + * half's read door landed as objectui#8058 and is not re-opened here — its pins + * in `MetadataFieldsPage.specKeyReference.test.tsx` still own that state and + * must stay green. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { act, cleanup, render, screen, waitFor } from '@testing-library/react'; +import { FieldSchema } from '@objectstack/spec/data'; +import { MetadataClient } from '@object-ui/data-objectstack'; +import { DESIGNER_FIELD_TYPES } from '@object-ui/types'; +import type { DesignerFieldDefinition } from '@object-ui/types'; + +interface RecordedDesignerProps { + objectName: string; + fields: DesignerFieldDefinition[]; + onFieldsChange?: (fields: DesignerFieldDefinition[]) => void; + readOnly?: boolean; +} + +let designerProps: RecordedDesignerProps | null = null; + +vi.mock('./FieldDesigner', () => ({ + FieldDesigner: (props: RecordedDesignerProps) => { + designerProps = props; + return null; + }, +})); + +import { MetadataFieldsPage } from './MetadataFieldsPage'; + +let puts: Array> = []; + +function json(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' }, + }); +} + +function clientServing(fields: Record>): MetadataClient { + return new MetadataClient({ + baseUrl: 'http://localhost:3000', + fetch: (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input); + const method = (init?.method ?? 'GET').toUpperCase(); + if (method === 'PUT') { + puts.push(JSON.parse(String(init?.body ?? '{}')) as Record); + return json({ success: true, name: 'probe_widget' }); + } + if (/\/meta\/object\/probe_widget(\?|$)/.test(url)) { + return json({ + type: 'object', + name: 'probe_widget', + item: { name: 'probe_widget', label: 'Widget', fields }, + lock: 'none', + provenance: 'org', + editable: true, + }); + } + return json({ items: [] }); + }) as unknown as typeof fetch, + }); +} + +async function renderServing(fields: Record>) { + render(); + await waitFor(() => expect(designerProps).not.toBeNull()); +} + +/** A plain relabel of an UNRELATED field — the author never touches the lookup. */ +async function relabelUnrelated() { + await act(async () => { + designerProps!.onFieldsChange!( + designerProps!.fields.map((f) => (f.name === 'name' ? { ...f, label: 'Full name' } : f)), + ); + }); +} + +function savedFields(): Record> { + return puts[puts.length - 1].fields as Record>; +} + +const unrecognizedKeys = (result: ReturnType): string[] => + result.success + ? [] + : result.error.issues + .filter((i) => i.code === 'unrecognized_keys') + .flatMap((i) => (i as unknown as { keys: string[] }).keys); + +beforeEach(() => { + puts = []; + designerProps = null; +}); + +afterEach(() => { + cleanup(); + designerProps = null; +}); + +describe('the instrument', () => { + it('`master_detail` is a stored type this designer cannot author — so it takes the PRESERVED branch', () => { + // If this ever became authorable the whole file would silently move to the + // designable branch and stop testing what it is named for. + expect((DESIGNER_FIELD_TYPES as readonly string[]).includes('master_detail')).toBe(false); + }); + + it('the installed `FieldSchema` refuses `referenceTo` by name and accepts `reference`', () => { + expect(unrecognizedKeys(FieldSchema.safeParse({ type: 'master_detail', label: 'P', referenceTo: 'invoice' }))) + .toContain('referenceTo'); + expect(FieldSchema.safeParse({ type: 'master_detail', label: 'P', reference: 'invoice' }).success).toBe(true); + }); +}); + +describe('objectui#8896 · a preserved `master_detail` whose target survives only as `referenceTo`', () => { + const STORED = { + name: { type: 'text', label: 'Name', required: true }, + parent_id: { type: 'master_detail', label: 'Parent', referenceTo: 'invoice' }, + }; + + it('saves — the unrelated edit goes out instead of being refused', async () => { + await renderServing(STORED); + await relabelUnrelated(); + await waitFor(() => expect(puts).toHaveLength(1)); + expect(screen.queryByTestId('metadata-fields-page-error')).toBeNull(); + }); + + it('carries the target through, under the spec spelling and never the retired one', async () => { + await renderServing(STORED); + await relabelUnrelated(); + await waitFor(() => expect(puts).toHaveLength(1)); + + const fields = savedFields(); + expect(fields.parent_id.reference).toBe('invoice'); + expect('referenceTo' in fields.parent_id).toBe(false); + // Falsification: the field is still carried through whole — the stored type + // survives, and the author's actual edit landed. + expect(fields.parent_id.type).toBe('master_detail'); + expect(fields.parent_id.label).toBe('Parent'); + expect(fields.name.label).toBe('Full name'); + }); + + it('every field it PUTs parses through the real FieldSchema', async () => { + await renderServing(STORED); + await relabelUnrelated(); + await waitFor(() => expect(puts).toHaveLength(1)); + + for (const [name, def] of Object.entries(savedFields())) { + const result = FieldSchema.safeParse(def); + expect(unrecognizedKeys(result), `field \`${name}\` emitted a refused key`).toEqual([]); + expect(result.success, `field \`${name}\` did not parse`).toBe(true); + } + }); +}); + +/** + * ⭐ The firing control. Without it every assertion above would also pass on a + * build that simply deleted `assertRelationshipTargetPresent` — which is the + * one repair this card forbids. + */ +describe('objectui#8896 · firing control — a genuinely target-less preserved field is STILL refused', () => { + it('refuses by name and issues no PUT when no spelling carries a target', async () => { + await renderServing({ + name: { type: 'text', label: 'Name', required: true }, + parent_id: { type: 'master_detail', label: 'Parent' }, + }); + await relabelUnrelated(); + + await waitFor(() => + expect(screen.getByTestId('metadata-fields-page-error').textContent).toMatch( + /cannot save the field `parent_id`: a `master_detail` field needs a `reference`/, + ), + ); + expect(puts).toEqual([]); + }); + + it('refuses a preserved field whose retired spelling holds an unusable value', async () => { + // The carry-through adopts what the document HOLDS and judges nothing — the + // same division of labour the designable half has: `storedRelationshipTarget` + // decides the SPELLING, `assertRelationshipTargetPresent` decides whether the + // value can be a target. So a whitespace-only retired target arrives as the + // emitted `reference` and is refused on the guard's own blank branch, by + // name and before the request, rather than being smuggled through as "a + // target was found". + await renderServing({ + name: { type: 'text', label: 'Name', required: true }, + parent_id: { type: 'master_detail', label: 'Parent', referenceTo: ' ' }, + }); + await relabelUnrelated(); + + await waitFor(() => + expect(screen.getByTestId('metadata-fields-page-error').textContent).toMatch( + /cannot save the field `parent_id`.*whitespace names no object/s, + ), + ); + expect(puts).toEqual([]); + }); +}); diff --git a/packages/plugin-designer/src/MetadataFieldsPage.tsx b/packages/plugin-designer/src/MetadataFieldsPage.tsx index 681f73f63c..397759dc3b 100644 --- a/packages/plugin-designer/src/MetadataFieldsPage.tsx +++ b/packages/plugin-designer/src/MetadataFieldsPage.tsx @@ -292,15 +292,18 @@ function toDesignerField(name: string, raw: ServerFieldSchema): DesignerFieldDef * with no target at all. Restating the conclusion without the read door is how * it went wrong the first time. * - * ⚠️ And it is cost-free on the DESIGNABLE half only — the half that has a read - * door. {@link toFieldsMap} re-emits the fields this designer cannot author - * through `carryOver(keep.raw)` directly (see {@link partitionStoredFields}), - * with no `toDesignerField` in the path, so a stored `master_detail` whose - * target lives only as `referenceTo` still has it stripped and still reaches - * {@link assertRelationshipTargetPresent} with nothing — measured, and worse - * there than here, because a preserved field is read-only on this page and the - * refusal names a control the author has no way to reach. Filed as - * objectui#8896; ⛔ not fixed here. `formula` is the one entry + * ⚠️ This function alone is cost-free on the DESIGNABLE half only — the half + * that has a read door. {@link toFieldsMap} re-emits the fields this designer + * cannot author from the stored document directly (see + * {@link partitionStoredFields}), with no `toDesignerField` in the path, so a + * stored `master_detail` whose target lived only as `referenceTo` had it + * stripped and reached {@link assertRelationshipTargetPresent} with nothing — + * worse there than here, because a preserved field is read-only on this page and + * the refusal named a control the author had no way to reach. objectui#8896 + * closed that by routing the preserved branch through + * {@link carryPreservedField}, which reads the target with the SAME + * {@link storedRelationshipTarget} this half uses; ⛔ the preserved branch calls + * that function, never this one. `formula` is the one entry * whose strip DROPS a value, and that is objectui#6043's deliberate trade: the * server refuses to store it, a blind rename to `expression` would launder * non-CEL text into a formula that parses green and evaluates to null, and @@ -324,6 +327,53 @@ function carryOver(prev?: ServerFieldSchema): ServerFieldSchema { return next; } +/** + * Carry one PRESERVED field through, keeping the relationship target the stored + * document holds — objectui#8896. + * + * ## Why the preserved half needs its own function + * + * objectui#8058 made the `referenceTo` strip cost nothing ON THE DESIGNABLE + * HALF, by teaching the read door to find the target under either spelling: the + * value reaches the designer model through {@link storedRelationshipTarget} and + * `fromDesignerField` re-emits it as `reference`, so the strip removes a KEY and + * not the relationship. {@link toFieldsMap} re-emits objectui#8060's preserved + * fields from the stored document directly, with no `toDesignerField` in the + * path — so there the strip WAS the last thing to touch the field, the target + * left with the key, and {@link assertRelationshipTargetPresent} refused the + * whole object's save. That refusal names a control this page does not have: a + * preserved field is rendered read-only here by design, so "Pick the target + * object" has nowhere to go and every later save of the object stayed refused. + * + * ## It states the SAME rule as the designable half, through the same reader + * + * `storedRelationshipTarget` is the one place that decides which spelling a + * stored target is read from, and both halves now go through it — the sibling + * writers' `toFieldsMap` / `carryOver` pair is already a family where a + * difference is a defect waiting to be found twice. Whether the value is USABLE + * stays entirely `assertRelationshipTargetPresent`'s question, exactly as on the + * designable half: this function adopts what the document holds and invents + * nothing, so a stored `referenceTo: ' '` is refused by name rather than + * smuggled through, and a field with no target under either spelling emits no + * `reference` key at all. + * + * ⛔ The retired KEY still never reaches the wire — `FieldSchema` refuses it by + * name, which is what the strip is for. ⛔ And this is NOT a `specEquivalent` + * migration driven off the tombstone registry; the registry is explicit that the + * field is "Documentation for the reader, NEVER an instruction to migrate a + * value mechanically". What makes THIS key readable under either spelling is + * argued once, at {@link storedRelationshipTarget}, and applies here unchanged. + */ +function carryPreservedField(prev: ServerFieldSchema): ServerFieldSchema { + const next = carryOver(prev); + const target = storedRelationshipTarget(prev); + // Assigned only when the document holds one, so a field with no target keeps + // no `reference` key — `describeUnusableTarget` then says "this one has none" + // rather than reporting a value the author never wrote. + if (target !== undefined) next.reference = target; + return next; +} + /** One stored field the designer cannot author, kept whole. */ interface PreservedField { name: string; @@ -725,7 +775,7 @@ function toFieldsMap( // document verbatim — `type` included — rather than being rebuilt from a // designer model that has no way to hold it. The tombstone strip still // applies, because those keys 422 whoever wrote them. - entries.push([keep.name, carryOver(keep.raw)]); + entries.push([keep.name, carryPreservedField(keep.raw)]); } }; diff --git a/packages/types/src/internal/retired-field-keys.ts b/packages/types/src/internal/retired-field-keys.ts index f9951abc39..98ed9c8b51 100644 --- a/packages/types/src/internal/retired-field-keys.ts +++ b/packages/types/src/internal/retired-field-keys.ts @@ -139,9 +139,45 @@ export const RETIRED_FIELD_KEY_TOMBSTONES = [ { /* * A rename: the spec spells the lookup target `reference` - * ("Did you mean `referenceTo` -> `reference`?"). The strip loses nothing — - * every write path re-emits the designer's target under `reference`, and - * the read door's writers never emit the retired spelling. + * ("Did you mean `referenceTo` -> `reference`?"). + * + * ⚠️ "The strip loses nothing" was written here without a qualifier, and + * objectui#8896 measured it false at two of the three sites. It was only + * ever true where a READ DOOR had already lifted the target out of the + * stored document — `fromDesignerField` re-emitting the designer's target + * under `reference`. The two sites with no read door in front of them + * (`metadataFieldsPageCarryOver`'s objectui#8060 preserved branch, which + * re-emits a stored document verbatim, and `metadataAdminFieldsReadDoor`, + * which IS the read) deleted the only copy of the target, and + * objectui#7714's guard then refused the whole object's save from a page + * that renders the field read-only. + * + * Each of those two sites now lifts the VALUE onto the spec key before + * dropping the retired one. The strip itself is unchanged everywhere: + * `FieldSchema` refuses this spelling BY NAME, so no site emits it. + * + * ⚠️ The claim is therefore per site, and is NOT restated as a third + * unconditional sentence — restating the conclusion without its condition is + * how this entry went wrong the first time: + * + * - `metadataFieldsPageCarryOver` — nothing is lost. Its designable branch + * reads through `storedRelationshipTarget` (objectui#8058) and its + * preserved branch through `carryPreservedField` (objectui#8896); both + * re-emit under `reference`. + * - `metadataAdminFieldsReadDoor` — nothing is lost. The door keeps the + * value under `reference` as it drops the key (objectui#8896). + * - `metadataServiceCarryOver` — ⛔ UNMEASURED, deliberately. That site + * strips a stored entry it merges UNDERNEATH an already-built + * `DesignerFieldDefinition`, and `toFieldPayload` writes `reference` + * from that model unconditionally. Whether a target survives therefore + * depends on the CALLER's read door, and `saveFields` has no in-repo + * caller to measure (it is a published service API). Whoever measures + * one: this is the same class, and the answer belongs here. + * + * ⛔ That recovery is written per site and keyed to THIS key. It is NOT + * driven off `specEquivalent` — see the field's own doc, and objectui#6043, + * for why a mechanical rename is refused in general. What makes this key + * different is that its value is a bare object NAME under both spellings. */ key: 'referenceTo', retiredBy: 'objectui#6041',