From 8bfeb94e37c9bf141ecd86119bc58082295cabd1 Mon Sep 17 00:00:00 2001 From: "daledah (via MelvinBot)" Date: Wed, 21 Oct 2026 07:37:38 +0000 Subject: [PATCH 1/8] Decouple the file download modal from report and Expensify Card logic Move PDFDownloadModal into the modal layer as a presentation-only FileDownloadModal, replace the two domain-specific global wrappers with a single FileDownloadModalWrapper, and move the report and card statement Onyx/download logic into their hooks, which update the open modal in place and never reopen a dismissed one. Co-authored-by: daledah --- src/CONST/index.ts | 5 + ...ExpensifyCardStatementPDFDownloadModal.tsx | 58 ----- .../FileDownloadModal.tsx} | 77 +++--- ...fyCardStatementPDFDownloadModalWrapper.tsx | 34 --- .../Modal/Global/FileDownloadModalWrapper.tsx | 48 ++++ .../Global/ReportPDFDownloadModalWrapper.tsx | 32 --- src/components/MoneyReportHeaderModals.tsx | 23 +- src/components/ReportPDFDownloadModal.tsx | 90 ------- ...eExpensifyCardStatementPDFDownloadModal.ts | 95 +++++++- src/hooks/useReportPDFDownloadModal.ts | 138 ++++++++++- src/libs/actions/IOU/ReportWorkflow.ts | 2 +- .../Search/SearchBulkActionsButtonTest.tsx | 2 +- ...rdStatementPDFDownloadModalWrapperTest.tsx | 59 ----- .../FileDownloadModalWrapperTest.tsx | 125 ++++++++++ .../components/ReportPDFDownloadModalTest.tsx | 77 ------ ...nsifyCardStatementPDFDownloadModalTest.tsx | 214 +++++++++++++++++ .../hooks/useReportPDFDownloadModalTest.tsx | 227 ++++++++++++++++++ .../useSearchBulkActionsDownloadPDFTest.ts | 10 + 18 files changed, 897 insertions(+), 419 deletions(-) delete mode 100644 src/components/ExpensifyCardStatementPDFDownloadModal.tsx rename src/components/{PDFDownloadModal.tsx => Modal/FileDownloadModal.tsx} (69%) delete mode 100644 src/components/Modal/Global/ExpensifyCardStatementPDFDownloadModalWrapper.tsx create mode 100644 src/components/Modal/Global/FileDownloadModalWrapper.tsx delete mode 100644 src/components/Modal/Global/ReportPDFDownloadModalWrapper.tsx delete mode 100644 src/components/ReportPDFDownloadModal.tsx delete mode 100644 tests/unit/components/ExpensifyCardStatementPDFDownloadModalWrapperTest.tsx create mode 100644 tests/unit/components/FileDownloadModalWrapperTest.tsx delete mode 100644 tests/unit/components/ReportPDFDownloadModalTest.tsx create mode 100644 tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx create mode 100644 tests/unit/hooks/useReportPDFDownloadModalTest.tsx diff --git a/src/CONST/index.ts b/src/CONST/index.ts index cd1d7ed0ed85..8306faffc34b 100644 --- a/src/CONST/index.ts +++ b/src/CONST/index.ts @@ -6454,6 +6454,11 @@ const CONST = { ERROR: 'error', INFO: 'info', }, + FILE_DOWNLOAD_STATUS: { + GENERATING: 'generating', + READY: 'ready', + FAILED: 'failed', + }, REPORT_DETAILS_MENU_ITEM: { GO_TO_ROOM: 'goToRoom', MEMBERS: 'member', diff --git a/src/components/ExpensifyCardStatementPDFDownloadModal.tsx b/src/components/ExpensifyCardStatementPDFDownloadModal.tsx deleted file mode 100644 index e53f6b447341..000000000000 --- a/src/components/ExpensifyCardStatementPDFDownloadModal.tsx +++ /dev/null @@ -1,58 +0,0 @@ -import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; -import useEnvironment from '@hooks/useEnvironment'; -import useLocalize from '@hooks/useLocalize'; -import useOnyx from '@hooks/useOnyx'; - -import {downloadExpensifyCardStatementPDF} from '@libs/ExpensifyCardStatementUtils'; -import type {ExpensifyCardStatementParams} from '@libs/ExpensifyCardStatementUtils'; - -import ONYXKEYS from '@src/ONYXKEYS'; - -import React from 'react'; - -import PDFDownloadModal from './PDFDownloadModal'; - -type ExpensifyCardStatementPDFDownloadModalProps = { - statementParams: ExpensifyCardStatementParams; - isVisible: boolean; - onClose: () => void; - onModalHide?: () => void; -}; - -function ExpensifyCardStatementPDFDownloadModal({statementParams, isVisible, onClose, onModalHide}: ExpensifyCardStatementPDFDownloadModalProps) { - const [expensifyCardStatement] = useOnyx(ONYXKEYS.EXPENSIFY_CARD_STATEMENT); - const [session] = useOnyx(ONYXKEYS.SESSION); - const {translate} = useLocalize(); - const {environment} = useEnvironment(); - const currentUserPersonalDetails = useCurrentUserPersonalDetails(); - const currentUserLogin = currentUserPersonalDetails?.login ?? ''; - const encryptedAuthToken = session?.encryptedAuthToken ?? ''; - - const statementKey = statementParams.statementKey; - const isGeneratingPDF = expensifyCardStatement?.isGenerating === true; - const statementFileName = statementKey ? expensifyCardStatement?.[statementKey] : undefined; - const hasFinishedPDFDownload = !!statementKey && !isGeneratingPDF && typeof statementFileName === 'string' && statementFileName.length > 0; - const message = hasFinishedPDFDownload ? translate('reportDetailsPage.successPDF') : translate('reportDetailsPage.waitForPDF'); - - return ( - { - if (!statementKey || typeof statementFileName !== 'string') { - return; - } - downloadExpensifyCardStatementPDF(translate, statementFileName, statementKey, currentUserLogin, encryptedAuthToken, environment); - }} - /> - ); -} - -ExpensifyCardStatementPDFDownloadModal.displayName = 'ExpensifyCardStatementPDFDownloadModal'; - -export default ExpensifyCardStatementPDFDownloadModal; diff --git a/src/components/PDFDownloadModal.tsx b/src/components/Modal/FileDownloadModal.tsx similarity index 69% rename from src/components/PDFDownloadModal.tsx rename to src/components/Modal/FileDownloadModal.tsx index 3de46f8088b3..c5cc898d72f1 100644 --- a/src/components/PDFDownloadModal.tsx +++ b/src/components/Modal/FileDownloadModal.tsx @@ -1,3 +1,10 @@ +import ActivityIndicator from '@components/ActivityIndicator'; +import Button from '@components/Button'; +import HeaderTitle from '@components/HeaderTitle'; +import Icon from '@components/Icon'; +import {PressableWithFeedback} from '@components/Pressable'; +import Text from '@components/Text'; + import useBottomSafeSafeAreaPaddingStyle from '@hooks/useBottomSafeSafeAreaPaddingStyle'; import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset'; import useLocalize from '@hooks/useLocalize'; @@ -7,34 +14,29 @@ import useThemeStyles from '@hooks/useThemeStyles'; import CONST from '@src/CONST'; +import type {ValueOf} from 'type-fest'; + import React, {useEffect, useRef} from 'react'; import {View} from 'react-native'; -import ActivityIndicator from './ActivityIndicator'; -import Button from './Button'; -import HeaderTitle from './HeaderTitle'; -import Icon from './Icon'; -import Modal from './Modal'; -import {PressableWithFeedback} from './Pressable'; -import Text from './Text'; +import Modal from '.'; -type PDFDownloadModalProps = { - /** Whether the PDF has finished generating and is ready to download */ - hasFinishedPDFDownload: boolean; +type FileDownloadStatus = ValueOf; - /** Whether the PDF generation failed, so there is nothing left to wait for or download */ - hasPDFDownloadFailed?: boolean; +type FileDownloadModalProps = { + /** Whether the file is still generating, ready to download, or failed to generate */ + status: FileDownloadStatus; /** Message shown under the header (generating / ready / error copy) */ message: string; - /** Downloads the generated PDF; called on auto-download and on the download button press */ - onDownloadPDF: () => void; + /** Downloads the generated file; called on auto-download and on the download button press */ + onDownload: () => void; /** Whether pressing the download button also closes the modal */ shouldCloseOnDownload?: boolean; - /** Whether the download button uses the success (green) style once the PDF is ready */ + /** Whether the download button uses the success (green) style once the file is ready */ shouldUseSuccessButton?: boolean; isVisible: boolean; @@ -50,23 +52,22 @@ type PDFDownloadModalProps = { }; /** - * Shared modal for OldDot-generated PDF downloads (report PDFs, Expensify Card statements, etc.). It shows a - * generating/ready state, auto-downloads once generation finishes while the modal is open, and offers a manual - * download button. Callers supply the status, copy, and download action for their specific PDF. + * Shared modal for server-generated file downloads (report PDFs, Expensify Card statements, etc.). It shows a + * generating/ready/failed state, auto-downloads once generation finishes while the modal is open, and offers a manual + * download button. Callers supply the status, copy, and download action for their specific file. */ -function PDFDownloadModal({ - hasFinishedPDFDownload, - hasPDFDownloadFailed = false, +function FileDownloadModal({ + status, message, - onDownloadPDF, + onDownload, shouldCloseOnDownload = false, shouldUseSuccessButton = false, isVisible, shouldTreatModalAsCovering = false, onClose, onModalHide, -}: PDFDownloadModalProps) { - const shouldAutoDownloadPDF = useRef(false); +}: FileDownloadModalProps) { + const shouldAutoDownload = useRef(false); const {translate} = useLocalize(); // We need to use isSmallScreenWidth here because the Modal breaks in RHP with shouldUseNarrowLayout. @@ -76,17 +77,20 @@ function PDFDownloadModal({ const theme = useTheme(); const expensifyIcons = useMemoizedLazyExpensifyIcons(['Close']); + const isReady = status === CONST.FILE_DOWNLOAD_STATUS.READY; + const hasFailed = status === CONST.FILE_DOWNLOAD_STATUS.FAILED; + useEffect(() => { - shouldAutoDownloadPDF.current = isVisible; + shouldAutoDownload.current = isVisible; }, [isVisible]); useEffect(() => { - if (!isVisible || !shouldAutoDownloadPDF.current || !hasFinishedPDFDownload) { + if (!isVisible || !shouldAutoDownload.current || !isReady) { return; } - onDownloadPDF(); - shouldAutoDownloadPDF.current = false; - }, [hasFinishedPDFDownload, isVisible, onDownloadPDF]); + onDownload(); + shouldAutoDownload.current = false; + }, [isReady, isVisible, onDownload]); const bottomSafeAreaPaddingStyle = useBottomSafeSafeAreaPaddingStyle({ addBottomSafeAreaPadding: isSmallScreenWidth, @@ -116,7 +120,7 @@ function PDFDownloadModal({ {message} - {!hasFinishedPDFDownload && !hasPDFDownloadFailed && ( + {status === CONST.FILE_DOWNLOAD_STATUS.GENERATING && ( setIsVisible(false)} - onModalHide={() => { - if (isVisible) { - return; - } - closeModal({action: ModalActions.CLOSE}); - }} - /> - ); -} - -export default ExpensifyCardStatementPDFDownloadModalWrapper; -export type {ExpensifyCardStatementPDFDownloadModalWrapperProps}; diff --git a/src/components/Modal/Global/FileDownloadModalWrapper.tsx b/src/components/Modal/Global/FileDownloadModalWrapper.tsx new file mode 100644 index 000000000000..91984f14f454 --- /dev/null +++ b/src/components/Modal/Global/FileDownloadModalWrapper.tsx @@ -0,0 +1,48 @@ +import type {FileDownloadModalProps} from '@components/Modal/FileDownloadModal'; +import FileDownloadModal from '@components/Modal/FileDownloadModal'; + +import React, {useRef, useState} from 'react'; + +import type {ModalProps} from './ModalContext'; + +import {ModalActions} from './ModalContext'; + +type FileDownloadModalWrapperProps = ModalProps & + Omit & { + /** Called when the user dismisses the modal, before it starts hiding */ + onClose?: () => void; + }; + +function FileDownloadModalWrapper({closeModal, onClose, ...props}: FileDownloadModalWrapperProps) { + const [isVisible, setIsVisible] = useState(true); + const hasClosedModalRef = useRef(false); + + const handleClose = () => { + onClose?.(); + setIsVisible(false); + }; + + // Remove the modal from the stack only once it has finished hiding, so callers awaiting the show promise run + // their cleanup after the modal is gone. + const handleModalHide = () => { + if (isVisible || hasClosedModalRef.current) { + return; + } + hasClosedModalRef.current = true; + closeModal({action: ModalActions.CLOSE}); + }; + + return ( + + ); +} + +export default FileDownloadModalWrapper; +export type {FileDownloadModalWrapperProps}; diff --git a/src/components/Modal/Global/ReportPDFDownloadModalWrapper.tsx b/src/components/Modal/Global/ReportPDFDownloadModalWrapper.tsx deleted file mode 100644 index 8fc1ce4b33b4..000000000000 --- a/src/components/Modal/Global/ReportPDFDownloadModalWrapper.tsx +++ /dev/null @@ -1,32 +0,0 @@ -import ReportPDFDownloadModal from '@components/ReportPDFDownloadModal'; - -import React, {useState} from 'react'; - -import type {ModalProps} from './ModalContext'; - -import {ModalActions} from './ModalContext'; - -type ReportPDFDownloadModalWrapperProps = ModalProps & { - reportID: string; -}; - -function ReportPDFDownloadModalWrapper({closeModal, reportID}: ReportPDFDownloadModalWrapperProps) { - const [isVisible, setIsVisible] = useState(true); - - return ( - setIsVisible(false)} - onModalHide={() => { - if (isVisible) { - return; - } - closeModal({action: ModalActions.CLOSE}); - }} - /> - ); -} - -export default ReportPDFDownloadModalWrapper; -export type {ReportPDFDownloadModalWrapperProps}; diff --git a/src/components/MoneyReportHeaderModals.tsx b/src/components/MoneyReportHeaderModals.tsx index 62902b02bd40..c24f974ca87b 100644 --- a/src/components/MoneyReportHeaderModals.tsx +++ b/src/components/MoneyReportHeaderModals.tsx @@ -4,6 +4,7 @@ import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails' import useHoldMenuModal from '@hooks/useHoldMenuModal'; import useOnyx from '@hooks/useOnyx'; import useReportIsArchived from '@hooks/useReportIsArchived'; +import useReportPDFDownloadModal from '@hooks/useReportPDFDownloadModal'; import useTransactionsAndViolationsForReport from '@hooks/useTransactionsAndViolationsForReport'; import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; @@ -15,7 +16,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; import type {ReactNode} from 'react'; -import React, {useRef, useState} from 'react'; +import React, {useRef} from 'react'; import type {MoneyReportHeaderEducationalModalsHandle, RejectModalAction} from './MoneyReportHeaderEducationalModals'; import type {HoldMenuParams} from './MoneyReportHeaderModalsContext'; @@ -23,7 +24,6 @@ import type {HoldMenuParams} from './MoneyReportHeaderModalsContext'; import MoneyReportHeaderEducationalModals from './MoneyReportHeaderEducationalModals'; import MoneyReportHeaderModalsContext from './MoneyReportHeaderModalsContext'; import {MoneyReportTransactionThreadProvider} from './MoneyReportTransactionThreadContext'; -import ReportPDFDownloadModal from './ReportPDFDownloadModal'; type MoneyReportHeaderModalsProps = { reportID: string | undefined; @@ -31,11 +31,6 @@ type MoneyReportHeaderModalsProps = { }; function MoneyReportHeaderModals({reportID, children}: MoneyReportHeaderModalsProps) { - // PDF modal state - const [isPDFModalVisible, setIsPDFModalVisible] = useState(false); - // Callback invoked when the PDF modal is dismissed while still generating (e.g. Submit via PDF retracts the submit). - const onPDFCancelRef = useRef<(() => void) | undefined>(undefined); - // Educational modals ref const educationalModalsRef = useRef(null); @@ -63,6 +58,7 @@ function MoneyReportHeaderModals({reportID, children}: MoneyReportHeaderModalsPr // Imperative modals const {showHoldMenu} = useHoldMenuModal(); const {showOfflineModal, showDownloadErrorModal} = useAlertModals(); + const {showReportPDFDownloadModal} = useReportPDFDownloadModal(); const openHoldMenu = ({requestType, paymentType, methodID, onConfirm}: HoldMenuParams): Promise => { const open = () => @@ -94,8 +90,10 @@ function MoneyReportHeaderModals({reportID, children}: MoneyReportHeaderModalsPr const contextValue = { openHoldMenu, openPDFDownload: (options?: {onCancel?: () => void}) => { - onPDFCancelRef.current = options?.onCancel; - setIsPDFModalVisible(true); + if (!moneyRequestReport?.reportID) { + return; + } + showReportPDFDownloadModal({reportID: moneyRequestReport.reportID, onCancel: options?.onCancel}); }, openHoldEducational: () => educationalModalsRef.current?.openHoldEducational(), openRejectModal: (action: RejectModalAction) => educationalModalsRef.current?.openRejectModal(action), @@ -112,13 +110,6 @@ function MoneyReportHeaderModals({reportID, children}: MoneyReportHeaderModalsPr ref={educationalModalsRef} reportID={moneyRequestReport?.reportID} /> - - setIsPDFModalVisible(false)} - onCancel={() => onPDFCancelRef.current?.()} - /> ); diff --git a/src/components/ReportPDFDownloadModal.tsx b/src/components/ReportPDFDownloadModal.tsx deleted file mode 100644 index 4b84f2c07186..000000000000 --- a/src/components/ReportPDFDownloadModal.tsx +++ /dev/null @@ -1,90 +0,0 @@ -import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; -import useLocalize from '@hooks/useLocalize'; -import useNetwork from '@hooks/useNetwork'; -import useOnyx from '@hooks/useOnyx'; -import useStallLogger from '@hooks/useStallLogger'; - -import {downloadReportPDF} from '@libs/actions/Report'; - -import CONST from '@src/CONST'; -import ONYXKEYS from '@src/ONYXKEYS'; - -import React from 'react'; - -import PDFDownloadModal from './PDFDownloadModal'; - -type ReportPDFDownloadModalProps = { - reportID: string | undefined; - isVisible: boolean; - onClose: () => void; - onModalHide?: () => void; - - /** Called when the modal is dismissed while the PDF is still generating (e.g. Submit via PDF retracts the submit). */ - onCancel?: () => void; -}; - -function ReportPDFDownloadModal({reportID, isVisible, onClose, onModalHide, onCancel}: ReportPDFDownloadModalProps) { - const [reportPDFFilename] = useOnyx(`${ONYXKEYS.COLLECTION.NVP_EXPENSIFY_REPORT_PDF_FILENAME}${reportID}`); - const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`); - const [session] = useOnyx(ONYXKEYS.SESSION); - - const {translate} = useLocalize(); - const currentUserPersonalDetails = useCurrentUserPersonalDetails(); - const currentUserLogin = currentUserPersonalDetails?.login ?? ''; - const encryptedAuthToken = session?.encryptedAuthToken ?? ''; - const reportName = report?.reportName ?? ''; - const {isOffline} = useNetwork(); - - const hasPDFDownloadFailed = reportPDFFilename === CONST.REPORT_DETAILS_MENU_ITEM.ERROR; - const hasFinishedPDFDownload = !!reportPDFFilename && !hasPDFDownloadFailed; - - // reportPDFFilename only ever resolves via a backend-pushed Onyx update (no client timeout), so a filename - // that never arrives and never errors leaves the spinner stuck with nothing else to log it. See Expensify#667674. - // Excluded while offline: the request is legitimately queued and waiting for connectivity, not stuck. - // Keyed by reportID (not just a boolean) so if this modal is ever reused for a different report without - // unmounting, the timer restarts against the new report instead of firing late and blaming the wrong one. - useStallLogger(isVisible && !isOffline && !reportPDFFilename ? reportID : false, '[PDFStall] reportPDFFilename never resolved to a filename or error while the download modal was open', { - reportID, - }); - - const message = (() => { - if (hasPDFDownloadFailed) { - return translate('reportDetailsPage.errorPDF'); - } - if (!hasFinishedPDFDownload) { - return translate('reportDetailsPage.waitForPDF'); - } - return translate('reportDetailsPage.successPDF'); - })(); - - // reportPDFFilename is null while the backend is still generating, so a dismissal at that point is a cancel - // (e.g. Submit via PDF retracts the submit); once it holds a filename or 'error' the submit has already resolved. - const handleClose = () => { - if (!reportPDFFilename) { - onCancel?.(); - } - onClose(); - }; - - return ( - { - if (!hasFinishedPDFDownload) { - return; - } - downloadReportPDF(reportPDFFilename, reportName, translate, currentUserLogin, encryptedAuthToken); - }} - /> - ); -} - -ReportPDFDownloadModal.displayName = 'ReportPDFDownloadModal'; - -export default ReportPDFDownloadModal; diff --git a/src/hooks/useExpensifyCardStatementPDFDownloadModal.ts b/src/hooks/useExpensifyCardStatementPDFDownloadModal.ts index cfbb727770e5..5d98cbf1e3bb 100644 --- a/src/hooks/useExpensifyCardStatementPDFDownloadModal.ts +++ b/src/hooks/useExpensifyCardStatementPDFDownloadModal.ts @@ -1,9 +1,24 @@ -import type {ExpensifyCardStatementPDFDownloadModalWrapperProps} from '@components/Modal/Global/ExpensifyCardStatementPDFDownloadModalWrapper'; -import ExpensifyCardStatementPDFDownloadModalWrapper from '@components/Modal/Global/ExpensifyCardStatementPDFDownloadModalWrapper'; +import type {FileDownloadModalWrapperProps} from '@components/Modal/Global/FileDownloadModalWrapper'; +import FileDownloadModalWrapper from '@components/Modal/Global/FileDownloadModalWrapper'; import type {ModalProps} from '@components/Modal/Global/ModalContext'; import {useModal} from '@components/Modal/Global/ModalContext'; -type ExpensifyCardStatementPDFDownloadModalOptions = Omit; +import {downloadExpensifyCardStatementPDF} from '@libs/ExpensifyCardStatementUtils'; +import type {ExpensifyCardStatementParams} from '@libs/ExpensifyCardStatementUtils'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; + +import {useEffect, useEffectEvent, useRef, useState} from 'react'; + +import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails'; +import useEnvironment from './useEnvironment'; +import useLocalize from './useLocalize'; +import useOnyx from './useOnyx'; + +type ExpensifyCardStatementPDFDownloadModalOptions = { + statementParams: ExpensifyCardStatementParams; +}; // A fixed id lets a second show call update the open modal in place, which is how the statementKey reaches it // once GetExpensifyCardStatementPDF returns. @@ -11,16 +26,80 @@ const EXPENSIFY_CARD_STATEMENT_PDF_DOWNLOAD_MODAL_ID = 'expensifyCardStatementPD const useExpensifyCardStatementPDFDownloadModal = () => { const context = useModal(); + const [statementParams, setStatementParams] = useState(); + + // Whether the modal is open. It turns false as soon as the modal is dismissed, so a late update can't show it + // again: a show call with the fixed id after the modal has closed pushes a new modal instead of updating it. + const isOpenRef = useRef(false); + + // Resolves once the modal has finished hiding. Set from the first show call until then. + const modalPromiseRef = useRef | undefined>(undefined); + + const [expensifyCardStatement] = useOnyx(ONYXKEYS.EXPENSIFY_CARD_STATEMENT); + const [session] = useOnyx(ONYXKEYS.SESSION); + const {translate} = useLocalize(); + const {environment} = useEnvironment(); + const currentUserPersonalDetails = useCurrentUserPersonalDetails(); + const currentUserLogin = currentUserPersonalDetails?.login ?? ''; + const encryptedAuthToken = session?.encryptedAuthToken ?? ''; + + const statementKey = statementParams?.statementKey; + const isGeneratingPDF = expensifyCardStatement?.isGenerating === true; + const statementFileName = statementKey ? expensifyCardStatement?.[statementKey] : undefined; + const hasFinishedPDFDownload = !!statementKey && !isGeneratingPDF && typeof statementFileName === 'string' && statementFileName.length > 0; + const message = hasFinishedPDFDownload ? translate('reportDetailsPage.successPDF') : translate('reportDetailsPage.waitForPDF'); + + const getModalProps = (): Omit => ({ + status: hasFinishedPDFDownload ? CONST.FILE_DOWNLOAD_STATUS.READY : CONST.FILE_DOWNLOAD_STATUS.GENERATING, + message, + shouldCloseOnDownload: true, + shouldUseSuccessButton: true, + onDownload: () => { + if (!statementKey || typeof statementFileName !== 'string') { + return; + } + downloadExpensifyCardStatementPDF(translate, statementFileName, statementKey, currentUserLogin, encryptedAuthToken, environment); + }, + onClose: () => { + isOpenRef.current = false; + }, + }); + + const updateOpenModal = useEffectEvent(() => { + if (!isOpenRef.current) { + return; + } + context.showModal({component: FileDownloadModalWrapper, id: EXPENSIFY_CARD_STATEMENT_PDF_DOWNLOAD_MODAL_ID, props: getModalProps()}); + }); + + // Push the latest generation state into the open modal so it stays reactive without remounting. + useEffect(() => { + updateOpenModal(); + }, [statementKey, statementFileName, message, currentUserLogin, encryptedAuthToken, environment]); const showExpensifyCardStatementPDFDownloadModal = (options: ExpensifyCardStatementPDFDownloadModalOptions) => { - return context.showModal({ - component: ExpensifyCardStatementPDFDownloadModalWrapper, - id: EXPENSIFY_CARD_STATEMENT_PDF_DOWNLOAD_MODAL_ID, - props: options, + setStatementParams(options.statementParams); + + // The modal is still on the stack (open or hiding), so the effect above updates it in place while it is open. + if (modalPromiseRef.current) { + return modalPromiseRef.current; + } + + isOpenRef.current = true; + const modalPromise = context.showModal({component: FileDownloadModalWrapper, id: EXPENSIFY_CARD_STATEMENT_PDF_DOWNLOAD_MODAL_ID, props: getModalProps()}).then((result) => { + isOpenRef.current = false; + modalPromiseRef.current = undefined; + setStatementParams(undefined); + return result; }); + modalPromiseRef.current = modalPromise; + return modalPromise; }; - const closeExpensifyCardStatementPDFDownloadModal = () => context.closeModalByID(EXPENSIFY_CARD_STATEMENT_PDF_DOWNLOAD_MODAL_ID); + const closeExpensifyCardStatementPDFDownloadModal = () => { + isOpenRef.current = false; + context.closeModalByID(EXPENSIFY_CARD_STATEMENT_PDF_DOWNLOAD_MODAL_ID); + }; return {showExpensifyCardStatementPDFDownloadModal, closeExpensifyCardStatementPDFDownloadModal}; }; diff --git a/src/hooks/useReportPDFDownloadModal.ts b/src/hooks/useReportPDFDownloadModal.ts index ea08357b1b47..023f917e68d4 100644 --- a/src/hooks/useReportPDFDownloadModal.ts +++ b/src/hooks/useReportPDFDownloadModal.ts @@ -1,18 +1,142 @@ +import type {FileDownloadModalWrapperProps} from '@components/Modal/Global/FileDownloadModalWrapper'; +import FileDownloadModalWrapper from '@components/Modal/Global/FileDownloadModalWrapper'; import type {ModalProps} from '@components/Modal/Global/ModalContext'; import {useModal} from '@components/Modal/Global/ModalContext'; -import type {ReportPDFDownloadModalWrapperProps} from '@components/Modal/Global/ReportPDFDownloadModalWrapper'; -import ReportPDFDownloadModalWrapper from '@components/Modal/Global/ReportPDFDownloadModalWrapper'; -type ReportPDFDownloadModalOptions = Omit; +import {downloadReportPDF} from '@libs/actions/Report'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; + +import {useEffect, useEffectEvent, useRef, useState} from 'react'; + +import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails'; +import useLocalize from './useLocalize'; +import useNetwork from './useNetwork'; +import useOnyx from './useOnyx'; +import useStallLogger from './useStallLogger'; + +type ReportPDFDownloadModalOptions = { + reportID: string; + + /** Called when the modal is dismissed while the PDF is still generating (e.g. Submit via PDF retracts the submit). */ + onCancel?: () => void; +}; + +// A fixed id lets the hook show the open modal again with fresh props, which updates it in place as the PDF +// filename arrives instead of stacking a new modal. +const REPORT_PDF_DOWNLOAD_MODAL_ID = 'reportPDFDownloadModal'; const useReportPDFDownloadModal = () => { const context = useModal(); + const [activeReportID, setActiveReportID] = useState(); + + // Whether the modal is open. It turns false as soon as the user dismisses the modal, so a late update can't show + // it again: a show call with the fixed id after the modal has closed pushes a new modal instead of updating it. + const isOpenRef = useRef(false); + + // Resolves once the modal has finished hiding. Set from the first show call until then. + const modalPromiseRef = useRef | undefined>(undefined); + const onCancelRef = useRef<(() => void) | undefined>(undefined); + + const [reportPDFFilename] = useOnyx(`${ONYXKEYS.COLLECTION.NVP_EXPENSIFY_REPORT_PDF_FILENAME}${activeReportID}`); + const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${activeReportID}`); + const [session] = useOnyx(ONYXKEYS.SESSION); + + const {translate} = useLocalize(); + const currentUserPersonalDetails = useCurrentUserPersonalDetails(); + const currentUserLogin = currentUserPersonalDetails?.login ?? ''; + const encryptedAuthToken = session?.encryptedAuthToken ?? ''; + const reportName = report?.reportName ?? ''; + const {isOffline} = useNetwork(); + + const hasPDFDownloadFailed = reportPDFFilename === CONST.REPORT_DETAILS_MENU_ITEM.ERROR; + const hasFinishedPDFDownload = !!reportPDFFilename && !hasPDFDownloadFailed; + + // reportPDFFilename only ever resolves via a backend-pushed Onyx update (no client timeout), so a filename + // that never arrives and never errors leaves the spinner stuck with nothing else to log it. See Expensify#667674. + // Excluded while offline: the request is legitimately queued and waiting for connectivity, not stuck. + // Keyed by reportID (not just a boolean) so if the modal is shown for a different report, the timer restarts + // against the new report instead of firing late and blaming the wrong one. + useStallLogger( + !!activeReportID && !isOffline && !reportPDFFilename ? activeReportID : false, + '[PDFStall] reportPDFFilename never resolved to a filename or error while the download modal was open', + { + reportID: activeReportID, + }, + ); + + const message = (() => { + if (hasPDFDownloadFailed) { + return translate('reportDetailsPage.errorPDF'); + } + if (!hasFinishedPDFDownload) { + return translate('reportDetailsPage.waitForPDF'); + } + return translate('reportDetailsPage.successPDF'); + })(); + + const getStatus = () => { + if (hasPDFDownloadFailed) { + return CONST.FILE_DOWNLOAD_STATUS.FAILED; + } + return hasFinishedPDFDownload ? CONST.FILE_DOWNLOAD_STATUS.READY : CONST.FILE_DOWNLOAD_STATUS.GENERATING; + }; + + const getModalProps = (): Omit => ({ + status: getStatus(), + message, + shouldTreatModalAsCovering: true, + onDownload: () => { + if (!hasFinishedPDFDownload) { + return; + } + downloadReportPDF(reportPDFFilename, reportName, translate, currentUserLogin, encryptedAuthToken); + }, + // reportPDFFilename is null while the backend is still generating, so a dismissal at that point is a cancel + // (e.g. Submit via PDF retracts the submit); once it holds a filename or 'error' the submit has already resolved. + onClose: () => { + if (!isOpenRef.current) { + return; + } + isOpenRef.current = false; + if (!reportPDFFilename) { + onCancelRef.current?.(); + } + }, + }); + + const updateOpenModal = useEffectEvent(() => { + if (!isOpenRef.current) { + return; + } + context.showModal({component: FileDownloadModalWrapper, id: REPORT_PDF_DOWNLOAD_MODAL_ID, props: getModalProps()}); + }); + + // Push the latest generation state into the open modal so it stays reactive without remounting. + useEffect(() => { + updateOpenModal(); + }, [activeReportID, reportPDFFilename, reportName, message, currentUserLogin, encryptedAuthToken]); + + const showReportPDFDownloadModal = ({reportID, onCancel}: ReportPDFDownloadModalOptions) => { + onCancelRef.current = onCancel; + setActiveReportID(reportID); + + // The modal is still on the stack (open or hiding), so the effect above updates it in place while it is open. + if (modalPromiseRef.current) { + return modalPromiseRef.current; + } - const showReportPDFDownloadModal = (options: ReportPDFDownloadModalOptions) => { - return context.showModal({ - component: ReportPDFDownloadModalWrapper, - props: options, + isOpenRef.current = true; + const modalPromise = context.showModal({component: FileDownloadModalWrapper, id: REPORT_PDF_DOWNLOAD_MODAL_ID, props: getModalProps()}).then((result) => { + isOpenRef.current = false; + modalPromiseRef.current = undefined; + onCancelRef.current = undefined; + setActiveReportID(undefined); + return result; }); + modalPromiseRef.current = modalPromise; + return modalPromise; }; return {showReportPDFDownloadModal}; diff --git a/src/libs/actions/IOU/ReportWorkflow.ts b/src/libs/actions/IOU/ReportWorkflow.ts index 2225dab6fa7c..02c0cc8c780a 100644 --- a/src/libs/actions/IOU/ReportWorkflow.ts +++ b/src/libs/actions/IOU/ReportWorkflow.ts @@ -1312,7 +1312,7 @@ function submitReport({ // Submit via PDF: on a Submit workspace where the submitter submits to themselves, the backend generates the // report PDF and writes its filename into nvp_expensify_report_PDFFilename_{reportID}. Prime that NVP the same way - // exportReportToPDF does so the ReportPDFDownloadModal shows progress and auto-downloads once the filename arrives. + // exportReportToPDF does so the report PDF download modal shows progress and auto-downloads once the filename arrives. if (shouldExportToPDF) { optimisticData.push({ onyxMethod: Onyx.METHOD.SET, diff --git a/tests/unit/Search/SearchBulkActionsButtonTest.tsx b/tests/unit/Search/SearchBulkActionsButtonTest.tsx index 12ef7024f21c..4b55b3a0dbc1 100644 --- a/tests/unit/Search/SearchBulkActionsButtonTest.tsx +++ b/tests/unit/Search/SearchBulkActionsButtonTest.tsx @@ -41,7 +41,7 @@ jest.mock('@components/ButtonWithDropdownMenu', () => ({ })); jest.mock('@components/HoldOrRejectEducationalModal', () => () => null); jest.mock('@components/HoldSubmitterEducationalModal', () => () => null); -jest.mock('@components/ReportPDFDownloadModal', () => () => null); +jest.mock('@components/Modal/FileDownloadModal', () => () => null); jest.mock('@components/KYCWall', () => ({ __esModule: true, default: ({children}: {children: (triggerKYCFlow: jest.Mock, buttonRef: React.RefObject) => React.ReactNode}) => children(jest.fn(), {current: null}), diff --git a/tests/unit/components/ExpensifyCardStatementPDFDownloadModalWrapperTest.tsx b/tests/unit/components/ExpensifyCardStatementPDFDownloadModalWrapperTest.tsx deleted file mode 100644 index e28cfd6b6ace..000000000000 --- a/tests/unit/components/ExpensifyCardStatementPDFDownloadModalWrapperTest.tsx +++ /dev/null @@ -1,59 +0,0 @@ -import {act, render} from '@testing-library/react-native'; - -import ExpensifyCardStatementPDFDownloadModal from '@components/ExpensifyCardStatementPDFDownloadModal'; -import ExpensifyCardStatementPDFDownloadModalWrapper from '@components/Modal/Global/ExpensifyCardStatementPDFDownloadModalWrapper'; -import {ModalActions} from '@components/Modal/Global/ModalContext'; - -import React from 'react'; - -jest.mock('@components/ExpensifyCardStatementPDFDownloadModal', () => jest.fn(() => null)); - -const STATEMENT_PARAMS = {policyID: 'policy1', feedCountry: 'US', entryIDs: [123], statementKey: 'statement-key'}; - -describe('ExpensifyCardStatementPDFDownloadModalWrapper', () => { - const mockedModal = jest.mocked(ExpensifyCardStatementPDFDownloadModal); - const getLatestModalProps = () => mockedModal.mock.calls.at(-1)?.[0]; - - beforeEach(() => { - jest.clearAllMocks(); - }); - - it('removes itself from the modal stack only after the modal has finished hiding', () => { - // Given the wrapper rendered by the global modal stack - const closeModal = jest.fn(); - render( - , - ); - - // Then it opens the statement modal with the params passed through the show API - expect(getLatestModalProps()?.isVisible).toBe(true); - expect(getLatestModalProps()?.statementParams).toEqual(STATEMENT_PARAMS); - - // When a hide event fires while the modal is still visible - act(() => { - getLatestModalProps()?.onModalHide?.(); - }); - - // Then the wrapper stays on the stack - expect(closeModal).not.toHaveBeenCalled(); - - // When the user closes the modal - act(() => { - getLatestModalProps()?.onClose(); - }); - - // Then it starts hiding but is not removed until the hide animation ends, so callers awaiting the show - // promise run their cleanup after the modal is gone - expect(getLatestModalProps()?.isVisible).toBe(false); - expect(closeModal).not.toHaveBeenCalled(); - - act(() => { - getLatestModalProps()?.onModalHide?.(); - }); - expect(closeModal).toHaveBeenCalledWith({action: ModalActions.CLOSE}); - }); -}); diff --git a/tests/unit/components/FileDownloadModalWrapperTest.tsx b/tests/unit/components/FileDownloadModalWrapperTest.tsx new file mode 100644 index 000000000000..5126f4c2b277 --- /dev/null +++ b/tests/unit/components/FileDownloadModalWrapperTest.tsx @@ -0,0 +1,125 @@ +import {act, fireEvent, render, screen} from '@testing-library/react-native'; + +import ActivityIndicator from '@components/ActivityIndicator'; +import Modal from '@components/Modal'; +import type {FileDownloadModalWrapperProps} from '@components/Modal/Global/FileDownloadModalWrapper'; +import FileDownloadModalWrapper from '@components/Modal/Global/FileDownloadModalWrapper'; +import {ModalActions} from '@components/Modal/Global/ModalContext'; + +import CONST from '@src/CONST'; + +import React from 'react'; + +jest.mock('@components/ActivityIndicator', () => jest.fn(() => null)); +jest.mock('@components/Modal', () => jest.fn(({children}: {children: React.ReactNode}) => children)); +jest.mock('@hooks/useLocalize', () => jest.fn(() => ({translate: (key: string) => key}))); + +describe('FileDownloadModalWrapper', () => { + const mockedModal = jest.mocked(Modal); + const mockedActivityIndicator = jest.mocked(ActivityIndicator); + const getLatestModalProps = () => mockedModal.mock.calls.at(-1)?.[0]; + + const getWrapperElement = (props: Partial = {}) => ( + + ); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('removes itself from the modal stack once, and only after the modal has finished hiding', () => { + // Given the wrapper rendered by the global modal stack + const closeModal = jest.fn(); + const onClose = jest.fn(); + render(getWrapperElement({closeModal, onClose})); + expect(getLatestModalProps()?.isVisible).toBe(true); + + // When a hide event fires while the modal is still visible + act(() => { + getLatestModalProps()?.onModalHide?.(); + }); + + // Then the wrapper stays on the stack + expect(closeModal).not.toHaveBeenCalled(); + + // When the user closes the modal + act(() => { + getLatestModalProps()?.onClose?.(); + }); + + // Then the caller is told right away, and the modal starts hiding but is not removed until the hide animation + // ends, so callers awaiting the show promise run their cleanup after the modal is gone + expect(onClose).toHaveBeenCalledTimes(1); + expect(getLatestModalProps()?.isVisible).toBe(false); + expect(closeModal).not.toHaveBeenCalled(); + + // When the hide event fires, even more than once + act(() => { + getLatestModalProps()?.onModalHide?.(); + getLatestModalProps()?.onModalHide?.(); + }); + + // Then the wrapper removes itself exactly once + expect(closeModal).toHaveBeenCalledTimes(1); + expect(closeModal).toHaveBeenCalledWith({action: ModalActions.CLOSE}); + }); + + it('shows the spinner and a cancel button while the file is still generating', () => { + // Given a file that is still generating + render(getWrapperElement({status: CONST.FILE_DOWNLOAD_STATUS.GENERATING, message: 'reportDetailsPage.waitForPDF'})); + + // Then the spinner and the generating copy are shown + expect(screen.getByText('reportDetailsPage.waitForPDF')).toBeOnTheScreen(); + expect(mockedActivityIndicator).toHaveBeenCalled(); + expect(screen.getByText('common.cancel')).toBeOnTheScreen(); + }); + + it('stops the spinner and offers a close button when the file generation fails', () => { + // Given a file whose generation failed + render(getWrapperElement({status: CONST.FILE_DOWNLOAD_STATUS.FAILED, message: 'reportDetailsPage.errorPDF'})); + + // Then there is nothing left to wait for, so the spinner stops and the button closes the modal + expect(screen.getByText('reportDetailsPage.errorPDF')).toBeOnTheScreen(); + expect(mockedActivityIndicator).not.toHaveBeenCalled(); + expect(screen.getByText('common.close')).toBeOnTheScreen(); + }); + + it('auto-downloads once when the file becomes ready while the modal is open', () => { + // Given an open modal waiting on the file + const onDownload = jest.fn(); + const {rerender} = render(getWrapperElement({onDownload})); + expect(onDownload).not.toHaveBeenCalled(); + + // When the caller updates the modal in place with the ready state + rerender(getWrapperElement({onDownload, status: CONST.FILE_DOWNLOAD_STATUS.READY, message: 'reportDetailsPage.successPDF'})); + rerender(getWrapperElement({onDownload, status: CONST.FILE_DOWNLOAD_STATUS.READY, message: 'reportDetailsPage.successPDF'})); + + // Then the file downloads once and the button offers a manual download + expect(onDownload).toHaveBeenCalledTimes(1); + expect(mockedActivityIndicator).toHaveBeenCalledTimes(1); + expect(screen.getByText('common.download')).toBeOnTheScreen(); + }); + + it('closes the modal after a manual download only when asked to', () => { + // Given a ready file whose caller wants the modal closed on download + const onDownload = jest.fn(); + const onClose = jest.fn(); + render(getWrapperElement({onDownload, onClose, status: CONST.FILE_DOWNLOAD_STATUS.READY, shouldCloseOnDownload: true})); + onDownload.mockClear(); + + // When the user presses the download button + fireEvent.press(screen.getByText('common.download')); + + // Then the file downloads and the modal starts hiding + expect(onDownload).toHaveBeenCalledTimes(1); + expect(onClose).toHaveBeenCalledTimes(1); + expect(getLatestModalProps()?.isVisible).toBe(false); + }); +}); diff --git a/tests/unit/components/ReportPDFDownloadModalTest.tsx b/tests/unit/components/ReportPDFDownloadModalTest.tsx deleted file mode 100644 index 2503885e803d..000000000000 --- a/tests/unit/components/ReportPDFDownloadModalTest.tsx +++ /dev/null @@ -1,77 +0,0 @@ -import {render, screen} from '@testing-library/react-native'; - -import ActivityIndicator from '@components/ActivityIndicator'; -import ReportPDFDownloadModal from '@components/ReportPDFDownloadModal'; - -import useOnyx from '@hooks/useOnyx'; - -import CONST from '@src/CONST'; -import ONYXKEYS from '@src/ONYXKEYS'; - -import React from 'react'; - -jest.mock('@components/ActivityIndicator', () => jest.fn(() => null)); -jest.mock('@components/Modal', () => jest.fn(({children}: {children: React.ReactNode}) => children)); -jest.mock('@hooks/useOnyx', () => jest.fn()); -jest.mock('@libs/actions/Report', () => ({downloadReportPDF: jest.fn()})); -jest.mock('@hooks/useNetwork', () => jest.fn(() => ({isOffline: false}))); -jest.mock('@hooks/useCurrentUserPersonalDetails', () => jest.fn(() => ({login: 'employee@mail.com'}))); -jest.mock('@hooks/useLocalize', () => jest.fn(() => ({translate: (key: string) => key}))); - -const REPORT_ID = '1'; - -describe('ReportPDFDownloadModal', () => { - const mockedUseOnyx = jest.mocked(useOnyx); - const mockedActivityIndicator = jest.mocked(ActivityIndicator); - - const mockPDFFilename = (reportPDFFilename: string | undefined) => { - (mockedUseOnyx as jest.Mock).mockImplementation((key: string) => { - if (key === `${ONYXKEYS.COLLECTION.NVP_EXPENSIFY_REPORT_PDF_FILENAME}${REPORT_ID}`) { - return [reportPDFFilename]; - } - return [undefined]; - }); - }; - - beforeEach(() => { - jest.clearAllMocks(); - }); - - const renderModal = () => - render( - , - ); - - it('shows the spinner while the PDF is still generating', () => { - mockPDFFilename(undefined); - - renderModal(); - - expect(screen.getByText('reportDetailsPage.waitForPDF')).toBeOnTheScreen(); - expect(mockedActivityIndicator).toHaveBeenCalled(); - }); - - it('stops the spinner and offers a close button when the PDF generation fails', () => { - mockPDFFilename(CONST.REPORT_DETAILS_MENU_ITEM.ERROR); - - renderModal(); - - expect(screen.getByText('reportDetailsPage.errorPDF')).toBeOnTheScreen(); - expect(mockedActivityIndicator).not.toHaveBeenCalled(); - expect(screen.getByText('common.close')).toBeOnTheScreen(); - }); - - it('stops the spinner and offers a download button once the PDF is ready', () => { - mockPDFFilename('report.pdf'); - - renderModal(); - - expect(screen.getByText('reportDetailsPage.successPDF')).toBeOnTheScreen(); - expect(mockedActivityIndicator).not.toHaveBeenCalled(); - expect(screen.getByText('common.download')).toBeOnTheScreen(); - }); -}); diff --git a/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx b/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx new file mode 100644 index 000000000000..311dc7a7b9a6 --- /dev/null +++ b/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx @@ -0,0 +1,214 @@ +import {act, renderHook} from '@testing-library/react-native'; + +import FileDownloadModal from '@components/Modal/FileDownloadModal'; +import {ModalProvider} from '@components/Modal/Global/ModalContext'; + +import useExpensifyCardStatementPDFDownloadModal from '@hooks/useExpensifyCardStatementPDFDownloadModal'; +import useOnyx from '@hooks/useOnyx'; + +import {downloadExpensifyCardStatementPDF} from '@libs/ExpensifyCardStatementUtils'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; + +import React from 'react'; + +import waitForBatchedUpdates from '../../utils/waitForBatchedUpdates'; + +// Counts real mounts, so a test can tell an in-place props update apart from a remount. +const mockOnFileDownloadModalMount = jest.fn(); +jest.mock('@components/Modal/FileDownloadModal', () => { + const {useEffect} = jest.requireActual('react'); + return jest.fn(() => { + useEffect(() => { + mockOnFileDownloadModalMount(); + }, []); + return null; + }); +}); +jest.mock('@hooks/useOnyx', () => jest.fn()); +jest.mock('@libs/ExpensifyCardStatementUtils', () => ({downloadExpensifyCardStatementPDF: jest.fn()})); +jest.mock('@hooks/useEnvironment', () => jest.fn(() => ({environment: 'development'}))); +jest.mock('@hooks/useCurrentUserPersonalDetails', () => jest.fn(() => ({login: 'employee@mail.com'}))); +jest.mock('@hooks/useLocalize', () => jest.fn(() => ({translate: (key: string) => key}))); + +const STATEMENT_PARAMS = {policyID: 'policy1', feedCountry: 'US', entryIDs: [123]}; +const STATEMENT_KEY = 'statement-key'; + +let mockOnyxValues: Record = {}; + +describe('useExpensifyCardStatementPDFDownloadModal', () => { + const mockedFileDownloadModal = jest.mocked(FileDownloadModal); + const getLatestModalProps = () => mockedFileDownloadModal.mock.calls.at(-1)?.[0]; + + const renderStatementPDFDownloadModal = () => + renderHook(() => useExpensifyCardStatementPDFDownloadModal(), { + wrapper: ({children}: {children: React.ReactNode}) => {children}, + }); + + // Mirrors a backend-pushed Onyx update: change the value, then re-render so the hook reads it. + const setStatementInOnyx = async (statement: Record, rerender: (props: unknown) => void) => { + mockOnyxValues = {...mockOnyxValues, [ONYXKEYS.EXPENSIFY_CARD_STATEMENT]: statement}; + rerender({}); + await act(async () => { + await waitForBatchedUpdates(); + }); + }; + + beforeEach(() => { + jest.clearAllMocks(); + mockOnyxValues = {[ONYXKEYS.SESSION]: {encryptedAuthToken: 'token'}}; + (jest.mocked(useOnyx) as jest.Mock).mockImplementation((key: string) => [mockOnyxValues[key]]); + }); + + it('updates the open modal in place as the statementKey and file arrive', async () => { + // Given the modal opened before GetExpensifyCardStatementPDF has returned a statementKey + const {result, rerender} = renderStatementPDFDownloadModal(); + let firstPromise: Promise | undefined; + await act(async () => { + firstPromise = result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: STATEMENT_PARAMS}); + await waitForBatchedUpdates(); + }); + + // Then it shows the generating state with the card statement button flags + expect(getLatestModalProps()).toEqual( + expect.objectContaining({ + isVisible: true, + status: CONST.FILE_DOWNLOAD_STATUS.GENERATING, + message: 'reportDetailsPage.waitForPDF', + shouldCloseOnDownload: true, + shouldUseSuccessButton: true, + }), + ); + + // When the statementKey arrives through a second show call and the backend pushes the generated file + let secondPromise: Promise | undefined; + await act(async () => { + secondPromise = result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: {...STATEMENT_PARAMS, statementKey: STATEMENT_KEY}}); + await waitForBatchedUpdates(); + }); + await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement.pdf'}, rerender); + + // Then the same modal instance switches to the ready state, and both callers share one close promise + expect(secondPromise).toBe(firstPromise); + expect(getLatestModalProps()).toEqual(expect.objectContaining({isVisible: true, status: CONST.FILE_DOWNLOAD_STATUS.READY, message: 'reportDetailsPage.successPDF'})); + expect(mockOnFileDownloadModalMount).toHaveBeenCalledTimes(1); + + // When the modal downloads the file + act(() => { + getLatestModalProps()?.onDownload(); + }); + + // Then it downloads the statement for that key + expect(downloadExpensifyCardStatementPDF).toHaveBeenCalledWith(expect.any(Function), 'statement.pdf', STATEMENT_KEY, 'employee@mail.com', 'token', 'development'); + }); + + it('keeps the generating state while the statement is still generating', async () => { + // Given the modal opened with a statementKey whose file is cached but being regenerated + const {result, rerender} = renderStatementPDFDownloadModal(); + await act(async () => { + result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: {...STATEMENT_PARAMS, statementKey: STATEMENT_KEY}}); + await waitForBatchedUpdates(); + }); + + // When Onyx says the statement is generating + await setStatementInOnyx({isGenerating: true, [STATEMENT_KEY]: 'statement.pdf'}, rerender); + + // Then the modal still waits instead of offering a stale file + expect(getLatestModalProps()?.status).toBe(CONST.FILE_DOWNLOAD_STATUS.GENERATING); + }); + + it('resolves the show promise only after the modal has finished hiding', async () => { + // Given an open modal + const {result} = renderStatementPDFDownloadModal(); + let hasResolved = false; + await act(async () => { + result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: STATEMENT_PARAMS}).then(() => { + hasResolved = true; + }); + await waitForBatchedUpdates(); + }); + + // When the user dismisses it + await act(async () => { + getLatestModalProps()?.onClose(); + await waitForBatchedUpdates(); + }); + + // Then it starts hiding but the caller, which clears the selection on resolve, is not told it is gone yet + expect(getLatestModalProps()?.isVisible).toBe(false); + expect(hasResolved).toBe(false); + + // When the hide animation ends + await act(async () => { + getLatestModalProps()?.onModalHide?.(); + await waitForBatchedUpdates(); + }); + + // Then the show promise resolves + expect(hasResolved).toBe(true); + }); + + it('does not reopen a dismissed modal when the statementKey arrives during the hide animation', async () => { + // Given the user dismissed the modal before the statementKey arrived + const {result, rerender} = renderStatementPDFDownloadModal(); + await act(async () => { + result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: STATEMENT_PARAMS}); + await waitForBatchedUpdates(); + }); + await act(async () => { + getLatestModalProps()?.onClose(); + await waitForBatchedUpdates(); + }); + + // When the statementKey and file arrive while the modal is hiding + await act(async () => { + result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: {...STATEMENT_PARAMS, statementKey: STATEMENT_KEY}}); + await waitForBatchedUpdates(); + }); + await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement.pdf'}, rerender); + + // Then the hiding modal is left alone + expect(getLatestModalProps()).toEqual(expect.objectContaining({isVisible: false, status: CONST.FILE_DOWNLOAD_STATUS.GENERATING})); + + // When the hide animation ends and another update arrives + await act(async () => { + getLatestModalProps()?.onModalHide?.(); + await waitForBatchedUpdates(); + }); + const renderCountAfterHide = mockedFileDownloadModal.mock.calls.length; + await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement-2.pdf'}, rerender); + + // Then the modal is not shown again + expect(mockedFileDownloadModal.mock.calls.length).toBe(renderCountAfterHide); + expect(mockOnFileDownloadModalMount).toHaveBeenCalledTimes(1); + }); + + it('closes the modal by id without reopening it on a later update', async () => { + // Given an open modal waiting on the statement + const {result, rerender} = renderStatementPDFDownloadModal(); + let hasResolved = false; + await act(async () => { + result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: {...STATEMENT_PARAMS, statementKey: STATEMENT_KEY}}).then(() => { + hasResolved = true; + }); + await waitForBatchedUpdates(); + }); + + // When the request fails and the caller closes the modal by id + await act(async () => { + result.current.closeExpensifyCardStatementPDFDownloadModal(); + await waitForBatchedUpdates(); + }); + const renderCountAfterClose = mockedFileDownloadModal.mock.calls.length; + + // Then the modal is gone and its promise resolves + expect(hasResolved).toBe(true); + + // When the file arrives afterwards + await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement.pdf'}, rerender); + + // Then the modal is not shown again + expect(mockedFileDownloadModal.mock.calls.length).toBe(renderCountAfterClose); + }); +}); diff --git a/tests/unit/hooks/useReportPDFDownloadModalTest.tsx b/tests/unit/hooks/useReportPDFDownloadModalTest.tsx new file mode 100644 index 000000000000..d9ea86600f91 --- /dev/null +++ b/tests/unit/hooks/useReportPDFDownloadModalTest.tsx @@ -0,0 +1,227 @@ +import {act, renderHook} from '@testing-library/react-native'; + +import FileDownloadModal from '@components/Modal/FileDownloadModal'; +import {ModalProvider} from '@components/Modal/Global/ModalContext'; + +import useOnyx from '@hooks/useOnyx'; +import useReportPDFDownloadModal from '@hooks/useReportPDFDownloadModal'; +import useStallLogger from '@hooks/useStallLogger'; + +import {downloadReportPDF} from '@libs/actions/Report'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; + +import React from 'react'; + +import waitForBatchedUpdates from '../../utils/waitForBatchedUpdates'; + +// Counts real mounts, so a test can tell an in-place props update apart from a remount. +const mockOnFileDownloadModalMount = jest.fn(); +jest.mock('@components/Modal/FileDownloadModal', () => { + const {useEffect} = jest.requireActual('react'); + return jest.fn(() => { + useEffect(() => { + mockOnFileDownloadModalMount(); + }, []); + return null; + }); +}); +jest.mock('@hooks/useOnyx', () => jest.fn()); +jest.mock('@hooks/useStallLogger', () => jest.fn()); +jest.mock('@libs/actions/Report', () => ({downloadReportPDF: jest.fn()})); +jest.mock('@hooks/useNetwork', () => jest.fn(() => ({isOffline: false}))); +jest.mock('@hooks/useCurrentUserPersonalDetails', () => jest.fn(() => ({login: 'employee@mail.com'}))); +jest.mock('@hooks/useLocalize', () => jest.fn(() => ({translate: (key: string) => key}))); + +const REPORT_ID = '1'; +const PDF_FILENAME_KEY = `${ONYXKEYS.COLLECTION.NVP_EXPENSIFY_REPORT_PDF_FILENAME}${REPORT_ID}`; + +let mockOnyxValues: Record = {}; + +describe('useReportPDFDownloadModal', () => { + const mockedFileDownloadModal = jest.mocked(FileDownloadModal); + const getLatestModalProps = () => mockedFileDownloadModal.mock.calls.at(-1)?.[0]; + + const renderReportPDFDownloadModal = () => + renderHook(() => useReportPDFDownloadModal(), { + wrapper: ({children}: {children: React.ReactNode}) => {children}, + }); + + // Mirrors a backend-pushed Onyx update: change the value, then re-render so the hook reads it. + const setOnyxValue = async (key: string, value: unknown, rerender: (props: unknown) => void) => { + mockOnyxValues = {...mockOnyxValues, [key]: value}; + rerender({}); + await act(async () => { + await waitForBatchedUpdates(); + }); + }; + + beforeEach(() => { + jest.clearAllMocks(); + mockOnyxValues = { + [`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`]: {reportID: REPORT_ID, reportName: 'Expense Report'}, + [ONYXKEYS.SESSION]: {encryptedAuthToken: 'token'}, + }; + (jest.mocked(useOnyx) as jest.Mock).mockImplementation((key: string) => [mockOnyxValues[key]]); + }); + + it('updates the open modal in place as the PDF generates', async () => { + // Given the modal opened for a report whose PDF is still generating + const {result, rerender} = renderReportPDFDownloadModal(); + await act(async () => { + result.current.showReportPDFDownloadModal({reportID: REPORT_ID}); + await waitForBatchedUpdates(); + }); + + // Then it shows the generating state with the spinner copy + expect(getLatestModalProps()).toEqual( + expect.objectContaining({isVisible: true, status: CONST.FILE_DOWNLOAD_STATUS.GENERATING, message: 'reportDetailsPage.waitForPDF', shouldTreatModalAsCovering: true}), + ); + + // When the backend pushes the generated filename + await setOnyxValue(PDF_FILENAME_KEY, 'report.pdf', rerender); + + // Then the same modal instance switches to the ready state instead of being remounted + expect(getLatestModalProps()).toEqual(expect.objectContaining({isVisible: true, status: CONST.FILE_DOWNLOAD_STATUS.READY, message: 'reportDetailsPage.successPDF'})); + expect(mockOnFileDownloadModalMount).toHaveBeenCalledTimes(1); + + // When the modal downloads the file + act(() => { + getLatestModalProps()?.onDownload(); + }); + + // Then it downloads the report's PDF + expect(downloadReportPDF).toHaveBeenCalledWith('report.pdf', 'Expense Report', expect.any(Function), 'employee@mail.com', 'token'); + }); + + it('shows the failed state when the PDF generation fails', async () => { + // Given the modal opened for a report + const {result, rerender} = renderReportPDFDownloadModal(); + await act(async () => { + result.current.showReportPDFDownloadModal({reportID: REPORT_ID}); + await waitForBatchedUpdates(); + }); + + // When the backend reports the generation failed + await setOnyxValue(PDF_FILENAME_KEY, CONST.REPORT_DETAILS_MENU_ITEM.ERROR, rerender); + + // Then the modal shows the error state, which stops the spinner + expect(getLatestModalProps()).toEqual(expect.objectContaining({status: CONST.FILE_DOWNLOAD_STATUS.FAILED, message: 'reportDetailsPage.errorPDF'})); + }); + + it('cancels once when dismissed while the PDF is still generating', async () => { + // Given the modal opened by Submit via PDF, which retracts the submit when cancelled + const onCancel = jest.fn(); + const {result} = renderReportPDFDownloadModal(); + await act(async () => { + result.current.showReportPDFDownloadModal({reportID: REPORT_ID, onCancel}); + await waitForBatchedUpdates(); + }); + + // When the user dismisses it before the filename arrives, and the close fires again from another control + act(() => { + getLatestModalProps()?.onClose(); + getLatestModalProps()?.onClose(); + }); + + // Then the submit is retracted exactly once + expect(onCancel).toHaveBeenCalledTimes(1); + }); + + it('does not cancel when dismissed after the PDF is ready', async () => { + // Given the modal opened by Submit via PDF and the filename has arrived, so the submit has already resolved + const onCancel = jest.fn(); + const {result, rerender} = renderReportPDFDownloadModal(); + await act(async () => { + result.current.showReportPDFDownloadModal({reportID: REPORT_ID, onCancel}); + await waitForBatchedUpdates(); + }); + await setOnyxValue(PDF_FILENAME_KEY, 'report.pdf', rerender); + + // When the user dismisses the modal + act(() => { + getLatestModalProps()?.onClose(); + }); + + // Then the submit is kept + expect(onCancel).not.toHaveBeenCalled(); + }); + + it('resolves the show promise only after the modal has finished hiding', async () => { + // Given an open modal + const {result} = renderReportPDFDownloadModal(); + let hasResolved = false; + await act(async () => { + result.current.showReportPDFDownloadModal({reportID: REPORT_ID}).then(() => { + hasResolved = true; + }); + await waitForBatchedUpdates(); + }); + + // When the user dismisses it + await act(async () => { + getLatestModalProps()?.onClose(); + await waitForBatchedUpdates(); + }); + + // Then it starts hiding but the caller is not told it is gone yet + expect(getLatestModalProps()?.isVisible).toBe(false); + expect(hasResolved).toBe(false); + + // When the hide animation ends + await act(async () => { + getLatestModalProps()?.onModalHide?.(); + await waitForBatchedUpdates(); + }); + + // Then the show promise resolves + expect(hasResolved).toBe(true); + }); + + it('does not reopen a dismissed modal when the filename arrives late', async () => { + // Given the user dismissed the modal while the PDF was still generating + const {result, rerender} = renderReportPDFDownloadModal(); + await act(async () => { + result.current.showReportPDFDownloadModal({reportID: REPORT_ID}); + await waitForBatchedUpdates(); + }); + await act(async () => { + getLatestModalProps()?.onClose(); + await waitForBatchedUpdates(); + }); + + // When the filename arrives while the modal is hiding, and again after it has gone + await setOnyxValue(PDF_FILENAME_KEY, 'report.pdf', rerender); + expect(getLatestModalProps()).toEqual(expect.objectContaining({isVisible: false, status: CONST.FILE_DOWNLOAD_STATUS.GENERATING})); + await act(async () => { + getLatestModalProps()?.onModalHide?.(); + await waitForBatchedUpdates(); + }); + const renderCountAfterHide = mockedFileDownloadModal.mock.calls.length; + await setOnyxValue(PDF_FILENAME_KEY, 'report-2.pdf', rerender); + + // Then the modal is not shown again + expect(mockedFileDownloadModal.mock.calls.length).toBe(renderCountAfterHide); + expect(mockOnFileDownloadModalMount).toHaveBeenCalledTimes(1); + }); + + it('logs a stall only while the open modal is waiting on the filename', async () => { + // Given the modal opened for a report whose PDF is still generating + const {result, rerender} = renderReportPDFDownloadModal(); + expect(jest.mocked(useStallLogger).mock.calls.at(-1)?.[0]).toBe(false); + await act(async () => { + result.current.showReportPDFDownloadModal({reportID: REPORT_ID}); + await waitForBatchedUpdates(); + }); + + // Then the stall timer is keyed by the report it waits on + expect(jest.mocked(useStallLogger).mock.calls.at(-1)?.[0]).toBe(REPORT_ID); + + // When the filename arrives + await setOnyxValue(PDF_FILENAME_KEY, 'report.pdf', rerender); + + // Then the stall timer stops + expect(jest.mocked(useStallLogger).mock.calls.at(-1)?.[0]).toBe(false); + }); +}); diff --git a/tests/unit/hooks/useSearchBulkActionsDownloadPDFTest.ts b/tests/unit/hooks/useSearchBulkActionsDownloadPDFTest.ts index 30d8355b4d9c..72f72f0b0f8d 100644 --- a/tests/unit/hooks/useSearchBulkActionsDownloadPDFTest.ts +++ b/tests/unit/hooks/useSearchBulkActionsDownloadPDFTest.ts @@ -59,6 +59,14 @@ jest.mock('@hooks/useExpensifyCardStatementPDFDownloadModal', () => ({ }), })); +// The report PDF hook owns its own Onyx subscriptions and modal lifecycle (covered by useReportPDFDownloadModalTest), +// so only its show call is observed here. +const mockShowReportPDFDownloadModal = jest.fn, [{reportID: string}]>(() => Promise.resolve({action: 'CLOSE'})); +jest.mock('@hooks/useReportPDFDownloadModal', () => ({ + __esModule: true, + default: () => ({showReportPDFDownloadModal: mockShowReportPDFDownloadModal}), +})); + let mockIsOffline = false; jest.mock('@hooks/useNetwork', () => ({ __esModule: true, @@ -364,6 +372,8 @@ describe('useSearchBulkActions - Download report', () => { expect(exportReportToPDF).toHaveBeenCalledTimes(1); expect(exportReportToPDF).toHaveBeenCalledWith({reportID: '1'}); + expect(mockShowReportPDFDownloadModal).toHaveBeenCalledTimes(1); + expect(mockShowReportPDFDownloadModal).toHaveBeenCalledWith({reportID: '1'}); }); it('should not call exportReportToPDF when offline', async () => { From 152d7d6aa0bf250d112f95a5753b29c0937dfce9 Mon Sep 17 00:00:00 2001 From: "daledah (via MelvinBot)" Date: Wed, 21 Oct 2026 07:44:06 +0000 Subject: [PATCH 2/8] Fix: remove unused FileDownloadStatus type export flagged by knip Co-authored-by: daledah --- src/components/Modal/FileDownloadModal.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Modal/FileDownloadModal.tsx b/src/components/Modal/FileDownloadModal.tsx index c5cc898d72f1..f07b16b7d988 100644 --- a/src/components/Modal/FileDownloadModal.tsx +++ b/src/components/Modal/FileDownloadModal.tsx @@ -168,4 +168,4 @@ function FileDownloadModal({ FileDownloadModal.displayName = 'FileDownloadModal'; export default FileDownloadModal; -export type {FileDownloadModalProps, FileDownloadStatus}; +export type {FileDownloadModalProps}; From ee24686caf388ecbf49a30012e578f00dd67669a Mon Sep 17 00:00:00 2001 From: "daledah (via MelvinBot)" Date: Wed, 21 Oct 2026 09:44:38 +0000 Subject: [PATCH 3/8] Move the file download modal lifecycle into a generic useFileDownloadModal hook - Add useFileDownloadModal, which owns the fixed-id show, in-place updates, and no-reopen-after-dismiss lifecycle. The report and Expensify Card statement hooks now only derive the title, status, message, and download action. - Drop onCancel and the wrapper's onClose passthrough. The show promise resolves with CLOSE when the modal is dismissed while generating and CONFIRM otherwise, and SubmitPrimaryAction chains the retract off openPDFDownload(). - Pass the header text into FileDownloadModal instead of hardcoding it. Co-authored-by: daledah --- src/components/Modal/FileDownloadModal.tsx | 6 +- .../Modal/Global/FileDownloadModalWrapper.tsx | 30 +++++-- src/components/Modal/Global/ModalContext.tsx | 2 +- src/components/MoneyReportHeaderModals.tsx | 6 +- .../MoneyReportHeaderModalsContext.tsx | 6 +- .../SubmitPrimaryAction.tsx | 28 +++--- ...eExpensifyCardStatementPDFDownloadModal.ts | 62 +++---------- src/hooks/useFileDownloadModal.ts | 86 +++++++++++++++++++ src/hooks/useReportPDFDownloadModal.ts | 75 +++------------- .../FileDownloadModalWrapperTest.tsx | 60 +++++++++++-- ...nsifyCardStatementPDFDownloadModalTest.tsx | 20 +++-- .../hooks/useReportPDFDownloadModalTest.tsx | 49 +++++++---- 12 files changed, 251 insertions(+), 179 deletions(-) create mode 100644 src/hooks/useFileDownloadModal.ts diff --git a/src/components/Modal/FileDownloadModal.tsx b/src/components/Modal/FileDownloadModal.tsx index f07b16b7d988..3d153d58f5ee 100644 --- a/src/components/Modal/FileDownloadModal.tsx +++ b/src/components/Modal/FileDownloadModal.tsx @@ -24,6 +24,9 @@ import Modal from '.'; type FileDownloadStatus = ValueOf; type FileDownloadModalProps = { + /** Header text shown at the top of the modal */ + title: string; + /** Whether the file is still generating, ready to download, or failed to generate */ status: FileDownloadStatus; @@ -57,6 +60,7 @@ type FileDownloadModalProps = { * download button. Callers supply the status, copy, and download action for their specific file. */ function FileDownloadModal({ + title, status, message, onDownload, @@ -114,7 +118,7 @@ function FileDownloadModal({ - {translate('reportDetailsPage.generatingPDF')} + {title} {message} diff --git a/src/components/Modal/Global/FileDownloadModalWrapper.tsx b/src/components/Modal/Global/FileDownloadModalWrapper.tsx index 91984f14f454..789c9aab661a 100644 --- a/src/components/Modal/Global/FileDownloadModalWrapper.tsx +++ b/src/components/Modal/Global/FileDownloadModalWrapper.tsx @@ -1,24 +1,36 @@ import type {FileDownloadModalProps} from '@components/Modal/FileDownloadModal'; import FileDownloadModal from '@components/Modal/FileDownloadModal'; +import CONST from '@src/CONST'; + +import type {ValueOf} from 'type-fest'; + import React, {useRef, useState} from 'react'; import type {ModalProps} from './ModalContext'; import {ModalActions} from './ModalContext'; -type FileDownloadModalWrapperProps = ModalProps & - Omit & { - /** Called when the user dismisses the modal, before it starts hiding */ - onClose?: () => void; - }; +type FileDownloadModalWrapperProps = ModalProps & Omit; -function FileDownloadModalWrapper({closeModal, onClose, ...props}: FileDownloadModalWrapperProps) { +function FileDownloadModalWrapper({closeModal, ...props}: FileDownloadModalWrapperProps) { const [isVisible, setIsVisible] = useState(true); + + // The props at the moment the user dismissed the modal. The caller keeps updating the modal in place until it has + // finished hiding, so render these instead to keep a dismissed modal from changing while it fades out. + const [dismissedProps, setDismissedProps] = useState(); + const closeActionRef = useRef>(ModalActions.CLOSE); const hasClosedModalRef = useRef(false); const handleClose = () => { - onClose?.(); + if (!isVisible) { + return; + } + + // Dismissing while the file is still generating cancels the download. Once it is ready or has failed, the + // flow has run its course. + closeActionRef.current = props.status === CONST.FILE_DOWNLOAD_STATUS.GENERATING ? ModalActions.CLOSE : ModalActions.CONFIRM; + setDismissedProps(props); setIsVisible(false); }; @@ -29,14 +41,14 @@ function FileDownloadModalWrapper({closeModal, onClose, ...props}: FileDownloadM return; } hasClosedModalRef.current = true; - closeModal({action: ModalActions.CLOSE}); + closeModal({action: closeActionRef.current}); }; return ( void}) => { + openPDFDownload: () => { if (!moneyRequestReport?.reportID) { - return; + return Promise.resolve(undefined); } - showReportPDFDownloadModal({reportID: moneyRequestReport.reportID, onCancel: options?.onCancel}); + return showReportPDFDownloadModal({reportID: moneyRequestReport.reportID}); }, openHoldEducational: () => educationalModalsRef.current?.openHoldEducational(), openRejectModal: (action: RejectModalAction) => educationalModalsRef.current?.openRejectModal(action), diff --git a/src/components/MoneyReportHeaderModalsContext.tsx b/src/components/MoneyReportHeaderModalsContext.tsx index 1e1e6821d362..f33438417d44 100644 --- a/src/components/MoneyReportHeaderModalsContext.tsx +++ b/src/components/MoneyReportHeaderModalsContext.tsx @@ -4,6 +4,7 @@ import type {PaymentMethodType} from '@src/types/onyx/OriginalMessage'; import {createContext, useContext} from 'react'; +import type {ModalStateChangePayload} from './Modal/Global/ModalContext'; import type {RejectModalAction} from './MoneyReportHeaderEducationalModals'; type HoldMenuParams = { @@ -15,7 +16,8 @@ type HoldMenuParams = { type MoneyReportHeaderModalsContextValue = { openHoldMenu: (params: HoldMenuParams) => Promise; - openPDFDownload: (options?: {onCancel?: () => void}) => void; + /** Resolves once the PDF download modal has finished hiding, with `CLOSE` if it was dismissed while the PDF was still generating */ + openPDFDownload: () => Promise; openHoldEducational: () => void; openRejectModal: (action: RejectModalAction) => void; showOfflineModal: () => void; @@ -24,7 +26,7 @@ type MoneyReportHeaderModalsContextValue = { const defaultValue: MoneyReportHeaderModalsContextValue = { openHoldMenu: () => Promise.resolve(), - openPDFDownload: () => {}, + openPDFDownload: () => Promise.resolve(undefined), openHoldEducational: () => {}, openRejectModal: () => {}, showOfflineModal: () => {}, diff --git a/src/components/MoneyReportHeaderPrimaryAction/SubmitPrimaryAction.tsx b/src/components/MoneyReportHeaderPrimaryAction/SubmitPrimaryAction.tsx index 58dde9b9352d..6e98feaed827 100644 --- a/src/components/MoneyReportHeaderPrimaryAction/SubmitPrimaryAction.tsx +++ b/src/components/MoneyReportHeaderPrimaryAction/SubmitPrimaryAction.tsx @@ -1,6 +1,7 @@ import AnimatedSubmitButton from '@components/AnimatedSubmitButton'; import ButtonWithDropdownMenu from '@components/ButtonWithDropdownMenu'; import type {DropdownOption} from '@components/ButtonWithDropdownMenu/types'; +import {ModalActions} from '@components/Modal/Global/ModalContext'; import {useMoneyReportHeaderModals} from '@components/MoneyReportHeaderModalsContext'; import {usePaymentAnimationsContext} from '@components/PaymentAnimationsContext'; import {ReportSubmitToPopoverAnchor, useOpenReportSubmitToPopover} from '@components/ReportSubmitToPopoverAnchor'; @@ -184,24 +185,15 @@ function SubmitPrimaryActionContent({reportID, isDisabled}: SubmitPrimaryActionP if (shouldExportToPDF) { // If the user cancels while the PDF is still generating, discard the submission (retract it back // to draft) so they can resubmit — matching the pre-submit-via-PDF behavior. - openPDFDownload({ - onCancel: () => { - // Stop the "Submitted" animation in lockstep with the retract so the header goes straight - // back to the Submit button instead of finishing the animation on a report that is open again. - stopAnimation(); - retractReport( - moneyRequestReport, - chatReport, - policy, - accountID, - email ?? '', - hasViolations, - isASAPSubmitBetaEnabled, - delegateEmail, - isTrackIntentUser, - rules, - ); - }, + openPDFDownload().then((result) => { + if (result?.action !== ModalActions.CLOSE) { + return; + } + + // Stop the "Submitted" animation in lockstep with the retract so the header goes straight + // back to the Submit button instead of finishing the animation on a report that is open again. + stopAnimation(); + retractReport(moneyRequestReport, chatReport, policy, accountID, email ?? '', hasViolations, isASAPSubmitBetaEnabled, delegateEmail, isTrackIntentUser, rules); }); } }, diff --git a/src/hooks/useExpensifyCardStatementPDFDownloadModal.ts b/src/hooks/useExpensifyCardStatementPDFDownloadModal.ts index 5d98cbf1e3bb..02fc27750f8a 100644 --- a/src/hooks/useExpensifyCardStatementPDFDownloadModal.ts +++ b/src/hooks/useExpensifyCardStatementPDFDownloadModal.ts @@ -1,18 +1,14 @@ -import type {FileDownloadModalWrapperProps} from '@components/Modal/Global/FileDownloadModalWrapper'; -import FileDownloadModalWrapper from '@components/Modal/Global/FileDownloadModalWrapper'; -import type {ModalProps} from '@components/Modal/Global/ModalContext'; -import {useModal} from '@components/Modal/Global/ModalContext'; - import {downloadExpensifyCardStatementPDF} from '@libs/ExpensifyCardStatementUtils'; import type {ExpensifyCardStatementParams} from '@libs/ExpensifyCardStatementUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import {useEffect, useEffectEvent, useRef, useState} from 'react'; +import {useState} from 'react'; import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails'; import useEnvironment from './useEnvironment'; +import useFileDownloadModal from './useFileDownloadModal'; import useLocalize from './useLocalize'; import useOnyx from './useOnyx'; @@ -20,21 +16,13 @@ type ExpensifyCardStatementPDFDownloadModalOptions = { statementParams: ExpensifyCardStatementParams; }; -// A fixed id lets a second show call update the open modal in place, which is how the statementKey reaches it -// once GetExpensifyCardStatementPDF returns. +// A second show call updates the open modal in place, which is how the statementKey reaches it once +// GetExpensifyCardStatementPDF returns. const EXPENSIFY_CARD_STATEMENT_PDF_DOWNLOAD_MODAL_ID = 'expensifyCardStatementPDFDownloadModal'; const useExpensifyCardStatementPDFDownloadModal = () => { - const context = useModal(); const [statementParams, setStatementParams] = useState(); - // Whether the modal is open. It turns false as soon as the modal is dismissed, so a late update can't show it - // again: a show call with the fixed id after the modal has closed pushes a new modal instead of updating it. - const isOpenRef = useRef(false); - - // Resolves once the modal has finished hiding. Set from the first show call until then. - const modalPromiseRef = useRef | undefined>(undefined); - const [expensifyCardStatement] = useOnyx(ONYXKEYS.EXPENSIFY_CARD_STATEMENT); const [session] = useOnyx(ONYXKEYS.SESSION); const {translate} = useLocalize(); @@ -47,11 +35,12 @@ const useExpensifyCardStatementPDFDownloadModal = () => { const isGeneratingPDF = expensifyCardStatement?.isGenerating === true; const statementFileName = statementKey ? expensifyCardStatement?.[statementKey] : undefined; const hasFinishedPDFDownload = !!statementKey && !isGeneratingPDF && typeof statementFileName === 'string' && statementFileName.length > 0; - const message = hasFinishedPDFDownload ? translate('reportDetailsPage.successPDF') : translate('reportDetailsPage.waitForPDF'); - const getModalProps = (): Omit => ({ + const {showFileDownloadModal, closeFileDownloadModal} = useFileDownloadModal({ + id: EXPENSIFY_CARD_STATEMENT_PDF_DOWNLOAD_MODAL_ID, + title: translate('reportDetailsPage.generatingPDF'), status: hasFinishedPDFDownload ? CONST.FILE_DOWNLOAD_STATUS.READY : CONST.FILE_DOWNLOAD_STATUS.GENERATING, - message, + message: hasFinishedPDFDownload ? translate('reportDetailsPage.successPDF') : translate('reportDetailsPage.waitForPDF'), shouldCloseOnDownload: true, shouldUseSuccessButton: true, onDownload: () => { @@ -60,48 +49,17 @@ const useExpensifyCardStatementPDFDownloadModal = () => { } downloadExpensifyCardStatementPDF(translate, statementFileName, statementKey, currentUserLogin, encryptedAuthToken, environment); }, - onClose: () => { - isOpenRef.current = false; - }, - }); - - const updateOpenModal = useEffectEvent(() => { - if (!isOpenRef.current) { - return; - } - context.showModal({component: FileDownloadModalWrapper, id: EXPENSIFY_CARD_STATEMENT_PDF_DOWNLOAD_MODAL_ID, props: getModalProps()}); }); - // Push the latest generation state into the open modal so it stays reactive without remounting. - useEffect(() => { - updateOpenModal(); - }, [statementKey, statementFileName, message, currentUserLogin, encryptedAuthToken, environment]); - const showExpensifyCardStatementPDFDownloadModal = (options: ExpensifyCardStatementPDFDownloadModalOptions) => { setStatementParams(options.statementParams); - - // The modal is still on the stack (open or hiding), so the effect above updates it in place while it is open. - if (modalPromiseRef.current) { - return modalPromiseRef.current; - } - - isOpenRef.current = true; - const modalPromise = context.showModal({component: FileDownloadModalWrapper, id: EXPENSIFY_CARD_STATEMENT_PDF_DOWNLOAD_MODAL_ID, props: getModalProps()}).then((result) => { - isOpenRef.current = false; - modalPromiseRef.current = undefined; + return showFileDownloadModal().then((result) => { setStatementParams(undefined); return result; }); - modalPromiseRef.current = modalPromise; - return modalPromise; - }; - - const closeExpensifyCardStatementPDFDownloadModal = () => { - isOpenRef.current = false; - context.closeModalByID(EXPENSIFY_CARD_STATEMENT_PDF_DOWNLOAD_MODAL_ID); }; - return {showExpensifyCardStatementPDFDownloadModal, closeExpensifyCardStatementPDFDownloadModal}; + return {showExpensifyCardStatementPDFDownloadModal, closeExpensifyCardStatementPDFDownloadModal: closeFileDownloadModal}; }; export default useExpensifyCardStatementPDFDownloadModal; diff --git a/src/hooks/useFileDownloadModal.ts b/src/hooks/useFileDownloadModal.ts new file mode 100644 index 000000000000..e3f2badcfe64 --- /dev/null +++ b/src/hooks/useFileDownloadModal.ts @@ -0,0 +1,86 @@ +import type {FileDownloadModalWrapperProps} from '@components/Modal/Global/FileDownloadModalWrapper'; +import FileDownloadModalWrapper from '@components/Modal/Global/FileDownloadModalWrapper'; +import type {ModalProps, ModalStateChangePayload} from '@components/Modal/Global/ModalContext'; +import {useModal} from '@components/Modal/Global/ModalContext'; + +import {useEffect, useEffectEvent, useRef} from 'react'; + +type FileDownloadModalOptions = Omit & { + /** + * Fixed id of the modal on the global stack. A show call with the same id updates the open modal in place instead + * of stacking a new one, which keeps it reactive as the file generates. + */ + id: string; +}; + +/** + * Shows a file download modal on the global modal stack and keeps it in sync with the options passed on every render, + * so callers only derive the status, copy, and download action for their file. + * + * The show promise resolves once the modal has finished hiding, with `CLOSE` if the user dismissed it while the file + * was still generating and `CONFIRM` otherwise. + */ +const useFileDownloadModal = ({id, onDownload, title, status, message, shouldCloseOnDownload, shouldUseSuccessButton, shouldTreatModalAsCovering}: FileDownloadModalOptions) => { + const context = useModal(); + + // Whether the modal is on the stack. It turns false once the modal is gone, so a late update can't show it again: a + // show call with the fixed id after the modal has closed pushes a new modal instead of updating it. + const isOpenRef = useRef(false); + + // Resolves once the modal has finished hiding. Set from the first show call until then. + const modalPromiseRef = useRef | undefined>(undefined); + + // Read through a ref so a download always uses the caller's latest data, without pushing the modal again every time + // the caller re-renders with a new callback. + const onDownloadRef = useRef(onDownload); + useEffect(() => { + onDownloadRef.current = onDownload; + }, [onDownload]); + + const getModalProps = (): Omit => ({ + title, + status, + message, + shouldCloseOnDownload, + shouldUseSuccessButton, + shouldTreatModalAsCovering, + onDownload: () => onDownloadRef.current(), + }); + + const updateOpenModal = useEffectEvent(() => { + if (!isOpenRef.current) { + return; + } + context.showModal({component: FileDownloadModalWrapper, id, props: getModalProps()}); + }); + + // Push the latest state into the open modal so it stays reactive without remounting. + useEffect(() => { + updateOpenModal(); + }, [title, status, message, shouldCloseOnDownload, shouldUseSuccessButton, shouldTreatModalAsCovering]); + + const showFileDownloadModal = () => { + // The modal is still on the stack (open or hiding), so the effect above updates it in place. + if (modalPromiseRef.current) { + return modalPromiseRef.current; + } + + isOpenRef.current = true; + const modalPromise = context.showModal({component: FileDownloadModalWrapper, id, props: getModalProps()}).then((result) => { + isOpenRef.current = false; + modalPromiseRef.current = undefined; + return result; + }); + modalPromiseRef.current = modalPromise; + return modalPromise; + }; + + const closeFileDownloadModal = () => { + isOpenRef.current = false; + context.closeModalByID(id); + }; + + return {showFileDownloadModal, closeFileDownloadModal}; +}; + +export default useFileDownloadModal; diff --git a/src/hooks/useReportPDFDownloadModal.ts b/src/hooks/useReportPDFDownloadModal.ts index 023f917e68d4..83444d21f15d 100644 --- a/src/hooks/useReportPDFDownloadModal.ts +++ b/src/hooks/useReportPDFDownloadModal.ts @@ -1,16 +1,12 @@ -import type {FileDownloadModalWrapperProps} from '@components/Modal/Global/FileDownloadModalWrapper'; -import FileDownloadModalWrapper from '@components/Modal/Global/FileDownloadModalWrapper'; -import type {ModalProps} from '@components/Modal/Global/ModalContext'; -import {useModal} from '@components/Modal/Global/ModalContext'; - import {downloadReportPDF} from '@libs/actions/Report'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import {useEffect, useEffectEvent, useRef, useState} from 'react'; +import {useState} from 'react'; import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails'; +import useFileDownloadModal from './useFileDownloadModal'; import useLocalize from './useLocalize'; import useNetwork from './useNetwork'; import useOnyx from './useOnyx'; @@ -18,27 +14,13 @@ import useStallLogger from './useStallLogger'; type ReportPDFDownloadModalOptions = { reportID: string; - - /** Called when the modal is dismissed while the PDF is still generating (e.g. Submit via PDF retracts the submit). */ - onCancel?: () => void; }; -// A fixed id lets the hook show the open modal again with fresh props, which updates it in place as the PDF -// filename arrives instead of stacking a new modal. const REPORT_PDF_DOWNLOAD_MODAL_ID = 'reportPDFDownloadModal'; const useReportPDFDownloadModal = () => { - const context = useModal(); const [activeReportID, setActiveReportID] = useState(); - // Whether the modal is open. It turns false as soon as the user dismisses the modal, so a late update can't show - // it again: a show call with the fixed id after the modal has closed pushes a new modal instead of updating it. - const isOpenRef = useRef(false); - - // Resolves once the modal has finished hiding. Set from the first show call until then. - const modalPromiseRef = useRef | undefined>(undefined); - const onCancelRef = useRef<(() => void) | undefined>(undefined); - const [reportPDFFilename] = useOnyx(`${ONYXKEYS.COLLECTION.NVP_EXPENSIFY_REPORT_PDF_FILENAME}${activeReportID}`); const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${activeReportID}`); const [session] = useOnyx(ONYXKEYS.SESSION); @@ -76,15 +58,19 @@ const useReportPDFDownloadModal = () => { return translate('reportDetailsPage.successPDF'); })(); - const getStatus = () => { + const status = (() => { if (hasPDFDownloadFailed) { return CONST.FILE_DOWNLOAD_STATUS.FAILED; } return hasFinishedPDFDownload ? CONST.FILE_DOWNLOAD_STATUS.READY : CONST.FILE_DOWNLOAD_STATUS.GENERATING; - }; + })(); - const getModalProps = (): Omit => ({ - status: getStatus(), + // reportPDFFilename is null while the backend is still generating, so the modal reports a dismissal at that point + // as a cancel (e.g. Submit via PDF retracts the submit); once it holds a filename or 'error' the submit has resolved. + const {showFileDownloadModal} = useFileDownloadModal({ + id: REPORT_PDF_DOWNLOAD_MODAL_ID, + title: translate('reportDetailsPage.generatingPDF'), + status, message, shouldTreatModalAsCovering: true, onDownload: () => { @@ -93,50 +79,15 @@ const useReportPDFDownloadModal = () => { } downloadReportPDF(reportPDFFilename, reportName, translate, currentUserLogin, encryptedAuthToken); }, - // reportPDFFilename is null while the backend is still generating, so a dismissal at that point is a cancel - // (e.g. Submit via PDF retracts the submit); once it holds a filename or 'error' the submit has already resolved. - onClose: () => { - if (!isOpenRef.current) { - return; - } - isOpenRef.current = false; - if (!reportPDFFilename) { - onCancelRef.current?.(); - } - }, - }); - - const updateOpenModal = useEffectEvent(() => { - if (!isOpenRef.current) { - return; - } - context.showModal({component: FileDownloadModalWrapper, id: REPORT_PDF_DOWNLOAD_MODAL_ID, props: getModalProps()}); }); - // Push the latest generation state into the open modal so it stays reactive without remounting. - useEffect(() => { - updateOpenModal(); - }, [activeReportID, reportPDFFilename, reportName, message, currentUserLogin, encryptedAuthToken]); - - const showReportPDFDownloadModal = ({reportID, onCancel}: ReportPDFDownloadModalOptions) => { - onCancelRef.current = onCancel; + /** Resolves once the modal has finished hiding, with `CLOSE` if it was dismissed while the PDF was still generating. */ + const showReportPDFDownloadModal = ({reportID}: ReportPDFDownloadModalOptions) => { setActiveReportID(reportID); - - // The modal is still on the stack (open or hiding), so the effect above updates it in place while it is open. - if (modalPromiseRef.current) { - return modalPromiseRef.current; - } - - isOpenRef.current = true; - const modalPromise = context.showModal({component: FileDownloadModalWrapper, id: REPORT_PDF_DOWNLOAD_MODAL_ID, props: getModalProps()}).then((result) => { - isOpenRef.current = false; - modalPromiseRef.current = undefined; - onCancelRef.current = undefined; + return showFileDownloadModal().then((result) => { setActiveReportID(undefined); return result; }); - modalPromiseRef.current = modalPromise; - return modalPromise; }; return {showReportPDFDownloadModal}; diff --git a/tests/unit/components/FileDownloadModalWrapperTest.tsx b/tests/unit/components/FileDownloadModalWrapperTest.tsx index 5126f4c2b277..a908d98c7912 100644 --- a/tests/unit/components/FileDownloadModalWrapperTest.tsx +++ b/tests/unit/components/FileDownloadModalWrapperTest.tsx @@ -23,6 +23,7 @@ describe('FileDownloadModalWrapper', () => { { it('removes itself from the modal stack once, and only after the modal has finished hiding', () => { // Given the wrapper rendered by the global modal stack const closeModal = jest.fn(); - const onClose = jest.fn(); - render(getWrapperElement({closeModal, onClose})); + render(getWrapperElement({closeModal})); expect(getLatestModalProps()?.isVisible).toBe(true); // When a hide event fires while the modal is still visible @@ -54,9 +54,8 @@ describe('FileDownloadModalWrapper', () => { getLatestModalProps()?.onClose?.(); }); - // Then the caller is told right away, and the modal starts hiding but is not removed until the hide animation - // ends, so callers awaiting the show promise run their cleanup after the modal is gone - expect(onClose).toHaveBeenCalledTimes(1); + // Then the modal starts hiding but is not removed until the hide animation ends, so callers awaiting the show + // promise run their cleanup after the modal is gone expect(getLatestModalProps()?.isVisible).toBe(false); expect(closeModal).not.toHaveBeenCalled(); @@ -66,11 +65,56 @@ describe('FileDownloadModalWrapper', () => { getLatestModalProps()?.onModalHide?.(); }); - // Then the wrapper removes itself exactly once + // Then the wrapper removes itself exactly once, reporting a dismissal while generating as a cancel expect(closeModal).toHaveBeenCalledTimes(1); expect(closeModal).toHaveBeenCalledWith({action: ModalActions.CLOSE}); }); + it.each([CONST.FILE_DOWNLOAD_STATUS.READY, CONST.FILE_DOWNLOAD_STATUS.FAILED])('confirms when dismissed once the file is %s', (status) => { + // Given a file that is no longer generating, so there is nothing left to cancel + const closeModal = jest.fn(); + render(getWrapperElement({closeModal, status})); + + // When the user closes the modal and it finishes hiding + act(() => { + getLatestModalProps()?.onClose?.(); + }); + act(() => { + getLatestModalProps()?.onModalHide?.(); + }); + + // Then the show promise resolves as confirmed, so callers like Submit via PDF keep what they did + expect(closeModal).toHaveBeenCalledWith({action: ModalActions.CONFIRM}); + }); + + it('keeps a dismissed modal unchanged while it hides', () => { + // Given the user dismissed the modal while the file was generating + const closeModal = jest.fn(); + const {rerender} = render(getWrapperElement({closeModal})); + act(() => { + getLatestModalProps()?.onClose?.(); + }); + + // When the caller updates the modal in place with the ready state during the hide animation + rerender(getWrapperElement({closeModal, status: CONST.FILE_DOWNLOAD_STATUS.READY, message: 'reportDetailsPage.successPDF'})); + + // Then the hiding modal keeps the state it had when dismissed, and still reports the dismissal as a cancel + expect(screen.getByText('message')).toBeOnTheScreen(); + expect(screen.queryByText('common.download')).not.toBeOnTheScreen(); + act(() => { + getLatestModalProps()?.onModalHide?.(); + }); + expect(closeModal).toHaveBeenCalledWith({action: ModalActions.CLOSE}); + }); + + it('shows the header text the caller passes', () => { + // Given a caller with its own header text + render(getWrapperElement({title: 'reportDetailsPage.generatingPDF'})); + + // Then the modal shows it + expect(screen.getByText('reportDetailsPage.generatingPDF')).toBeOnTheScreen(); + }); + it('shows the spinner and a cancel button while the file is still generating', () => { // Given a file that is still generating render(getWrapperElement({status: CONST.FILE_DOWNLOAD_STATUS.GENERATING, message: 'reportDetailsPage.waitForPDF'})); @@ -110,8 +154,7 @@ describe('FileDownloadModalWrapper', () => { it('closes the modal after a manual download only when asked to', () => { // Given a ready file whose caller wants the modal closed on download const onDownload = jest.fn(); - const onClose = jest.fn(); - render(getWrapperElement({onDownload, onClose, status: CONST.FILE_DOWNLOAD_STATUS.READY, shouldCloseOnDownload: true})); + render(getWrapperElement({onDownload, status: CONST.FILE_DOWNLOAD_STATUS.READY, shouldCloseOnDownload: true})); onDownload.mockClear(); // When the user presses the download button @@ -119,7 +162,6 @@ describe('FileDownloadModalWrapper', () => { // Then the file downloads and the modal starts hiding expect(onDownload).toHaveBeenCalledTimes(1); - expect(onClose).toHaveBeenCalledTimes(1); expect(getLatestModalProps()?.isVisible).toBe(false); }); }); diff --git a/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx b/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx index 311dc7a7b9a6..09d4a13f1ebb 100644 --- a/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx +++ b/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx @@ -1,7 +1,7 @@ import {act, renderHook} from '@testing-library/react-native'; import FileDownloadModal from '@components/Modal/FileDownloadModal'; -import {ModalProvider} from '@components/Modal/Global/ModalContext'; +import {ModalActions, ModalProvider} from '@components/Modal/Global/ModalContext'; import useExpensifyCardStatementPDFDownloadModal from '@hooks/useExpensifyCardStatementPDFDownloadModal'; import useOnyx from '@hooks/useOnyx'; @@ -74,6 +74,7 @@ describe('useExpensifyCardStatementPDFDownloadModal', () => { expect(getLatestModalProps()).toEqual( expect.objectContaining({ isVisible: true, + title: 'reportDetailsPage.generatingPDF', status: CONST.FILE_DOWNLOAD_STATUS.GENERATING, message: 'reportDetailsPage.waitForPDF', shouldCloseOnDownload: true, @@ -89,18 +90,25 @@ describe('useExpensifyCardStatementPDFDownloadModal', () => { }); await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement.pdf'}, rerender); - // Then the same modal instance switches to the ready state, and both callers share one close promise - expect(secondPromise).toBe(firstPromise); + // Then the same modal instance switches to the ready state expect(getLatestModalProps()).toEqual(expect.objectContaining({isVisible: true, status: CONST.FILE_DOWNLOAD_STATUS.READY, message: 'reportDetailsPage.successPDF'})); expect(mockOnFileDownloadModalMount).toHaveBeenCalledTimes(1); - // When the modal downloads the file - act(() => { + // When the modal downloads the file, which also closes it, and it finishes hiding + await act(async () => { getLatestModalProps()?.onDownload(); + getLatestModalProps()?.onClose(); + await waitForBatchedUpdates(); + }); + await act(async () => { + getLatestModalProps()?.onModalHide?.(); + await waitForBatchedUpdates(); }); - // Then it downloads the statement for that key + // Then it downloads the statement for that key, and both callers learn the flow completed expect(downloadExpensifyCardStatementPDF).toHaveBeenCalledWith(expect.any(Function), 'statement.pdf', STATEMENT_KEY, 'employee@mail.com', 'token', 'development'); + await expect(firstPromise).resolves.toEqual({action: ModalActions.CONFIRM}); + await expect(secondPromise).resolves.toEqual({action: ModalActions.CONFIRM}); }); it('keeps the generating state while the statement is still generating', async () => { diff --git a/tests/unit/hooks/useReportPDFDownloadModalTest.tsx b/tests/unit/hooks/useReportPDFDownloadModalTest.tsx index d9ea86600f91..04bc736c67eb 100644 --- a/tests/unit/hooks/useReportPDFDownloadModalTest.tsx +++ b/tests/unit/hooks/useReportPDFDownloadModalTest.tsx @@ -1,7 +1,7 @@ import {act, renderHook} from '@testing-library/react-native'; import FileDownloadModal from '@components/Modal/FileDownloadModal'; -import {ModalProvider} from '@components/Modal/Global/ModalContext'; +import {ModalActions, ModalProvider} from '@components/Modal/Global/ModalContext'; import useOnyx from '@hooks/useOnyx'; import useReportPDFDownloadModal from '@hooks/useReportPDFDownloadModal'; @@ -76,7 +76,13 @@ describe('useReportPDFDownloadModal', () => { // Then it shows the generating state with the spinner copy expect(getLatestModalProps()).toEqual( - expect.objectContaining({isVisible: true, status: CONST.FILE_DOWNLOAD_STATUS.GENERATING, message: 'reportDetailsPage.waitForPDF', shouldTreatModalAsCovering: true}), + expect.objectContaining({ + isVisible: true, + title: 'reportDetailsPage.generatingPDF', + status: CONST.FILE_DOWNLOAD_STATUS.GENERATING, + message: 'reportDetailsPage.waitForPDF', + shouldTreatModalAsCovering: true, + }), ); // When the backend pushes the generated filename @@ -110,42 +116,53 @@ describe('useReportPDFDownloadModal', () => { expect(getLatestModalProps()).toEqual(expect.objectContaining({status: CONST.FILE_DOWNLOAD_STATUS.FAILED, message: 'reportDetailsPage.errorPDF'})); }); - it('cancels once when dismissed while the PDF is still generating', async () => { - // Given the modal opened by Submit via PDF, which retracts the submit when cancelled - const onCancel = jest.fn(); + it('resolves with CLOSE when dismissed while the PDF is still generating', async () => { + // Given the modal opened by Submit via PDF, which retracts the submit when the modal is cancelled + const onResolve = jest.fn(); const {result} = renderReportPDFDownloadModal(); await act(async () => { - result.current.showReportPDFDownloadModal({reportID: REPORT_ID, onCancel}); + result.current.showReportPDFDownloadModal({reportID: REPORT_ID}).then(onResolve); await waitForBatchedUpdates(); }); // When the user dismisses it before the filename arrives, and the close fires again from another control - act(() => { + await act(async () => { getLatestModalProps()?.onClose(); getLatestModalProps()?.onClose(); + await waitForBatchedUpdates(); + }); + await act(async () => { + getLatestModalProps()?.onModalHide?.(); + await waitForBatchedUpdates(); }); - // Then the submit is retracted exactly once - expect(onCancel).toHaveBeenCalledTimes(1); + // Then the caller is told once that the modal was cancelled + expect(onResolve).toHaveBeenCalledTimes(1); + expect(onResolve).toHaveBeenCalledWith({action: ModalActions.CLOSE}); }); - it('does not cancel when dismissed after the PDF is ready', async () => { + it('resolves with CONFIRM when dismissed after the PDF is ready', async () => { // Given the modal opened by Submit via PDF and the filename has arrived, so the submit has already resolved - const onCancel = jest.fn(); + const onResolve = jest.fn(); const {result, rerender} = renderReportPDFDownloadModal(); await act(async () => { - result.current.showReportPDFDownloadModal({reportID: REPORT_ID, onCancel}); + result.current.showReportPDFDownloadModal({reportID: REPORT_ID}).then(onResolve); await waitForBatchedUpdates(); }); await setOnyxValue(PDF_FILENAME_KEY, 'report.pdf', rerender); - // When the user dismisses the modal - act(() => { + // When the user dismisses the modal and it finishes hiding + await act(async () => { getLatestModalProps()?.onClose(); + await waitForBatchedUpdates(); + }); + await act(async () => { + getLatestModalProps()?.onModalHide?.(); + await waitForBatchedUpdates(); }); - // Then the submit is kept - expect(onCancel).not.toHaveBeenCalled(); + // Then the caller is told the flow completed, so the submit is kept + expect(onResolve).toHaveBeenCalledWith({action: ModalActions.CONFIRM}); }); it('resolves the show promise only after the modal has finished hiding', async () => { From 522afc5d766465cd4ebb2a9a7e8c8ce44ebe6ac5 Mon Sep 17 00:00:00 2001 From: "daledah (via MelvinBot)" Date: Thu, 22 Oct 2026 09:19:57 +0000 Subject: [PATCH 4/8] Fix: Apply oxfmt formatting to MoneyReportHeaderModals Co-authored-by: daledah --- src/components/MoneyReportHeaderModals.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/components/MoneyReportHeaderModals.tsx b/src/components/MoneyReportHeaderModals.tsx index 25581c9bb387..7821666a916b 100644 --- a/src/components/MoneyReportHeaderModals.tsx +++ b/src/components/MoneyReportHeaderModals.tsx @@ -109,11 +109,7 @@ function MoneyReportHeaderModalsContent({reportID, children}: MoneyReportHeaderM showDownloadErrorModal, }; - return ( - - {children} - - ); + return {children}; } export default MoneyReportHeaderModals; From 69947a0ba7a7fe7a5787ac103cc9c7582a93ff61 Mon Sep 17 00:00:00 2001 From: "daledah (via MelvinBot)" Date: Thu, 22 Oct 2026 18:54:14 +0000 Subject: [PATCH 5/8] Open a new file download modal when shown right after closing by id, and guard the report PDF Onyx keys Co-authored-by: daledah --- src/components/Modal/Global/ModalContext.tsx | 14 ++++---- ...eExpensifyCardStatementPDFDownloadModal.ts | 3 +- src/hooks/useFileDownloadModal.ts | 9 +++-- src/hooks/useReportPDFDownloadModal.ts | 5 +-- tests/unit/ModalContextCloseByIDTest.tsx | 36 +++++++++++++++++++ ...nsifyCardStatementPDFDownloadModalTest.tsx | 31 ++++++++++++++++ 6 files changed, 87 insertions(+), 11 deletions(-) diff --git a/src/components/Modal/Global/ModalContext.tsx b/src/components/Modal/Global/ModalContext.tsx index deccf915cc89..48a6e711d6bd 100644 --- a/src/components/Modal/Global/ModalContext.tsx +++ b/src/components/Modal/Global/ModalContext.tsx @@ -137,17 +137,19 @@ function ModalProvider({children}: {children: React.ReactNode}) { // about has gone away and the user never answered. Such a caller cannot use `closeModal`, because by then another // modal may sit above it and that is the one `closeModal` would take down. const closeModalByID: ModalContextType['closeModalByID'] = (id, data = {action: ModalActions.CLOSE}) => { + // Released right away rather than in the state updater, so a show call with the same id in the same tick + // pushes a new modal instead of updating the one being closed and getting its promise back. + const modalPromise = modalPromisesStack.current?.[id]; + if (modalPromise) { + modalPromise.resolve(data); + delete modalPromisesStack.current[id]; + } + setModalStack((prevState) => { if (!prevState.modals.some((modal) => modal.id === id)) { return prevState; } - const modalPromise = modalPromisesStack.current?.[id]; - if (modalPromise) { - modalPromise.resolve(data); - delete modalPromisesStack.current[id]; - } - return { ...prevState, modals: prevState.modals.filter((modal) => modal.id !== id), diff --git a/src/hooks/useExpensifyCardStatementPDFDownloadModal.ts b/src/hooks/useExpensifyCardStatementPDFDownloadModal.ts index 02fc27750f8a..3d9f265117af 100644 --- a/src/hooks/useExpensifyCardStatementPDFDownloadModal.ts +++ b/src/hooks/useExpensifyCardStatementPDFDownloadModal.ts @@ -54,7 +54,8 @@ const useExpensifyCardStatementPDFDownloadModal = () => { const showExpensifyCardStatementPDFDownloadModal = (options: ExpensifyCardStatementPDFDownloadModalOptions) => { setStatementParams(options.statementParams); return showFileDownloadModal().then((result) => { - setStatementParams(undefined); + // A modal closed by id can resolve after a newer show call, so leave that call's params in place. + setStatementParams((currentParams) => (currentParams === options.statementParams ? undefined : currentParams)); return result; }); }; diff --git a/src/hooks/useFileDownloadModal.ts b/src/hooks/useFileDownloadModal.ts index e3f2badcfe64..eb7cd921e250 100644 --- a/src/hooks/useFileDownloadModal.ts +++ b/src/hooks/useFileDownloadModal.ts @@ -67,8 +67,11 @@ const useFileDownloadModal = ({id, onDownload, title, status, message, shouldClo isOpenRef.current = true; const modalPromise = context.showModal({component: FileDownloadModalWrapper, id, props: getModalProps()}).then((result) => { - isOpenRef.current = false; - modalPromiseRef.current = undefined; + // A modal closed by id resolves after a newer one may have been shown, so only clear the state it still owns. + if (modalPromiseRef.current === modalPromise) { + isOpenRef.current = false; + modalPromiseRef.current = undefined; + } return result; }); modalPromiseRef.current = modalPromise; @@ -77,6 +80,8 @@ const useFileDownloadModal = ({id, onDownload, title, status, message, shouldClo const closeFileDownloadModal = () => { isOpenRef.current = false; + // Cleared here too so a show call right after closing opens a new modal instead of returning the closed one's promise. + modalPromiseRef.current = undefined; context.closeModalByID(id); }; diff --git a/src/hooks/useReportPDFDownloadModal.ts b/src/hooks/useReportPDFDownloadModal.ts index 83444d21f15d..70e262e60c99 100644 --- a/src/hooks/useReportPDFDownloadModal.ts +++ b/src/hooks/useReportPDFDownloadModal.ts @@ -1,4 +1,5 @@ import {downloadReportPDF} from '@libs/actions/Report'; +import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -21,8 +22,8 @@ const REPORT_PDF_DOWNLOAD_MODAL_ID = 'reportPDFDownloadModal'; const useReportPDFDownloadModal = () => { const [activeReportID, setActiveReportID] = useState(); - const [reportPDFFilename] = useOnyx(`${ONYXKEYS.COLLECTION.NVP_EXPENSIFY_REPORT_PDF_FILENAME}${activeReportID}`); - const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${activeReportID}`); + const [reportPDFFilename] = useOnyx(`${ONYXKEYS.COLLECTION.NVP_EXPENSIFY_REPORT_PDF_FILENAME}${getNonEmptyStringOnyxID(activeReportID)}`); + const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(activeReportID)}`); const [session] = useOnyx(ONYXKEYS.SESSION); const {translate} = useLocalize(); diff --git a/tests/unit/ModalContextCloseByIDTest.tsx b/tests/unit/ModalContextCloseByIDTest.tsx index 19b346f0fc26..88cc1a877af6 100644 --- a/tests/unit/ModalContextCloseByIDTest.tsx +++ b/tests/unit/ModalContextCloseByIDTest.tsx @@ -114,4 +114,40 @@ describe('ModalProvider closeModalByID', () => { // Then it resolves on its own promise, proving the id was released rather than left pointing at the dead one expect(secondResult).toEqual({action: ModalActions.CONFIRM}); }); + + it('should show a new modal when the same id is shown in the same tick it was closed by id', async () => { + // Given an owned prompt + const {result} = renderModalContext(); + + let firstResult: {action: string} | undefined; + await act(async () => { + result.current.showModal({component: StubModal, id: OWNED_MODAL_ID}).then((payload) => { + firstResult = payload; + }); + await waitForBatchedUpdates(); + }); + + // When it is closed by id and shown again before React has processed the close + let secondResult: {action: string} | undefined; + await act(async () => { + result.current.closeModalByID(OWNED_MODAL_ID); + result.current.showModal({component: StubModal, id: OWNED_MODAL_ID}).then((payload) => { + secondResult = payload; + }); + await waitForBatchedUpdates(); + }); + + // Then the first prompt resolves as a close while the second one stays open on its own promise + expect(firstResult).toEqual({action: ModalActions.CLOSE}); + expect(secondResult).toBeUndefined(); + + // When the user answers the second prompt + await act(async () => { + result.current.closeModal({action: ModalActions.CONFIRM}); + await waitForBatchedUpdates(); + }); + + // Then it resolves with that answer + expect(secondResult).toEqual({action: ModalActions.CONFIRM}); + }); }); diff --git a/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx b/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx index 09d4a13f1ebb..9ccb83af9b14 100644 --- a/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx +++ b/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx @@ -219,4 +219,35 @@ describe('useExpensifyCardStatementPDFDownloadModal', () => { // Then the modal is not shown again expect(mockedFileDownloadModal.mock.calls.length).toBe(renderCountAfterClose); }); + + it('opens a new modal when shown right after closing by id', async () => { + // Given an open modal + const {result, rerender} = renderStatementPDFDownloadModal(); + let firstPromise: Promise | undefined; + await act(async () => { + firstPromise = result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: STATEMENT_PARAMS}); + await waitForBatchedUpdates(); + }); + + // When the caller closes it by id and shows it again in the same tick + let hasSecondResolved = false; + await act(async () => { + result.current.closeExpensifyCardStatementPDFDownloadModal(); + result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: {...STATEMENT_PARAMS, statementKey: STATEMENT_KEY}}).then(() => { + hasSecondResolved = true; + }); + await waitForBatchedUpdates(); + }); + + // Then the closed modal's promise resolves and a new modal is shown instead of reusing the closed one's promise + await expect(firstPromise).resolves.toEqual({action: ModalActions.CLOSE}); + expect(hasSecondResolved).toBe(false); + expect(getLatestModalProps()?.isVisible).toBe(true); + + // When the file arrives + await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement.pdf'}, rerender); + + // Then the new modal still receives updates + expect(getLatestModalProps()).toEqual(expect.objectContaining({isVisible: true, status: CONST.FILE_DOWNLOAD_STATUS.READY})); + }); }); From a66607ce1c6f8b5f93770bd35f06828c87d2dade Mon Sep 17 00:00:00 2001 From: "daledah (via MelvinBot)" Date: Fri, 23 Oct 2026 02:55:54 +0000 Subject: [PATCH 6/8] Split the report PDF modal comment into two sentences Co-authored-by: daledah --- src/hooks/useReportPDFDownloadModal.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/useReportPDFDownloadModal.ts b/src/hooks/useReportPDFDownloadModal.ts index 70e262e60c99..b2d5b23dab7b 100644 --- a/src/hooks/useReportPDFDownloadModal.ts +++ b/src/hooks/useReportPDFDownloadModal.ts @@ -67,7 +67,7 @@ const useReportPDFDownloadModal = () => { })(); // reportPDFFilename is null while the backend is still generating, so the modal reports a dismissal at that point - // as a cancel (e.g. Submit via PDF retracts the submit); once it holds a filename or 'error' the submit has resolved. + // as a cancel (e.g. Submit via PDF retracts the submit). Once it holds a filename or 'error' the submit has resolved. const {showFileDownloadModal} = useFileDownloadModal({ id: REPORT_PDF_DOWNLOAD_MODAL_ID, title: translate('reportDetailsPage.generatingPDF'), From 7e2a88c81994efad56db90d36b395dac475a5b7e Mon Sep 17 00:00:00 2001 From: "daledah (via MelvinBot)" Date: Fri, 23 Oct 2026 10:22:30 +0000 Subject: [PATCH 7/8] Move the PDF download modal subscriptions into components the global modal stack renders Co-authored-by: daledah --- ...ExpensifyCardStatementPDFDownloadModal.tsx | 60 ++++++++++++ .../Modal/Global/FileDownloadModalWrapper.tsx | 4 +- src/components/ReportPDFDownloadModal.tsx | 88 +++++++++++++++++ ...eExpensifyCardStatementPDFDownloadModal.ts | 68 +++----------- src/hooks/useFileDownloadModal.ts | 91 ------------------ src/hooks/useReportPDFDownloadModal.ts | 94 ++----------------- ...nsifyCardStatementPDFDownloadModalTest.tsx | 68 ++++++++++---- .../hooks/useReportPDFDownloadModalTest.tsx | 70 ++++++++++---- 8 files changed, 273 insertions(+), 270 deletions(-) create mode 100644 src/components/ExpensifyCardStatementPDFDownloadModal.tsx create mode 100644 src/components/ReportPDFDownloadModal.tsx delete mode 100644 src/hooks/useFileDownloadModal.ts diff --git a/src/components/ExpensifyCardStatementPDFDownloadModal.tsx b/src/components/ExpensifyCardStatementPDFDownloadModal.tsx new file mode 100644 index 000000000000..b43e9224761d --- /dev/null +++ b/src/components/ExpensifyCardStatementPDFDownloadModal.tsx @@ -0,0 +1,60 @@ +import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; +import useEnvironment from '@hooks/useEnvironment'; +import useLocalize from '@hooks/useLocalize'; +import useOnyx from '@hooks/useOnyx'; + +import {downloadExpensifyCardStatementPDF} from '@libs/ExpensifyCardStatementUtils'; +import type {ExpensifyCardStatementParams} from '@libs/ExpensifyCardStatementUtils'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; + +import React from 'react'; + +import type {ModalProps} from './Modal/Global/ModalContext'; + +import FileDownloadModalWrapper from './Modal/Global/FileDownloadModalWrapper'; + +type ExpensifyCardStatementPDFDownloadModalProps = ModalProps & { + statementParams: ExpensifyCardStatementParams; +}; + +/** + * Rendered by the global modal stack, so its Onyx subscription lives as long as the modal does. The caller that opened + * it can unmount while the statement is generating and the modal still picks up the file. + */ +function ExpensifyCardStatementPDFDownloadModal({statementParams, closeModal, resolveModal}: ExpensifyCardStatementPDFDownloadModalProps) { + const [expensifyCardStatement] = useOnyx(ONYXKEYS.EXPENSIFY_CARD_STATEMENT); + const [session] = useOnyx(ONYXKEYS.SESSION); + const {translate} = useLocalize(); + const {environment} = useEnvironment(); + const currentUserPersonalDetails = useCurrentUserPersonalDetails(); + const currentUserLogin = currentUserPersonalDetails?.login ?? ''; + const encryptedAuthToken = session?.encryptedAuthToken ?? ''; + + const statementKey = statementParams.statementKey; + const isGeneratingPDF = expensifyCardStatement?.isGenerating === true; + const statementFileName = statementKey ? expensifyCardStatement?.[statementKey] : undefined; + const hasFinishedPDFDownload = !!statementKey && !isGeneratingPDF && typeof statementFileName === 'string' && statementFileName.length > 0; + + return ( + { + if (!statementKey || typeof statementFileName !== 'string') { + return; + } + downloadExpensifyCardStatementPDF(translate, statementFileName, statementKey, currentUserLogin, encryptedAuthToken, environment); + }} + /> + ); +} + +export default ExpensifyCardStatementPDFDownloadModal; +export type {ExpensifyCardStatementPDFDownloadModalProps}; diff --git a/src/components/Modal/Global/FileDownloadModalWrapper.tsx b/src/components/Modal/Global/FileDownloadModalWrapper.tsx index 789c9aab661a..f9550a14479c 100644 --- a/src/components/Modal/Global/FileDownloadModalWrapper.tsx +++ b/src/components/Modal/Global/FileDownloadModalWrapper.tsx @@ -16,8 +16,8 @@ type FileDownloadModalWrapperProps = ModalProps & Omit(); const closeActionRef = useRef>(ModalActions.CLOSE); const hasClosedModalRef = useRef(false); diff --git a/src/components/ReportPDFDownloadModal.tsx b/src/components/ReportPDFDownloadModal.tsx new file mode 100644 index 000000000000..6bcc4f4e75d4 --- /dev/null +++ b/src/components/ReportPDFDownloadModal.tsx @@ -0,0 +1,88 @@ +import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; +import useLocalize from '@hooks/useLocalize'; +import useNetwork from '@hooks/useNetwork'; +import useOnyx from '@hooks/useOnyx'; +import useStallLogger from '@hooks/useStallLogger'; + +import {downloadReportPDF} from '@libs/actions/Report'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; + +import React from 'react'; + +import type {ModalProps} from './Modal/Global/ModalContext'; + +import FileDownloadModalWrapper from './Modal/Global/FileDownloadModalWrapper'; + +type ReportPDFDownloadModalProps = ModalProps & { + reportID: string; +}; + +/** + * Rendered by the global modal stack, so its Onyx subscriptions live as long as the modal does. The caller that opened + * it can unmount while the PDF is generating and the modal still picks up the filename. + */ +function ReportPDFDownloadModal({reportID, closeModal, resolveModal}: ReportPDFDownloadModalProps) { + const [reportPDFFilename] = useOnyx(`${ONYXKEYS.COLLECTION.NVP_EXPENSIFY_REPORT_PDF_FILENAME}${reportID}`); + const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`); + const [session] = useOnyx(ONYXKEYS.SESSION); + + const {translate} = useLocalize(); + const currentUserPersonalDetails = useCurrentUserPersonalDetails(); + const currentUserLogin = currentUserPersonalDetails?.login ?? ''; + const encryptedAuthToken = session?.encryptedAuthToken ?? ''; + const reportName = report?.reportName ?? ''; + const {isOffline} = useNetwork(); + + const hasPDFDownloadFailed = reportPDFFilename === CONST.REPORT_DETAILS_MENU_ITEM.ERROR; + const hasFinishedPDFDownload = !!reportPDFFilename && !hasPDFDownloadFailed; + + // reportPDFFilename only ever resolves via a backend-pushed Onyx update (no client timeout), so a filename + // that never arrives and never errors leaves the spinner stuck with nothing else to log it. See Expensify#667674. + // Excluded while offline: the request is legitimately queued and waiting for connectivity, not stuck. + // Keyed by reportID (not just a boolean) so if this modal is ever reused for a different report without + // unmounting, the timer restarts against the new report instead of firing late and blaming the wrong one. + useStallLogger(!isOffline && !reportPDFFilename ? reportID : false, '[PDFStall] reportPDFFilename never resolved to a filename or error while the download modal was open', { + reportID, + }); + + const message = (() => { + if (hasPDFDownloadFailed) { + return translate('reportDetailsPage.errorPDF'); + } + if (!hasFinishedPDFDownload) { + return translate('reportDetailsPage.waitForPDF'); + } + return translate('reportDetailsPage.successPDF'); + })(); + + // reportPDFFilename is null while the backend is still generating, so the wrapper reports a dismissal at that point + // as a cancel (e.g. Submit via PDF retracts the submit). Once it holds a filename or 'error' the submit has resolved. + const status = (() => { + if (hasPDFDownloadFailed) { + return CONST.FILE_DOWNLOAD_STATUS.FAILED; + } + return hasFinishedPDFDownload ? CONST.FILE_DOWNLOAD_STATUS.READY : CONST.FILE_DOWNLOAD_STATUS.GENERATING; + })(); + + return ( + { + if (!hasFinishedPDFDownload) { + return; + } + downloadReportPDF(reportPDFFilename, reportName, translate, currentUserLogin, encryptedAuthToken); + }} + /> + ); +} + +export default ReportPDFDownloadModal; +export type {ReportPDFDownloadModalProps}; diff --git a/src/hooks/useExpensifyCardStatementPDFDownloadModal.ts b/src/hooks/useExpensifyCardStatementPDFDownloadModal.ts index 3d9f265117af..db9fadd2f786 100644 --- a/src/hooks/useExpensifyCardStatementPDFDownloadModal.ts +++ b/src/hooks/useExpensifyCardStatementPDFDownloadModal.ts @@ -1,66 +1,24 @@ -import {downloadExpensifyCardStatementPDF} from '@libs/ExpensifyCardStatementUtils'; -import type {ExpensifyCardStatementParams} from '@libs/ExpensifyCardStatementUtils'; +import ExpensifyCardStatementPDFDownloadModal from '@components/ExpensifyCardStatementPDFDownloadModal'; +import type {ExpensifyCardStatementPDFDownloadModalProps} from '@components/ExpensifyCardStatementPDFDownloadModal'; +import {useModal} from '@components/Modal/Global/ModalContext'; +import type {ModalProps} from '@components/Modal/Global/ModalContext'; -import CONST from '@src/CONST'; -import ONYXKEYS from '@src/ONYXKEYS'; +type ExpensifyCardStatementPDFDownloadModalOptions = Omit; -import {useState} from 'react'; - -import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails'; -import useEnvironment from './useEnvironment'; -import useFileDownloadModal from './useFileDownloadModal'; -import useLocalize from './useLocalize'; -import useOnyx from './useOnyx'; - -type ExpensifyCardStatementPDFDownloadModalOptions = { - statementParams: ExpensifyCardStatementParams; -}; - -// A second show call updates the open modal in place, which is how the statementKey reaches it once -// GetExpensifyCardStatementPDF returns. +// A fixed id lets a second show call update the open modal in place, which is how the statementKey reaches it +// once GetExpensifyCardStatementPDF returns. const EXPENSIFY_CARD_STATEMENT_PDF_DOWNLOAD_MODAL_ID = 'expensifyCardStatementPDFDownloadModal'; const useExpensifyCardStatementPDFDownloadModal = () => { - const [statementParams, setStatementParams] = useState(); - - const [expensifyCardStatement] = useOnyx(ONYXKEYS.EXPENSIFY_CARD_STATEMENT); - const [session] = useOnyx(ONYXKEYS.SESSION); - const {translate} = useLocalize(); - const {environment} = useEnvironment(); - const currentUserPersonalDetails = useCurrentUserPersonalDetails(); - const currentUserLogin = currentUserPersonalDetails?.login ?? ''; - const encryptedAuthToken = session?.encryptedAuthToken ?? ''; - - const statementKey = statementParams?.statementKey; - const isGeneratingPDF = expensifyCardStatement?.isGenerating === true; - const statementFileName = statementKey ? expensifyCardStatement?.[statementKey] : undefined; - const hasFinishedPDFDownload = !!statementKey && !isGeneratingPDF && typeof statementFileName === 'string' && statementFileName.length > 0; + const context = useModal(); - const {showFileDownloadModal, closeFileDownloadModal} = useFileDownloadModal({ - id: EXPENSIFY_CARD_STATEMENT_PDF_DOWNLOAD_MODAL_ID, - title: translate('reportDetailsPage.generatingPDF'), - status: hasFinishedPDFDownload ? CONST.FILE_DOWNLOAD_STATUS.READY : CONST.FILE_DOWNLOAD_STATUS.GENERATING, - message: hasFinishedPDFDownload ? translate('reportDetailsPage.successPDF') : translate('reportDetailsPage.waitForPDF'), - shouldCloseOnDownload: true, - shouldUseSuccessButton: true, - onDownload: () => { - if (!statementKey || typeof statementFileName !== 'string') { - return; - } - downloadExpensifyCardStatementPDF(translate, statementFileName, statementKey, currentUserLogin, encryptedAuthToken, environment); - }, - }); + /** Resolves once the modal has finished hiding, with `CLOSE` if it was dismissed while the statement was still generating. */ + const showExpensifyCardStatementPDFDownloadModal = (options: ExpensifyCardStatementPDFDownloadModalOptions) => + context.showModal({component: ExpensifyCardStatementPDFDownloadModal, id: EXPENSIFY_CARD_STATEMENT_PDF_DOWNLOAD_MODAL_ID, props: options}); - const showExpensifyCardStatementPDFDownloadModal = (options: ExpensifyCardStatementPDFDownloadModalOptions) => { - setStatementParams(options.statementParams); - return showFileDownloadModal().then((result) => { - // A modal closed by id can resolve after a newer show call, so leave that call's params in place. - setStatementParams((currentParams) => (currentParams === options.statementParams ? undefined : currentParams)); - return result; - }); - }; + const closeExpensifyCardStatementPDFDownloadModal = () => context.closeModalByID(EXPENSIFY_CARD_STATEMENT_PDF_DOWNLOAD_MODAL_ID); - return {showExpensifyCardStatementPDFDownloadModal, closeExpensifyCardStatementPDFDownloadModal: closeFileDownloadModal}; + return {showExpensifyCardStatementPDFDownloadModal, closeExpensifyCardStatementPDFDownloadModal}; }; export default useExpensifyCardStatementPDFDownloadModal; diff --git a/src/hooks/useFileDownloadModal.ts b/src/hooks/useFileDownloadModal.ts deleted file mode 100644 index eb7cd921e250..000000000000 --- a/src/hooks/useFileDownloadModal.ts +++ /dev/null @@ -1,91 +0,0 @@ -import type {FileDownloadModalWrapperProps} from '@components/Modal/Global/FileDownloadModalWrapper'; -import FileDownloadModalWrapper from '@components/Modal/Global/FileDownloadModalWrapper'; -import type {ModalProps, ModalStateChangePayload} from '@components/Modal/Global/ModalContext'; -import {useModal} from '@components/Modal/Global/ModalContext'; - -import {useEffect, useEffectEvent, useRef} from 'react'; - -type FileDownloadModalOptions = Omit & { - /** - * Fixed id of the modal on the global stack. A show call with the same id updates the open modal in place instead - * of stacking a new one, which keeps it reactive as the file generates. - */ - id: string; -}; - -/** - * Shows a file download modal on the global modal stack and keeps it in sync with the options passed on every render, - * so callers only derive the status, copy, and download action for their file. - * - * The show promise resolves once the modal has finished hiding, with `CLOSE` if the user dismissed it while the file - * was still generating and `CONFIRM` otherwise. - */ -const useFileDownloadModal = ({id, onDownload, title, status, message, shouldCloseOnDownload, shouldUseSuccessButton, shouldTreatModalAsCovering}: FileDownloadModalOptions) => { - const context = useModal(); - - // Whether the modal is on the stack. It turns false once the modal is gone, so a late update can't show it again: a - // show call with the fixed id after the modal has closed pushes a new modal instead of updating it. - const isOpenRef = useRef(false); - - // Resolves once the modal has finished hiding. Set from the first show call until then. - const modalPromiseRef = useRef | undefined>(undefined); - - // Read through a ref so a download always uses the caller's latest data, without pushing the modal again every time - // the caller re-renders with a new callback. - const onDownloadRef = useRef(onDownload); - useEffect(() => { - onDownloadRef.current = onDownload; - }, [onDownload]); - - const getModalProps = (): Omit => ({ - title, - status, - message, - shouldCloseOnDownload, - shouldUseSuccessButton, - shouldTreatModalAsCovering, - onDownload: () => onDownloadRef.current(), - }); - - const updateOpenModal = useEffectEvent(() => { - if (!isOpenRef.current) { - return; - } - context.showModal({component: FileDownloadModalWrapper, id, props: getModalProps()}); - }); - - // Push the latest state into the open modal so it stays reactive without remounting. - useEffect(() => { - updateOpenModal(); - }, [title, status, message, shouldCloseOnDownload, shouldUseSuccessButton, shouldTreatModalAsCovering]); - - const showFileDownloadModal = () => { - // The modal is still on the stack (open or hiding), so the effect above updates it in place. - if (modalPromiseRef.current) { - return modalPromiseRef.current; - } - - isOpenRef.current = true; - const modalPromise = context.showModal({component: FileDownloadModalWrapper, id, props: getModalProps()}).then((result) => { - // A modal closed by id resolves after a newer one may have been shown, so only clear the state it still owns. - if (modalPromiseRef.current === modalPromise) { - isOpenRef.current = false; - modalPromiseRef.current = undefined; - } - return result; - }); - modalPromiseRef.current = modalPromise; - return modalPromise; - }; - - const closeFileDownloadModal = () => { - isOpenRef.current = false; - // Cleared here too so a show call right after closing opens a new modal instead of returning the closed one's promise. - modalPromiseRef.current = undefined; - context.closeModalByID(id); - }; - - return {showFileDownloadModal, closeFileDownloadModal}; -}; - -export default useFileDownloadModal; diff --git a/src/hooks/useReportPDFDownloadModal.ts b/src/hooks/useReportPDFDownloadModal.ts index b2d5b23dab7b..f69c77beea8a 100644 --- a/src/hooks/useReportPDFDownloadModal.ts +++ b/src/hooks/useReportPDFDownloadModal.ts @@ -1,95 +1,15 @@ -import {downloadReportPDF} from '@libs/actions/Report'; -import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; +import {useModal} from '@components/Modal/Global/ModalContext'; +import type {ModalProps} from '@components/Modal/Global/ModalContext'; +import ReportPDFDownloadModal from '@components/ReportPDFDownloadModal'; +import type {ReportPDFDownloadModalProps} from '@components/ReportPDFDownloadModal'; -import CONST from '@src/CONST'; -import ONYXKEYS from '@src/ONYXKEYS'; - -import {useState} from 'react'; - -import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails'; -import useFileDownloadModal from './useFileDownloadModal'; -import useLocalize from './useLocalize'; -import useNetwork from './useNetwork'; -import useOnyx from './useOnyx'; -import useStallLogger from './useStallLogger'; - -type ReportPDFDownloadModalOptions = { - reportID: string; -}; - -const REPORT_PDF_DOWNLOAD_MODAL_ID = 'reportPDFDownloadModal'; +type ReportPDFDownloadModalOptions = Omit; const useReportPDFDownloadModal = () => { - const [activeReportID, setActiveReportID] = useState(); - - const [reportPDFFilename] = useOnyx(`${ONYXKEYS.COLLECTION.NVP_EXPENSIFY_REPORT_PDF_FILENAME}${getNonEmptyStringOnyxID(activeReportID)}`); - const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(activeReportID)}`); - const [session] = useOnyx(ONYXKEYS.SESSION); - - const {translate} = useLocalize(); - const currentUserPersonalDetails = useCurrentUserPersonalDetails(); - const currentUserLogin = currentUserPersonalDetails?.login ?? ''; - const encryptedAuthToken = session?.encryptedAuthToken ?? ''; - const reportName = report?.reportName ?? ''; - const {isOffline} = useNetwork(); - - const hasPDFDownloadFailed = reportPDFFilename === CONST.REPORT_DETAILS_MENU_ITEM.ERROR; - const hasFinishedPDFDownload = !!reportPDFFilename && !hasPDFDownloadFailed; - - // reportPDFFilename only ever resolves via a backend-pushed Onyx update (no client timeout), so a filename - // that never arrives and never errors leaves the spinner stuck with nothing else to log it. See Expensify#667674. - // Excluded while offline: the request is legitimately queued and waiting for connectivity, not stuck. - // Keyed by reportID (not just a boolean) so if the modal is shown for a different report, the timer restarts - // against the new report instead of firing late and blaming the wrong one. - useStallLogger( - !!activeReportID && !isOffline && !reportPDFFilename ? activeReportID : false, - '[PDFStall] reportPDFFilename never resolved to a filename or error while the download modal was open', - { - reportID: activeReportID, - }, - ); - - const message = (() => { - if (hasPDFDownloadFailed) { - return translate('reportDetailsPage.errorPDF'); - } - if (!hasFinishedPDFDownload) { - return translate('reportDetailsPage.waitForPDF'); - } - return translate('reportDetailsPage.successPDF'); - })(); - - const status = (() => { - if (hasPDFDownloadFailed) { - return CONST.FILE_DOWNLOAD_STATUS.FAILED; - } - return hasFinishedPDFDownload ? CONST.FILE_DOWNLOAD_STATUS.READY : CONST.FILE_DOWNLOAD_STATUS.GENERATING; - })(); - - // reportPDFFilename is null while the backend is still generating, so the modal reports a dismissal at that point - // as a cancel (e.g. Submit via PDF retracts the submit). Once it holds a filename or 'error' the submit has resolved. - const {showFileDownloadModal} = useFileDownloadModal({ - id: REPORT_PDF_DOWNLOAD_MODAL_ID, - title: translate('reportDetailsPage.generatingPDF'), - status, - message, - shouldTreatModalAsCovering: true, - onDownload: () => { - if (!hasFinishedPDFDownload) { - return; - } - downloadReportPDF(reportPDFFilename, reportName, translate, currentUserLogin, encryptedAuthToken); - }, - }); + const context = useModal(); /** Resolves once the modal has finished hiding, with `CLOSE` if it was dismissed while the PDF was still generating. */ - const showReportPDFDownloadModal = ({reportID}: ReportPDFDownloadModalOptions) => { - setActiveReportID(reportID); - return showFileDownloadModal().then((result) => { - setActiveReportID(undefined); - return result; - }); - }; + const showReportPDFDownloadModal = (options: ReportPDFDownloadModalOptions) => context.showModal({component: ReportPDFDownloadModal, props: options}); return {showReportPDFDownloadModal}; }; diff --git a/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx b/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx index 9ccb83af9b14..a85e1ddaae4c 100644 --- a/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx +++ b/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx @@ -11,7 +11,7 @@ import {downloadExpensifyCardStatementPDF} from '@libs/ExpensifyCardStatementUti import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import React from 'react'; +import React, {useSyncExternalStore} from 'react'; import waitForBatchedUpdates from '../../utils/waitForBatchedUpdates'; @@ -36,6 +36,14 @@ const STATEMENT_PARAMS = {policyID: 'policy1', feedCountry: 'US', entryIDs: [123 const STATEMENT_KEY = 'statement-key'; let mockOnyxValues: Record = {}; +const onyxListeners = new Set<() => void>(); +const subscribeToOnyx = (listener: () => void) => { + onyxListeners.add(listener); + return () => onyxListeners.delete(listener); +}; + +// A minimal reactive store, so an update re-renders the modal the global modal stack renders, like a real Onyx subscription. +const useMockOnyx = (key: string) => [useSyncExternalStore(subscribeToOnyx, () => mockOnyxValues[key])]; describe('useExpensifyCardStatementPDFDownloadModal', () => { const mockedFileDownloadModal = jest.mocked(FileDownloadModal); @@ -46,11 +54,13 @@ describe('useExpensifyCardStatementPDFDownloadModal', () => { wrapper: ({children}: {children: React.ReactNode}) => {children}, }); - // Mirrors a backend-pushed Onyx update: change the value, then re-render so the hook reads it. - const setStatementInOnyx = async (statement: Record, rerender: (props: unknown) => void) => { - mockOnyxValues = {...mockOnyxValues, [ONYXKEYS.EXPENSIFY_CARD_STATEMENT]: statement}; - rerender({}); + // Mirrors a backend-pushed Onyx update. + const setStatementInOnyx = async (statement: Record) => { await act(async () => { + mockOnyxValues = {...mockOnyxValues, [ONYXKEYS.EXPENSIFY_CARD_STATEMENT]: statement}; + for (const listener of onyxListeners) { + listener(); + } await waitForBatchedUpdates(); }); }; @@ -58,12 +68,12 @@ describe('useExpensifyCardStatementPDFDownloadModal', () => { beforeEach(() => { jest.clearAllMocks(); mockOnyxValues = {[ONYXKEYS.SESSION]: {encryptedAuthToken: 'token'}}; - (jest.mocked(useOnyx) as jest.Mock).mockImplementation((key: string) => [mockOnyxValues[key]]); + (jest.mocked(useOnyx) as jest.Mock).mockImplementation(useMockOnyx); }); it('updates the open modal in place as the statementKey and file arrive', async () => { // Given the modal opened before GetExpensifyCardStatementPDF has returned a statementKey - const {result, rerender} = renderStatementPDFDownloadModal(); + const {result} = renderStatementPDFDownloadModal(); let firstPromise: Promise | undefined; await act(async () => { firstPromise = result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: STATEMENT_PARAMS}); @@ -88,7 +98,7 @@ describe('useExpensifyCardStatementPDFDownloadModal', () => { secondPromise = result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: {...STATEMENT_PARAMS, statementKey: STATEMENT_KEY}}); await waitForBatchedUpdates(); }); - await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement.pdf'}, rerender); + await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement.pdf'}); // Then the same modal instance switches to the ready state expect(getLatestModalProps()).toEqual(expect.objectContaining({isVisible: true, status: CONST.FILE_DOWNLOAD_STATUS.READY, message: 'reportDetailsPage.successPDF'})); @@ -111,16 +121,40 @@ describe('useExpensifyCardStatementPDFDownloadModal', () => { await expect(secondPromise).resolves.toEqual({action: ModalActions.CONFIRM}); }); + it('keeps updating the modal after the caller unmounts', async () => { + // Given a caller, such as the Search page, that opens the modal and then unmounts while the statement is generating + let shouldRenderCaller = true; + const {result, rerender} = renderHook(() => useExpensifyCardStatementPDFDownloadModal(), { + wrapper: ({children}: {children: React.ReactNode}) => {shouldRenderCaller ? children : null}, + }); + await act(async () => { + result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: {...STATEMENT_PARAMS, statementKey: STATEMENT_KEY}}); + await waitForBatchedUpdates(); + }); + shouldRenderCaller = false; + rerender({}); + + // When the backend pushes the generated file + await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement.pdf'}); + + // Then the modal, which owns its own subscription, still switches to the ready state and offers the download + expect(getLatestModalProps()).toEqual(expect.objectContaining({isVisible: true, status: CONST.FILE_DOWNLOAD_STATUS.READY})); + act(() => { + getLatestModalProps()?.onDownload(); + }); + expect(downloadExpensifyCardStatementPDF).toHaveBeenCalledWith(expect.any(Function), 'statement.pdf', STATEMENT_KEY, 'employee@mail.com', 'token', 'development'); + }); + it('keeps the generating state while the statement is still generating', async () => { // Given the modal opened with a statementKey whose file is cached but being regenerated - const {result, rerender} = renderStatementPDFDownloadModal(); + const {result} = renderStatementPDFDownloadModal(); await act(async () => { result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: {...STATEMENT_PARAMS, statementKey: STATEMENT_KEY}}); await waitForBatchedUpdates(); }); // When Onyx says the statement is generating - await setStatementInOnyx({isGenerating: true, [STATEMENT_KEY]: 'statement.pdf'}, rerender); + await setStatementInOnyx({isGenerating: true, [STATEMENT_KEY]: 'statement.pdf'}); // Then the modal still waits instead of offering a stale file expect(getLatestModalProps()?.status).toBe(CONST.FILE_DOWNLOAD_STATUS.GENERATING); @@ -159,7 +193,7 @@ describe('useExpensifyCardStatementPDFDownloadModal', () => { it('does not reopen a dismissed modal when the statementKey arrives during the hide animation', async () => { // Given the user dismissed the modal before the statementKey arrived - const {result, rerender} = renderStatementPDFDownloadModal(); + const {result} = renderStatementPDFDownloadModal(); await act(async () => { result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: STATEMENT_PARAMS}); await waitForBatchedUpdates(); @@ -174,7 +208,7 @@ describe('useExpensifyCardStatementPDFDownloadModal', () => { result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: {...STATEMENT_PARAMS, statementKey: STATEMENT_KEY}}); await waitForBatchedUpdates(); }); - await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement.pdf'}, rerender); + await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement.pdf'}); // Then the hiding modal is left alone expect(getLatestModalProps()).toEqual(expect.objectContaining({isVisible: false, status: CONST.FILE_DOWNLOAD_STATUS.GENERATING})); @@ -185,7 +219,7 @@ describe('useExpensifyCardStatementPDFDownloadModal', () => { await waitForBatchedUpdates(); }); const renderCountAfterHide = mockedFileDownloadModal.mock.calls.length; - await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement-2.pdf'}, rerender); + await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement-2.pdf'}); // Then the modal is not shown again expect(mockedFileDownloadModal.mock.calls.length).toBe(renderCountAfterHide); @@ -194,7 +228,7 @@ describe('useExpensifyCardStatementPDFDownloadModal', () => { it('closes the modal by id without reopening it on a later update', async () => { // Given an open modal waiting on the statement - const {result, rerender} = renderStatementPDFDownloadModal(); + const {result} = renderStatementPDFDownloadModal(); let hasResolved = false; await act(async () => { result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: {...STATEMENT_PARAMS, statementKey: STATEMENT_KEY}}).then(() => { @@ -214,7 +248,7 @@ describe('useExpensifyCardStatementPDFDownloadModal', () => { expect(hasResolved).toBe(true); // When the file arrives afterwards - await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement.pdf'}, rerender); + await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement.pdf'}); // Then the modal is not shown again expect(mockedFileDownloadModal.mock.calls.length).toBe(renderCountAfterClose); @@ -222,7 +256,7 @@ describe('useExpensifyCardStatementPDFDownloadModal', () => { it('opens a new modal when shown right after closing by id', async () => { // Given an open modal - const {result, rerender} = renderStatementPDFDownloadModal(); + const {result} = renderStatementPDFDownloadModal(); let firstPromise: Promise | undefined; await act(async () => { firstPromise = result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: STATEMENT_PARAMS}); @@ -245,7 +279,7 @@ describe('useExpensifyCardStatementPDFDownloadModal', () => { expect(getLatestModalProps()?.isVisible).toBe(true); // When the file arrives - await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement.pdf'}, rerender); + await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement.pdf'}); // Then the new modal still receives updates expect(getLatestModalProps()).toEqual(expect.objectContaining({isVisible: true, status: CONST.FILE_DOWNLOAD_STATUS.READY})); diff --git a/tests/unit/hooks/useReportPDFDownloadModalTest.tsx b/tests/unit/hooks/useReportPDFDownloadModalTest.tsx index 04bc736c67eb..5c406b0c9103 100644 --- a/tests/unit/hooks/useReportPDFDownloadModalTest.tsx +++ b/tests/unit/hooks/useReportPDFDownloadModalTest.tsx @@ -12,7 +12,7 @@ import {downloadReportPDF} from '@libs/actions/Report'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import React from 'react'; +import React, {useSyncExternalStore} from 'react'; import waitForBatchedUpdates from '../../utils/waitForBatchedUpdates'; @@ -38,6 +38,14 @@ const REPORT_ID = '1'; const PDF_FILENAME_KEY = `${ONYXKEYS.COLLECTION.NVP_EXPENSIFY_REPORT_PDF_FILENAME}${REPORT_ID}`; let mockOnyxValues: Record = {}; +const onyxListeners = new Set<() => void>(); +const subscribeToOnyx = (listener: () => void) => { + onyxListeners.add(listener); + return () => onyxListeners.delete(listener); +}; + +// A minimal reactive store, so an update re-renders the modal the global modal stack renders, like a real Onyx subscription. +const useMockOnyx = (key: string) => [useSyncExternalStore(subscribeToOnyx, () => mockOnyxValues[key])]; describe('useReportPDFDownloadModal', () => { const mockedFileDownloadModal = jest.mocked(FileDownloadModal); @@ -48,11 +56,13 @@ describe('useReportPDFDownloadModal', () => { wrapper: ({children}: {children: React.ReactNode}) => {children}, }); - // Mirrors a backend-pushed Onyx update: change the value, then re-render so the hook reads it. - const setOnyxValue = async (key: string, value: unknown, rerender: (props: unknown) => void) => { - mockOnyxValues = {...mockOnyxValues, [key]: value}; - rerender({}); + // Mirrors a backend-pushed Onyx update. + const setOnyxValue = async (key: string, value: unknown) => { await act(async () => { + mockOnyxValues = {...mockOnyxValues, [key]: value}; + for (const listener of onyxListeners) { + listener(); + } await waitForBatchedUpdates(); }); }; @@ -63,12 +73,12 @@ describe('useReportPDFDownloadModal', () => { [`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`]: {reportID: REPORT_ID, reportName: 'Expense Report'}, [ONYXKEYS.SESSION]: {encryptedAuthToken: 'token'}, }; - (jest.mocked(useOnyx) as jest.Mock).mockImplementation((key: string) => [mockOnyxValues[key]]); + (jest.mocked(useOnyx) as jest.Mock).mockImplementation(useMockOnyx); }); it('updates the open modal in place as the PDF generates', async () => { // Given the modal opened for a report whose PDF is still generating - const {result, rerender} = renderReportPDFDownloadModal(); + const {result} = renderReportPDFDownloadModal(); await act(async () => { result.current.showReportPDFDownloadModal({reportID: REPORT_ID}); await waitForBatchedUpdates(); @@ -86,7 +96,7 @@ describe('useReportPDFDownloadModal', () => { ); // When the backend pushes the generated filename - await setOnyxValue(PDF_FILENAME_KEY, 'report.pdf', rerender); + await setOnyxValue(PDF_FILENAME_KEY, 'report.pdf'); // Then the same modal instance switches to the ready state instead of being remounted expect(getLatestModalProps()).toEqual(expect.objectContaining({isVisible: true, status: CONST.FILE_DOWNLOAD_STATUS.READY, message: 'reportDetailsPage.successPDF'})); @@ -103,14 +113,14 @@ describe('useReportPDFDownloadModal', () => { it('shows the failed state when the PDF generation fails', async () => { // Given the modal opened for a report - const {result, rerender} = renderReportPDFDownloadModal(); + const {result} = renderReportPDFDownloadModal(); await act(async () => { result.current.showReportPDFDownloadModal({reportID: REPORT_ID}); await waitForBatchedUpdates(); }); // When the backend reports the generation failed - await setOnyxValue(PDF_FILENAME_KEY, CONST.REPORT_DETAILS_MENU_ITEM.ERROR, rerender); + await setOnyxValue(PDF_FILENAME_KEY, CONST.REPORT_DETAILS_MENU_ITEM.ERROR); // Then the modal shows the error state, which stops the spinner expect(getLatestModalProps()).toEqual(expect.objectContaining({status: CONST.FILE_DOWNLOAD_STATUS.FAILED, message: 'reportDetailsPage.errorPDF'})); @@ -144,12 +154,12 @@ describe('useReportPDFDownloadModal', () => { it('resolves with CONFIRM when dismissed after the PDF is ready', async () => { // Given the modal opened by Submit via PDF and the filename has arrived, so the submit has already resolved const onResolve = jest.fn(); - const {result, rerender} = renderReportPDFDownloadModal(); + const {result} = renderReportPDFDownloadModal(); await act(async () => { result.current.showReportPDFDownloadModal({reportID: REPORT_ID}).then(onResolve); await waitForBatchedUpdates(); }); - await setOnyxValue(PDF_FILENAME_KEY, 'report.pdf', rerender); + await setOnyxValue(PDF_FILENAME_KEY, 'report.pdf'); // When the user dismisses the modal and it finishes hiding await act(async () => { @@ -198,7 +208,7 @@ describe('useReportPDFDownloadModal', () => { it('does not reopen a dismissed modal when the filename arrives late', async () => { // Given the user dismissed the modal while the PDF was still generating - const {result, rerender} = renderReportPDFDownloadModal(); + const {result} = renderReportPDFDownloadModal(); await act(async () => { result.current.showReportPDFDownloadModal({reportID: REPORT_ID}); await waitForBatchedUpdates(); @@ -209,24 +219,48 @@ describe('useReportPDFDownloadModal', () => { }); // When the filename arrives while the modal is hiding, and again after it has gone - await setOnyxValue(PDF_FILENAME_KEY, 'report.pdf', rerender); + await setOnyxValue(PDF_FILENAME_KEY, 'report.pdf'); expect(getLatestModalProps()).toEqual(expect.objectContaining({isVisible: false, status: CONST.FILE_DOWNLOAD_STATUS.GENERATING})); await act(async () => { getLatestModalProps()?.onModalHide?.(); await waitForBatchedUpdates(); }); const renderCountAfterHide = mockedFileDownloadModal.mock.calls.length; - await setOnyxValue(PDF_FILENAME_KEY, 'report-2.pdf', rerender); + await setOnyxValue(PDF_FILENAME_KEY, 'report-2.pdf'); // Then the modal is not shown again expect(mockedFileDownloadModal.mock.calls.length).toBe(renderCountAfterHide); expect(mockOnFileDownloadModalMount).toHaveBeenCalledTimes(1); }); + it('keeps updating the modal after the caller unmounts', async () => { + // Given a caller, such as the report header, that opens the modal and then unmounts while the PDF is generating + let shouldRenderCaller = true; + const {result, rerender} = renderHook(() => useReportPDFDownloadModal(), { + wrapper: ({children}: {children: React.ReactNode}) => {shouldRenderCaller ? children : null}, + }); + await act(async () => { + result.current.showReportPDFDownloadModal({reportID: REPORT_ID}); + await waitForBatchedUpdates(); + }); + shouldRenderCaller = false; + rerender({}); + + // When the backend pushes the generated filename + await setOnyxValue(PDF_FILENAME_KEY, 'report.pdf'); + + // Then the modal, which owns its own subscriptions, still switches to the ready state and offers the download + expect(getLatestModalProps()).toEqual(expect.objectContaining({isVisible: true, status: CONST.FILE_DOWNLOAD_STATUS.READY})); + act(() => { + getLatestModalProps()?.onDownload(); + }); + expect(downloadReportPDF).toHaveBeenCalledWith('report.pdf', 'Expense Report', expect.any(Function), 'employee@mail.com', 'token'); + }); + it('logs a stall only while the open modal is waiting on the filename', async () => { // Given the modal opened for a report whose PDF is still generating - const {result, rerender} = renderReportPDFDownloadModal(); - expect(jest.mocked(useStallLogger).mock.calls.at(-1)?.[0]).toBe(false); + const {result} = renderReportPDFDownloadModal(); + expect(useStallLogger).not.toHaveBeenCalled(); await act(async () => { result.current.showReportPDFDownloadModal({reportID: REPORT_ID}); await waitForBatchedUpdates(); @@ -236,7 +270,7 @@ describe('useReportPDFDownloadModal', () => { expect(jest.mocked(useStallLogger).mock.calls.at(-1)?.[0]).toBe(REPORT_ID); // When the filename arrives - await setOnyxValue(PDF_FILENAME_KEY, 'report.pdf', rerender); + await setOnyxValue(PDF_FILENAME_KEY, 'report.pdf'); // Then the stall timer stops expect(jest.mocked(useStallLogger).mock.calls.at(-1)?.[0]).toBe(false); From 4895720e2ca9d5500c0e4197147b431883ea2f3b Mon Sep 17 00:00:00 2001 From: "daledah (via MelvinBot)" Date: Fri, 23 Oct 2026 16:57:52 +0000 Subject: [PATCH 8/8] Wrap openPDFDownload, revert the closeModalByID change, and keep resolveModal out of FileDownloadModal Co-authored-by: daledah --- .../Modal/Global/FileDownloadModalWrapper.tsx | 4 +-- src/components/Modal/Global/ModalContext.tsx | 14 ++++---- .../MoneyReportHeaderSecondaryActions.tsx | 4 ++- tests/unit/ModalContextCloseByIDTest.tsx | 36 ------------------- ...nsifyCardStatementPDFDownloadModalTest.tsx | 31 ---------------- 5 files changed, 10 insertions(+), 79 deletions(-) diff --git a/src/components/Modal/Global/FileDownloadModalWrapper.tsx b/src/components/Modal/Global/FileDownloadModalWrapper.tsx index f9550a14479c..4d729fdd8e30 100644 --- a/src/components/Modal/Global/FileDownloadModalWrapper.tsx +++ b/src/components/Modal/Global/FileDownloadModalWrapper.tsx @@ -13,7 +13,7 @@ import {ModalActions} from './ModalContext'; type FileDownloadModalWrapperProps = ModalProps & Omit; -function FileDownloadModalWrapper({closeModal, ...props}: FileDownloadModalWrapperProps) { +function FileDownloadModalWrapper({closeModal, resolveModal, ...props}: FileDownloadModalWrapperProps) { const [isVisible, setIsVisible] = useState(true); // The props at the moment the user dismissed the modal. The props keep changing as the file generates until the @@ -46,8 +46,6 @@ function FileDownloadModalWrapper({closeModal, ...props}: FileDownloadModalWrapp return ( { - // Released right away rather than in the state updater, so a show call with the same id in the same tick - // pushes a new modal instead of updating the one being closed and getting its promise back. - const modalPromise = modalPromisesStack.current?.[id]; - if (modalPromise) { - modalPromise.resolve(data); - delete modalPromisesStack.current[id]; - } - setModalStack((prevState) => { if (!prevState.modals.some((modal) => modal.id === id)) { return prevState; } + const modalPromise = modalPromisesStack.current?.[id]; + if (modalPromise) { + modalPromise.resolve(data); + delete modalPromisesStack.current[id]; + } + return { ...prevState, modals: prevState.modals.filter((modal) => modal.id !== id), diff --git a/src/components/MoneyReportHeaderActions/MoneyReportHeaderSecondaryActions.tsx b/src/components/MoneyReportHeaderActions/MoneyReportHeaderSecondaryActions.tsx index 866ead5ca229..1590cfa78d83 100644 --- a/src/components/MoneyReportHeaderActions/MoneyReportHeaderSecondaryActions.tsx +++ b/src/components/MoneyReportHeaderActions/MoneyReportHeaderSecondaryActions.tsx @@ -394,7 +394,9 @@ function MoneyReportHeaderSecondaryActionsInner({reportID, primaryAction, isRepo const {exportActionEntries} = useExportActions({ reportID, policy, - onPDFModalOpen: openPDFDownload, + onPDFModalOpen: () => { + openPDFDownload(); + }, }); const shareActions = useShareActions({reportID, policy}); diff --git a/tests/unit/ModalContextCloseByIDTest.tsx b/tests/unit/ModalContextCloseByIDTest.tsx index 88cc1a877af6..19b346f0fc26 100644 --- a/tests/unit/ModalContextCloseByIDTest.tsx +++ b/tests/unit/ModalContextCloseByIDTest.tsx @@ -114,40 +114,4 @@ describe('ModalProvider closeModalByID', () => { // Then it resolves on its own promise, proving the id was released rather than left pointing at the dead one expect(secondResult).toEqual({action: ModalActions.CONFIRM}); }); - - it('should show a new modal when the same id is shown in the same tick it was closed by id', async () => { - // Given an owned prompt - const {result} = renderModalContext(); - - let firstResult: {action: string} | undefined; - await act(async () => { - result.current.showModal({component: StubModal, id: OWNED_MODAL_ID}).then((payload) => { - firstResult = payload; - }); - await waitForBatchedUpdates(); - }); - - // When it is closed by id and shown again before React has processed the close - let secondResult: {action: string} | undefined; - await act(async () => { - result.current.closeModalByID(OWNED_MODAL_ID); - result.current.showModal({component: StubModal, id: OWNED_MODAL_ID}).then((payload) => { - secondResult = payload; - }); - await waitForBatchedUpdates(); - }); - - // Then the first prompt resolves as a close while the second one stays open on its own promise - expect(firstResult).toEqual({action: ModalActions.CLOSE}); - expect(secondResult).toBeUndefined(); - - // When the user answers the second prompt - await act(async () => { - result.current.closeModal({action: ModalActions.CONFIRM}); - await waitForBatchedUpdates(); - }); - - // Then it resolves with that answer - expect(secondResult).toEqual({action: ModalActions.CONFIRM}); - }); }); diff --git a/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx b/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx index a85e1ddaae4c..e12736581a3a 100644 --- a/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx +++ b/tests/unit/hooks/useExpensifyCardStatementPDFDownloadModalTest.tsx @@ -253,35 +253,4 @@ describe('useExpensifyCardStatementPDFDownloadModal', () => { // Then the modal is not shown again expect(mockedFileDownloadModal.mock.calls.length).toBe(renderCountAfterClose); }); - - it('opens a new modal when shown right after closing by id', async () => { - // Given an open modal - const {result} = renderStatementPDFDownloadModal(); - let firstPromise: Promise | undefined; - await act(async () => { - firstPromise = result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: STATEMENT_PARAMS}); - await waitForBatchedUpdates(); - }); - - // When the caller closes it by id and shows it again in the same tick - let hasSecondResolved = false; - await act(async () => { - result.current.closeExpensifyCardStatementPDFDownloadModal(); - result.current.showExpensifyCardStatementPDFDownloadModal({statementParams: {...STATEMENT_PARAMS, statementKey: STATEMENT_KEY}}).then(() => { - hasSecondResolved = true; - }); - await waitForBatchedUpdates(); - }); - - // Then the closed modal's promise resolves and a new modal is shown instead of reusing the closed one's promise - await expect(firstPromise).resolves.toEqual({action: ModalActions.CLOSE}); - expect(hasSecondResolved).toBe(false); - expect(getLatestModalProps()?.isVisible).toBe(true); - - // When the file arrives - await setStatementInOnyx({isGenerating: false, [STATEMENT_KEY]: 'statement.pdf'}); - - // Then the new modal still receives updates - expect(getLatestModalProps()).toEqual(expect.objectContaining({isVisible: true, status: CONST.FILE_DOWNLOAD_STATUS.READY})); - }); });