Clear delete navigate back URL when the deleted expense screen unmounts

src/libs/actions/Report/index.ts

@@ -83,6 +83,7 @@ import isSearchTopmostFullScreenRoute from '@libs/Navigation/helpers/isSearchTop
 import type {LinkToOptions} from '@libs/Navigation/helpers/linkTo/types';
 import Navigation from '@libs/Navigation/Navigation';
 import REPORT_LINK_ROUTE_PARAMS from '@libs/Navigation/reportLinkRouteParams';
+import TransitionTracker from '@libs/Navigation/TransitionTracker';
 import enhanceParameters from '@libs/Network/enhanceParameters';
 import {getDBTimeWithSkew, getIsOffline as isOfflineNetwork} from '@libs/NetworkState';
 import {buildOptimisticNextStep} from '@libs/NextStepUtils';
@@ -190,6 +191,7 @@ import {
 import {buildOptimisticSnapshotData, getCurrentSearchQueryJSON} from '@libs/SearchQueryUtils';
 import playSound, {SOUNDS} from '@libs/Sound';
 import {startSendMessagePhase} from '@libs/telemetry/sendMessageSpans';
+import {doesDeleteNavigateBackUrlIncludeDuplicatesReview} from '@libs/TransactionNavigationUtils';
 import {
     getAmount,
     getCurrency,
@@ -7273,14 +7275,41 @@ function downloadReportPDF(fileName: string, reportName: string, translate: Loca
     fileDownload(translate, addEncryptedAuthTokenToURL(pdfURL, encryptedAuthToken, true), downloadFileName, '', Browser.isMobileSafari()).then(() => setDownload(fileName, false));
 }
 
+// Read when the clear fires, since the unmounting screen may not have the value yet.
+let deleteTransactionNavigateBackUrl: string | undefined;
+// A newer deletion cancels this so a stale clear can't wipe its URL.
+let pendingDeleteTransactionNavigateBackUrlClear: {cancel: () => void} | undefined;
+Onyx.connectWithoutView({
+    key: ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL,
+    callback: (value) => {
+        deleteTransactionNavigateBackUrl = value;
+    },
+});
+
 function setDeleteTransactionNavigateBackUrl(url: string) {
+    pendingDeleteTransactionNavigateBackUrlClear?.cancel();
+    pendingDeleteTransactionNavigateBackUrlClear = undefined;
     Onyx.set(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL, url);
 }
 
 function clearDeleteTransactionNavigateBackUrl() {
     Onyx.merge(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL, null);
 }
 
+function clearDeleteTransactionNavigateBackUrlAfterTransitions() {
+    pendingDeleteTransactionNavigateBackUrlClear?.cancel();
+    pendingDeleteTransactionNavigateBackUrlClear = TransitionTracker.runAfterTransitions({
+        callback: () => {
+            pendingDeleteTransactionNavigateBackUrlClear = undefined;
+            if (!deleteTransactionNavigateBackUrl || doesDeleteNavigateBackUrlIncludeDuplicatesReview(deleteTransactionNavigateBackUrl)) {
+                return;
+            }
+            requestAnimationFrame(clearDeleteTransactionNavigateBackUrl);
+        },
+        waitForUpcomingTransition: true,
+    });
+}
+
 type DeleteAppReportProps = {
     report: OnyxEntry<Report>;
     reportActions: OnyxEntry<ReportActions>;
@@ -9371,6 +9400,7 @@ export {
     clearAddRoomMemberError,
     clearAvatarErrors,
     clearDeleteTransactionNavigateBackUrl,
+    clearDeleteTransactionNavigateBackUrlAfterTransitions,
     clearIOUError,
     clearNewRoomFormError,
     setNewRoomFormLoading,

src/pages/Search/SearchMoneyRequestReportPage.tsx

@@ -42,7 +42,13 @@ import ReactionListWrapper from '@pages/inbox/ReactionListWrapper';
 import {ReportActionEditMessageContextProvider} from '@pages/inbox/report/ReportActionEditMessageContext';
 import useClearReportActionDraftsOnReportChange from '@pages/inbox/report/useClearReportActionDraftsOnReportChange';
 
-import {clearDeleteTransactionNavigateBackUrl, createTransactionThreadReport, openReport, updateLastVisitTime} from '@userActions/Report';
+import {
+    clearDeleteTransactionNavigateBackUrl,
+    clearDeleteTransactionNavigateBackUrlAfterTransitions,
+    createTransactionThreadReport,
+    openReport,
+    updateLastVisitTime,
+} from '@userActions/Report';
 
 import CONST from '@src/CONST';
 import ONYXKEYS from '@src/ONYXKEYS';
@@ -125,6 +131,9 @@ function SearchMoneyRequestReportPage({route}: SearchMoneyRequestPageProps) {
         return () => handle.cancel();
     }, [isFocused, deleteTransactionNavigateBackUrl]);
 
+    // Native stack never renders a popped screen unfocused, so also clear on unmount.
+    useEffect(() => () => clearDeleteTransactionNavigateBackUrlAfterTransitions(), []);
+
     const [reportLoadingState = defaultReportLoadingState] = useOnyx(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${reportIDFromRoute}`);
     const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP);
     const isReportArchived = useReportIsArchived(report?.reportID);

src/pages/inbox/DeleteTransactionNavigateBackHandler.tsx

@@ -1,23 +1,31 @@
 import useOnyx from '@hooks/useOnyx';
 
+import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
 import TransitionTracker from '@libs/Navigation/TransitionTracker';
+import {isReportTransactionThread} from '@libs/ReportUtils';
 import {doesDeleteNavigateBackUrlIncludeDuplicatesReview} from '@libs/TransactionNavigationUtils';
 
-import {clearDeleteTransactionNavigateBackUrl} from '@userActions/Report';
+import {clearDeleteTransactionNavigateBackUrl, clearDeleteTransactionNavigateBackUrlAfterTransitions} from '@userActions/Report';
 
 import ONYXKEYS from '@src/ONYXKEYS';
 
 import {useIsFocused} from '@react-navigation/native';
 import {useEffect} from 'react';
 
+type DeleteTransactionNavigateBackHandlerProps = {
+    /** The reportID of the report screen this handler is mounted in */
+    reportID: string | undefined;
+};
+
 /**
  * Component that does not render anything but isolates the NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL
  * subscription from ReportScreen. Clears the URL after interactions complete
  * when the report is no longer focused.
  */
-function DeleteTransactionNavigateBackHandler() {
+function DeleteTransactionNavigateBackHandler({reportID}: DeleteTransactionNavigateBackHandlerProps) {
     const isFocused = useIsFocused();
     const [deleteTransactionNavigateBackUrl] = useOnyx(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL);
+    const [isTransactionThread = false] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(reportID)}`, {selector: isReportTransactionThread});
 
     useEffect(() => {
         if (isFocused || !deleteTransactionNavigateBackUrl) {
@@ -36,6 +44,14 @@ function DeleteTransactionNavigateBackHandler() {
         return () => handle.cancel();
     }, [isFocused, deleteTransactionNavigateBackUrl]);
 
+    // Native stack never renders a popped screen unfocused, so also clear when a transaction thread unmounts.
+    useEffect(() => {
+        if (!isTransactionThread) {
+            return;
+        }
+        return () => clearDeleteTransactionNavigateBackUrlAfterTransitions();
+    }, [isTransactionThread]);
+
     return null;
 }
 

src/pages/inbox/ReportScreen.tsx

@@ -118,7 +118,7 @@ function ReportScreen({route, navigation, shouldDeferReportActions = false}: Rep
                         >
                             {!shouldDeferNonEssentials && (
                                 <>
-                                    <DeleteTransactionNavigateBackHandler />
+                                    <DeleteTransactionNavigateBackHandler reportID={reportIDFromRoute} />
                                     <ReportRouteParamHandler />
                                     <OneTransactionThreadRedirectHandler />
                                     <ReportFetchHandler />

tests/ui/SearchMoneyRequestReportPageTest.tsx

@@ -73,6 +73,7 @@ jest.mock('@userActions/Report', () => ({
     openReport: jest.fn(),
     updateLastVisitTime: jest.fn(),
     clearDeleteTransactionNavigateBackUrl: jest.fn(),
+    clearDeleteTransactionNavigateBackUrlAfterTransitions: jest.fn(),
 }));
 
 // All child components are mocked to null: none of them need to actually render for these tests,

tests/unit/DeleteTransactionNavigateBackHandlerTest.tsx

@@ -0,0 +1,157 @@
+import {render} from '@testing-library/react-native';
+
+import TransitionTracker from '@libs/Navigation/TransitionTracker';
+
+import DeleteTransactionNavigateBackHandler from '@pages/inbox/DeleteTransactionNavigateBackHandler';
+
+import {setDeleteTransactionNavigateBackUrl} from '@userActions/Report';
+
+import ONYXKEYS from '@src/ONYXKEYS';
+import type {Report} from '@src/types/onyx';
+
+import type * as Navigation from '@react-navigation/native';
+
+import Onyx from 'react-native-onyx';
+
+import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';
+
+jest.mock('@react-navigation/native', () => ({
+    ...jest.requireActual<typeof Navigation>('@react-navigation/native'),
+    useIsFocused: jest.fn(() => true),
+}));
+
+jest.mock('@libs/Navigation/TransitionTracker', () => ({
+    runAfterTransitions: jest.fn(({callback}: {callback: () => void}) => {
+        callback();
+        return {cancel: jest.fn()};
+    }),
+}));
+
+jest.mock('@libs/ReportUtils', () => ({
+    ...jest.requireActual<Record<string, unknown>>('@libs/ReportUtils'),
+    isReportTransactionThread: jest.fn((report?: Report) => !!report?.parentReportActionID),
+}));
+
+const THREAD_REPORT_ID = '1';
+const CHAT_REPORT_ID = '2';
+const REPORT_URL = 'r/123';
+const NEXT_REPORT_URL = 'r/456';
+const DUPLICATES_REVIEW_URL = 'r/123/duplicates/review';
+
+async function flushClear() {
+    await waitForBatchedUpdates();
+    await new Promise<void>((resolve) => {
+        requestAnimationFrame(() => resolve());
+    });
+    await waitForBatchedUpdates();
+}
+
+async function getNavigateBackUrl() {
+    return new Promise<string | undefined>((resolve) => {
+        const connection = Onyx.connectWithoutView({
+            key: ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL,
+            callback: (value) => {
+                Onyx.disconnect(connection);
+                resolve(value);
+            },
+        });
+    });
+}
+
+describe('DeleteTransactionNavigateBackHandler', () => {
+    beforeAll(() => {
+        Onyx.init({keys: ONYXKEYS});
+    });
+
+    beforeEach(async () => {
+        await Onyx.clear();
+        await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${THREAD_REPORT_ID}`, {reportID: THREAD_REPORT_ID, parentReportID: '3', parentReportActionID: '4'});
+        await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${CHAT_REPORT_ID}`, {reportID: CHAT_REPORT_ID});
+        await waitForBatchedUpdates();
+    });
+
+    it('clears the navigate back URL when the focused screen unmounts', async () => {
+        // Given a focused expense screen that was just deleted, so the URL is set while it is still focused
+        const {unmount} = render(<DeleteTransactionNavigateBackHandler reportID={THREAD_REPORT_ID} />);
+        await Onyx.set(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL, REPORT_URL);
+        await waitForBatchedUpdates();
+
+        // When the native stack pops it, it unmounts without ever rendering unfocused
+        unmount();
+        await flushClear();
+
+        // Then the URL is cleared so it does not leak into the next expense
+        expect(await getNavigateBackUrl()).toBeUndefined();
+    });
+
+    it('keeps the navigate back URL while the screen stays focused', async () => {
+        // Given a focused expense screen with the URL set
+        render(<DeleteTransactionNavigateBackHandler reportID={THREAD_REPORT_ID} />);
+        await Onyx.set(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL, REPORT_URL);
+
+        // When nothing navigates away
+        await flushClear();
+
+        // Then the URL stays, so the deleted expense does not flash Not Found before the pop
+        expect(await getNavigateBackUrl()).toBe(REPORT_URL);
+    });
+
+    it('keeps the duplicates review URL when the screen unmounts', async () => {
+        // Given a focused screen whose URL points to duplicates review, which clears the URL by itself
+        const {unmount} = render(<DeleteTransactionNavigateBackHandler reportID={THREAD_REPORT_ID} />);
+        await Onyx.set(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL, DUPLICATES_REVIEW_URL);
+        await waitForBatchedUpdates();
+
+        // When the screen unmounts
+        unmount();
+        await flushClear();
+
+        // Then the URL is left for the duplicates review flow
+        expect(await getNavigateBackUrl()).toBe(DUPLICATES_REVIEW_URL);
+    });
+
+    it('does not clear a newer deletion URL from a remount clear that is still waiting', async () => {
+        // Given a screen that remounts without a deletion, so its clear waits for a later transition
+        let pendingCallback: (() => void) | undefined;
+        let isCancelled = false;
+        jest.mocked(TransitionTracker.runAfterTransitions).mockImplementationOnce(({callback}) => {
+            pendingCallback = () => {
+                callback();
+            };
+            return {
+                cancel: () => {
+                    isCancelled = true;
+                },
+            };
+        });
+        const {unmount} = render(<DeleteTransactionNavigateBackHandler reportID={THREAD_REPORT_ID} />);
+        await waitForBatchedUpdates();
+        unmount();
+
+        // When the next expense is deleted before that transition finishes
+        setDeleteTransactionNavigateBackUrl(NEXT_REPORT_URL);
+        await waitForBatchedUpdates();
+        if (!isCancelled) {
+            pendingCallback?.();
+        }
+        await flushClear();
+
+        // Then the stale clear is cancelled and the new URL stays until its own screen leaves
+        expect(isCancelled).toBe(true);
+        expect(await getNavigateBackUrl()).toBe(NEXT_REPORT_URL);
+    });
+
+    it('keeps the navigate back URL when a non transaction thread report unmounts', async () => {
+        // Given a chat report screen while a deletion URL is set
+        const {unmount} = render(<DeleteTransactionNavigateBackHandler reportID={CHAT_REPORT_ID} />);
+        await Onyx.set(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL, REPORT_URL);
+        await waitForBatchedUpdates();
+
+        // When that chat unmounts
+        unmount();
+        await flushClear();
+
+        // Then the URL stays, since only a deleted transaction thread should clear it on unmount
+        expect(await getNavigateBackUrl()).toBe(REPORT_URL);
+    });
+});