Skip to content

Commit 5dc4f0c

Browse files
authored
Merge pull request Expensify#73730 from bernhardoj/fix/73192-unreported-expense-amount-added-to-total-with-diff-currency
Only update the total if the curreny is the same when change report
2 parents 151d4e8 + 3bb5c7c commit 5dc4f0c

3 files changed

Lines changed: 211 additions & 10 deletions

File tree

src/libs/actions/Transaction.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -895,10 +895,11 @@ function changeTransactionsReport(
895895
// 3. Keep track of the new report totals
896896
const isUnreported = reportID === CONST.REPORT.UNREPORTED_REPORT_ID;
897897
const targetReportID = isUnreported ? selfDMReportID : reportID;
898-
const transactionAmount = getTransactionDetails(transaction, undefined, undefined, allowNegative)?.amount ?? 0;
899-
const updatedReportTotal = transactionAmount < 0 ? (oldReport?.total ?? 0 - transactionAmount) : (oldReport?.total ?? 0 + transactionAmount);
898+
const {amount: transactionAmount = 0, currency: transactionCurrency} = getTransactionDetails(transaction, undefined, undefined, allowNegative) ?? {};
899+
const oldReportTotal = oldReport?.total ?? 0;
900+
const updatedReportTotal = transactionAmount < 0 ? oldReportTotal - transactionAmount : oldReportTotal + transactionAmount;
900901

901-
if (oldReport) {
902+
if (oldReport && oldReport.currency === transactionCurrency) {
902903
updatedReportTotals[oldReportID] = updatedReportTotals[oldReportID] ? updatedReportTotals[oldReportID] : updatedReportTotal;
903904
updatedReportNonReimbursableTotals[oldReportID] =
904905
(updatedReportNonReimbursableTotals[oldReportID] ? updatedReportNonReimbursableTotals[oldReportID] : (oldReport?.nonReimbursableTotal ?? 0)) +
@@ -913,14 +914,16 @@ function changeTransactionsReport(
913914
const targetReport =
914915
allReports?.[targetReportKey] ?? (targetReportID === newReport?.reportID ? newReport : undefined) ?? (targetReportID === selfDMReport?.reportID ? selfDMReport : undefined);
915916

916-
const currentTotal = updatedReportTotals[targetReportID] ?? targetReport?.total ?? 0;
917-
updatedReportTotals[targetReportID] = currentTotal - transactionAmount;
917+
if (transactionCurrency === targetReport?.currency) {
918+
const currentTotal = updatedReportTotals[targetReportID] ?? targetReport?.total ?? 0;
919+
updatedReportTotals[targetReportID] = currentTotal - transactionAmount;
918920

919-
const currentNonReimbursableTotal = updatedReportNonReimbursableTotals[targetReportID] ?? targetReport?.nonReimbursableTotal ?? 0;
920-
updatedReportNonReimbursableTotals[targetReportID] = currentNonReimbursableTotal - (transactionReimbursable ? 0 : transactionAmount);
921+
const currentNonReimbursableTotal = updatedReportNonReimbursableTotals[targetReportID] ?? targetReport?.nonReimbursableTotal ?? 0;
922+
updatedReportNonReimbursableTotals[targetReportID] = currentNonReimbursableTotal - (transactionReimbursable ? 0 : transactionAmount);
921923

922-
const currentUnheldNonReimbursableTotal = updatedReportUnheldNonReimbursableTotals[targetReportID] ?? targetReport?.unheldNonReimbursableTotal ?? 0;
923-
updatedReportUnheldNonReimbursableTotals[targetReportID] = currentUnheldNonReimbursableTotal - (transactionReimbursable && !isOnHold(transaction) ? 0 : transactionAmount);
924+
const currentUnheldNonReimbursableTotal = updatedReportUnheldNonReimbursableTotals[targetReportID] ?? targetReport?.unheldNonReimbursableTotal ?? 0;
925+
updatedReportUnheldNonReimbursableTotals[targetReportID] = currentUnheldNonReimbursableTotal - (transactionReimbursable && !isOnHold(transaction) ? 0 : transactionAmount);
926+
}
924927
}
925928

926929
// 4. Optimistically update the IOU action reportID

tests/actions/IOUTest.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7434,7 +7434,11 @@ describe('actions/IOU', () => {
74347434
makeMeAdmin: true,
74357435
policyName: "Carlos's Workspace",
74367436
policyID,
7437+
currency: CONST.CURRENCY.USD,
74377438
});
7439+
7440+
await waitForBatchedUpdates();
7441+
74387442
createNewReport(creatorPersonalDetails, true, false, policyID);
74397443
// Create a tracked expense
74407444
const selfDMReport: Report = {

tests/unit/TransactionTest.ts

Lines changed: 195 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ import ONYXKEYS from '@src/ONYXKEYS';
1313
import type {Attendee} from '@src/types/onyx/IOU';
1414
import type {ReportCollectionDataSet} from '@src/types/onyx/Report';
1515
import * as TransactionUtils from '../../src/libs/TransactionUtils';
16-
import type {RecentWaypoint, ReportAction, ReportActions, Transaction} from '../../src/types/onyx';
16+
import type {RecentWaypoint, Report, ReportAction, ReportActions, Transaction} from '../../src/types/onyx';
17+
import {createRandomReport} from '../utils/collections/reports';
1718
import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';
1819

1920
function generateTransaction(values: Partial<Transaction> = {}): Transaction {
@@ -381,6 +382,199 @@ describe('Transaction', () => {
381382

382383
mockAPIWrite.mockRestore();
383384
});
385+
386+
it('should update the target report total when the currency is the same', async () => {
387+
const transaction = {
388+
...generateTransaction({
389+
reportID: CONST.REPORT.UNREPORTED_REPORT_ID,
390+
}),
391+
amount: -100,
392+
currency: CONST.CURRENCY.USD,
393+
reimbursable: false,
394+
};
395+
const oldIOUAction: OnyxEntry<ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU>> = {
396+
reportActionID: rand64(),
397+
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
398+
actorAccountID: CURRENT_USER_ID,
399+
created: DateUtils.getDBTime(),
400+
originalMessage: {
401+
IOUReportID: '0',
402+
IOUTransactionID: transaction.transactionID,
403+
amount: transaction.amount,
404+
currency: transaction.currency,
405+
type: CONST.IOU.REPORT_ACTION_TYPE.TRACK,
406+
},
407+
};
408+
const expenseReport = {
409+
...createRandomReport(1, undefined),
410+
total: -200,
411+
nonReimbursableTotal: 0,
412+
currency: CONST.CURRENCY.USD,
413+
};
414+
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction);
415+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`, expenseReport);
416+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${FAKE_SELF_DM_REPORT_ID}`, {[oldIOUAction.reportActionID]: oldIOUAction});
417+
418+
changeTransactionsReport([transaction.transactionID], false, CURRENT_USER_ID, 'test@example.com', expenseReport);
419+
await waitForBatchedUpdates();
420+
const report = await new Promise<OnyxEntry<Report>>((resolve) => {
421+
const connection = Onyx.connect({
422+
key: `${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`,
423+
callback: (value) => {
424+
Onyx.disconnect(connection);
425+
resolve(value);
426+
},
427+
});
428+
});
429+
430+
expect(report?.total).toBe(expenseReport.total + transaction.amount);
431+
expect(report?.nonReimbursableTotal).toBe(expenseReport.nonReimbursableTotal + transaction.amount);
432+
});
433+
434+
it('should not update the target report total when the currency is different', async () => {
435+
const transaction = {
436+
...generateTransaction({
437+
reportID: CONST.REPORT.UNREPORTED_REPORT_ID,
438+
}),
439+
currency: 'IDR',
440+
reimbursable: false,
441+
};
442+
const oldIOUAction: OnyxEntry<ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU>> = {
443+
reportActionID: rand64(),
444+
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
445+
actorAccountID: CURRENT_USER_ID,
446+
created: DateUtils.getDBTime(),
447+
originalMessage: {
448+
IOUReportID: '0',
449+
IOUTransactionID: transaction.transactionID,
450+
amount: transaction.amount,
451+
currency: transaction.currency,
452+
type: CONST.IOU.REPORT_ACTION_TYPE.TRACK,
453+
},
454+
};
455+
const expenseReport = {
456+
...createRandomReport(1, undefined),
457+
total: -200,
458+
nonReimbursableTotal: 0,
459+
currency: CONST.CURRENCY.USD,
460+
};
461+
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction);
462+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`, expenseReport);
463+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${FAKE_SELF_DM_REPORT_ID}`, {[oldIOUAction.reportActionID]: oldIOUAction});
464+
465+
changeTransactionsReport([transaction.transactionID], false, CURRENT_USER_ID, 'test@example.com', expenseReport);
466+
await waitForBatchedUpdates();
467+
const report = await new Promise<OnyxEntry<Report>>((resolve) => {
468+
const connection = Onyx.connect({
469+
key: `${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`,
470+
callback: (value) => {
471+
Onyx.disconnect(connection);
472+
resolve(value);
473+
},
474+
});
475+
});
476+
477+
expect(report?.total).toBe(expenseReport.total);
478+
expect(report?.nonReimbursableTotal).toBe(expenseReport.nonReimbursableTotal);
479+
});
480+
481+
it('should update the old report total when the currency is the same', async () => {
482+
const oldExpenseReport = {
483+
...createRandomReport(1, undefined),
484+
total: -200,
485+
nonReimbursableTotal: -200,
486+
currency: CONST.CURRENCY.USD,
487+
};
488+
const transaction = {
489+
...generateTransaction({
490+
reportID: oldExpenseReport.reportID,
491+
}),
492+
amount: -100,
493+
reimbursable: false,
494+
};
495+
const oldIOUAction: OnyxEntry<ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU>> = {
496+
reportActionID: rand64(),
497+
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
498+
actorAccountID: CURRENT_USER_ID,
499+
created: DateUtils.getDBTime(),
500+
originalMessage: {
501+
IOUReportID: FAKE_OLD_REPORT_ID,
502+
IOUTransactionID: transaction.transactionID,
503+
amount: transaction.amount,
504+
currency: transaction.currency,
505+
type: CONST.IOU.REPORT_ACTION_TYPE.CREATE,
506+
},
507+
};
508+
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction);
509+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${oldExpenseReport.reportID}`, oldExpenseReport);
510+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${oldExpenseReport.reportID}`, {[oldIOUAction.reportActionID]: oldIOUAction});
511+
512+
const fakeReport = await getReportFromUseOnyx(FAKE_NEW_REPORT_ID);
513+
changeTransactionsReport([transaction.transactionID], false, CURRENT_USER_ID, 'test@example.com', fakeReport);
514+
await waitForBatchedUpdates();
515+
516+
const report = await new Promise<OnyxEntry<Report>>((resolve) => {
517+
const connection = Onyx.connect({
518+
key: `${ONYXKEYS.COLLECTION.REPORT}${oldExpenseReport.reportID}`,
519+
callback: (value) => {
520+
Onyx.disconnect(connection);
521+
resolve(value);
522+
},
523+
});
524+
});
525+
526+
expect(report?.total).toBe(oldExpenseReport.total - transaction.amount);
527+
expect(report?.nonReimbursableTotal).toBe(oldExpenseReport.nonReimbursableTotal - transaction.amount);
528+
});
529+
530+
it('should not update the old report total when the currency is different', async () => {
531+
const oldExpenseReport = {
532+
...createRandomReport(1, undefined),
533+
total: -200,
534+
nonReimbursableTotal: -200,
535+
currency: CONST.CURRENCY.USD,
536+
};
537+
const transaction = {
538+
...generateTransaction({
539+
reportID: oldExpenseReport.reportID,
540+
}),
541+
reimbursable: false,
542+
currency: 'IDR',
543+
};
544+
const oldIOUAction: OnyxEntry<ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU>> = {
545+
reportActionID: rand64(),
546+
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
547+
actorAccountID: CURRENT_USER_ID,
548+
created: DateUtils.getDBTime(),
549+
originalMessage: {
550+
IOUReportID: FAKE_OLD_REPORT_ID,
551+
IOUTransactionID: transaction.transactionID,
552+
amount: transaction.amount,
553+
currency: transaction.currency,
554+
type: CONST.IOU.REPORT_ACTION_TYPE.CREATE,
555+
},
556+
};
557+
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction);
558+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${oldExpenseReport.reportID}`, oldExpenseReport);
559+
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${oldExpenseReport.reportID}`, {[oldIOUAction.reportActionID]: oldIOUAction});
560+
561+
const fakeReport = await getReportFromUseOnyx(FAKE_NEW_REPORT_ID);
562+
changeTransactionsReport([transaction.transactionID], false, CURRENT_USER_ID, 'test@example.com', fakeReport);
563+
await waitForBatchedUpdates();
564+
565+
const report = await new Promise<OnyxEntry<Report>>((resolve) => {
566+
const connection = Onyx.connect({
567+
key: `${ONYXKEYS.COLLECTION.REPORT}${oldExpenseReport.reportID}`,
568+
callback: (value) => {
569+
Onyx.disconnect(connection);
570+
resolve(value);
571+
},
572+
});
573+
});
574+
575+
expect(report?.total).toBe(oldExpenseReport.total);
576+
expect(report?.nonReimbursableTotal).toBe(oldExpenseReport.nonReimbursableTotal);
577+
});
384578
});
385579

386580
describe('getAllNonDeletedTransactions', () => {

0 commit comments

Comments
 (0)