diff --git a/ios/ShareViewController/ShareViewController.swift b/ios/ShareViewController/ShareViewController.swift index 4ad39131a40c..3d3aed365840 100644 --- a/ios/ShareViewController/ShareViewController.swift +++ b/ios/ShareViewController/ShareViewController.swift @@ -36,7 +36,7 @@ class ShareViewController: UIViewController { os_log("Saving file to: %@", filePath.path) do { - try fileData.write(to: filePath, options: .completeFileProtection) + try fileData.write(to: filePath) os_log("File saved successfully at: %@", filePath.path) return filePath } catch { diff --git a/src/libs/fileDownload/checkFileExists/index.ts b/src/libs/fileDownload/checkFileExists/index.ts index de899a7b0c7a..d8feee8ff6fd 100644 --- a/src/libs/fileDownload/checkFileExists/index.ts +++ b/src/libs/fileDownload/checkFileExists/index.ts @@ -1,4 +1,5 @@ import fileURIToPath from '@libs/fileURIToPath'; +import {logReceiptStatFailed} from '@libs/telemetry/ReceiptObservability'; import RNFS from 'react-native-fs'; @@ -22,7 +23,12 @@ function checkFileExists(path: string | undefined): Promise { const statIsFile = (candidate: string) => RNFS.stat(candidate).then((fileStat) => fileStat.isFile()); - return statIsFile(decodedPath).catch(() => (decodedPath === rawPath ? false : statIsFile(rawPath).catch(() => false))); + const statFailed = (error: unknown) => { + logReceiptStatFailed(typeof error === 'object' && error !== null && 'code' in error ? String(error.code) : undefined); + return false; + }; + + return statIsFile(decodedPath).catch((error: unknown) => (decodedPath === rawPath ? statFailed(error) : statIsFile(rawPath).catch(statFailed))); } export default checkFileExists; diff --git a/src/libs/telemetry/ReceiptObservability.ts b/src/libs/telemetry/ReceiptObservability.ts index e832391dbe6e..bc95ca0b8be7 100644 --- a/src/libs/telemetry/ReceiptObservability.ts +++ b/src/libs/telemetry/ReceiptObservability.ts @@ -178,6 +178,13 @@ function logReceiptDropped({ }); } +function logReceiptStatFailed(code: string | undefined) { + Log.info(`${RECEIPT_LOG_PREFIX} stat failed`, false, { + event: 'statFailed', + code, + }); +} + function logReceiptAdoptFailed({error, captureSource}: {error: unknown; captureSource: ReceiptCaptureSource}) { Log.alert(`${RECEIPT_LOG_PREFIX} adopt failed`, { event: 'adoptFailed', @@ -246,6 +253,7 @@ export { logReceiptSubmitted, logReceiptEnqueued, logReceiptDropped, + logReceiptStatFailed, logReceiptAdoptFailed, logReceiptQueueSnapshot, getPickerCaptureSource, diff --git a/src/libs/telemetry/forwardLogsToSentry.ts b/src/libs/telemetry/forwardLogsToSentry.ts index 95eaa25961f4..4b5e86270da8 100644 --- a/src/libs/telemetry/forwardLogsToSentry.ts +++ b/src/libs/telemetry/forwardLogsToSentry.ts @@ -42,7 +42,7 @@ type ForwardedLogPrefix = TupleToUnion; * receipt keys tied to the receipt logs instead of widening the global whitelist. */ const PREFIX_SCOPED_PARAMETERS_WHITELIST = new Map>([ - ['[Receipt]', ['receiptTraceId', 'transactionID', 'event', 'captureSource']], + ['[Receipt]', ['receiptTraceId', 'transactionID', 'event', 'captureSource', 'code']], ['[PDFStall]', ['reportID']], ['[OpenReportStall]', ['reportID']], ]); diff --git a/src/pages/Share/SubmitDetailsPage.tsx b/src/pages/Share/SubmitDetailsPage.tsx index f032f040ba9b..bcdac418ea5a 100644 --- a/src/pages/Share/SubmitDetailsPage.tsx +++ b/src/pages/Share/SubmitDetailsPage.tsx @@ -47,10 +47,11 @@ import {rand64} from '@libs/NumberUtils'; import {isTrackOnboardingChoice} from '@libs/OnboardingUtils'; import {getParticipantsOption, getReportOption} from '@libs/OptionsListUtils'; import {hasOnlyPersonalPolicies as hasOnlyPersonalPoliciesUtil, isGroupPolicy} from '@libs/PolicyUtils'; +import ReceiptStorage from '@libs/ReceiptStorage'; import {shouldValidateFile} from '@libs/ReceiptUtils'; import {getReportOrDraftReport, isMoneyRequestReport, isSelfDM} from '@libs/ReportUtils'; import {cancelSpan, endSpan} from '@libs/telemetry/activeSpans'; -import {logReceiptCaptured, logReceiptSubmitted, mintAndStampReceiptTraceId} from '@libs/telemetry/ReceiptObservability'; +import {logReceiptAdoptFailed, logReceiptCaptured, logReceiptSubmitted, mintAndStampReceiptTraceId} from '@libs/telemetry/ReceiptObservability'; import {cancelTracking} from '@libs/telemetry/submitFollowUpAction'; import {getDefaultTaxCode, getIsFromGlobalCreate, getTaxValue} from '@libs/TransactionUtils'; @@ -563,17 +564,31 @@ function SubmitDetailsPage({ return; } formHasBeenSubmitted.current = true; - readFileAsync( - currentReceiptSource, - currentReceiptName, - (file) => onSuccess(file, locationPermissionGranted), - () => { - // Allow retry after a file-read failure. - formHasBeenSubmitted.current = false; - setIsConfirming(false); - }, - currentReceiptType, - ); + // The share extension wipes its folder on the next share. Adopting at submit, not at ingestion, keeps cancelled + // shares out of a folder nothing prunes. + ReceiptStorage.adopt(currentReceiptSource, currentReceiptName) + .then((durableName) => { + const uri = ReceiptStorage.toLocalUri(durableName); + // The shared path is empty once the move lands, and the draft is what a retry re-reads. + setMoneyRequestReceipt(CONST.IOU.OPTIMISTIC_TRANSACTION_ID, uri, currentReceiptName, true, currentReceiptType); + return uri; + }) + .catch((error: unknown) => { + logReceiptAdoptFailed({error, captureSource: 'share'}); + return currentReceiptSource; + }) + .then((uri) => + readFileAsync( + uri, + currentReceiptName, + (file) => onSuccess(file, locationPermissionGranted), + () => { + formHasBeenSubmitted.current = false; + setIsConfirming(false); + }, + currentReceiptType, + ), + ); }; const onConfirm = (gpsRequired?: boolean) => { diff --git a/tests/ui/SubmitDetailsPageTest.tsx b/tests/ui/SubmitDetailsPageTest.tsx index 0d73f667425b..c27f364ef3c9 100644 --- a/tests/ui/SubmitDetailsPageTest.tsx +++ b/tests/ui/SubmitDetailsPageTest.tsx @@ -8,6 +8,7 @@ import {readFileAsync} from '@libs/fileDownload/FileUtils'; import getIsNarrowLayout from '@libs/getIsNarrowLayout'; import Log from '@libs/Log'; import Navigation from '@libs/Navigation/Navigation'; +import ReceiptStorage from '@libs/ReceiptStorage'; import {getReportOrDraftReport} from '@libs/ReportUtils'; import {Scheduler} from '@libs/Scheduler'; @@ -64,6 +65,16 @@ jest.mock('@libs/fileDownload/FileUtils', () => { }; }); +// Jest resolves the bare specifier to the web no-op, so mock the shape and let each test opt into the native move. +jest.mock('@libs/ReceiptStorage', () => ({ + __esModule: true, + default: { + adopt: jest.fn((uriOrPath: string) => Promise.resolve(uriOrPath)), + toLocalUri: jest.fn((durableName: string) => durableName), + resolve: jest.fn((source: string) => source), + }, +})); + jest.mock('@libs/getCurrentPosition', () => jest.fn()); jest.mock('@hooks/useNetwork', () => jest.fn(() => ({isOffline: false}))); @@ -344,6 +355,61 @@ describe('SubmitDetailsPage', () => { expect(jest.mocked(readFileAsync)).not.toHaveBeenCalled(); }); + // Error #12 — the share extension wipes its folder on the next share, so the read must come from the receipts folder. + it('adopts the shared file into the receipts folder and uploads from there', async () => { + // Given a share whose file adopt moves into the receipts folder + const durableUri = 'file:///Documents/Receipts-Upload/shared_1234.jpg'; + jest.mocked(ReceiptStorage.adopt).mockResolvedValueOnce('shared_1234.jpg'); + jest.mocked(ReceiptStorage.toLocalUri).mockReturnValueOnce(durableUri); + + // When the user confirms the submit + await renderAndConfirm(); + + // Then the upload reads the durable copy, never the app-group path + expect(ReceiptStorage.adopt).toHaveBeenCalledWith('file://shared.jpg', expect.any(String)); + expect(jest.mocked(readFileAsync).mock.calls.at(0)?.[0]).toBe(durableUri); + }); + + // Error #12b — a failed move must not block the submit: the shared path still works for this session. + it('submits with the shared path when adopting the file fails', async () => { + // Given adopt rejects + const logAlertSpy = jest.spyOn(Log, 'alert').mockImplementation(() => {}); + jest.mocked(ReceiptStorage.adopt).mockRejectedValueOnce(new Error('move failed')); + + // When the user confirms the submit + await renderAndConfirm(); + + // Then the upload falls back to the shared path, logs the failure, and the expense is still created + expect(jest.mocked(readFileAsync).mock.calls.at(0)?.[0]).toBe('file://shared.jpg'); + expect(logAlertSpy).toHaveBeenCalledWith(expect.stringContaining('adopt failed'), expect.objectContaining({captureSource: 'share'})); + expect(TrackExpense.requestMoney).toHaveBeenCalled(); + + logAlertSpy.mockRestore(); + }); + + // Error #12c — adopt is a move, so a retry after a read failure has to work off the durable copy. + it('retries from the durable copy after a file-read failure', async () => { + // Given a share that adopts fine but fails its first read + const durableUri = 'file:///Documents/Receipts-Upload/shared_1234.jpg'; + jest.mocked(ReceiptStorage.adopt).mockResolvedValueOnce('shared_1234.jpg'); + jest.mocked(ReceiptStorage.toLocalUri).mockReturnValueOnce(durableUri); + jest.mocked(readFileAsync).mockImplementationOnce((_path, _fileName, _onSuccess, onFailure) => { + onFailure?.('[FileUtils] Could not read uploaded file'); + return Promise.resolve(); + }); + + // When the user confirms, the read fails, and the user confirms again + await renderAndConfirm(); + fireEvent.press(screen.getByTestId('mock-confirm-button')); + await waitForBatchedUpdatesWithAct(); + + // Then the second attempt works off the durable copy, not the shared path the move emptied + expect(jest.mocked(readFileAsync).mock.calls.at(0)?.[0]).toBe(durableUri); + expect(jest.mocked(ReceiptStorage.adopt).mock.calls.at(1)?.[0]).toBe(durableUri); + expect(jest.mocked(readFileAsync).mock.calls.at(1)?.[0]).toBe(durableUri); + expect(TrackExpense.requestMoney).toHaveBeenCalled(); + }); + // Error #5 — wide layout fallback: when destination is not topmost, reveal it via revealRouteBeforeDismissingModal // and defer navigation to cleanup (shouldNavigate: false) so we do not double-navigate after dismiss. it('wide layout: reveals destination via revealRouteBeforeDismissingModal when another report is topmost', async () => { diff --git a/tests/unit/checkFileExistsTest.ts b/tests/unit/checkFileExistsTest.ts index af1e350486d3..7c47f8d7dc8b 100644 --- a/tests/unit/checkFileExistsTest.ts +++ b/tests/unit/checkFileExistsTest.ts @@ -1,4 +1,5 @@ import checkFileExists from '@libs/fileDownload/checkFileExists/index'; +import Log from '@libs/Log'; import type RNFS from 'react-native-fs'; @@ -99,6 +100,21 @@ describe('checkFileExists', () => { expect(result).toBe(false); }); + it('should log the stat error code so a locked device is distinguishable from a missing file', async () => { + // Given a stat that fails the way a locked device fails it + const logInfoSpy = jest.spyOn(Log, 'info').mockImplementation(() => {}); + mockStat.mockRejectedValue(Object.assign(new Error('Operation not permitted'), {code: 'EPERM'})); + + // When the file is checked + const result = await checkFileExists('/var/mobile/Containers/sharedFiles/locked.jpg'); + + // Then it still reports missing, but the code reaches telemetry + expect(result).toBe(false); + expect(logInfoSpy).toHaveBeenCalledWith(expect.stringContaining('stat failed'), false, {event: 'statFailed', code: 'EPERM'}); + + logInfoSpy.mockRestore(); + }); + it('should return false when path is a directory', async () => { mockStat.mockResolvedValue(buildStatResult(false)); const result = await checkFileExists('/var/mobile/Containers/');