Skip to content

Roll back optimistic new-chat expense on money-request failure #96573

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MelvinBot wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Roll back optimistic new-chat expense on money-request failure #96573

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
37720ee
Roll back optimistic new-chat expense on money-request failure
MelvinBot Jul 21, 2026
721cc53
Update RedBrickRoad test to expect rollback for brand-new-chat expens…
MelvinBot Jul 21, 2026
651831d
Switch to option 2: keep failed new-chat expense visible, roll back o…
MelvinBot Jul 21, 2026
9ad4345
Merge remote-tracking branch 'origin/claude-rollbackFailedNewChatExpe…
MelvinBot Jul 21, 2026
471b268
Restore RedBrickRoad test assertions for option 2 (failed expense sta…
MelvinBot Jul 21, 2026
ef00b98
Address review: delete parent chat on failed new-chat expense dismiss…
MelvinBot Jul 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/libs/actions/IOU/MoneyRequestBuilder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep created-row dismiss deleting failed new chats

For a failed brand-new chat, this flips the chat metadata to non-optimistic, but the created/welcome row close handler still calls clearCreateChatError, which deletes the report only when REPORT_METADATA.isOptimisticReport is true; once this failure merge runs, that handler only clears errorFields.createChat. If the user closes the red error on the created row first, the optimistic chat and IOU shell are left behind, and the expense close path will no longer see report.errorFields.createChat to clean them up.

Useful? React with 👍 / 👎.

@MelvinBot MelvinBot Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ef00b98. The created-row dismiss (ReportActionItemCreated) now detects a failed brand-new chat (errorFields.createChat + non-optimistic) and deletes the chat with the same cascade + navigate-away used by the failed-expense dismiss path, instead of falling through to clearCreateChatError (which only clears the error once isOptimisticReport is false). So closing the red error on the created row first no longer orphans the chat/IOU shells.

},
});
}
if (shouldCreateNewMoneyRequestReport) {
onyxData.failureData?.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${iou.report.reportID}`,
value: {
isOptimisticReport: false,
},
});
}
}

if (shouldGenerateTransactionThreadReport) {
Expand Down
2 changes: 2 additions & 0 deletions src/libs/actions/Report/DeleteReport.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ function deleteReport(reportID: string | undefined, shouldDeleteChildReports = f
const onyxData: Record<string, null> = {
[`${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
Expand Down
12 changes: 12 additions & 0 deletions src/pages/inbox/report/ReportActionItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -259,6 +261,16 @@ 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 — deleteReport cascades to the linked IOU report and the transaction thread(s).
if (reportID && report?.errorFields?.createChat) {
Navigation.goBack(undefined, {afterTransition: () => deleteReport(reportID, true)});

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Delete the failed chat, not the current IOU report

When the user opens the failed expense report and dismisses the IOU action error, reportID is the IOU report ID because the failure data attaches the action errors under REPORT_ACTIONS<iouReportID> and errorFields.createChat to that IOU report. Calling deleteReport(reportID, true) from this context deletes only the IOU report and its transaction thread; deleteReport does not follow chatReportID back to the brand-new DM, so the chat shell/preview remains orphaned. Use the parent chat report ID for this rollback path, or otherwise distinguish whether the current report is the chat or the expense report.

Useful? React with 👍 / 👎.

@MelvinBot MelvinBot Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ef00b98. dismissError now deletes the parent chat report (report.chatReportID ?? reportID) instead of the current report, so the cascade removes the IOU report + transaction thread AND the chat. Since this error is dismissed from the expense report, reportID was the IOU report ID as you noted. Added a DeleteReportTest case asserting the chat→IOU→transaction-thread→transaction cascade.

return;
}

if (action.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD && isReportActionLinked) {
navigation.setParams({reportActionID: ''});
}
Expand Down
97 changes: 97 additions & 0 deletions tests/actions/IOU/BuildOnyxDataForMoneyRequestTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,4 +475,101 @@ describe('buildOnyxDataForMoneyRequest', () => {
expect((chatReportEntry?.value as Partial<Report>)?.iouReportID).toBeUndefined();
});
});

describe('failure handling for a brand-new chat', () => {
function buildNewChatParams(overrides?: Partial<BuildOnyxDataParams>): 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<typeof buildOnyxDataForMoneyRequest>['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<Report>)?.errorFields?.createChat).toBeDefined();
expect(iouReportEntry?.onyxMethod).toBe(Onyx.METHOD.MERGE);
expect((iouReportEntry?.value as Partial<Report>)?.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();
});
});
});
Loading