diff --git a/.changeset/10887-dashboard-filter-options-bus-reader.md b/.changeset/10887-dashboard-filter-options-bus-reader.md new file mode 100644 index 0000000000..eeeb3fd42e --- /dev/null +++ b/.changeset/10887-dashboard-filter-options-bus-reader.md @@ -0,0 +1,23 @@ +--- +'@object-ui/plugin-dashboard': patch +--- + +fix(plugin-dashboard): a dashboard filter's `optionsFrom` options re-read on the data-invalidation bus + +A `select` or `lookup` entry in a dashboard's `globalFilters` that reads its +options through `optionsFrom` now re-reads them when the bus reports a change to +`optionsFrom.object` (`notifyDataChanged`, as a page action over raw HTTP +does), or an unscoped `'*'`, on either read: the server-side dataset GROUP BY +and the client-side `find` fallback. A change to another object does not +re-read. The options on screen stay until the re-read answers, and the value the +user selected is kept: it is the dashboard variable's, and the read never writes +it. Before, the options refreshed after such a write only when the host +remounted the dashboard, and `PageView` is about to stop doing that +(objectui#10519). + +A filter with authored `options` and no `optionsFrom` reads nothing and does not +subscribe. + +**Clause-②: no** — no exported symbol, prop or authored key is added, removed, +renamed or retyped, and no accept set moves. What changes is when an existing +read runs. diff --git a/.changeset/10887-object-view-non-grid-bus-reader.md b/.changeset/10887-object-view-non-grid-bus-reader.md new file mode 100644 index 0000000000..abfea1447b --- /dev/null +++ b/.changeset/10887-object-view-non-grid-bus-reader.md @@ -0,0 +1,26 @@ +--- +'@object-ui/plugin-view': patch +--- + +fix(plugin-view): an `object-view` in a non-grid view re-reads its rows on the data-invalidation bus + +For the non-grid views (kanban, calendar, gallery, timeline, map, gantt), +`ObjectView` fetches the rows itself and hands them to the inner view as +`data`, which switches off that view's own bus reader. That fetch +now names the `useDataInvalidation` nonce for `schema.objectName`, so a write +declared on the bus (`notifyDataChanged`, as a page action over raw HTTP does), +or an unscoped `'*'`, re-reads the rows in place: the inner view is not +remounted. A change to another object does not re-read. Before, such a write +reached these views only when their host remounted them, and `PageView` is +about to stop doing that (objectui#10519). + +The subscription follows the rows the view draws: a host `renderListView` (its +`ListView` reads the bus itself) and the grid (`ObjectGrid` does too) do not +subscribe here, and neither does a view with no object or no data source. Nor +do the host-only `tree` and `chart` views: their renderers query for themselves +and read the bus themselves, so a re-read here would only add reads beside +theirs. + +**Clause-②: no** — no exported symbol, prop or authored key is added, removed, +renamed or retyped, and no accept set moves. What changes is when an existing +read runs. diff --git a/packages/plugin-dashboard/src/DashboardFilterBar.tsx b/packages/plugin-dashboard/src/DashboardFilterBar.tsx index 0d534faf48..010cd9583f 100644 --- a/packages/plugin-dashboard/src/DashboardFilterBar.tsx +++ b/packages/plugin-dashboard/src/DashboardFilterBar.tsx @@ -33,6 +33,7 @@ import { } from '@object-ui/components'; import { CalendarIcon, RotateCcw } from 'lucide-react'; import { useSafeTranslate, useObjectTranslation, useSafeFieldLabel, pickLocalized } from '@object-ui/i18n'; +import { useDataInvalidation } from '@object-ui/react'; import { DATE_RANGE_PRESETS, toDisplayDate, @@ -397,6 +398,22 @@ function SelectFilter({ def, value, onChange, dataSource }: { def: DashboardFilt // it, by CONTENT: an equal filter in a fresh object is not a change // (AGENTS.md #10). const optionsFilterKey = JSON.stringify(from?.filter ?? null); + // objectui#10887 — the data-invalidation bus (`notifyDataChanged` from + // `@object-ui/react`), read the objectui#10853 way (the record picker's + // options): the nonce moves when the bus reports a change to the object the + // options are read from (or `'*'`), and the effect below names it, so the + // options are re-read. Before, a page action over raw HTTP left them stale + // unless `PageView` remounted the page, and objectui#10519 removes that + // remount. The re-read is in place: the options on screen stay until the + // answer swaps them (nothing here resets `dynamicOptions`), and the selected + // value is the dashboard variable's, which this effect never writes. + // Subscribed exactly when the effect below can read (an `optionsFrom` and an + // adapter that serves either read). + const invalidationNonce = useDataInvalidation( + from && dataSource && (typeof dataSource.queryDataset === 'function' || typeof dataSource.find === 'function') + ? from.object || undefined + : undefined, + ); useEffect(() => { if (!from || !dataSource) return; let cancelled = false; @@ -473,7 +490,7 @@ function SelectFilter({ def, value, onChange, dataSource }: { def: DashboardFilt } return () => { cancelled = true; }; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [from?.object, from?.valueField, from?.labelField, optionsFilterKey, dataSource]); + }, [from?.object, from?.valueField, from?.labelField, optionsFilterKey, dataSource, invalidationNonce]); const localizedOptions = useMemo(() => { // `def.options` is already normalized to `{ value, label }` PAIRS by diff --git a/packages/plugin-dashboard/src/__tests__/DashboardFilterBar.busReread-10887.test.tsx b/packages/plugin-dashboard/src/__tests__/DashboardFilterBar.busReread-10887.test.tsx new file mode 100644 index 0000000000..6f67c7c8a7 --- /dev/null +++ b/packages/plugin-dashboard/src/__tests__/DashboardFilterBar.busReread-10887.test.tsx @@ -0,0 +1,203 @@ +/** + * 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#10887 member 2 — a dashboard filter's `optionsFrom` options are + * re-read when the data-invalidation bus (`notifyDataChanged` from + * `@object-ui/react`) reports a change to `optionsFrom.object`, and the value + * the user selected survives the re-read. + * + * Before this card the options effect keyed on the object, the fields, the + * options filter and the data source, and on no nonce. After a page action over + * raw HTTP the filter kept offering the pre-action values until `PageView` + * remounted the page, and objectui#10519 removes that remount. The effect now + * names the `useDataInvalidation` nonce for `optionsFrom.object`, the + * objectui#10853 shape (the record picker's options). + * + * Rendered through the real `SchemaRenderer` and this package's own + * registration of `dashboard`, with the adapter injected by + * `SchemaRendererProvider`, on both option reads: the server-side dataset + * GROUP BY and the client-side `find` fallback. The selected value lives in the + * dashboard's own variables provider, so it is chosen through the real select. + * The bare `useDataInvalidation` reader beside the dashboard and an + * `object-metric` block (it reads the bus itself) are the positive controls. + */ +import * as React from 'react'; +import { describe, it, expect, vi, beforeAll, afterEach } from 'vitest'; +import { render, act, cleanup, fireEvent, screen, waitFor } from '@testing-library/react'; +import { SchemaRenderer, SchemaRendererProvider, notifyDataChanged, useDataInvalidation } from '@object-ui/react'; +// Module scope, not a hook: this import IS the registration of `dashboard`. +import '../index'; + +// Radix Select opens on pointer events the DOM environment does not implement; +// the same shim `DashboardFilterBar.options.test.tsx` uses. +beforeAll(() => { + class MockPointerEvent extends Event { + button: number; + ctrlKey: boolean; + pointerType: string; + constructor(type: string, props: PointerEventInit = {}) { + super(type, props); + this.button = props.button ?? 0; + this.ctrlKey = props.ctrlKey ?? false; + this.pointerType = props.pointerType ?? 'mouse'; + } + } + Object.assign(window, { PointerEvent: MockPointerEvent }); + Object.assign(HTMLElement.prototype, { + hasPointerCapture: vi.fn(), + releasePointerCapture: vi.fn(), + scrollIntoView: vi.fn(), + }); +}); + +afterEach(cleanup); + +type Path = 'dataset' | 'fallback'; + +/** + * The options on the first read are `finance` and `retail`; every later read + * adds `energy`, so a re-read that landed is visible in the list. + */ +function makeDataSource(path: Path) { + let answered = 0; + const values = () => { + answered += 1; + return answered === 1 ? ['finance', 'retail'] : ['energy', 'finance', 'retail']; + }; + const find = vi.fn(async (_object: string, _query?: unknown) => ({ data: values().map((industry) => ({ industry })) })); + const queryDataset = vi.fn(async (_draft: unknown, _selection?: unknown) => ({ + rows: values().map((industry) => ({ industry, option_count: 1 })), + })); + return path === 'dataset' ? { find, queryDataset } : { find }; +} + +/** The option reads, whichever of the two paths served them. */ +const reads = (ds: ReturnType, path: Path) => + path === 'dataset' ? (ds as { queryDataset: ReturnType }).queryDataset : ds.find; + +/** The positive control: a bare reader of the filter's source object. */ +function BusControl() { + return {useDataInvalidation('accounts')}; +} + +const selectFilter = (extra: Record = {}) => ({ + name: 'industry', + field: 'industry', + label: 'Industry', + type: 'select', + ...extra, +}); + +const dashboardWith = (filter: Record) => ({ type: 'dashboard', globalFilters: [filter], widgets: [] }); + +const OPTIONS_FROM = { optionsFrom: { object: 'accounts', valueField: 'industry', labelField: 'industry' } }; + +function mount(path: Path, node: Record = dashboardWith(selectFilter(OPTIONS_FROM))) { + const ds = makeDataSource(path); + render( + + + + , + ); + return ds; +} + +const rest = () => act(() => new Promise((resolve) => setTimeout(resolve, 60))); +const emit = (change: { objectName: string; recordId?: string }) => + act(async () => { + notifyDataChanged(change); + }); +const trigger = () => screen.getByTestId('dashboard-filter-industry'); + +async function mountAtRest(path: Path) { + const ds = mount(path); + const read = reads(ds, path); + await waitFor(() => expect(read).toHaveBeenCalledTimes(1)); + await rest(); + expect(read, 'one read on mount').toHaveBeenCalledTimes(1); + return { ds, read }; +} + +describe('a dashboard filter re-reads its optionsFrom options on the data-invalidation bus (objectui#10887)', () => { + for (const path of ['dataset', 'fallback'] as const) { + it(`an unscoped change ("*") re-reads the options once (${path} read)`, async () => { + const { read } = await mountAtRest(path); + + await emit({ objectName: '*' }); + await rest(); + + expect(screen.getByTestId('bus-control').textContent, 'control: the event never reached a subscriber').toBe('1'); + expect(read, 'the options never re-read after the bus reported a change').toHaveBeenCalledTimes(2); + }); + + it(`a change to its own object re-reads once; an unrelated object does not (${path} read)`, async () => { + const { read } = await mountAtRest(path); + + await emit({ objectName: 'unrelated_object' }); + await rest(); + expect(read, 'a change to another object re-read the options').toHaveBeenCalledTimes(1); + + await emit({ objectName: 'accounts', recordId: 'a1' }); + await rest(); + expect(read, 'a change to its own object did not re-read the options exactly once').toHaveBeenCalledTimes(2); + }); + } + + it('the selected value survives the re-read, and the re-read options reach the list', async () => { + const { read } = await mountAtRest('dataset'); + + fireEvent.pointerDown(trigger(), { button: 0 }); + fireEvent.click(await screen.findByRole('option', { name: 'retail' })); + await waitFor(() => expect(trigger().textContent).toBe('retail')); + await rest(); + const triggerNode = trigger(); + + await emit({ objectName: '*' }); + await rest(); + expect(read).toHaveBeenCalledTimes(2); + + expect(trigger(), 'the re-read remounted the filter').toBe(triggerNode); + expect(trigger().textContent, 'the re-read dropped the selected value').toBe('retail'); + fireEvent.pointerDown(trigger(), { button: 0 }); + expect(await screen.findByRole('option', { name: 'energy' }), 'the re-read options never reached the list').toBeTruthy(); + expect(trigger().textContent).toBe('retail'); + }); + + it('lit control: an object-metric block beside the dashboard re-reads on the same event through its own reader', async () => { + const ds = { ...makeDataSource('dataset'), aggregate: vi.fn(async () => [{ industry: 3 }]) }; + render( + + + + , + ); + await waitFor(() => expect(ds.aggregate).toHaveBeenCalledTimes(1)); + await rest(); + + await emit({ objectName: '*' }); + await rest(); + + expect(ds.aggregate, 'the metric never re-read: the event did not reach a block that reads the bus').toHaveBeenCalledTimes(2); + }); + + it('control: a select filter with no optionsFrom reads nothing, on mount or on an invalidation', async () => { + const ds = mount('dataset', dashboardWith(selectFilter({ options: [{ value: 'finance', label: 'Finance' }] }))); + await rest(); + + await emit({ objectName: '*' }); + await rest(); + + expect(screen.getByTestId('bus-control').textContent).toBe('1'); + expect(reads(ds, 'dataset')).not.toHaveBeenCalled(); + expect(ds.find).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/plugin-view/src/ObjectView.tsx b/packages/plugin-view/src/ObjectView.tsx index 55cf6088c9..b3891b799c 100644 --- a/packages/plugin-view/src/ObjectView.tsx +++ b/packages/plugin-view/src/ObjectView.tsx @@ -81,7 +81,7 @@ import { resolveFilterPlaceholders, type FilterTokenScope, } from '@object-ui/core'; -import { SchemaRenderer as ImportedSchemaRenderer, useSettledSchema, notifyDataChanged, useFilterScope } from '@object-ui/react'; +import { SchemaRenderer as ImportedSchemaRenderer, useSettledSchema, notifyDataChanged, useDataInvalidation, useFilterScope } from '@object-ui/react'; import type { HandleClickModifiers } from '@object-ui/react'; import { usePermissions } from '@object-ui/permissions'; import { ViewSwitcher } from './ViewSwitcher'; @@ -1100,6 +1100,30 @@ export const ObjectView: React.FC = ({ // an equal sort in a fresh array is not a change (AGENTS.md #10). const tableSortKey = JSON.stringify(schema.table?.sort ?? null); + // objectui#10887 — the data-invalidation bus (`notifyDataChanged` from + // `@object-ui/react`), read the objectui#10623 / objectui#10778 / + // objectui#10853 way: the nonce moves when the bus reports a change to the + // object this fetch QUERIES (or `'*'`), and the effect below names it, so + // the rows are re-read in place. The inner view receives them as `data`, + // which switches off its own bus reader, and `refreshKey` moves only on this + // view's own write and `onMutation`; a page action over raw HTTP fires + // neither, so before this the rows were re-read only when `PageView` + // remounted the page (objectui#10519 removes that remount). + // + // Subscribed exactly when these rows are what the view draws. A host + // `renderListView` (its `ListView` reads the bus itself) and the grid + // (`ObjectGrid` does too) are not this effect's query, and neither is a view + // with no object or no adapter. The two host-only types query for + // themselves and read the bus themselves, so a re-read here would only add + // reads: `ObjectTree` runs its own query ahead of the rows handed to it + // (objectui#10778) and re-queries whenever that array changes, and + // `ObjectChart` never reads them (objectui#10035). + const fetchDrawsView = + !renderListView && currentViewType !== 'grid' && currentViewType !== 'tree' && currentViewType !== 'chart'; + const invalidationNonce = useDataInvalidation( + fetchDrawsView && dataSource ? schema.objectName || undefined : undefined, + ); + // Fetch data for non-grid view types (grid handles its own data via ObjectGrid) useEffect(() => { let isMounted = true; @@ -1309,6 +1333,7 @@ export const ObjectView: React.FC = ({ schema.objectName, dataSource, currentViewType, refreshKey, currentNamedViewConfig, activeViewQueryInputs, renderListView, objectSchemaReady, objectSchema, perms, authoredFilters, tableSortKey, + invalidationNonce, ]); // Determine layout mode. #2578: default the record surface from how heavy the diff --git a/packages/plugin-view/src/__tests__/ObjectView.busReread-10887.test.tsx b/packages/plugin-view/src/__tests__/ObjectView.busReread-10887.test.tsx new file mode 100644 index 0000000000..dc8ac5e6e4 --- /dev/null +++ b/packages/plugin-view/src/__tests__/ObjectView.busReread-10887.test.tsx @@ -0,0 +1,211 @@ +/** + * 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#10887 member 1 — an `object-view` in a non-grid view re-reads its + * rows when the data-invalidation bus (`notifyDataChanged` from + * `@object-ui/react`) reports a change to the object it queries. + * + * For every non-grid view `ObjectView` fetches the rows itself and hands them + * to the inner view as `data`, which switches off that view's own bus reader. + * Before this card the fetch effect keyed on `refreshKey`, which moves only on + * the view's own write and `dataSource.onMutation`. A page action over raw HTTP + * fires neither, so the rows were re-read only because `PageView` remounted + * the page, and objectui#10519 removes that remount. The effect now names the + * `useDataInvalidation` nonce for `schema.objectName`, the objectui#10623 / + * #10778 / #10853 shape. + * + * Rendered through the real `SchemaRenderer` and this package's own + * registration of `object-view`, in the page-region shape (`properties`). The + * fake data source counts reads and implements no `onMutation`, as a page + * action's raw HTTP write announces none. The inner views are stand-ins + * registered in the real registry: this package does not depend on the + * plugins that register them, and the rows are this component's read. Each + * stand-in carries an instance id from a `useState` initializer, so a changed + * id is a remount. + * + * Controls: the bare `useDataInvalidation` reader beside the view and a grid + * `object-view` (`ObjectGrid` reads the bus itself) move on every event, a + * view with no object never reads, and the two host-only types whose + * renderers query for themselves (`tree`, `chart`) are not re-read by this + * fetch. + */ +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, act, cleanup, screen, waitFor } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +import { + SchemaRenderer, + SchemaRendererProvider, + notifyDataChanged, + useDataInvalidation, +} from '@object-ui/react'; +// Module scope, not a hook: this import IS the registration of `object-view`. +import '../index'; +import { ObjectView } from '../ObjectView'; + +const NON_GRID_TYPES = ['kanban', 'calendar', 'gallery', 'timeline', 'map', 'gantt'] as const; + +let instanceSeq = 0; +function InnerViewStandIn({ schema, data }: { schema?: { type?: string }; data?: unknown }) { + const [id] = React.useState(() => ++instanceSeq); + return ( +
+ ); +} +for (const t of [...NON_GRID_TYPES, 'tree', 'chart']) ComponentRegistry.register(`object-${t}`, InnerViewStandIn as never); + +/** The positive control: a bare reader of the view's object. */ +function BusControl() { + return {useDataInvalidation('deal')}; +} + +beforeEach(() => { + // Best-effort metadata probes are not what these cases are about. + vi.stubGlobal('fetch', vi.fn(async () => ({ ok: true, status: 200, json: async () => ({}), text: async () => '{}' }))); +}); +afterEach(() => { + vi.unstubAllGlobals(); + cleanup(); +}); + +/** Answers one row more on every read, so a later delivery is distinguishable. */ +function makeDataSource() { + let answered = 0; + return { + // The parameters are declared so `mock.calls[i][0]` below is typed. + find: vi.fn(async (_objectName: string, _query?: unknown) => { + answered += 1; + const rows = Array.from({ length: answered }, (_, i) => ({ id: String(i + 1), name: `Deal ${i + 1}` })); + return { data: rows, total: rows.length }; + }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn(async () => ({ name: 'deal', fields: { name: { type: 'text' } } })), + }; +} + +function mount(properties: Record) { + const ds = makeDataSource(); + render( + + + + , + ); + return ds; +} + +const rest = () => act(() => new Promise((resolve) => setTimeout(resolve, 60))); +const emit = (change: { objectName: string; recordId?: string }) => + act(async () => { + notifyDataChanged(change); + }); +const inner = () => screen.getByTestId('inner-view'); + +async function mountAtRest(viewType: string) { + const ds = mount({ objectName: 'deal', defaultViewType: viewType }); + await waitFor(() => expect(ds.find).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(inner().getAttribute('data-rows')).toBe('1')); + await rest(); + expect(ds.find, 'one read on mount').toHaveBeenCalledTimes(1); + return ds; +} + +describe('object-view non-grid views re-read on the data-invalidation bus (objectui#10887)', () => { + for (const viewType of NON_GRID_TYPES) { + it(`${viewType}: an unscoped change ("*") re-reads once, in place`, async () => { + const ds = await mountAtRest(viewType); + const instance = inner().getAttribute('data-instance'); + + await emit({ objectName: '*' }); + await rest(); + + expect(screen.getByTestId('bus-control').textContent, 'control: the event never reached a subscriber').toBe('1'); + expect(ds.find, 'the rows never re-read after the bus reported a change').toHaveBeenCalledTimes(2); + expect(ds.find.mock.calls[1][0]).toBe('deal'); + await waitFor(() => expect(inner().getAttribute('data-rows'), 'the re-read rows never reached the view').toBe('2')); + expect(inner().getAttribute('data-type')).toBe(`object-${viewType}`); + expect(inner().getAttribute('data-instance'), 'the re-read remounted the view').toBe(instance); + }); + + it(`${viewType}: a change to its own object re-reads once; an unrelated object does not`, async () => { + const ds = await mountAtRest(viewType); + + await emit({ objectName: 'unrelated_object' }); + await rest(); + expect(ds.find, 'a change to another object re-read the rows').toHaveBeenCalledTimes(1); + + await emit({ objectName: 'deal', recordId: '1' }); + await rest(); + expect(ds.find, 'a change to its own object did not re-read the rows exactly once').toHaveBeenCalledTimes(2); + await waitFor(() => expect(inner().getAttribute('data-rows')).toBe('2')); + }); + } + + it('lit control: a grid object-view re-reads on the same event through its own reader (ObjectGrid)', async () => { + const ds = mount({ objectName: 'deal', defaultViewType: 'grid' }); + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + await rest(); + const before = ds.find.mock.calls.length; + + await emit({ objectName: '*' }); + await rest(); + + expect(ds.find, 'the grid never re-read: the event did not reach a block that reads the bus').toHaveBeenCalledTimes(before + 1); + }); + + it('control: an object-view with no object reads nothing, on mount or on an invalidation', async () => { + const ds = mount({ defaultViewType: 'kanban' }); + await rest(); + + await emit({ objectName: '*' }); + await rest(); + + expect(screen.getByTestId('bus-control').textContent).toBe('1'); + expect(ds.find).not.toHaveBeenCalled(); + }); + + // `tree` and `chart` are reachable only from a host `views` prop, not through + // the registered renderer, so these compose the component the way a host + // does. Their renderers query for themselves and read the bus themselves: + // `ObjectTree` runs its own query ahead of the rows handed to it and + // re-queries when that array changes, and `ObjectChart` never reads them. A + // re-read of this component's rows would only add reads beside theirs. + for (const hostType of ['tree', 'chart'] as const) { + it(`control: a host-composed ${hostType} view is not re-read by this fetch`, async () => { + const ds = makeDataSource(); + render( + + + + , + ); + await waitFor(() => expect(inner().getAttribute('data-type')).toBe(`object-${hostType}`)); + await rest(); + const before = ds.find.mock.calls.length; + + await emit({ objectName: '*' }); + await rest(); + + expect(screen.getByTestId('bus-control').textContent).toBe('1'); + expect(ds.find, `the ${hostType} view's rows were re-read beside its own reader`).toHaveBeenCalledTimes(before); + }); + } +});