diff --git a/docs/docs/reference/dashboard/list-views/data-table.mdx b/docs/docs/reference/dashboard/list-views/data-table.mdx index 26d4d43150..d1277e0f2e 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 78736a39ed..0c1b63fe94 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 0000000000..194dabb6cf --- /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 0000000000..adefa897f6 --- /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 0000000000..5bc49c9ddc --- /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 20fb31217a..33df31b7a9 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 f898daea2f..9de199106c 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 86aa7f2b1b..282651933d 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 0000000000..5a945baf06 --- /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 dc6a973284..042ccc95e7 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 {