diff --git a/src/__tests__/CompareResults/ResultsTable.test.tsx b/src/__tests__/CompareResults/ResultsTable.test.tsx index bfd8e35bd..18605c806 100644 --- a/src/__tests__/CompareResults/ResultsTable.test.tsx +++ b/src/__tests__/CompareResults/ResultsTable.test.tsx @@ -1365,3 +1365,121 @@ describe('Advanced-columns toggle for mann-whitney-u testVersion', () => { expect(advancedParam()).toBeNull(); }); }); + +describe('cookie persistence vs. shareable URLs', () => { + it('seeds filters from cookies and marks the URL initialized on a fresh URL', async () => { + document.cookie = 'perfcompare_filter_status=regression; path=/'; + const { testCompareData } = getTestData(); + setupAndRender(testCompareData, 'test_version=student-t'); + + await screen.findByText('a11yr'); + + // The remembered cookie is applied to the view... + expect(summarizeVisibleRows()).toEqual([ + 'a11yr dhtml.html spam opt e10s fission stylo webrender', + ' - Linux 18.04, Regression, 1.85 %, Medium', + ]); + // ...and materialised into the URL, which is now marked initialized so the + // link reproduces this exact view for anyone. + expect(summarizeTableFiltersFromUrl()).toEqual({ status: ['regression'] }); + expect(new URLSearchParams(window.location.search).get('initialized')).toBe( + '1', + ); + }); + + it('ignores cookies when the URL is already initialized', async () => { + // A different viewer's cookie must not change what an initialized (shared) + // URL displays. + document.cookie = 'perfcompare_filter_status=regression; path=/'; + const { testCompareData } = getTestData(); + setupAndRender(testCompareData, 'test_version=student-t&initialized=1'); + + await screen.findByText('a11yr'); + + // Cookie is ignored: every status stays visible. + expect(summarizeVisibleRows()).toEqual([ + 'a11yr dhtml.html spam opt e10s fission stylo webrender', + ' - Linux 18.04, Regression, 1.85 %, Medium', + ' - macOS 10.15, Improvement, 1.08 %, Low', + ' - Windows 10, -, -24 %, -', + ' - Windows 10, -, -2.4 %, High', + ]); + // ...and the cookie is not written into the URL. + expect(summarizeTableFiltersFromUrl()).toEqual({}); + }); + + it('keeps the initialized marker after toggling a filter', async () => { + const { testCompareData } = getTestData(); + setupAndRender(testCompareData, 'test_version=student-t'); + + await screen.findByText('a11yr'); + + const user = userEvent.setup({ + advanceTimers: jest.advanceTimersByTime, + }); + await clickMenuItem(user, 'Status', /No changes/); + + expect(summarizeTableFiltersFromUrl()).toEqual({ + status: ['improvement', 'regression'], + }); + expect(new URLSearchParams(window.location.search).get('initialized')).toBe( + '1', + ); + }); + + it('keeps the initialized marker and seeded filters after a search-term change', async () => { + document.cookie = 'perfcompare_filter_status=regression; path=/'; + const { testCompareData } = getTestData(); + setupAndRender(testCompareData, 'test_version=student-t'); + + await screen.findByText('a11yr'); + + // Seeded from the cookie and marked initialized. + expect(summarizeTableFiltersFromUrl()).toEqual({ status: ['regression'] }); + expect(new URLSearchParams(window.location.search).get('initialized')).toBe( + '1', + ); + + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + // Submit with Enter so the write happens immediately (bypasses the input's + // debounce, which fake timers don't flush after typing). + await user.type( + screen.getByPlaceholderText('Filter results'), + 'linux{Enter}', + ); + + // The search term is written, and the out-of-band params survive. + const params = new URLSearchParams(window.location.search); + expect(params.get('search')).toBe('linux'); + expect(params.get('initialized')).toBe('1'); + expect(params.get('filter_status')).toBe('regression'); + }); + + it('keeps the initialized marker and seeded filters after a test-version change', async () => { + document.cookie = 'perfcompare_filter_status=regression; path=/'; + const { testCompareData } = getTestData(); + setupAndRender(testCompareData, 'test_version=student-t'); + + await screen.findByText('a11yr'); + expect(new URLSearchParams(window.location.search).get('initialized')).toBe( + '1', + ); + + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + await user.click( + screen.getByRole('combobox', { name: 'Stats Test Version' }), + ); + await user.click( + await screen.findByRole('option', { name: 'Mann-Whitney-U' }), + ); + + // The test version changes (a router navigation), and the marker + seeded + // filter ride along instead of being dropped. + await waitFor(() => { + const params = new URLSearchParams(window.location.search); + expect(params.get('test_version')).toBe('mann-whitney-u'); + expect(params.get('initialized')).toBe('1'); + expect(params.get('filter_status')).toBe('regression'); + }); + }); +}); diff --git a/src/__tests__/CompareResults/__snapshots__/SubtestsResultsView.test.tsx.snap b/src/__tests__/CompareResults/__snapshots__/SubtestsResultsView.test.tsx.snap index 7ba930a5c..302421820 100644 --- a/src/__tests__/CompareResults/__snapshots__/SubtestsResultsView.test.tsx.snap +++ b/src/__tests__/CompareResults/__snapshots__/SubtestsResultsView.test.tsx.snap @@ -55,7 +55,7 @@ exports[`SubtestsResultsView Component Tests for mann-whitney-u testVersion tabl style="width: 2.8284271247461903px; height: 2.8284271247461903px; top: -1.4142135623730951px; left: -1.4142135623730951px;" > state.theme.mode); const advancedColumns = useAdvancedColumns(); - const [rawSearchParams, updateRawSearchParams] = useRawSearchParams(); + const [, updateRawSearchParams] = useRawSearchParams(); // Track the Select's open state so the tooltip can be suppressed while the // dropdown is open — otherwise it renders over (and hides) the checkboxes. const [menuOpen, setMenuOpen] = useState(false); @@ -54,7 +55,9 @@ function AdvancedColumnsMenu() { dispatch(updateShowCles(next.cles)); dispatch(updateShowSignificance(next.significance)); - const params = new URLSearchParams(rawSearchParams); + // Build from the live URL so toggling columns doesn't drop params written + // out-of-band (e.g. the `initialized` marker and cookie-seeded filter/sort). + const params = currentUrlParams(); const value = serializeAdvancedColumns(next); if (value) { params.set(ADVANCED_COLUMNS_PARAM, value); diff --git a/src/components/CompareResults/ResultsMain.tsx b/src/components/CompareResults/ResultsMain.tsx index 95a179b63..b58d584fc 100644 --- a/src/components/CompareResults/ResultsMain.tsx +++ b/src/components/CompareResults/ResultsMain.tsx @@ -23,6 +23,7 @@ import { Colors, FontsRaw, FontSizeRaw, Spacing } from '../../styles'; import pencilDark from '../../theme/img/pencil-dark.svg'; import pencil from '../../theme/img/pencil.svg'; import type { TestVersion } from '../../types/types'; +import { currentUrlParams } from '../../utils/tableStatePersistence'; import EditTitleInput from '../CompareResults/EditTitleInput'; import ToggleReplicatesButton from '../Shared/ToggleReplicatesButton'; @@ -116,8 +117,11 @@ function ResultsMain() { }; const onSaveButtonClick = () => { - rawSearchParams.set('title', comparisonTitleName); - updateRawSearchParams(rawSearchParams); + // Build from the live URL so we don't drop params written out-of-band + // (e.g. the `initialized` marker and cookie-seeded filter/sort). + const params = currentUrlParams(); + params.set('title', comparisonTitleName); + updateRawSearchParams(params); showEditComparisonTitleInput(false); }; diff --git a/src/components/CompareResults/ResultsTable.tsx b/src/components/CompareResults/ResultsTable.tsx index 6e09acf78..bacc612c4 100644 --- a/src/components/CompareResults/ResultsTable.tsx +++ b/src/components/CompareResults/ResultsTable.tsx @@ -11,6 +11,7 @@ import TableContent from './TableContent'; import TableHeader from './TableHeader'; import { MANN_WHITNEY_U } from '../../common/constants'; import useAdvancedColumns from '../../hooks/useAdvancedColumns'; +import useInitializeTableStateFromCookies from '../../hooks/useInitializeTableStateFromCookies'; import useRawSearchParams from '../../hooks/useRawSearchParams'; import useSeedAdvancedColumnsFromUrl from '../../hooks/useSeedAdvancedColumnsFromUrl'; import useTableFilters from '../../hooks/useTableFilters'; @@ -20,6 +21,7 @@ import { getColumnsConfiguration, toGridTemplateColumns, } from '../../utils/rowTemplateColumns'; +import { currentUrlParams } from '../../utils/tableStatePersistence'; type CombinedLoaderReturnValue = LoaderReturnValue | OverTimeLoaderReturnValue; export default function ResultsTable() { @@ -32,7 +34,7 @@ export default function ResultsTable() { testVersion, } = useLoaderData(); - const [searchParams, setSearchParams] = useSearchParams(); + const [, setSearchParams] = useSearchParams(); // This is our custom hook that updates the search params without a rerender. const [rawSearchParams, updateRawSearchParams] = useRawSearchParams(); @@ -50,6 +52,10 @@ export default function ResultsTable() { [testVersion, advancedColumns], ); + // On a fresh (uninitialized) URL, seed filter/sort from cookies into the URL + // and mark it initialized, so shared links reproduce the same view. + useInitializeTableStateFromCookies(columnsConfig); + // This is our custom hook that manages table filters // and provides methods for clearing and toggling them. const { tableFilters, onClearFilter, onToggleFilter } = @@ -65,29 +71,36 @@ export default function ResultsTable() { ); const [expandAll, setExpandAll] = useState(false); + // These writers build from the *live* URL (currentUrlParams) rather than a + // render-time snapshot, so they preserve params written out-of-band — most + // importantly the `initialized` marker and cookie-seeded filter/sort — that a + // stale snapshot would drop (see useRawSearchParams / tableStatePersistence). const onFrameworkChange = (newFrameworkId: Framework['id']) => { setFrameworkIdVal(newFrameworkId); - searchParams.set('framework', newFrameworkId.toString()); - setSearchParams(searchParams); + const params = currentUrlParams(); + params.set('framework', newFrameworkId.toString()); + setSearchParams(params); }; const onSearchTermChange = (newSearchTerm: string) => { setSearchTerm(newSearchTerm); + const params = currentUrlParams(); if (newSearchTerm) { - rawSearchParams.set('search', newSearchTerm); + params.set('search', newSearchTerm); } else { - rawSearchParams.delete('search'); + params.delete('search'); } - updateRawSearchParams(rawSearchParams); + updateRawSearchParams(params); }; const onTestVersionChange = (testVersion: TestVersion): void => { setTestVersionVal(testVersion); - searchParams.set('test_version', testVersion); + const params = currentUrlParams(); + params.set('test_version', testVersion); if (testVersion !== MANN_WHITNEY_U) { - searchParams.delete('replicates'); + params.delete('replicates'); } - setSearchParams(searchParams); + setSearchParams(params); }; const rowGridTemplateColumns = toGridTemplateColumns(columnsConfig); diff --git a/src/components/CompareResults/SubtestsResults/SubtestsResultsTable.tsx b/src/components/CompareResults/SubtestsResults/SubtestsResultsTable.tsx index 89c31337d..b2c9e303c 100644 --- a/src/components/CompareResults/SubtestsResults/SubtestsResultsTable.tsx +++ b/src/components/CompareResults/SubtestsResults/SubtestsResultsTable.tsx @@ -9,6 +9,7 @@ import NoResultsFound from '.././NoResultsFound'; import TableHeader from '.././TableHeader'; import { STUDENT_T } from '../../../common/constants'; import useAdvancedColumns from '../../../hooks/useAdvancedColumns'; +import useInitializeTableStateFromCookies from '../../../hooks/useInitializeTableStateFromCookies'; import useSeedAdvancedColumnsFromUrl from '../../../hooks/useSeedAdvancedColumnsFromUrl'; import useTableFilters, { filterResults } from '../../../hooks/useTableFilters'; import useTableSort, { sortResults } from '../../../hooks/useTableSort'; @@ -88,6 +89,11 @@ function SubtestsResultsTable({ getColumnsConfiguration(true, testVersion ?? STUDENT_T, advancedColumns), [testVersion, advancedColumns], ); + + // On a fresh (uninitialized) URL, seed filter/sort from cookies into the URL + // and mark it initialized, so shared links reproduce the same view. + useInitializeTableStateFromCookies(columnsConfiguration); + // This is our custom hook that manages table filters // and provides methods for clearing and toggling them. const { tableFilters, onClearFilter, onToggleFilter } = diff --git a/src/hooks/useInitializeTableStateFromCookies.ts b/src/hooks/useInitializeTableStateFromCookies.ts new file mode 100644 index 000000000..43b3b9dd0 --- /dev/null +++ b/src/hooks/useInitializeTableStateFromCookies.ts @@ -0,0 +1,74 @@ +import { useEffect } from 'react'; + +import useRawSearchParams from './useRawSearchParams'; +import type { + CompareResultsTableConfig, + CompareMannWhitneyResultsTableConfig, +} from '../types/types'; +import { getCookie } from '../utils/cookies'; +import { + INITIALIZED_PARAM, + SORT_PARAM, + SORT_COOKIE, + filterParam, + filterCookie, + isTableStateInitialized, + currentUrlParams, +} from '../utils/tableStatePersistence'; + +// On the first load of an *uninitialized* results URL (e.g. arriving from the +// search form), copy the remembered filter/sort cookies into the URL and stamp +// it as initialized. From then on the URL fully describes the view, so sharing +// it reproduces the same result for everyone — the recipient's cookies are +// ignored because the URL is initialized (see useTableFilters/useTableSort). +// +// This runs exactly once and only mutates the URL via history.replaceState +// (through useRawSearchParams), so it never triggers a re-render or a loader +// refetch. It intentionally has no effect on an already-initialized URL. +const useInitializeTableStateFromCookies = ( + columnsConfiguration: + | CompareResultsTableConfig + | CompareMannWhitneyResultsTableConfig, +) => { + const [, updateRawSearchParams] = useRawSearchParams(); + + useEffect(() => { + if (isTableStateInitialized(window.location.search)) { + return; + } + + const params = currentUrlParams(); + + // Only seed a value from a cookie when the URL doesn't already specify it, + // so an explicit URL param always wins over the cookie. + for (const column of columnsConfiguration) { + if (!('filter' in column)) { + continue; + } + const param = filterParam(column.key); + if (!params.has(param)) { + const cookieValue = getCookie(filterCookie(column.key)); + if (cookieValue) { + params.set(param, cookieValue); + } + } + } + + if (!params.has(SORT_PARAM)) { + const sortCookie = getCookie(SORT_COOKIE); + if (sortCookie) { + params.set(SORT_PARAM, sortCookie); + } + } + + // Stamp the marker even when there were no cookies, so a filter-free view + // is still "initialized" and can't pick up cookies on this or another + // browser later. + params.set(INITIALIZED_PARAM, '1'); + updateRawSearchParams(params); + // Mount-only: the URL is materialised once and the data hooks have already + // seeded their state from the same cookies during the first render. + }, []); +}; + +export default useInitializeTableStateFromCookies; diff --git a/src/hooks/useTableFilters.ts b/src/hooks/useTableFilters.ts index eb105ccf8..07c3c18e5 100644 --- a/src/hooks/useTableFilters.ts +++ b/src/hooks/useTableFilters.ts @@ -12,6 +12,12 @@ import type { CompareMannWhitneyResultsTableColumn, } from '../types/types'; import { getCookie, setCookie, deleteCookie } from '../utils/cookies'; +import { + filterParam, + filterCookie, + isTableStateInitialized, + currentUrlParams, +} from '../utils/tableStatePersistence'; // This hook handles the state that handles table filtering, and also takes care // of handling the URL parameters that mirror this state. @@ -65,6 +71,10 @@ const useTableFilters = ( // This function collects the table filters from the search params. It will // only be called once at mount time. + // Cookies are only consulted for an uninitialized URL; an initialized URL is + // the single source of truth so a shared link reproduces the same view. + const initialized = isTableStateInitialized(window.location.search); + const getInitialTableFilters = () => { const result: Map> = new Map(); for (const columnConfiguration of columnsConfiguration) { @@ -75,8 +85,8 @@ const useTableFilters = ( const { key: columnKey, possibleValues } = columnConfiguration; const paramValue = - rawSearchParams.get('filter_' + columnKey) ?? - getCookie('perfcompare_filter_' + columnKey); + rawSearchParams.get(filterParam(columnKey)) ?? + (initialized ? null : getCookie(filterCookie(columnKey))); if (paramValue) { const configuredValuesSet = new Set( paramValue.split(',').map((item) => item.trim()), @@ -101,9 +111,10 @@ const useTableFilters = ( const [tableFilters, setTableFilters] = useState(getInitialTableFilters); const onClearFilter = (columnId: string) => { - rawSearchParams.delete(`filter_${columnId}`); - updateRawSearchParams(rawSearchParams); - deleteCookie(`perfcompare_filter_${columnId}`); + const params = currentUrlParams(); + params.delete(filterParam(columnId)); + updateRawSearchParams(params); + deleteCookie(filterCookie(columnId)); setTableFilters((oldFilters) => { const newFilters = new Map(oldFilters); @@ -123,14 +134,15 @@ const useTableFilters = ( return; } + const params = currentUrlParams(); if (filters.size < columnConfiguration.possibleValues.length) { - rawSearchParams.set(`filter_${columnId}`, [...filters].join(',')); - setCookie(`perfcompare_filter_${columnId}`, [...filters].join(',')); + params.set(filterParam(columnId), [...filters].join(',')); + setCookie(filterCookie(columnId), [...filters].join(',')); } else { - rawSearchParams.delete(`filter_${columnId}`); - deleteCookie(`perfcompare_filter_${columnId}`); + params.delete(filterParam(columnId)); + deleteCookie(filterCookie(columnId)); } - updateRawSearchParams(rawSearchParams); + updateRawSearchParams(params); setTableFilters((oldFilters) => { const newFilters = new Map(oldFilters); diff --git a/src/hooks/useTableSort.ts b/src/hooks/useTableSort.ts index 7c8b21362..060515ec0 100644 --- a/src/hooks/useTableSort.ts +++ b/src/hooks/useTableSort.ts @@ -8,6 +8,12 @@ import type { SortFunc, } from '../types/types'; import { getCookie, setCookie, deleteCookie } from '../utils/cookies'; +import { + SORT_PARAM, + SORT_COOKIE, + isTableStateInitialized, + currentUrlParams, +} from '../utils/tableStatePersistence'; // This hook handles the state that handles table sorting, and also takes care // of handling the URL parameters that mirror this state. @@ -25,8 +31,13 @@ const useTableSort = ( // This is our custom hook that updates the search params without a rerender. const [rawSearchParams, updateRawSearchParams] = useRawSearchParams(); + // Cookies are only consulted for an uninitialized URL; an initialized URL is + // the single source of truth so a shared link reproduces the same view. + const initialized = isTableStateInitialized(window.location.search); const sortFromUrl = - rawSearchParams.get('sort') ?? getCookie('perfcompare_sort') ?? ''; + rawSearchParams.get(SORT_PARAM) ?? + (initialized ? null : getCookie(SORT_COOKIE)) ?? + ''; const [columnId, direction] = useMemo(() => { const [columnId, direction] = sortFromUrl.split('|'); if (!columnId) { @@ -57,18 +68,19 @@ const useTableSort = ( columnId: string, newSortDirection: 'asc' | 'desc' | null, ) => { + const params = currentUrlParams(); if (newSortDirection === null) { setSortColumn(null); setSortDirection(null); - rawSearchParams.delete('sort'); - deleteCookie('perfcompare_sort'); + params.delete(SORT_PARAM); + deleteCookie(SORT_COOKIE); } else { setSortColumn(columnId); setSortDirection(newSortDirection); - rawSearchParams.set('sort', columnId + '|' + newSortDirection); - setCookie('perfcompare_sort', columnId + '|' + newSortDirection); + params.set(SORT_PARAM, columnId + '|' + newSortDirection); + setCookie(SORT_COOKIE, columnId + '|' + newSortDirection); } - updateRawSearchParams(rawSearchParams); + updateRawSearchParams(params); }; return { sortDirection, sortColumn, onToggleSort }; diff --git a/src/utils/tableStatePersistence.ts b/src/utils/tableStatePersistence.ts new file mode 100644 index 000000000..f577bd5e5 --- /dev/null +++ b/src/utils/tableStatePersistence.ts @@ -0,0 +1,39 @@ +// Central definitions for how the results table's filter/sort state is +// persisted, so the query-param and cookie keys live in exactly one place. +// +// The state lives in two layers: +// * the URL (`filter_`, `sort`) — the shareable source of truth; +// * cookies (`perfcompare_filter_`, `perfcompare_sort`) — a per-browser +// memory of the last-used values. +// +// A URL is considered "initialized" once the app has materialised the table +// state into it (marker below). Cookies are only ever *read* for an +// *uninitialized* URL; an initialized URL is the single source of truth, so a +// shared link reproduces the same view for everyone regardless of their +// cookies. Cookies are still *written* on every change, so the memory survives +// for the next fresh visit. + +// Marker that flags a URL as "initialized" (see above). +export const INITIALIZED_PARAM = 'initialized'; + +// URL query-parameter keys. +export const SORT_PARAM = 'sort'; +export const filterParam = (columnKey: string) => `filter_${columnKey}`; + +// Cookie keys. +export const SORT_COOKIE = 'perfcompare_sort'; +export const filterCookie = (columnKey: string) => + `perfcompare_filter_${columnKey}`; + +// Whether the URL already carries the initialized marker. +export function isTableStateInitialized(search: string): boolean { + return new URLSearchParams(search).has(INITIALIZED_PARAM); +} + +// Read the *live* URL params. Writes must start from this rather than a +// memoized snapshot, otherwise params added out-of-band (e.g. the initialized +// marker, seeded once on mount) would be clobbered by a later filter/sort +// change. +export function currentUrlParams(): URLSearchParams { + return new URLSearchParams(window.location.search); +}