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 dbaec580d..5e05ea3e8 100644 Binary files a/packages/core/src/utils/record-title.ts and b/packages/core/src/utils/record-title.ts differ diff --git a/packages/fields/src/__tests__/lookupCellDisplayName.test.tsx b/packages/fields/src/__tests__/lookupCellDisplayName.test.tsx new file mode 100644 index 000000000..158fc10c6 --- /dev/null +++ b/packages/fields/src/__tests__/lookupCellDisplayName.test.tsx @@ -0,0 +1,119 @@ +/** + * Regression for objectstack-ai/objectui#2357: the lookup chip + * (LookupCellRenderer — record header highlight + detail sections) resolved + * display names via the local key heuristic (`pickRecordDisplayName`), whose + * `_number`/`_code` suffix scan surfaced an autonumber (`0001`) over the + * record's real name whenever the referenced object declared its title in + * `nameField`/`titleFormat`. The picker (LookupField) already resolved via the + * unified ADR-0079 resolver, so the two surfaces disagreed. + * + * The chip must now resolve through the referenced object's schema + * (`dataSource.getObjectSchema`) with the precedence + * `displayField → nameField/titleFormat → type-aware derivation`, keeping the + * key heuristic only as the no-schema fallback. + */ +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { LookupCellRenderer } from '../index'; +import { SchemaRendererProvider } from '@object-ui/react'; + +// The expanded record: the autonumber (`req_number`) precedes the real name +// (`req_name`) in key order, so the OLD heuristic-only path rendered "0001". +const EXPANDED = { id: 'rec1', req_number: '0001', req_name: 'Annual Maintenance' }; + +function makeDataSource(objectName: string, schema: any, record?: any) { + return { + find: vi.fn(), + findOne: vi.fn(async (object: string, id: string) => + 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 f6e17a074..42374fd25 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, preferredField?: string): 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); @@ -115,7 +210,9 @@ function useLookupName(referenceTo: string | undefined, value: unknown, preferre // The preferred display field is part of the cache identity: two columns // targeting the same record with different `display_field`s must not // serve each other's cached name (#2926 ⑧). - const cacheKey = isResolvable ? `${referenceTo}:${String(value)}:${preferredField ?? ''}` : ''; + const cacheKey = isResolvable + ? `${referenceTo}:${String(value)}:${displayField ?? ''}` + : ''; React.useEffect(() => { if (!isResolvable) return; @@ -138,7 +235,11 @@ function useLookupName(referenceTo: string | undefined, value: unknown, preferre : (result?.value || result?.data || []); record = records[0]; } - const name = pickRecordDisplayName(record, preferredField); + // 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' }); @@ -147,7 +248,7 @@ function useLookupName(referenceTo: string | undefined, value: unknown, preferre })(); 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); @@ -1170,6 +1271,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 @@ -1181,12 +1287,18 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R (field as { reference_to?: string }).reference_to || (field as { reference?: string }).reference; - // [#2926 ⑧] Honor the target object's configured display field. ObjectGrid - // forwards `display_field` on the column meta (RELATIONAL_META_KEYS) the - // same way it forwards `reference`; without threading it into - // pickRecordDisplayName the cell always fell back to the hardcoded - // heuristics (`name` first) and ignored displayNameField configuration. - const displayField = (field as { display_field?: string }).display_field; + // Explicit author-chosen display field on the lookup — beats every resolver. + // ObjectGrid forwards `display_field` on the column meta (RELATIONAL_META_KEYS) + // the same way it forwards `reference` (#2926 ⑧). + 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). @@ -1223,7 +1335,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, displayField) || + resolveLookupRecordName(parsed, refSchema, displayField) || String(parsed.externalId ?? parsed.id ?? parsed._id ?? ''); if (display) return {display}; } @@ -1235,7 +1347,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, displayField) || String(obj.id || obj._id || ''); + const display = + resolveLookupRecordName(obj, refSchema, displayField) || String(obj.id || obj._id || ''); if (display) { return {display}; } @@ -1269,7 +1382,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, displayField) || 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; @@ -1295,7 +1408,7 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R if (typeof value === 'object' && value !== null) { const label = - pickRecordDisplayName(value as Record, displayField) || + resolveLookupRecordName(value as Record, refSchema, displayField) || String((value as any).id || (value as any)._id || '[Object]'); return {label}; }