Clear delete navigate back URL when the deleted expense screen unmounts

src/hooks/useClearDeleteTransactionNavigateBackUrlOnUnmount.ts

@@ -0,0 +1,29 @@
+import {doesDeleteNavigateBackUrlIncludeDuplicatesReview} from '@libs/TransactionNavigationUtils';
+
+import {clearDeleteTransactionNavigateBackUrl, clearDeleteTransactionNavigateBackUrlAfterTransitions} from '@userActions/Report';
+
+import ONYXKEYS from '@src/ONYXKEYS';
+
+import {useEffect} from 'react';
+import Onyx from 'react-native-onyx';
+
+/** Clears the delete navigate back URL on unmount, since native stack never renders a popped screen unfocused. */
+function useClearDeleteTransactionNavigateBackUrlOnUnmount(shouldClear: boolean) {
+    useEffect(() => {
+        if (!shouldClear) {
+            return;
+        }
+        return () =>
+            clearDeleteTransactionNavigateBackUrlAfterTransitions(() => {
+                // Read when the clear fires, the unmounted screen may never have received the URL.
+                Onyx.get(ONYXKEYS.NVP_DELETE_TRANSACTION_NAVIGATE_BACK_URL).then((url) => {
+                    if (!url || doesDeleteNavigateBackUrlIncludeDuplicatesReview(url)) {
+                        return;
+                    }
+                    requestAnimationFrame(clearDeleteTransactionNavigateBackUrl);
+                });
+            });
+    }, [shouldClear]);
+}
+
+export default useClearDeleteTransactionNavigateBackUrlOnUnmount;

src/libs/actions/Report/index.ts

@@ -84,6 +84,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';
@@ -7304,14 +7305,30 @@ function downloadReportPDF(fileName: string, reportName: string, translate: Loca
     fileDownload(translate, addEncryptedAuthTokenToURL(pdfURL, encryptedAuthToken, true), downloadFileName, '', Browser.isMobileSafari()).then(() => setDownload(fileName, false));
 }
 
+// A newer deletion cancels this so a stale clear can't wipe its URL.
+let pendingDeleteTransactionNavigateBackUrlClear: {cancel: () => void} | undefined;
+
 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(onTransitionsEnd: () => void) {
+    pendingDeleteTransactionNavigateBackUrlClear?.cancel();
+    pendingDeleteTransactionNavigateBackUrlClear = TransitionTracker.runAfterTransitions({
+        callback: () => {
+            pendingDeleteTransactionNavigateBackUrlClear = undefined;
+            onTransitionsEnd();
+        },
+        waitForUpcomingTransition: true,
+    });
+}
+
 type DeleteAppReportProps = {
     report: OnyxEntry<Report>;
     reportActions: OnyxEntry<ReportActions>;
@@ -9402,6 +9419,7 @@ export {
     clearAddRoomMemberError,
     clearAvatarErrors,
     clearDeleteTransactionNavigateBackUrl,
+    clearDeleteTransactionNavigateBackUrlAfterTransitions,
     clearIOUError,
     clearNewRoomFormError,
     setNewRoomFormLoading,

src/pages/Search/SearchMoneyRequestReportPage.tsx

@@ -7,6 +7,7 @@ import {useSearchResultsContext} from '@components/Search/SearchContext';
 import useRHPWidth from '@components/WideRHPContextProvider/useRHPWidth';
 import WideRHPOverlayWrapper from '@components/WideRHPOverlayWrapper';
 
+import useClearDeleteTransactionNavigateBackUrlOnUnmount from '@hooks/useClearDeleteTransactionNavigateBackUrlOnUnmount';
 import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
 import useDismissOnMoneyRequestReportRemoval from '@hooks/useDismissOnMoneyRequestReportRemoval';
 import useDocumentTitle from '@hooks/useDocumentTitle';
@@ -125,6 +126,8 @@ function SearchMoneyRequestReportPage({route}: SearchMoneyRequestPageProps) {
         return () => handle.cancel();
     }, [isFocused, deleteTransactionNavigateBackUrl]);
 
+    useClearDeleteTransactionNavigateBackUrlOnUnmount(true);
+
     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,6 +1,9 @@
+import useClearDeleteTransactionNavigateBackUrlOnUnmount from '@hooks/useClearDeleteTransactionNavigateBackUrlOnUnmount';
 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';
@@ -10,14 +13,20 @@ 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 +45,8 @@ function DeleteTransactionNavigateBackHandler() {
         return () => handle.cancel();
     }, [isFocused, deleteTransactionNavigateBackUrl]);
 
+    useClearDeleteTransactionNavigateBackUrlOnUnmount(isTransactionThread);
+
     return null;
 }
 

src/pages/inbox/ReportScreen.tsx

@@ -120,7 +120,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);
+    });
+});