From 4ac67e54684446f570959d5a6ab0ee0a0d959a3c Mon Sep 17 00:00:00 2001 From: Ihor Barmak Date: Sat, 5 Sep 2026 21:38:38 +0300 Subject: [PATCH] feat(dashboard): add defaultColumnFilters to ListPage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a `defaultColumnFilters` prop to `ListPage`, letting a route declare column filters that apply until the user configures their own. This is what a list page needs in order to hide rows behind a soft-hide flag by default — archived products, unpublished drafts, out-of-season items — while still letting the user clear the filter to see them. The defaults apply only while the user has no filter state of their own, which means the saved table settings have to tell "has never configured filters on this page" apart from "has cleared every filter". Both used to look identical: the data table reported its filter state on mount, and `ListPage` persisted whatever it was told, so simply visiting a page saved an empty filter state. The data table now reports filter changes only when the filters actually change, so the absence of a saved `columnFilters` entry means the user has never touched them, and an empty array means they cleared them. Reapplying the defaults over a deliberately cleared state is what that distinction prevents. Existing installs already carry the empty entries that mount-time write produced, one for every list page anyone has ever opened, so without a migration the new prop would be inert for exactly the users it is meant for. The user settings now carry a `tableSettingsVersion` stamp, and settings loaded below the current stamp — from localStorage and from the server-side SettingsStore alike — are brought up to it and the stamp written back. A legacy auto-write cannot be told apart from a genuine clear, so both are dropped: a user who really had cleared every filter on a page that declares defaults sees those defaults one more time and clears them once more, which is much less bad than defaults that never apply at all. The migration is a ladder of individually gated steps rather than one block, so that each step runs only against settings stamped below it. Version 1's step drops the empty entries; once a copy carries that stamp an empty `columnFilters` in it is a real user clear, and a later bump must run only its own new step over it. Settings stamped above the current version — what a rolled-back install reads — are left alone rather than migrated and stamped backwards. `ListPage` also holds the data table back until the user settings have resolved, but only on pages that pass both `defaultColumnFilters` and `pageId`. The table seeds its filter state from props once, on mount, so mounting before the server settings arrive would put the default filter chips on screen over a list that was still being fetched unfiltered. `defaultColumnFilters` requires `pageId`, since without it the filters live only in the URL, which has no way to express the difference; in development, setting one without the other logs a warning rather than silently doing nothing. --- .../dashboard/list-views/data-table.mdx | 4 +- .../dashboard/list-views/list-page.mdx | 32 ++- .../list-page-filter-persistence.spec.ts | 152 +++++++++++++ .../data-table-column-filters.spec.ts | 32 +++ .../data-table/data-table-column-filters.ts | 17 ++ .../lib/components/data-table/data-table.tsx | 16 +- .../src/lib/framework/page/list-page.spec.tsx | 113 +++++++++- .../src/lib/framework/page/list-page.tsx | 75 ++++++- .../src/lib/providers/user-settings.spec.tsx | 211 ++++++++++++++++++ .../src/lib/providers/user-settings.tsx | 84 ++++++- 10 files changed, 713 insertions(+), 23 deletions(-) create mode 100644 packages/dashboard/e2e/tests/components/list-page-filter-persistence.spec.ts create mode 100644 packages/dashboard/src/lib/components/data-table/data-table-column-filters.spec.ts create mode 100644 packages/dashboard/src/lib/components/data-table/data-table-column-filters.ts create mode 100644 packages/dashboard/src/lib/providers/user-settings.spec.tsx diff --git a/docs/docs/reference/dashboard/list-views/data-table.mdx b/docs/docs/reference/dashboard/list-views/data-table.mdx index 26d4d431504..d1277e0f2e7 100644 --- a/docs/docs/reference/dashboard/list-views/data-table.mdx +++ b/docs/docs/reference/dashboard/list-views/data-table.mdx @@ -4,7 +4,7 @@ generated: true --- ## DataTable - + A data table which includes sorting, filtering, pagination, bulk actions, column controls etc. @@ -21,7 +21,7 @@ Parameters ## DataTableProps - + Props for configuring the [DataTable](/reference/dashboard/list-views/data-table#datatable). diff --git a/docs/docs/reference/dashboard/list-views/list-page.mdx b/docs/docs/reference/dashboard/list-views/list-page.mdx index 78736a39ed9..0c1b63fe944 100644 --- a/docs/docs/reference/dashboard/list-views/list-page.mdx +++ b/docs/docs/reference/dashboard/list-views/list-page.mdx @@ -4,7 +4,7 @@ generated: true --- ## ListPage - + Auto-generates a list page with columns generated based on the provided query document fields. @@ -103,7 +103,7 @@ Parameters ## ListPageProps - + Props to configure the [ListPage](/reference/dashboard/list-views/list-page#listpage) component. @@ -121,6 +121,7 @@ interface ListPageProps, U extends ListQuerySh additionalColumns?: AC; defaultColumnOrder?: (keyof ListQueryFields | keyof AC | CustomFieldKeysOfItem>)[]; defaultSort?: SortingState; + defaultColumnFilters?: ColumnFiltersState; defaultVisibility?: Partial< Record | keyof AC | CustomFieldKeysOfItem>, boolean> >; @@ -378,6 +379,33 @@ Allows you to specify the default sorting applied to the table. ```tsx defaultSort={[{ id: 'updatedAt', desc: true }]} ``` +### defaultColumnFilters + + + +Allows you to specify column filters which are applied when the current user has not +yet configured any filters for this page. They behave exactly as if the user had set +them: they show up as active filter chips, and can be edited or removed. + +Once the user edits the filters, their choice is persisted and these defaults are no +longer applied — including when the user removes every filter. Clearing the filters is +persisted as a deliberate choice, so the defaults do not reappear on the next visit. + +Requires `pageId` to be set, since the persisted table settings are what distinguishes +"not configured yet" from "cleared by the user". Without a `pageId` the filters live +only in the URL, which cannot express that difference, and the defaults are ignored. + +*Example* + +```tsx + +``` ### defaultVisibility | keyof AC | CustomFieldKeysOfItem>, boolean> >`} /> diff --git a/packages/dashboard/e2e/tests/components/list-page-filter-persistence.spec.ts b/packages/dashboard/e2e/tests/components/list-page-filter-persistence.spec.ts new file mode 100644 index 00000000000..194dabb6cf0 --- /dev/null +++ b/packages/dashboard/e2e/tests/components/list-page-filter-persistence.spec.ts @@ -0,0 +1,152 @@ +import { type Page, expect, test } from '@playwright/test'; + +import { BaseListPage } from '../../page-objects/list-page.base.js'; + +// Must match `LS_KEY_USER_SETTINGS` in src/lib/constants.ts. Inlined rather than imported: +// that module pulls in the generated GraphQL schema enums, which do not belong in the +// Playwright process. +const USER_SETTINGS_KEY = 'vendure-user-settings'; +const PAGE_ID = 'product-list'; + +/** + * Reads the saved column filters for a page, distinguishing "no value has been saved" + * from "an empty value has been saved" — `undefined` alone cannot survive the trip out + * of `page.evaluate`. + */ +async function readSavedColumnFilters(page: Page, pageId: string) { + return page.evaluate( + ([key, id]) => { + const settings = JSON.parse(localStorage.getItem(key) || '{}'); + const tableSettings = settings.tableSettings?.[id] ?? {}; + return { + saved: Object.prototype.hasOwnProperty.call(tableSettings, 'columnFilters'), + value: tableSettings.columnFilters ?? null, + }; + }, + [USER_SETTINGS_KEY, pageId] as const, + ); +} + +/** + * Puts the page back into the "user has never configured filters here" state that a + * first-time visitor sees. The stored auth state is shared between tests, so the saved + * settings cannot be assumed to be empty at the start of a test. + */ +async function resetSavedTableSettings(page: Page, pageId: string) { + await page.evaluate( + ([key, id]) => { + const settings = JSON.parse(localStorage.getItem(key) || '{}'); + if (settings.tableSettings) { + delete settings.tableSettings[id]; + } + localStorage.setItem(key, JSON.stringify(settings)); + }, + [USER_SETTINGS_KEY, pageId] as const, + ); +} + +/** + * Waits for the products list query itself to come back, rather than for any admin-api + * response — the dashboard has other requests in flight (user settings, saved views), and + * matching one of those would let the assertions run against a list that has not refreshed. + * + * Returns the pending wait, so callers can register it before the action that triggers the + * refetch; awaiting it afterwards would race the response. + */ +function waitForProductList(page: Page) { + return page.waitForResponse( + resp => + resp.url().includes('/admin-api') && + resp.request().postData()?.includes('ProductList') === true && + resp.status() === 200, + ); +} + +function productList(page: Page) { + return new BaseListPage(page, { + path: '/products', + title: 'Products', + newButtonLabel: 'New Product', + }); +} + +async function applyNameFilter(page: Page, value: string) { + const lp = productList(page); + await lp.openAddFilterMenu(); + const dropdown = page.locator('[data-slot="dropdown-menu-content"]'); + await expect(dropdown).toBeVisible(); + await dropdown.getByRole('menuitem', { name: /name/i }).click(); + + const dialog = page.locator('[role="dialog"]'); + await expect(dialog).toBeVisible(); + await dialog.getByPlaceholder('Enter filter value...').fill(value); + await Promise.all([ + waitForProductList(page), + dialog.getByRole('button', { name: 'Apply filter' }).click(), + ]); +} + +test.describe('List page column filter persistence', () => { + // #5294 — `ListPage`'s `defaultColumnFilters` applies a page's default filters only until + // the user configures their own, which requires the saved table settings to tell "never + // configured filters here" apart from "cleared every filter". Both used to be an empty + // array, because the data table reported its filter state on mount and the list page + // persisted whatever it was told. + test('should not save a filter state merely because the list page was visited', async ({ page }) => { + const lp = productList(page); + await lp.goto(); + await lp.expectLoaded(); + + // Start from a clean slate, then load the page as a first-time visitor would. + await resetSavedTableSettings(page, PAGE_ID); + await page.reload(); + await lp.expectLoaded(); + await lp.expectRowsLoaded(); + + const savedFilters = await readSavedColumnFilters(page, PAGE_ID); + expect(savedFilters.saved).toBe(false); + }); + + // #5294 — clearing every filter is a deliberate choice and has to be saved as such, so + // that a page's `defaultColumnFilters` are not re-applied on the next visit. + test('should save cleared filters as an empty state that survives a reload', async ({ page }) => { + const lp = productList(page); + await lp.goto(); + await lp.expectLoaded(); + + await resetSavedTableSettings(page, PAGE_ID); + await page.reload(); + await lp.expectLoaded(); + await lp.expectRowsLoaded(); + + const initialCount = await lp.getRows().count(); + + await applyNameFilter(page, 'Camera'); + const filteredCount = await lp.getRows().count(); + expect(filteredCount).toBeLessThan(initialCount); + + const afterFiltering = await readSavedColumnFilters(page, PAGE_ID); + expect(afterFiltering.saved).toBe(true); + expect(afterFiltering.value).toHaveLength(1); + + await Promise.all([ + waitForProductList(page), + page.getByRole('button', { name: 'Clear all' }).click(), + ]); + await lp.expectRowCount(initialCount); + + // Saved, but empty: the user has configured the filters and chosen to have none. + const afterClearing = await readSavedColumnFilters(page, PAGE_ID); + expect(afterClearing.saved).toBe(true); + expect(afterClearing.value).toEqual([]); + + await page.reload(); + await lp.expectLoaded(); + await lp.expectRowsLoaded(); + await lp.expectRowCount(initialCount); + + const afterReload = await readSavedColumnFilters(page, PAGE_ID); + expect(afterReload.saved).toBe(true); + expect(afterReload.value).toEqual([]); + }); +}); diff --git a/packages/dashboard/src/lib/components/data-table/data-table-column-filters.spec.ts b/packages/dashboard/src/lib/components/data-table/data-table-column-filters.spec.ts new file mode 100644 index 00000000000..adefa897f66 --- /dev/null +++ b/packages/dashboard/src/lib/components/data-table/data-table-column-filters.spec.ts @@ -0,0 +1,32 @@ +import { ColumnFiltersState } from '@tanstack/react-table'; +import { describe, expect, it } from 'vitest'; + +import { columnFiltersEqual } from './data-table-column-filters.js'; + +describe('columnFiltersEqual', () => { + const filters: ColumnFiltersState = [{ id: 'isArchived', value: { eq: false } }]; + + it('treats a re-rendered copy of the same filters as unchanged', () => { + // The DataTable reports a filter change by persisting it, so an initial render — or any + // re-render that hands over an equivalent array — must not look like a user edit. + expect(columnFiltersEqual(filters, [{ id: 'isArchived', value: { eq: false } }])).toBe(true); + }); + + it('treats two empty filter states as unchanged', () => { + expect(columnFiltersEqual([], [])).toBe(true); + }); + + it('treats clearing all filters as a change', () => { + // The counterpart to the above: going from some filters to none is a deliberate user + // action and must be reported, so that it can be persisted as "cleared". + expect(columnFiltersEqual(filters, [])).toBe(false); + }); + + it('treats a changed filter value as a change', () => { + expect(columnFiltersEqual(filters, [{ id: 'isArchived', value: { eq: true } }])).toBe(false); + }); + + it('treats an added filter as a change', () => { + expect(columnFiltersEqual([], filters)).toBe(false); + }); +}); diff --git a/packages/dashboard/src/lib/components/data-table/data-table-column-filters.ts b/packages/dashboard/src/lib/components/data-table/data-table-column-filters.ts new file mode 100644 index 00000000000..5bc49c9ddce --- /dev/null +++ b/packages/dashboard/src/lib/components/data-table/data-table-column-filters.ts @@ -0,0 +1,17 @@ +import type { ColumnFiltersState } from '@tanstack/react-table'; + +/** + * Compares two column filter states by value. The DataTable keeps its filters in React + * state, so a re-render can produce a new array with identical contents; only a change + * in content means the user actually edited the filters. + * + * Consumers persist the filters they are told about, so reporting a no-op change would + * write an empty filter state on mount and make "never configured filters on this page" + * indistinguishable from "cleared every filter" — see `ListPage`'s `defaultColumnFilters`. + * + * Deliberately not exported from the package barrel: this is an internal detail of the + * DataTable, not public API. + */ +export function columnFiltersEqual(a: ColumnFiltersState, b: ColumnFiltersState): boolean { + return JSON.stringify(a) === JSON.stringify(b); +} diff --git a/packages/dashboard/src/lib/components/data-table/data-table.tsx b/packages/dashboard/src/lib/components/data-table/data-table.tsx index 20fb31217ad..33df31b7a96 100644 --- a/packages/dashboard/src/lib/components/data-table/data-table.tsx +++ b/packages/dashboard/src/lib/components/data-table/data-table.tsx @@ -35,6 +35,7 @@ import React, { Suspense, useEffect, useMemo, useRef } from 'react'; import { AddFilterMenu } from './add-filter-menu.js'; import { ActiveFiltersPopover } from './data-table-active-filters-popover.js'; import { DataTableBulkActions, getRowItemId } from './data-table-bulk-actions.js'; +import { columnFiltersEqual } from './data-table-column-filters.js'; import { DataTableProvider } from './data-table-context.js'; import { createPaginationState, syncPaginationState } from './data-table-pagination-state.js'; import { @@ -434,20 +435,21 @@ export function DataTable({ }, [onPageChange, searchTerm]); useEffect(() => { + // `prevColumnFiltersRef` starts out holding the initial filter state, so this also + // covers the mount run: the filters the table was given are not a change the user + // made, and reporting them would persist them as if they were. + if (columnFiltersEqual(prevColumnFiltersRef.current, columnFilters)) { + return; + } + prevColumnFiltersRef.current = columnFilters; onFilterChange?.(tableRef.current, columnFilters); - if ( - page && - page > 1 && - itemsPerPage && - JSON.stringify(prevColumnFiltersRef.current) !== JSON.stringify(columnFilters) - ) { + if (page && page > 1 && itemsPerPage) { // Set the page back to 1 when filters change setPagination({ ...pagination, pageIndex: 0, }); } - prevColumnFiltersRef.current = columnFilters; }, [columnFilters]); const handleSearchChange = (value: string) => { diff --git a/packages/dashboard/src/lib/framework/page/list-page.spec.tsx b/packages/dashboard/src/lib/framework/page/list-page.spec.tsx index f898daea2f3..9de199106cf 100644 --- a/packages/dashboard/src/lib/framework/page/list-page.spec.tsx +++ b/packages/dashboard/src/lib/framework/page/list-page.spec.tsx @@ -6,6 +6,10 @@ import { ListPage } from './list-page.js'; // Capture the props ListPage forwards to PaginatedListDataTable. const captured: { props?: Record } = {}; +// Lets each test control the user settings ListPage reads its persisted table state from, +// and whether those settings have finished resolving. +const mocks = vi.hoisted(() => ({ settings: {} as Record, settingsReady: true })); + vi.mock('@/vdb/components/shared/paginated-list-data-table.js', () => ({ PaginatedListDataTable: (props: Record) => { captured.props = props; @@ -22,7 +26,11 @@ vi.mock('../layout-engine/page-layout.js', () => ({ })); vi.mock('@/vdb/hooks/use-user-settings.js', () => ({ - useUserSettings: () => ({ setTableSettings: vi.fn(), settings: {} }), + useUserSettings: () => ({ + setTableSettings: vi.fn(), + settings: mocks.settings, + settingsReady: mocks.settingsReady, + }), })); vi.mock('@tanstack/react-router', () => ({ @@ -38,6 +46,8 @@ describe('ListPage prop forwarding', () => { beforeEach(() => { captured.props = undefined; + mocks.settings = {}; + mocks.settingsReady = true; }); it('forwards transformQueryKey, disableViewOptions and includeSelectionColumn to PaginatedListDataTable', () => { @@ -60,3 +70,104 @@ describe('ListPage prop forwarding', () => { expect(captured.props?.includeSelectionColumn).toBe(false); }); }); + +describe('ListPage defaultColumnFilters', () => { + const defaultColumnFilters = [{ id: 'isArchived', value: { eq: false } }]; + + const baseProps = { + route: { useSearch: () => ({}), fullPath: '/' } as any, + title: 'Test', + listQuery: {} as any, + pageId: 'test-list', + defaultColumnFilters, + }; + + beforeEach(() => { + captured.props = undefined; + mocks.settings = {}; + mocks.settingsReady = true; + }); + + it('applies the defaults when the user has not configured filters for the page', () => { + renderToStaticMarkup(); + + expect(captured.props?.columnFilters).toEqual(defaultColumnFilters); + }); + + it('applies the defaults when the page has table settings but no saved filters', () => { + mocks.settings = { tableSettings: { 'test-list': { pageSize: 25 } } }; + + renderToStaticMarkup(); + + expect(captured.props?.columnFilters).toEqual(defaultColumnFilters); + }); + + it('prefers the filters the user has saved over the defaults', () => { + const userFilters = [{ id: 'name', value: { contains: 'shoe' } }]; + mocks.settings = { tableSettings: { 'test-list': { columnFilters: userFilters } } }; + + renderToStaticMarkup(); + + expect(captured.props?.columnFilters).toEqual(userFilters); + }); + + // An empty saved filter state means the user cleared every filter. That is a deliberate + // choice, so the defaults must not come back — this is what distinguishes it from the + // "never configured" state, which is the absence of a saved value. + it('does not re-apply the defaults once the user has cleared all filters', () => { + mocks.settings = { tableSettings: { 'test-list': { columnFilters: [] } } }; + + renderToStaticMarkup(); + + expect(captured.props?.columnFilters).toEqual([]); + }); + + it('ignores the defaults when no pageId is set, since there is nowhere to persist a change', () => { + renderToStaticMarkup(); + + expect(captured.props?.columnFilters).toBeUndefined(); + }); + + it('leaves pages without defaults unaffected', () => { + renderToStaticMarkup(); + + expect(captured.props?.columnFilters).toBeUndefined(); + }); + + // The data table seeds its filter state from `columnFilters` once, on mount, while the + // user settings resolve asynchronously. Mounting it before the settings have resolved + // would show the default filter chips over a list that is still being fetched unfiltered. + it('does not mount the data table until the user settings have resolved', () => { + mocks.settingsReady = false; + + renderToStaticMarkup(); + + expect(captured.props).toBeUndefined(); + }); + + it('mounts the data table once the user settings have resolved', () => { + mocks.settingsReady = true; + + renderToStaticMarkup(); + + expect(captured.props?.columnFilters).toEqual(defaultColumnFilters); + }); + + // The gate is scoped to the new feature, so pages that do not use it mount exactly as + // they did before, regardless of where the settings have got to. + it('still mounts the data table for pages with no defaults while settings are resolving', () => { + mocks.settingsReady = false; + + renderToStaticMarkup(); + + expect(captured.props).toBeDefined(); + }); + + it('still mounts the data table for pages with no pageId while settings are resolving', () => { + mocks.settingsReady = false; + + renderToStaticMarkup(); + + expect(captured.props).toBeDefined(); + }); +}); diff --git a/packages/dashboard/src/lib/framework/page/list-page.tsx b/packages/dashboard/src/lib/framework/page/list-page.tsx index 86aa7f2b1bf..282651933de 100644 --- a/packages/dashboard/src/lib/framework/page/list-page.tsx +++ b/packages/dashboard/src/lib/framework/page/list-page.tsx @@ -15,6 +15,7 @@ import { TypedDocumentNode } from '@graphql-typed-document-node/core'; import { AnyRoute, AnyRouter, useNavigate } from '@tanstack/react-router'; import { ColumnFiltersState, SortingState, Table } from '@tanstack/react-table'; import { TableOptions } from '@tanstack/table-core'; +import { useEffect } from 'react'; import { BulkActionsInput } from '@/vdb/framework/extension-api/types/index.js'; import { @@ -272,6 +273,33 @@ export interface ListPageProps< * ``` */ defaultSort?: SortingState; + /** + * @description + * Allows you to specify column filters which are applied when the current user has not + * yet configured any filters for this page. They behave exactly as if the user had set + * them: they show up as active filter chips, and can be edited or removed. + * + * Once the user edits the filters, their choice is persisted and these defaults are no + * longer applied — including when the user removes every filter. Clearing the filters is + * persisted as a deliberate choice, so the defaults do not reappear on the next visit. + * + * Requires `pageId` to be set, since the persisted table settings are what distinguishes + * "not configured yet" from "cleared by the user". Without a `pageId` the filters live + * only in the URL, which cannot express that difference, and the defaults are ignored. + * + * @example + * ```tsx + * + * ``` + * @since 3.8.0 + */ + defaultColumnFilters?: ColumnFiltersState; /** * @description * Allows you to specify the default columns that are visible in the table. @@ -536,6 +564,7 @@ export function ListPage< additionalColumns, defaultColumnOrder, defaultSort, + defaultColumnFilters, route: routeOrFn, defaultVisibility, onSearchTermChange, @@ -558,9 +587,22 @@ export function ListPage< const route = typeof routeOrFn === 'function' ? routeOrFn() : routeOrFn; const routeSearch = route.useSearch(); const navigate = useNavigate({ from: route.fullPath }); - const { setTableSettings, settings } = useUserSettings(); + const { setTableSettings, settings, settingsReady } = useUserSettings(); const tableSettings = pageId ? settings.tableSettings?.[pageId] : undefined; + // `defaultColumnFilters` is a silent no-op without a `pageId`, so say so in development + // rather than leaving the author to wonder why their defaults never show up. + useEffect(() => { + if (process.env.NODE_ENV !== 'production' && defaultColumnFilters && !pageId) { + // eslint-disable-next-line no-console + console.warn( + `ListPage: "defaultColumnFilters" was set without a "pageId", so it has no effect. ` + + `The persisted table settings are what tell "not configured yet" apart from ` + + `"cleared by the user", and there are none without a "pageId".`, + ); + } + }, [defaultColumnFilters, pageId]); + const pagination = { page: routeSearch.page ? Number.parseInt(routeSearch.page) : 1, itemsPerPage: routeSearch.perPage @@ -570,7 +612,22 @@ export function ListPage< // Column visibility/order user-settings merging is owned by useViewOptionDefaults inside // PaginatedListDataTable, so only raw code defaults are passed down here. - const columnFilters = pageId ? tableSettings?.columnFilters : routeSearch.filters; + // `defaultColumnFilters` only applies until the user has configured filters for this page. + // The saved table settings hold no `columnFilters` entry until the user edits the filters, + // and hold an empty array once the user has cleared them all, so an absent entry — rather + // than an empty one — is what marks the defaults as still applicable. + const columnFilters = pageId + ? (tableSettings?.columnFilters ?? defaultColumnFilters) + : routeSearch.filters; + + // The DataTable seeds its filter state from `columnFilters` once, on mount. The user + // settings resolve asynchronously — the local values are in effect until the server-side + // SettingsStore responds and replaces them — so a table mounted before that keeps filters + // that no longer match the persisted state. With `defaultColumnFilters` that becomes + // user-visible: the default chips would be on screen while the list query ran unfiltered. + // So hold the table back until the settings have resolved, but only on pages that use the + // feature, leaving every other list page mounting exactly as before. + const awaitingSettings = !!defaultColumnFilters && !!pageId && !settingsReady; const sorting: SortingState = (routeSearch.sort ?? '') .split(',') @@ -662,12 +719,14 @@ export function ListPage< {children} - + {awaitingSettings ? null : ( + + )} diff --git a/packages/dashboard/src/lib/providers/user-settings.spec.tsx b/packages/dashboard/src/lib/providers/user-settings.spec.tsx new file mode 100644 index 00000000000..5a945baf069 --- /dev/null +++ b/packages/dashboard/src/lib/providers/user-settings.spec.tsx @@ -0,0 +1,211 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import * as React from 'react'; +import { act } from 'react'; +import { createRoot, Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { LS_KEY_USER_SETTINGS } from '@/vdb/constants.js'; + +import { + migrateTableSettings, + TABLE_SETTINGS_VERSION, + UserSettings, + UserSettingsContext, + UserSettingsContextType, + UserSettingsProvider, +} from './user-settings.js'; + +const mocks = vi.hoisted(() => ({ + serverSettings: null as unknown, + saved: [] as unknown[], +})); + +vi.mock('../graphql/api.js', () => ({ + api: { + query: () => Promise.resolve({ getSettingsStoreValue: mocks.serverSettings }), + mutate: (_doc: unknown, variables: any) => { + mocks.saved.push(variables.input.value); + return Promise.resolve({ setSettingsStoreValue: { result: 'UPDATED' } }); + }, + }, +})); + +describe('migrateTableSettings', () => { + it('drops an empty saved columnFilters entry left behind by older versions', () => { + // Before this migration, merely visiting a list page saved `columnFilters: []` for it. + // Leaving that in place would make every existing user look like they had cleared + // their filters, and `defaultColumnFilters` would never apply to any of them. + const migrated = migrateTableSettings({ + tableSettings: { 'product-list': { columnFilters: [], pageSize: 25 } }, + } as UserSettings); + + expect(migrated.tableSettings?.['product-list']).toEqual({ pageSize: 25 }); + expect(migrated.tableSettings?.['product-list']).not.toHaveProperty('columnFilters'); + }); + + it('preserves a non-empty saved columnFilters entry', () => { + const columnFilters = [{ id: 'name', value: { contains: 'shoe' } }]; + + const migrated = migrateTableSettings({ + tableSettings: { 'product-list': { columnFilters } }, + } as UserSettings); + + expect(migrated.tableSettings?.['product-list'].columnFilters).toEqual(columnFilters); + }); + + it('leaves other table settings untouched', () => { + const migrated = migrateTableSettings({ + tableSettings: { + 'product-list': { columnFilters: [], columnOrder: ['name'] }, + 'order-list': { pageSize: 50 }, + }, + } as UserSettings); + + expect(migrated.tableSettings?.['product-list']).toEqual({ columnOrder: ['name'] }); + expect(migrated.tableSettings?.['order-list']).toEqual({ pageSize: 50 }); + }); + + it('stamps the settings so the migration is not repeated', () => { + const migrated = migrateTableSettings({ tableSettings: {} } as UserSettings); + + expect(migrated.tableSettingsVersion).toBe(TABLE_SETTINGS_VERSION); + }); + + it('migrates settings stamped below the current version', () => { + // Version 0 is the un-stamped shape, whether the stamp is absent or explicitly 0. + const migrated = migrateTableSettings({ + tableSettings: { 'product-list': { columnFilters: [], pageSize: 25 } }, + tableSettingsVersion: 0, + } as UserSettings); + + expect(migrated.tableSettings?.['product-list']).toEqual({ pageSize: 25 }); + expect(migrated.tableSettingsVersion).toBe(TABLE_SETTINGS_VERSION); + }); + + it('returns settings stamped above the current version untouched', () => { + // A copy written by a newer dashboard that this install has rolled back from. Running + // the migration would drop filters the newer version treats as a real clear, and stamp + // the version backwards; leaving it alone lets the newer version pick up where it left off. + const settings = { + tableSettings: { 'product-list': { columnFilters: [] } }, + tableSettingsVersion: TABLE_SETTINGS_VERSION + 1, + } as UserSettings; + + const migrated = migrateTableSettings(settings); + + expect(migrated).toBe(settings); + expect(migrated.tableSettingsVersion).toBe(TABLE_SETTINGS_VERSION + 1); + expect(migrated.tableSettings?.['product-list'].columnFilters).toEqual([]); + }); + + it('returns already-stamped settings untouched', () => { + // An empty `columnFilters` in settings that carry the stamp is a real user choice — + // the user cleared every filter after upgrading — and must survive. + const settings = { + tableSettings: { 'product-list': { columnFilters: [] } }, + tableSettingsVersion: TABLE_SETTINGS_VERSION, + } as UserSettings; + + const migrated = migrateTableSettings(settings); + + // Same reference: callers rely on this to tell whether anything needs writing back. + expect(migrated).toBe(settings); + expect(migrated.tableSettings?.['product-list'].columnFilters).toEqual([]); + }); +}); + +describe('UserSettingsProvider table settings migration', () => { + let container: HTMLDivElement; + let root: Root; + let latest: UserSettingsContextType | undefined; + + function Probe() { + latest = React.useContext(UserSettingsContext); + return null; + } + + async function renderProvider() { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + await act(async () => { + root.render( + + + + + , + ); + }); + // Let the settings query resolve and the resulting save effect run. + await act(async () => { + await new Promise(resolve => setTimeout(resolve, 0)); + }); + } + + beforeEach(() => { + (globalThis as any).IS_REACT_ACT_ENVIRONMENT = true; + localStorage.clear(); + mocks.serverSettings = null; + mocks.saved = []; + latest = undefined; + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + }); + + afterEach(() => { + act(() => root.unmount()); + container.remove(); + }); + + it('migrates settings loaded from localStorage', async () => { + localStorage.setItem( + LS_KEY_USER_SETTINGS, + JSON.stringify({ tableSettings: { 'product-list': { columnFilters: [], pageSize: 25 } } }), + ); + + await renderProvider(); + + expect(latest?.settings.tableSettings?.['product-list']).toEqual({ pageSize: 25 }); + }); + + it('migrates the settings payload that comes back from the server', async () => { + // The server copy replaces the local one wholesale once it arrives, so it needs the + // same migration — otherwise it would resurrect the empty entries just dropped locally. + localStorage.setItem( + LS_KEY_USER_SETTINGS, + JSON.stringify({ tableSettings: { 'product-list': { columnFilters: [] } } }), + ); + mocks.serverSettings = { + tableSettings: { 'product-list': { columnFilters: [], pageSize: 50 } }, + }; + + await renderProvider(); + + expect(latest?.settings.tableSettings?.['product-list']).toEqual({ pageSize: 50 }); + }); + + it('writes the migrated settings back to the server so the migration runs once', async () => { + mocks.serverSettings = { tableSettings: { 'product-list': { columnFilters: [] } } }; + + await renderProvider(); + + const lastSaved = mocks.saved.at(-1) as UserSettings | undefined; + expect(lastSaved).toBeDefined(); + expect(lastSaved?.tableSettingsVersion).toBe(TABLE_SETTINGS_VERSION); + expect(lastSaved?.tableSettings?.['product-list']).toEqual({}); + }); + + it('leaves already-migrated server settings alone', async () => { + mocks.serverSettings = { + tableSettings: { 'product-list': { columnFilters: [] } }, + tableSettingsVersion: TABLE_SETTINGS_VERSION, + }; + + await renderProvider(); + + // The user really did clear every filter, so the empty state survives... + expect(latest?.settings.tableSettings?.['product-list'].columnFilters).toEqual([]); + // ...and nothing needed writing back. + expect(mocks.saved).toHaveLength(0); + }); +}); diff --git a/packages/dashboard/src/lib/providers/user-settings.tsx b/packages/dashboard/src/lib/providers/user-settings.tsx index dc6a973284d..042ccc95e71 100644 --- a/packages/dashboard/src/lib/providers/user-settings.tsx +++ b/packages/dashboard/src/lib/providers/user-settings.tsx @@ -56,6 +56,72 @@ export interface UserSettings { * newly registered widgets are visible by default with no migration needed. */ hiddenWidgets?: string[]; + /** + * @description + * The version of the persisted `tableSettings` shape, written by + * {@link migrateTableSettings}. Absent in settings saved before that migration existed. + */ + tableSettingsVersion?: number; +} + +/** + * The current version of the persisted `tableSettings` shape. Bump this when a change to + * how table settings are written means existing saved settings need migrating on load, and + * add a correspondingly-gated step to {@link migrateTableSettings}. Bumping never re-runs + * the steps that came before: a stored copy only ever runs the steps above its own stamp. + */ +export const TABLE_SETTINGS_VERSION = 1; + +/** + * Version 1: drops saved `columnFilters` entries that are empty. See + * {@link migrateTableSettings} for why. + */ +function dropEmptyColumnFilters(settings: UserSettings): UserSettings { + const tableSettings = Object.fromEntries( + Object.entries(settings.tableSettings ?? {}).map(([tableId, table]) => { + if (Array.isArray(table?.columnFilters) && table.columnFilters.length === 0) { + const { columnFilters, ...rest } = table; + return [tableId, rest]; + } + return [tableId, table]; + }), + ); + return { ...settings, tableSettings }; +} + +/** + * @description + * Brings persisted table settings written by an earlier dashboard version up to + * `TABLE_SETTINGS_VERSION`, and stamps them so it only ever runs once per stored copy. + * + * Version 1 drops saved `columnFilters` entries that are empty. Until this version, the + * DataTable reported its filter state on mount, so merely visiting a list page saved + * `columnFilters: []` for it. That makes "has never configured filters here" and "has + * cleared every filter" the same stored value, and `ListPage`'s `defaultColumnFilters` + * relies on telling those apart — without this, the defaults would never apply for anyone + * who had visited the page before upgrading, which is every existing user. + * + * A legacy auto-write is indistinguishable from a genuine clear, so this drops both. The + * cost is that a user who really had cleared every filter on a page that declares defaults + * sees those defaults once more, and clears them once more; that is much less bad than + * defaults that silently never apply. + * + * Each version is a separately gated step, run only against settings stamped below it, so a + * later bump does not re-run version 1's drop over an empty `columnFilters` that by then is + * a genuine user clear. Settings stamped at or above the current version — including a copy + * written by a newer dashboard the install has since rolled back from — are returned + * unchanged, by reference, so callers can tell whether anything needs writing back. + */ +export function migrateTableSettings(settings: UserSettings): UserSettings { + const fromVersion = settings.tableSettingsVersion ?? 0; + if (fromVersion >= TABLE_SETTINGS_VERSION) { + return settings; + } + let migrated = settings; + if (fromVersion < 1) { + migrated = dropEmptyColumnFilters(migrated); + } + return { ...migrated, tableSettingsVersion: TABLE_SETTINGS_VERSION }; } const defaultSettings: UserSettings = { @@ -69,6 +135,9 @@ const defaultSettings: UserSettings = { devMode: false, hasSeenOnboarding: false, tableSettings: {}, + // No `tableSettingsVersion` here on purpose: these defaults are spread *under* the stored + // settings, so a version stamped here would make un-migrated stored settings look current. + // The stamp is added by `migrateTableSettings()`, which every load path runs through. }; export interface UserSettingsContextType { @@ -149,12 +218,12 @@ export const UserSettingsProvider: React.FC = ({ quer try { const storedSettings = localStorage.getItem(LS_KEY_USER_SETTINGS); if (storedSettings) { - return { ...defaultSettings, ...JSON.parse(storedSettings) }; + return migrateTableSettings({ ...defaultSettings, ...JSON.parse(storedSettings) }); } } catch (e) { console.error('Failed to load user settings from localStorage', e); } - return { ...defaultSettings }; + return migrateTableSettings({ ...defaultSettings }); }; const [settings, setSettings] = useState(loadSettings); @@ -217,7 +286,16 @@ export const UserSettingsProvider: React.FC = ({ quer if (serverSettingsData) { // Server has settings, use them const mergedSettings = { ...defaultSettings, ...serverSettingsData }; - setSettings(mergedSettings); + // The server copy was written by whichever dashboard version saved it last, + // so it needs the same migration as the local copy. Without this, a server + // payload in the old shape would land on top of the already-migrated local + // settings and resurrect the empty `columnFilters` entries just dropped. + const migratedSettings = migrateTableSettings(mergedSettings); + setSettings(migratedSettings); + // `serverSettings` mirrors what the server actually holds. When the migration + // changed something, keeping the pre-migration value here makes the save + // effect below push the migrated settings — and the version stamp — back up, + // so the migration runs once rather than on every session. setServerSettings(mergedSettings); setIsReady(true); } else {