Skip to content

Commit

Permalink
Merge pull request #54232 from bernhardoj/fix/54035-invoice-thread-is…
Browse files Browse the repository at this point in the history 
…n't-removed-when-dismissing-create-error

Fix invoice thread isn't deleted when dismissing create error
  • Loading branch information
@marcochavezf
marcochavezf authored Dec 18, 2024
2 parents 0ad4b2b + ff96dd7 commit f3df091
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 28 deletions.
1 change: 0 additions & 1 deletion src/libs/actions/IOU.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1377,7 +1377,6 @@ function buildOnyxDataForInvoice(
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`,
value: {
errors: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericCreateInvoiceFailureMessage'),
pendingAction: null,
pendingFields: clearedPendingFields,
},
},
Expand Down
2 changes: 1 addition & 1 deletion src/types/utils/CollectionDataSet.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ type CollectionDataSet<TCollectionKey extends OnyxCollectionKey> = Record<`${TCo
const toCollectionDataSet = <TCollectionKey extends OnyxCollectionKey>(
collectionKey: TCollectionKey,
collection: Array<OnyxInputValue<OnyxCollectionValuesMapping[TCollectionKey]>>,
idSelector: (collectionValue: OnyxCollectionValuesMapping[TCollectionKey]) => string,
idSelector: (collectionValue: OnyxCollectionValuesMapping[TCollectionKey]) => string | undefined,
) => {
const collectionDataSet = collection.reduce<CollectionDataSet<TCollectionKey>>((result, collectionValue) => {
if (collectionValue) {
Expand Down
99 changes: 73 additions & 26 deletions tests/actions/IOUTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -615,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,
);

Expand Down Expand Up @@ -664,7 +665,7 @@ describe('actions/IOU', () => {
let iouReportID: string | undefined;
let createdAction: OnyxEntry<OnyxTypes.ReportAction>;
let iouAction: OnyxEntry<OnyxTypes.ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU>>;
let transactionID: string;
let transactionID: string | undefined;
let transactionThreadReport: OnyxEntry<OnyxTypes.Report>;
let transactionThreadAction: OnyxEntry<OnyxTypes.ReportAction>;
mockFetch?.pause?.();
Expand Down Expand Up @@ -778,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);
Expand Down Expand Up @@ -856,7 +857,9 @@ describe('actions/IOU', () => {
.then(
() =>
new Promise<void>((resolve) => {
ReportActions.clearAllRelatedReportActionErrors(iouReportID ?? '-1', iouAction ?? null);
if (iouReportID) {
ReportActions.clearAllRelatedReportActionErrors(iouReportID, iouAction ?? null);
}
resolve();
}),
)
Expand Down Expand Up @@ -937,8 +940,12 @@ describe('actions/IOU', () => {
.then(
() =>
new Promise<void>((resolve) => {
Report.deleteReport(chatReportID ?? '-1');
Report.deleteReport(transactionThreadReport?.reportID ?? '-1');
if (chatReportID) {
Report.deleteReport(chatReportID);
}
if (transactionThreadReport?.reportID) {
Report.deleteReport(transactionThreadReport?.reportID);
}
resolve();
}),
)
Expand Down Expand Up @@ -1107,7 +1114,7 @@ describe('actions/IOU', () => {
[carlosCreatedAction.reportActionID]: carlosCreatedAction,
},
],
(item) => item[carlosCreatedAction.reportActionID].reportID ?? '-1',
(item) => item[carlosCreatedAction.reportActionID].reportID,
);

const julesActionsCollectionDataSet = toCollectionDataSet(
Expand All @@ -1118,7 +1125,7 @@ describe('actions/IOU', () => {
[julesExistingIOUAction.reportActionID]: julesExistingIOUAction,
},
],
(item) => item[julesCreatedAction.reportActionID].reportID ?? '-1',
(item) => item[julesCreatedAction.reportActionID].reportID,
);

const julesCreatedActionsCollectionDataSet = toCollectionDataSet(
Expand All @@ -1128,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, {
Expand Down Expand Up @@ -2007,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<OnyxTypes.ReportAction> = {
actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT,
Expand Down Expand Up @@ -2089,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();

Expand Down Expand Up @@ -2269,12 +2276,14 @@ 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
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);
Expand All @@ -2286,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
Expand Down Expand Up @@ -2568,15 +2577,15 @@ 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);

await waitForBatchedUpdates();

// 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?.();
Expand Down Expand Up @@ -2681,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);
Expand All @@ -2692,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();
Expand All @@ -2709,11 +2718,13 @@ 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);
reportActionID = resultAction?.reportActionID ?? '-1';
reportActionID = resultAction?.reportActionID;

expect(resultAction?.message).toEqual(REPORT_ACTION.message);
expect(resultAction?.person).toEqual(REPORT_ACTION.person);
Expand All @@ -2724,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();
Expand Down Expand Up @@ -2815,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');

Expand Down Expand Up @@ -2857,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);
Expand Down Expand Up @@ -2958,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', () => {
Expand All @@ -2969,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));
}
});
});

Expand Down Expand Up @@ -3303,4 +3322,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<void>((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();
},
});
});
});
});
});

0 comments on commit f3df091

Please sign in to comment.