diff --git a/packages/fields/src/widgets/LookupField.hydration.test.tsx b/packages/fields/src/widgets/LookupField.hydration.test.tsx new file mode 100644 index 000000000..e46bb3dc9 --- /dev/null +++ b/packages/fields/src/widgets/LookupField.hydration.test.tsx @@ -0,0 +1,136 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Regression (#3108): a multi-value lookup hydrated its committed ids with a + * SERIAL `findOne` per id — 41 selected users meant 41 sequential round-trips, + * during which the trigger rendered the bare "Select…" placeholder, + * indistinguishable from having no value at all. + * + * Fixed behaviour under test: + * 1. unresolved ids are fetched in ONE batched `$in` query (chunked), not + * one findOne per id; + * 2. while hydration is in flight the trigger shows a loading indicator and + * the committed-value count instead of the empty placeholder; + * 3. the loading state settles (falls back to placeholder) when hydration + * fails, instead of spinning forever. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, cleanup, waitFor, screen } from '@testing-library/react'; +import { LookupField } from './LookupField'; + +afterEach(cleanup); + +const USERS = [ + { id: 'u1', name: 'Ada Lovelace' }, + { id: 'u2', name: 'Grace Hopper' }, + { id: 'u3', name: 'Margaret Hamilton' }, +]; + +describe('LookupField — multi-value hydration batches and shows loading (#3108)', () => { + it('hydrates several ids with one $in query instead of a findOne per id', async () => { + const find = vi.fn(async (_obj: string, params: any) => { + const wanted: any[] = params?.$filter?.id?.$in ?? []; + return { data: USERS.filter((u) => wanted.includes(u.id)) }; + }); + const findOne = vi.fn(async () => null); + render( + {}} + dataSource={{ find, findOne } as never} + field={{ reference_to: 'sys_user', multiple: true } as never} + />, + ); + + await waitFor(() => expect(screen.getByText('Ada Lovelace')).toBeTruthy()); + expect(screen.getByText('Grace Hopper')).toBeTruthy(); + expect(screen.getByText('Margaret Hamilton')).toBeTruthy(); + + // One batched round-trip, zero per-id GETs. + expect(findOne).not.toHaveBeenCalled(); + const hydrationCalls = find.mock.calls.filter((c) => c[1]?.$filter?.id?.$in); + expect(hydrationCalls).toHaveLength(1); + expect(hydrationCalls[0][1].$filter.id.$in).toEqual(['u1', 'u2', 'u3']); + }); + + it('shows the committed count + spinner while hydration is in flight, never the empty placeholder', async () => { + let resolveFind!: (v: any) => void; + const find = vi.fn( + () => new Promise((resolve) => { resolveFind = resolve; }), + ); + render( + {}} + dataSource={{ find } as never} + field={{ reference_to: 'sys_user', multiple: true } as never} + />, + ); + + // In-flight: loading indicator + raw committed count, no "Select…". + await waitFor(() => expect(screen.getByTestId('lookup-hydrating')).toBeTruthy()); + expect(screen.queryByText('Select...')).toBeNull(); + expect(screen.getByText(/3/)).toBeTruthy(); + + resolveFind({ data: USERS }); + await waitFor(() => expect(screen.getByText('Ada Lovelace')).toBeTruthy()); + expect(screen.queryByTestId('lookup-hydrating')).toBeNull(); + }); + + it('settles (no infinite spinner) when the hydration fetch fails', async () => { + const find = vi.fn(async (_obj: string, params: any) => { + if (params?.$filter?.id?.$in) throw new Error('boom'); + return { data: [] }; + }); + render( + {}} + dataSource={{ find } as never} + field={{ reference_to: 'sys_user', multiple: true } as never} + />, + ); + + await waitFor(() => expect(find).toHaveBeenCalled()); + // The failed attempt is marked settled — the spinner clears instead of + // spinning forever (pre-existing fallback: unresolvable ids drop). + await waitFor(() => expect(screen.queryByTestId('lookup-hydrating')).toBeNull()); + }); + + it('readonly mode shows the loading count instead of an empty value while hydrating', async () => { + let resolveFind!: (v: any) => void; + const find = vi.fn( + () => new Promise((resolve) => { resolveFind = resolve; }), + ); + render( + {}} + readonly + dataSource={{ find } as never} + field={{ reference_to: 'sys_user', multiple: true } as never} + />, + ); + + await waitFor(() => expect(screen.getByTestId('lookup-hydrating')).toBeTruthy()); + + resolveFind({ data: USERS.slice(0, 2) }); + await waitFor(() => expect(screen.getByText('Ada Lovelace')).toBeTruthy()); + }); + + it('keeps the cheap findOne GET for a single unresolved primary id', async () => { + const find = vi.fn(async () => ({ data: [] })); + const findOne = vi.fn(async () => ({ id: 'u1', name: 'Ada' })); + render( + {}} + dataSource={{ find, findOne } as never} + field={{ reference_to: 'sys_user', multiple: true } as never} + />, + ); + await waitFor(() => expect(findOne).toHaveBeenCalledWith('sys_user', 'u1')); + }); +}); diff --git a/packages/fields/src/widgets/LookupField.tsx b/packages/fields/src/widgets/LookupField.tsx index 701015e8f..c8d20ef70 100644 --- a/packages/fields/src/widgets/LookupField.tsx +++ b/packages/fields/src/widgets/LookupField.tsx @@ -205,6 +205,12 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel // even when the record wasn't part of the Level 1 popover fetch. const [pickerResolvedRecords, setPickerResolvedRecords] = useState([]); + // Ids whose hydration attempt has FINISHED — found or not — keyed by + // String(id). Distinguishes "still loading" from "unresolvable" (deleted + // record, fetch failure) so the trigger's hydrating state can't stick + // forever on an id that will never resolve (#3108). + const [hydrationSettled, setHydrationSettled] = useState>(() => new Set()); + // Arrow-key active index (-1 = none) const [activeIndex, setActiveIndex] = useState(-1); const listRef = useRef(null); @@ -476,11 +482,12 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel (async () => { try { const fetched: LookupOption[] = []; - for (const id of unresolved) { - // `findOne` resolves by PRIMARY id only. When the field commits a - // different column (`id_field: 'name'` — e.g. position machine - // names, objectstack #3508), hydrate by filtering on that column or - // the stored value would never resolve to a label. + // Single id: the pre-existing cheap paths — a primary-id `findOne` + // GET, or an equality filter when the field commits a different + // column (`id_field: 'name'` — e.g. position machine names, + // objectstack #3508). + if (unresolved.length === 1) { + const id = unresolved[0]; if (typeof (dataSource as any).findOne === 'function' && idField === 'id') { const rec = await (dataSource as any).findOne(referenceTo, id); if (rec) fetched.push(recordToOption(rec, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema)); @@ -489,9 +496,35 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel $filter: { [idField]: id }, $top: 1, } as QueryParams); - const rows = res?.data ?? res ?? []; + const rows = (res as any)?.data ?? res ?? []; if (rows[0]) fetched.push(recordToOption(rows[0], displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema)); } + } else { + // SEVERAL unresolved ids: one `$in` query per chunk. A multi-value + // field can hold dozens of ids, and the old serial findOne-per-id + // took seconds while the trigger sat on the empty "Select…" + // placeholder (#3108). + // Chunked to LOOKUP_PAGE_SIZE so no request exceeds the page size + // a server may cap `$top` at; chunks run in parallel. + const chunks: any[][] = []; + for (let i = 0; i < unresolved.length; i += LOOKUP_PAGE_SIZE) { + chunks.push(unresolved.slice(i, i + LOOKUP_PAGE_SIZE)); + } + const results = await Promise.all( + chunks.map((chunk) => + dataSource.find(referenceTo, { + $filter: { [idField]: { $in: chunk } }, + $top: chunk.length, + } as QueryParams), + ), + ); + for (const res of results) { + const rows = (res as any)?.data ?? res ?? []; + if (!Array.isArray(rows)) continue; + for (const row of rows) { + fetched.push(recordToOption(row, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema)); + } + } } if (!cancelled && fetched.length) { setPickerResolvedRecords((prev) => { @@ -502,6 +535,17 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel } } catch { // Ignore — chip will fall back to showing the raw id. + } finally { + // Mark the attempt settled — found or not — so `hydrating` below + // clears even for ids that no longer resolve (deleted records, + // fetch failures) instead of spinning forever. + if (!cancelled) { + setHydrationSettled((prev) => { + const next = new Set(prev); + for (const id of unresolved) next.add(String(id)); + return next; + }); + } } })(); return () => { @@ -580,6 +624,31 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel [findOption, findOptionLoose, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema], ); + // A value can hold bare ids that no option list resolves YET — the batch + // hydration above is still in flight. While any such id is pending, the + // trigger must not present the field as empty: a 41-value lookup rendered + // the bare "Select…" placeholder for seconds, indistinguishable from + // having no value at all (#3108). + const hydrating = useMemo(() => { + if (!hasDataSource) return false; + const raw: any[] = multiple + ? Array.isArray(value) ? value : [] + : value != null && value !== '' ? [value] : []; + return raw.some( + (v) => + v != null && v !== '' && typeof v !== 'object' && !parseReferenceObjectString(v) && + !findOption(v) && !findOptionLoose(v) && + !hydrationSettled.has(String(v)), + ); + }, [hasDataSource, multiple, value, findOption, findOptionLoose, hydrationSettled]); + + // Committed-value count. During hydration the resolved-option count + // undercounts (unresolved ids are filtered out below), so the trigger + // shows this raw count instead. + const rawSelectedCount = multiple + ? (Array.isArray(value) ? value : []).filter((v) => v != null && v !== '').length + : value != null && value !== '' ? 1 : 0; + const selectedOptions = multiple ? (Array.isArray(value) ? value : []).map(resolveSelectedOption).filter(Boolean) : value ? [resolveSelectedOption(value)].filter(Boolean) : []; @@ -801,6 +870,17 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel if (readonly) { if (!selectedOptions.length) { + if (hydrating) { + return ( + + + {multiple ? t('table.selected', { count: rawSelectedCount }) : t('lookup.loading')} + + ); + } return ; } @@ -846,15 +926,28 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel ? t('lookup.selectFirst', { fields: dependsOn.map(d => d.field).join(', ') }) : undefined} > - + {hydrating ? ( + + ) : ( + + )} {dependenciesMissing ? t('lookup.selectFirst', { fields: dependsOn.map(d => d.field).join(', ') }) - : compact && !multiple && selectedOptions.length > 0 - ? singleSelectedLabel - : selectedOptions.length === 0 - ? lookupField?.placeholder || t('common.select') - : multiple ? t('table.selected', { count: selectedOptions.length }) : t('common.select')} + : hydrating + // The value EXISTS but its labels are still loading — say so + // instead of the empty placeholder (#3108). + ? multiple + ? t('table.selected', { count: rawSelectedCount }) + : t('lookup.loading') + : compact && !multiple && selectedOptions.length > 0 + ? singleSelectedLabel + : selectedOptions.length === 0 + ? lookupField?.placeholder || t('common.select') + : multiple ? t('table.selected', { count: selectedOptions.length }) : t('common.select')} );