diff --git a/.changeset/6528-resolve-reference-to-census.md b/.changeset/6528-resolve-reference-to-census.md new file mode 100644 index 0000000000..37ea994a95 --- /dev/null +++ b/.changeset/6528-resolve-reference-to-census.md @@ -0,0 +1,37 @@ +--- +'@object-ui/app-shell': patch +'@object-ui/core': patch +--- + +Resolve a relationship target from `reference` only — the spec spelling +(objectui#6528). + +`resolveReferenceTo` (dataset designer) and its sibling +`resolveRelationshipTarget` (`chart-series.ts`) each read a relationship field's +target through a four-spelling tolerant chain — `reference ?? reference_to ?? +referenceTo ?? reference_to_object`. Measured against every producer that can +reach them, three of the four are unfounded, so the chain is narrowed to +`reference` in BOTH places in one pass (they must not diverge — a fix leaving +them disagreeing recreates the defect one file over). + +The census, with `reference` itself as the positive control every zero is +measured against: + +| spelling | `ObjectSchema.safeParse` (spec 17.2.0) | producers on the object-metadata surface | +|---|---|---| +| `reference` | ACCEPTED | live — both designer writers emit it; 445 of 565 lookup/master_detail defs in the framework tree | +| `reference_to` | REFUSED BY NAME | 0 (live only on ObjectUI's own view/field schema — a different contract) | +| `referenceTo` | REFUSED BY NAME | 0 (producers retired by objectui#6041; stripped by the read door since objectui#6519) | +| `reference_to_object` | REFUSED (not even an alias) | 0 anywhere in either tree, outside the chain and its own test | + +Behaviour change, and it is deliberate: `chart-series.ts` reads +`GET /meta/object/:name` directly, with no read door stripping retired keys, so +a stored pre-objectui#6041 row spelling the target `referenceTo` no longer +resolves there. The walk is best-effort by construction — no entry is yielded +and the caller keeps the raw value — so such a row degrades visibly instead of +being silently absorbed. Per AGENTS.md #0.1 that row is a producer-side defect, +and a lenient consumer is where it would have stayed hidden. `reference` was +already head of the old chain, so any document carrying both is unaffected. + +The string / array / `{ object }` carriers are untouched: the carrier is a +separate axis from the spelling and narrowing it needs its own census. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.catalog.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.catalog.test.tsx index 7db81a0b04..094f27cd72 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.catalog.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.catalog.test.tsx @@ -44,14 +44,14 @@ const DOCS: Record> = { label: 'Opportunity', fields: { amount: { type: 'currency', label: 'Amount' }, - account: { type: 'lookup', reference_to: 'account', label: 'Account' }, + account: { type: 'lookup', reference: 'account', label: 'Account' }, }, }, account: { label: 'Account', fields: { name: { type: 'text', label: 'Account Name' }, - region: { type: 'lookup', reference_to: 'region', label: 'Region' }, + region: { type: 'lookup', reference: 'region', label: 'Region' }, }, }, region: { @@ -150,7 +150,7 @@ describe('useDatasetFieldCatalog — the include round-trip', () => { ...DOCS, opportunity: { label: 'Opportunity', - fields: { 'odd,name': { type: 'lookup', reference_to: 'account', label: 'Odd' } }, + fields: { 'odd,name': { type: 'lookup', reference: 'account', label: 'Odd' } }, }, }; get.mockImplementation(async (_t: string, name: string) => docs[name as keyof typeof docs] ?? null); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.test.ts index 93447cb17e..5935f9fb25 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.test.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.test.ts @@ -23,16 +23,45 @@ describe('resolveLabel', () => { }); describe('resolveReferenceTo', () => { - it('reads string / camel / snake variants', () => { - expect(resolveReferenceTo({ reference: 'account' })).toBe('account'); // framework lookup shape - expect(resolveReferenceTo({ reference_to: 'account' })).toBe('account'); - expect(resolveReferenceTo({ referenceTo: 'account' })).toBe('account'); - expect(resolveReferenceTo({ reference_to_object: 'account' })).toBe('account'); - }); - it('reads array + object shapes', () => { - expect(resolveReferenceTo({ reference_to: ['account', 'lead'] })).toBe('account'); - expect(resolveReferenceTo({ reference_to: { object: 'account' } })).toBe('account'); + it('reads the spec spelling `reference`', () => { + expect(resolveReferenceTo({ reference: 'account' })).toBe('account'); + }); + + it('reads array + object carriers off `reference`', () => { + // The CARRIER (bare name / one-element array / `{ object }`) is a separate + // axis from the SPELLING, and objectui#6528 narrowed only the latter. + expect(resolveReferenceTo({ reference: ['account', 'lead'] })).toBe('account'); + expect(resolveReferenceTo({ reference: { object: 'account' } })).toBe('account'); + }); + + /** + * objectui#6528 — the refusal is the point, so it is pinned rather than left + * as the absence of a test. + * + * These three spellings were a tolerant fallback chain here until the census + * measured them against every producer that can reach this helper and found + * none: `ObjectSchema.safeParse` (spec 17.2.0) REFUSES all three BY NAME while + * ACCEPTING `reference` (the positive control asserted above), `referenceTo`'s + * producers were retired by objectui#6041 and are stripped by the read door + * (objectui#6519), `reference_to` is live only on ObjectUI's own view/field + * schema — a different contract — and `reference_to_object` never had a + * producer at all. + * + * A def that reaches here spelling the target any of these ways is a PRODUCER + * defect (AGENTS.md #0.1). Resolving it would re-hide that producer, so this + * asserts it stays unresolved — deleting the narrowing turns this RED. + */ + it.each(['reference_to', 'referenceTo', 'reference_to_object'])( + 'refuses the legacy spelling `%s` — a producer emitting it is the bug', + (spelling) => { + expect(resolveReferenceTo({ [spelling]: 'account' })).toBeUndefined(); + }, + ); + + it('prefers `reference` on a partially-migrated def carrying both', () => { + expect(resolveReferenceTo({ reference: 'account', referenceTo: 'stale_legacy' })).toBe('account'); }); + it('returns undefined when absent', () => expect(resolveReferenceTo({ type: 'text' })).toBeUndefined()); }); @@ -60,7 +89,7 @@ describe('normalizeObject', () => { label: 'Opportunity', fields: { amount: { type: 'currency', label: 'Amount' }, - account: { type: 'lookup', label: 'Account', reference_to: 'account' }, + account: { type: 'lookup', label: 'Account', reference: 'account' }, stage: { type: 'text' }, }, }, @@ -73,7 +102,7 @@ describe('normalizeObject', () => { it('reads array-shaped fields too', () => { const norm = normalizeObject( - { fields: [{ name: 'owner', type: 'master_detail', reference_to: 'user' }] }, + { fields: [{ name: 'owner', type: 'master_detail', reference: 'user' }] }, 'task', ); expect(norm.relationships).toEqual([{ name: 'owner', label: 'owner', referenceTo: 'user' }]); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.ts b/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.ts index 2915019789..f424f2db02 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.ts @@ -73,12 +73,53 @@ export function resolveLabel(label: unknown, fallback: string): string { return fallback; } -/** Read a lookup/master_detail field's target object from its raw def. */ +/** + * Read a lookup/master_detail field's target object from its raw def. + * + * `reference` is the ONLY spelling this reads, and that is a measurement rather + * than a preference (objectui#6528). The four-spelling fallback chain this + * replaces (`reference ?? reference_to ?? referenceTo ?? reference_to_object`) + * was censused against every producer that can reach this helper: + * + * reference ACCEPTED by `ObjectSchema.safeParse` (spec 17.2.0) and + * emitted by both designer writers today + * (`MetadataService.toFieldPayload`, + * `MetadataFieldsPage.fromDesignerField`, each + * `reference: `). 445 of the 565 lookup / + * master_detail defs in the framework tree spell it + * this way. This is the positive control every zero + * below is measured against. + * reference_to REFUSED BY NAME — "Did you mean `reference_to` -> + * `reference`?". Zero producers on THIS surface. It is + * a live key only on ObjectUI's own view/field schema + * (`@object-ui/types` `views.zod.ts`), a different + * contract that `plugin-detail` translates INTO from + * `reference`; an object metadata document never + * carries it. + * referenceTo REFUSED BY NAME. Its two historical producers were + * retired at the producer by objectui#6041, and + * objectui#6519 added it to `RETIRED_FIELD_KEYS` so the + * read door strips it — `normalizeObject` builds every + * def through `readFields`, so this branch could not + * receive a value here even from a stored pre-#6041 row. + * reference_to_object REFUSED, and not even a recognised alias (the spec's + * alias table folds `relatedTo` / `referenceTo` / + * `target` / `targetObject` / `lookupObject` onto + * `reference` — never this one). Zero occurrences + * anywhere in either tree except this chain and the unit + * test that called it: it was only ever produced by its + * own test. + * + * Keeping the chain would be the shape AGENTS.md #0.1 forbids — a lenient + * consumer is where a wrong producer hides. A doc that reaches here spelling the + * target any other way is a PRODUCER defect, and must fail visibly here so it is + * fixed there. + * + * The string / array / `{ object }` CARRIERS are untouched: they are a separate + * axis from the spelling, and narrowing them needs its own census. + */ export function resolveReferenceTo(def: Record): string | undefined { - // Framework lookup/master_detail fields carry the target object in `reference`; - // older / spec shapes use `reference_to` / `referenceTo` / `reference_to_object`. - const raw = - def.reference ?? def.reference_to ?? (def as any).referenceTo ?? (def as any).reference_to_object; + const raw = def.reference; if (typeof raw === 'string' && raw) return raw; if (Array.isArray(raw) && typeof raw[0] === 'string') return raw[0]; if (raw && typeof raw === 'object') { diff --git a/packages/core/src/utils/__tests__/chart-series.test.ts b/packages/core/src/utils/__tests__/chart-series.test.ts index 1fe4fbc96e..0d580af87e 100644 --- a/packages/core/src/utils/__tests__/chart-series.test.ts +++ b/packages/core/src/utils/__tests__/chart-series.test.ts @@ -195,16 +195,44 @@ describe('relabelDimensions + buildChartSeries (the value≠label chart bug, clo }); describe('resolveRelationshipTarget (objectui#4053)', () => { - it('reads the target off every spelling of the reference key', () => { + it('reads the target off the spec spelling `reference`', () => { expect(resolveRelationshipTarget({ type: 'lookup', reference: 'crm_account' })).toBe('crm_account'); - expect(resolveRelationshipTarget({ type: 'lookup', reference_to: 'crm_account' })).toBe('crm_account'); - expect(resolveRelationshipTarget({ type: 'lookup', referenceTo: 'crm_account' })).toBe('crm_account'); - expect(resolveRelationshipTarget({ type: 'lookup', reference_to_object: 'crm_account' })).toBe('crm_account'); - // Array and `{ object }` carriers, both seen on spec-shaped defs. + // Array and `{ object }` carriers. The CARRIER is a separate axis from the + // SPELLING, and objectui#6528 narrowed only the latter. expect(resolveRelationshipTarget({ type: 'lookup', reference: ['crm_account'] })).toBe('crm_account'); expect(resolveRelationshipTarget({ type: 'lookup', reference: { object: 'crm_account' } })).toBe('crm_account'); }); + /** + * objectui#6528 — the mirror of the dataset designer's refusal pin, kept in + * lockstep so the two canonicalizations cannot drift (the divergence would + * recreate the defect one file over). + * + * The census that removed these three: `ObjectSchema.safeParse` (spec 17.2.0) + * REFUSES `reference_to` / `referenceTo` / `reference_to_object` BY NAME and + * ACCEPTS `reference` (asserted above as the positive control); no producer + * emits any of the three onto an object metadata document. + * + * This path matters MORE than the designer's: it reads + * `GET /meta/object/:name` directly, with no `readFields` door stripping + * retired keys. An unresolved target is best-effort by construction — the + * walk yields no entry and the caller keeps the raw value — so a producer + * emitting a legacy spelling degrades visibly instead of being silently + * absorbed here (AGENTS.md #0.1). + */ + it.each(['reference_to', 'referenceTo', 'reference_to_object'])( + 'refuses the legacy spelling `%s` — a producer emitting it is the bug', + (spelling) => { + expect(resolveRelationshipTarget({ type: 'lookup', [spelling]: 'crm_account' })).toBeUndefined(); + }, + ); + + it('prefers `reference` on a partially-migrated def carrying both', () => { + expect( + resolveRelationshipTarget({ type: 'lookup', reference: 'crm_account', referenceTo: 'stale_legacy' }), + ).toBe('crm_account'); + }); + it('accepts every master-detail spelling, case-insensitively', () => { for (const type of ['master_detail', 'masterDetail', 'master-detail', 'LOOKUP']) { expect(resolveRelationshipTarget({ type, reference: 'crm_account' })).toBe('crm_account'); diff --git a/packages/core/src/utils/chart-series.ts b/packages/core/src/utils/chart-series.ts index 372630d981..91c12d7d0f 100644 --- a/packages/core/src/utils/chart-series.ts +++ b/packages/core/src/utils/chart-series.ts @@ -940,10 +940,32 @@ function fieldDefsOf(schema: unknown): Record | null { * The object a relationship field points at, or `undefined` when the field is * not a relationship (or names no target). * - * The target lives under `reference` on framework-served field defs; older / - * spec shapes spell it `reference_to` / `referenceTo` / `reference_to_object`, - * and any of them may carry a bare name, a one-element array, or `{ object }`. - * Same canonicalization as the dataset designer's `resolveReferenceTo`. + * The target lives under `reference` — the ONLY spelling read here, and the same + * canonicalization as the dataset designer's `resolveReferenceTo`, which is + * narrowed to match in the same pass (objectui#6528). The value may still carry + * a bare name, a one-element array, or `{ object }`. + * + * The three legacy spellings this dropped (`reference_to` / `referenceTo` / + * `reference_to_object`) were censused against every producer, with `reference` + * itself as the positive control (445 of 565 lookup / master_detail defs in the + * framework tree; ACCEPTED by `ObjectSchema.safeParse` on spec 17.2.0, which + * REFUSES all three others BY NAME). No producer emits any of them onto an + * object metadata document: `reference_to` is live only on ObjectUI's own + * view/field schema (a different contract, translated INTO from `reference`), + * `referenceTo`'s two producers were retired by objectui#6041, and + * `reference_to_object` occurs nowhere in either tree outside this chain and the + * test that called it. + * + * ⚠ Unlike `resolveReferenceTo`, this helper reads the `GET /meta/object/:name` + * document DIRECTLY — no `readFields` door strips retired keys on this path. So + * the narrowing is load-bearing rather than cosmetic: a stored pre-objectui#6041 + * row spelling the target `referenceTo` now resolves to `undefined` here, the + * walk yields no entry, and the caller keeps the raw value (best-effort by + * construction — see {@link resolveDimensionFieldMeta}). That is the intended + * outcome: per AGENTS.md #0.1 such a row is a PRODUCER-side defect, and a + * lenient consumer here is exactly where it would have stayed hidden. Note + * `reference` was already HEAD of the old chain, so any doc carrying both is + * unaffected. * * The **type gate is deliberate**: only a declared relationship is walked, so a * path segment naming a plain field can never be turned into an object name and @@ -954,13 +976,10 @@ export function resolveRelationshipTarget(fieldDef: unknown): string | undefined const def = fieldDef as { type?: unknown; reference?: unknown; - reference_to?: unknown; - referenceTo?: unknown; - reference_to_object?: unknown; }; const type = typeof def.type === 'string' ? def.type.toLowerCase() : ''; if (!RELATIONSHIP_FIELD_TYPES.has(type)) return undefined; - const raw = def.reference ?? def.reference_to ?? def.referenceTo ?? def.reference_to_object; + const raw = def.reference; if (typeof raw === 'string' && raw) return raw; if (Array.isArray(raw) && typeof raw[0] === 'string' && raw[0]) return raw[0]; if (raw && typeof raw === 'object') {