diff --git a/src/libs/actions/IOU/MoneyRequestBuilder.ts b/src/libs/actions/IOU/MoneyRequestBuilder.ts index 09668a66b821..fef5b80378d4 100644 --- a/src/libs/actions/IOU/MoneyRequestBuilder.ts +++ b/src/libs/actions/IOU/MoneyRequestBuilder.ts @@ -1050,6 +1050,29 @@ function buildOnyxDataForMoneyRequest(moneyRequestParams: BuildOnyxDataForMoneyR }, }, ); + + // Reset the newly-created chat/IOU report metadata to non-optimistic on failure, mirroring the success path + // (isOptimisticReport: false). Without this the new chat stays isOptimisticReport: true, so `fetchReport` never + // calls `openReport` and the chat is stuck on an infinite loading skeleton. The failed expense's red-brick-road + // error stays visible; dismissing it then fully removes the orphaned chat and IOU report shells. See #93542. + if (isNewChatReport) { + onyxData.failureData?.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${chat.report?.reportID}`, + value: { + isOptimisticReport: false, + }, + }); + } + if (shouldCreateNewMoneyRequestReport) { + onyxData.failureData?.push({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${iou.report.reportID}`, + value: { + isOptimisticReport: false, + }, + }); + } } if (shouldGenerateTransactionThreadReport) { diff --git a/src/libs/actions/Report/DeleteReport.ts b/src/libs/actions/Report/DeleteReport.ts index a358be0411bd..14005263e0f3 100644 --- a/src/libs/actions/Report/DeleteReport.ts +++ b/src/libs/actions/Report/DeleteReport.ts @@ -36,6 +36,8 @@ function deleteReport(reportID: string | undefined, shouldDeleteChildReports = f const onyxData: Record = { [`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]: null, [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`]: null, + // Also clear the report metadata (e.g. isOptimisticReport) so a deleted report leaves nothing orphaned behind. + [`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`]: null, }; // Delete linked transactions diff --git a/src/pages/inbox/report/ReportActionItem.tsx b/src/pages/inbox/report/ReportActionItem.tsx index 8cd3388621fc..61807f7f14c5 100644 --- a/src/pages/inbox/report/ReportActionItem.tsx +++ b/src/pages/inbox/report/ReportActionItem.tsx @@ -31,6 +31,7 @@ import {canUseTouchScreen, hasHoverSupport} from '@libs/DeviceCapabilities'; import type {OnyxDataWithErrors} from '@libs/ErrorUtils'; import {getLatestErrorMessageField, isReceiptError} from '@libs/ErrorUtils'; import {isReportMessageAttachment} from '@libs/isReportMessageAttachment'; +import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackNavigationProp} from '@libs/Navigation/PlatformStackNavigation/types'; import type {ReportsSplitNavigatorParamList} from '@libs/Navigation/types'; import Permissions from '@libs/Permissions'; @@ -65,6 +66,7 @@ import AttachmentModalContext from '@pages/media/AttachmentModalScreen/Attachmen import {clearAllRelatedReportActionErrors} from '@userActions/ClearReportActionErrors'; import {hideEmojiPicker, isActive} from '@userActions/EmojiPickerAction'; import {expandURLPreview} from '@userActions/Report'; +import deleteReport from '@userActions/Report/DeleteReport'; import {clearError} from '@userActions/Transaction'; import CONST from '@src/CONST'; @@ -259,6 +261,19 @@ function ReportActionItem({ cleanUpMoneyRequest(transactionIDToDismiss, action, reportID, transactionThreadReport, report, chatReport, undefined, originalReportID, true, iouPolicy); return; } + + // When the expense created a brand-new chat that failed to be created on the server, the chat, IOU report and + // everything created solely for this request are orphaned optimistic shells. Dismissing the error should remove + // them entirely (see #93542) rather than only clearing the error, so navigate the user out of the now-deleted + // chat and delete it. We delete the parent chat report — deleteReport cascades from the chat down to the linked + // IOU report and the transaction thread(s). Deleting the current report here would only remove the IOU report + // (this error is dismissed from the expense report, so reportID is the IOU report ID) and leave the chat orphaned. + if (reportID && report?.errorFields?.createChat) { + const chatReportIDToDelete = report.chatReportID ?? reportID; + Navigation.goBack(undefined, {afterTransition: () => deleteReport(chatReportIDToDelete, true)}); + return; + } + if (action.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD && isReportActionLinked) { navigation.setParams({reportActionID: ''}); } diff --git a/src/pages/inbox/report/ReportActionItemCreated.tsx b/src/pages/inbox/report/ReportActionItemCreated.tsx index 3e76244c6f0d..868356ceea05 100644 --- a/src/pages/inbox/report/ReportActionItemCreated.tsx +++ b/src/pages/inbox/report/ReportActionItemCreated.tsx @@ -10,9 +10,11 @@ import useResponsiveLayout from '@hooks/useResponsiveLayout'; import useThemeStyles from '@hooks/useThemeStyles'; import {hasDeferredWriteForReport} from '@libs/deferredLayoutWrite'; +import Navigation from '@libs/Navigation/Navigation'; import {isChatReport, isCurrentUserInvoiceReceiver, isInvoiceRoom, navigateToDetailsPage, shouldDisableDetailPage as shouldDisableDetailPageReportUtils} from '@libs/ReportUtils'; import {clearCreateChatError} from '@userActions/Report'; +import deleteReport from '@userActions/Report/DeleteReport'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -38,6 +40,7 @@ function ReportActionItemCreated({reportID, policyID}: ReportActionItemCreatedPr const {translate} = useLocalize(); const {shouldUseNarrowLayout} = useResponsiveLayout(); const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`); + const [reportMetadata] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`); const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`); const [conciergeReportID] = useOnyx(ONYXKEYS.CONCIERGE_REPORT_ID); const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED); @@ -66,7 +69,17 @@ function ReportActionItemCreated({reportID, policyID}: ReportActionItemCreatedPr pendingAction={report?.pendingFields?.addWorkspaceRoom ?? report?.pendingFields?.createChat} errors={report?.errorFields?.addWorkspaceRoom ?? report?.errorFields?.createChat} errorRowStyles={[styles.ml10, styles.mr2, styles.mb2]} - onClose={() => + onClose={() => { + // A brand-new chat whose server creation failed (e.g. via a failed new-chat expense, see #93542) is + // reset to non-optimistic so it can load instead of spinning on an infinite skeleton. But that means + // clearCreateChatError would only clear the createChat error and leave the chat + its linked IOU report + // orphaned. Dismissing the created row on such a chat should remove it entirely — mirroring the + // failed-expense dismiss path — so delete the chat (deleteReport cascades to the IOU report and + // transaction thread(s)) and navigate the user out of the now-deleted chat. + if (report?.errorFields?.createChat && !reportMetadata?.isOptimisticReport) { + Navigation.goBack(undefined, {afterTransition: () => deleteReport(report.reportID, true)}); + return; + } clearCreateChatError( report, conciergeReportID, @@ -77,8 +90,8 @@ function ReportActionItemCreated({reportID, policyID}: ReportActionItemCreatedPr reportOwnerPersonalDetail, currentUserPersonalDetail, conciergePersonalDetail, - ) - } + ); + }} > {/* hasDeferredWriteForReport is non-reactive (reads a module-level Map, not tracked by React). diff --git a/tests/actions/IOU/BuildOnyxDataForMoneyRequestTest.ts b/tests/actions/IOU/BuildOnyxDataForMoneyRequestTest.ts index 9517afe347c5..64a7a27da225 100644 --- a/tests/actions/IOU/BuildOnyxDataForMoneyRequestTest.ts +++ b/tests/actions/IOU/BuildOnyxDataForMoneyRequestTest.ts @@ -475,4 +475,101 @@ describe('buildOnyxDataForMoneyRequest', () => { expect((chatReportEntry?.value as Partial)?.iouReportID).toBeUndefined(); }); }); + + describe('failure handling for a brand-new chat', () => { + function buildNewChatParams(overrides?: Partial): BuildOnyxDataParams { + return { + isNewChatReport: true, + shouldCreateNewMoneyRequestReport: true, + shouldGenerateTransactionThreadReport: true, + isASAPSubmitBetaEnabled: false, + currentUserAccountIDParam: CURRENT_USER_ACCOUNT_ID, + currentUserEmailParam: CURRENT_USER_EMAIL, + hasViolations: false, + quickAction: undefined, + isSelfDMSplit: false, + optimisticParams: buildBaseOptimisticParams(IOU_REPORT_ID), + delegateAccountID: undefined, + isTrackIntentUser: false, + ...overrides, + }; + } + + function findFailureEntry(failureData: ReturnType['failureData'], key: string) { + return failureData?.find((entry) => entry.key === key); + } + + it('keeps the failed expense visible by MERGE-ing createChat errors onto the new chat and IOU reports instead of deleting them', () => { + const {failureData} = buildOnyxDataForMoneyRequest(buildNewChatParams()); + + const chatReportEntry = findFailureEntry(failureData, `${ONYXKEYS.COLLECTION.REPORT}${CHAT_REPORT_ID}`); + const iouReportEntry = findFailureEntry(failureData, `${ONYXKEYS.COLLECTION.REPORT}${IOU_REPORT_ID}`); + + expect(chatReportEntry?.onyxMethod).toBe(Onyx.METHOD.MERGE); + expect((chatReportEntry?.value as Partial)?.errorFields?.createChat).toBeDefined(); + expect(iouReportEntry?.onyxMethod).toBe(Onyx.METHOD.MERGE); + expect((iouReportEntry?.value as Partial)?.errorFields?.createChat).toBeDefined(); + }); + + it('resets the new chat and IOU report metadata to non-optimistic so the chat is not stuck on an infinite loading skeleton', () => { + const {failureData} = buildOnyxDataForMoneyRequest(buildNewChatParams()); + + const chatMetadataEntry = findFailureEntry(failureData, `${ONYXKEYS.COLLECTION.REPORT_METADATA}${CHAT_REPORT_ID}`); + const iouMetadataEntry = findFailureEntry(failureData, `${ONYXKEYS.COLLECTION.REPORT_METADATA}${IOU_REPORT_ID}`); + + expect(chatMetadataEntry?.onyxMethod).toBe(Onyx.METHOD.MERGE); + expect((chatMetadataEntry?.value as {isOptimisticReport?: boolean})?.isOptimisticReport).toBe(false); + expect(iouMetadataEntry?.onyxMethod).toBe(Onyx.METHOD.MERGE); + expect((iouMetadataEntry?.value as {isOptimisticReport?: boolean})?.isOptimisticReport).toBe(false); + }); + + it('does not SET any of the new-chat entities to null on failure (rollback happens on dismiss, not on failure)', () => { + const {failureData} = buildOnyxDataForMoneyRequest(buildNewChatParams()); + + const keys = [ + `${ONYXKEYS.COLLECTION.REPORT}${CHAT_REPORT_ID}`, + `${ONYXKEYS.COLLECTION.REPORT}${IOU_REPORT_ID}`, + `${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`, + `${ONYXKEYS.COLLECTION.REPORT}${THREAD_REPORT_ID}`, + ]; + + for (const key of keys) { + const entry = findFailureEntry(failureData, key); + expect(entry).toBeDefined(); + expect(entry?.value).not.toBeNull(); + } + }); + + it('does not reset report metadata when the expense is added to an existing chat and IOU report', () => { + const {failureData} = buildOnyxDataForMoneyRequest( + buildNewChatParams({ + isNewChatReport: false, + shouldCreateNewMoneyRequestReport: false, + }), + ); + + const chatMetadataEntry = findFailureEntry(failureData, `${ONYXKEYS.COLLECTION.REPORT_METADATA}${CHAT_REPORT_ID}`); + const iouMetadataEntry = findFailureEntry(failureData, `${ONYXKEYS.COLLECTION.REPORT_METADATA}${IOU_REPORT_ID}`); + + expect(chatMetadataEntry).toBeUndefined(); + expect(iouMetadataEntry).toBeUndefined(); + }); + + it('keeps the standard error-merge behavior when the expense is added to an existing chat and IOU report (retryable)', () => { + const {failureData} = buildOnyxDataForMoneyRequest( + buildNewChatParams({ + isNewChatReport: false, + shouldCreateNewMoneyRequestReport: false, + }), + ); + + const chatReportEntry = findFailureEntry(failureData, `${ONYXKEYS.COLLECTION.REPORT}${CHAT_REPORT_ID}`); + const iouReportEntry = findFailureEntry(failureData, `${ONYXKEYS.COLLECTION.REPORT}${IOU_REPORT_ID}`); + + expect(chatReportEntry?.onyxMethod).toBe(Onyx.METHOD.MERGE); + expect(chatReportEntry?.value).not.toBeNull(); + expect(iouReportEntry?.onyxMethod).toBe(Onyx.METHOD.MERGE); + expect(iouReportEntry?.value).not.toBeNull(); + }); + }); }); diff --git a/tests/unit/DeleteReportTest.ts b/tests/unit/DeleteReportTest.ts index e873648be75e..10bb7b6e2a1c 100644 --- a/tests/unit/DeleteReportTest.ts +++ b/tests/unit/DeleteReportTest.ts @@ -114,4 +114,80 @@ describe('actions/Report/DeleteReport', () => { expect(await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`)).toBeUndefined(); expect(await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${iouReportID}`)).toBeUndefined(); }); + + it('should clear the report metadata when a report is deleted', async () => { + // Given a report that has metadata (e.g. an optimistic report) + const reportID = '1'; + const report = createRandomReport(Number(reportID), undefined); + + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, report); + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, {isOptimisticReport: true}); + + // When we delete the report + deleteReport(reportID); + + await waitForBatchedUpdates(); + + // Then the report metadata should be cleared so nothing (e.g. isOptimisticReport) is left orphaned behind + expect(await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`)).toBeUndefined(); + }); + + it('should cascade from a chat report to its linked IOU report, transaction thread, and transaction', async () => { + // Given a brand-new chat report linked to an IOU report (via iouReportID and a report preview child action), + // an IOU report whose money-request action opens a transaction thread, and that transaction. This mirrors the + // failed new-chat expense rollback: dismissing deletes the parent chat, which must cascade to everything below. + const chatReportID = '1'; + const iouReportID = '2'; + const transactionThreadReportID = '3'; + const transactionID = '123'; + + const chatReport = { + ...createRandomReport(Number(chatReportID), undefined), + iouReportID, + }; + const iouReport = createRandomReport(Number(iouReportID), undefined); + const transactionThreadReport = createRandomReport(Number(transactionThreadReportID), undefined); + const transaction = createRandomTransaction(Number(transactionID)); + + // Report preview action in the chat linking to the IOU report + const reportPreviewAction = { + ...createRandomReportAction(100), + childReportID: iouReportID, + }; + // Money-request action in the IOU report linking to the transaction thread and transaction + const iouAction: ReportAction = { + ...createRandomReportAction(200), + actionName: CONST.REPORT.ACTIONS.TYPE.IOU, + childReportID: transactionThreadReportID, + previousMessage: [], + message: [{text: '', html: 'iou', type: 'COMMENT'}], + originalMessage: { + type: CONST.IOU.REPORT_ACTION_TYPE.CREATE, + IOUTransactionID: transactionID, + IOUDetails: { + amount: transaction.amount, + currency: transaction.currency, + comment: '', + }, + }, + }; + + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${chatReportID}`, chatReport); + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${iouReportID}`, iouReport); + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${transactionThreadReportID}`, transactionThreadReport); + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReportID}`, {[reportPreviewAction.reportActionID]: reportPreviewAction}); + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReportID}`, {[iouAction.reportActionID]: iouAction}); + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, transaction); + + // When we delete the parent chat report with cascade + deleteReport(chatReportID, true); + + await waitForBatchedUpdates(); + + // Then the chat, the linked IOU report, the transaction thread, and the transaction are all removed + expect(await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${chatReportID}`)).toBeUndefined(); + expect(await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${iouReportID}`)).toBeUndefined(); + expect(await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${transactionThreadReportID}`)).toBeUndefined(); + expect(await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`)).toBeUndefined(); + }); });