[Payment due @marufsharifi] Show export status in one app-level modal instead of per screen #98521
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1014,6 +1014,7 @@ | |
| "superapp", | ||
| "superpowered", | ||
| "supportal", | ||
| "surfaceable", | ||
| "svgs", | ||
| "symbolicate", | ||
| "symbolicated", | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| import useOnyx from '@hooks/useOnyx'; | ||
|
|
||
| import {clearExportDownload, wasExportInitiatedLocally} from '@libs/actions/Export'; | ||
|
|
||
| import CONST from '@src/CONST'; | ||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
|
|
||
| import React, {useState} from 'react'; | ||
|
|
||
| import ExportDownloadStatusModal from './ExportDownloadStatusModal'; | ||
|
|
||
| /** | ||
| * Renders the export status modal for the whole app. It watches the export collection and shows the modal | ||
| * for the active export (preparing or ready). Because it is the single owner of the modal and reads straight | ||
| * from Onyx, a screen only has to start an export (which writes its record); this shows the progress, delivers | ||
| * the file when it is ready, and still surfaces it after a reload. There is no per-screen modal to coordinate with. | ||
| */ | ||
| function ExportDownloadStatusManager() { | ||
| const [exportDownloads, exportDownloadsMetadata] = useOnyx(ONYXKEYS.COLLECTION.EXPORT_DOWNLOAD); | ||
| const [openExportID, setOpenExportID] = useState<string | undefined>(); | ||
|
|
||
| // Record keys that already existed when this tab loaded. A reload re-reads the collection and re-seeds this, | ||
| // which is how an in-flight export resurfaces after a reload. A record that appears later because another | ||
| // tab started an export is not in here, so it does not pop the modal open in this tab until it reloads. | ||
| const [recordKeysAtLoad, setRecordKeysAtLoad] = useState<Set<string> | undefined>(undefined); | ||
| if (recordKeysAtLoad === undefined && exportDownloadsMetadata.status === 'loaded') { | ||
| setRecordKeysAtLoad(new Set(Object.keys(exportDownloads ?? {}))); | ||
| } | ||
|
|
||
| const surfaceableCandidate = Object.entries(exportDownloads ?? {}).find(([key, exportDownload]) => { | ||
| if (!exportDownload) { | ||
| return false; | ||
| } | ||
|
|
||
| // Concierge hand-off is owned by the BE worker: it delivers via the Concierge chat on success and posts | ||
| // a failure notice there too. There is nothing useful to show in a modal, so we never surface these. | ||
| if (exportDownload.shouldSendFromConcierge) { | ||
| return false; | ||
| } | ||
| const isSurfaceableState = exportDownload.state === CONST.EXPORT_DOWNLOAD.STATE.PREPARING || exportDownload.state === CONST.EXPORT_DOWNLOAD.STATE.READY; | ||
| if (!isSurfaceableState) { | ||
| return false; | ||
| } | ||
|
|
||
| // Only surface an export this tab owns: one it started itself, or one that already existed when it | ||
|
mollfpr marked this conversation as resolved.
|
||
| // loaded. An export started in another tab must not pop open here until this tab reloads. | ||
| const exportID = key.replace(ONYXKEYS.COLLECTION.EXPORT_DOWNLOAD, ''); | ||
| return !!recordKeysAtLoad?.has(key) || wasExportInitiatedLocally(exportID); | ||
| }); | ||
| const [surfaceableKey] = surfaceableCandidate ?? []; | ||
| const surfaceableExportID = surfaceableKey?.replace(ONYXKEYS.COLLECTION.EXPORT_DOWNLOAD, ''); | ||
|
|
||
| // Adjust state to avoid a flash. openExportID keeps the modal open through the state changes | ||
| // the selector above skips (like the flip to shouldSendFromConcierge), and close it only once | ||
| // the record is gone. | ||
| if (!openExportID && surfaceableExportID) { | ||
| setOpenExportID(surfaceableExportID); | ||
| } | ||
| if (openExportID && !exportDownloads?.[`${ONYXKEYS.COLLECTION.EXPORT_DOWNLOAD}${openExportID}`]) { | ||
|
Contributor
There was a problem hiding this comment. Untested branch: reset when the tracked record disappears When the tracked record is removed (e.g. cleared from another tab), the manager must drop Add to it('resets and surfaces a new export after the tracked record is removed', async () => {
await Onyx.set(EXPORT_KEY, {state: 'ready', fileName: CSV_FILE_NAME});
render(<ExportDownloadStatusManager />);
await waitForBatchedUpdatesWithAct();
await Onyx.set(EXPORT_KEY, null);
await waitForBatchedUpdatesWithAct();
expect(screen.queryByText('exportDownload.readyTitle')).toBeNull();
const SECOND_KEY = `${ONYXKEYS.COLLECTION.EXPORT_DOWNLOAD}second-export` as const;
await Onyx.set(SECOND_KEY, {state: 'preparing'});
await waitForBatchedUpdatesWithAct();
expect(screen.getByText('exportDownload.preparingTitle')).toBeTruthy();
}); |
||
| setOpenExportID(undefined); | ||
| } | ||
|
|
||
| if (!openExportID) { | ||
| return null; | ||
| } | ||
|
|
||
| const openExportKey = `${ONYXKEYS.COLLECTION.EXPORT_DOWNLOAD}${openExportID}`; | ||
| const exportDownload = exportDownloads?.[openExportKey]; | ||
| if (!exportDownload) { | ||
| return null; | ||
| } | ||
|
|
||
| const handleClose = () => { | ||
|
Contributor
There was a problem hiding this comment. This component is the heart of the refactor, but it has no test file. Both non-trivial pieces are uncovered:
The deleted |
||
| if (exportDownload.state === CONST.EXPORT_DOWNLOAD.STATE.PREPARING && !exportDownload.shouldSendFromConcierge) { | ||
|
Contributor
There was a problem hiding this comment. Untested branch: preparing close is a no-op Dismissing while still preparing must keep the modal open and clear nothing. The existing test titled "dismissing a preparing export is a no-op" actually presses the Send-from-Concierge button ( Add to it('dismissing while preparing keeps the modal open and clears nothing', async () => {
await Onyx.set(EXPORT_KEY, {state: 'preparing'});
render(<ExportDownloadStatusManager />);
await waitForBatchedUpdatesWithAct();
fireEvent.press(screen.getByText('exportDownload.close')); // or backdrop dismiss
expect(mockClearExportDownload).not.toHaveBeenCalled();
expect(screen.getByText('exportDownload.preparingTitle')).toBeTruthy();
});
Contributor
Author
There was a problem hiding this comment. This is already covered in |
||
| return; | ||
| } | ||
| if (exportDownload.shouldSendFromConcierge) { | ||
|
Contributor
There was a problem hiding this comment. Untested branch: Concierge hand-off close must not clear the record This branch ( Add to it('closing after hand-off to Concierge drops the modal without clearing the record', async () => {
await Onyx.set(EXPORT_KEY, {state: 'preparing'});
render(<ExportDownloadStatusManager />);
await waitForBatchedUpdatesWithAct();
// Worker now owns delivery.
await Onyx.merge(EXPORT_KEY, {shouldSendFromConcierge: true});
await waitForBatchedUpdatesWithAct();
// Trigger onClose (dismiss/backdrop) and assert the record is preserved.
fireEvent.press(screen.getByText('exportDownload.close'));
expect(mockClearExportDownload).not.toHaveBeenCalled();
}); |
||
| setOpenExportID(undefined); | ||
| return; | ||
| } | ||
| clearExportDownload(openExportID, exportDownload); | ||
| setOpenExportID(undefined); | ||
| }; | ||
|
|
||
|
mollfpr marked this conversation as resolved.
|
||
| return ( | ||
| <ExportDownloadStatusModal | ||
| key={openExportID} | ||
| exportID={openExportID} | ||
| isVisible | ||
| onClose={handleClose} | ||
|
mollfpr marked this conversation as resolved.
|
||
| /> | ||
| ); | ||
| } | ||
|
|
||
| ExportDownloadStatusManager.displayName = 'ExportDownloadStatusManager'; | ||
|
|
||
| export default ExportDownloadStatusManager; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,18 +8,23 @@ import usePreviousDefined from '@hooks/usePreviousDefined'; | |
| import useResponsiveLayout from '@hooks/useResponsiveLayout'; | ||
| import useThemeStyles from '@hooks/useThemeStyles'; | ||
|
|
||
| import {isClientTheLeader} from '@libs/ActiveClientManager'; | ||
| import addEncryptedAuthTokenToURL from '@libs/addEncryptedAuthTokenToURL'; | ||
| import {isMobileSafari} from '@libs/Browser'; | ||
| import {getOldDotURLFromEnvironment} from '@libs/Environment/Environment'; | ||
| import fileDownload from '@libs/fileDownload'; | ||
| import {buildSecureDownloadURL} from '@libs/UrlUtils'; | ||
|
|
||
| import {sendExportFileFromConcierge} from '@userActions/Export'; | ||
| import {close} from '@userActions/Modal'; | ||
|
|
||
| import CONST from '@src/CONST'; | ||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
| import type {Session} from '@src/types/onyx'; | ||
|
|
||
| import type {OnyxEntry} from 'react-native-onyx'; | ||
|
|
||
| import React, {useEffect, useRef} from 'react'; | ||
| import {View} from 'react-native'; | ||
|
|
||
| import ActivityIndicator from './ActivityIndicator'; | ||
|
|
@@ -28,6 +33,8 @@ import Modal from './Modal'; | |
| import RenderHTML from './RenderHTML'; | ||
| import Text from './Text'; | ||
|
|
||
| const selectEncryptedAuthToken = (session: OnyxEntry<Session>) => session?.encryptedAuthToken; | ||
|
|
||
| type ExportDownloadStatusModalProps = { | ||
| /** The export ID to subscribe to */ | ||
| exportID: string; | ||
|
|
@@ -51,10 +58,11 @@ function ExportDownloadStatusModal({exportID, isVisible, onClose, failedBody}: E | |
| const {login: currentUserLogin} = useCurrentUserPersonalDetails(); | ||
| const {environment} = useEnvironment(); | ||
|
|
||
| const [encryptedAuthToken] = useOnyx(ONYXKEYS.SESSION, {selector: selectEncryptedAuthToken}); | ||
|
|
||
| const [exportDownload] = useOnyx(`${ONYXKEYS.COLLECTION.EXPORT_DOWNLOAD}${exportID}`); | ||
| const displayedExport = usePreviousDefined(exportDownload); | ||
| const wasRecordCleared = !exportDownload && !!displayedExport; | ||
|
|
||
| const state = displayedExport?.state; | ||
| const shouldSendFromConcierge = displayedExport?.shouldSendFromConcierge; | ||
|
|
@@ -70,9 +78,18 @@ function ExportDownloadStatusModal({exportID, isVisible, onClose, failedBody}: E | |
| const isFailed = state === CONST.EXPORT_DOWNLOAD.STATE.FAILED; | ||
| const isEmptyReceipts = isReady && exportType === CONST.EXPORT_DOWNLOAD.TYPE.RECEIPTS && receiptCount === 0; | ||
|
|
||
| const wasPreparingRef = useRef(false); | ||
| useEffect(() => { | ||
| if (state !== CONST.EXPORT_DOWNLOAD.STATE.PREPARING) { | ||
| return; | ||
| } | ||
|
|
||
| wasPreparingRef.current = true; | ||
| }, [state]); | ||
|
|
||
| // Build the secure download URL the same way downloadReportPDF does, so the host always follows | ||
| // the app's current environment (instead of the env baked into a backend-built URL) and authenticates | ||
| // via the encryptedAuthToken, so no separate OldDot sign-in is needed. | ||
| const downloadFile = () => { | ||
| if (!fileName || !currentUserLogin) { | ||
| return; | ||
|
|
@@ -85,7 +102,8 @@ function ExportDownloadStatusModal({exportID, isVisible, onClose, failedBody}: E | |
| }; | ||
|
|
||
| useEffect(() => { | ||
| // Only the leader tab auto-downloads, so a ready export isn't downloaded once per open tab. | ||
| if (!isReady || !fileName || shouldSendFromConcierge || isEmptyReceipts || !isClientTheLeader() || !wasPreparingRef.current) { | ||
| return; | ||
| } | ||
| downloadFile(); | ||
|
Comment on lines
+105
to
109
Contributor
There was a problem hiding this comment. Re-downloads the file on every reload while a READY export is still on screen
useEffect(() => {
if (!isReady || !fileName || shouldSendFromConcierge || isEmptyReceipts || !isClientTheLeader()) {
return;
}
downloadFile();
}, [isReady, fileName, shouldSendFromConcierge, isEmptyReceipts]);
Suggested fix — only auto-download when this session watched it finish preparing (the const wasPreparingRef = useRef(false);
useEffect(() => {
if (state === CONST.EXPORT_DOWNLOAD.STATE.PREPARING) {
wasPreparingRef.current = true;
}
}, [state]);
useEffect(() => {
// A resurfaced (post-reload) ready export shows the Download button instead of silently re-downloading.
if (!isReady || !fileName || shouldSendFromConcierge || isEmptyReceipts || !isClientTheLeader() || !wasPreparingRef.current) {
return;
}
downloadFile();
}, [isReady, fileName, shouldSendFromConcierge, isEmptyReceipts]);RF-1.webm |
||
|
|
@@ -96,10 +114,14 @@ function ExportDownloadStatusModal({exportID, isVisible, onClose, failedBody}: E | |
| sendExportFileFromConcierge(exportID, displayedExport ?? undefined); | ||
| }; | ||
| const {openConciergeAnywhere} = useOpenConciergeAnywhere(); | ||
| const bottomSafeAreaPaddingStyle = useBottomSafeSafeAreaPaddingStyle({addBottomSafeAreaPadding: isSmallScreenWidth, addOfflineIndicatorBottomSafeAreaPadding: false, style: styles.m5}); | ||
|
|
||
| if (wasRecordCleared) { | ||
| return null; | ||
| } | ||
|
|
||
| const handleGoToConcierge = () => { | ||
| close(() => openConciergeAnywhere({forceConcierge: true})); | ||
| }; | ||
|
|
||
| const handleDownloadFile = () => { | ||
|
|
@@ -110,7 +132,6 @@ function ExportDownloadStatusModal({exportID, isVisible, onClose, failedBody}: E | |
| }; | ||
|
|
||
| const isNonDismissible = isPreparing; | ||
|
|
||
| const renderContent = () => { | ||
| if (isPreparing) { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.