Skip to content

Commit 2c637cd

Browse files
trasnake87trasnake87
committed
Extract shared expense-report sign helper and normalize converted amounts
Move the IOU-to-expense-report transaction sign conversion into a single getExpenseReportSignedTransaction helper in TransactionUtils so the logic is no longer duplicated between convertIOUReportToExpenseReport and createWorkspaceFromIOUPayment. IOU transactions can be stored with either sign, so negate the absolute value of convertedAmount/convertedTaxAmount. The expense-report convention flips the stored sign for display, so a negative magnitude keeps the table total positive even when the source IOU stored the converted amounts negative. Added a regression test covering that case.
1 parent ef1024b commit 2c637cd

4 files changed

Lines changed: 72 additions & 16 deletions

File tree

‎src/libs/TransactionUtils/index.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,26 @@ function hasValidModifiedAmount(transaction: OnyxEntry<Transaction> | null): boo
636636
return transaction?.modifiedAmount !== undefined && transaction?.modifiedAmount !== null && transaction?.modifiedAmount !== '';
637637
}
638638

639+
/**
640+
* Builds the optimistic transaction used when an IOU report is converted to an expense report.
641+
*
642+
* Expense reports store amounts with the opposite sign of IOU reports (see `getAmount`/`getConvertedAmount`),
643+
* so `amount` and `modifiedAmount` are negated. `convertedAmount`/`convertedTaxAmount` are stored as a
644+
* negative magnitude: IOU transactions can be stored with either sign, and the expense-report convention
645+
* flips the stored sign for display, so using `-Math.abs(...)` guarantees the table total renders positive
646+
* regardless of the sign the IOU transaction happened to be stored with. Absent converted values are not
647+
* added so they keep being derived from the amount.
648+
*/
649+
function getExpenseReportSignedTransaction(transaction: Transaction): Transaction {
650+
return {
651+
...transaction,
652+
amount: -transaction.amount,
653+
modifiedAmount: hasValidModifiedAmount(transaction) ? -Number(transaction.modifiedAmount) : '',
654+
...(transaction.convertedAmount != null && {convertedAmount: -Math.abs(transaction.convertedAmount)}),
655+
...(transaction.convertedTaxAmount != null && {convertedTaxAmount: -Math.abs(transaction.convertedTaxAmount)}),
656+
};
657+
}
658+
639659
function isCreatedMissing(transaction: OnyxEntry<Transaction>) {
640660
if (!transaction) {
641661
return true;
@@ -2953,6 +2973,7 @@ export {
29532973
hasPendingRTERViolation,
29542974
hasAnyPendingRTERViolation,
29552975
hasValidModifiedAmount,
2976+
getExpenseReportSignedTransaction,
29562977
allHavePendingRTERViolation,
29572978
hasPendingUI,
29582979
getWaypointIndex,

‎src/libs/actions/Policy/Policy.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ import * as PhoneNumber from '@libs/PhoneNumber';
9595
import * as PolicyUtils from '@libs/PolicyUtils';
9696
import {getCustomUnitsForDuplication, getMemberAccountIDsForWorkspace, goBackWhenEnableFeature, isControlPolicy, navigateToExpensifyCardPage} from '@libs/PolicyUtils';
9797
import * as ReportUtils from '@libs/ReportUtils';
98-
import {hasValidModifiedAmount} from '@libs/TransactionUtils';
98+
import {getExpenseReportSignedTransaction} from '@libs/TransactionUtils';
9999
import type {AvatarSource} from '@libs/UserAvatarUtils';
100100
import type {Feature} from '@pages/OnboardingInterestedFeatures/types';
101101
import * as PaymentMethods from '@userActions/PaymentMethods';
@@ -4471,13 +4471,7 @@ function createWorkspaceFromIOUPayment(
44714471
const transactionsOptimisticData: Record<string, Transaction> = {};
44724472
const transactionFailureData: Record<string, Transaction> = {};
44734473
for (const transaction of reportTransactions) {
4474-
transactionsOptimisticData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = {
4475-
...transaction,
4476-
amount: -transaction.amount,
4477-
modifiedAmount: hasValidModifiedAmount(transaction) ? -Number(transaction.modifiedAmount) : '',
4478-
...(transaction.convertedAmount != null && {convertedAmount: -transaction.convertedAmount}),
4479-
...(transaction.convertedTaxAmount != null && {convertedTaxAmount: -transaction.convertedTaxAmount}),
4480-
};
4474+
transactionsOptimisticData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = getExpenseReportSignedTransaction(transaction);
44814475

44824476
transactionFailureData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = transaction;
44834477
}

‎src/libs/actions/Report/index.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ import {
169169
} from '@libs/ReportUtils';
170170
import {buildOptimisticSnapshotData, getCurrentSearchQueryJSON} from '@libs/SearchQueryUtils';
171171
import playSound, {SOUNDS} from '@libs/Sound';
172-
import {getAmount, getCurrency, hasValidModifiedAmount, isOnHold, recalculateUnreportedTransactionDetails, shouldClearConvertedAmount} from '@libs/TransactionUtils';
172+
import {getAmount, getCurrency, getExpenseReportSignedTransaction, isOnHold, recalculateUnreportedTransactionDetails, shouldClearConvertedAmount} from '@libs/TransactionUtils';
173173
import addTrailingForwardSlash from '@libs/UrlUtils';
174174
import Visibility from '@libs/Visibility';
175175
import {cacheAttachment, removeCachedAttachment} from '@userActions/Attachment';
@@ -6789,13 +6789,7 @@ function convertIOUReportToExpenseReport(iouReport: Report, policy: Policy, poli
67896789
const transactionsOptimisticData: Record<string, Transaction> = {};
67906790
const transactionFailureData: Record<string, Transaction> = {};
67916791
for (const transaction of reportTransactions) {
6792-
transactionsOptimisticData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = {
6793-
...transaction,
6794-
amount: -transaction.amount,
6795-
modifiedAmount: hasValidModifiedAmount(transaction) ? -Number(transaction.modifiedAmount) : '',
6796-
...(transaction.convertedAmount != null && {convertedAmount: -transaction.convertedAmount}),
6797-
...(transaction.convertedTaxAmount != null && {convertedTaxAmount: -transaction.convertedTaxAmount}),
6798-
};
6792+
transactionsOptimisticData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = getExpenseReportSignedTransaction(transaction);
67996793

68006794
transactionFailureData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = transaction;
68016795
}

‎tests/actions/ReportTest.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3993,6 +3993,53 @@ describe('actions/Report', () => {
39933993
expect(rolledBackTransaction?.convertedAmount).toBe(6000);
39943994
expect(rolledBackTransaction?.convertedTaxAmount).toBe(600);
39953995
});
3996+
3997+
it('should store a negative converted magnitude even when the IOU transaction already has negative converted amounts', () => {
3998+
// Given an IOU transaction whose converted amounts are stored negative (IOU amounts can be stored with either sign)
3999+
const policyID = '303';
4000+
const policyWithEmptyFieldList: OnyxTypes.Policy = {
4001+
...createRandomPolicy(Number(policyID)),
4002+
id: policyID,
4003+
type: CONST.POLICY.TYPE.TEAM,
4004+
fieldList: {},
4005+
name: 'Test Policy',
4006+
};
4007+
4008+
const iouReport: OnyxTypes.Report = {
4009+
...createRandomReport(3, undefined),
4010+
reportID: 'iouReport303',
4011+
type: CONST.REPORT.TYPE.IOU,
4012+
ownerAccountID: 3,
4013+
reportName: 'Original IOU Report Name',
4014+
total: 5000,
4015+
};
4016+
4017+
const transaction: OnyxTypes.Transaction = {
4018+
...createRandomTransaction(303),
4019+
transactionID: 'transaction303',
4020+
reportID: iouReport.reportID,
4021+
amount: 5000,
4022+
modifiedAmount: '',
4023+
convertedAmount: -6000,
4024+
convertedTaxAmount: -600,
4025+
};
4026+
4027+
Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policyWithEmptyFieldList);
4028+
4029+
// When converting the IOU report to an expense report
4030+
const result = Report.convertIOUReportToExpenseReport(iouReport, policyWithEmptyFieldList, policyID, 'expenseChat303', [transaction]);
4031+
4032+
// Then the optimistic transaction still stores a negative converted magnitude so the expense-report
4033+
// sign convention renders a positive table total (a plain negation would have flipped it back to positive here)
4034+
const transactionUpdate = result.optimisticData.find((update) => update.key === ONYXKEYS.COLLECTION.TRANSACTION) as
4035+
| OnyxUpdate<typeof ONYXKEYS.COLLECTION.TRANSACTION>
4036+
| undefined;
4037+
const optimisticTransaction = (transactionUpdate?.value as Record<string, OnyxTypes.Transaction> | undefined)?.[
4038+
`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`
4039+
];
4040+
expect(optimisticTransaction?.convertedAmount).toBe(-6000);
4041+
expect(optimisticTransaction?.convertedTaxAmount).toBe(-600);
4042+
});
39964043
});
39974044
});
39984045

0 commit comments

Comments
 (0)