From a5a6df95127640f07bef7da288f2194d67d45021 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 06:30:10 +0000 Subject: [PATCH] fix(plugin-grid,plugin-dashboard): derive the relational copy set from its consumers, so a lookup cell honours the spec-declared `displayField` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The copy set ObjectGrid writes onto each relational column's fieldMeta was hand-kept and had become a strict subset of what its own lookup cell renderer and inline picker read off that bag. displayField, descriptionField and lookupColumns were read on this grid's path and never copied — and they are the spellings a spec-compliant producer actually emits, since @objectstack/spec 17.2.0's strict FieldSchema declares them and none of the snake_case twins. Derived instead of restated: relationalMetaKeys.ts classifies every key the consumers read, and a gate re-extracts that read set from the consumer sources so the two cannot drift apart again. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM --- .../6875-grid-relational-meta-derive.md | 33 +++ .../lookupRelationalMeta-6694.test.tsx | 18 +- .../plugin-dashboard/src/recordFields.tsx | 30 ++- packages/plugin-grid/src/ObjectGrid.tsx | 95 +------ .../lookupDisplayFieldSpelling-6875.test.tsx | 158 ++++++++++++ .../relationalMetaCopySet.derivation.test.ts | 229 +++++++++++++++++ .../plugin-grid/src/relationalMetaKeys.ts | 235 ++++++++++++++++++ 7 files changed, 707 insertions(+), 91 deletions(-) create mode 100644 .changeset/6875-grid-relational-meta-derive.md create mode 100644 packages/plugin-grid/src/__tests__/lookupDisplayFieldSpelling-6875.test.tsx create mode 100644 packages/plugin-grid/src/__tests__/relationalMetaCopySet.derivation.test.ts create mode 100644 packages/plugin-grid/src/relationalMetaKeys.ts diff --git a/.changeset/6875-grid-relational-meta-derive.md b/.changeset/6875-grid-relational-meta-derive.md new file mode 100644 index 0000000000..cf8a567aaf --- /dev/null +++ b/.changeset/6875-grid-relational-meta-derive.md @@ -0,0 +1,33 @@ +--- +'@object-ui/plugin-grid': patch +'@object-ui/plugin-dashboard': patch +--- + +A lookup cell in `ObjectGrid` now honours the author's `displayField` +(objectui#6875). + +`ObjectGrid` copies a set of relational keys off the object-schema field def +onto each column's `fieldMeta`, and that bag is what the lookup cell renderer +and the inline picker receive. The set was hand-kept and had become a strict +SUBSET of what those two consumers read — `displayField`, `descriptionField` +and `lookupColumns` were read on the grid's own path and never copied. + +They are the spellings that matter. `@objectstack/spec` 17.2.0's `FieldSchema` +is strict and declares `displayField` / `descriptionField` / `lookupColumns` / +`lookupFilters` / `reference`, and none of the snake_case twins the copy set +mostly carried — those parse to `unrecognized_keys`, so a spec-compliant +producer cannot emit them. Nothing renames anything on the way in either: the +adapter's `getObjectSchema` choke point rewrites only the `reference` ⇄ +`reference_to` pair. So an author who declared `displayField: 'project_code'` +got a grid cell showing the referenced record's generic `.name` instead. + +- The copy set is now DERIVED, in `plugin-grid/src/relationalMetaKeys.ts`, from + a table that classifies every key the consumers read off this bag. A gate + re-extracts that read set from the consumer sources on each run and fails on + any unclassified spelling or orphan, so the two cannot drift apart again. +- `reference_field` and `lookup_columns` — the other two never-copied keys — + stay out on purpose: `FieldSchema` declares neither, so no producer can fill + them. The gate proves that against the installed spec rather than asserting it + in prose. +- `plugin-dashboard`'s `CELL_RELATIONAL_META_KEYS` had the same omission in the + same fallback chain and gains `displayField` too. diff --git a/packages/plugin-dashboard/src/__tests__/lookupRelationalMeta-6694.test.tsx b/packages/plugin-dashboard/src/__tests__/lookupRelationalMeta-6694.test.tsx index 8b836f96ea..65bca934f4 100644 --- a/packages/plugin-dashboard/src/__tests__/lookupRelationalMeta-6694.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/lookupRelationalMeta-6694.test.tsx @@ -260,6 +260,14 @@ describe('objectui#6694 — buildFieldMeta copies the cell-read relational keys reference_to: 'project', reference: 'project', display_field: 'project_code', + // The SPEC spelling of the same pointer (objectui#6875). `FieldSchema` + // declares `displayField` and none of the snake twins, so this is the leg a + // live `getObjectSchema` actually serves — it must be copied. + displayField: 'project_code', + // The chain's third leg. Read by `LookupCellRenderer`, but `FieldSchema` + // refuses it with `unrecognized_keys`, so no producer can emit it and + // copying it would reach nothing (objectui#6711's reasoning). NOT copied. + reference_field: 'x', // Six keys with no reader on this path. FOUR of them the grid still copies // (its picker-only keys); the other two it has since retired as well — // `reference_to_field` (objectui#6711) and `titleFormat` (objectui#6874). @@ -273,11 +281,14 @@ describe('objectui#6694 — buildFieldMeta copies the cell-read relational keys titleFormat: '{project_code}', }; - it('copies reference_to / reference / display_field', () => { + it('copies reference_to / reference / display_field / displayField', () => { const meta = buildFieldMeta({ accessorKey: 'project', label: 'Project', def }) as any; expect(meta.reference_to).toBe('project'); expect(meta.reference).toBe('project'); expect(meta.display_field).toBe('project_code'); + // objectui#6875 — the spec-declared spelling, previously dropped here and in + // `ObjectGrid` at the same time. + expect(meta.displayField).toBe('project_code'); }); it('does NOT copy the picker-only keys', () => { @@ -285,6 +296,9 @@ describe('objectui#6694 — buildFieldMeta copies the cell-read relational keys for (const k of [ 'reference_to_field', 'id_field', 'description_field', 'lookup_filters', 'lookupFilters', 'titleFormat', + // Read by `LookupCellRenderer`, but unproducible under the strict + // `FieldSchema` — objectui#6875 measured it and left it out on purpose. + 'reference_field', ]) { expect(meta).not.toHaveProperty(k); } @@ -294,7 +308,7 @@ describe('objectui#6694 — buildFieldMeta copies the cell-read relational keys const meta = buildFieldMeta({ accessorKey: 'amount', label: 'Amount', def: { type: 'currency' }, }) as any; - for (const k of ['reference_to', 'reference', 'display_field']) { + for (const k of ['reference_to', 'reference', 'display_field', 'displayField']) { expect(meta).not.toHaveProperty(k); } }); diff --git a/packages/plugin-dashboard/src/recordFields.tsx b/packages/plugin-dashboard/src/recordFields.tsx index da500e028f..32088e34c8 100644 --- a/packages/plugin-dashboard/src/recordFields.tsx +++ b/packages/plugin-dashboard/src/recordFields.tsx @@ -112,8 +112,27 @@ export const NUMERIC_FIELD_TYPES = new Set([ * the module `getCellRenderer` dispatches into — the complete set of relational * keys read off a cell's `field` prop is: * - * - `reference_to`, `reference`, `display_field` — read by + * - `reference_to`, `reference`, `display_field`, `displayField` — read by * `LookupCellRenderer` itself. ✅ COPIED. + * + * ⭐ `displayField` ARRIVED with objectui#6875. The enumeration above used + * to name three keys, because it was written from the FIRST leg of each + * chain rather than from the whole chain: `LookupCellRenderer` resolves the + * display pointer as `display_field || displayField || reference_field`, and + * the two extra spellings in that one chain were missed here and in the + * grid's own list at the same time. `displayField` is the spelling + * `@objectstack/spec` 17.2.0's strict `FieldSchema` DECLARES — so on a live + * path served through `getObjectSchema` it is the only one that can arrive, + * and a lookup cell here rendered the referenced record's generic `.name` + * instead of the author's pointer. The grid's twin of this defect is pinned + * behaviourally in `plugin-grid/src/__tests__/lookupDisplayFieldSpelling-6875.test.tsx`. + * + * - `reference_field` — the chain's third leg, and still ⛔ NOT copied. + * `FieldSchema` does not declare it (it parses to `unrecognized_keys`) and + * the producer repo has zero occurrences of the identifier, against a + * `displayField` control that hits 68 files. Copying it would write a member + * from the def on every call that no producer can fill — objectui#6711's + * reasoning, unchanged. * - `id_field`, `description_field`, `lookup_filters`, `lookupFilters` — ZERO * mentions in that module; read only by `fields/src/widgets/LookupField.tsx` * and `UserField.tsx`, both EDITORS. ⛔ NOT copied. @@ -136,7 +155,7 @@ export const NUMERIC_FIELD_TYPES = new Set([ * picker keys. The boundary is pinned in * `__tests__/lookupRelationalMeta-6694.test.tsx`. */ -const CELL_RELATIONAL_META_KEYS = ['reference_to', 'reference', 'display_field'] as const; +const CELL_RELATIONAL_META_KEYS = ['reference_to', 'reference', 'display_field', 'displayField'] as const; /** * Copy {@link CELL_RELATIONAL_META_KEYS} off a schema field def, with @@ -228,6 +247,13 @@ export interface FieldMeta { reference?: string; /** Author-declared display field on the lookup — beats every resolver in the cell. */ display_field?: string; + /** + * Same pointer, SPEC spelling (`FieldSchema.displayField`) — the second leg of + * `LookupCellRenderer`'s `display_field || displayField || reference_field` + * chain, and the only leg a spec-compliant producer can actually emit + * (objectui#6875). + */ + displayField?: string; } /** diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 1de48053a1..83a0fa5663 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -46,6 +46,7 @@ import { useColumnSummary } from './useColumnSummary'; import { resolveRowCrudAffordances, resolveRowRecordCrudAffordance } from './rowCrudAffordances'; import { useRecordCrudVerdicts } from './hooks/useRecordCrudVerdicts'; import { resolveLegacyRowActions } from './resolveLegacyRowActions'; +import { applyRelationalMeta } from './relationalMetaKeys'; import { resolveBulkActions } from './resolveBulkActions'; import { partitionBulkRows } from './bulkEligibility'; import { resolvesToDataColumn, describeUnresolvedColumns } from './columnSpellingDiagnostics'; @@ -421,84 +422,14 @@ function getDataConfig(schema: ObjectGridSchema): ViewData | null { } /** - * Relational field metadata that a lookup / master_detail / user cell needs to - * (a) resolve a bare foreign-key id to a display name (LookupCellRenderer → - * `field.reference_to`) and (b) drive the inline picker's query (LookupField - * reads reference_to/reference, display_field, id_field, description_field, - * lookup_filters). These are dropped if we only copy the scalar-display props - * (label/currency/precision/…), which is why an inline-edited lookup showed the - * raw id after moving to another row. Copy them from the object-schema field - * definition onto the built `fieldMeta` for every column-building path. - * - * ## ⛔ Two keys were in this list and are RETIRED - * - * Every key here has to have a measured reader on this grid's own render path — - * the cell renderers and inline editors in `@object-ui/fields` that - * `getCellRenderer` dispatches into. Two keys had none, for two different - * reasons, and each retirement was its own adjudication. - * - * ### `reference_to_field` — objectui#6711 - * - * Swept across `packages/` and `apps/` (and again across the producer repo), the - * only occurrences of the identifier anywhere were this array literal — the - * write — and prose recording that nothing reads it. No member access, no - * destructuring, no bracket read. `@objectstack/spec`'s FieldSchema does not - * declare it either, so nothing authorable produces it. - * - * ### `titleFormat` — objectui#6874 - * - * A zero of a different kind, and a stronger one. `titleFormat` is a real, live - * key with plenty of readers — it simply has no FIELD-meta reader. The sweep did - * not fail to find readers; it found every member read of the identifier across - * `packages/` and `apps/` (tests included) and classified each one by receiver: - * - * - `objectDef` / `objectSchema` / `objSchema` — `core/utils/record-title.ts`, - * `components/.../containers.tsx`, `plugin-detail/DetailView.tsx`, - * `ObjectKanban.tsx`, `ObjectCalendar.tsx`, `react/hooks/useRecordSearch.ts`. - * OBJECT schema, every one. - * - `refObjectSchema?.titleFormat` — `fields/widgets/LookupField.tsx`: the - * REFERENCED object's schema, fetched by `getSchema(referenceTo)`. Also an - * OBJECT schema, and the one that matters here — it is what this grid's own - * inline picker reads. - * - `param.titleFormat` — `app-shell/utils/paramToField.ts`, off a resolved - * `ActionParamDef`; the field-def read next to it is `field.title_format`, - * a different spelling on a different surface. - * - * `RecordPickerDialog` and `lookupColumnDisplay` receive it as a PROP, and the - * repo's single `titleFormat=` pass is `titleFormat={refTitleFormat}` — - * object-schema sourced. ⇒ copying `reference_to` is what makes `titleFormat` - * work on this path; copying `titleFormat` onto the meta reached nothing. - * `plugin-dashboard/src/recordFields.tsx` recorded this same measurement first - * and declined to copy the key, so it was a measured no-op in two seams and had - * been retired from only one. - * - * ### The control that makes both zeros a reading - * - * Not an artefact of how the sweep was written: the same sweep over the - * surviving list-mates finds a real FIELD-meta reader for every one of them — - * `reference_to` / `reference` / `display_field` off the cell's `field` prop in - * `LookupCellRenderer` (`fields/src/index.tsx`), and `id_field` / - * `description_field` / `lookup_filters` / `lookupFilters` off `fieldMeta?.…` - * in `LookupField` / `UserField`. There is no third reader-less key: all seven - * survivors are read off a field meta. - * - * ⚠️ The sweep bounds these two repos. A host application outside them could - * still be reading either key off `fieldMeta`; the repo's own contract is what - * these retirements are about. - * - * ⛔ Do not re-add a key for symmetry with the object-schema field def. A - * member written from the def on every column build and read by nothing is - * exactly what objectui#6625 (`decimals`) and objectui#6597 (`referenceTo`) - * retired from the sibling producer. Add a key when a reader on THIS path is - * measured, not before. Both absences are pinned, at all three call sites — - * `__tests__/relationalMetaCopySet-6711.test.tsx` and - * `__tests__/relationalMetaCopySet-6874.test.tsx`. + * The relational copy set and `applyRelationalMeta` moved to + * `./relationalMetaKeys` for objectui#6875. The list there is DERIVED from a + * table classifying every key the grid's own cell renderer and inline picker + * read off this bag, and a gate re-derives that read set from the consumer + * sources — so the copy set can no longer drift into being a strict subset of + * what its consumers read, which is what it had silently become. Read that + * file's docblock before adding, removing or re-spelling a key. */ -const RELATIONAL_META_KEYS = [ - 'reference_to', 'reference', - 'display_field', 'id_field', 'description_field', - 'lookup_filters', 'lookupFilters', -] as const; /** * Content signature of a host's find-params, used as the query-change signal for @@ -517,16 +448,6 @@ function findParamsSignature(params: Record | null | undefined) ); } -function applyRelationalMeta( - fieldMeta: Record, - fieldDef: Record | undefined | null, -): void { - if (!fieldDef) return; - for (const key of RELATIONAL_META_KEYS) { - if (fieldDef[key] !== undefined) fieldMeta[key] = fieldDef[key]; - } -} - /** * Helper to normalize columns configuration * Handles both string[] and ListColumn[] formats diff --git a/packages/plugin-grid/src/__tests__/lookupDisplayFieldSpelling-6875.test.tsx b/packages/plugin-grid/src/__tests__/lookupDisplayFieldSpelling-6875.test.tsx new file mode 100644 index 0000000000..386e634b1b --- /dev/null +++ b/packages/plugin-grid/src/__tests__/lookupDisplayFieldSpelling-6875.test.tsx @@ -0,0 +1,158 @@ +/** + * 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#6875 — a lookup cell in `ObjectGrid` must honour the author's + * `displayField`, the SPEC-DECLARED spelling. + * + * ## Why this file renders a cell instead of asserting a key list + * + * The sibling pins in this directory (`relationalMetaCopySet-6711` / + * `-6874`) assert what does and does not land on the `fieldMeta` bag. That is + * the right instrument for a retirement, where nothing renders differently. + * Here something DOES render differently, and the card was filed as a static + * producer-vs-consumer measurement with no user report attached — so the thing + * worth pinning is the user-visible outcome, not the copy set that causes it. + * + * ## The measurement this file encodes + * + * `@objectstack/spec` 17.2.0's `FieldSchema` is a strict object with 71 props. + * It declares `displayField`, `descriptionField`, `lookupColumns`, + * `lookupFilters` and `reference`. It declares NONE of `display_field`, + * `description_field`, `lookup_columns`, `reference_to`, `reference_field` — + * those parse to `unrecognized_keys`, the same code a nonsense key gets, so no + * spec-compliant producer can ship one. Measured on the installed package, with + * `name`/`type`/`label` as the positive control. + * + * Nothing renames them on the way in either: `ObjectStackAdapter.getObjectSchema` + * (`@object-ui/data-objectstack`) is the choke point every schema read goes + * through, and its only key rewrite is `normalizeSchemaReferenceKeys` — the + * `reference` ⇄ `reference_to` pair, nothing else. `applyFieldWidgetOverrides` + * adds `widget` and touches no other key. + * + * ⇒ On a live path the ONLY display-field spelling that can reach this grid is + * `displayField`, and it was the one spelling `RELATIONAL_META_KEYS` did not + * copy. The consumer — `LookupCellRenderer` in `@object-ui/fields` — reads + * `display_field || displayField || reference_field`, so it was ready for the + * key the whole time; the value simply never arrived. + * + * ## The control that makes the red half a reading + * + * Two columns render from ONE data source, ONE referenced record and ONE cell + * renderer, differing only in the spelling on the field def: + * + * `code_camel` → `{ displayField: 'project_code' }` (spec-declared) + * `code_snake` → `{ display_field: 'project_code' }` (already copied) + * + * The snake column is the positive control. Before the fix it resolved + * `ACME-42` while the camel column resolved `Wrong Name` — the referenced + * record's `name`, via the generic heuristic that runs when no display field is + * declared. A single-column test could not tell "the key never arrived" apart + * from "the fixture never reached the lookup path at all"; the control column + * is what separates them, and it must stay green in both directions. + */ +import { describe, it, expect, vi, beforeAll } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { ObjectGrid } from '../ObjectGrid'; +import { registerAllFields } from '@object-ui/fields'; +import { ActionProvider, SchemaRendererProvider } from '@object-ui/react'; + +registerAllFields(); + +const OBJECT = 'os_6875_task'; +const REFERENCED = 'os_6875_project'; + +/** The referenced record. `name` is deliberately WRONG so the fallback is visible. */ +const PROJECT = { id: 'p1', name: 'Wrong Name', project_code: 'ACME-42' }; + +const ROWS = [{ id: 't1', title: 'Task one', code_camel: 'p1', code_snake: 'p1' }]; + +function makeDataSource() { + return { + find: vi.fn(async (objectName: string) => { + if (objectName === REFERENCED) { + return { data: [PROJECT], total: 1, hasMore: false, pageSize: 50 }; + } + return { data: ROWS, total: ROWS.length, hasMore: false, pageSize: 50 }; + }), + findOne: vi.fn(async (objectName: string, id: string) => + objectName === REFERENCED && id === 'p1' ? PROJECT : null, + ), + getObjectSchema: async (name: string) => { + if (name === REFERENCED) { + // No `nameField`, no `titleFormat` — so nothing but the field def's + // display pointer can produce `ACME-42`. + return { name, fields: { id: { type: 'text' }, name: { type: 'text' }, project_code: { type: 'text' } } }; + } + return { + name, + fields: { + id: { type: 'text' }, + title: { type: 'text', label: 'Title' }, + // Spec-declared spelling. This is what a live `getObjectSchema` can carry. + code_camel: { type: 'lookup', label: 'Project (spec spelling)', reference: REFERENCED, displayField: 'project_code' }, + // Runtime spelling, already in the copy set — the positive control. + code_snake: { type: 'lookup', label: 'Project (runtime spelling)', reference: REFERENCED, display_field: 'project_code' }, + }, + }; + }, + } as any; +} + +beforeAll(() => { + if (!Element.prototype.scrollIntoView) { + Element.prototype.scrollIntoView = vi.fn() as any; + } +}); + +async function renderGrid() { + const ds = makeDataSource(); + const schema: any = { + type: 'object-grid', + objectName: OBJECT, + data: ROWS, + pagination: { pageSize: 50 }, + columns: [ + { field: 'code_camel', label: 'Project (spec spelling)', type: 'lookup' }, + { field: 'code_snake', label: 'Project (runtime spelling)', type: 'lookup' }, + ], + }; + render( + + + + + , + ); + // The control column proves the lookup path itself is reached and resolving. + await waitFor(() => { + expect(screen.getByText('Project (runtime spelling)')).toBeInTheDocument(); + }); +} + +describe('objectui#6875 — ObjectGrid lookup cells honour the spec-declared `displayField`', () => { + it('the runtime spelling `display_field` resolves the declared display value (CONTROL)', async () => { + await renderGrid(); + await waitFor(() => { + expect(screen.getAllByText('ACME-42').length).toBeGreaterThan(0); + }, { timeout: 4000 }); + }); + + it('the spec spelling `displayField` resolves it too, and the row never shows the referenced record’s `name`', async () => { + await renderGrid(); + await waitFor(() => { + // Both columns resolved through the author's pointer. + expect(screen.getAllByText('ACME-42').length).toBe(2); + }, { timeout: 4000 }); + // The generic `.name` heuristic must not surface anywhere in the row. + expect(screen.queryByText('Wrong Name')).not.toBeInTheDocument(); + }); +}); diff --git a/packages/plugin-grid/src/__tests__/relationalMetaCopySet.derivation.test.ts b/packages/plugin-grid/src/__tests__/relationalMetaCopySet.derivation.test.ts new file mode 100644 index 0000000000..2c4bc77ee2 --- /dev/null +++ b/packages/plugin-grid/src/__tests__/relationalMetaCopySet.derivation.test.ts @@ -0,0 +1,229 @@ +/** + * 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#6875 — the gate that DERIVES `ObjectGrid`'s relational copy set from + * its consumers instead of trusting a hand-kept literal. + * + * ## What went wrong, and why a longer literal would not have fixed it + * + * `RELATIONAL_META_KEYS` was governed by a rule stated only in prose — *every + * key here has to have a measured reader on this grid's own render path*. The + * repo enforced that rule in one direction: objectui#6711 and objectui#6874 + * each removed a key that had NO reader, and each left a behavioural pin + * behind. Nothing enforced the other direction, so five keys that DO have + * readers on this path were never copied, and the list's own spellings had gone + * inconsistent — `lookupFilters` (camel) sat in it while `displayField` and + * `descriptionField` did not, though all three come off the same kind of + * chain in the same files. + * + * Adding the missing spellings by hand would restore the invariant for exactly + * as long as nobody touches a consumer. This file is the invariant itself: it + * re-extracts the read set from the consumer sources on every run and requires + * `RELATIONAL_META_READ_SET` to match it EXACTLY, in both directions. A new + * spelling in any chain is unclassified → red. A key deleted from a chain is an + * orphan in the table → red. + * + * ## The three consumers, and how each is read + * + * `generateColumns()` hands `fieldMeta` to `CellRenderer` as the `field` prop. + * `getCellRenderer` dispatches a relational column to `LookupCellRenderer` + * (`@object-ui/fields/src/index.tsx`), which reads its keys through + * `(field as { k?: T }).k` casts — the untyped-read shape this seam uses. The + * inline editor dispatches the same bag into `LookupField` (receiver + * `fieldMeta`) and `UserField` (receiver `meta`), which use optional-chained + * member reads. + * + * ⚠️ `UserField` is swept even though it forwards its whole meta into + * `LookupField` via a spread. A delegating consumer is exactly where a false + * zero hides: a key it read and did NOT forward would be invisible in + * `LookupField`'s own source. Its extracted set being a subset is a RESULT + * here, not an assumption. + * + * ## ⛔ The extractor is bounded, and says so + * + * It reads member accesses off named receivers. A key that reaches a consumer + * some other way — destructuring, a computed `meta[expr]`, a helper that takes + * the whole bag — is outside its reach. That is the honest limit; the five + * spellings objectui#6875 measured were all plain member reads, and so is every + * key the three chains use today. `assertExtractorFoundKnownChains` is the + * positive control that keeps a silently-empty extraction from reading as a + * clean bill of health. + */ +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import path from 'node:path'; + +import { FieldSchema } from '@objectstack/spec/data'; + +import { RELATIONAL_META_READ_SET, RELATIONAL_META_KEYS } from '../relationalMetaKeys'; + +const FIELDS_SRC = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + '../../../fields/src', +); + +function read(rel: string): string { + return readFileSync(path.join(FIELDS_SRC, rel), 'utf8'); +} + +/** + * The body of `LookupCellRenderer`, bounded by its declaration and the first + * column-0 `}` after it. Bounding matters: `fields/src/index.tsx` holds every + * cell renderer, and the unrelated ones read `field.min`, `field.pattern`, + * `field.required_message` and friends off the same identifier. + */ +function lookupCellRendererBody(): string { + const src = read('index.tsx'); + const lines = src.split('\n'); + const start = lines.findIndex((l) => l.startsWith('export function LookupCellRenderer(')); + if (start < 0) throw new Error('LookupCellRenderer declaration not found — extractor is stale'); + const end = lines.findIndex((l, i) => i > start && l === '}'); + if (end < 0) throw new Error('LookupCellRenderer end brace not found — extractor is stale'); + return lines.slice(start, end + 1).join('\n'); +} + +/** `recv?.key` / `recv.key` member reads off one named receiver. */ +function memberReads(src: string, receiver: string): Set { + const out = new Set(); + const re = new RegExp(`\\b${receiver}\\s*\\??\\.\\s*([A-Za-z_$][\\w$]*)`, 'g'); + let m: RegExpExecArray | null; + while ((m = re.exec(src))) out.add(m[1]); + return out; +} + +/** `(recv as { key?: T }).key` cast reads — how `LookupCellRenderer` reads. */ +function castReads(src: string, receiver: string): Set { + const out = new Set(); + const re = new RegExp(`\\(\\s*${receiver}\\s+as\\s+\\{\\s*([A-Za-z_$][\\w$]*)\\s*\\??:`, 'g'); + let m: RegExpExecArray | null; + while ((m = re.exec(src))) out.add(m[1]); + return out; +} + +interface Extraction { + readonly all: Set; + readonly perConsumer: Readonly>>; +} + +function extractReadSet(): Extraction { + const cell = lookupCellRendererBody(); + const perConsumer = { + 'index.tsx#LookupCellRenderer': castReads(cell, 'field'), + 'widgets/LookupField.tsx': memberReads(read('widgets/LookupField.tsx'), 'fieldMeta'), + 'widgets/UserField.tsx': memberReads(read('widgets/UserField.tsx'), 'meta'), + }; + const all = new Set(); + for (const set of Object.values(perConsumer)) for (const k of set) all.add(k); + return { all, perConsumer }; +} + +/** + * Positive control. An extractor that silently matched nothing would make every + * "no unclassified key" assertion below pass vacuously — the classic shape of a + * green gate over an empty measurement. These five spellings are quoted from + * the three consumers by hand; if the extractor stops finding them it is broken, + * whatever the copy set says. + */ +function assertExtractorFoundKnownChains(x: Extraction): void { + expect(x.perConsumer['index.tsx#LookupCellRenderer']).toContain('display_field'); + expect(x.perConsumer['index.tsx#LookupCellRenderer']).toContain('displayField'); + expect(x.perConsumer['widgets/LookupField.tsx']).toContain('lookup_columns'); + expect(x.perConsumer['widgets/LookupField.tsx']).toContain('lookupColumns'); + expect(x.perConsumer['widgets/UserField.tsx']).toContain('reference_field'); +} + +const specProps = new Set(Object.keys((FieldSchema as any).shape)); + +describe('objectui#6875 — the copy set is derived from the consumers, not restated', () => { + it('the extractor reaches all three consumers (CONTROL)', () => { + const x = extractReadSet(); + assertExtractorFoundKnownChains(x); + // Every consumer contributes; a zero from any one of them is a broken sweep, + // not a consumer that reads nothing. + for (const [name, set] of Object.entries(x.perConsumer)) { + expect(set.size, `${name} contributed no reads`).toBeGreaterThan(0); + } + }); + + it('classifies every key the consumers read — no unclassified spelling', () => { + const { all } = extractReadSet(); + const unclassified = [...all].filter((k) => !(k in RELATIONAL_META_READ_SET)).sort(); + expect( + unclassified, + 'A consumer reads these off the field meta and the table does not classify them. ' + + 'Add each to RELATIONAL_META_READ_SET with a verdict — that decision is the fix ' + + 'objectui#6875 exists to make unforgettable.', + ).toEqual([]); + }); + + it('carries no orphan — every classified key is still read by a consumer', () => { + const { all } = extractReadSet(); + const orphans = Object.keys(RELATIONAL_META_READ_SET).filter((k) => !all.has(k)).sort(); + expect( + orphans, + 'These are classified but no consumer reads them any more. A key written from the ' + + 'schema def on every column build and read by nothing is what objectui#6711 and ' + + 'objectui#6874 retired.', + ).toEqual([]); + }); + + it('proves each `no-producer` verdict against the installed spec, not against prose', () => { + const claimed = Object.entries(RELATIONAL_META_READ_SET) + .filter(([, e]) => e.verdict === 'no-producer') + .map(([k]) => k); + expect(claimed.length).toBeGreaterThan(0); + // Control first: the assertion below is "absent from a 71-prop strict + // schema", and an empty/misresolved shape would satisfy it for every key. + expect(specProps.size).toBeGreaterThan(60); + expect(specProps.has('displayField')).toBe(true); + for (const key of claimed) { + expect(specProps.has(key), `${key} is classified no-producer but FieldSchema declares it`).toBe(false); + } + }); + + it('proves each `spec` verdict against the installed spec', () => { + for (const [key, e] of Object.entries(RELATIONAL_META_READ_SET)) { + if (e.verdict !== 'spec') continue; + expect(specProps.has(key), `${key} is classified spec but FieldSchema does not declare it`).toBe(true); + } + }); + + it('records the `legacy-alias` asymmetry mechanically — none of them is authorable', () => { + // These are copied for back-compat and cannot be produced by a + // spec-compliant author. Asserting it here keeps the docblock's claim from + // going stale silently if a future spec version declares one of them — at + // which point the verdict should become `spec`. + for (const [key, e] of Object.entries(RELATIONAL_META_READ_SET)) { + if (e.verdict !== 'legacy-alias') continue; + expect(specProps.has(key), `${key} is now spec-declared — reclassify it as 'spec'`).toBe(false); + } + }); + + it('the copy set is exactly the copied verdicts, and includes the three keys objectui#6875 measured missing', () => { + const expected = Object.entries(RELATIONAL_META_READ_SET) + .filter(([, e]) => e.verdict === 'spec' || e.verdict === 'adapter-stamped' || e.verdict === 'legacy-alias') + .map(([k]) => k); + expect([...RELATIONAL_META_KEYS].sort()).toEqual(expected.sort()); + for (const key of ['displayField', 'descriptionField', 'lookupColumns']) { + expect(RELATIONAL_META_KEYS).toContain(key); + } + // The two named keys that are NOT reachable stay out — copying them would + // write a member no producer can fill (objectui#6711's reasoning). + for (const key of ['reference_field', 'lookup_columns']) { + expect(RELATIONAL_META_KEYS).not.toContain(key); + } + }); + + it('every entry carries a note — a verdict with no reason is not a decision', () => { + for (const [key, e] of Object.entries(RELATIONAL_META_READ_SET)) { + expect(e.note.length, `${key} has no note`).toBeGreaterThan(20); + } + }); +}); diff --git a/packages/plugin-grid/src/relationalMetaKeys.ts b/packages/plugin-grid/src/relationalMetaKeys.ts new file mode 100644 index 0000000000..b9b046c2cf --- /dev/null +++ b/packages/plugin-grid/src/relationalMetaKeys.ts @@ -0,0 +1,235 @@ +/** + * 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. + */ + +/** + * ⭐ WHAT `ObjectGrid` COPIES OFF AN OBJECT-SCHEMA FIELD DEF ONTO A COLUMN'S + * `fieldMeta` — objectui#6875. + * + * ## The defect this file exists to make unrepeatable + * + * `RELATIONAL_META_KEYS` used to be a bare array literal inside `ObjectGrid.tsx`, + * governed by a rule stated in prose: *every key here has to have a measured + * reader on this grid's own render path*. The rule was right; nothing enforced + * it in the OTHER direction. The list was a strict SUBSET of what its own + * consumers read, and had been for as long as anyone had looked: + * + * copied, never read — none (objectui#6711 and objectui#6874 closed that half) + * read, never copied — `displayField`, `descriptionField`, `lookupColumns`, + * `reference_field`, `lookup_columns` + * + * The list was also internally inconsistent about spelling, which is the tell + * that it was assembled from defect reports rather than derived: `lookupFilters` + * (camel) sat in it next to `lookup_filters`, while `displayField` and + * `descriptionField` did NOT sit next to their snake twins. Same fallback + * chains, same file, opposite outcomes. + * + * So the fix is not a longer literal. {@link RELATIONAL_META_READ_SET} below + * classifies EVERY key the consumers read off this bag, and + * `__tests__/relationalMetaCopySet.derivation.test.ts` re-derives that read set + * from the consumer sources on every run. A spelling added to any consumer + * chain lands in neither column of the table and turns the gate red; it cannot + * silently become a sixth never-copied key. + * + * ## The consumers, and why they are exactly these three + * + * `generateColumns()` hands `fieldMeta` to `CellRenderer` as the `field` prop, + * and `getCellRenderer` dispatches a relational column into + * `LookupCellRenderer` (`@object-ui/fields/src/index.tsx`). The grid's inline + * editor dispatches the same bag into `LookupField` and `UserField` + * (`@object-ui/fields/src/widgets/`). `UserField` reads a few keys itself and + * then spreads its whole meta into `LookupField` — so its own read set is a + * subset and it adds nothing; it is swept anyway, because a key it read and + * did NOT forward would otherwise be invisible here. + * + * ## ⭐ Why a key can be READ and still not be worth copying + * + * `@objectstack/spec` 17.2.0's `FieldSchema` is a **strict** object of 71 + * properties. A key it does not declare parses to `unrecognized_keys` — the + * same code a nonsense key gets — so `PUT /api/v1/meta/object/:name` refuses + * it and no spec-compliant producer can put it on a field def. Measured on the + * installed package, with `name` / `type` / `label` as the positive control. + * + * Nothing manufactures one on the way in, either. `getObjectSchema` in + * `@object-ui/data-objectstack` is the choke point every schema read passes + * through, and its only key rewrites are `normalizeSchemaReferenceKeys` (the + * `reference` ⇄ `reference_to` pair) and `applyFieldWidgetOverrides` (`widget`). + * Whatever the server serves for every other property arrives verbatim. + * + * ⇒ Copying a key that is neither spec-declared nor adapter-stamped writes a + * member from the def on every column build that no producer can ever fill. + * That is precisely what objectui#6711 (`reference_to_field`), objectui#6625 + * (`decimals`) and objectui#6597 (`referenceTo`) retired, and what + * objectui#6531 removed from `getRecordDisplayName` on the same reasoning. So + * `reference_field` and `lookup_columns` — two of the five keys objectui#6875 + * named — are deliberately NOT copied, and the gate proves their absence from + * `FieldSchema` rather than taking this docblock's word for it. + * + * ## ⚠️ The asymmetry this file does NOT resolve + * + * Four keys already in the copy set — `display_field`, `description_field`, + * `lookup_filters`, `id_field` — fail that same producer test. They are kept: + * retiring a key that has shipped is its own adjudication (that is what + * objectui#6711 and objectui#6874 each were), a host `DataSource` outside these + * two repos may still hand-feed them, and legacy metadata predating the strict + * schema is not measurable from here. Recorded as `legacy-alias` so the + * asymmetry is visible rather than implied. + * + * ## ⛔ Two keys were in this list and are RETIRED — do not re-add them + * + * Both were measured out under the same rule this file now enforces + * mechanically: a key belongs here only when a consumer on THIS grid's render + * path reads it off a FIELD meta. Under the derivation they can no longer be + * re-added by hand at all — neither appears in the read set the gate extracts, + * so adding either to the table below turns the gate red as an "extra". + * + * ### `reference_to_field` — objectui#6711 + * + * Swept across `packages/` and `apps/` (and again across the producer repo), + * the only occurrences of the identifier anywhere were the array literal — the + * write — and prose recording that nothing reads it. No member access, no + * destructuring, no bracket read. `FieldSchema` does not declare it either, so + * nothing authorable produces it. + * + * ### `titleFormat` — objectui#6874 + * + * A zero of a different kind, and a stronger one. `titleFormat` is a real, live + * key with plenty of readers — it simply has no FIELD-meta reader. The sweep + * found every member read of the identifier across `packages/` and `apps/` + * (tests included) and classified each by receiver: `objectDef` / + * `objectSchema` (`core/utils/record-title.ts`, `containers.tsx`, + * `plugin-detail/DetailView.tsx`, `ObjectKanban.tsx`, `ObjectCalendar.tsx`, + * `react/hooks/useRecordSearch.ts`) — OBJECT schema, every one; + * `refObjectSchema?.titleFormat` in `LookupField.tsx` — the REFERENCED object's + * schema, fetched by `getSchema(referenceTo)`, and the one that matters here; + * `param.titleFormat` in `app-shell/utils/paramToField.ts`, off a resolved + * `ActionParamDef`. ⇒ copying `reference_to` is what makes `titleFormat` work + * on this path; copying `titleFormat` onto the meta reached nothing. + * `plugin-dashboard/src/recordFields.tsx` recorded the same measurement first. + * + * Both absences stay pinned behaviourally at all three of `generateColumns`'s + * call sites — `__tests__/relationalMetaCopySet-6711.test.tsx` and + * `__tests__/relationalMetaCopySet-6874.test.tsx`. + * + * ⚠️ Every sweep quoted above bounds these two repos. A host application + * outside them could still be reading any of these keys off `fieldMeta`; the + * repo's own contract is what these verdicts are about. + */ + +/** What the grid does with a key its consumers read off the field meta. */ +export type RelationalMetaVerdict = + /** Spec-declared on `FieldSchema`. The spelling a live `getObjectSchema` serves. */ + | 'spec' + /** Not spec-declared, but stamped onto every def by the adapter's choke point. */ + | 'adapter-stamped' + /** Not producible under the installed contract; copied only for back-compat. */ + | 'legacy-alias' + /** Read, but no producer can emit it — copying it would reach nothing. */ + | 'no-producer' + /** Producible and read, but written onto the meta by another block already. */ + | 'handled-elsewhere' + /** Producible and read, but outside this helper's contract — see `note`. */ + | 'deferred'; + +/** Verdicts whose keys ARE copied. Everything else is deliberately skipped. */ +const COPIED_VERDICTS: ReadonlySet = new Set([ + 'spec', + 'adapter-stamped', + 'legacy-alias', +]); + +export interface RelationalMetaEntry { + readonly verdict: RelationalMetaVerdict; + readonly note: string; +} + +/** + * Every key the three consumers read off this bag, each with a verdict. + * + * ⛔ Do not add a key here to "restore symmetry" with the field def, and do not + * remove one because it looks unused — the gate reads the consumer sources, and + * this table has to match what it finds, exactly and in both directions. + */ +export const RELATIONAL_META_READ_SET: Readonly> = { + // ── The relational target ──────────────────────────────────────────────── + reference: { verdict: 'spec', note: "FieldSchema.reference — the served spelling for a lookup's target object." }, + reference_to: { verdict: 'adapter-stamped', note: 'normalizeSchemaReferenceKeys stamps it from `reference` at the getObjectSchema choke point.' }, + reference_field: { verdict: 'no-producer', note: 'Third leg of the display-field chain. Not on FieldSchema; zero occurrences in the producer repo (control: `displayField`, 68 files). objectui#6875.' }, + + // ── The display value ─────────────────────────────────────────────────── + displayField: { verdict: 'spec', note: 'FieldSchema.displayField. ⭐ Added by objectui#6875 — the only display spelling a spec-compliant producer can emit, and the one that never arrived.' }, + display_field: { verdict: 'legacy-alias', note: 'Runtime spelling, first leg of every display chain. Not on FieldSchema; kept for back-compat.' }, + + // ── The picker's secondary line ───────────────────────────────────────── + descriptionField: { verdict: 'spec', note: 'FieldSchema.descriptionField. ⭐ Added by objectui#6875.' }, + description_field: { verdict: 'legacy-alias', note: 'Runtime spelling. Not on FieldSchema; kept for back-compat.' }, + + // ── The picker's table ────────────────────────────────────────────────── + lookupColumns: { verdict: 'spec', note: 'FieldSchema.lookupColumns. ⭐ Added by objectui#6875.' }, + lookup_columns: { verdict: 'no-producer', note: 'Runtime twin of `lookupColumns`, read but never producible. Not on FieldSchema. objectui#6875.' }, + + // ── The picker's base scoping ─────────────────────────────────────────── + lookupFilters: { verdict: 'spec', note: 'FieldSchema.lookupFilters.' }, + lookup_filters: { verdict: 'legacy-alias', note: 'Runtime spelling. Not on FieldSchema; kept for back-compat.' }, + + // ── The picker's id column ────────────────────────────────────────────── + id_field: { verdict: 'legacy-alias', note: 'Picker id column. Neither spelling is on FieldSchema (`idField` is absent too); kept for back-compat.' }, + + // ── Read on this path, producible, and NOT copied ─────────────────────── + // Found by objectui#6875's re-sweep, outside the relational display/target + // contract this helper owns. Each is spec-declared with a measured reader — + // i.e. the same defect class as the three keys above, one seam over. + multiple: { verdict: 'deferred', note: 'FieldSchema.multiple — picker cardinality, not relational display/target meta.' }, + allowCreate: { verdict: 'deferred', note: 'FieldSchema.allowCreate — picker quick-create affordance.' }, + lookupPageSize: { verdict: 'deferred', note: 'FieldSchema.lookupPageSize — picker page size.' }, + dependsOn: { verdict: 'deferred', note: 'FieldSchema.dependsOn — cascading picker filter.' }, + + // ── Read on this path, no producer ────────────────────────────────────── + allow_create: { verdict: 'no-producer', note: 'Runtime twin of `allowCreate`. Not on FieldSchema.' }, + lookup_page_size: { verdict: 'no-producer', note: 'Runtime twin of `lookupPageSize`. Not on FieldSchema.' }, + depends_on: { verdict: 'no-producer', note: 'Runtime twin of `dependsOn`. Not on FieldSchema.' }, + picker: { verdict: 'no-producer', note: 'PeoplePicker variant opt-in. Not on FieldSchema.' }, + subtitle: { verdict: 'no-producer', note: 'PeoplePicker subtitle fields. Not on FieldSchema.' }, + avatarField: { verdict: 'no-producer', note: 'PeoplePicker avatar field. Not on FieldSchema.' }, + avatar_field: { verdict: 'no-producer', note: 'Runtime twin of `avatarField`. Not on FieldSchema.' }, + + // ── Written by another block of the same column build ─────────────────── + options: { verdict: 'handled-elsewhere', note: 'Written by generateColumns as `translateOptions(...)`, which localises the labels; a raw copy would undo that.' }, + dataSource: { verdict: 'handled-elsewhere', note: 'Not a schema key — LookupField reads its own `props.dataSource` fallback off the meta bag.' }, +}; + +/** + * The copy set, DERIVED from {@link RELATIONAL_META_READ_SET}. + * + * Order is the table's, which groups a chain's spellings together — it does not + * matter to `applyRelationalMeta` (each key is written independently), but it + * keeps a diff of this file readable. + */ +export const RELATIONAL_META_KEYS: readonly string[] = Object.freeze( + Object.entries(RELATIONAL_META_READ_SET) + .filter(([, entry]) => COPIED_VERDICTS.has(entry.verdict)) + .map(([key]) => key), +); + +/** + * Copy the relational metadata a lookup / master_detail / user cell needs off + * the object-schema field definition onto a column's built `fieldMeta`. + * + * A key is written only when the def actually carries it, so a non-relational + * field's meta gains no keys at all and an absent key never lands as an + * explicit `undefined` — the semantics `plugin-dashboard`'s sibling + * `pickCellRelationalMeta` copies. + */ +export function applyRelationalMeta( + fieldMeta: Record, + fieldDef: Record | undefined | null, +): void { + if (!fieldDef) return; + for (const key of RELATIONAL_META_KEYS) { + if (fieldDef[key] !== undefined) fieldMeta[key] = fieldDef[key]; + } +}