Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions src/libs/actions/replaceOptimisticReportWithActualReport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ function replaceOptimisticReportWithActualReport(report: Report, draftReportComm
return;
}

// API sometimes returns the parent as the preexisting report, which would parent it to itself
if (preexistingReportID === parentReportID) {
return;
Comment on lines +77 to +78

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 Reconcile instead of retaining the rejected optimistic ID

When the API returns this equality, preexistingReportID still means the server rejected the optimistic reportID; HandleUnusedOptimisticID explicitly rewrites queued requests because using that ID would otherwise return 404. This early return leaves the user and the parent action pointing to that local-only report, so any comment sent after the response uses the rejected ID via addActions and fails instead of reaching a server-backed thread. Avoid the self-parent assignment while still reconciling the optimistic report to a valid server report.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is temporary and specific to Supportal, we will unwind this later

}

// Handle cleanup of stale optimistic IOU report and its report preview separately
if (isMoneyRequestReport(report) && parentReportID && parentReportActionID) {
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID}`, {
Expand Down
90 changes: 90 additions & 0 deletions tests/actions/ReplaceOptimisticReportWithActualReportTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1412,4 +1412,94 @@ describe('replaceOptimisticReportWithActualReport', () => {
// And it should be updated to point to the preexisting report
expect(parentActions?.[reportActionID]?.childReportID).toBe(preexistingReportID);
});

const setUpParentReturnedAsPreexistingReport = async () => {
const parentReportID = '9999';
const optimisticReportID = '1234';
const reportActionID = 'action123';

await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${parentReportID}`, {
reportID: parentReportID,
type: CONST.REPORT.TYPE.CHAT,
chatType: CONST.REPORT.CHAT_TYPE.SELF_DM,
reportName: 'Self DM',
});
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID}`, {
[reportActionID]: {
reportActionID,
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
originalMessage: {type: CONST.IOU.REPORT_ACTION_TYPE.CREATE, IOUTransactionID: 'trans123'},
childReportID: optimisticReportID,
},
});

const optimisticReport = {
reportID: optimisticReportID,
type: CONST.REPORT.TYPE.CHAT,
parentReportID,
parentReportActionID: reportActionID,
preexistingReportID: parentReportID,
};

await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}`, optimisticReport);
await waitForBatchedUpdates();

return {parentReportID, optimisticReportID, reportActionID, optimisticReport};
};

it('should not overwrite the parent report when the preexisting report is the parent report', async () => {
// Given the API returned the parent report itself as the preexisting report
const {parentReportID, optimisticReport} = await setUpParentReturnedAsPreexistingReport();

// When replaceOptimisticReportWithActualReport is called
replaceOptimisticReportWithActualReport(optimisticReport, undefined, 1);
await waitForBatchedUpdates();

// Then the parent report keeps its own data and is not left parented to itself
const parentReport = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${parentReportID}`);
expect(parentReport?.parentReportID).toBeUndefined();
expect(parentReport?.chatType).toBe(CONST.REPORT.CHAT_TYPE.SELF_DM);
expect(parentReport?.reportName).toBe('Self DM');
});

it('should keep the optimistic report when the preexisting report is the parent report', async () => {
// Given the API returned the parent report itself as the preexisting report
const {parentReportID, optimisticReportID, reportActionID, optimisticReport} = await setUpParentReturnedAsPreexistingReport();

// When replaceOptimisticReportWithActualReport is called
replaceOptimisticReportWithActualReport(optimisticReport, undefined, 1);
await waitForBatchedUpdates();

// Then the optimistic report is not cleared, so the parent action's childReportID does not dangle
const keptReport = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}`);
expect(keptReport).toBeDefined();

const parentActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID}`);
expect(parentActions?.[reportActionID]?.childReportID).toBe(optimisticReportID);
});

it('should not navigate away when the preexisting report is the parent report and the user is viewing the optimistic report', async () => {
// Given the user is viewing the optimistic report
mockIsReady.mockReturnValue(true);
mockGetCurrentRoute.mockReturnValue({name: SCREENS.REPORT, params: {}});

// And the API returned the parent report itself as the preexisting report
const {optimisticReportID, optimisticReport} = await setUpParentReturnedAsPreexistingReport();
mockGetActiveRoute.mockReturnValue(`/r/${optimisticReportID}`);

let capturedEventData: SwitchReportEventData | undefined;
const subscription = DeviceEventEmitter.addListener(`switchToPreExistingReport_${optimisticReportID}`, (data: SwitchReportEventData) => {
capturedEventData = data;
});

// When replaceOptimisticReportWithActualReport is called
replaceOptimisticReportWithActualReport(optimisticReport, undefined, 1);
await waitForBatchedUpdates();

// Then the user is not switched to the parent report
expect(capturedEventData).toBeUndefined();
expect(mockSetParams).not.toHaveBeenCalled();

subscription.remove();
});
});
Loading