Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -500,11 +500,13 @@ function MoneyRequestReportTransactionList({
if (anchorTransactionID && latestActiveTransactionIDs?.includes(anchorTransactionID)) {
return;
}
// Don't take over a snapshot-backed carousel (identified by its sibling descriptors, e.g. the Home
// "Recently added" flow) that belongs to the transaction thread sitting underneath this report.
// Overwriting and then clearing it would drop that carousel when the user navigates back. Row presses
// still seed the correct siblings lazily via useNavigateToTransactionThread.
if (getActiveTransactionIDs().descriptors) {
// Don't take over a snapshot-backed carousel (identified by its snapshot hash, e.g. the Spend page
// search, or its sibling descriptors, e.g. the Home "Recently added" flow) that belongs to the
// transaction thread sitting underneath this report. Overwriting and then clearing it would drop that
// carousel when the user navigates back. Row presses still seed the correct siblings lazily via
// useNavigateToTransactionThread.
const {snapshotHash: activeSnapshotHash, descriptors: activeDescriptors} = getActiveTransactionIDs();
if (activeSnapshotHash != null || activeDescriptors) {
return;
}

Expand Down
11 changes: 6 additions & 5 deletions src/libs/actions/TransactionThreadNavigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,13 @@ function setActiveTransactionIDs(ids: string[], snapshotHash?: number, siblingDe
}

/**
* Returns the currently active transaction IDs and sibling descriptors. Used by screens that would otherwise
* take over the carousel context (e.g. a money request report opened on top of an existing transaction thread)
* so they can detect a snapshot-backed carousel (one with descriptors) and avoid clobbering it.
* Returns the currently active transaction IDs, snapshot hash, and sibling descriptors. Used by screens that
* would otherwise take over the carousel context (e.g. a money request report opened on top of an existing
* transaction thread) so they can detect a snapshot-backed carousel (one with a snapshot hash and/or
* descriptors) and avoid clobbering it.
*/
function getActiveTransactionIDs(): {ids: string[] | null; descriptors: Record<string, TransactionThreadNavigationDescriptor> | null} {
return {ids: lastSetIDs, descriptors: lastSetDescriptors};
function getActiveTransactionIDs(): {ids: string[] | null; snapshotHash: number | null; descriptors: Record<string, TransactionThreadNavigationDescriptor> | null} {
return {ids: lastSetIDs, snapshotHash: lastSetSnapshotHash, descriptors: lastSetDescriptors};
}

function clearActiveTransactionIDs() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import createRandomTransaction from '../utils/collections/transaction';
jest.mock('@libs/actions/TransactionThreadNavigation', () => ({
setActiveTransactionIDs: jest.fn(() => Promise.resolve()),
clearActiveTransactionIDs: jest.fn(() => Promise.resolve()),
getActiveTransactionIDs: jest.fn(() => ({ids: null, descriptors: null})),
getActiveTransactionIDs: jest.fn(() => ({ids: null, snapshotHash: null, descriptors: null})),
}));

// Mock the navigation module
Expand Down Expand Up @@ -44,7 +44,8 @@ function useActiveTransactionIDsEffect(visualOrderTransactionIDs: string[]) {
if (focusedRoute?.name !== SCREENS.RIGHT_MODAL.SEARCH_REPORT) {
return;
}
if (getActiveTransactionIDs().descriptors) {
const {snapshotHash: activeSnapshotHash, descriptors: activeDescriptors} = getActiveTransactionIDs();
if (activeSnapshotHash != null || activeDescriptors) {
return;
}
setActiveTransactionIDs(visualOrderTransactionIDs);
Expand All @@ -65,7 +66,7 @@ describe('MoneyRequestReportTransactionList - Active Transaction IDs Effect', ()

beforeEach(() => {
jest.clearAllMocks();
mockGetActiveTransactionIDs.mockReturnValue({ids: null, descriptors: null});
mockGetActiveTransactionIDs.mockReturnValue({ids: null, snapshotHash: null, descriptors: null});
(navigationRef.getRootState as jest.Mock).mockReturnValue({} as ReturnType<typeof navigationRef.getRootState>);
});

Expand Down Expand Up @@ -191,6 +192,7 @@ describe('MoneyRequestReportTransactionList - Active Transaction IDs Effect', ()
mockFindFocusedRoute.mockReturnValue({name: SCREENS.RIGHT_MODAL.SEARCH_REPORT, key: 'test-key'});
mockGetActiveTransactionIDs.mockReturnValue({
ids: ['recentlyAdded1', 'recentlyAdded2'],
snapshotHash: null,
descriptors: {recentlyAdded1: {reportID: 'r1', transaction: {...createRandomTransaction(1), transactionID: 'recentlyAdded1'}}},
});

Expand All @@ -207,6 +209,29 @@ describe('MoneyRequestReportTransactionList - Active Transaction IDs Effect', ()
expect(mockClearActiveTransactionIDs).not.toHaveBeenCalled();
});

it('should NOT take over a snapshot-backed carousel that already has a snapshot hash (e.g. the Spend page)', () => {
// Given the focused route is SEARCH_REPORT and a snapshot-backed carousel (e.g. the Spend page search) is active
mockFindFocusedRoute.mockReturnValue({name: SCREENS.RIGHT_MODAL.SEARCH_REPORT, key: 'test-key'});
mockGetActiveTransactionIDs.mockReturnValue({
ids: ['spend1', 'spend2'],
snapshotHash: 1234,
descriptors: null,
});

const transactionIDs = ['trans1', 'trans2', 'trans3'];

// When the hook is rendered and then unmounted
const {unmount} = renderHook(() => useActiveTransactionIDsEffect(transactionIDs));

// Then it should neither overwrite nor clear the existing carousel context, so the underlying
// Spend-page carousel (and its prev/next arrows) survives navigating back out of the report.
expect(mockSetActiveTransactionIDs).not.toHaveBeenCalled();

unmount();

expect(mockClearActiveTransactionIDs).not.toHaveBeenCalled();
});

it('should handle empty transaction IDs array', () => {
// Given the focused route is SEARCH_REPORT
mockFindFocusedRoute.mockReturnValue({name: SCREENS.RIGHT_MODAL.SEARCH_REPORT, key: 'test-key'});
Expand Down
Loading