diff --git a/.changeset/10512-dataset-chart-applied-filter.md b/.changeset/10512-dataset-chart-applied-filter.md new file mode 100644 index 0000000000..60e2282d63 --- /dev/null +++ b/.changeset/10512-dataset-chart-applied-filter.md @@ -0,0 +1,22 @@ +--- +'@object-ui/plugin-list': patch +--- + +On a `chart` list view bound to a semantic `dataset`, a user filter that is already set +no longer narrows `ListView`'s own fetch (objectui#10512). + +The toolbar withholds the Filter builder and the `userFilters` chips on such a view +(objectui#10327), but a filter group the host restored at mount through +`initialFilters`, or a group or chip set on another view before switching, was still +applied to `ListView`'s own fetch of the object's rows. That fetch still runs on the +chart and feeds the record-count bar, so the count was narrowed by a filter nothing on +screen showed or could clear. + +- While a dataset-bound chart is on screen, `ListView`'s fetch and its export apply + the view's own `filter` only; the toolbar's filter group and the chips are not sent. +- The held state is kept rather than cleared: switching back to a view that offers the + controls applies the same group again, and `onFilterChange` is not called for the + switch. +- A chart bound to the list object, and every other view, apply the toolbar's filter + group and the chips as before. A switch into a dataset chart with no user filter set + issues no new request. diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index 03798ad2b3..d44a1bbb86 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -683,6 +683,14 @@ export function buildEffectiveFilter( ); } +/** + * What `ListView` applies in place of a held user filter it withholds + * (`appliedFilters` / `appliedUserFilterConditions`, objectui#10512). Module + * constants, so every render hands the fetch effect the same identity. + */ +const WITHHELD_FILTER_GROUP: FilterGroup = { id: 'root', logic: 'and', conditions: [] }; +const WITHHELD_USER_FILTER_CONDITIONS: unknown[] = []; + export function convertFilterGroupToAST(group: FilterGroup): any[] { if (!group || !group.conditions || group.conditions.length === 0) return []; @@ -1145,6 +1153,15 @@ export const ListView = React.forwardRef(({ * a dataset chart — is refused at authoring by `@object-ui/types`' * `ListViewSchema`. * + * Two readers, one answer: the toolbar flags below, and the APPLIED user + * filter (`appliedFilters` / `appliedUserFilterConditions`, objectui#10512). + * A group the host restores at mount, or one set on a grid before a switch, + * therefore does not go on narrowing this component's fetch — and with it the + * record-count bar — with no control on screen to show or clear it. ⛔ The + * held state itself is kept: switching back to a view that offers the + * controls applies it again. The view's own `schema.filter` is not the + * user's filter and stays applied. + * * Asked of `resolveListChartBinding`, the SAME resolver `case 'chart'` routes * on, so the toolbar and the render branch cannot disagree about the shape. * A primitive, so the memo below keys on the answer, not on an identity @@ -1488,6 +1505,22 @@ export const ListView = React.forwardRef(({ // User Filters State (Airtable Interfaces-style) const [userFilterConditions, setUserFilterConditions] = React.useState([]); + // The user filter this component APPLIES — its fetch, the export, the node of + // a view that queries for itself, the empty-state copy. Empty while a + // dataset-bound chart is on screen (`datasetChartOnScreen`, objectui#10512). + // The held `currentFilters` / `userFilterConditions` stay as they are, for + // the controls and for the host's storage. Each is swapped only when it holds + // something, so a switch with nothing held hands the fetch effect the + // identities it already had and re-issues no query (objectui#7394). + const appliedFilters = + datasetChartOnScreen && currentFilters.conditions && currentFilters.conditions.length > 0 + ? WITHHELD_FILTER_GROUP + : currentFilters; + const appliedUserFilterConditions = + datasetChartOnScreen && userFilterConditions.length > 0 + ? WITHHELD_USER_FILTER_CONDITIONS + : userFilterConditions; + // User filters render ONLY when explicitly configured (ADR-0047 §data // mode): saved list views already act as the preset switcher, so an // unconfigured view keeps a clean toolbar instead of growing auto-derived @@ -2154,7 +2187,7 @@ export const ListView = React.forwardRef(({ try { // Construct filter — shared with the export path so the file a user // downloads is built from the same three sources as the rows on screen. - const finalFilter = buildEffectiveFilter(schema.filter, currentFilters, userFilterConditions); + const finalFilter = buildEffectiveFilter(schema.filter, appliedFilters, appliedUserFilterConditions); // Convert sort to query format // Use array format to ensure order is preserved (Object keys are not guaranteed ordered) @@ -2663,7 +2696,7 @@ export const ListView = React.forwardRef(({ // silently un-suppresses nothing, because the finding it was suppressing // simply moves elsewhere. Add prose ABOVE this point, never below it. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [schema.objectName, schema.data, dataSource, schema.filter, effectivePageSize, currentSort, currentFilters, userFilterConditions, refreshKey, searchTerm, schema.searchableFields, schema.columns, (schema as any).kanban, (schema as any).calendar, (schema as any).gallery, (schema as any).timeline, (schema as any).gantt, schema.map, (schema as any).options, objectDef?.fields, objectDefLoaded, schema.refreshTrigger, perms, fetchSkip, groupingConfig, ganttOwnsData]); // Re-fetch on filter/sort/search/refreshTrigger/perms/window change + }, [schema.objectName, schema.data, dataSource, schema.filter, effectivePageSize, currentSort, appliedFilters, appliedUserFilterConditions, refreshKey, searchTerm, schema.searchableFields, schema.columns, (schema as any).kanban, (schema as any).calendar, (schema as any).gallery, (schema as any).timeline, (schema as any).gantt, schema.map, (schema as any).options, objectDef?.fields, objectDefLoaded, schema.refreshTrigger, perms, fetchSkip, groupingConfig, ganttOwnsData]); // Re-fetch on filter/sort/search/refreshTrigger/perms/window change // Any change to the result-defining inputs (object, filters, sort, search, // grouping, page size) invalidates the current page number — snap back to @@ -2674,7 +2707,7 @@ export const ListView = React.forwardRef(({ // the signature, so turning the page never triggers a reset. const pageResetSignature = JSON.stringify([ schema.objectName, schema.filter, effectivePageSize, currentSort, - currentFilters, userFilterConditions, searchTerm, currentView, groupingConfig, + appliedFilters, appliedUserFilterConditions, searchTerm, currentView, groupingConfig, ]); const prevPageResetSignature = React.useRef(pageResetSignature); React.useEffect(() => { @@ -3042,7 +3075,7 @@ export const ListView = React.forwardRef(({ let selfQueryFilter: unknown = schema.filter; if (currentView === 'gantt' || currentView === 'tree' || currentView === 'chart') { try { - const value = buildEffectiveFilter(schema.filter, currentFilters, userFilterConditions); + const value = buildEffectiveFilter(schema.filter, appliedFilters, appliedUserFilterConditions); const key = JSON.stringify(value ?? null); const cached = selfQueryFilterRef.current; if (cached && cached.key === key) { @@ -3964,7 +3997,7 @@ export const ListView = React.forwardRef(({ .filter(Boolean) as string[]; // The same three filter sources as the data fetch, from the same function. - const finalFilter = buildEffectiveFilter(schema.filter, currentFilters, userFilterConditions); + const finalFilter = buildEffectiveFilter(schema.filter, appliedFilters, appliedUserFilterConditions); const sort = currentSort.length > 0 ? currentSort @@ -4066,7 +4099,7 @@ export const ListView = React.forwardRef(({ setShowExport(false); // `searchTerm` / `searchableFields` belong here: the export now narrows by // the active search, so a stale closure would export the wrong row set. - }, [data, effectiveFields, resolvedExportOptions, schema.objectName, schema.filter, schema.searchableFields, exportPermitted, dataSource, currentFilters, userFilterConditions, currentSort, searchTerm, objectDef, resolveObjectLabel]); + }, [data, effectiveFields, resolvedExportOptions, schema.objectName, schema.filter, schema.searchableFields, exportPermitted, dataSource, appliedFilters, appliedUserFilterConditions, currentSort, searchTerm, objectDef, resolveObjectLabel]); // All available fields for hide/show (with i18n) const allFields = React.useMemo(() => { @@ -4933,8 +4966,8 @@ export const ListView = React.forwardRef(({ const hasActiveQuery = !!(searchTerm && searchTerm.trim()) || hasBaseFilter || - (Array.isArray(userFilterConditions) && userFilterConditions.length > 0) || - (Array.isArray(currentFilters?.conditions) && currentFilters.conditions.length > 0); + (Array.isArray(appliedUserFilterConditions) && appliedUserFilterConditions.length > 0) || + (Array.isArray(appliedFilters?.conditions) && appliedFilters.conditions.length > 0); const title = (typeof schema.emptyState?.title === 'string' ? schema.emptyState.title : undefined) ?? (hasActiveQuery ? t('list.noMatches') : t('list.firstRunTitle')); const description = (typeof schema.emptyState?.message === 'string' ? schema.emptyState.message : undefined) diff --git a/packages/plugin-list/src/__tests__/ListView.datasetChartAppliedFilter-10512.test.tsx b/packages/plugin-list/src/__tests__/ListView.datasetChartAppliedFilter-10512.test.tsx new file mode 100644 index 0000000000..04e1c1eb1b --- /dev/null +++ b/packages/plugin-list/src/__tests__/ListView.datasetChartAppliedFilter-10512.test.tsx @@ -0,0 +1,246 @@ +/** + * 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#10512 — while a dataset-bound `chart` is on screen, a user filter + * that is already held does not narrow `ListView`'s own fetch. + * + * THE DEFECT. objectui#10327 withholds the toolbar Filter builder and the + * `UserFilters` chips on a dataset-bound chart (`datasetChartOnScreen`): the + * chart takes its scope from the dataset, and neither control reaches it. It + * withheld only the CONTROLS. A Filter group the host restores at mount + * (`initialFilters`, which app-shell's `ObjectView` reads back from per-view + * storage), or one set on a grid before switching, still reached this + * component's own fetch of the list object's rows. That fetch still runs on + * the chart and feeds the record-count bar, which still draws there. So the + * count was narrowed by a filter nothing on screen showed or could clear. + * + * objectui#10326 closed the same shape for Search (`searchTerm` is derived + * empty beside the held `searchInput`); this is the Filter half. + * + * WHAT IS PINNED. + * + * - On a dataset-bound chart the fetch runs and the count bar draws, and a + * restored Filter group narrows neither: the data source receives no + * `$filter`, and the bar counts the unnarrowed rows. + * - The view's own `filter` is not the user's. On the same chart it stays in + * the query, alone. + * - The held state is KEPT, not cleared: a switch back to the grid applies + * the same group again, the builder shows it, and the host is never told + * to drop it. Chips toggled on the grid are withheld on the chart too. + * - CONTROLS: on a grid, and on a chart bound to the list object (whose node + * carries the effective filter, objectui#10250), the restored group still + * narrows the fetch. + * - With nothing held, the switch into the dataset chart re-issues no query: + * the withholding does not reopen objectui#7394. + * + * Each case first waits for the view's stand-in, so a mount that never reached + * the view's render branch reads as a broken harness, never as a pass. + */ + +import React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor, cleanup, fireEvent } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +import { SchemaRendererProvider } from '@object-ui/react'; +import { ListView } from '../ListView'; + +let chartNodes: Array> = []; + +ComponentRegistry.register( + 'object-grid', + () =>
, + { namespace: 'test', label: 'object-grid stand-in', category: 'view' }, +); +ComponentRegistry.register( + 'object-chart', + ({ schema }: { schema: Record }) => { + chartNodes.push(schema); + return
; + }, + { namespace: 'test', label: 'object-chart stand-in', category: 'view' }, +); + +const objectDef = { + name: 'duly_task', + label: 'Task', + fields: { + id: { name: 'id', type: 'text' }, + subject: { name: 'subject', type: 'text', label: 'Subject' }, + status: { name: 'status', type: 'text', label: 'Status' }, + priority: { name: 'priority', type: 'text', label: 'Priority' }, + owner: { name: 'owner', type: 'text', label: 'Owner' }, + estimate: { name: 'estimate', type: 'number', label: 'Estimate' }, + }, +}; + +const ROWS = [ + { id: '1', subject: 'Task 1', status: 'open', priority: 'high', owner: 'ada', estimate: 3 }, + { id: '2', subject: 'Task 2', status: 'done', priority: 'low', owner: 'ada', estimate: 5 }, + { id: '3', subject: 'Task 3', status: 'done', priority: null, owner: 'bob', estimate: 8 }, +]; + +/** + * Answers each query from the fields its `$filter` names, so the record-count + * bar reads a different number for a narrowed query than for an unnarrowed one. + */ +function makeDataSource() { + return { + find: vi.fn(async (_object: string, params?: Record) => { + const named = JSON.stringify(params?.$filter ?? null); + let out = ROWS; + if (named.includes('"status"')) out = out.filter((r) => r.status === 'open'); + if (named.includes('"priority"')) out = out.filter((r) => r.priority != null); + if (named.includes('"owner"')) out = out.filter((r) => r.owner === 'ada'); + return { data: out, total: out.length }; + }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn(async () => objectDef), + } as never as { find: ReturnType }; +} + +const DATASET_BLOCK = { chartType: 'bar', dataset: 'task_ds', dimensions: ['status'], values: ['total_estimate'] }; +const OBJECT_BLOCK = { chartType: 'bar', xAxisField: 'status', yAxisFields: ['estimate'], aggregation: 'sum' }; +/** A `UserFilters` toggle chip, OFF at mount; clicking it adds `priority != null`. */ +const TOGGLE_CHIP = { element: 'toggle', fields: [{ field: 'priority' }] }; +/** The view's own `filter` — authored, or merged in by the host (URL filters). */ +const BASE_FILTER = [['owner', '=', 'ada']]; + +/** The toolbar Filter builder's group, as the host restores it at mount. */ +const RESTORED_FILTERS = { + id: 'root', + logic: 'and' as const, + conditions: [{ id: 'c1', field: 'status', operator: 'equals', value: 'open' }], +}; + +function mount(view: Record, listProps: Record = {}) { + const dataSource = makeDataSource(); + const schema = { type: 'list-view', objectName: 'duly_task', columns: ['subject'], ...view }; + render( + + + , + ); + return dataSource; +} + +const onScreen = (testId: string) => waitFor(() => expect(screen.getByTestId(testId)).toBeInTheDocument()); +/** The `$filter` of every host (`ListView`) fetch, in order. */ +const hostFilters = (dataSource: { find: ReturnType }) => + dataSource.find.mock.calls.map((c: unknown[]) => (c[1] as Record | undefined)?.$filter); +const names = (filter: unknown, field: string) => JSON.stringify(filter ?? null).includes(`"${field}"`); +const countBar = () => screen.getByTestId('record-count-bar'); +const filterButton = () => screen.queryByRole('button', { name: /filter/i }); +const settle = () => new Promise((r) => setTimeout(r, 50)); + +const pick = (name: string) => { + const trigger = screen.queryByTestId('view-switcher-dropdown'); + if (trigger) fireEvent.click(trigger); + fireEvent.click(screen.queryByRole('tab', { name }) ?? screen.getByRole('button', { name })); +}; + +beforeEach(() => { + chartNodes = []; +}); +afterEach(cleanup); + +describe('objectui#10512 — a held user filter does not narrow ListView\'s fetch while a dataset-bound chart is on screen', () => { + it('CONTROL: on a grid, a restored Filter group narrows the fetch, and the count bar counts the narrowed rows', async () => { + const dataSource = mount({ viewType: 'grid' }, { initialFilters: RESTORED_FILTERS }); + await onScreen('grid-standin'); + await waitFor(() => expect(names(hostFilters(dataSource).at(-1), 'status')).toBe(true)); + await waitFor(() => expect(countBar()).toHaveTextContent('1 record')); + }); + + it('CONTROL: on a chart bound to the list object, a restored Filter group still narrows the fetch', async () => { + const dataSource = mount({ viewType: 'chart', chart: OBJECT_BLOCK }, { initialFilters: RESTORED_FILTERS }); + await onScreen('chart-standin'); + expect(chartNodes.at(-1)).toMatchObject({ type: 'object-chart', objectName: 'duly_task' }); + await waitFor(() => expect(dataSource.find).toHaveBeenCalled()); + await settle(); + expect(names(hostFilters(dataSource).at(-1), 'status')).toBe(true); + }); + + it('on a dataset-bound chart the fetch runs and the count bar draws, and a restored Filter group narrows neither', async () => { + const onFilterChange = vi.fn(); + const dataSource = mount({ viewType: 'chart', chart: DATASET_BLOCK }, { initialFilters: RESTORED_FILTERS, onFilterChange }); + await onScreen('chart-standin'); + expect(chartNodes.at(-1)).toMatchObject({ type: 'object-chart', dataset: 'task_ds' }); + await waitFor(() => expect(dataSource.find).toHaveBeenCalled()); + await settle(); + expect(hostFilters(dataSource).every((f) => f === undefined)).toBe(true); + expect(countBar()).toHaveTextContent('3 records'); + expect(onFilterChange).not.toHaveBeenCalled(); + }); + + it('the view\'s own `filter` is not the user\'s: on the dataset chart it stays in the query, alone', async () => { + const dataSource = mount( + { viewType: 'chart', chart: DATASET_BLOCK, filter: BASE_FILTER }, + { initialFilters: RESTORED_FILTERS }, + ); + await onScreen('chart-standin'); + await waitFor(() => expect(dataSource.find).toHaveBeenCalled()); + await settle(); + expect(hostFilters(dataSource).every((f) => JSON.stringify(f) === JSON.stringify(BASE_FILTER))).toBe(true); + expect(countBar()).toHaveTextContent('2 records'); + }); + + it('a filter held on a grid stops narrowing the fetch on a switch to the dataset chart, and applies again back on the grid', async () => { + const onFilterChange = vi.fn(); + const dataSource = mount( + { + viewType: 'grid', + chart: DATASET_BLOCK, + userFilters: TOGGLE_CHIP, + appearance: { allowedVisualizations: ['grid', 'chart'] }, + }, + { showViewSwitcher: true, initialFilters: RESTORED_FILTERS, onFilterChange }, + ); + await onScreen('grid-standin'); + await waitFor(() => expect(names(hostFilters(dataSource).at(-1), 'status')).toBe(true)); + + // A chip toggled on the grid joins the held state. + fireEvent.click(await screen.findByRole('button', { name: /priority/i })); + await waitFor(() => expect(names(hostFilters(dataSource).at(-1), 'priority')).toBe(true)); + + pick('Chart'); + await onScreen('chart-standin'); + await waitFor(() => expect(hostFilters(dataSource).at(-1)).toBeUndefined()); + await waitFor(() => expect(countBar()).toHaveTextContent('3 records')); + + pick('Grid'); + await onScreen('grid-standin'); + await waitFor(() => expect(names(hostFilters(dataSource).at(-1), 'status')).toBe(true)); + // Kept, not cleared: the builder still shows the group, and the host was + // never told to drop it. ⛔ The chip's own return is not asserted here: its + // on/off state lives inside `UserFilters`, which is not mounted while the + // dataset chart is on screen, so what it draws on the way back is that + // component's answer, not this derivation's. + expect(filterButton()).toHaveTextContent('1'); + expect(onFilterChange).not.toHaveBeenCalled(); + }); + + it('CONTROL (objectui#7394): with nothing held, the switch to the dataset chart re-issues no query', async () => { + const dataSource = mount( + { viewType: 'grid', chart: DATASET_BLOCK, appearance: { allowedVisualizations: ['grid', 'chart'] } }, + { showViewSwitcher: true }, + ); + await onScreen('grid-standin'); + await waitFor(() => expect(dataSource.find).toHaveBeenCalled()); + await settle(); + const before = dataSource.find.mock.calls.length; + + pick('Chart'); + await onScreen('chart-standin'); + await settle(); + expect(dataSource.find.mock.calls.length).toBe(before); + }); +});