diff --git a/src/libs/Navigation/helpers/cleanupAndNavigateAfterExpenseCreate.ts b/src/libs/Navigation/helpers/cleanupAndNavigateAfterExpenseCreate.ts index 8b85cf2d2fb8..d8db226cca7c 100644 --- a/src/libs/Navigation/helpers/cleanupAndNavigateAfterExpenseCreate.ts +++ b/src/libs/Navigation/helpers/cleanupAndNavigateAfterExpenseCreate.ts @@ -1,4 +1,6 @@ +import Log from '@libs/Log'; import {getReportOrDraftReport, isMoneyRequestReport} from '@libs/ReportUtils'; +import {isTracking} from '@libs/telemetry/submitFollowUpAction'; import CONST from '@src/CONST'; import type {Report, ReportAction} from '@src/types/onyx'; @@ -19,6 +21,18 @@ type CleanupAndNavigateAfterExpenseCreateParams = { isInvoice?: boolean; linkedTrackedExpenseReportAction?: OnyxEntry; action: DeepValueOf; + /** When false, runs cleanup only — use when dismiss/reveal already handled navigation. + * IMPORTANT: Caller must own telemetry span lifecycle. SubmitExpenseOrchestrator starts + * SPAN_SUBMIT_EXPENSE before calling createTransaction; when shouldNavigate=false, caller + * is responsible for ending the span (see useExpenseSubmission createTransaction). + * Skips shouldWaitForUpcomingTransition, so transition never arrives (no 1s timeout). + */ + shouldNavigate?: boolean; + /** Pre-computed navigation report ID. When provided, used instead of recomputing + * from report/backToReport/optimisticChatReportID to ensure UI and state register + * against the same destination. + */ + navigationReportID?: string; }; function cleanupAndNavigateAfterExpenseCreate({ @@ -31,14 +45,20 @@ function cleanupAndNavigateAfterExpenseCreate({ isInvoice, linkedTrackedExpenseReportAction, action, + shouldNavigate = true, + navigationReportID, }: CleanupAndNavigateAfterExpenseCreateParams) { + if (__DEV__ && isTracking() && !shouldNavigate) { + Log.warn('[cleanupAndNavigateAfterExpenseCreate] shouldNavigate=false but span is active. Caller must own span lifecycle — miss this and span hangs 60s until dropped.'); + } + cleanupAfterExpenseCreate({ draftTransactionIDs, linkedTrackedExpenseReportAction, - shouldWaitForUpcomingTransition: true, + shouldWaitForUpcomingTransition: shouldNavigate, }); - const finalActiveReportID = backToReport ?? report?.reportID ?? optimisticChatReportID; + const finalActiveReportID = navigationReportID ?? backToReport ?? report?.reportID ?? optimisticChatReportID; const hasMultipleTransactions = isInvoice ? false : isMoneyRequestReport(finalActiveReportID === report?.reportID ? report : getReportOrDraftReport(finalActiveReportID)); const shouldAddPendingNewTransactionIDs = action === CONST.IOU.ACTION.CATEGORIZE || action === CONST.IOU.ACTION.SHARE ? true : !isInvoice && !!finalActiveReportID && !hasMultipleTransactions; @@ -50,6 +70,7 @@ function cleanupAndNavigateAfterExpenseCreate({ isInvoice, hasMultipleTransactions, shouldAddPendingNewTransactionIDs, + shouldNavigate, }); } diff --git a/src/pages/Share/SubmitDetailsPage.tsx b/src/pages/Share/SubmitDetailsPage.tsx index 722674c377d9..0edc187864a8 100644 --- a/src/pages/Share/SubmitDetailsPage.tsx +++ b/src/pages/Share/SubmitDetailsPage.tsx @@ -13,6 +13,7 @@ import useOnyx from '@hooks/useOnyx'; import usePermissions from '@hooks/usePermissions'; import usePersonalPolicy from '@hooks/usePersonalPolicy'; import usePolicyForTransaction from '@hooks/usePolicyForTransaction'; +import usePreMountDestination from '@hooks/usePreMountDestination'; import usePrivateIsArchivedMap from '@hooks/usePrivateIsArchivedMap'; import useReportAttributes from '@hooks/useReportAttributes'; import useReportIsArchived from '@hooks/useReportIsArchived'; @@ -46,15 +47,18 @@ import {isTrackOnboardingChoice} from '@libs/OnboardingUtils'; import {getParticipantsOption, getReportOption} from '@libs/OptionsListUtils'; import {hasOnlyPersonalPolicies as hasOnlyPersonalPoliciesUtil, isGroupPolicy} from '@libs/PolicyUtils'; import {shouldValidateFile} from '@libs/ReceiptUtils'; -import {isMoneyRequestReport, isSelfDM} from '@libs/ReportUtils'; +import {getReportOrDraftReport, isMoneyRequestReport, isSelfDM} from '@libs/ReportUtils'; import {cancelSpan, endSpan} from '@libs/telemetry/activeSpans'; import {logReceiptCaptured, logReceiptSubmitted, mintAndStampReceiptTraceId} from '@libs/telemetry/ReceiptObservability'; +import {cancelTracking} from '@libs/telemetry/submitFollowUpAction'; import {getDefaultTaxCode, getIsFromGlobalCreate, getTaxValue} from '@libs/TransactionUtils'; import DraftWorkspaceOpener from '@pages/iou/request/step/confirmation/DraftWorkspaceOpener'; +import getSubmitExpensePreMountDestinationRoute from '@pages/iou/request/step/confirmation/getSubmitExpensePreMountDestinationRoute'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; +import ROUTES from '@src/ROUTES'; import type SCREENS from '@src/SCREENS'; import type {Report as ReportType} from '@src/types/onyx'; import type {Receipt} from '@src/types/onyx/Transaction'; @@ -71,6 +75,7 @@ import {showErrorAlert} from './ShareRootPage'; import useShareFileSizeValidation from './useShareFileSizeValidation'; type ShareDetailsPageProps = StackScreenProps; + function SubmitDetailsPage({ route: { params: {reportOrAccountID}, @@ -125,7 +130,13 @@ function SubmitDetailsPage({ const personalPolicy = usePersonalPolicy(); const [startLocationPermissionFlow, setStartLocationPermissionFlow] = useState(false); const [isConfirming, setIsConfirming] = useState(false); + // Set when the destination report didn't exist at submit time (e.g. a brand-new recipient) — the expense create + // writes it optimistically, and we keep the confirm button in its loading state until it lands so dismissing doesn't flash the inbox. + const [pendingNavigationReportID, setPendingNavigationReportID] = useState(undefined); + const [pendingNavigationReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(pendingNavigationReportID)}`); + const hasStartedPendingNavigation = useRef(false); const formHasBeenSubmitted = useRef(false); + const hasCalledReveal = useRef(false); const [userLocation] = useOnyx(ONYXKEYS.USER_LOCATION); const [errorTitle, setErrorTitle] = useState(undefined); @@ -262,6 +273,7 @@ function SubmitDetailsPage({ const listOfParticipants = participants.filter((participant) => participant.selected); const participant = listOfParticipants.at(0) ?? selectedParticipants.at(0); const reportToSubmit = resolveReportForMoneyRequest({transaction, transactionReport, routeReport: report, policy}); + const postSubmitNavigationReportID = (isSelfDM(report) ? report : reportToSubmit)?.reportID ?? reportOrAccountID; const isIouReport = isMoneyRequestReport(reportToSubmit); const policyTagsForRequestMoney = useMoneyRequestPolicyTags({ moneyRequestReportID: isIouReport ? reportToSubmit?.reportID : undefined, @@ -269,6 +281,78 @@ function SubmitDetailsPage({ participantReportID: participant?.reportID, }); + const hasNavigationDestination = !!transaction && !!postSubmitNavigationReportID; + // Empty draft skips REPORT_DRAFT fallback — report must be in COLLECTION.REPORT to render behind the share modal. + const destinationReportInCollection = hasNavigationDestination ? getReportOrDraftReport(postSubmitNavigationReportID, undefined, undefined, {}) : undefined; + // Destination report isn't in COLLECTION.REPORT yet (e.g. a recipient with no existing chat) — it will only + // exist after the expense create writes it optimistically, so pre-mounting is impossible. + const isDestinationReportMissing = hasNavigationDestination && !destinationReportInCollection?.reportID; + // Keep pre-mount off while pending navigation owns the reveal — otherwise once the optimistic + // report lands, isDestinationReportMissing flips false and usePreMountDestination would schedule + // a narrow pre-insert alongside the pending revealRouteBeforeDismissingModal (dual nav). + const preMountDestinationRoute = getSubmitExpensePreMountDestinationRoute({ + isTransactionReady: hasNavigationDestination && !isDestinationReportMissing && !pendingNavigationReportID, + destinationReportID: postSubmitNavigationReportID, + destinationReport: destinationReportInCollection, + isFromGlobalCreate: false, + canPreInsertSearch: false, + iouType, + isCreatingTrackExpense, + isSelfDMDestination: isSelfDM(report), + }); + + const {reveal: revealPreMountDestination, cleanupPreMount} = usePreMountDestination(preMountDestinationRoute, { + shouldPreservePreInsertedRouteOnUnmount: () => hasCalledReveal.current, + }); + + // Single entry point for the pending-navigation reveal — the ref guard makes it safe to call from either + // path below, so whichever fires first wins and the other becomes a no-op. + const revealPendingNavigation = (reportID: string) => { + if (hasStartedPendingNavigation.current) { + return; + } + hasStartedPendingNavigation.current = true; + Navigation.revealRouteBeforeDismissingModal(ROUTES.REPORT_WITH_ID.getRoute(reportID), { + afterTransition: () => { + setIsConfirming(false); + }, + }); + }; + + // Once the optimistically created destination report lands in Onyx, reveal it directly over the modal — + // navigating before it exists would dismiss to the inbox and flash it while the report screen mounts. + useEffect(() => { + if (!pendingNavigationReportID || !pendingNavigationReport?.reportID) { + return; + } + revealPendingNavigation(pendingNavigationReportID); + }, [pendingNavigationReportID, pendingNavigationReport?.reportID, revealPendingNavigation]); + + // Fallback: if optimistic report lands under different ID, still reveal to intended destination after timeout. + useEffect(() => { + if (!pendingNavigationReportID || hasStartedPendingNavigation.current || pendingNavigationReport?.reportID) { + return; + } + + const timeoutId = setTimeout(() => revealPendingNavigation(pendingNavigationReportID), 500); + + return () => clearTimeout(timeoutId); + }, [pendingNavigationReportID, pendingNavigationReport?.reportID, revealPendingNavigation]); + + // Timeout for pending report arrival — if optimistic write doesn't land within 5s, something's broken anyway. + // Fallback dismisses spinner and lets user navigate back without indefinite hang. + useEffect(() => { + if (!pendingNavigationReportID) { + return; + } + + const timeoutId = setTimeout(() => { + setIsConfirming(false); + }, 5000); + + return () => clearTimeout(timeoutId); + }, [pendingNavigationReportID]); + const finishRequestAndNavigate = (receipt: Receipt, gpsPoint?: GpsPoint) => { if (!transaction || !participant) { return; @@ -286,107 +370,157 @@ function SubmitDetailsPage({ iouType, }); - if (isSelfDM(report)) { - trackExpense({ - report: report ?? {reportID: reportOrAccountID}, - isDraftPolicy: false, - isDraftChatReport: !!reportDraft, - participantParams: {payeeEmail: currentUserPersonalDetails.login, payeeAccountID: currentUserPersonalDetails.accountID, participant}, - policyParams: {policy, policyTagList: policyTags, policyCategories}, - action: CONST.IOU.TYPE.CREATE, - transactionParams: { - attendees: transaction.comment?.attendees, - amount: transactionAmount, - currency: transaction.currency, - comment: trimmedComment, - receipt, - category: transaction.category, - tag: transaction.tag, - taxCode: transactionTaxCode, - taxAmount: transactionTaxAmount, - taxValue: transactionTaxValue, - billable: transaction.billable, - reimbursable: transaction.reimbursable, - merchant: transaction.merchant ?? '', - created: transaction.created, - actionableWhisperReportActionID: transaction.actionableWhisperReportActionID, - linkedTrackedExpenseReportAction: transaction.linkedTrackedExpenseReportAction, - linkedTrackedExpenseReportID: transaction.linkedTrackedExpenseReportID, - isLinkedTrackedExpenseReportArchived, + const performExpenseCreate = () => { + if (isSelfDM(report)) { + trackExpense({ + report: report ?? {reportID: reportOrAccountID}, + isDraftPolicy: false, + isDraftChatReport: !!reportDraft, + participantParams: {payeeEmail: currentUserPersonalDetails.login, payeeAccountID: currentUserPersonalDetails.accountID, participant}, + policyParams: {policy, policyTagList: policyTags, policyCategories}, + action: CONST.IOU.TYPE.CREATE, + transactionParams: { + attendees: transaction.comment?.attendees, + amount: transactionAmount, + currency: transaction.currency, + comment: trimmedComment, + receipt, + category: transaction.category, + tag: transaction.tag, + taxCode: transactionTaxCode, + taxAmount: transactionTaxAmount, + taxValue: transactionTaxValue, + billable: transaction.billable, + reimbursable: transaction.reimbursable, + merchant: transaction.merchant ?? '', + created: transaction.created, + actionableWhisperReportActionID: transaction.actionableWhisperReportActionID, + linkedTrackedExpenseReportAction: transaction.linkedTrackedExpenseReportAction, + linkedTrackedExpenseReportID: transaction.linkedTrackedExpenseReportID, + isLinkedTrackedExpenseReportArchived, + gpsPoint, + }, + existingTransaction: transaction, + isASAPSubmitBetaEnabled, + currentUser: {accountID: currentUserPersonalDetails.accountID, email: currentUserPersonalDetails.login ?? ''}, + introSelected, + conciergeChat, + quickAction, + recentWaypoints, + betas, + draftTransactionIDs, + isSelfTourViewed, + optimisticTransactionID, + currentUserLocalCurrency: currentUserPersonalDetails.localCurrencyCode ?? CONST.CURRENCY.USD, + delegateAccountID, + reportActionsList: undefined, + }); + } else { + const existingTransactionDraft = existingTransactionID ? transactionDrafts?.[existingTransactionID] : undefined; + + requestMoney({ + report: reportToSubmit, + participantParams: {payeeEmail: currentUserPersonalDetails.login, payeeAccountID: currentUserPersonalDetails.accountID, participant}, + policyParams: {policy, policyTagList: policyTagsForRequestMoney, policyCategories, policyRecentlyUsedCategories, policyRecentlyUsedTags}, gpsPoint, - }, - existingTransaction: transaction, - isASAPSubmitBetaEnabled, - currentUser: {accountID: currentUserPersonalDetails.accountID, email: currentUserPersonalDetails.login ?? ''}, - introSelected, - conciergeChat, - quickAction, - recentWaypoints, - betas, - draftTransactionIDs, - isSelfTourViewed, - optimisticTransactionID, - currentUserLocalCurrency: currentUserPersonalDetails.localCurrencyCode ?? CONST.CURRENCY.USD, - delegateAccountID, - reportActionsList: undefined, - }); - } else { - const existingTransactionDraft = existingTransactionID ? transactionDrafts?.[existingTransactionID] : undefined; - - requestMoney({ - report: reportToSubmit, - participantParams: {payeeEmail: currentUserPersonalDetails.login, payeeAccountID: currentUserPersonalDetails.accountID, participant}, - policyParams: {policy, policyTagList: policyTagsForRequestMoney, policyCategories, policyRecentlyUsedCategories, policyRecentlyUsedTags}, - gpsPoint, - action: CONST.IOU.TYPE.CREATE, - transactionParams: { - attendees: transaction.comment?.attendees, - amount: transactionAmount, - currency: transaction.currency, - comment: trimmedComment, - receipt, - category: transaction.category, - tag: transaction.tag, - taxCode: transactionTaxCode, - taxAmount: transactionTaxAmount, - taxValue: transactionTaxValue, - billable: transaction.billable, - reimbursable: transaction.reimbursable, - merchant: transaction.merchant ?? '', - created: transaction.created, - actionableWhisperReportActionID: transaction.actionableWhisperReportActionID, - linkedTrackedExpenseReportAction: transaction.linkedTrackedExpenseReportAction, - linkedTrackedExpenseReportID: transaction.linkedTrackedExpenseReportID, - isLinkedTrackedExpenseReportArchived, - }, - shouldGenerateTransactionThreadReport: false, - isASAPSubmitBetaEnabled, - currentUserAccountIDParam: currentUserPersonalDetails.accountID, - currentUserEmailParam: currentUserPersonalDetails.login ?? '', - transactionViolations, - policyRecentlyUsedCurrencies: policyRecentlyUsedCurrencies ?? [], - quickAction, - existingTransactionDraft, - existingTransaction: storedTransaction ?? transaction, - draftTransactionIDs, - isSelfTourViewed, - conciergeChat, - betas, - personalDetails, - optimisticTransactionID, - isTrackIntentUser, - delegateAccountID, - }); - } - cleanupAndNavigateAfterExpenseCreate({ + action: CONST.IOU.TYPE.CREATE, + transactionParams: { + attendees: transaction.comment?.attendees, + amount: transactionAmount, + currency: transaction.currency, + comment: trimmedComment, + receipt, + category: transaction.category, + tag: transaction.tag, + taxCode: transactionTaxCode, + taxAmount: transactionTaxAmount, + taxValue: transactionTaxValue, + billable: transaction.billable, + reimbursable: transaction.reimbursable, + merchant: transaction.merchant ?? '', + created: transaction.created, + actionableWhisperReportActionID: transaction.actionableWhisperReportActionID, + linkedTrackedExpenseReportAction: transaction.linkedTrackedExpenseReportAction, + linkedTrackedExpenseReportID: transaction.linkedTrackedExpenseReportID, + isLinkedTrackedExpenseReportArchived, + }, + shouldGenerateTransactionThreadReport: false, + isASAPSubmitBetaEnabled, + currentUserAccountIDParam: currentUserPersonalDetails.accountID, + currentUserEmailParam: currentUserPersonalDetails.login ?? '', + transactionViolations, + policyRecentlyUsedCurrencies: policyRecentlyUsedCurrencies ?? [], + quickAction, + existingTransactionDraft, + existingTransaction: storedTransaction ?? transaction, + draftTransactionIDs, + isSelfTourViewed, + conciergeChat, + betas, + personalDetails, + optimisticTransactionID, + isTrackIntentUser, + delegateAccountID, + optimisticChatReportID: routeReportID, + }); + } + }; + + const cleanupParams = { report: isSelfDM(report) ? report : reportToSubmit, action: CONST.IOU.ACTION.CREATE, draftTransactionIDs, transactionID: optimisticTransactionID, isFromGlobalCreate: getIsFromGlobalCreate(transaction), - optimisticChatReportID: reportOrAccountID, + optimisticChatReportID: routeReportID, + navigationReportID: postSubmitNavigationReportID, linkedTrackedExpenseReportAction: transaction.linkedTrackedExpenseReportAction, - }); + }; + + const runExpenseCreateAndCleanup = (shouldNavigate: boolean) => { + performExpenseCreate(); + cleanupAndNavigateAfterExpenseCreate({...cleanupParams, shouldNavigate}); + }; + + // Share never calls startTracking, so cancel any stale span from a prior flow to avoid the warning + // at cleanupAndNavigateAfterExpenseCreate when shouldNavigate: false. + cancelTracking(); + + if (preMountDestinationRoute) { + performExpenseCreate(); + hasCalledReveal.current = true; + revealPreMountDestination(() => { + cleanupAndNavigateAfterExpenseCreate({...cleanupParams, shouldNavigate: false}); + setIsConfirming(false); + }); + return; + } + + // Pre-mount wasn't possible because the destination report doesn't exist yet. Create the expense without + // navigating — the optimistic write creates the report — and let the pending-navigation effect reveal it + // once it lands. isConfirming stays true until the reveal transition ends, so the confirm button keeps + // its loading state exactly like the pre-mounted path. + if (isDestinationReportMissing) { + runExpenseCreateAndCleanup(false); + setPendingNavigationReportID(postSubmitNavigationReportID); + return; + } + + // Wide layout fallback: destination exists but is not topmost — reveal it via dismissal modal instead of pre-insert. + const topmostReportId = Navigation.getTopmostReportId(); + if (topmostReportId !== postSubmitNavigationReportID) { + performExpenseCreate(); + hasCalledReveal.current = true; + Navigation.revealRouteBeforeDismissingModal(ROUTES.REPORT_WITH_ID.getRoute(postSubmitNavigationReportID), { + afterTransition: () => { + cleanupAndNavigateAfterExpenseCreate({...cleanupParams, shouldNavigate: false}); + setIsConfirming(false); + }, + }); + return; + } + + runExpenseCreateAndCleanup(true); }; const onSuccess = (file: File, locationPermissionGranted?: boolean) => { @@ -477,7 +611,10 @@ function SubmitDetailsPage({ /> Navigation.goBack()} + onBackButtonPress={() => { + cleanupPreMount(); + Navigation.goBack(); + }} /> & {getReportOrDraftReport: GetReportOrDraftReportFn}; + jest.mock('@libs/actions/IOU/TrackExpense', () => { const actual = jest.requireActual('@libs/actions/IOU/TrackExpense'); return { @@ -32,13 +41,22 @@ jest.mock('@libs/actions/IOU/TrackExpense', () => { }; }); +jest.mock('@libs/ReportUtils', () => { + const actual = jest.requireActual('@libs/ReportUtils'); + return { + ...actual, + getReportOrDraftReport: jest.fn(actual.getReportOrDraftReport), + }; +}); + +// Guards against error #1: cleanup navigation targeting a different transaction than requestMoney created. jest.mock('@libs/Navigation/helpers/cleanupAndNavigateAfterExpenseCreate', () => jest.fn()); jest.mock('@libs/fileDownload/FileUtils', () => { const actual = jest.requireActual('@libs/fileDownload/FileUtils'); return { ...actual, - // Fire the success callback synchronously with a minimal File-like object so finishRequestAndNavigate runs. + // Drives confirm → performUpload → finishRequestAndNavigate for tests #1, #2, #3, and #4. readFileAsync: jest.fn((_uri: string, name: string, onSuccess: (file: {name: string; uri: string}) => void) => { onSuccess({name, uri: 'file://test-receipt.jpg'}); }), @@ -65,6 +83,32 @@ jest.mock('@hooks/useReportIsArchived', () => jest.fn(() => false)); // eslint-disable-next-line @typescript-eslint/naming-convention -- numeric account IDs are the Onyx participants key shape jest.mock('@hooks/useReportOrReportDraft', () => jest.fn((reportID?: string) => (reportID ? {reportID, participants: {1: {}, 2: {}}, type: 'chat'} : undefined))); +jest.mock('@libs/getIsNarrowLayout', () => jest.fn(() => true)); + +jest.mock('@libs/Navigation/TransitionTracker', () => ({ + runAfterTransitions: jest.fn(({callback}: {callback: () => void}) => { + callback(); + return {cancel: jest.fn()}; + }), +})); + +jest.mock('@libs/Navigation/helpers/isSearchTopmostFullScreenRoute', () => jest.fn(() => false)); + +jest.mock('@libs/Navigation/helpers/isReportTopmostSplitNavigator', () => jest.fn(() => false)); + +jest.mock('@libs/Navigation/helpers/isReportOpenInRHP', () => jest.fn(() => false)); + +jest.mock('@libs/Scheduler', () => ({ + Scheduler: { + scheduleWhenIdle: jest.fn((callback: () => void) => { + callback(); + return {cancel: jest.fn()}; + }), + }, +})); + +// Infrastructure for usePreMountDestination (added to SubmitDetailsPage): without these, render throws +// "getTopmostReportId is not a function" and tests never reach their assertions. jest.mock('@libs/Navigation/Navigation', () => ({ navigate: jest.fn(), goBack: jest.fn(), @@ -72,16 +116,42 @@ jest.mock('@libs/Navigation/Navigation', () => ({ getReportRouteByID: jest.fn(() => undefined), removeScreenByKey: jest.fn(), setNavigationActionToMicrotaskQueue: jest.fn((cb: () => void) => cb()), + dismissModal: jest.fn((options?: {afterTransition?: () => void}) => { + options?.afterTransition?.(); + }), dismissModalWithReport: jest.fn(), getActiveRouteWithoutParams: jest.fn(() => ''), getActiveRoute: jest.fn(() => ''), - navigationRef: {getCurrentRoute: jest.fn(() => undefined), getState: jest.fn(() => ({}))}, + getTopmostReportId: jest.fn(() => 'report-share-1'), + getIsFullscreenPreInsertedUnderRHP: jest.fn(() => false), + getPreInsertedFullscreenRouteName: jest.fn(() => undefined), + clearFullscreenPreInsertedFlag: jest.fn(), + revealRouteBeforeDismissingModal: jest.fn((_route: unknown, options?: {afterTransition?: () => void}) => { + options?.afterTransition?.(); + }), + preInsertFullscreenUnderRHP: jest.fn(), + removePreInsertedFullscreenIfNeeded: jest.fn(), + navigationRef: { + getCurrentRoute: jest.fn(() => ({name: 'Share', key: 'Share'})), + getState: jest.fn(() => ({routes: [{name: 'Share', key: 'Share'}]})), + getRootState: jest.fn(() => ({routes: [{name: 'Share', key: 'Share'}], stale: false})), + }, })); jest.mock('@pages/Share/ShareRootPage', () => ({showErrorAlert: jest.fn()})); jest.mock('@pages/Share/useShareFileSizeValidation', () => jest.fn()); +jest.mock('@components/HeaderWithBackButton', () => { + const React2 = require('react'); + const {Pressable, Text} = require('react-native'); + return { + __esModule: true, + default: ({onBackButtonPress}: {onBackButtonPress?: () => void}) => + React2.createElement(Pressable, {testID: 'mock-back-button', onPress: onBackButtonPress}, React2.createElement(Text, null, 'back')), + }; +}); + // Mock the confirmation list down to a button that fires onConfirm — isolates the test from the form internals. jest.mock('@components/MoneyRequestConfirmationList', () => { const React2 = require('react'); @@ -133,7 +203,7 @@ function createDraftTransaction(): Transaction { } as Transaction; } -// Seed the shared Onyx state both tests drive: a chat report, a draft transaction, and the shared file. +// Seed the shared Onyx state all tests drive: a chat report, a draft transaction, and the shared file. async function seedShareState() { await act(async () => { await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${SHARED_REPORT_ID}`, createTestReport()); @@ -147,18 +217,37 @@ async function seedShareState() { }); } -// Render the page and press the mocked confirm button, which is the single entry point both tests exercise. +// Render the page and press the mocked confirm button — the single entry point submit tests exercise. async function renderAndConfirm() { - render( + renderSubmitDetailsPage(); + await waitForBatchedUpdatesWithAct(); + fireEvent.press(screen.getByTestId('mock-confirm-button')); + await waitForBatchedUpdatesWithAct(); +} + +function renderSubmitDetailsPage() { + return render( , ); - await waitForBatchedUpdatesWithAct(); - fireEvent.press(screen.getByTestId('mock-confirm-button')); - await waitForBatchedUpdatesWithAct(); +} + +function resetNavigationMocksForSubmitDetailsPageTests() { + jest.mocked(Navigation.getTopmostReportId).mockReturnValue('report-share-1'); + jest.mocked(getIsNarrowLayout).mockReturnValue(true); + jest.mocked(Navigation.getIsFullscreenPreInsertedUnderRHP).mockReturnValue(false); + jest.mocked(Navigation.preInsertFullscreenUnderRHP).mockImplementation(() => { + jest.mocked(Navigation.getIsFullscreenPreInsertedUnderRHP).mockReturnValue(true); + jest.mocked(Navigation.getTopmostReportId).mockReturnValue(SHARED_REPORT_ID); + }); + jest.mocked(Navigation.revealRouteBeforeDismissingModal).mockImplementation((_route: unknown, options?: {afterTransition?: () => void}) => { + options?.afterTransition?.(); + }); + jest.mocked(Navigation.dismissModal).mockImplementation((options?: {afterTransition?: () => void}) => { + options?.afterTransition?.(); + }); } describe('SubmitDetailsPage', () => { @@ -168,11 +257,16 @@ describe('SubmitDetailsPage', () => { beforeEach(async () => { jest.clearAllMocks(); + const actualGetReportOrDraftReport = jest.requireActual('@libs/ReportUtils').getReportOrDraftReport; + jest.mocked(getReportOrDraftReport).mockImplementation(actualGetReportOrDraftReport); + resetNavigationMocksForSubmitDetailsPageTests(); await Onyx.clear(); await waitForBatchedUpdates(); await seedShareState(); }); + // Error #1 — requestMoney and cleanupAndNavigateAfterExpenseCreate must share one optimisticTransactionID + // (not the draft placeholder), or post-submit navigation can miss the created expense. it('threads the same optimisticTransactionID into requestMoney AND cleanupAndNavigateAfterExpenseCreate (V8 contract)', async () => { await renderAndConfirm(); @@ -188,6 +282,8 @@ describe('SubmitDetailsPage', () => { expect(cleanupArg?.transactionID).toBe(requestMoneyArg?.optimisticTransactionID); }); + // Error #2 — share receipts must carry a trace id and log captured/submitted milestones so receipt + // upload issues can be correlated end-to-end in production logs. it('stamps the shared receipt with a trace id and logs the capture and submit milestones with source share', async () => { const logInfoSpy = jest.spyOn(Log, 'info').mockImplementation(() => {}); @@ -216,6 +312,8 @@ describe('SubmitDetailsPage', () => { logInfoSpy.mockRestore(); }); + // Error #3 — HEIC shares must upload the converted JPEG from VALIDATED_FILE_OBJECT, not the raw .heic + // the backend rejects. it('uploads the converted JPEG (not the raw HEIC) when the shared file needed validation', async () => { // A HEIC share: SHARE_TEMP_FILE holds the raw .heic file, VALIDATED_FILE_OBJECT holds the converted JPEG. await act(async () => { @@ -231,6 +329,7 @@ describe('SubmitDetailsPage', () => { expect(readFileArg?.[4]).toBe(CONST.RECEIPT_ALLOWED_FILE_TYPES.JPEG); }); + // Error #4 — confirm must wait for HEIC→JPEG conversion; a fast tap must not upload raw HEIC early. it('does not upload while a share that needs validation is still awaiting its converted file', async () => { // A HEIC share whose conversion has not landed yet: VALIDATED_FILE_OBJECT is empty. await act(async () => { @@ -243,4 +342,183 @@ describe('SubmitDetailsPage', () => { // Confirm must bail out (no raw HEIC uploaded) until the converted file is ready. expect(jest.mocked(readFileAsync)).not.toHaveBeenCalled(); }); + + // Error #5 — wide layout fallback: when destination is not topmost, reveal it via revealRouteBeforeDismissingModal + // and defer navigation to cleanup (shouldNavigate: false) so we do not double-navigate after dismiss. + it('wide layout: reveals destination via revealRouteBeforeDismissingModal when another report is topmost', async () => { + jest.mocked(Navigation.getTopmostReportId).mockReturnValue(undefined); + jest.mocked(getIsNarrowLayout).mockReturnValue(false); + + await renderAndConfirm(); + + expect(Navigation.revealRouteBeforeDismissingModal).toHaveBeenCalledWith( + ROUTES.REPORT_WITH_ID.getRoute(SHARED_REPORT_ID), + expect.objectContaining({afterTransition: expect.any(Function)}), + ); + expect(jest.mocked(cleanupAndNavigateAfterExpenseCreate).mock.calls.at(0)?.[0]?.shouldNavigate).toBe(false); + expect(TrackExpense.requestMoney).toHaveBeenCalled(); + }); + + // Error #5b — narrow layout headline: pre-insert destination, then submit resolves via dismissModal. + it('narrow layout: pre-inserts destination and dismisses modal when another report is topmost', async () => { + jest.mocked(Navigation.getTopmostReportId).mockReturnValue(undefined); + + await renderAndConfirm(); + + expect(Navigation.preInsertFullscreenUnderRHP).toHaveBeenCalledWith(ROUTES.REPORT_WITH_ID.getRoute(SHARED_REPORT_ID)); + expect(Navigation.dismissModal).toHaveBeenCalledWith(expect.objectContaining({afterTransition: expect.any(Function)})); + expect(jest.mocked(cleanupAndNavigateAfterExpenseCreate).mock.calls.at(0)?.[0]?.shouldNavigate).toBe(false); + expect(TrackExpense.requestMoney).toHaveBeenCalled(); + }); + + // Error #6 — when the destination chat is already topmost, submit should skip pre-mount reveal and let + // cleanupAndNavigateAfterExpenseCreate handle navigation. + it('skips pre-mount reveal and passes shouldNavigate true to cleanup when the destination report is already topmost', async () => { + await renderAndConfirm(); + + expect(Navigation.revealRouteBeforeDismissingModal).not.toHaveBeenCalled(); + expect(jest.mocked(cleanupAndNavigateAfterExpenseCreate).mock.calls.at(0)?.[0]?.shouldNavigate).toBe(true); + expect(TrackExpense.requestMoney).toHaveBeenCalled(); + }); + + // Error #7 — pre-mount only runs when the destination report exists in COLLECTION.REPORT (not draft-only), + // otherwise reveal would mount an empty screen behind the share modal. + it('does not pre-mount when the destination report is missing from COLLECTION.REPORT', async () => { + jest.mocked(Navigation.getTopmostReportId).mockReturnValue(undefined); + await act(async () => { + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${SHARED_REPORT_ID}`, null); + }); + + await renderAndConfirm(); + + expect(Navigation.preInsertFullscreenUnderRHP).not.toHaveBeenCalled(); + expect(Navigation.revealRouteBeforeDismissingModal).not.toHaveBeenCalled(); + expect(jest.mocked(cleanupAndNavigateAfterExpenseCreate).mock.calls.at(0)?.[0]?.shouldNavigate).toBe(false); + }); + + // Error #7b — after a missing-destination submit, the optimistic report landing must not arm + // usePreMountDestination (narrow pre-insert) alongside the pending reveal — that dual-nav race + // leaves a stale pre-insert flag under the RHP. + it('does not arm pre-mount after the optimistic destination lands on the pending-navigation path', async () => { + jest.mocked(Navigation.getTopmostReportId).mockReturnValue(undefined); + await act(async () => { + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${SHARED_REPORT_ID}`, null); + }); + + await renderAndConfirm(); + + expect(Navigation.preInsertFullscreenUnderRHP).not.toHaveBeenCalled(); + expect(jest.mocked(cleanupAndNavigateAfterExpenseCreate).mock.calls.at(0)?.[0]?.shouldNavigate).toBe(false); + + // Simulate the expense create writing the destination into COLLECTION.REPORT (requestMoney is mocked). + const landedReport = createTestReport(); + await act(async () => { + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${SHARED_REPORT_ID}`, landedReport); + }); + await waitForBatchedUpdatesWithAct(); + + expect(Navigation.preInsertFullscreenUnderRHP).not.toHaveBeenCalled(); + expect(Navigation.revealRouteBeforeDismissingModal).toHaveBeenCalledTimes(1); + expect(Navigation.revealRouteBeforeDismissingModal).toHaveBeenCalledWith( + ROUTES.REPORT_WITH_ID.getRoute(SHARED_REPORT_ID), + expect.objectContaining({afterTransition: expect.any(Function)}), + ); + }); + + // Error #8 — backing out before submit must tear down any pre-inserted destination route before goBack, + // or the stale route can flash behind the next modal dismiss. + it('cleans up a pre-inserted destination route before goBack when the user backs out without submitting', async () => { + jest.mocked(Navigation.getTopmostReportId).mockReturnValue(undefined); + + renderSubmitDetailsPage(); + await waitForBatchedUpdatesWithAct(); + + expect(Navigation.preInsertFullscreenUnderRHP).toHaveBeenCalledWith(ROUTES.REPORT_WITH_ID.getRoute(SHARED_REPORT_ID)); + + fireEvent.press(screen.getByTestId('mock-back-button')); + await waitForBatchedUpdatesWithAct(); + + expect(Navigation.removePreInsertedFullscreenIfNeeded).toHaveBeenCalled(); + expect(Navigation.goBack).toHaveBeenCalled(); + }); + + // Error #9 — unmounting between formHasBeenSubmitted and reveal() must not crash or leak stale callbacks. + it('handles unmount between submit and reveal without crashing', async () => { + jest.mocked(Navigation.getTopmostReportId).mockReturnValue(undefined); + + let pendingRevealCallback: (() => void) | undefined; + jest.mocked(Navigation.revealRouteBeforeDismissingModal).mockImplementation((_route: unknown, options?: {afterTransition?: () => void}) => { + // Simulate reveal being async—store callback but don't call it yet. + pendingRevealCallback = options?.afterTransition; + }); + + const {unmount} = renderSubmitDetailsPage(); + await waitForBatchedUpdatesWithAct(); + + fireEvent.press(screen.getByTestId('mock-confirm-button')); + await waitForBatchedUpdatesWithAct(); + + // Component unmounts before reveal callback fires. + unmount(); + + // Fire the callback after unmount—should not crash even though component is gone. + expect(() => pendingRevealCallback?.()).not.toThrow(); + }); + + // Error #10 — if optimistic report lands under wrong ID, submit must navigate to intended destination, not stray. + it('navigates to intended reportID even if optimistic report lands under different ID', async () => { + jest.useFakeTimers(); + try { + jest.mocked(Navigation.getTopmostReportId).mockReturnValue(undefined); + await act(async () => { + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${SHARED_REPORT_ID}`, null); + }); + + const wrongReportID = 'report-wrong-id'; + + await renderAndConfirm(); + + // Simulate expense create landing a report under a different ID (e.g., due to conflict or dedupe). + const wrongReport = createTestReport(); + wrongReport.reportID = wrongReportID; + jest.mocked(getReportOrDraftReport).mockReturnValue(wrongReport); + await act(async () => { + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${wrongReportID}`, wrongReport); + }); + await waitForBatchedUpdatesWithAct(); + + // Advance timers to trigger the fallback reveal timeout + jest.advanceTimersByTime(600); + await waitForBatchedUpdatesWithAct(); + + // Should navigate to the ORIGINAL intended report (SHARED_REPORT_ID), not the mismatched one. + expect(Navigation.revealRouteBeforeDismissingModal).toHaveBeenCalledWith( + ROUTES.REPORT_WITH_ID.getRoute(SHARED_REPORT_ID), + expect.objectContaining({afterTransition: expect.any(Function)}), + ); + } finally { + jest.useRealTimers(); + } + }); + + // Error #11 — narrow layout race: confirm fires before scheduleWhenIdle runs pre-insert setup. + // Pre-insert should not happen, but submit must still complete without crashing. + it('narrow layout: handles confirm before scheduleWhenIdle fires pre-insert setup', async () => { + jest.mocked(Navigation.getTopmostReportId).mockReturnValue(undefined); + + // scheduleWhenIdle callback never fires — pre-insert setup won't run + jest.mocked(Scheduler.scheduleWhenIdle).mockImplementation(() => ({cancel: jest.fn()})); + + await renderAndConfirm(); + + // Pre-insert should NOT be called since callback never fired + expect(Navigation.preInsertFullscreenUnderRHP).not.toHaveBeenCalled(); + + // Submit should still execute + expect(TrackExpense.requestMoney).toHaveBeenCalled(); + + // Should fall back to reveal path for wide layout or skip nav (shouldNavigate false) + // depending on how code handles the race — critical is no crash and proper cleanup + expect(jest.mocked(cleanupAndNavigateAfterExpenseCreate).mock.calls.at(0)?.[0]).toBeDefined(); + }); }); diff --git a/tests/unit/cleanupAndNavigateAfterExpenseCreateTest.ts b/tests/unit/cleanupAndNavigateAfterExpenseCreateTest.ts index 7d574256d57d..249229ed7ed4 100644 --- a/tests/unit/cleanupAndNavigateAfterExpenseCreateTest.ts +++ b/tests/unit/cleanupAndNavigateAfterExpenseCreateTest.ts @@ -1,7 +1,9 @@ +import Log from '@libs/Log'; import cleanupAfterExpenseCreate from '@libs/Navigation/helpers/cleanupAfterExpenseCreate'; import cleanupAndNavigateAfterExpenseCreate from '@libs/Navigation/helpers/cleanupAndNavigateAfterExpenseCreate'; import navigateAfterExpenseCreate from '@libs/Navigation/helpers/navigateAfterExpenseCreate'; import {getReportOrDraftReport, isMoneyRequestReport} from '@libs/ReportUtils'; +import {isTracking} from '@libs/telemetry/submitFollowUpAction'; import CONST from '@src/CONST'; import type {Report, ReportAction} from '@src/types/onyx'; @@ -10,6 +12,9 @@ import type {OnyxEntry} from 'react-native-onyx'; jest.mock('@libs/Navigation/helpers/cleanupAfterExpenseCreate', () => jest.fn()); jest.mock('@libs/Navigation/helpers/navigateAfterExpenseCreate', () => jest.fn()); +jest.mock('@libs/telemetry/submitFollowUpAction', () => ({ + isTracking: jest.fn(() => false), +})); jest.mock('@libs/ReportUtils', () => ({ getReportOrDraftReport: jest.fn(), @@ -22,6 +27,7 @@ const expenseReport = {reportID: 'expense-1', chatReportID: 'linked-chat-1'} as describe('cleanupAndNavigateAfterExpenseCreate', () => { beforeEach(() => { jest.clearAllMocks(); + jest.mocked(isTracking).mockReturnValue(false); (isMoneyRequestReport as jest.Mock).mockReturnValue(false); (getReportOrDraftReport as jest.Mock).mockReturnValue(undefined); }); @@ -47,6 +53,43 @@ describe('cleanupAndNavigateAfterExpenseCreate', () => { expect(navigateAfterExpenseCreate).toHaveBeenCalledTimes(1); }); + it('should pass shouldWaitForUpcomingTransition=false when shouldNavigate is false', () => { + cleanupAndNavigateAfterExpenseCreate({ + action: CONST.IOU.ACTION.CREATE, + report: chatReport, + draftTransactionIDs: ['txn-1'], + transactionID: 'txn-1', + isFromGlobalCreate: false, + shouldNavigate: false, + }); + + expect(cleanupAfterExpenseCreate).toHaveBeenCalledWith( + expect.objectContaining({ + shouldWaitForUpcomingTransition: false, + }), + ); + expect(navigateAfterExpenseCreate).toHaveBeenCalledWith(expect.objectContaining({shouldNavigate: false})); + }); + + it('should warn in __DEV__ when shouldNavigate is false but a submit span is still active', () => { + const warnSpy = jest.spyOn(Log, 'warn').mockImplementation(() => {}); + jest.mocked(isTracking).mockReturnValue(true); + + cleanupAndNavigateAfterExpenseCreate({ + action: CONST.IOU.ACTION.CREATE, + report: chatReport, + draftTransactionIDs: ['txn-1'], + transactionID: 'txn-1', + isFromGlobalCreate: false, + shouldNavigate: false, + }); + + expect(warnSpy).toHaveBeenCalledWith( + '[cleanupAndNavigateAfterExpenseCreate] shouldNavigate=false but span is active. Caller must own span lifecycle — miss this and span hangs 60s until dropped.', + ); + warnSpy.mockRestore(); + }); + it('should resolve activeReportID to backToReport when provided', () => { cleanupAndNavigateAfterExpenseCreate({ action: CONST.IOU.ACTION.CREATE, @@ -194,6 +237,29 @@ describe('cleanupAndNavigateAfterExpenseCreate', () => { isInvoice: true, hasMultipleTransactions: false, shouldAddPendingNewTransactionIDs: false, + shouldNavigate: true, + }); + }); + + it('should pass isInvoice, isFromGlobalCreate, and transactionID through to navigateAfterExpenseCreate when shouldNavigate is false', () => { + cleanupAndNavigateAfterExpenseCreate({ + action: CONST.IOU.ACTION.CREATE, + report: chatReport, + draftTransactionIDs: [], + transactionID: 'txn-42', + isFromGlobalCreate: true, + isInvoice: true, + shouldNavigate: false, + }); + + expect(navigateAfterExpenseCreate).toHaveBeenCalledWith({ + activeReportID: 'chat-1', + transactionID: 'txn-42', + isFromGlobalCreate: true, + isInvoice: true, + hasMultipleTransactions: false, + shouldAddPendingNewTransactionIDs: false, + shouldNavigate: false, }); });