Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 136 additions & 0 deletions packages/fields/src/widgets/LookupField.hydration.test.tsx
Original file line number Diff line number Diff line change
@@ -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(
<LookupField
value={['u1', 'u2', 'u3']}
onChange={() => {}}
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(
<LookupField
value={['u1', 'u2', 'u3']}
onChange={() => {}}
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(
<LookupField
value={['u1', 'u2']}
onChange={() => {}}
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(
<LookupField
value={['u1', 'u2']}
onChange={() => {}}
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(
<LookupField
value={['u1']}
onChange={() => {}}
dataSource={{ find, findOne } as never}
field={{ reference_to: 'sys_user', multiple: true } as never}
/>,
);
await waitFor(() => expect(findOne).toHaveBeenCalledWith('sys_user', 'u1'));
});
});
117 changes: 105 additions & 12 deletions packages/fields/src/widgets/LookupField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<LookupOption[]>([]);

// 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<Set<string>>(() => new Set());

// Arrow-key active index (-1 = none)
const [activeIndex, setActiveIndex] = useState(-1);
const listRef = useRef<HTMLDivElement>(null);
Expand Down Expand Up @@ -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));
Expand All @@ -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) => {
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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) : [];
Expand Down Expand Up @@ -801,6 +870,17 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel

if (readonly) {
if (!selectedOptions.length) {
if (hydrating) {
return (
<span
className="inline-flex items-center gap-1.5 text-sm text-muted-foreground"
data-testid="lookup-hydrating"
>
<Loader2 className="size-3.5 animate-spin" />
{multiple ? t('table.selected', { count: rawSelectedCount }) : t('lookup.loading')}
</span>
);
}
return <EmptyValue />;
}

Expand Down Expand Up @@ -846,15 +926,28 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
? t('lookup.selectFirst', { fields: dependsOn.map(d => d.field).join(', ') })
: undefined}
>
<Search className={cn('size-4 shrink-0 text-muted-foreground', compact ? 'mr-1.5' : 'mr-2')} />
{hydrating ? (
<Loader2
className={cn('size-4 shrink-0 animate-spin text-muted-foreground', compact ? 'mr-1.5' : 'mr-2')}
data-testid="lookup-hydrating"
/>
) : (
<Search className={cn('size-4 shrink-0 text-muted-foreground', compact ? 'mr-1.5' : 'mr-2')} />
)}
<span className={cn('truncate', compact && selectedOptions.length === 0 && 'text-muted-foreground')}>
{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')}
</span>
</Button>
);
Expand Down
Loading