diff --git a/.changeset/6711-retire-reference-to-field.md b/.changeset/6711-retire-reference-to-field.md new file mode 100644 index 000000000..61af2cd9a --- /dev/null +++ b/.changeset/6711-retire-reference-to-field.md @@ -0,0 +1,40 @@ +--- +'@object-ui/plugin-grid': patch +--- + +`ObjectGrid` no longer copies `reference_to_field` onto a relational column's `fieldMeta` +(objectui#6711). + +`RELATIONAL_META_KEYS` listed nine keys that `applyRelationalMeta` copies off the +object-schema field def onto the built `fieldMeta`, at all three of `generateColumns`'s +column-building call sites. `reference_to_field` had **zero member reads**: swept across +`packages/` and `apps/` (and again across the producer repo), the only occurrences of the +identifier anywhere were the array literal itself — the write — and prose recording that +nothing reads it. No member access, no destructuring, no bracket read. + +The control that makes that zero a reading rather than an artefact of how the sweep was +written: the same sweep over its list-mates finds real readers for each of them — +`reference_to` / `reference` / `display_field` in `LookupCellRenderer`, and `id_field` / +`description_field` / `lookup_filters` / `lookupFilters` in `LookupField` / `UserField`, +which is what the grid's editable cells need. + +Nothing renders differently. The key is not a member of any declared type on either end: +`applyRelationalMeta` writes into a `Record`, the bag reaches cell renderers +through an `as any` cast, and the declared `FieldMetadata` union it is cast to does not +declare it (nor does `BaseFieldMetadata` carry an index signature). `@objectstack/spec` +17.2.0's `FieldSchema` does not declare it either — it is in none of that schema's 64 +props — so nothing authorable produces it. This is the same defect class the sibling +producer retired twice: objectui#6625 (`FieldMeta.decimals`) and objectui#6597 +(`FieldMeta.referenceTo`). + +⚠️ **What the measurement bounds.** The sweep covers this repo and the producer repo. A +host application outside them could still be reading `reference_to_field` off the +`fieldMeta` a cell renderer receives; that was never a declared promise this renderer made, +and this repo's own contract is what the retirement is about — but the world was not +measured, and a host reading the key gets `undefined` after this change. + +Because the key had no readers, the suite stays green whether or not the removal is +correct, so the absence is pinned directly instead +(`__tests__/relationalMetaCopySet-6711.test.tsx`): all three call sites, each with a +presence assertion on the eight surviving keys as the control against a fixture that +passes by never reaching the copy path. diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 2b55805e4..012b1acbd 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -429,9 +429,41 @@ function getDataConfig(schema: ObjectGridSchema): ViewData | null { * (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. + * + * ## ⛔ `reference_to_field` was in this list and is RETIRED (objectui#6711) + * + * 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. `reference_to_field` had none: 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. + * + * The control that makes that zero a reading, not an artefact of how the sweep + * was written: the same sweep over its list-mates finds real readers for each of + * them — `reference_to` / `reference` / `display_field` in `LookupCellRenderer`, + * `id_field` / `description_field` / `lookup_filters` / `lookupFilters` in + * `LookupField` / `UserField`. ⚠️ One exception, measured and deliberately NOT + * acted on here: `titleFormat` has no FIELD-meta reader either — every reader + * takes it off the OBJECT schema, which reaches the picker through + * `useRefObjectSchema(reference_to)` (`plugin-dashboard/src/recordFields.tsx` + * records the same measurement). Retiring it is a separate adjudication. + * + * ⚠️ The sweep bounds these two repos. A host application outside them could + * still be reading the key off `fieldMeta`; the repo's own contract is what this + * retirement is 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. The absence is pinned in + * `__tests__/relationalMetaCopySet-6711.test.tsx`. */ const RELATIONAL_META_KEYS = [ - 'reference_to', 'reference', 'reference_to_field', + 'reference_to', 'reference', 'display_field', 'id_field', 'description_field', 'lookup_filters', 'lookupFilters', 'titleFormat', ] as const; diff --git a/packages/plugin-grid/src/__tests__/relationalMetaCopySet-6711.test.tsx b/packages/plugin-grid/src/__tests__/relationalMetaCopySet-6711.test.tsx new file mode 100644 index 000000000..dd9168c7a --- /dev/null +++ b/packages/plugin-grid/src/__tests__/relationalMetaCopySet-6711.test.tsx @@ -0,0 +1,170 @@ +/** + * objectui#6711 — `ObjectGrid`'s relational copy set must NOT carry + * `reference_to_field`. + * + * ## ⚠️ Why this is a pin and not a behaviour test + * + * The retired key had ZERO readers anywhere in the repo, so removing it changes + * no rendering at all: every other test in this package stays green whether or + * not the removal is correct. A green suite therefore proves nothing here. What + * CAN be asserted is the thing that was actually measured — the key is no + * longer WRITTEN onto the `fieldMeta` a cell renderer receives — so this file + * asserts that absence directly, at all three of `generateColumns`'s + * column-building call sites, and goes red the moment the key is re-added to + * `RELATIONAL_META_KEYS`. + * + * ## The control against vacuity lives in the same assertions + * + * Each case also asserts that the eight SURVIVING keys do arrive on the same + * meta. An absence assertion on its own passes for the wrong reason as soon as + * the fixture stops reaching the copy path at all (a renamed helper, a column + * path that no longer resolves this renderer, a def the grid never reads); the + * presence half is what makes each `not.toHaveProperty` a measurement rather + * than a tautology. + * + * The probe replaces the registered `lookup` cell renderer, which is exactly + * how the real `LookupCellRenderer` receives this bag — `getCellRenderer` checks + * the registry first — so what it captures is the `field` prop the shipped + * renderer would have been handed. + */ +import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest'; +import { render, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { ObjectGrid } from '../ObjectGrid'; +import { + registerAllFields, + registerFieldRenderer, + getCellRenderer, + type CellRendererProps, +} from '@object-ui/fields'; +import { ActionProvider, SchemaRendererProvider } from '@object-ui/react'; + +registerAllFields(); + +const OBJECT = 'os_6711_report'; + +/** + * One field def carrying EVERY relational key the grid has ever copied, + * including the retired one. A def that omitted `reference_to_field` could not + * tell "the grid stopped copying it" apart from "the fixture never offered it". + */ +const MANAGER_DEF = { + type: 'lookup', + label: 'Manager', + reference_to: 'users', + reference: 'users', + display_field: 'name', + id_field: 'id', + description_field: 'title', + lookup_filters: [['active', '=', true]], + lookupFilters: [['active', '=', true]], + titleFormat: '{name}', + // The retired key (objectui#6711). Kept on the fixture on purpose. + reference_to_field: 'MUST_NOT_BE_COPIED', +}; + +/** The eight keys that survive the retirement — the control. */ +const SURVIVING_KEYS = [ + 'reference_to', 'reference', + 'display_field', 'id_field', 'description_field', + 'lookup_filters', 'lookupFilters', 'titleFormat', +] as const; + +const ROWS = [{ id: 'r1', name: 'Tower T1', manager: 'u1' }]; + +const captured: Record[] = []; + +function ProbeCell({ value, field }: CellRendererProps): React.ReactElement { + captured.push(field as unknown as Record); + return {String(value ?? '')}; +} + +let originalLookupRenderer: React.FC; + +beforeAll(() => { + if (!Element.prototype.scrollIntoView) { + Element.prototype.scrollIntoView = vi.fn() as any; + } + originalLookupRenderer = getCellRenderer('lookup'); + registerFieldRenderer('lookup', ProbeCell); +}); + +afterAll(() => { + registerFieldRenderer('lookup', originalLookupRenderer); +}); + +function makeDataSource() { + return { + find: vi.fn(async () => ({ data: ROWS, total: ROWS.length, hasMore: false, pageSize: 50 })), + getObjectSchema: async (name: string) => ({ + name, + fields: { + id: { type: 'text' }, + name: { type: 'text', label: 'Name' }, + manager: { ...MANAGER_DEF }, + }, + }), + } as any; +} + +async function renderAndCaptureMeta(schemaExtra: Record) { + captured.length = 0; + const ds = makeDataSource(); + const schema: any = { + type: 'object-grid', + objectName: OBJECT, + data: ROWS, + pagination: { pageSize: 50 }, + ...schemaExtra, + }; + render( + + + + + , + ); + // ⚠️ Wait for the ENRICHED meta, not merely the first one. The object schema + // arrives from an async fetch, so the first paint hands the renderer a bare + // `{ name, type }` — on which `not.toHaveProperty('reference_to_field')` + // passes for the wrong reason. `label` is the signal because it is written + // from the same `objectDefField` block, immediately BEFORE + // `applyRelationalMeta`, and is not itself one of the keys under test — so + // the wait cannot manufacture the assertions below. + await waitFor(() => { + expect(captured.length).toBeGreaterThan(0); + expect(captured[captured.length - 1]).toHaveProperty('label'); + }); + return captured[captured.length - 1]; +} + +/** + * The three call sites of `applyRelationalMeta`, reached by the three shapes + * `generateColumns` branches on: ListColumn objects, a string array, and the + * inline-data path (rows handed down + an authored `fields` projection). + */ +const CALL_SITES: Array<[string, Record]> = [ + ['ListColumn objects', { columns: [{ field: 'manager', label: 'Manager', type: 'lookup' }] }], + ['string columns', { columns: ['manager'] }], + ['inline data + fields projection', { fields: ['manager'] }], +]; + +describe('objectui#6711 — ObjectGrid no longer copies `reference_to_field` onto fieldMeta', () => { + for (const [name, schemaExtra] of CALL_SITES) { + it(`does not copy it (${name})`, async () => { + const meta = await renderAndCaptureMeta(schemaExtra); + expect(meta).not.toHaveProperty('reference_to_field'); + }); + + it(`still copies the eight surviving relational keys (${name})`, async () => { + const meta = await renderAndCaptureMeta(schemaExtra); + for (const key of SURVIVING_KEYS) { + expect(meta).toHaveProperty(key); + } + expect(meta.reference_to).toBe('users'); + expect(meta.display_field).toBe('name'); + }); + } +});