Skip to content

Commit 4280cb6

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 37c689e commit 4280cb6

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;
@@ -2991,6 +3011,7 @@ export {
29913011
hasPendingRTERViolation,
29923012
hasAnyPendingRTERViolation,
29933013
hasValidModifiedAmount,
3014+
getExpenseReportSignedTransaction,
29943015
allHavePendingRTERViolation,
29953016
hasPendingUI,
29963017
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';
@@ -4461,13 +4461,7 @@ function createWorkspaceFromIOUPayment(
44614461
const transactionsOptimisticData: Record<string, Transaction> = {};
44624462
const transactionFailureData: Record<string, Transaction> = {};
44634463
for (const transaction of reportTransactions) {
4464-
transactionsOptimisticData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = {
4465-
...transaction,
4466-
amount: -transaction.amount,
4467-
modifiedAmount: hasValidModifiedAmount(transaction) ? -Number(transaction.modifiedAmount) : '',
4468-
...(transaction.convertedAmount != null && {convertedAmount: -transaction.convertedAmount}),
4469-
...(transaction.convertedTaxAmount != null && {convertedTaxAmount: -transaction.convertedTaxAmount}),
4470-
};
4464+
transactionsOptimisticData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = getExpenseReportSignedTransaction(transaction);
44714465

44724466
transactionFailureData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = transaction;
44734467
}

‎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';
@@ -6781,13 +6781,7 @@ function convertIOUReportToExpenseReport(iouReport: Report, policy: Policy, poli
67816781
const transactionsOptimisticData: Record<string, Transaction> = {};
67826782
const transactionFailureData: Record<string, Transaction> = {};
67836783
for (const transaction of reportTransactions) {
6784-
transactionsOptimisticData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = {
6785-
...transaction,
6786-
amount: -transaction.amount,
6787-
modifiedAmount: hasValidModifiedAmount(transaction) ? -Number(transaction.modifiedAmount) : '',
6788-
...(transaction.convertedAmount != null && {convertedAmount: -transaction.convertedAmount}),
6789-
...(transaction.convertedTaxAmount != null && {convertedTaxAmount: -transaction.convertedTaxAmount}),
6790-
};
6784+
transactionsOptimisticData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = getExpenseReportSignedTransaction(transaction);
67916785

67926786
transactionFailureData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = transaction;
67936787
}

‎tests/actions/ReportTest.ts‎

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

0 commit comments

Comments
 (0)