diff --git a/.changeset/7333-gantt-empty-host-data.md b/.changeset/7333-gantt-empty-host-data.md new file mode 100644 index 0000000000..d443111b4e --- /dev/null +++ b/.changeset/7333-gantt-empty-host-data.md @@ -0,0 +1,23 @@ +--- +'@object-ui/plugin-gantt': patch +--- + +`ObjectGantt` no longer treats an EMPTY host `data` array as the rows it was handed (objectui#7333). + +`ObjectGantt` read a host `data` prop at two places, and both accepted `[]`. The +reload's short-circuit tested `data && Array.isArray(data)`, and an empty array +is truthy, so the chart adopted it and returned before its own query. The gating +flag tested `Array.isArray(data)` alone, so the same array also switched off the +object-schema gate and the data-invalidation subscription of that query. A gantt +whose `data` names its own source, for example `{ provider: 'api', read }`, +therefore painted an EMPTY chart whenever a host handed it an array it had not +filled yet. + +Both places now read one predicate. A host `data` array is adopted only when it +has rows. An empty one reads as "no host rows yet": the chart queries its own +source, exactly as it does with no host array at all, and adopts the host's rows +when they arrive. + +Who can reach this: the registered `object-gantt` renderer still forwards no +host prop, and `ObjectGanttProps` does not declare `data`, so only a caller that +renders `ObjectGantt` directly and spreads `data` in was affected. diff --git a/packages/plugin-gantt/src/ObjectGantt.emptyHostData-7333.test.tsx b/packages/plugin-gantt/src/ObjectGantt.emptyHostData-7333.test.tsx new file mode 100644 index 0000000000..9c251f3435 --- /dev/null +++ b/packages/plugin-gantt/src/ObjectGantt.emptyHostData-7333.test.tsx @@ -0,0 +1,218 @@ +/** + * 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. + */ + +/** + * objectui#7333 — an EMPTY host `data` array is "no host rows yet", not "the + * host owns zero rows". + * + * `ObjectGantt` read a host `data` prop at two sites, and both took `[]` as an + * authoritative answer: `reload`'s short-circuit (`data && Array.isArray(data)` + * — `[]` is truthy) adopted it and returned before the chart's own query, and + * the gating flag (`Array.isArray(data)`) switched off the object-schema gate + * and the invalidation subscription that belong to that query. So a gantt whose + * `data` names its own source painted an EMPTY chart the moment a host handed + * it an array it had not filled yet. + * + * Both sites now read one predicate, `hostRows` in `ObjectGantt.tsx`: a + * non-empty host array is adopted as it always was, and an empty one leaves the + * chart reading from its own source, exactly as with no host array at all. + * + * ⚠️ Reach: the registered `object-gantt` renderer forwards no host prop, which + * `ObjectGantt.hostDataProp-7210.test.tsx` pins, and `ObjectGanttProps` does + * not declare `data`. So every case here renders `ObjectGantt` directly and + * spreads the prop in, as a direct caller would have to. + * + * Cases 1 and 4 are the pins; 2 and 3 are their controls, and 5 is the "yet": + * 1. api provider + host `[]` ⇒ the endpoint is read and its rows paint; + * 2. api provider + host rows ⇒ those rows paint, the endpoint is never read; + * 3. api provider + no host `data` ⇒ the endpoint is read; + * 4. object provider + host `[]` ⇒ the query waits for the object schema and + * goes out ONCE, already expanded (the gating flag's half); + * 5. host rows that arrive after an empty first render are adopted. + */ + +import React from 'react'; +import { render, screen, waitFor, act } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { SchemaRendererProvider } from '@object-ui/react'; +import { ObjectGantt } from './ObjectGantt'; + +vi.mock('sonner', () => ({ toast: { error: vi.fn() } })); + +vi.mock('./GanttView', () => ({ + GanttView: ({ tasks }: any) => ( +
+ {tasks.map((t: any) => ( +
{t.title}
+ ))} +
+ ), +})); + +vi.mock('@object-ui/plugin-detail', async (importOriginal) => ({ + ...(await importOriginal()), + RecordDetailDrawer: () => null, + deriveRecordPageHref: () => null, +})); + +/** What the gantt's own `api` endpoint answers with. */ +const ENDPOINT_ROWS = [ + { id: 'e1', name: 'Endpoint task 1', start_date: '2026-01-01', end_date: '2026-01-05' }, + { id: 'e2', name: 'Endpoint task 2', start_date: '2026-01-02', end_date: '2026-01-06' }, +]; + +/** What a host hands down as `data` when it DOES have rows. */ +const HOST_ROWS = [ + { id: 'h1', name: 'Host task 1', start_date: '2026-02-01', end_date: '2026-02-03' }, +]; + +const ENDPOINT = '/api/gantt/tree'; + +function apiSchema(): any { + return { + type: 'object-gantt', + objectName: 'tasks', + gantt: { titleField: 'name', startDateField: 'start_date', endDateField: 'end_date' }, + data: { provider: 'api', read: { url: ENDPOINT, method: 'GET' } }, + }; +} + +function makeApiFetch() { + return vi.fn(async () => + new Response(JSON.stringify({ data: ENDPOINT_ROWS, total: ENDPOINT_ROWS.length }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ); +} + +/** The URLs the endpoint was read at, in order. */ +function readUrls(apiFetch: ReturnType): string[] { + return (apiFetch.mock.calls as unknown as Array<[unknown]>).map(([url]) => String(url)); +} + +/** Renders an api-provider gantt, spreading `hostProps` in as a direct caller would. */ +function renderApiGantt(apiFetch: ReturnType, hostProps: Record) { + const tree = (props: Record) => ( + + + + ); + const utils = render(tree(hostProps)); + return { ...utils, rerenderWith: (props: Record) => utils.rerender(tree(props)) }; +} + +const taskCount = () => screen.getByTestId('gantt-view').getAttribute('data-task-count'); + +/** Lets every in-flight read settle, so a query that would follow is counted. */ +const settle = () => act(() => new Promise((resolve) => setTimeout(resolve, 100))); + +describe('objectui#7333 — an api-provider gantt handed an EMPTY host array still reads its endpoint', () => { + // No test here may reach the network through the bare global fetch. + const bareFetch = vi.fn(async () => new Response('{}', { status: 599 })); + + beforeEach(() => { + bareFetch.mockClear(); + vi.stubGlobal('fetch', bareFetch); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('host `data: []` ⇒ the endpoint is read and its rows paint', async () => { + const apiFetch = makeApiFetch(); + + renderApiGantt(apiFetch, { data: [] }); + + await waitFor(() => expect(screen.getByTestId('gv-task-e2')).toBeTruthy()); + expect(taskCount()).toBe(String(ENDPOINT_ROWS.length)); + expect(readUrls(apiFetch).some((url) => url.includes(ENDPOINT))).toBe(true); + expect(bareFetch).not.toHaveBeenCalled(); + }); + + it('CONTROL — host `data` with rows ⇒ those rows paint and the endpoint is never read', async () => { + const apiFetch = makeApiFetch(); + + renderApiGantt(apiFetch, { data: HOST_ROWS }); + + await waitFor(() => expect(screen.getByTestId('gv-task-h1')).toBeTruthy()); + expect(taskCount()).toBe(String(HOST_ROWS.length)); + expect(screen.queryByTestId('gv-task-e1')).toBeNull(); + expect(apiFetch).not.toHaveBeenCalled(); + }); + + it('CONTROL — no host `data` ⇒ the endpoint is read', async () => { + const apiFetch = makeApiFetch(); + + renderApiGantt(apiFetch, {}); + + await waitFor(() => expect(screen.getByTestId('gv-task-e2')).toBeTruthy()); + expect(readUrls(apiFetch).some((url) => url.includes(ENDPOINT))).toBe(true); + }); + + it('host rows that arrive after an empty first render are adopted', async () => { + const apiFetch = makeApiFetch(); + + const { rerenderWith } = renderApiGantt(apiFetch, { data: [] }); + await waitFor(() => expect(screen.getByTestId('gv-task-e2')).toBeTruthy()); + + rerenderWith({ data: HOST_ROWS }); + + await waitFor(() => expect(screen.getByTestId('gv-task-h1')).toBeTruthy()); + expect(taskCount()).toBe(String(HOST_ROWS.length)); + expect(screen.queryByTestId('gv-task-e1')).toBeNull(); + }); +}); + +describe('objectui#7333 — an EMPTY host array does not switch off the object-schema gate', () => { + /** A definition with a lookup, so the gated query has a real `$expand` to carry. */ + const OBJECT_DEF = { + name: 'task', + fields: { + id: { name: 'id', type: 'text' }, + subject: { name: 'subject', type: 'text' }, + owner: { name: 'owner', type: 'lookup', reference_to: 'user' }, + visible_from: { name: 'visible_from', type: 'date' }, + due_date: { name: 'due_date', type: 'date' }, + }, + }; + + const OBJECT_ROWS = [ + { id: '1', subject: 'Renewal', owner: 'u1', visible_from: '2026-01-01', due_date: '2026-01-05' }, + ]; + + const objectSchema: any = { + type: 'object-gantt', + objectName: 'task', + gantt: { titleField: 'subject', startDateField: 'visible_from', endDateField: 'due_date' }, + }; + + it('host `data: []` on an object-bound gantt ⇒ ONE query, issued after the schema settles, already expanded', async () => { + const adapter = { + find: vi.fn(async () => ({ data: OBJECT_ROWS, total: OBJECT_ROWS.length })), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn(async () => OBJECT_DEF), + } as any; + + render(); + + await waitFor(() => expect(taskCount()).toBe(String(OBJECT_ROWS.length))); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + await settle(); + // Ungated, the first query goes out before the definition lands and + // carries no expansion, and a second follows once it does: `[null, + // ['owner']]` — the signature `ObjectGantt.fetchGate-7225.test.tsx` + // records for the ungated regime. + const expandSets = adapter.find.mock.calls.map(([, params]: [string, any]) => params?.$expand ?? null); + expect(expandSets).toEqual([['owner']]); + }); +}); diff --git a/packages/plugin-gantt/src/ObjectGantt.tsx b/packages/plugin-gantt/src/ObjectGantt.tsx index 08438cf4c2..78497743f8 100644 --- a/packages/plugin-gantt/src/ObjectGantt.tsx +++ b/packages/plugin-gantt/src/ObjectGantt.tsx @@ -840,6 +840,37 @@ export const ObjectGantt: React.FC = ({ * over by hand. */ const reportOwedRef = useRef(false); + /** + * Rows a HOST component handed down as a `data` prop, or `null` when it + * handed none (objectui#7333). + * + * ⭐ An EMPTY array is `null` here. It is "no host rows yet", ⛔ not "the + * host owns zero rows". Both sites that read this prop — the short-circuit + * at the head of `reload` and `recordQueryDerivesExpand` below — used to + * take `[]` as an authoritative answer (`[]` is truthy, and + * `Array.isArray([])` holds): the chart adopted it and returned before its + * own query, and painted an EMPTY chart in place of the rows its `data` + * config names. A host array that has not been filled yet looks exactly like + * that. So an empty array leaves the chart reading from its own source, as it + * does with no host array at all; a NON-EMPTY one is adopted as it always + * was. Pinned in `ObjectGantt.emptyHostData-7333.test.tsx`. + * + * One predicate, evaluated once per render, for BOTH sites. `reload` reads it + * through `hostRowsRef` rather than closing over `rest`: `reload` does not + * key on the host rows, so a closed-over value is the one from whichever + * render last rebuilt `reload`. When a host's rows arrive after an empty + * first render, `recordQueryDerivesExpand` flips and fires the fetch effect, + * and that run must see the rows that flipped it, not the `[]` before them. + * + * ⚠️ The registry path never hands this prop: the registered `object-gantt` + * renderer forwards no host prop (`ObjectGantt.hostDataProp-7210.test.tsx` + * pins that), and `ObjectGanttProps` does not declare `data`. What reaches it + * is a direct caller that spreads one in. + */ + const hostRowsProp: unknown = (rest as Record).data; + const hostRows = Array.isArray(hostRowsProp) && hostRowsProp.length > 0 ? hostRowsProp : null; + const hostRowsRef = useRef(hostRows); + hostRowsRef.current = hostRows; const reload = useCallback(async ({ silent = false, inPlace = silent }: { silent?: boolean; inPlace?: boolean } = {}) => { const seq = ++reloadSeqRef.current; const isCurrent = () => reloadSeqRef.current === seq; @@ -848,10 +879,12 @@ export const ObjectGantt: React.FC = ({ try { if (inPlace) setRefreshing(true); else setLoading(true); - // 1. Check for data prop (Unified ListView) - if ((rest as any).data && Array.isArray((rest as any).data)) { + // 1. Rows a host handed down — non-empty only (objectui#7333); see + // `hostRows` above for why this reads the ref. + const handedRows = hostRowsRef.current; + if (handedRows) { if (isCurrent()) { - setData((rest as any).data); + setData(handedRows); setRowCeiling(null); // Committed rows clear an earlier failure (objectui#10578) — the // reasoning sits on the adapter's commit below. @@ -1003,18 +1036,21 @@ export const ObjectGantt: React.FC = ({ // The fetch effect below repeats this list for the same reason; the pins in // `ObjectGantt.discardedReloadIdentity-10036.test.tsx` hold the two in // parity by exercising each entry. - // eslint-disable-next-line react-hooks/exhaustive-deps -- (rest as any).data intentionally untracked, matching the original effect + // eslint-disable-next-line react-hooks/exhaustive-deps -- `searchFields` is keyed by its serialised value, `searchFieldsKey`; the host rows are read through `hostRowsRef` (objectui#7333) }, [adapterInputsKey, dataSource, apiFetch, resource, hasInlineData, dataProvider, queryFilter, schema.sort, searchTerm, searchFieldsKey, objectSchema, perms]); /** * Does the query this effect is about to issue DERIVE anything from the - * object schema? Only the adapter branch does. A host-supplied `data` array - * and an inline `value` set both paint with no metadata read at all, so - * gating them would hold a paint on a resolution that buys them nothing. - * Same scoping ObjectCalendar's gate uses, and for the same reason. + * object schema? Only the adapter branch does. Rows a host handed down and + * an inline `value` set both paint with no metadata read at all, so gating + * them would hold a paint on a resolution that buys them nothing. Same + * scoping ObjectCalendar's gate uses, and for the same reason. + * + * "Rows a host handed down" is `hostRows`, the predicate `reload` reads — an + * EMPTY host array is not one (objectui#7333), so it keeps the gate and the + * invalidation subscription of the query the chart still issues. */ - const hasHostData = Array.isArray((rest as any).data); - const recordQueryDerivesExpand = !hasHostData && !hasInlineData; + const recordQueryDerivesExpand = hostRows === null && !hasInlineData; // ⭐ objectui#7225 ask 2 (objectui#6482's undischarged gating half) — the // object schema GATES this query; it does not refine it afterwards. diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index dffb7bf592..8a2ef6e812 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -2071,12 +2071,12 @@ export const ListView = React.forwardRef(({ // The withholding is kept anyway: it is unreachable, not wrong, and it stops // being unreachable the moment that wrapper forwards host props — whether a // non-grid view may fetch unbounded at all is an open maintainer decision - // (objectui#7210, half 2). `ObjectGantt.reload` takes its `rest.data` - // short-circuit on `data && Array.isArray(data)`, and `[]` satisfies both, - // while this view's rows array is never filled (the fetch effect below - // returns early for it). Forwarding it would therefore replace the endpoint's - // tree with an EMPTY chart — not with the stale object rows the old comment - // warned about. + // (objectui#7210, half 2). This view's rows array is not the endpoint's + // answer (the fetch effect below returns early for it), and `ObjectGantt` + // adopts any NON-EMPTY host `data` array as its rows. An EMPTY one no longer + // blanks the chart: it reads as "no host rows yet" and the chart still + // queries its endpoint (objectui#7333). So the withholding guards against + // this array ever being adopted in the endpoint's place. const ganttOwnsData = currentView === 'gantt' && !!schema.data && diff --git a/packages/plugin-list/src/__tests__/ListView.test.tsx b/packages/plugin-list/src/__tests__/ListView.test.tsx index b0a984d362..03f8c841b0 100644 --- a/packages/plugin-list/src/__tests__/ListView.test.tsx +++ b/packages/plugin-list/src/__tests__/ListView.test.tsx @@ -2686,10 +2686,10 @@ describe('ListView — gantt view fed by an api-provider ViewData', () => { // `plugin-gantt/src/ObjectGantt.hostDataProp-7210.test.tsx`). That makes (c) // a guard on a path the registry does not currently reach — keep it. The day // that wrapper forwards host props (objectui#7210, half 2, an open maintainer - // decision) it becomes load-bearing: `ObjectGantt.reload` short-circuits on - // `data && Array.isArray(data)`, and this view's rows array is a truthy `[]` - // forever, so handing it down would paint an EMPTY chart in place of the - // endpoint's tree. + // decision) it becomes load-bearing: this view's rows array is not the + // endpoint's answer, and `ObjectGantt` adopts any NON-EMPTY host array as its + // rows. (An EMPTY one no longer blanks the chart: it reads as "no host rows + // yet" and the chart still queries its endpoint, objectui#7333.) let prevObjectGantt: ReturnType; let ganttCalls: any[];