Skip to content

Commit adfc258

Browse files
dhasilvaclaude
dhasilva
and
claude
authored
Backup: Reset the Overview's remembered state before every test (#53816)
* Backup: Reset the Overview's remembered state before every test The Overview keeps the reader's place in the list and its page-view latch in module state, so they outlive an unmount. Every test file had to remember to reset them, and seven that render the Overview did not. Move that state to screens/overview-state.ts, which has no runtime imports, and reset it from tests/jest.setup.js before every test. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * changelog Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent f142698 commit adfc258

12 files changed

Lines changed: 49 additions & 65 deletions

‎projects/packages/backup/changelog/backup-overview-resets‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Significance: patch
2+
Type: changed
3+
Comment: Test-only change: reset the Overview's remembered state before every test.
4+
5+

‎projects/packages/backup/src/dashboard/screens/overview-state.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import type { View } from '@wordpress/dataviews';
2+
3+
/**
4+
* What the Overview remembers across an unmount, which a trip to Download or Restore and a
5+
* gate verdict change both cause.
6+
*
7+
* Kept out of `overview.tsx` so the Jest setup can reset it without loading the screen.
8+
*/
9+
export const overviewState: {
10+
/** Whether this page load has recorded its view; `overview.tsx` says why this is not a ref. */
11+
hasRecordedPageView: boolean;
12+
/** Where the reader last was in the list, or null for the screen's initial view. */
13+
lastView: View | null;
14+
/** The row the reader last chose. */
15+
lastSelectedId: string | null;
16+
} = {
17+
hasRecordedPageView: false,
18+
lastView: null,
19+
lastSelectedId: null,
20+
};
21+
22+
/**
23+
* Forget the visit, as a new page load would. Test-only.
24+
*/
25+
export function resetOverviewStateForTesting(): void {
26+
overviewState.hasRecordedPageView = false;
27+
overviewState.lastView = null;
28+
overviewState.lastSelectedId = null;
29+
}

‎projects/packages/backup/src/dashboard/screens/overview.tsx‎

Lines changed: 7 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { useBackupRequested } from '../hooks/use-enqueue-backup';
3131
import { useRefreshActivityOnBackupComplete } from '../hooks/use-refresh-activity-on-backup-complete';
3232
import { useRevealDetail } from '../hooks/use-reveal-detail';
3333
import { isBackupItem } from '../types/activity';
34+
import { overviewState } from './overview-state';
3435
import type { ActivitySortOrder } from '../data/api/activity-log';
3536
import type { View } from '@wordpress/dataviews';
3637

@@ -59,45 +60,6 @@ export const INITIAL_VIEW: View = {
5960
fields: [],
6061
};
6162

62-
/**
63-
* Whether this page load has already recorded its view.
64-
*
65-
* See the effect below for why this is module state rather than a ref.
66-
*/
67-
let hasRecordedPageView = false;
68-
69-
/**
70-
* Where the reader last was in the list, and which row they last chose.
71-
*
72-
* Module state for the same reason `hasRecordedPageView` is: a trip to Download or
73-
* Restore unmounts this screen, and a gate verdict change unmounts its body — so
74-
* anything held in the component is gone by the time the reader comes back.
75-
*/
76-
let lastView: View = INITIAL_VIEW;
77-
let lastSelectedId: string | null = null;
78-
79-
/**
80-
* Reset the page-view latch. Test-only.
81-
*
82-
* The latch is module state precisely so it outlives an unmount, which
83-
* also means one test's render would otherwise silence every later one
84-
* in the same file.
85-
*/
86-
export function resetPageViewForTesting(): void {
87-
hasRecordedPageView = false;
88-
}
89-
90-
/**
91-
* Forget where the reader was. Test-only.
92-
*
93-
* Same hazard as the latch above: this state outlives an unmount, so one test's
94-
* paging would otherwise seed every later one in the same file.
95-
*/
96-
export function resetListStateForTesting(): void {
97-
lastView = INITIAL_VIEW;
98-
lastSelectedId = null;
99-
}
100-
10163
/**
10264
* Overview screen for the modernized Backup dashboard.
10365
*
@@ -128,11 +90,11 @@ export default function OverviewScreen() {
12890
// second view for the same visit, which is the over-counting this
12991
// whole decision exists to avoid. Module scope also subsumes the
13092
// StrictMode double-invocation a ref was reaching for.
131-
if ( hasRecordedPageView ) {
93+
if ( overviewState.hasRecordedPageView ) {
13294
return;
13395
}
13496

135-
hasRecordedPageView = true;
97+
overviewState.hasRecordedPageView = true;
13698
tracks.recordEvent( 'jetpack_backup_admin_page_view' );
13799
}, [ tracks ] );
138100

@@ -160,11 +122,11 @@ function OverviewBody() {
160122
const navigate = useNavigate();
161123
// View state lives here so RightPane's `useActivityById` can
162124
// subscribe to the same paginated query the list reads from.
163-
const [ view, setView ] = useState< View >( lastView );
125+
const [ view, setView ] = useState< View >( overviewState.lastView ?? INITIAL_VIEW );
164126
// Written on the way through, because the return trip has nothing to read it from:
165127
// the back links on Download and Restore carry no search.
166128
const rememberView = useCallback( ( next: View ) => {
167-
lastView = next;
129+
overviewState.lastView = next;
168130
setView( next );
169131
}, [] );
170132
// Same derivation `<ActivityList>` uses, so the right pane reads the cache
@@ -180,13 +142,13 @@ function OverviewBody() {
180142
// The remembered row stands in for a missing one only on mount, which is the return trip
181143
// from Download or Restore. Back to the entry that had none means the newest backup again.
182144
const [ returnTripId, setReturnTripId ] = useState( () =>
183-
urlSelectedId ? null : lastSelectedId
145+
urlSelectedId ? null : overviewState.lastSelectedId
184146
);
185147
const selectedId = urlSelectedId ?? returnTripId ?? defaultSelectedId;
186148
// Only an explicit choice is remembered: `defaultSelectedId` moves as new backups
187149
// land, and pinning the row it happened to name would outlive its own reason.
188150
useEffect( () => {
189-
lastSelectedId = urlSelectedId ?? returnTripId;
151+
overviewState.lastSelectedId = urlSelectedId ?? returnTripId;
190152
}, [ urlSelectedId, returnTripId ] );
191153
// Puts the remembered row in the address, so `clearSelected` below always has a real
192154
// change to make: without one, TanStack treats its navigate as a no-op and pushes no

‎projects/packages/backup/tests/cleared-selection-back-stack.test.tsx‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import userEvent from '@testing-library/user-event';
2424
import { stage as OverviewStage } from '../routes/dashboard/stage';
2525
import { stage as DownloadStage } from '../routes/download/stage';
2626
import { queryClient } from '../src/dashboard/data/query-client';
27-
import { resetListStateForTesting } from '../src/dashboard/screens/overview';
2827

2928
const CONNECTED = { isRegistered: true, hasConnectedOwner: true, isUserConnected: true };
3029
const PER_PAGE = 10;
@@ -168,7 +167,6 @@ async function roundTripViaDownload( view: ReturnType< typeof renderApp > ): Pro
168167
beforeEach( () => {
169168
queryClient.clear();
170169
queryClient.setDefaultOptions( { queries: { retry: false } } );
171-
resetListStateForTesting();
172170

173171
log = Array.from( { length: PER_PAGE }, ( _, index ) => rewindId( index ) );
174172
mockApiFetch.mockReset();

‎projects/packages/backup/tests/error-surfacing.test.tsx‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import userEvent from '@testing-library/user-event';
3030
import { stage as OverviewStage } from '../routes/dashboard/stage';
3131
import ErrorBoundary from '../src/dashboard/components/error-boundary';
3232
import { queryClient } from '../src/dashboard/data/query-client';
33-
import { resetListStateForTesting } from '../src/dashboard/screens/overview';
3433

3534
const CONNECTED = { isRegistered: true, hasConnectedOwner: true, isUserConnected: true };
3635
const CAPABILITIES = { hasBackupPlan: true, hasScan: false };
@@ -135,7 +134,6 @@ function mockTwoPages( page2: () => Promise< unknown > ) {
135134
beforeEach( () => {
136135
queryClient.clear();
137136
queryClient.setDefaultOptions( { queries: { retry: false } } );
138-
resetListStateForTesting();
139137

140138
mockApiFetch.mockReset();
141139
mockApiFetch.mockResolvedValue( CAPABILITIES );

‎projects/packages/backup/tests/jest.setup.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ Object.defineProperty( window.HTMLElement.prototype, 'scrollIntoView', {
2222
writable: true,
2323
} );
2424

25+
// The Overview's state outlives an unmount by design, so each test starts as a fresh page load.
26+
beforeEach( () => {
27+
require( '../src/dashboard/screens/overview-state' ).resetOverviewStateForTesting();
28+
} );
29+
2530
// Required lazily: loaded up front, it would bind the real `speak()` before a test file mocks it.
2631
afterEach( () => {
2732
require( '../src/dashboard/data/announce' ).resetAnnouncerForTesting();

‎projects/packages/backup/tests/list-state-round-trip.test.tsx‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ import { stage as OverviewStage } from '../routes/dashboard/stage';
4242
import { stage as DownloadStage } from '../routes/download/stage';
4343
import { stage as RestoreStage } from '../routes/restore/stage';
4444
import { keys, queryClient } from '../src/dashboard/data/query-client';
45-
import { resetListStateForTesting } from '../src/dashboard/screens/overview';
45+
import { resetOverviewStateForTesting } from '../src/dashboard/screens/overview-state';
4646

4747
const CONNECTED = { isRegistered: true, hasConnectedOwner: true, isUserConnected: true };
4848

@@ -251,7 +251,6 @@ async function roundTripFromPage2Of20( Stage: () => React.JSX.Element ): Promise
251251
beforeEach( () => {
252252
queryClient.clear();
253253
queryClient.setDefaultOptions( { queries: { retry: false } } );
254-
resetListStateForTesting();
255254

256255
hasBackupPlan = true;
257256
routerSearch = {};
@@ -384,7 +383,7 @@ describe( 'A fresh page load', () => {
384383
await roundTripVia( DownloadStage );
385384

386385
// The reload: module state is what a new page load does not inherit.
387-
resetListStateForTesting();
386+
resetOverviewStateForTesting();
388387
queryClient.clear();
389388

390389
render( <OverviewStage /> );

‎projects/packages/backup/tests/overview-empty-pane-states.test.tsx‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import { onlineManager } from '@tanstack/react-query';
2121
import { render, screen } from '@testing-library/react';
2222
import { stage as OverviewStage } from '../routes/dashboard/stage';
2323
import { keys, queryClient } from '../src/dashboard/data/query-client';
24-
import { resetListStateForTesting } from '../src/dashboard/screens/overview';
2524

2625
const PROMPT = 'Select an item from the list to see details.';
2726
const LOADING = 'Loading item details…';
@@ -85,7 +84,6 @@ function mockEndpoints( activity: Activity ) {
8584
}
8685

8786
beforeEach( () => {
88-
resetListStateForTesting();
8987
queryClient.clear();
9088
queryClient.setDefaultOptions( { queries: { retry: false } } );
9189
mockApiFetch.mockReset();

‎projects/packages/backup/tests/restore-rows-in-activity-list.test.tsx‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import { render, screen } from '@testing-library/react';
2020
import { stage as OverviewStage } from '../routes/dashboard/stage';
2121
import { keys, queryClient } from '../src/dashboard/data/query-client';
2222
import { ACTIVITY_LOG_DEFAULT_PER_PAGE } from '../src/dashboard/hooks/use-activity-log';
23-
import { resetListStateForTesting } from '../src/dashboard/screens/overview';
2423

2524
const CONNECTED = { isRegistered: true, hasConnectedOwner: true, isUserConnected: true };
2625

@@ -139,8 +138,6 @@ async function rowTexts(): Promise< string[] > {
139138
}
140139

141140
beforeEach( () => {
142-
resetListStateForTesting();
143-
144141
queryClient.clear();
145142
queryClient.setDefaultOptions( { queries: { retry: false } } );
146143

‎projects/packages/backup/tests/stale-selection-recovery.test.tsx‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ import { speak } from '@wordpress/a11y';
5050
import { stage as OverviewStage } from '../routes/dashboard/stage';
5151
import { keys, queryClient } from '../src/dashboard/data/query-client';
5252
import { ACTIVITY_LOG_DEFAULT_PER_PAGE } from '../src/dashboard/hooks/use-activity-log';
53-
import { resetListStateForTesting } from '../src/dashboard/screens/overview';
5453

5554
const CONNECTED = { isRegistered: true, hasConnectedOwner: true, isUserConnected: true };
5655

@@ -197,9 +196,6 @@ function mockEndpoints( {
197196
}
198197

199198
beforeEach( () => {
200-
// Module state, so one test's selection would otherwise seed the next.
201-
resetListStateForTesting();
202-
203199
queryClient.clear();
204200
queryClient.setDefaultOptions( { queries: { retry: false } } );
205201

0 commit comments

Comments
 (0)