From 6526526ea90a8780871b6fa086be5ebfd659d09c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8C=85=E5=91=A8=E6=B6=9B?= Date: Wed, 15 Jul 2026 10:28:26 -0700 Subject: [PATCH] fix(fields): resolve lookup chip display name via referenced object schema, not the autonumber-prone key heuristic (#2357) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lookup read cell (LookupCellRenderer — record header highlight + detail sections) resolved display names with the local pickRecordDisplayName heuristic, whose _number/_code suffix scan surfaced an autonumber ("0001") over the record's real name whenever the referenced object declared its title via nameField/titleFormat. The picker (LookupField) already resolved through the unified ADR-0079 resolver, so chip and picker disagreed. - LookupCellRenderer now fetches the referenced object's schema (dataSource.getObjectSchema, module-level cache shared across cells) and resolves expanded records through getRecordDisplayName with the precedence displayField → nameField/displayNameField → titleFormat → type-aware derivation; the key heuristic remains only as the no-schema fallback. - useLookupName (fetch-on-demand for bare foreign-key ids) resolves the fetched record through the same path and honours the field's explicit display_field. - @object-ui/core: NON_TITLE_TYPES now also excludes the ObjectStack spelling auto_number, so type-aware derivation can never pick an autonumber as a record title. Closes #2357 --- .../src/utils/__tests__/record-title.test.ts | 1 + packages/core/src/utils/record-title.ts | Bin 18457 -> 18664 bytes .../__tests__/lookupCellDisplayName.test.tsx | 119 +++++++++++++++ packages/fields/src/index.tsx | 138 ++++++++++++++++-- 4 files changed, 248 insertions(+), 10 deletions(-) create mode 100644 packages/fields/src/__tests__/lookupCellDisplayName.test.tsx diff --git a/packages/core/src/utils/__tests__/record-title.test.ts b/packages/core/src/utils/__tests__/record-title.test.ts index 7899e8805..86e9210b1 100644 --- a/packages/core/src/utils/__tests__/record-title.test.ts +++ b/packages/core/src/utils/__tests__/record-title.test.ts @@ -347,6 +347,7 @@ describe('isTitleEligibleField — type gate', () => { ['lookup', false], ['master_detail', false], ['autonumber', false], + ['auto_number', false], ['phone', false], ])('type %s -> eligible=%s', (type, eligible) => { expect(isTitleEligibleField({ type })).toBe(eligible); diff --git a/packages/core/src/utils/record-title.ts b/packages/core/src/utils/record-title.ts index dbaec580d0c691909efc575c665ec0fe79f38191..5e05ea3e856d27818dd7f5d5e1516f9b8aa6aeaa 100644 GIT binary patch delta 212 zcmXxdF%rR06oui@Dy-pj$RJT#fLdVzawS(>US7<5H;hrR1ZFfEz0QmssVqYJ1f zpUbiKI@RvC#y$)&nl3oeu~M>x<4nA|ZJ4uCN=oo6Hb!iQVg{WvJmn1Ct{5%P)tYl1 zs46j)T1;%2Puazo37fRLku+odM@glyZ9ETWkj01Fa`@pevSh?j6!l|>2mOkUlftD4 ae$Ai*%j*h@dZ?)3Ud5W<*T=Q78@vGwC`#S{ delta 14 WcmaDck#Xh(#tkd^HdpW$SONesdj; + object === objectName && record && id === record.id ? record : null, + ), + getObjectSchema: vi.fn(async (object: string) => + object === objectName ? schema : null, + ), + } as any; +} + +describe('LookupCellRenderer — schema-aware display name (issue #2357)', () => { + it('expanded object: nameField beats the autonumber suffix heuristic', async () => { + const ds = makeDataSource('mtc_request_a', { nameField: 'req_name' }); + render( + + + , + ); + await waitFor(() => { + expect(screen.getByText('Annual Maintenance')).toBeInTheDocument(); + }); + expect(screen.queryByText('0001')).not.toBeInTheDocument(); + }); + + it('expanded object: titleFormat template resolves when no nameField', async () => { + const ds = makeDataSource('mtc_request_b', { titleFormat: '{req_name} - {req_number}' }); + render( + + + , + ); + await waitFor(() => { + expect(screen.getByText('Annual Maintenance - 0001')).toBeInTheDocument(); + }); + }); + + it('explicit displayField wins over the schema nameField', async () => { + const ds = makeDataSource('mtc_request_c', { nameField: 'req_name' }); + render( + + + , + ); + // Author explicitly asked for the number — honour it (also the documented + // workaround in the issue). + expect(await screen.findByText('0001')).toBeInTheDocument(); + }); + + it('fetch-on-demand primitive id resolves via the schema nameField too', async () => { + // Opaque ULID-ish id so the old path would have resolved the fetched + // record through the heuristic and shown its autonumber. + const id = '01ARZ3NDEKTSV4RRFFQ69G5FBB'; + const record = { id, req_number: '0007', req_name: 'Pump Overhaul' }; + const ds = makeDataSource('mtc_request_d', { nameField: 'req_name' }, record); + render( + + + , + ); + await waitFor(() => { + expect(screen.getByText('Pump Overhaul')).toBeInTheDocument(); + }); + expect(screen.queryByText('0007')).not.toBeInTheDocument(); + expect(ds.getObjectSchema).toHaveBeenCalledWith('mtc_request_d'); + }); + + it('falls back to the key heuristic when the data source has no schema API', () => { + const ds = { find: vi.fn(), findOne: vi.fn() } as any; // no getObjectSchema + render( + + + , + ); + expect(screen.getByText('SOX')).toBeInTheDocument(); + }); +}); diff --git a/packages/fields/src/index.tsx b/packages/fields/src/index.tsx index d67b95b7a..42ea43de5 100644 --- a/packages/fields/src/index.tsx +++ b/packages/fields/src/index.tsx @@ -8,7 +8,7 @@ import React from 'react'; import type { FieldMetadata, SelectOptionMetadata } from '@object-ui/types'; -import { ComponentRegistry, percentDisplayValue } from '@object-ui/core'; +import { ComponentRegistry, percentDisplayValue, getRecordDisplayName } from '@object-ui/core'; import { useLocalization } from '@object-ui/i18n'; import { Badge, Avatar, AvatarImage, AvatarFallback, Button, Checkbox, EmptyValue, cn } from '@object-ui/components'; import { Check, X, Copy, Phone as PhoneIcon, MapPin } from 'lucide-react'; @@ -78,6 +78,97 @@ export function pickRecordDisplayName( return undefined; } +// Module-level cache of referenced-object schemas (keyed by object name) so +// many lookup cells pointing at the same object trigger ONE metadata fetch. +type RefSchemaCacheEntry = + | { state: 'pending'; promise: Promise } + | { state: 'ok'; schema: unknown } + | { state: 'err' }; +const refObjectSchemaCache: Map = new Map(); + +/** + * Fetch (and cache) the referenced object's schema via + * `dataSource.getObjectSchema`. Resolves to `undefined` when the data source + * doesn't expose schema metadata or the fetch fails — callers fall back to the + * key heuristic in that case. + */ +async function fetchRefObjectSchema(dataSource: any, referenceTo: string): Promise { + const settled = () => { + const entry = refObjectSchemaCache.get(referenceTo); + return entry?.state === 'ok' ? entry.schema : undefined; + }; + const existing = refObjectSchemaCache.get(referenceTo); + if (existing?.state === 'pending') { + await existing.promise; + return settled(); + } + if (existing) return settled(); + const getSchema = dataSource?.getObjectSchema; + if (typeof getSchema !== 'function') return undefined; + const promise = Promise.resolve() + .then(() => getSchema.call(dataSource, referenceTo)) + .then((schema: unknown) => { refObjectSchemaCache.set(referenceTo, { state: 'ok', schema }); }) + .catch(() => { refObjectSchemaCache.set(referenceTo, { state: 'err' }); }); + refObjectSchemaCache.set(referenceTo, { state: 'pending', promise }); + await promise; + return settled(); +} + +/** + * React hook wrapper over {@link fetchRefObjectSchema}: returns the referenced + * object's schema once loaded (re-rendering when it lands), `undefined` while + * pending or when schema metadata isn't available. + */ +function useRefObjectSchema(referenceTo: string | undefined): any { + const ctx = React.useContext(_SchemaRendererContext); + const dataSource = ctx?.dataSource as any; + const [, force] = React.useState(0); + const canFetch = + !!referenceTo && !!dataSource && typeof dataSource.getObjectSchema === 'function'; + + React.useEffect(() => { + if (!canFetch) return; + const entry = refObjectSchemaCache.get(referenceTo!); + if (entry && entry.state !== 'pending') return; // settled before this render + let alive = true; + fetchRefObjectSchema(dataSource, referenceTo!).then(() => { + if (alive) force((n) => n + 1); + }); + return () => { alive = false; }; + }, [canFetch, referenceTo, dataSource]); + + if (!canFetch) return undefined; + const entry = refObjectSchemaCache.get(referenceTo!); + return entry?.state === 'ok' ? entry.schema : undefined; +} + +/** + * Resolve an expanded/fetched lookup record to its display name the same way + * the picker does (issue #2357): explicit `displayField` first, then the + * unified ADR-0079 resolver against the referenced object's schema + * (`nameField`/`displayNameField` → `titleFormat` → type-aware derivation, + * which excludes autonumber). Only when no schema is available does it fall + * back to {@link pickRecordDisplayName}, whose `_number`/`_code` suffix scan + * can otherwise surface an autonumber (`0001`) over the record's real name. + */ +function resolveLookupRecordName( + record: Record | null | undefined, + refSchema?: any, + displayField?: string, +): string | undefined { + if (!record || typeof record !== 'object') return undefined; + if (refSchema) { + const resolved = getRecordDisplayName(refSchema, record, { titleField: displayField }); + // Stop short of the resolver's `Record #` / `Untitled` floor so the + // cell keeps its own id-placeholder handling for nameless records. + const id = (record as any).id ?? (record as any)._id; + const isFloor = + resolved === 'Untitled' || (id != null && resolved === `Record #${id}`); + if (!isFloor && resolved) return resolved; + } + return pickRecordDisplayName(record, displayField); +} + /** * Heuristic: detect strings that look like opaque foreign-key IDs (e.g. nanoid * or BSON ObjectId). Used so we don't display random gibberish to users when @@ -101,7 +192,11 @@ export function isLikelyOpaqueId(v: unknown): boolean { * the context isn't installed. Returns the resolved display name or * `undefined` while pending or unresolvable. */ -function useLookupName(referenceTo: string | undefined, value: unknown): string | undefined { +function useLookupName( + referenceTo: string | undefined, + value: unknown, + displayField?: string, +): string | undefined { const ctx = React.useContext(_SchemaRendererContext); const dataSource = ctx?.dataSource; const [, force] = React.useState(0); @@ -112,7 +207,9 @@ function useLookupName(referenceTo: string | undefined, value: unknown): string typeof dataSource.find === 'function' && (typeof value === 'string' || typeof value === 'number') && value !== ''; - const cacheKey = isResolvable ? `${referenceTo}:${String(value)}` : ''; + const cacheKey = isResolvable + ? `${referenceTo}:${String(value)}:${displayField ?? ''}` + : ''; React.useEffect(() => { if (!isResolvable) return; @@ -135,7 +232,11 @@ function useLookupName(referenceTo: string | undefined, value: unknown): string : (result?.value || result?.data || []); record = records[0]; } - const name = pickRecordDisplayName(record); + // Resolve through the referenced object's schema (nameField / + // titleFormat) so the chip agrees with the picker — the bare key + // heuristic alone can surface an autonumber over the real name. + const schema = await fetchRefObjectSchema(dataSource, referenceTo!); + const name = resolveLookupRecordName(record, schema, displayField); lookupNameCache.set(cacheKey, { state: 'ok', name }); } catch { lookupNameCache.set(cacheKey, { state: 'err' }); @@ -144,7 +245,7 @@ function useLookupName(referenceTo: string | undefined, value: unknown): string })(); lookupNameCache.set(cacheKey, { state: 'pending', promise }); - }, [cacheKey, isResolvable, referenceTo, value, dataSource]); + }, [cacheKey, isResolvable, referenceTo, value, displayField, dataSource]); if (!isResolvable) return undefined; const entry = lookupNameCache.get(cacheKey); @@ -1167,6 +1268,11 @@ export function ImageCellRenderer({ value }: CellRendererProps): React.ReactElem * 3. Fetch-on-demand: when the value is a primitive ID and `field.reference_to` * is known, resolve via dataSource and show the related record's display name. * Falls back to a muted placeholder while pending and on failure. + * + * Record → name resolution (1 and 3) goes through the referenced object's + * schema when the data source exposes it (`displayField` → nameField/titleFormat + * → derivation, see {@link resolveLookupRecordName}), so the chip and the + * picker agree (issue #2357). */ export function LookupCellRenderer({ value, field }: CellRendererProps): React.ReactElement { // ObjectStack object metadata uses `reference` for the lookup target while the @@ -1178,6 +1284,17 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R (field as { reference_to?: string }).reference_to || (field as { reference?: string }).reference; + // Explicit author-chosen display field on the lookup — beats every resolver. + const displayField = + (field as { display_field?: string }).display_field || + (field as { displayField?: string }).displayField || + (field as { reference_field?: string }).reference_field || + undefined; + + // Referenced object's schema (nameField / titleFormat) so expanded records + // resolve through the same unified resolver as the picker (issue #2357). + const refSchema = useRefObjectSchema(referenceTo); + // Pick the FIRST primitive id we see (for arrays, only the first one is auto-resolved // to keep the cell cheap; multi-value lookups should generally be expanded server-side). const primaryPrimitiveId = (() => { @@ -1199,7 +1316,7 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R })(); // Always call the hook (rules of hooks). It safely no-ops when inputs are missing. - const resolvedName = useLookupName(referenceTo, primaryPrimitiveId); + const resolvedName = useLookupName(referenceTo, primaryPrimitiveId, displayField); if (value == null || value === '') return ; @@ -1213,7 +1330,7 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R const parsed = JSON.parse(s) as Record; if (parsed && typeof parsed === 'object') { const display = - pickRecordDisplayName(parsed) || + resolveLookupRecordName(parsed, refSchema, displayField) || String(parsed.externalId ?? parsed.id ?? parsed._id ?? ''); if (display) return {display}; } @@ -1225,7 +1342,8 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R // (e.g. { id, name }). Render its display name directly — no fetch needed. if (!Array.isArray(value) && typeof value === 'object') { const obj = value as Record; - const display = pickRecordDisplayName(obj) || String(obj.id || obj._id || ''); + const display = + resolveLookupRecordName(obj, refSchema, displayField) || String(obj.id || obj._id || ''); if (display) { return {display}; } @@ -1259,7 +1377,7 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R let label: string; let muted = false; if (item != null && typeof item === 'object') { - label = pickRecordDisplayName(item as Record) || String((item as any).id || (item as any)._id || '[Object]'); + label = resolveLookupRecordName(item as Record, refSchema, displayField) || String((item as any).id || (item as any)._id || '[Object]'); } else { const r = resolveLabel(item); label = r.text; @@ -1285,7 +1403,7 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R if (typeof value === 'object' && value !== null) { const label = - pickRecordDisplayName(value as Record) || + resolveLookupRecordName(value as Record, refSchema, displayField) || String((value as any).id || (value as any)._id || '[Object]'); return {label}; }