From df2657f7f9557c1773a45c0d7ac985eafdbaf7c7 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Tue, 17 Dec 2024 14:30:07 +0800 Subject: [PATCH 1/4] fix invoice thread isn't deleted when dismissing create error --- src/libs/actions/IOU.ts | 1 - tests/actions/IOUTest.ts | 29 +++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 696853f49fd7..4d6d30ae6d40 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -1377,7 +1377,6 @@ function buildOnyxDataForInvoice( key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, value: { errors: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericCreateInvoiceFailureMessage'), - pendingAction: null, pendingFields: clearedPendingFields, }, }, diff --git a/tests/actions/IOUTest.ts b/tests/actions/IOUTest.ts index 11ff821cb240..7598089ba43c 100644 --- a/tests/actions/IOUTest.ts +++ b/tests/actions/IOUTest.ts @@ -21,6 +21,7 @@ import type * as OnyxTypes from '@src/types/onyx'; import type {Participant} from '@src/types/onyx/Report'; import {toCollectionDataSet} from '@src/types/utils/CollectionDataSet'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; +import createRandomTransaction from '../utils/collections/transaction'; import PusherHelper from '../utils/PusherHelper'; import type {MockFetch} from '../utils/TestHelper'; import * as TestHelper from '../utils/TestHelper'; @@ -3303,4 +3304,32 @@ describe('actions/IOU', () => { ); }); }); + + describe('sendInvoice', () => { + it('should not clear transaction pending action when send invoice fails', async () => { + // Given a send invoice request + mockFetch?.pause?.(); + IOU.sendInvoice(1, createRandomTransaction(1)); + + // When the request fails + mockFetch?.fail?.(); + mockFetch?.resume?.(); + await waitForBatchedUpdates(); + + // Then the pending action of the optimistic transaction shouldn't be cleared + await new Promise((resolve) => { + const connection = Onyx.connect({ + key: ONYXKEYS.COLLECTION.TRANSACTION, + waitForCollectionCallback: true, + callback: (transactions) => { + Onyx.disconnect(connection); + const transaction = Object.values(transactions).at(0); + expect(transaction?.errors).not.toBeUndefined(); + expect(transaction?.pendingAction).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD); + resolve(); + }, + }); + }); + }); + }); }); From ffbd1845db5f22b39ca98da45bbf2fc2c3e7180d Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Tue, 17 Dec 2024 21:55:01 +0800 Subject: [PATCH 2/4] lint --- tests/actions/IOUTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/actions/IOUTest.ts b/tests/actions/IOUTest.ts index 7598089ba43c..2735fff07d35 100644 --- a/tests/actions/IOUTest.ts +++ b/tests/actions/IOUTest.ts @@ -616,7 +616,7 @@ describe('actions/IOU', () => { expect(newTransaction?.pendingAction).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD); // The transactionID on the iou action should match the one from the transactions collection - expect(ReportActionsUtils.isMoneyRequestAction(newIOUAction) ? ReportActionsUtils.getOriginalMessage(newIOUAction)?.IOUTransactionID : '0').toBe( + expect(ReportActionsUtils.isMoneyRequestAction(newIOUAction) ? ReportActionsUtils.getOriginalMessage(newIOUAction)?.IOUTransactionID : undefined).toBe( newTransaction?.transactionID, ); From 9e1dcb35d85a2985a44aa585db88fffd5039d898 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Tue, 17 Dec 2024 22:44:46 +0800 Subject: [PATCH 3/4] lint --- src/types/utils/CollectionDataSet.ts | 2 +- tests/actions/IOUTest.ts | 44 ++++++++++++++++++---------- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/src/types/utils/CollectionDataSet.ts b/src/types/utils/CollectionDataSet.ts index c48d1568534e..a3132a6f9554 100644 --- a/src/types/utils/CollectionDataSet.ts +++ b/src/types/utils/CollectionDataSet.ts @@ -7,7 +7,7 @@ type CollectionDataSet = Record<`${TCo const toCollectionDataSet = ( collectionKey: TCollectionKey, collection: Array>, - idSelector: (collectionValue: OnyxCollectionValuesMapping[TCollectionKey]) => string, + idSelector: (collectionValue: OnyxCollectionValuesMapping[TCollectionKey]) => string | undefined, ) => { const collectionDataSet = collection.reduce>((result, collectionValue) => { if (collectionValue) { diff --git a/tests/actions/IOUTest.ts b/tests/actions/IOUTest.ts index 2735fff07d35..2066da3524f2 100644 --- a/tests/actions/IOUTest.ts +++ b/tests/actions/IOUTest.ts @@ -665,7 +665,7 @@ describe('actions/IOU', () => { let iouReportID: string | undefined; let createdAction: OnyxEntry; let iouAction: OnyxEntry>; - let transactionID: string; + let transactionID: string | undefined; let transactionThreadReport: OnyxEntry; let transactionThreadAction: OnyxEntry; mockFetch?.pause?.(); @@ -779,7 +779,7 @@ describe('actions/IOU', () => { // There should be one transaction expect(Object.values(allTransactions ?? {}).length).toBe(1); const transaction = Object.values(allTransactions ?? {}).find((t) => !isEmptyObject(t)); - transactionID = transaction?.transactionID ?? '-1'; + transactionID = transaction?.transactionID; expect(transaction?.reportID).toBe(iouReportID); expect(transaction?.amount).toBe(amount); @@ -857,7 +857,9 @@ describe('actions/IOU', () => { .then( () => new Promise((resolve) => { - ReportActions.clearAllRelatedReportActionErrors(iouReportID ?? '-1', iouAction ?? null); + if (iouReportID) { + ReportActions.clearAllRelatedReportActionErrors(iouReportID, iouAction ?? null); + } resolve(); }), ) @@ -938,8 +940,12 @@ describe('actions/IOU', () => { .then( () => new Promise((resolve) => { - Report.deleteReport(chatReportID ?? '-1'); - Report.deleteReport(transactionThreadReport?.reportID ?? '-1'); + if (chatReportID) { + Report.deleteReport(chatReportID); + } + if (transactionThreadReport?.reportID) { + Report.deleteReport(transactionThreadReport?.reportID); + } resolve(); }), ) @@ -1108,7 +1114,7 @@ describe('actions/IOU', () => { [carlosCreatedAction.reportActionID]: carlosCreatedAction, }, ], - (item) => item[carlosCreatedAction.reportActionID].reportID ?? '-1', + (item) => item[carlosCreatedAction.reportActionID].reportID, ); const julesActionsCollectionDataSet = toCollectionDataSet( @@ -1119,7 +1125,7 @@ describe('actions/IOU', () => { [julesExistingIOUAction.reportActionID]: julesExistingIOUAction, }, ], - (item) => item[julesCreatedAction.reportActionID].reportID ?? '-1', + (item) => item[julesCreatedAction.reportActionID].reportID, ); const julesCreatedActionsCollectionDataSet = toCollectionDataSet( @@ -1129,7 +1135,7 @@ describe('actions/IOU', () => { [julesChatCreatedAction.reportActionID]: julesChatCreatedAction, }, ], - (item) => item[julesChatCreatedAction.reportActionID].reportID ?? '-1', + (item) => item[julesChatCreatedAction.reportActionID].reportID, ); return Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, { @@ -2008,7 +2014,7 @@ describe('actions/IOU', () => { let thread: OptimisticChatReport; const TEST_USER_ACCOUNT_ID = 1; const TEST_USER_LOGIN = 'test@test.com'; - let IOU_REPORT_ID: string; + let IOU_REPORT_ID: string | undefined; let reportActionID; const REPORT_ACTION: OnyxEntry = { actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT, @@ -2090,7 +2096,7 @@ describe('actions/IOU', () => { expect(iouReport?.chatReportID).toBe(chatReport?.reportID); // Storing IOU Report ID for further reference - IOU_REPORT_ID = chatReport?.iouReportID ?? '-1'; + IOU_REPORT_ID = chatReport?.iouReportID; await waitForBatchedUpdates(); @@ -2270,7 +2276,9 @@ describe('actions/IOU', () => { jest.advanceTimersByTime(10); // When a comment is added to the IOU report - Report.addComment(IOU_REPORT_ID, 'Testing a comment'); + if (IOU_REPORT_ID) { + Report.addComment(IOU_REPORT_ID, 'Testing a comment'); + } await waitForBatchedUpdates(); // Then verify that the comment is correctly added @@ -2710,7 +2718,9 @@ describe('actions/IOU', () => { jest.advanceTimersByTime(10); - Report.addComment(IOU_REPORT_ID, 'Testing a comment'); + if (IOU_REPORT_ID) { + Report.addComment(IOU_REPORT_ID, 'Testing a comment'); + } await waitForBatchedUpdates(); resultAction = Object.values(reportActions ?? {}).find((reportAction) => reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT); @@ -2858,7 +2868,9 @@ describe('actions/IOU', () => { jest.advanceTimersByTime(10); - Report.addComment(IOU_REPORT_ID, 'Testing a comment'); + if (IOU_REPORT_ID) { + Report.addComment(IOU_REPORT_ID, 'Testing a comment'); + } await waitForBatchedUpdates(); const resultAction = Object.values(reportActions ?? {}).find((reportAction) => reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT); @@ -2959,8 +2971,10 @@ describe('actions/IOU', () => { expect(iouReport).toHaveProperty('chatReportID'); // Then we expect to navigate to the iou report - - expect(navigateToAfterDelete).toEqual(ROUTES.REPORT_WITH_ID.getRoute(IOU_REPORT_ID)); + expect(IOU_REPORT_ID).not.toBeUndefined(); + if (IOU_REPORT_ID) { + expect(navigateToAfterDelete).toEqual(ROUTES.REPORT_WITH_ID.getRoute(IOU_REPORT_ID)); + } }); it('navigate the user correctly to the chat Report when appropriate', () => { From ff96dd7b3ede989caa1de0f61db0ca5990815839 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Tue, 17 Dec 2024 23:04:02 +0800 Subject: [PATCH 4/4] more lint --- tests/actions/IOUTest.ts | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/tests/actions/IOUTest.ts b/tests/actions/IOUTest.ts index 2066da3524f2..2127b4b819ca 100644 --- a/tests/actions/IOUTest.ts +++ b/tests/actions/IOUTest.ts @@ -2283,7 +2283,7 @@ describe('actions/IOU', () => { // Then verify that the comment is correctly added const resultAction = Object.values(reportActions ?? {}).find((reportAction) => reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT); - reportActionID = resultAction?.reportActionID ?? '-1'; + reportActionID = resultAction?.reportActionID; expect(resultAction?.message).toEqual(REPORT_ACTION.message); expect(resultAction?.person).toEqual(REPORT_ACTION.person); @@ -2295,7 +2295,7 @@ describe('actions/IOU', () => { expect(Object.values(reportActions ?? {}).length).toBe(3); // Then check the loading state of our action - const resultActionAfterUpdate = reportActions?.[reportActionID]; + const resultActionAfterUpdate = reportActionID ? reportActions?.[reportActionID] : undefined; expect(resultActionAfterUpdate?.pendingAction).toBeUndefined(); // When we attempt to delete an expense from the IOU report @@ -2577,7 +2577,7 @@ describe('actions/IOU', () => { // Then comment details should match the expected report action const resultAction = Object.values(reportActions ?? {}).find((reportAction) => reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT); - reportActionID = resultAction?.reportActionID ?? '-1'; + reportActionID = resultAction?.reportActionID; expect(resultAction?.message).toEqual(REPORT_ACTION.message); expect(resultAction?.person).toEqual(REPORT_ACTION.person); @@ -2585,7 +2585,7 @@ describe('actions/IOU', () => { // Then the report should have 2 actions expect(Object.values(reportActions ?? {}).length).toBe(2); - const resultActionAfter = reportActions?.[reportActionID]; + const resultActionAfter = reportActionID ? reportActions?.[reportActionID] : undefined; expect(resultActionAfter?.pendingAction).toBeUndefined(); mockFetch?.pause?.(); @@ -2690,7 +2690,7 @@ describe('actions/IOU', () => { }); let resultAction = Object.values(reportActions ?? {}).find((reportAction) => reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT); - reportActionID = resultAction?.reportActionID ?? '-1'; + reportActionID = resultAction?.reportActionID; expect(resultAction?.message).toEqual(REPORT_ACTION.message); expect(resultAction?.person).toEqual(REPORT_ACTION.person); @@ -2701,7 +2701,7 @@ describe('actions/IOU', () => { // Verify there are three actions (created + addcomment) and our optimistic comment has been removed expect(Object.values(reportActions ?? {}).length).toBe(2); - let resultActionAfterUpdate = reportActions?.[reportActionID]; + let resultActionAfterUpdate = reportActionID ? reportActions?.[reportActionID] : undefined; // Verify that our action is no longer in the loading state expect(resultActionAfterUpdate?.pendingAction).toBeUndefined(); @@ -2724,7 +2724,7 @@ describe('actions/IOU', () => { await waitForBatchedUpdates(); resultAction = Object.values(reportActions ?? {}).find((reportAction) => reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT); - reportActionID = resultAction?.reportActionID ?? '-1'; + reportActionID = resultAction?.reportActionID; expect(resultAction?.message).toEqual(REPORT_ACTION.message); expect(resultAction?.person).toEqual(REPORT_ACTION.person); @@ -2735,7 +2735,7 @@ describe('actions/IOU', () => { // Verify there are three actions (created + iou + addcomment) and our optimistic comment has been removed expect(Object.values(reportActions ?? {}).length).toBe(3); - resultActionAfterUpdate = reportActions?.[reportActionID]; + resultActionAfterUpdate = reportActionID ? reportActions?.[reportActionID] : undefined; // Verify that our action is no longer in the loading state expect(resultActionAfterUpdate?.pendingAction).toBeUndefined(); @@ -2826,7 +2826,7 @@ describe('actions/IOU', () => { expect(iouReport).toHaveProperty('chatReportID'); expect(iouReport?.total).toBe(30000); - const ioupreview = ReportActionsUtils.getReportPreviewAction(chatReport?.reportID ?? '-1', iouReport?.reportID ?? '-1'); + const ioupreview = chatReport?.reportID && iouReport?.reportID ? ReportActionsUtils.getReportPreviewAction(chatReport.reportID, iouReport.reportID) : undefined; expect(ioupreview).toBeTruthy(); expect(ReportActionsUtils.getReportActionText(ioupreview)).toBe('rory@expensifail.com owes $300.00'); @@ -2984,7 +2984,11 @@ describe('actions/IOU', () => { navigateToAfterDelete = IOU.deleteMoneyRequest(transaction.transactionID, createIOUAction, false); } // Then we expect to navigate to the chat report - expect(navigateToAfterDelete).toEqual(ROUTES.REPORT_WITH_ID.getRoute(chatReport?.reportID ?? '-1')); + expect(chatReport?.reportID).not.toBeUndefined(); + + if (chatReport?.reportID) { + expect(navigateToAfterDelete).toEqual(ROUTES.REPORT_WITH_ID.getRoute(chatReport?.reportID)); + } }); });