From 6172b4419e0e96a6fe234937d4fb979db88b34f7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 18:02:27 +0000 Subject: [PATCH] fix(plugin-dashboard): feed lookup cells their reference target in ObjectDataTable / RecordDetailDrawer (#6694) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `buildFieldMeta` never wrote `reference_to` / `reference` / `display_field` onto the `FieldMeta` both widgets hand to `renderFieldValue` -> `getCellRenderer` -> `LookupCellRenderer`, so that renderer resolved `undefined` and two things failed silently: `useRefObjectSchema` never loaded the referenced object's schema (the ADR-0079 / #2357 resolution never ran, cells fell back to the generic `.name`/`.title` heuristic), and `ReferencedRecordLink`'s `objectName` was always `undefined` (`navigable` always false, so no lookup cell in either widget ever rendered a real anchor). Adopts `plugin-grid`'s existing `applyRelationalMeta` move, made once in the seam both widgets funnel through. The copy set is 3 of the grid's 9, measured per key: the grid's cells are editable and its extra keys feed the inline picker, while these two widgets are read-only and `packages/fields/src/index.tsx` reads exactly these three off a cell's `field` prop. `FieldMeta` is internal — not re-exported through the barrel, and the `exports` map publishes only ".", so the subpath is refused with ERR_PACKAGE_PATH_NOT_EXPORTED. No published type is widened. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- .../6694-dashboard-lookup-reference-meta.md | 57 ++++ .../plugin-dashboard/src/ObjectDataTable.tsx | 14 +- .../lookupRelationalMeta-6694.test.tsx | 294 ++++++++++++++++++ .../plugin-dashboard/src/recordFields.tsx | 113 ++++++- 4 files changed, 474 insertions(+), 4 deletions(-) create mode 100644 .changeset/6694-dashboard-lookup-reference-meta.md create mode 100644 packages/plugin-dashboard/src/__tests__/lookupRelationalMeta-6694.test.tsx diff --git a/.changeset/6694-dashboard-lookup-reference-meta.md b/.changeset/6694-dashboard-lookup-reference-meta.md new file mode 100644 index 0000000000..a746a1dbd9 --- /dev/null +++ b/.changeset/6694-dashboard-lookup-reference-meta.md @@ -0,0 +1,57 @@ +--- +'@object-ui/plugin-dashboard': patch +--- + +Feed the lookup cells in `ObjectDataTable` and `RecordDetailDrawer` their reference target, +so schema-aware display-name resolution and drill-through links engage for the first time +(objectui#6694). + +Both widgets build their cell meta with `buildFieldMeta` and render it through +`renderFieldValue` → `getCellRenderer` → `LookupCellRenderer` (`@object-ui/fields`). That +renderer resolves its target from `field.reference_to || field.reference`, and `FieldMeta` +carried neither spelling — nor `display_field`. So the renderer resolved `undefined` and two +things failed, independently and both silently: + +- `useRefObjectSchema` never loaded the referenced object's schema, so the ADR-0079 / + objectui#2357 resolution never ran and every cell fell back to `pickRecordDisplayName`'s + generic `.name` / `.title` heuristic. Quiet, because that heuristic usually still produces + a readable name — it diverges only when the referenced object's display field is not + literally `name` / `title`, and then it silently shows the wrong one. +- `ReferencedRecordLink`'s `objectName` was always `undefined`, so `navigable` was always + `false` and no lookup cell in either widget ever rendered a real anchor — no + drill-through, no middle-click-new-tab, no copy-link. Quiet, because the cell still + rendered its value as plain text. + +This RESTORES intended behaviour rather than adding surface. `ReferencedRecordLink` was +placed in the shared cell renderer precisely so every surface would get the affordance once +("Both surfaces resolve through `LookupCellRenderer`, so the affordance belongs here, +once"), and `plugin-grid`'s `ObjectGrid` has fed it all along via `applyRelationalMeta` at +all three of its column-building call sites. These two widgets simply never adopted that +copy. Nothing new is authorable: the keys come off the OBJECT SCHEMA field def authors +already write, never off a column override — the distinction objectui#6597 measured when it +retired `referenceTo`, and the reason this needs no new column hold. + +The copy is made once in `buildFieldMeta`, the seam both widgets funnel through, so the two +surfaces cannot drift — which is what that module exists for. + +⚠️ The copy set is three keys where `ObjectGrid`'s `RELATIONAL_META_KEYS` is nine, and the +difference is measured per key, not preferred. The grid's cells are EDITABLE, so its extra +keys drive the inline picker's query (`LookupField` / `UserField` read `id_field`, +`description_field`, `lookup_filters`, `lookupFilters`); these two widgets are read-only and +their render path ends at a cell renderer. `packages/fields/src/index.tsx` reads exactly +`reference_to`, `reference` and `display_field` off a cell's `field` prop; `titleFormat` is +never read off a field meta at all (its readers take it off the object schema, which arrives +here through `useRefObjectSchema(reference_to)`), and `reference_to_field` has zero member +reads anywhere in the repo. Copying the other six would mint six members written on every +call and read by nothing — precisely what objectui#6625 (`decimals`) and objectui#6597 +(`referenceTo`) retired from this same file. + +No published type widened: `FieldMeta` is internal to the package — it is not re-exported +through the barrel (`dist/index.d.ts` names neither it nor `recordFields`) and the `exports` +map publishes only `"."`, so Node refuses `@object-ui/plugin-dashboard/recordFields` with +`ERR_PACKAGE_PATH_NOT_EXPORTED`. + +Behaviour note for existing dashboards: a lookup cell whose referenced object declares a +`nameField` other than `name` / `title` will now show that declared name instead of the +heuristic's pick, and valued lookup cells become links wherever the host publishes +`recordHref`. diff --git a/packages/plugin-dashboard/src/ObjectDataTable.tsx b/packages/plugin-dashboard/src/ObjectDataTable.tsx index 77380736ae..b844fe13e1 100644 --- a/packages/plugin-dashboard/src/ObjectDataTable.tsx +++ b/packages/plugin-dashboard/src/ObjectDataTable.tsx @@ -355,9 +355,19 @@ export interface ObjectDataTableColumnHolds {} * tombstone and the band together are the same verdict, unchanged since each * key's ruling. * - * The pool is what shrank, so what THIS band still refuses is `name` and + * The pool shrank, then grew again. What THIS band refuses is `name` and * `label` — both of them `FieldMeta` members with answers this seam already - * has (see the docblock above). + * has (see the docblock above) — plus, since objectui#6694, the three + * relational members that card added: `reference_to`, `reference` and + * `display_field`. + * + * ⭐ Those three are the derivation working as designed, and their verdict is + * the one objectui#6597 already reached for `referenceTo`: an AUTHORED column + * may not source a lookup's reference target. They are refused HERE while + * `buildFieldMeta` writes them freely, because that write's source is the + * OBJECT SCHEMA field def and never the column — the same distinction + * objectui#6597 measured, and the reason adding them needed no new hold. They + * reached this band without anyone extending a list. */ export type UnheldFieldMetaOverrideKey = Exclude; diff --git a/packages/plugin-dashboard/src/__tests__/lookupRelationalMeta-6694.test.tsx b/packages/plugin-dashboard/src/__tests__/lookupRelationalMeta-6694.test.tsx new file mode 100644 index 0000000000..d79f5f1ada --- /dev/null +++ b/packages/plugin-dashboard/src/__tests__/lookupRelationalMeta-6694.test.tsx @@ -0,0 +1,294 @@ +/** + * objectui#6694 — lookup cells in `ObjectDataTable` / `RecordDetailDrawer` must + * carry their reference target. + * + * Both widgets build their cell meta with `buildFieldMeta` and render it through + * `renderFieldValue` -> `getCellRenderer` -> `LookupCellRenderer` + * (`@object-ui/fields`). That renderer resolves its target from + * `field.reference_to || field.reference` and its display field from + * `field.display_field`. `buildFieldMeta` wrote NONE of those spellings, so the + * renderer resolved `undefined` and two things failed — independently, and both + * silently: + * + * 1. `useRefObjectSchema(referenceTo)` never loaded the referenced object's + * schema, so `resolveLookupRecordName` fell through the ADR-0079 resolver to + * `pickRecordDisplayName`'s generic `.name` / `.title` heuristic. + * 2. `ReferencedRecordLink`'s `objectName` was always `undefined`, so + * `navigable` was always `false` and the cell never rendered a real anchor — + * no drill-through, no middle-click-new-tab, no copy-link. + * + * ⚠️ Consequence 1 is pinned with a referenced object whose display field is + * `project_code` — NOT `name` / `title`. That is the whole point: the generic + * fallback usually still produces a readable name, so a fixture whose display + * field IS `name` passes before and after the fix and pins nothing. Each record + * below therefore ALSO carries a `name`, holding the value the broken path + * produced, and every assertion checks that the wrong one is absent. + * + * The two consequences get separate assertions because either can be fixed while + * the other stays broken: consequence 2 needs only `reference_to`, while + * consequence 1 additionally needs the referenced schema to actually load. + */ +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { + SchemaRendererContext, + RelatedRecordActionsProvider, + type RelatedRecordActionsValue, +} from '@object-ui/react'; + +vi.mock('@object-ui/react', async () => { + const actual: any = await vi.importActual('@object-ui/react'); + return { + ...actual, + // Only the table shell is stubbed. Everything the assertions depend on — + // `SchemaRendererContext`, `RelatedRecordActionsProvider` — is the REAL + // export by identity, so the contexts this file installs are the same + // objects `@object-ui/fields` reads from. + SchemaRenderer: ({ schema }: any) => { + const cols = schema.columns || []; + const rows = schema.data || []; + return ( + + + {rows.map((row: any, i: number) => ( + + {cols.map((c: any) => ( + + ))} + + ))} + +
+ {typeof c.cell === 'function' + ? c.cell(row[c.accessorKey], row) + : String(row[c.accessorKey] ?? '')} +
+ ); + }, + useDataScope: () => undefined, + }; +}); + +import { ObjectDataTable } from '../ObjectDataTable'; +import { RecordDetailDrawer } from '../RecordDetailDrawer'; +import { buildFieldMeta } from '../recordFields'; + +/** + * The host's route builder — the ONLY thing that can turn a lookup cell into an + * anchor. `ReferencedRecordLink` calls it with the resolved `objectName`, so it + * is never reached at all while that value is `undefined`. + */ +const HOST: RelatedRecordActionsValue = { + resolve: () => ({}), + recordHref: (objectName, recordId) => `/app/${objectName}/view/${recordId}`, +}; + +/** + * `refObjectSchemaCache` in `@object-ui/fields` is module-level and never + * cleared, so each case below references its OWN object name. A shared name + * would let one case's resolved schema satisfy another's assertion. + */ +function makeDataSource(opts: { + ownerObject: string; + refObject: string; + rows: any[]; +}) { + const ownerSchema = { + name: opts.ownerObject, + fields: { + project: { type: 'lookup', label: 'Project', reference_to: opts.refObject }, + }, + }; + const refSchema = { + name: opts.refObject, + // ADR-0079 canonical record-title pointer. Deliberately NOT `name`/`title`. + nameField: 'project_code', + fields: { + project_code: { type: 'text', label: 'Code' }, + name: { type: 'text', label: 'Name' }, + }, + }; + return { + find: async () => ({ data: opts.rows }), + getObjectSchema: async (n: string) => (n === opts.refObject ? refSchema : ownerSchema), + __ownerSchema: ownerSchema, + }; +} + +/** An `$expand`-ed lookup value whose generic `.name` is the WRONG answer. */ +function expandedProject(id: string) { + return { id, project_code: 'APOLLO-7', name: 'generic-fallback-name' }; +} + +describe('objectui#6694 — ObjectDataTable lookup cells carry their reference target', () => { + it('pin 1: resolves the display name through the REFERENCED object schema, not the generic .name heuristic', async () => { + const ds = makeDataSource({ + ownerObject: 'account_6694_a', + refObject: 'project_6694_a', + rows: [{ project: expandedProject('p-1') }], + }); + + render( + + + , + ); + + // `nameField: 'project_code'` wins — the ADR-0079 / issue #2357 resolution + // that only runs once `useRefObjectSchema` has a reference target to load. + await waitFor( + () => expect(screen.getByText('APOLLO-7')).toBeInTheDocument(), + { timeout: 3000 }, + ); + // …and the generic heuristic's answer is NOT what rendered. Without this the + // assertion above could pass on a fixture where both agree. + expect(screen.queryByText('generic-fallback-name')).not.toBeInTheDocument(); + }); + + it('pin 2: renders a real drill-through anchor built by the host', async () => { + const ds = makeDataSource({ + ownerObject: 'account_6694_b', + refObject: 'project_6694_b', + rows: [{ project: expandedProject('p-2') }], + }); + + render( + + + + + , + ); + + // `navigable` is `!!objectName && recordId != null`. The id was always + // there; the object name is what the missing `reference_to` withheld, so + // this anchor is the whole of consequence 2. + const link = await waitFor(() => screen.getByRole('link'), { timeout: 3000 }); + expect(link).toHaveAttribute('href', '/app/project_6694_b/view/p-2'); + }); +}); + +describe('objectui#6694 — RecordDetailDrawer lookup rows carry their reference target', () => { + it('pin 1: resolves the display name through the REFERENCED object schema', async () => { + const ds = makeDataSource({ + ownerObject: 'account_6694_c', + refObject: 'project_6694_c', + rows: [], + }); + + render( + + {}} + /> + , + ); + + await waitFor( + () => expect(screen.getByText('APOLLO-7')).toBeInTheDocument(), + { timeout: 3000 }, + ); + expect(screen.queryByText('generic-fallback-name')).not.toBeInTheDocument(); + }); + + it('pin 2: renders a real drill-through anchor built by the host', async () => { + const ds = makeDataSource({ + ownerObject: 'account_6694_d', + refObject: 'project_6694_d', + rows: [], + }); + + render( + + + {}} + /> + + , + ); + + const link = await waitFor(() => screen.getByRole('link'), { timeout: 3000 }); + expect(link).toHaveAttribute('href', '/app/project_6694_d/view/p-4'); + }); +}); + +/** + * The copy-set boundary. + * + * `ObjectGrid`'s `applyRelationalMeta` copies NINE keys; this seam copies THREE, + * and the difference is measured rather than preferred: the grid's cells are + * EDITABLE, so its extra keys feed the inline picker (`LookupField` / `UserField` + * read `id_field`, `description_field`, `lookup_filters`, `lookupFilters`). + * These two widgets are read-only — their only render path ends at a CELL + * renderer — and `packages/fields/src/index.tsx` reads exactly three relational + * keys off a cell's `field` prop. + * + * ⛔ This is what stops the omitted six from being added back "for parity": a + * `FieldMeta` member written on every call and read by nothing is precisely what + * objectui#6625 (`decimals`) and objectui#6597 (`referenceTo`) retired from this + * same file. If these widgets ever gain inline editing, that is the event that + * earns the picker keys — not symmetry with the grid. + */ +describe('objectui#6694 — buildFieldMeta copies the cell-read relational keys and no others', () => { + const def = { + type: 'lookup', + reference_to: 'project', + reference: 'project', + display_field: 'project_code', + // The six the grid also copies, which have no reader on this path: + reference_to_field: 'x', + id_field: 'x', + description_field: 'x', + lookup_filters: [['a', '=', 1]], + lookupFilters: [['a', '=', 1]], + titleFormat: '{project_code}', + }; + + it('copies reference_to / reference / display_field', () => { + 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'); + }); + + it('does NOT copy the picker-only keys', () => { + const meta = buildFieldMeta({ accessorKey: 'project', label: 'Project', def }) as any; + for (const k of [ + 'reference_to_field', 'id_field', 'description_field', + 'lookup_filters', 'lookupFilters', 'titleFormat', + ]) { + expect(meta).not.toHaveProperty(k); + } + }); + + it('adds no relational keys at all to a non-relational field', () => { + const meta = buildFieldMeta({ + accessorKey: 'amount', label: 'Amount', def: { type: 'currency' }, + }) as any; + for (const k of ['reference_to', 'reference', 'display_field']) { + expect(meta).not.toHaveProperty(k); + } + }); +}); diff --git a/packages/plugin-dashboard/src/recordFields.tsx b/packages/plugin-dashboard/src/recordFields.tsx index 2f8dd34cf5..f8de11b91a 100644 --- a/packages/plugin-dashboard/src/recordFields.tsx +++ b/packages/plugin-dashboard/src/recordFields.tsx @@ -65,6 +65,88 @@ export const NUMERIC_FIELD_TYPES = new Set([ 'currency', 'money', 'number', 'integer', 'decimal', 'float', 'percent', 'percentage', ]); +/** + * Relational keys copied from the OBJECT SCHEMA field def onto the built + * {@link FieldMeta}, so a lookup / master_detail / tree cell can resolve the + * record it references (objectui#6694). + * + * ## What was broken + * + * `LookupCellRenderer` (`@object-ui/fields`) resolves its target from + * `field.reference_to || field.reference` and its display field from + * `field.display_field`. `FieldMeta` carried NONE of those spellings, so for + * every lookup cell in `ObjectDataTable` and `RecordDetailDrawer` the renderer + * resolved `undefined` and two things failed silently: `useRefObjectSchema` + * never loaded the referenced object's schema (so the ADR-0079 / issue #2357 + * resolution never ran and the cell fell back to `pickRecordDisplayName`'s + * generic `.name`/`.title` heuristic), and `ReferencedRecordLink`'s `objectName` + * was always `undefined` (so `navigable` was always `false` and the cell never + * rendered a real anchor — no drill-through, no middle-click, no copy-link). + * + * ## This is ADOPTED, not invented + * + * `plugin-grid`'s `ObjectGrid` already makes this exact copy — + * `applyRelationalMeta`, off its `RELATIONAL_META_KEYS`, at all three of its + * column-building call sites — which is why its lookup cells have always had + * both behaviours. This is that same move, made once in the seam BOTH dashboard + * widgets funnel through, which is what this module exists for (see the file + * header: the two surfaces must never drift). + * + * ## ⚠️ The copy set is DELIBERATELY 3 of the grid's 9 — measured, per key + * + * `RELATIONAL_META_KEYS` is `reference_to`, `reference`, `reference_to_field`, + * `display_field`, `id_field`, `description_field`, `lookup_filters`, + * `lookupFilters`, `titleFormat`. The grid needs all nine because its cells are + * EDITABLE — its own docblock says the extra keys "drive the inline picker's + * query (LookupField reads reference_to/reference, display_field, id_field, + * description_field, lookup_filters)", and the defect that earned them was an + * inline-edited lookup showing a raw id. + * + * These two widgets are READ-ONLY. Their only render path is + * {@link renderFieldValue} → `getCellRenderer` → a CELL renderer; no field + * EDITOR is reachable from it. Measured on `packages/fields/src/index.tsx` — + * 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 + * `LookupCellRenderer` itself. ✅ COPIED. + * - `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. + * - `reference_to_field` — ZERO member reads anywhere in the repo. ⛔ NOT + * copied. + * - `titleFormat` — never read off a FIELD meta at all; every reader takes it + * off the OBJECT schema (`getRecordDisplayName` in `@object-ui/core`, + * `containers.tsx`). On this path that object schema arrives through + * `useRefObjectSchema(reference_to)` — so copying `reference_to` is what + * makes `titleFormat` work, and copying `titleFormat` here would reach + * nothing. ⛔ NOT copied. + * + * ⛔ Do not "restore parity" by widening this to the grid's nine. A member + * written from the schema def on every call and read by nothing is exactly what + * objectui#6625 (`decimals`) and objectui#6597 (`referenceTo`) retired from this + * very file. Add a key when a reader on THIS path is measured, not before; if + * these widgets ever gain inline editing, that is the event that earns the + * picker keys. The boundary is pinned in + * `__tests__/lookupRelationalMeta-6694.test.tsx`. + */ +const CELL_RELATIONAL_META_KEYS = ['reference_to', 'reference', 'display_field'] as const; + +/** + * Copy {@link CELL_RELATIONAL_META_KEYS} off a schema field def, with + * `applyRelationalMeta`'s own semantics: 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`. + */ +function pickCellRelationalMeta(def: any): Partial { + const out: Partial = {}; + if (!def) return out; + for (const key of CELL_RELATIONAL_META_KEYS) { + if (def[key] !== undefined) out[key] = def[key]; + } + return out; +} + /** * The override vocabulary this package's two field surfaces share. * @@ -109,6 +191,23 @@ export const NUMERIC_FIELD_TYPES = new Set([ * ever wanted here, it reads `reference_to` / `reference` off the schema field * def directly — the spelling `LookupCellRenderer` and `computeLookupExpand` * actually use. ⛔ Do not resurrect `referenceTo`. + * + * ⭐ That reader ARRIVED (objectui#6694): `reference_to` / `reference` / + * `display_field` below, copied from the SCHEMA field def by + * {@link buildFieldMeta} and justified per key on `CELL_RELATIONAL_META_KEYS`. + * They are the "future reader" both retirement notes predicted, in the spelling + * they named, and they change neither verdict — the source is the schema field + * def, never an authored column override, which is the exact distinction + * objectui#6597 measured and withdrew on. + * + * ⚠️ Being `FieldMeta` members they GROW both derived bands in + * `ObjectDataTable.tsx` — `EnrichedColumn`'s emit tombstones and + * `UnheldFieldMetaOverrideKey`'s read-side refusal. That is the intended + * verdict rather than a side effect: an AUTHORED column may not source a + * lookup's reference target (objectui#6597 measured no authoring story for + * one), while the schema-derived write is reached by neither band. They landed + * in both without anyone editing a list — the property this derivation exists + * for. */ export interface FieldMeta { name: string; @@ -117,6 +216,12 @@ export interface FieldMeta { options?: Array<{ value: any; label: string; color?: string }>; format?: string; currency?: string; + /** Lookup target object, snake_case — the spelling `LookupCellRenderer` reads first. */ + reference_to?: string; + /** Lookup target object, ObjectStack object-metadata spelling; the renderer's `||` fallback. */ + reference?: string; + /** Author-declared display field on the lookup — beats every resolver in the cell. */ + display_field?: string; } /** @@ -205,8 +310,12 @@ export function buildFieldMeta(params: BuildFieldMetaParams): FieldMeta { // withdraw). It resolved `overrides.referenceTo ?? meta?.referenceTo ?? // meta?.reference(.to) ?? meta?.target` on every call and reached no // reader: `LookupCellRenderer` resolves its target from - // `reference_to` / `reference`, never this spelling. A future reader - // reads `reference_to` / `reference` off the schema field def. + // `reference_to` / `reference`, never this spelling. ⭐ That future reader + // ARRIVED in objectui#6694 — the spread below, in the schema field def's own + // spelling, which is the one that retirement note pointed at. The + // retirement stands: this is a SCHEMA-derived write with no `overrides.` + // leg, so it makes no authored key live. + ...pickCellRelationalMeta(meta), }; }