Skip to content
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

Fix invoice thread isn't deleted when dismissing create error #54232

Merged
merged 6 commits into from
Dec 18, 2024
Merged

Fix invoice thread isn't deleted when dismissing create error #54232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift click to select a range
df2657f
fix invoice thread isn't deleted when dismissing create error
bernhardoj Dec 17, 2024
dbc6db5
Merge branch 'main' into fix/54035-invoice-thread-isn't-removed-when-…
bernhardoj Dec 17, 2024
ffbd184
lint
bernhardoj Dec 17, 2024
f3917b7
Merge branch 'main' into fix/54035-invoice-thread-isn't-removed-when-…
bernhardoj Dec 17, 2024
9e1dcb3
lint
bernhardoj Dec 17, 2024
ff96dd7
more lint
bernhardoj Dec 17, 2024
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
1 change: 0 additions & 1 deletion src/libs/actions/IOU.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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 = '[email protected]';
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('[email protected] 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();
},
});
});
});
});
});
Loading