diff --git a/.changeset/bulk-fanout-host-find-params.md b/.changeset/bulk-fanout-host-find-params.md new file mode 100644 index 000000000..f9eb106dc --- /dev/null +++ b/.changeset/bulk-fanout-host-find-params.md @@ -0,0 +1,33 @@ +--- +'@object-ui/plugin-grid': minor +--- + +fix(plugin-grid): cross-page "select all N matching" replays the host's real query — or abstains — instead of fanning out unfiltered + +`resolveBulkRows` re-issues the view's query in 500-record pages so a bulk action +receives the whole match set rather than the visible window. The query it +replayed came from `lastFindParamsRef`, whose only writer is ObjectGrid's own +data loader. Under a host that fetches the rows itself — ListView passing `data` +plus `manualPagination` and `rowCount`, which is what the console does — that +loader never runs, so the ref was not the query behind the rows on screen: +absent, or stale from an earlier own-fetch. Either way the `?? {}` default let +the fan-out ask the server for the WHOLE OBJECT — no `$filter`, no `$orderby`, +no `$search` — and hand up to 5000 unmatched records to a destructive executor +(`onBulkDelete`) while the bar read "All N matching records are selected". + +The host now hands its query down as the new optional `findParams` prop on +`ObjectGridExternalPaginationProps` (the same shape the internal loader stores), +and the fan-out reads whichever side owns the fetch. There is deliberately no +grid-side default: when no query is available for the current data path the +escalation is **not offered at all** — a host that forgets `findParams` loses +the affordance rather than silently collecting the whole object, which is what +makes the unfiltered fan-out structurally unreachable rather than merely +currently-wired-right. A changed `findParams` also resets the escalation, +mirroring the `setSelectAllMatching(false)` the internal loader runs next to its +own params write, so "All N matching" cannot survive the host's filter, search, +sort or page changing; the comparison is by content, so a host re-render that +rebuilds an equal object does not drop the user's escalation. + +The internal-loader path is unchanged: with the ref populated the fan-out issues +the same params it always did, and the `selection.type: 'single'` suppression is +untouched. diff --git a/.changeset/listview-hands-down-find-params.md b/.changeset/listview-hands-down-find-params.md new file mode 100644 index 000000000..1d084d35b --- /dev/null +++ b/.changeset/listview-hands-down-find-params.md @@ -0,0 +1,18 @@ +--- +'@object-ui/plugin-list': patch +--- + +fix(plugin-list): ListView hands the child grid the query behind the window it passes down + +ListView owns the fetch on the external-pagination path — it holds the filter, +the search term and the sort, and it is the side that calls `dataSource.find`. +The grid it hands the window to has a cross-page "select all N matching" +escalation that RE-ISSUES that query to collect the whole match set, and with +nothing handed down it replayed its own never-written params ref and so asked the +server for the entire object, feeding unmatched records to bulk delete. + +The params object is now hoisted out of the `find` call — one object, one query, +no reconstruction that could drift from what was actually asked — recorded past +the stale-request guard so it is always the query that produced the rows on +screen, and forwarded as `findParams` in the same handoff block as `rowCount`, +`page` and `onPageChange`. No public API of `ListView` changes. diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 934ba2010..89c8df5d3 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -226,6 +226,36 @@ export interface ObjectGridExternalPaginationProps * callback would misstate both the payload and when it fires. */ onColumnStateChange?: (state: ObjectGridColumnState) => void; + + /** + * Grid-only: `DataTableSchema` has no counterpart to derive from — a table is + * handed rows, it never issues a query. + * + * The params the host passed to `dataSource.find()` for the window it is + * handing down in `data` — the SAME shape this grid's own loader stores in + * `lastFindParamsRef` (`$filter` / `$orderby` / `$select` / `$search` / + * `$searchFields` / `$expand` / `$top` / `$skip`), so both paths feed one + * reader. `$top`/`$skip` may be present and are ignored: the fan-out windows + * the replay itself. + * + * Why a prop rather than a fallback inside the grid (objectui#4501): the + * cross-page "select all N matching" escalation re-issues the view's query to + * collect the whole match set, and the query has to come from whichever side + * owns the fetch. Under external pagination that is the host, and the grid's + * own ref is either empty or stale — replaying it asked the server for the + * WHOLE OBJECT (no `$filter`) and handed up to 5000 unmatched records to a + * destructive executor. A grid-side `?? {}` default is what produced that, + * which is why the missing case is not defaulted but REFUSED: with no params + * for the current data path the escalation is not offered at all (see + * `bulkFanoutParams` in the component body). + * + * A changed value is also the host's query-change signal: it resets the + * escalation, mirroring the `setSelectAllMatching(false)` the internal loader + * runs next to its own `lastFindParamsRef` write. Compared by CONTENT, so a + * host re-render that rebuilds an equal object does not drop the user's + * escalation. + */ + findParams?: Record | null; } export interface ObjectGridProps extends ObjectGridExternalPaginationProps { @@ -297,6 +327,23 @@ const RELATIONAL_META_KEYS = [ 'lookup_filters', 'lookupFilters', 'titleFormat', ] as const; +/** + * Content signature of a host's find-params, used as the query-change signal for + * the cross-page escalation (objectui#4501 clause 2). + * + * CONTENT and not identity: a host re-render that rebuilds an equal params + * object must not drop an escalation the user just made, and identity is the + * one thing a host cannot be relied on to keep stable. Top-level keys are + * sorted so key ORDER — which differs between the host's literal and this + * grid's own loader — never reads as a query change. + */ +function findParamsSignature(params: Record | null | undefined): string | null { + if (!params) return null; + return JSON.stringify( + Object.keys(params).sort().map((k) => [k, params[k] ?? null]), + ); +} + function applyRelationalMeta( fieldMeta: Record, fieldDef: Record | undefined | null, @@ -390,6 +437,7 @@ export const ObjectGrid: React.FC = ({ onSortChange: hostOnSortChange, search: hostSearch, onSearchChange: hostOnSearchChange, + findParams: hostFindParams, onColumnStateChange, }) => { const [data, setData] = useState([]); @@ -856,6 +904,25 @@ export const ObjectGrid: React.FC = ({ }; }, [objectName, schemaFields, schemaColumns, schemaFilter, schemaSort, headerSort, searchTerm, schemaPagination, schemaPageSize, serverPage, serverPageSize, dataSource, hasInlineData, dataConfig, refreshKey]); + // The same reset, for the path the loader above never runs on (objectui#4501 + // clause 2). "All N matching are selected" is a claim about ONE query, so it + // must not survive that query changing — the loader drops it in place (three + // lines up, next to its `lastFindParamsRef` write), and under a host-driven + // fetch the host's params changing is the identical signal. Without this a + // user could escalate, change the filter in the host's toolbar, and keep an + // escalation that now reads against a match set they never saw. + // + // Keyed on the CONTENT signature, not the prop's identity: a host re-render + // that rebuilds an equal object is not a query change, and dropping the + // escalation on one would make the affordance unusable. + const hostFindParamsKey = React.useMemo( + () => findParamsSignature(hostFindParams), + [hostFindParams], + ); + React.useEffect(() => { + setSelectAllMatching(false); + }, [hostFindParamsKey]); + // Reset to page 1 whenever the query itself changes (object / filter / sort / // search), so we never request a page index that no longer exists for the new // result set (e.g. applying a filter while sitting on page 5 of the old @@ -2029,6 +2096,33 @@ export const ObjectGrid: React.FC = ({ // matching" escalation must never be offered. const singleSelection = selectionMode === 'single'; + // The query the cross-page fan-out would replay — from whichever side owns the + // fetch (objectui#4501). ONE value, read by the fan-out AND by the affordance + // gate below, so the offer and the thing it promises can never disagree. + // + // `hasInlineData` is exactly the data loader's own guard (`if (hasInlineData) + // return`), which makes it the precise test for "this grid did not issue the + // query behind the rows on screen". In that case `lastFindParamsRef` is not + // merely empty, it is WRONG — either never written, or left over from an + // earlier own-fetch — so the host's `findParams` is the only admissible + // source, and there is deliberately no fallback to the ref and no `?? {}` + // default: replaying `{}` is what asked the server for the whole object and + // fed up to 5000 unmatched records to bulk delete. + const bulkFanoutParams: Record | null = hasInlineData + ? (hostFindParams ?? null) + : (lastFindParamsRef.current ?? null); + + // The floor. With no query to replay there is no honest "all N matching", so + // the escalation is not offered — the same answer as a match set that does not + // exist. This is what makes an unfiltered fan-out structurally unreachable + // rather than merely currently-wired-right: a host that forgets `findParams` + // loses the affordance, it does not silently get the whole object. + // + // ONE condition, consumed by both `BulkActionBar` sites below. Do NOT re-spell + // it at a consumption site — a second copy is how one of them gets missed + // (objectui#4138, #4464). + const canOfferSelectAllMatching = !singleSelection && bulkFanoutParams !== null; + // Resolve the rows the bulk action should actually operate on. When // "select all N matching" is active, fan out a paged find against the // current query so we can hand a complete record list to the executor. @@ -2039,7 +2133,12 @@ export const ObjectGrid: React.FC = ({ if (!selectAllMatching) return rowsHint; const objectName = schema.objectName; if (!dataSource || !objectName) return rowsHint; - const base = { ...(lastFindParamsRef.current ?? {}) } as Record; + // The floor again, at the point of consumption: the affordance gate above + // means an escalation cannot be reached without params, and this means it + // cannot be ACTED on without them either. Same single source, so the two + // cannot drift. + if (!bulkFanoutParams) return rowsHint; + const base = { ...bulkFanoutParams } as Record; delete (base as any).$top; delete (base as any).$skip; const HARD_CAP = 5000; @@ -3088,9 +3187,9 @@ export const ObjectGrid: React.FC = ({ onActionDef={dispatchBulkActionDef} onClearSelection={resetSelection} pageSize={data.length} - totalMatching={singleSelection ? undefined : totalMatching} + totalMatching={canOfferSelectAllMatching ? totalMatching : undefined} allMatchingSelected={selectAllMatching} - onSelectAllMatching={singleSelection ? undefined : () => setSelectAllMatching(true)} + onSelectAllMatching={canOfferSelectAllMatching ? () => setSelectAllMatching(true) : undefined} /> } @@ -3126,9 +3225,9 @@ export const ObjectGrid: React.FC = ({ onActionDef={dispatchBulkActionDef} onClearSelection={resetSelection} pageSize={data.length} - totalMatching={singleSelection ? undefined : totalMatching} + totalMatching={canOfferSelectAllMatching ? totalMatching : undefined} allMatchingSelected={selectAllMatching} - onSelectAllMatching={singleSelection ? undefined : () => setSelectAllMatching(true)} + onSelectAllMatching={canOfferSelectAllMatching ? () => setSelectAllMatching(true) : undefined} /> {navigation.isOverlay && ( ({ + usePermissions: () => ({ + isLoaded: false, + checkField: () => true, + getObjectApiOperations: () => undefined, + can: () => true, + }), +})); + +import { ObjectGrid } from '../ObjectGrid'; +import { registerAllFields } from '@object-ui/fields'; +import { ActionProvider, I18nProvider } from '@object-ui/react'; + +registerAllFields(); + +beforeAll(() => { + if (!Element.prototype.scrollIntoView) { + Element.prototype.scrollIntoView = vi.fn() as any; + } +}); + +const OBJECT = 'showcase_contact'; +const PAGE_SIZE = 10; + +/** The whole object — what an unfiltered fan-out collects. */ +const ALL = Array.from({ length: 40 }, (_, i) => ({ + id: `r-${i}`, + name: `Row ${i}`, + status: i >= 14 ? 'active' : 'archived', +})); +/** What the HOST's query matches: 26 records, `r-14` … `r-39`. */ +const MATCHING = ALL.filter(r => r.status === 'active'); + +/** + * The params the external-pagination host used for the window it handed down — + * measured against `ListView.tsx`'s own `dataSource.find` literal, key for key: + * `$filter` (lowered AST) / `$orderby` (SortNode[]) / `$top` / `$select` / + * `$search`, `$skip` present only past page 1. + */ +const HOST_PARAMS: Record = { + $filter: ['status', '=', 'active'], + $orderby: [{ field: 'name', order: 'asc' }], + $top: PAGE_SIZE, + $select: ['id', 'name', 'status'], + $search: 'Row', +}; + +/** A second host query — the user changed the filter in the host's toolbar. */ +const HOST_PARAMS_2: Record = { + ...HOST_PARAMS, + $filter: ['status', '=', 'archived'], +}; + +function makeDataSource() { + const find = vi.fn(async (_object: string, params: any) => { + // The server answers the query it is given: with a `$filter` it returns the + // 26 matching records, without one the whole 40-record object. + const source = params?.$filter ? MATCHING : ALL; + const top = params?.$top ?? PAGE_SIZE; + const skip = params?.$skip ?? 0; + const data = source.slice(skip, skip + top).map(r => ({ ...r })); + return { data, total: source.length, hasMore: skip + data.length < source.length }; + }); + return { + find, + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: async (name: string) => ({ + name, + fields: { id: { type: 'text' }, name: { type: 'text' }, status: { type: 'text' } }, + }), + } as any; +} + +const schema: any = { + type: 'object-grid', + objectName: OBJECT, + columns: ['name'], + selection: { type: 'multiple' }, + // A declared bulk action is a WIRING declaration; `onBulkDelete` below owns + // the execution, so clicking Delete is what materializes the fanned-out set. + bulkActions: ['delete'], + pagination: { pageSize: PAGE_SIZE }, +}; + +/** The host window: page 1 of the FILTERED collection (`Row 14` … `Row 23`). */ +const HOST_WINDOW = MATCHING.slice(0, PAGE_SIZE); + +function hostProps(over: Record = {}) { + return { + data: HOST_WINDOW, + manualPagination: true, + rowCount: MATCHING.length, + page: 1, + pageSize: PAGE_SIZE, + onPageChange: () => {}, + ...over, + }; +} + +function tree(ds: any, onBulkDelete: any, extra: Record) { + return ( + + + + + + ); +} + +const headerCheckbox = () => + document.querySelector('thead [role="checkbox"]') as HTMLElement | null; +const rowCheckboxes = () => + Array.from(document.querySelectorAll('tbody [role="checkbox"]')) as HTMLElement[]; + +/** + * One own-fetch on the INTERNAL path (so `totalMatching` and `lastFindParamsRef` + * are populated), then hand the grid the host's filtered window. + */ +async function renderThenHandOff(ds: any, hostOver: Record = {}) { + const onBulkDelete = vi.fn(); + const { rerender } = render(tree(ds, onBulkDelete, {})); + // Internal load: the grid's own unfiltered query. + await waitFor(() => expect(screen.getByText('Row 0')).toBeInTheDocument()); + const internalParams = { ...ds.find.mock.calls[0][1] }; + + rerender(tree(ds, onBulkDelete, hostProps(hostOver))); + // The rows on screen are now the host's filtered window. + await waitFor(() => expect(screen.getByText('Row 14')).toBeInTheDocument()); + await waitFor(() => expect(rowCheckboxes().length).toBe(PAGE_SIZE)); + return { onBulkDelete, rerender, internalParams }; +} + +async function selectWholePage() { + fireEvent.click(headerCheckbox() as HTMLElement); + await waitFor(() => expect(screen.getByTestId('bulk-actions-bar')).toBeInTheDocument()); + await waitFor(() => + expect(within(screen.getByTestId('bulk-actions-bar')).getByText(/10 selected/)).toBeInTheDocument(), + ); +} + +/** Click Delete and return the params of every `find` the fan-out issued. */ +async function fanOutParams(ds: any, onBulkDelete: any) { + const before = ds.find.mock.calls.length; + fireEvent.click(screen.getByTestId('bulk-action-delete')); + await waitFor(() => expect(onBulkDelete).toHaveBeenCalled()); + return ds.find.mock.calls.slice(before).map((c: any[]) => c[1]); +} + +beforeEach(() => { vi.clearAllMocks(); }); +afterEach(() => { cleanup(); }); + +describe('#4501 clause 1 — the fan-out replays the HOST\'s query', () => { + it('issues the host\'s $filter/$orderby/$search/$select, not an unfiltered whole-object read', async () => { + const ds = makeDataSource(); + const { onBulkDelete } = await renderThenHandOff(ds, { findParams: HOST_PARAMS }); + + await selectWholePage(); + fireEvent.click(await screen.findByTestId('bulk-select-all-matching')); + + const calls = await fanOutParams(ds, onBulkDelete); + expect(calls).toHaveLength(1); + // Verbatim: the host's query, windowed by the fan-out's own paging. + expect(calls[0]).toEqual({ + $filter: ['status', '=', 'active'], + $orderby: [{ field: 'name', order: 'asc' }], + $select: ['id', 'name', 'status'], + $search: 'Row', + $top: 500, + $skip: 0, + }); + + // …and therefore the executor receives the MATCH SET, nothing else. + const delivered = onBulkDelete.mock.calls[0][0]; + expect(delivered).toHaveLength(MATCHING.length); + expect(delivered.every((r: any) => r.status === 'active')).toBe(true); + // `r-0` is in the object but NOT in the view's match set. Pre-fix the + // unfiltered fan-out collected it and handed it to delete. + expect(delivered.some((r: any) => r.id === 'r-0')).toBe(false); + }); +}); + +describe('#4501 clause 1b — the abstain floor', () => { + it('does NOT offer the escalation when the host hands down no params', async () => { + const ds = makeDataSource(); + // Same composition, one prop short: the host drives pagination but never + // says what it queried. There is no query to replay, so the affordance is + // absent — the same answer as a match set that does not exist. + await renderThenHandOff(ds); + + await selectWholePage(); + + expect(screen.queryByTestId('bulk-cross-page-banner')).not.toBeInTheDocument(); + expect(screen.queryByTestId('bulk-select-all-matching')).not.toBeInTheDocument(); + // The ordinary page selection is untouched — only the escalation is gone. + expect(within(screen.getByTestId('bulk-actions-bar')).getByText(/10 selected/)).toBeInTheDocument(); + expect(screen.getByTestId('bulk-action-delete')).toBeInTheDocument(); + }); + + it('keeps the single-selection suppression', async () => { + const ds = makeDataSource(); + const onBulkDelete = vi.fn(); + const singleSchema = { ...schema, selection: { type: 'single' } }; + const single = (extra: Record) => ( + + + + + + ); + const { rerender } = render(single({})); + await waitFor(() => expect(screen.getByText('Row 0')).toBeInTheDocument()); + rerender(single({ ...hostProps(), findParams: HOST_PARAMS })); + await waitFor(() => expect(screen.getByText('Row 14')).toBeInTheDocument()); + + fireEvent.click(rowCheckboxes()[0]); + await waitFor(() => expect(screen.getByTestId('bulk-actions-bar')).toBeInTheDocument()); + // Params ARE available here; the suppression is the `single` clause alone. + expect(screen.queryByTestId('bulk-cross-page-banner')).not.toBeInTheDocument(); + expect(screen.queryByTestId('bulk-select-all-matching')).not.toBeInTheDocument(); + }); +}); + +describe('#4501 clause 2 — the escalation resets when the host\'s query changes', () => { + it('drops "all matching" when the host hands down a different query', async () => { + const ds = makeDataSource(); + const { onBulkDelete, rerender } = await renderThenHandOff(ds, { findParams: HOST_PARAMS }); + + await selectWholePage(); + fireEvent.click(await screen.findByTestId('bulk-select-all-matching')); + await waitFor(() => + expect(screen.getByTestId('bulk-cross-page-banner')).toHaveTextContent(/matching records are selected/), + ); + + // The user changes the filter in the host's toolbar. + rerender(tree(ds, onBulkDelete, hostProps({ findParams: HOST_PARAMS_2 }))); + + await waitFor(() => + expect(screen.getByTestId('bulk-select-all-matching')).toBeInTheDocument(), + ); + expect(screen.getByTestId('bulk-cross-page-banner')).toHaveTextContent( + 'All 10 on this page are selected.', + ); + }); + + it('survives a re-render that hands down an EQUAL query (a plain refetch)', async () => { + const ds = makeDataSource(); + const { onBulkDelete, rerender } = await renderThenHandOff(ds, { findParams: HOST_PARAMS }); + + await selectWholePage(); + fireEvent.click(await screen.findByTestId('bulk-select-all-matching')); + await waitFor(() => + expect(screen.getByTestId('bulk-cross-page-banner')).toHaveTextContent(/matching records are selected/), + ); + + // A fresh object with identical content — what a host re-render produces. + rerender(tree(ds, onBulkDelete, hostProps({ findParams: { ...HOST_PARAMS } }))); + + await waitFor(() => expect(screen.getByTestId('bulk-cross-page-banner')).toBeInTheDocument()); + expect(screen.getByTestId('bulk-cross-page-banner')).toHaveTextContent( + /matching records are selected/, + ); + expect(screen.queryByTestId('bulk-select-all-matching')).not.toBeInTheDocument(); + }); +}); + +describe('#4501 must-not-change — the internal-loader path', () => { + it('fans out from the ref, byte for byte, when the grid owns the fetch', async () => { + const ds = makeDataSource(); + const onBulkDelete = vi.fn(); + // A grid with its own filtered query: the loader runs, so the ref holds it. + const own = { ...schema, filter: [{ field: 'status', operator: 'equals', value: 'active' }] }; + render( + + + + + , + ); + await waitFor(() => expect(screen.getByText('Row 14')).toBeInTheDocument()); + const loadParams = { ...ds.find.mock.calls[0][1] }; + expect(loadParams.$filter).toBeTruthy(); + + await selectWholePage(); + fireEvent.click(await screen.findByTestId('bulk-select-all-matching')); + const calls = await fanOutParams(ds, onBulkDelete); + + // Exactly the loader's own params, with the fan-out's window swapped in. + const { $top: _t, $skip: _s, ...rest } = loadParams; + expect(calls[0]).toEqual({ ...rest, $top: 500, $skip: 0 }); + expect(onBulkDelete.mock.calls[0][0]).toHaveLength(MATCHING.length); + }); + + it('still honours HARD_CAP over a match set larger than the cap', async () => { + const ds = makeDataSource(); + const BIG = 6000; + ds.find = vi.fn(async (_o: string, params: any) => { + const top = params?.$top ?? PAGE_SIZE; + const skip = params?.$skip ?? 0; + const data = Array.from( + { length: Math.max(0, Math.min(top, BIG - skip)) }, + (_, i) => ({ id: `b-${skip + i}`, name: `Row ${skip + i}` }), + ); + return { data, total: BIG, hasMore: skip + data.length < BIG }; + }); + const onBulkDelete = vi.fn(); + render( + + + + + , + ); + await waitFor(() => expect(screen.getByText('Row 0')).toBeInTheDocument()); + + await selectWholePage(); + fireEvent.click(await screen.findByTestId('bulk-select-all-matching')); + const calls = await fanOutParams(ds, onBulkDelete); + + // 5000 / 500 = 10 pages, then the cap stops it — the 6000th record is never + // requested and never delivered. + expect(calls).toHaveLength(10); + expect(calls.map((p: any) => p.$skip)).toEqual([0, 500, 1000, 1500, 2000, 2500, 3000, 3500, 4000, 4500]); + expect(onBulkDelete.mock.calls[0][0]).toHaveLength(5000); + }); +}); diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index f93c54f9b..902c0ecb6 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -866,6 +866,15 @@ export const ListView = React.forwardRef(({ // the first window are reachable, and we never stack a second pager on top. const [serverPage, setServerPage] = React.useState(1); const [serverTotal, setServerTotal] = React.useState(null); + // The params of the last successful fetch — the query behind the window this + // view is currently showing (objectui#4501). Handed DOWN with that window, in + // the same block as `rowCount`/`page`: whoever renders the rows may need to + // re-issue the query (the grid's cross-page "select all N matching" fans out + // over the whole match set), and this view is the only side that knows what it + // asked for. Held in state rather than a ref so a consumer re-renders when the + // query moves, and written only inside the stale-request guard below, so it is + // always the query that produced the rows on screen. + const [lastFindParams, setLastFindParams] = React.useState | null>(null); // Grouping state (initialized from schema, user can add/remove via popover). // Supports three input shapes from the schema: @@ -1466,7 +1475,11 @@ export const ListView = React.forwardRef(({ const paginate = currentView === 'grid' && !(groupingConfig?.fields?.length); const skip = paginate ? (serverPage - 1) * effectivePageSize : 0; - const results = await dataSource.find(schema.objectName, { + // Hoisted out of the `find` call so the exact params that produced this + // window can be handed down with it (objectui#4501). One object, one + // query — a second literal reconstructed for the consumer would be a + // copy free to drift from what was actually asked. + const findParams: Record = { ...(hasFilter ? { $filter: finalFilter } : {}), $orderby: sort, $top: effectivePageSize, @@ -1479,7 +1492,9 @@ export const ListView = React.forwardRef(({ ? { $searchFields: schema.searchableFields } : {}), } : {}), - }); + }; + + const results = await dataSource.find(schema.objectName, findParams); // Stale request guard: only apply the latest request's results if (!isMounted || requestId !== fetchRequestIdRef.current) return; @@ -1508,6 +1523,9 @@ export const ListView = React.forwardRef(({ : undefined; const knownTotal = typeof rawTotal === 'number' ? rawTotal : null; setServerTotal(paginate ? knownTotal : null); + // Past the stale-request guard, so this is the query behind the rows + // that were just set — never an in-flight one that lost the race. + setLastFindParams(findParams); setDataLimitReached( !(paginate && knownTotal != null) && items.length >= effectivePageSize, ); @@ -3132,6 +3150,15 @@ export const ListView = React.forwardRef(({ // saved view's sort" a non-question: there is one sort. sort: currentSort, onSortChange: handleHeaderSort, + // …and the query itself (objectui#4501). The grid's + // cross-page "select all N matching" re-issues it to collect + // the whole match set; on this path the grid never ran a + // fetch, so without this it had no query to replay and asked + // the server for the unfiltered object. Handed down here + // rather than anywhere else because this block IS the + // handoff: the window, its total, its page — and what was + // asked to get them. + findParams: lastFindParams, } : {})} /> diff --git a/packages/plugin-list/src/__tests__/ListView.findParamsHandoff.test.tsx b/packages/plugin-list/src/__tests__/ListView.findParamsHandoff.test.tsx new file mode 100644 index 000000000..0087f5c37 --- /dev/null +++ b/packages/plugin-list/src/__tests__/ListView.findParamsHandoff.test.tsx @@ -0,0 +1,176 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * ListView hands the child grid the query behind the window (objectui#4501). + * + * ListView owns the fetch on this path — it holds the filter, the search term + * and the sort, and it is the side that calls `dataSource.find`. The grid it + * hands the window to has a cross-page "select all N matching" escalation that + * RE-ISSUES that query to collect the whole match set; with nothing handed down + * it replayed `lastFindParamsRef`, which only its own (never-run) loader writes, + * and so asked the server for the entire object — no `$filter`, no `$orderby`, + * no `$search` — and fed up to 5000 unmatched records to bulk delete. + * + * These cases pin the PRODUCER half: whatever went out on the wire is what goes + * down to the grid, key for key, and it tracks the toolbar. The consumer half + * (the fan-out replaying it, and the abstain floor when it is absent) is pinned + * in `plugin-grid/src/__tests__/bulkFanoutHostParams.test.tsx`. + * + * plugin-grid is not a dependency of plugin-list (avoids a cycle), so — as in + * `ListView.serverPagination.test.tsx` — a stub `object-grid` records the props + * ListView feeds it. + */ +import { describe, it, expect, vi, beforeAll, afterAll, beforeEach, afterEach } from 'vitest'; +import { cleanup, render, waitFor } from '@testing-library/react'; +import React from 'react'; +import { ComponentRegistry } from '@object-ui/core'; +import { ListView } from '../ListView'; +import { SchemaRendererProvider } from '@object-ui/react'; +import type { ListViewSchema } from '@object-ui/types'; + +const OBJECT = 'showcase_contact'; +const TOTAL = 26; +const PAGE_SIZE = 10; + +let lastGridProps: any = null; + +function makeDataSource() { + const find = vi.fn(async (_object: string, params: any) => { + const top = params?.$top ?? PAGE_SIZE; + const skip = params?.$skip ?? 0; + const data = Array.from( + { length: Math.max(0, Math.min(top, TOTAL - skip)) }, + (_, i) => ({ id: `c-${skip + i}`, name: `Contact ${skip + i}` }), + ); + return { data, total: TOTAL, hasMore: skip + data.length < TOTAL }; + }); + return { + find, + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: async (name: string) => ({ + name, + fields: { id: { type: 'text' }, name: { type: 'text' }, status: { type: 'text' } }, + }), + } as any; +} + +const listSchema = (over: Record = {}): ListViewSchema => ({ + type: 'list-view', + objectName: OBJECT, + columns: ['name'], + pagination: { pageSize: PAGE_SIZE }, + ...over, +} as unknown as ListViewSchema); + +const lastFindCall = (ds: any) => ds.find.mock.calls[ds.find.mock.calls.length - 1][1]; + +let prevObjectGrid: any; +beforeAll(() => { + prevObjectGrid = ComponentRegistry.get('object-grid'); + ComponentRegistry.register('object-grid', (props: any) => { + lastGridProps = props; + return
; + }); +}); +afterAll(() => { + if (prevObjectGrid) ComponentRegistry.register('object-grid', prevObjectGrid); + else ComponentRegistry.unregister('object-grid'); +}); + +beforeEach(() => { lastGridProps = null; }); +afterEach(() => { cleanup(); lastGridProps = null; }); + +function renderList(ds: any, schema: ListViewSchema) { + return render( + + + , + ); +} + +describe('ListView → grid: the find-params handoff (#4501)', () => { + it('hands down exactly the params it queried with, alongside the window', async () => { + const ds = makeDataSource(); + renderList(ds, listSchema({ + filter: [{ field: 'status', operator: 'equals', value: 'active' }], + sort: [{ field: 'name', order: 'asc' }], + })); + + await waitFor(() => expect(lastGridProps?.findParams).toBeTruthy()); + // Key for key with what actually went out on the wire — no reconstruction. + expect(lastGridProps.findParams).toEqual(lastFindCall(ds)); + // …and it is the real query, not an empty object: this is the content the + // grid's fan-out has to replay. + expect(lastGridProps.findParams.$filter).toBeTruthy(); + expect(lastGridProps.findParams.$orderby).toEqual([{ field: 'name', order: 'asc' }]); + expect(lastGridProps.findParams.$select).toContain('name'); + expect(lastGridProps.findParams.$top).toBe(PAGE_SIZE); + + // It rides the same handoff as the rest of the external-pagination block. + expect(lastGridProps.manualPagination).toBe(true); + expect(lastGridProps.rowCount).toBe(TOTAL); + }); + + it('carries the SEARCH term the toolbar holds', async () => { + const ds = makeDataSource(); + // `initialSearchTerm` is the toolbar's seed — the same state the search box + // writes, so this is the term a user typed as far as the query is concerned. + render( + + + , + ); + + await waitFor(() => expect(lastGridProps?.findParams?.$search).toBe('ada')); + expect(lastGridProps.findParams).toEqual(lastFindCall(ds)); + expect(lastGridProps.findParams.$searchFields).toEqual(['name']); + }); + + it('tracks the query when it changes — the grid never holds a stale one', async () => { + const ds = makeDataSource(); + const { rerender } = render( + + + , + ); + await waitFor(() => expect(lastGridProps?.findParams).toBeTruthy()); + expect(lastGridProps.findParams.$filter).toBeUndefined(); + + rerender( + + + , + ); + + await waitFor(() => expect(lastGridProps?.findParams?.$filter).toBeTruthy()); + expect(lastGridProps.findParams).toEqual(lastFindCall(ds)); + }); + + it('turning the page hands down the NEW window\'s params', async () => { + const ds = makeDataSource(); + renderList(ds, listSchema()); + await waitFor(() => expect(lastGridProps?.onPageChange).toBeTruthy()); + expect(lastGridProps.findParams.$skip).toBeUndefined(); // page 1 + + lastGridProps.onPageChange(2); + + await waitFor(() => expect(lastGridProps?.findParams?.$skip).toBe(PAGE_SIZE)); + expect(lastGridProps.findParams).toEqual(lastFindCall(ds)); + }); +});