From e41d98a631054243437f24858a54e633838c8fd8 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Thu, 2 Jul 2026 00:22:55 +0800 Subject: [PATCH] fix(fields): align inline lookup value resolution with the read cell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #2125. `LookupField`'s inline display now resolves every value shape the read cell (`LookupCellRenderer`) does, closing the remaining gaps: - JSON-encoded external-id references ('{"externalId":"…"}') are parsed and shown by their external id, and excluded from the hydration fetch so we never findOne() a raw JSON string. `recordToOption` gained an `externalId` fallback for both value and label. - Tolerant id matching: a String()-coerced fallback (`findOptionLoose`) resolves a numeric cell value against a string-keyed option (and vice versa), matching the read cell's comparison. Only consulted when the strict match misses. - `normalizeId` also unwraps JSON-encoded reference strings. Adds inline-editor tests for user / owner fields (they delegate to LookupField via UserField), completing coverage for the relational set wired inline in #2122. Co-Authored-By: Claude Opus 4.8 --- .changeset/lookup-inline-resolution-align.md | 21 ++++++ packages/fields/src/complex-widgets.test.tsx | 73 ++++++++++++++++++++ packages/fields/src/widgets/LookupField.tsx | 72 +++++++++++++++---- 3 files changed, 154 insertions(+), 12 deletions(-) create mode 100644 .changeset/lookup-inline-resolution-align.md diff --git a/.changeset/lookup-inline-resolution-align.md b/.changeset/lookup-inline-resolution-align.md new file mode 100644 index 0000000000..a112663f3a --- /dev/null +++ b/.changeset/lookup-inline-resolution-align.md @@ -0,0 +1,21 @@ +--- +"@object-ui/fields": patch +--- + +fix(fields): align inline lookup value resolution with the read cell (external-id strings, tolerant id match) + +Follow-up to #2125. `LookupField`'s inline display now resolves every value +shape the read cell (`LookupCellRenderer`) does: + +- **JSON-encoded external-id references** (`'{"externalId":"Website Relaunch"}'`) + are parsed and shown by their external id, and excluded from the hydration + fetch (so we never `findOne` with a raw JSON string). `recordToOption` gained + an `externalId` fallback for both the value and the label. +- **Tolerant id matching** — a `String()`-coerced fallback (`findOptionLoose`) + resolves a numeric cell value against a string-keyed option (and vice versa), + matching the read cell's `String(a) === String(b)` comparison. Only consulted + when the strict match misses, so homogeneous option lists are unaffected. + +Also adds explicit inline-editor tests for `user` / `owner` fields (they +delegate to `LookupField` via `UserField`), completing coverage for the full +relational set wired inline in #2122. diff --git a/packages/fields/src/complex-widgets.test.tsx b/packages/fields/src/complex-widgets.test.tsx index eb402d5a39..d41d2bcc20 100644 --- a/packages/fields/src/complex-widgets.test.tsx +++ b/packages/fields/src/complex-widgets.test.tsx @@ -620,6 +620,79 @@ describe('Complex & Relationship Widgets', () => { }); }); + describe('LookupField — inline value-shape alignment with the read cell', () => { + // Rounds out #2125: user/owner inline editing, JSON-encoded external-id + // reference values, and String()-tolerant id matching — so the inline + // editor resolves every value shape the read cell (LookupCellRenderer) + // does, instead of falling back to the "Select…" placeholder. + const mockDataSource = { + find: vi.fn(), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + }; + + beforeEach(() => { + vi.clearAllMocks(); + try { localStorage.clear(); } catch { /* jsdom */ } + }); + + it('user field resolves an expanded-object value inline (via UserField → LookupField)', () => { + render( + + ); + expect(screen.getByText('Ada Lovelace')).toBeInTheDocument(); + }); + + it('owner field resolves an expanded-object value inline', () => { + render( + + ); + expect(screen.getByText('Grace Hopper')).toBeInTheDocument(); + }); + + it('resolves a JSON-encoded external-id reference string without a bogus fetch', () => { + render( + + ); + // The external id is used as the display label (mirrors the read cell). + expect(screen.getByText('Website Relaunch')).toBeInTheDocument(); + // Never fetch by passing the raw JSON string as an id. + expect(mockDataSource.findOne).not.toHaveBeenCalled(); + expect(mockDataSource.find).not.toHaveBeenCalled(); + }); + + it('resolves a numeric cell value against a string-keyed option (tolerant match)', () => { + render( + + ); + // Strict === misses (number 1 vs string '1'); the loose fallback resolves it. + expect(screen.getByText('One')).toBeInTheDocument(); + }); + }); + describe('MasterDetailField', () => { const items = [ { id: '1', label: 'Item 1' }, diff --git a/packages/fields/src/widgets/LookupField.tsx b/packages/fields/src/widgets/LookupField.tsx index 3254cc1003..27352c4608 100644 --- a/packages/fields/src/widgets/LookupField.tsx +++ b/packages/fields/src/widgets/LookupField.tsx @@ -90,7 +90,7 @@ function recordToOption( titleFormat?: string | null, objectDef?: any, ): LookupOption { - const val = record[idField] ?? record.id ?? record._id; + const val = record[idField] ?? record.id ?? record._id ?? record.externalId; const templated = titleFormat ? formatRecordTitle(record, titleFormat) : null; // Object-level resolver fallback (displayNameField + derivation), excluding @@ -114,11 +114,31 @@ function recordToOption( record.full_name ?? record.title ?? record.subject ?? + record.externalId ?? String(val); const description = descriptionField ? record[descriptionField] : undefined; return { value: val, label: String(label), description, ...record }; } +/** + * A reference value can arrive JSON-encoded — e.g. an unresolved external-id + * reference `'{"externalId":"Website Relaunch"}'`. Parse such a string into its + * object form so the inline editor resolves it through the same path as a + * server-`$expand`ed record. Returns null for anything that isn't a JSON object + * string. Mirrors the read cell (`LookupCellRenderer`) so the two stay aligned. + */ +function parseReferenceObjectString(v: any): Record | null { + if (typeof v !== 'string') return null; + const s = v.trim(); + if (!s.startsWith('{') || !s.endsWith('}')) return null; + try { + const parsed = JSON.parse(s); + return parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? parsed : null; + } catch { + return null; + } +} + /** * Map a LookupColumnDef.type to a filter input type for the filter bar. * Returns undefined if the field type is not filterable. @@ -466,11 +486,14 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel const raw: any[] = multiple ? Array.isArray(value) ? value : [] : value != null && value !== '' ? [value] : []; - // Expanded-reference values (server `$expand`) already arrive as the related - // record object and resolve directly in `resolveSelectedOption` — only bare - // ids need a fetch. Passing an object to `findOne` would query for a bogus - // id and leave the trigger stuck on the placeholder. - const ids = raw.filter((v) => v != null && v !== '' && typeof v !== 'object'); + // Expanded-reference values (server `$expand`, or their JSON-encoded string + // form) already carry their display fields and resolve directly in + // `resolveSelectedOption` — only bare ids need a fetch. Passing an object (or + // a JSON string) to `findOne` would query for a bogus id and leave the + // trigger stuck on the placeholder. + const ids = raw.filter( + (v) => v != null && v !== '' && typeof v !== 'object' && !parseReferenceObjectString(v), + ); if (!ids.length) return; // Only fetch records we haven't resolved yet. const unresolved = ids.filter((v) => !findOption(v)); @@ -522,11 +545,29 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel [staticOptions, fetchedOptions, pickerResolvedRecords], ); + // String-coerced fallback for `findOption` — matches the read cell's tolerant + // `String(a) === String(b)` comparison so a numeric cell value still resolves + // against a string-keyed option (and vice versa). Only consulted when the + // strict match misses, so homogeneous option lists are unaffected. + const findOptionLoose = useCallback( + (v: any): LookupOption | undefined => { + const key = String(v); + return ( + staticOptions.find(opt => String(opt.value) === key) ?? + fetchedOptions.find(opt => String(opt.value) === key) ?? + pickerResolvedRecords.find(opt => String(opt.value) === key) + ); + }, + [staticOptions, fetchedOptions, pickerResolvedRecords], + ); + // Collapse an expanded-reference value (the related record object returned by // server `$expand`) to its bare id — used for option matching / highlighting. const normalizeId = useCallback( - (raw: any): any => - raw != null && typeof raw === 'object' ? (raw[idField] ?? raw.id ?? raw._id) : raw, + (raw: any): any => { + const obj = raw != null && typeof raw === 'object' ? raw : parseReferenceObjectString(raw); + return obj ? (obj[idField] ?? obj.id ?? obj._id ?? obj.externalId) : raw; + }, [idField], ); @@ -537,12 +578,19 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel const resolveSelectedOption = useCallback( (raw: any): LookupOption | undefined => { if (raw == null || raw === '') return undefined; - if (typeof raw === 'object') { - return recordToOption(raw, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema); + // An expanded-reference object (server `$expand`) — or its JSON-encoded + // string form, e.g. an external-id reference `'{"externalId":"…"}'` — is + // mapped directly, mirroring the read cell (`LookupCellRenderer`). + const asObject = typeof raw === 'object' ? raw : parseReferenceObjectString(raw); + if (asObject) { + return recordToOption(asObject, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema); } - return findOption(raw); + // Bare id: strict match first, then a String()-coerced fallback so a + // numeric cell value still resolves against a string-keyed option (and + // vice versa) — matching the read cell's tolerant comparison. + return findOption(raw) ?? findOptionLoose(raw); }, - [findOption, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema], + [findOption, findOptionLoose, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema], ); const selectedOptions = multiple