From 37c689e825da6264ac50b9efb74a79810a3f4006 Mon Sep 17 00:00:00 2001 From: Tom Date: Tue, 19 May 2026 06:06:25 -0700 Subject: [PATCH 01/11] Fix negative transaction total after moving IOU report to a workspace When an IOU report is converted to an expense report, the optimistic transaction data negated amount and modifiedAmount to match the expense-report sign convention but left convertedAmount and convertedTaxAmount untouched. Because getConvertedAmount flips the sign for expense reports, the per-row TOTAL column rendered the still-positive convertedAmount as a negative value. Negate convertedAmount and convertedTaxAmount in the optimistic data in both convertIOUReportToExpenseReport and createWorkspaceFromIOUPayment, using a conditional spread so absent values are not overwritten. Rollback data is built from the unmodified transaction, so failure restores the original values. --- src/libs/actions/Policy/Policy.ts | 2 ++ src/libs/actions/Report/index.ts | 2 ++ tests/actions/ReportTest.ts | 58 +++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+) diff --git a/src/libs/actions/Policy/Policy.ts b/src/libs/actions/Policy/Policy.ts index 909e7274448c..67846c1b067c 100644 --- a/src/libs/actions/Policy/Policy.ts +++ b/src/libs/actions/Policy/Policy.ts @@ -4465,6 +4465,8 @@ function createWorkspaceFromIOUPayment( ...transaction, amount: -transaction.amount, modifiedAmount: hasValidModifiedAmount(transaction) ? -Number(transaction.modifiedAmount) : '', + ...(transaction.convertedAmount != null && {convertedAmount: -transaction.convertedAmount}), + ...(transaction.convertedTaxAmount != null && {convertedTaxAmount: -transaction.convertedTaxAmount}), }; transactionFailureData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = transaction; diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index d667f59fd4c1..869082d2b1de 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -6785,6 +6785,8 @@ function convertIOUReportToExpenseReport(iouReport: Report, policy: Policy, poli ...transaction, amount: -transaction.amount, modifiedAmount: hasValidModifiedAmount(transaction) ? -Number(transaction.modifiedAmount) : '', + ...(transaction.convertedAmount != null && {convertedAmount: -transaction.convertedAmount}), + ...(transaction.convertedTaxAmount != null && {convertedTaxAmount: -transaction.convertedTaxAmount}), }; transactionFailureData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = transaction; diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index 5c99b6d11603..949ebac49f04 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -3933,6 +3933,64 @@ describe('actions/Report', () => { const reportValue = reportUpdate?.value as Partial | undefined; expect(reportValue?.reportName).toBe(CONST.REPORT.DEFAULT_EXPENSE_REPORT_NAME); }); + + it('should negate convertedAmount and convertedTaxAmount on the optimistic transactions so the table total stays positive', () => { + // Given a policy and an IOU report with a transaction that has positive converted amounts (IOU sign convention) + const policyID = '302'; + const policyWithEmptyFieldList: OnyxTypes.Policy = { + ...createRandomPolicy(Number(policyID)), + id: policyID, + type: CONST.POLICY.TYPE.TEAM, + fieldList: {}, + name: 'Test Policy', + }; + + const iouReport: OnyxTypes.Report = { + ...createRandomReport(3, undefined), + reportID: 'iouReport302', + type: CONST.REPORT.TYPE.IOU, + ownerAccountID: 3, + reportName: 'Original IOU Report Name', + total: 5000, + }; + + const transaction: OnyxTypes.Transaction = { + ...createRandomTransaction(302), + transactionID: 'transaction302', + reportID: iouReport.reportID, + amount: 5000, + modifiedAmount: '', + convertedAmount: 6000, + convertedTaxAmount: 600, + }; + + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policyWithEmptyFieldList); + + // When converting the IOU report to an expense report + const result = Report.convertIOUReportToExpenseReport(iouReport, policyWithEmptyFieldList, policyID, 'expenseChat302', [transaction]); + + // Then the optimistic transaction stores amount, convertedAmount and convertedTaxAmount with the expense-report (negative) sign convention + const transactionUpdate = result.optimisticData.find((update) => update.key === ONYXKEYS.COLLECTION.TRANSACTION) as + | OnyxUpdate + | undefined; + const optimisticTransaction = (transactionUpdate?.value as Record | undefined)?.[ + `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}` + ]; + expect(optimisticTransaction?.amount).toBe(-5000); + expect(optimisticTransaction?.convertedAmount).toBe(-6000); + expect(optimisticTransaction?.convertedTaxAmount).toBe(-600); + + // And the failure data restores the original positive values on rollback + const transactionFailure = result.failureData.find((update) => update.key === ONYXKEYS.COLLECTION.TRANSACTION) as + | OnyxUpdate + | undefined; + const rolledBackTransaction = (transactionFailure?.value as Record | undefined)?.[ + `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}` + ]; + expect(rolledBackTransaction?.amount).toBe(5000); + expect(rolledBackTransaction?.convertedAmount).toBe(6000); + expect(rolledBackTransaction?.convertedTaxAmount).toBe(600); + }); }); }); From 4280cb69f3368b30eee17a506fc5b9f680ea2665 Mon Sep 17 00:00:00 2001 From: Tom Date: Tue, 19 May 2026 08:08:54 -0700 Subject: [PATCH 02/11] 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. --- src/libs/TransactionUtils/index.ts | 21 +++++++++++++ src/libs/actions/Policy/Policy.ts | 10 ++----- src/libs/actions/Report/index.ts | 10 ++----- tests/actions/ReportTest.ts | 47 ++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 16 deletions(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index c41e65cd11f3..383bfbbdcc0f 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -636,6 +636,26 @@ function hasValidModifiedAmount(transaction: OnyxEntry | null): boo return transaction?.modifiedAmount !== undefined && transaction?.modifiedAmount !== null && transaction?.modifiedAmount !== ''; } +/** + * Builds the optimistic transaction used when an IOU report is converted to an expense report. + * + * Expense reports store amounts with the opposite sign of IOU reports (see `getAmount`/`getConvertedAmount`), + * so `amount` and `modifiedAmount` are negated. `convertedAmount`/`convertedTaxAmount` are stored as a + * negative magnitude: IOU transactions can be stored with either sign, and the expense-report convention + * flips the stored sign for display, so using `-Math.abs(...)` guarantees the table total renders positive + * regardless of the sign the IOU transaction happened to be stored with. Absent converted values are not + * added so they keep being derived from the amount. + */ +function getExpenseReportSignedTransaction(transaction: Transaction): Transaction { + return { + ...transaction, + amount: -transaction.amount, + modifiedAmount: hasValidModifiedAmount(transaction) ? -Number(transaction.modifiedAmount) : '', + ...(transaction.convertedAmount != null && {convertedAmount: -Math.abs(transaction.convertedAmount)}), + ...(transaction.convertedTaxAmount != null && {convertedTaxAmount: -Math.abs(transaction.convertedTaxAmount)}), + }; +} + function isCreatedMissing(transaction: OnyxEntry) { if (!transaction) { return true; @@ -2991,6 +3011,7 @@ export { hasPendingRTERViolation, hasAnyPendingRTERViolation, hasValidModifiedAmount, + getExpenseReportSignedTransaction, allHavePendingRTERViolation, hasPendingUI, getWaypointIndex, diff --git a/src/libs/actions/Policy/Policy.ts b/src/libs/actions/Policy/Policy.ts index 67846c1b067c..546dfefa1f4e 100644 --- a/src/libs/actions/Policy/Policy.ts +++ b/src/libs/actions/Policy/Policy.ts @@ -95,7 +95,7 @@ import * as PhoneNumber from '@libs/PhoneNumber'; import * as PolicyUtils from '@libs/PolicyUtils'; import {getCustomUnitsForDuplication, getMemberAccountIDsForWorkspace, goBackWhenEnableFeature, isControlPolicy, navigateToExpensifyCardPage} from '@libs/PolicyUtils'; import * as ReportUtils from '@libs/ReportUtils'; -import {hasValidModifiedAmount} from '@libs/TransactionUtils'; +import {getExpenseReportSignedTransaction} from '@libs/TransactionUtils'; import type {AvatarSource} from '@libs/UserAvatarUtils'; import type {Feature} from '@pages/OnboardingInterestedFeatures/types'; import * as PaymentMethods from '@userActions/PaymentMethods'; @@ -4461,13 +4461,7 @@ function createWorkspaceFromIOUPayment( const transactionsOptimisticData: Record = {}; const transactionFailureData: Record = {}; for (const transaction of reportTransactions) { - transactionsOptimisticData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = { - ...transaction, - amount: -transaction.amount, - modifiedAmount: hasValidModifiedAmount(transaction) ? -Number(transaction.modifiedAmount) : '', - ...(transaction.convertedAmount != null && {convertedAmount: -transaction.convertedAmount}), - ...(transaction.convertedTaxAmount != null && {convertedTaxAmount: -transaction.convertedTaxAmount}), - }; + transactionsOptimisticData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = getExpenseReportSignedTransaction(transaction); transactionFailureData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = transaction; } diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index 869082d2b1de..6db6ef4c5f0e 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -169,7 +169,7 @@ import { } from '@libs/ReportUtils'; import {buildOptimisticSnapshotData, getCurrentSearchQueryJSON} from '@libs/SearchQueryUtils'; import playSound, {SOUNDS} from '@libs/Sound'; -import {getAmount, getCurrency, hasValidModifiedAmount, isOnHold, recalculateUnreportedTransactionDetails, shouldClearConvertedAmount} from '@libs/TransactionUtils'; +import {getAmount, getCurrency, getExpenseReportSignedTransaction, isOnHold, recalculateUnreportedTransactionDetails, shouldClearConvertedAmount} from '@libs/TransactionUtils'; import addTrailingForwardSlash from '@libs/UrlUtils'; import Visibility from '@libs/Visibility'; import {cacheAttachment, removeCachedAttachment} from '@userActions/Attachment'; @@ -6781,13 +6781,7 @@ function convertIOUReportToExpenseReport(iouReport: Report, policy: Policy, poli const transactionsOptimisticData: Record = {}; const transactionFailureData: Record = {}; for (const transaction of reportTransactions) { - transactionsOptimisticData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = { - ...transaction, - amount: -transaction.amount, - modifiedAmount: hasValidModifiedAmount(transaction) ? -Number(transaction.modifiedAmount) : '', - ...(transaction.convertedAmount != null && {convertedAmount: -transaction.convertedAmount}), - ...(transaction.convertedTaxAmount != null && {convertedTaxAmount: -transaction.convertedTaxAmount}), - }; + transactionsOptimisticData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = getExpenseReportSignedTransaction(transaction); transactionFailureData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = transaction; } diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index 949ebac49f04..10e54af0156a 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -3991,6 +3991,53 @@ describe('actions/Report', () => { expect(rolledBackTransaction?.convertedAmount).toBe(6000); expect(rolledBackTransaction?.convertedTaxAmount).toBe(600); }); + + it('should store a negative converted magnitude even when the IOU transaction already has negative converted amounts', () => { + // Given an IOU transaction whose converted amounts are stored negative (IOU amounts can be stored with either sign) + const policyID = '303'; + const policyWithEmptyFieldList: OnyxTypes.Policy = { + ...createRandomPolicy(Number(policyID)), + id: policyID, + type: CONST.POLICY.TYPE.TEAM, + fieldList: {}, + name: 'Test Policy', + }; + + const iouReport: OnyxTypes.Report = { + ...createRandomReport(3, undefined), + reportID: 'iouReport303', + type: CONST.REPORT.TYPE.IOU, + ownerAccountID: 3, + reportName: 'Original IOU Report Name', + total: 5000, + }; + + const transaction: OnyxTypes.Transaction = { + ...createRandomTransaction(303), + transactionID: 'transaction303', + reportID: iouReport.reportID, + amount: 5000, + modifiedAmount: '', + convertedAmount: -6000, + convertedTaxAmount: -600, + }; + + Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policyWithEmptyFieldList); + + // When converting the IOU report to an expense report + const result = Report.convertIOUReportToExpenseReport(iouReport, policyWithEmptyFieldList, policyID, 'expenseChat303', [transaction]); + + // Then the optimistic transaction still stores a negative converted magnitude so the expense-report + // sign convention renders a positive table total (a plain negation would have flipped it back to positive here) + const transactionUpdate = result.optimisticData.find((update) => update.key === ONYXKEYS.COLLECTION.TRANSACTION) as + | OnyxUpdate + | undefined; + const optimisticTransaction = (transactionUpdate?.value as Record | undefined)?.[ + `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}` + ]; + expect(optimisticTransaction?.convertedAmount).toBe(-6000); + expect(optimisticTransaction?.convertedTaxAmount).toBe(-600); + }); }); }); From c2e1d234e0c948ff516955c3c5f72294909a1a10 Mon Sep 17 00:00:00 2001 From: Tom Date: Wed, 20 May 2026 14:06:38 -0700 Subject: [PATCH 03/11] Add coverage test for createWorkspaceFromIOUPayment transaction sign conversion Cover the Policy.ts call site of getExpenseReportSignedTransaction with a test that feeds an IOU transaction whose converted amounts are stored negative and asserts the optimistic data stores a negative magnitude (so the expense-report sign convention renders a positive table total) and that the failure data restores the original values. --- tests/actions/PolicyTest.ts | 69 ++++++++++++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/tests/actions/PolicyTest.ts b/tests/actions/PolicyTest.ts index 848b12b86f2a..dbfdb1c3d015 100644 --- a/tests/actions/PolicyTest.ts +++ b/tests/actions/PolicyTest.ts @@ -11,10 +11,11 @@ import OnyxUpdateManager from '@src/libs/actions/OnyxUpdateManager'; import {askToJoinPolicy, joinAccessiblePolicy} from '@src/libs/actions/Policy/Member'; import * as Policy from '@src/libs/actions/Policy/Policy'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {Onboarding, PolicyJoinMember, PolicyReportField, Policy as PolicyType, Report, ReportAction, ReportActions, TransactionViolations} from '@src/types/onyx'; +import type {Onboarding, PolicyJoinMember, PolicyReportField, Policy as PolicyType, Report, ReportAction, ReportActions, Transaction, TransactionViolations} from '@src/types/onyx'; import type {Participant} from '@src/types/onyx/Report'; import createRandomPolicy from '../utils/collections/policies'; import {createRandomReport} from '../utils/collections/reports'; +import createRandomTransaction from '../utils/collections/transaction'; import getOnyxValue from '../utils/getOnyxValue'; import * as TestHelper from '../utils/TestHelper'; import type {MockFetch} from '../utils/TestHelper'; @@ -7093,5 +7094,71 @@ describe('actions/Policy', () => { apiWriteSpy.mockRestore(); isIOUReportUsingReportSpy.mockRestore(); }); + + it('should negate the converted transaction amounts on the optimistic data so the expense-report table total stays positive', async () => { + await Onyx.set(ONYXKEYS.SESSION, {email: ESH_EMAIL, accountID: ESH_ACCOUNT_ID}); + await waitForBatchedUpdates(); + + const employeeAccountID = 400; + const iouReportOwnerEmail = 'employee@example.com'; + + const iouReport: Report = { + ...createRandomReport(1, undefined), + reportID: '900', + type: CONST.REPORT.TYPE.IOU, + ownerAccountID: employeeAccountID, + chatReportID: '901', + policyID: 'oldPolicyID', + currency: CONST.CURRENCY.USD, + total: 5000, + }; + + // IOU transactions can be stored with either sign; use the negative case here so the test fails + // if the helper ever regresses to a plain `-convertedAmount` (which would flip back to positive) + const transaction: Transaction = { + ...createRandomTransaction(900), + transactionID: 'transaction900', + reportID: iouReport.reportID, + amount: 5000, + modifiedAmount: '', + convertedAmount: -6000, + convertedTaxAmount: -600, + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${iouReport.reportID}`, iouReport); + await waitForBatchedUpdates(); + + const apiWriteSpy = jest.spyOn(require('@libs/API'), 'write').mockImplementation(() => Promise.resolve()); + const isIOUReportUsingReportSpy = jest.spyOn(ReportUtils, 'isIOUReportUsingReport').mockReturnValue(true); + const getReportTransactionsSpy = jest.spyOn(ReportUtils, 'getReportTransactions').mockReturnValue([transaction]); + + const mockTranslate = ((key: string) => key) as unknown as Parameters[8]; + Policy.createWorkspaceFromIOUPayment(iouReport, undefined, ESH_ACCOUNT_ID, ESH_EMAIL, iouReportOwnerEmail, undefined, CONST.CURRENCY.USD, undefined, mockTranslate, {}); + await waitForBatchedUpdates(); + + const writeOptions = apiWriteSpy.mock.calls.at(0)?.at(2) as { + optimisticData?: Array<{key?: string; value?: Record | null}>; + failureData?: Array<{key?: string; value?: Record | null}>; + }; + + const transactionOptimisticUpdate = (writeOptions?.optimisticData ?? []).find((update) => update.key === ONYXKEYS.COLLECTION.TRANSACTION); + const optimisticTransaction = transactionOptimisticUpdate?.value?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`]; + + // The expense-report sign convention flips the stored sign for display, so the converted magnitudes must be stored negative + expect(optimisticTransaction?.amount).toBe(-5000); + expect(optimisticTransaction?.convertedAmount).toBe(-6000); + expect(optimisticTransaction?.convertedTaxAmount).toBe(-600); + + // And the failure data restores the original values on rollback + const transactionFailureUpdate = (writeOptions?.failureData ?? []).find((update) => update.key === ONYXKEYS.COLLECTION.TRANSACTION); + const rolledBackTransaction = transactionFailureUpdate?.value?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`]; + expect(rolledBackTransaction?.amount).toBe(5000); + expect(rolledBackTransaction?.convertedAmount).toBe(-6000); + expect(rolledBackTransaction?.convertedTaxAmount).toBe(-600); + + apiWriteSpy.mockRestore(); + isIOUReportUsingReportSpy.mockRestore(); + getReportTransactionsSpy.mockRestore(); + }); }); }); From c20917cf0371020d6a9893aa0c5f384eecc70e9b Mon Sep 17 00:00:00 2001 From: Tom Date: Mon, 25 May 2026 09:43:52 -0700 Subject: [PATCH 04/11] =?UTF-8?q?Address=20review=20feedback=20on=20IOU?= =?UTF-8?q?=E2=86=92expense=20sign=20conversion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - getExpenseReportSignedTransaction: drop the convertedTaxAmount branch (IOUs don't carry tax) and use a plain negation on convertedAmount (mirrors the amount line; IOU convertedAmount is positive). - PolicyTest createWorkspaceFromIOUPayment: rewrite to seed the IOU report and transaction in Onyx with a positive convertedAmount, run the action, and assert the stored transaction via Onyx (pause fetch for the optimistic check, then fail+resume for the rollback). - ReportTest convertIOUReportToExpenseReport: same Onyx-read pattern via Onyx.update on the returned optimistic and failure data; drop the obsolete "negative IOU input" test alongside the abs handling. --- src/libs/TransactionUtils/index.ts | 10 +-- tests/actions/PolicyTest.ts | 57 ++++++++++-------- tests/actions/ReportTest.ts | 97 +++++++++--------------------- 3 files changed, 64 insertions(+), 100 deletions(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 383bfbbdcc0f..d88eb1a4a8c0 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -640,19 +640,15 @@ function hasValidModifiedAmount(transaction: OnyxEntry | null): boo * Builds the optimistic transaction used when an IOU report is converted to an expense report. * * Expense reports store amounts with the opposite sign of IOU reports (see `getAmount`/`getConvertedAmount`), - * so `amount` and `modifiedAmount` are negated. `convertedAmount`/`convertedTaxAmount` are stored as a - * negative magnitude: IOU transactions can be stored with either sign, and the expense-report convention - * flips the stored sign for display, so using `-Math.abs(...)` guarantees the table total renders positive - * regardless of the sign the IOU transaction happened to be stored with. Absent converted values are not - * added so they keep being derived from the amount. + * so `amount`, `modifiedAmount` and `convertedAmount` are negated to match the expense-report convention. + * Absent converted values are not added so they keep being derived from the amount. */ function getExpenseReportSignedTransaction(transaction: Transaction): Transaction { return { ...transaction, amount: -transaction.amount, modifiedAmount: hasValidModifiedAmount(transaction) ? -Number(transaction.modifiedAmount) : '', - ...(transaction.convertedAmount != null && {convertedAmount: -Math.abs(transaction.convertedAmount)}), - ...(transaction.convertedTaxAmount != null && {convertedTaxAmount: -Math.abs(transaction.convertedTaxAmount)}), + ...(transaction.convertedAmount != null && {convertedAmount: -transaction.convertedAmount}), }; } diff --git a/tests/actions/PolicyTest.ts b/tests/actions/PolicyTest.ts index dbfdb1c3d015..5a1fcb2f4c0f 100644 --- a/tests/actions/PolicyTest.ts +++ b/tests/actions/PolicyTest.ts @@ -7097,7 +7097,6 @@ describe('actions/Policy', () => { it('should negate the converted transaction amounts on the optimistic data so the expense-report table total stays positive', async () => { await Onyx.set(ONYXKEYS.SESSION, {email: ESH_EMAIL, accountID: ESH_ACCOUNT_ID}); - await waitForBatchedUpdates(); const employeeAccountID = 400; const iouReportOwnerEmail = 'employee@example.com'; @@ -7113,52 +7112,58 @@ describe('actions/Policy', () => { total: 5000, }; - // IOU transactions can be stored with either sign; use the negative case here so the test fails - // if the helper ever regresses to a plain `-convertedAmount` (which would flip back to positive) + // IOU transactions are stored with a positive `convertedAmount`; the expense-report + // sign convention flips it on storage so the table total renders positive. const transaction: Transaction = { ...createRandomTransaction(900), transactionID: 'transaction900', reportID: iouReport.reportID, amount: 5000, modifiedAmount: '', - convertedAmount: -6000, - convertedTaxAmount: -600, + convertedAmount: 6000, }; await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${iouReport.reportID}`, iouReport); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction); await waitForBatchedUpdates(); - const apiWriteSpy = jest.spyOn(require('@libs/API'), 'write').mockImplementation(() => Promise.resolve()); - const isIOUReportUsingReportSpy = jest.spyOn(ReportUtils, 'isIOUReportUsingReport').mockReturnValue(true); - const getReportTransactionsSpy = jest.spyOn(ReportUtils, 'getReportTransactions').mockReturnValue([transaction]); + mockFetch?.pause?.(); const mockTranslate = ((key: string) => key) as unknown as Parameters[8]; Policy.createWorkspaceFromIOUPayment(iouReport, undefined, ESH_ACCOUNT_ID, ESH_EMAIL, iouReportOwnerEmail, undefined, CONST.CURRENCY.USD, undefined, mockTranslate, {}); await waitForBatchedUpdates(); - const writeOptions = apiWriteSpy.mock.calls.at(0)?.at(2) as { - optimisticData?: Array<{key?: string; value?: Record | null}>; - failureData?: Array<{key?: string; value?: Record | null}>; - }; - - const transactionOptimisticUpdate = (writeOptions?.optimisticData ?? []).find((update) => update.key === ONYXKEYS.COLLECTION.TRANSACTION); - const optimisticTransaction = transactionOptimisticUpdate?.value?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`]; + // Optimistic merge: read the stored transaction from Onyx + const optimisticTransaction: OnyxEntry = await new Promise((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, + callback: (value) => { + Onyx.disconnect(connection); + resolve(value); + }, + }); + }); - // The expense-report sign convention flips the stored sign for display, so the converted magnitudes must be stored negative expect(optimisticTransaction?.amount).toBe(-5000); expect(optimisticTransaction?.convertedAmount).toBe(-6000); - expect(optimisticTransaction?.convertedTaxAmount).toBe(-600); - // And the failure data restores the original values on rollback - const transactionFailureUpdate = (writeOptions?.failureData ?? []).find((update) => update.key === ONYXKEYS.COLLECTION.TRANSACTION); - const rolledBackTransaction = transactionFailureUpdate?.value?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`]; - expect(rolledBackTransaction?.amount).toBe(5000); - expect(rolledBackTransaction?.convertedAmount).toBe(-6000); - expect(rolledBackTransaction?.convertedTaxAmount).toBe(-600); + // Failure rollback: fail the network and verify the original positive values are restored in Onyx + mockFetch?.fail?.(); + await mockFetch?.resume?.(); + await waitForBatchedUpdates(); - apiWriteSpy.mockRestore(); - isIOUReportUsingReportSpy.mockRestore(); - getReportTransactionsSpy.mockRestore(); + const rolledBackTransaction: OnyxEntry = await new Promise((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, + callback: (value) => { + Onyx.disconnect(connection); + resolve(value); + }, + }); + }); + + expect(rolledBackTransaction?.amount).toBe(5000); + expect(rolledBackTransaction?.convertedAmount).toBe(6000); }); }); }); diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index 10e54af0156a..340ceaeb2d1a 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -3934,7 +3934,7 @@ describe('actions/Report', () => { expect(reportValue?.reportName).toBe(CONST.REPORT.DEFAULT_EXPENSE_REPORT_NAME); }); - it('should negate convertedAmount and convertedTaxAmount on the optimistic transactions so the table total stays positive', () => { + it('should negate convertedAmount on the optimistic transactions so the table total stays positive', async () => { // Given a policy and an IOU report with a transaction that has positive converted amounts (IOU sign convention) const policyID = '302'; const policyWithEmptyFieldList: OnyxTypes.Policy = { @@ -3961,82 +3961,45 @@ describe('actions/Report', () => { amount: 5000, modifiedAmount: '', convertedAmount: 6000, - convertedTaxAmount: 600, }; - Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policyWithEmptyFieldList); + await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policyWithEmptyFieldList); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction); + await waitForBatchedUpdates(); - // When converting the IOU report to an expense report + // When converting the IOU report to an expense report, apply the resulting optimistic data to Onyx const result = Report.convertIOUReportToExpenseReport(iouReport, policyWithEmptyFieldList, policyID, 'expenseChat302', [transaction]); - - // Then the optimistic transaction stores amount, convertedAmount and convertedTaxAmount with the expense-report (negative) sign convention - const transactionUpdate = result.optimisticData.find((update) => update.key === ONYXKEYS.COLLECTION.TRANSACTION) as - | OnyxUpdate - | undefined; - const optimisticTransaction = (transactionUpdate?.value as Record | undefined)?.[ - `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}` - ]; + await Onyx.update(result.optimisticData); + await waitForBatchedUpdates(); + + // Then the stored transaction in Onyx has amount and convertedAmount negated to the expense-report sign convention + const optimisticTransaction: OnyxEntry = await new Promise((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, + callback: (value) => { + Onyx.disconnect(connection); + resolve(value); + }, + }); + }); expect(optimisticTransaction?.amount).toBe(-5000); expect(optimisticTransaction?.convertedAmount).toBe(-6000); - expect(optimisticTransaction?.convertedTaxAmount).toBe(-600); // And the failure data restores the original positive values on rollback - const transactionFailure = result.failureData.find((update) => update.key === ONYXKEYS.COLLECTION.TRANSACTION) as - | OnyxUpdate - | undefined; - const rolledBackTransaction = (transactionFailure?.value as Record | undefined)?.[ - `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}` - ]; + await Onyx.update(result.failureData); + await waitForBatchedUpdates(); + + const rolledBackTransaction: OnyxEntry = await new Promise((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, + callback: (value) => { + Onyx.disconnect(connection); + resolve(value); + }, + }); + }); expect(rolledBackTransaction?.amount).toBe(5000); expect(rolledBackTransaction?.convertedAmount).toBe(6000); - expect(rolledBackTransaction?.convertedTaxAmount).toBe(600); - }); - - it('should store a negative converted magnitude even when the IOU transaction already has negative converted amounts', () => { - // Given an IOU transaction whose converted amounts are stored negative (IOU amounts can be stored with either sign) - const policyID = '303'; - const policyWithEmptyFieldList: OnyxTypes.Policy = { - ...createRandomPolicy(Number(policyID)), - id: policyID, - type: CONST.POLICY.TYPE.TEAM, - fieldList: {}, - name: 'Test Policy', - }; - - const iouReport: OnyxTypes.Report = { - ...createRandomReport(3, undefined), - reportID: 'iouReport303', - type: CONST.REPORT.TYPE.IOU, - ownerAccountID: 3, - reportName: 'Original IOU Report Name', - total: 5000, - }; - - const transaction: OnyxTypes.Transaction = { - ...createRandomTransaction(303), - transactionID: 'transaction303', - reportID: iouReport.reportID, - amount: 5000, - modifiedAmount: '', - convertedAmount: -6000, - convertedTaxAmount: -600, - }; - - Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policyWithEmptyFieldList); - - // When converting the IOU report to an expense report - const result = Report.convertIOUReportToExpenseReport(iouReport, policyWithEmptyFieldList, policyID, 'expenseChat303', [transaction]); - - // Then the optimistic transaction still stores a negative converted magnitude so the expense-report - // sign convention renders a positive table total (a plain negation would have flipped it back to positive here) - const transactionUpdate = result.optimisticData.find((update) => update.key === ONYXKEYS.COLLECTION.TRANSACTION) as - | OnyxUpdate - | undefined; - const optimisticTransaction = (transactionUpdate?.value as Record | undefined)?.[ - `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}` - ]; - expect(optimisticTransaction?.convertedAmount).toBe(-6000); - expect(optimisticTransaction?.convertedTaxAmount).toBe(-600); }); }); }); From 1d1ee2653b8d4d1f3c3aebda5263a787319105ed Mon Sep 17 00:00:00 2001 From: Tom Date: Wed, 27 May 2026 16:06:39 -0700 Subject: [PATCH 05/11] Address review: rename helper + simplify negate-amount tests - Rename getExpenseReportSignedTransaction to getNegatedAmountTransaction - ReportTest: inspect optimisticData directly, drop Onyx round-trip and failure-rollback block - PolicyTest: drop failure-rollback block on the negate-amount test --- src/libs/TransactionUtils/index.ts | 4 +-- src/libs/actions/Policy/Policy.ts | 4 +-- src/libs/actions/Report/index.ts | 4 +-- tests/actions/PolicyTest.ts | 18 ---------- tests/actions/ReportTest.ts | 53 ++++++++---------------------- 5 files changed, 20 insertions(+), 63 deletions(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index d88eb1a4a8c0..79d2dd15db97 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -643,7 +643,7 @@ function hasValidModifiedAmount(transaction: OnyxEntry | null): boo * so `amount`, `modifiedAmount` and `convertedAmount` are negated to match the expense-report convention. * Absent converted values are not added so they keep being derived from the amount. */ -function getExpenseReportSignedTransaction(transaction: Transaction): Transaction { +function getNegatedAmountTransaction(transaction: Transaction): Transaction { return { ...transaction, amount: -transaction.amount, @@ -3007,7 +3007,7 @@ export { hasPendingRTERViolation, hasAnyPendingRTERViolation, hasValidModifiedAmount, - getExpenseReportSignedTransaction, + getNegatedAmountTransaction, allHavePendingRTERViolation, hasPendingUI, getWaypointIndex, diff --git a/src/libs/actions/Policy/Policy.ts b/src/libs/actions/Policy/Policy.ts index 546dfefa1f4e..eb97c1360fc0 100644 --- a/src/libs/actions/Policy/Policy.ts +++ b/src/libs/actions/Policy/Policy.ts @@ -95,7 +95,7 @@ import * as PhoneNumber from '@libs/PhoneNumber'; import * as PolicyUtils from '@libs/PolicyUtils'; import {getCustomUnitsForDuplication, getMemberAccountIDsForWorkspace, goBackWhenEnableFeature, isControlPolicy, navigateToExpensifyCardPage} from '@libs/PolicyUtils'; import * as ReportUtils from '@libs/ReportUtils'; -import {getExpenseReportSignedTransaction} from '@libs/TransactionUtils'; +import {getNegatedAmountTransaction} from '@libs/TransactionUtils'; import type {AvatarSource} from '@libs/UserAvatarUtils'; import type {Feature} from '@pages/OnboardingInterestedFeatures/types'; import * as PaymentMethods from '@userActions/PaymentMethods'; @@ -4461,7 +4461,7 @@ function createWorkspaceFromIOUPayment( const transactionsOptimisticData: Record = {}; const transactionFailureData: Record = {}; for (const transaction of reportTransactions) { - transactionsOptimisticData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = getExpenseReportSignedTransaction(transaction); + transactionsOptimisticData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = getNegatedAmountTransaction(transaction); transactionFailureData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = transaction; } diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index 6db6ef4c5f0e..832d521935ab 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -169,7 +169,7 @@ import { } from '@libs/ReportUtils'; import {buildOptimisticSnapshotData, getCurrentSearchQueryJSON} from '@libs/SearchQueryUtils'; import playSound, {SOUNDS} from '@libs/Sound'; -import {getAmount, getCurrency, getExpenseReportSignedTransaction, isOnHold, recalculateUnreportedTransactionDetails, shouldClearConvertedAmount} from '@libs/TransactionUtils'; +import {getAmount, getCurrency, getNegatedAmountTransaction, isOnHold, recalculateUnreportedTransactionDetails, shouldClearConvertedAmount} from '@libs/TransactionUtils'; import addTrailingForwardSlash from '@libs/UrlUtils'; import Visibility from '@libs/Visibility'; import {cacheAttachment, removeCachedAttachment} from '@userActions/Attachment'; @@ -6781,7 +6781,7 @@ function convertIOUReportToExpenseReport(iouReport: Report, policy: Policy, poli const transactionsOptimisticData: Record = {}; const transactionFailureData: Record = {}; for (const transaction of reportTransactions) { - transactionsOptimisticData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = getExpenseReportSignedTransaction(transaction); + transactionsOptimisticData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = getNegatedAmountTransaction(transaction); transactionFailureData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`] = transaction; } diff --git a/tests/actions/PolicyTest.ts b/tests/actions/PolicyTest.ts index 5a1fcb2f4c0f..7ac024140253 100644 --- a/tests/actions/PolicyTest.ts +++ b/tests/actions/PolicyTest.ts @@ -7146,24 +7146,6 @@ describe('actions/Policy', () => { expect(optimisticTransaction?.amount).toBe(-5000); expect(optimisticTransaction?.convertedAmount).toBe(-6000); - - // Failure rollback: fail the network and verify the original positive values are restored in Onyx - mockFetch?.fail?.(); - await mockFetch?.resume?.(); - await waitForBatchedUpdates(); - - const rolledBackTransaction: OnyxEntry = await new Promise((resolve) => { - const connection = Onyx.connect({ - key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, - callback: (value) => { - Onyx.disconnect(connection); - resolve(value); - }, - }); - }); - - expect(rolledBackTransaction?.amount).toBe(5000); - expect(rolledBackTransaction?.convertedAmount).toBe(6000); }); }); }); diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index 340ceaeb2d1a..c34bad980236 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -3934,10 +3934,10 @@ describe('actions/Report', () => { expect(reportValue?.reportName).toBe(CONST.REPORT.DEFAULT_EXPENSE_REPORT_NAME); }); - it('should negate convertedAmount on the optimistic transactions so the table total stays positive', async () => { + it('should negate convertedAmount on the optimistic transactions so the table total stays positive', () => { // Given a policy and an IOU report with a transaction that has positive converted amounts (IOU sign convention) const policyID = '302'; - const policyWithEmptyFieldList: OnyxTypes.Policy = { + const policy: OnyxTypes.Policy = { ...createRandomPolicy(Number(policyID)), id: policyID, type: CONST.POLICY.TYPE.TEAM, @@ -3963,43 +3963,18 @@ describe('actions/Report', () => { convertedAmount: 6000, }; - await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policyWithEmptyFieldList); - await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction); - await waitForBatchedUpdates(); - - // When converting the IOU report to an expense report, apply the resulting optimistic data to Onyx - const result = Report.convertIOUReportToExpenseReport(iouReport, policyWithEmptyFieldList, policyID, 'expenseChat302', [transaction]); - await Onyx.update(result.optimisticData); - await waitForBatchedUpdates(); - - // Then the stored transaction in Onyx has amount and convertedAmount negated to the expense-report sign convention - const optimisticTransaction: OnyxEntry = await new Promise((resolve) => { - const connection = Onyx.connect({ - key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, - callback: (value) => { - Onyx.disconnect(connection); - resolve(value); - }, - }); - }); - expect(optimisticTransaction?.amount).toBe(-5000); - expect(optimisticTransaction?.convertedAmount).toBe(-6000); - - // And the failure data restores the original positive values on rollback - await Onyx.update(result.failureData); - await waitForBatchedUpdates(); - - const rolledBackTransaction: OnyxEntry = await new Promise((resolve) => { - const connection = Onyx.connect({ - key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, - callback: (value) => { - Onyx.disconnect(connection); - resolve(value); - }, - }); - }); - expect(rolledBackTransaction?.amount).toBe(5000); - expect(rolledBackTransaction?.convertedAmount).toBe(6000); + // When converting the IOU report to an expense report + const result = Report.convertIOUReportToExpenseReport(iouReport, policy, policyID, 'expenseChat302', [transaction]); + + // Then the optimistic transactions update negates amount and convertedAmount to the expense-report sign convention + const transactionsUpdate = result.optimisticData.find( + (update) => update.onyxMethod === Onyx.METHOD.MERGE_COLLECTION && update.key === ONYXKEYS.COLLECTION.TRANSACTION, + ); + const negatedTransaction = (transactionsUpdate?.value as Record | undefined)?.[ + `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}` + ]; + expect(negatedTransaction?.amount).toBe(-5000); + expect(negatedTransaction?.convertedAmount).toBe(-6000); }); }); }); From a52262c58831a439ef6c2165dc86a4e275b9d0a5 Mon Sep 17 00:00:00 2001 From: Tom Date: Wed, 27 May 2026 17:43:59 -0700 Subject: [PATCH 06/11] Run prettier on ReportTest after rename --- tests/actions/ReportTest.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index c34bad980236..b78c821c755f 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -3967,9 +3967,7 @@ describe('actions/Report', () => { const result = Report.convertIOUReportToExpenseReport(iouReport, policy, policyID, 'expenseChat302', [transaction]); // Then the optimistic transactions update negates amount and convertedAmount to the expense-report sign convention - const transactionsUpdate = result.optimisticData.find( - (update) => update.onyxMethod === Onyx.METHOD.MERGE_COLLECTION && update.key === ONYXKEYS.COLLECTION.TRANSACTION, - ); + const transactionsUpdate = result.optimisticData.find((update) => update.onyxMethod === Onyx.METHOD.MERGE_COLLECTION && update.key === ONYXKEYS.COLLECTION.TRANSACTION); const negatedTransaction = (transactionsUpdate?.value as Record | undefined)?.[ `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}` ]; From 3a0aca7211a613611b2eaecf42a1fddfde88c8e1 Mon Sep 17 00:00:00 2001 From: Tom Date: Thu, 28 May 2026 10:05:31 -0700 Subject: [PATCH 07/11] Assert negated transaction by reading from Onyx in convertIOUReportToExpenseReport test --- tests/actions/ReportTest.ts | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index b78c821c755f..fe25781e6df1 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -3934,7 +3934,7 @@ describe('actions/Report', () => { expect(reportValue?.reportName).toBe(CONST.REPORT.DEFAULT_EXPENSE_REPORT_NAME); }); - it('should negate convertedAmount on the optimistic transactions so the table total stays positive', () => { + it('should negate convertedAmount on the optimistic transactions so the table total stays positive', async () => { // Given a policy and an IOU report with a transaction that has positive converted amounts (IOU sign convention) const policyID = '302'; const policy: OnyxTypes.Policy = { @@ -3963,16 +3963,27 @@ describe('actions/Report', () => { convertedAmount: 6000, }; - // When converting the IOU report to an expense report - const result = Report.convertIOUReportToExpenseReport(iouReport, policy, policyID, 'expenseChat302', [transaction]); + await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policy); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction); + await waitForBatchedUpdates(); - // Then the optimistic transactions update negates amount and convertedAmount to the expense-report sign convention - const transactionsUpdate = result.optimisticData.find((update) => update.onyxMethod === Onyx.METHOD.MERGE_COLLECTION && update.key === ONYXKEYS.COLLECTION.TRANSACTION); - const negatedTransaction = (transactionsUpdate?.value as Record | undefined)?.[ - `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}` - ]; - expect(negatedTransaction?.amount).toBe(-5000); - expect(negatedTransaction?.convertedAmount).toBe(-6000); + // When converting the IOU report to an expense report and applying the optimistic data to Onyx + const result = Report.convertIOUReportToExpenseReport(iouReport, policy, policyID, 'expenseChat302', [transaction]); + await Onyx.update(result.optimisticData); + await waitForBatchedUpdates(); + + // Then the stored transaction in Onyx has amount and convertedAmount negated to the expense-report sign convention + const optimisticTransaction: OnyxEntry = await new Promise((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, + callback: (value) => { + Onyx.disconnect(connection); + resolve(value); + }, + }); + }); + expect(optimisticTransaction?.amount).toBe(-5000); + expect(optimisticTransaction?.convertedAmount).toBe(-6000); }); }); }); From a2f977ba82ce6716e0639d8cc8abf69e88c705c3 Mon Sep 17 00:00:00 2001 From: Tom Date: Thu, 28 May 2026 14:11:36 -0700 Subject: [PATCH 08/11] Assert convertedAmount negation via the move-IOU action test --- tests/actions/ReportTest.ts | 54 ++----------------------------------- 1 file changed, 2 insertions(+), 52 deletions(-) diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index fe25781e6df1..4f328b095867 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -3933,58 +3933,6 @@ describe('actions/Report', () => { const reportValue = reportUpdate?.value as Partial | undefined; expect(reportValue?.reportName).toBe(CONST.REPORT.DEFAULT_EXPENSE_REPORT_NAME); }); - - it('should negate convertedAmount on the optimistic transactions so the table total stays positive', async () => { - // Given a policy and an IOU report with a transaction that has positive converted amounts (IOU sign convention) - const policyID = '302'; - const policy: OnyxTypes.Policy = { - ...createRandomPolicy(Number(policyID)), - id: policyID, - type: CONST.POLICY.TYPE.TEAM, - fieldList: {}, - name: 'Test Policy', - }; - - const iouReport: OnyxTypes.Report = { - ...createRandomReport(3, undefined), - reportID: 'iouReport302', - type: CONST.REPORT.TYPE.IOU, - ownerAccountID: 3, - reportName: 'Original IOU Report Name', - total: 5000, - }; - - const transaction: OnyxTypes.Transaction = { - ...createRandomTransaction(302), - transactionID: 'transaction302', - reportID: iouReport.reportID, - amount: 5000, - modifiedAmount: '', - convertedAmount: 6000, - }; - - await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policy); - await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction); - await waitForBatchedUpdates(); - - // When converting the IOU report to an expense report and applying the optimistic data to Onyx - const result = Report.convertIOUReportToExpenseReport(iouReport, policy, policyID, 'expenseChat302', [transaction]); - await Onyx.update(result.optimisticData); - await waitForBatchedUpdates(); - - // Then the stored transaction in Onyx has amount and convertedAmount negated to the expense-report sign convention - const optimisticTransaction: OnyxEntry = await new Promise((resolve) => { - const connection = Onyx.connect({ - key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, - callback: (value) => { - Onyx.disconnect(connection); - resolve(value); - }, - }); - }); - expect(optimisticTransaction?.amount).toBe(-5000); - expect(optimisticTransaction?.convertedAmount).toBe(-6000); - }); }); }); @@ -4122,6 +4070,7 @@ describe('actions/Report', () => { reportID: iouReport.reportID, amount: 5000, modifiedAmount: 6000, + convertedAmount: 6000, }; await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, { @@ -4149,6 +4098,7 @@ describe('actions/Report', () => { }); expect(updatedTransaction?.amount).toBe(-5000); expect(updatedTransaction?.modifiedAmount).toBe(-6000); + expect(updatedTransaction?.convertedAmount).toBe(-6000); }); it('should convert IOU report to expense report with correct policyID when reportTransactions are provided', async () => { From 544b927e2d896bdb404a8451e000aaf86d1ef161 Mon Sep 17 00:00:00 2001 From: Tom Date: Thu, 28 May 2026 16:18:30 -0700 Subject: [PATCH 09/11] Remove redundant convertedAmount negation test from PolicyTest The convertedAmount negation is already asserted through the action path in the ReportTest moveIOUReportToPolicyAndInviteSubmitter test, so the createWorkspaceFromIOUPayment-based test here duplicated that coverage. Drop it along with the now-unused imports. --- tests/actions/PolicyTest.ts | 56 +------------------------------------ 1 file changed, 1 insertion(+), 55 deletions(-) diff --git a/tests/actions/PolicyTest.ts b/tests/actions/PolicyTest.ts index 7ac024140253..848b12b86f2a 100644 --- a/tests/actions/PolicyTest.ts +++ b/tests/actions/PolicyTest.ts @@ -11,11 +11,10 @@ import OnyxUpdateManager from '@src/libs/actions/OnyxUpdateManager'; import {askToJoinPolicy, joinAccessiblePolicy} from '@src/libs/actions/Policy/Member'; import * as Policy from '@src/libs/actions/Policy/Policy'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {Onboarding, PolicyJoinMember, PolicyReportField, Policy as PolicyType, Report, ReportAction, ReportActions, Transaction, TransactionViolations} from '@src/types/onyx'; +import type {Onboarding, PolicyJoinMember, PolicyReportField, Policy as PolicyType, Report, ReportAction, ReportActions, TransactionViolations} from '@src/types/onyx'; import type {Participant} from '@src/types/onyx/Report'; import createRandomPolicy from '../utils/collections/policies'; import {createRandomReport} from '../utils/collections/reports'; -import createRandomTransaction from '../utils/collections/transaction'; import getOnyxValue from '../utils/getOnyxValue'; import * as TestHelper from '../utils/TestHelper'; import type {MockFetch} from '../utils/TestHelper'; @@ -7094,58 +7093,5 @@ describe('actions/Policy', () => { apiWriteSpy.mockRestore(); isIOUReportUsingReportSpy.mockRestore(); }); - - it('should negate the converted transaction amounts on the optimistic data so the expense-report table total stays positive', async () => { - await Onyx.set(ONYXKEYS.SESSION, {email: ESH_EMAIL, accountID: ESH_ACCOUNT_ID}); - - const employeeAccountID = 400; - const iouReportOwnerEmail = 'employee@example.com'; - - const iouReport: Report = { - ...createRandomReport(1, undefined), - reportID: '900', - type: CONST.REPORT.TYPE.IOU, - ownerAccountID: employeeAccountID, - chatReportID: '901', - policyID: 'oldPolicyID', - currency: CONST.CURRENCY.USD, - total: 5000, - }; - - // IOU transactions are stored with a positive `convertedAmount`; the expense-report - // sign convention flips it on storage so the table total renders positive. - const transaction: Transaction = { - ...createRandomTransaction(900), - transactionID: 'transaction900', - reportID: iouReport.reportID, - amount: 5000, - modifiedAmount: '', - convertedAmount: 6000, - }; - - await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${iouReport.reportID}`, iouReport); - await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction); - await waitForBatchedUpdates(); - - mockFetch?.pause?.(); - - const mockTranslate = ((key: string) => key) as unknown as Parameters[8]; - Policy.createWorkspaceFromIOUPayment(iouReport, undefined, ESH_ACCOUNT_ID, ESH_EMAIL, iouReportOwnerEmail, undefined, CONST.CURRENCY.USD, undefined, mockTranslate, {}); - await waitForBatchedUpdates(); - - // Optimistic merge: read the stored transaction from Onyx - const optimisticTransaction: OnyxEntry = await new Promise((resolve) => { - const connection = Onyx.connect({ - key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, - callback: (value) => { - Onyx.disconnect(connection); - resolve(value); - }, - }); - }); - - expect(optimisticTransaction?.amount).toBe(-5000); - expect(optimisticTransaction?.convertedAmount).toBe(-6000); - }); }); }); From c072bca7502b64676511cc6ed87844ce5fae0340 Mon Sep 17 00:00:00 2001 From: Tom Date: Fri, 29 May 2026 08:11:48 -0700 Subject: [PATCH 10/11] Keep IOU negation test, drop unnecessary mockFetch.pause() The negated optimistic amounts persist in Onyx after the action resolves, so the test reads them back without pausing the fetch mock. --- tests/actions/PolicyTest.ts | 54 ++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/tests/actions/PolicyTest.ts b/tests/actions/PolicyTest.ts index 848b12b86f2a..29c77b8569ea 100644 --- a/tests/actions/PolicyTest.ts +++ b/tests/actions/PolicyTest.ts @@ -11,10 +11,11 @@ import OnyxUpdateManager from '@src/libs/actions/OnyxUpdateManager'; import {askToJoinPolicy, joinAccessiblePolicy} from '@src/libs/actions/Policy/Member'; import * as Policy from '@src/libs/actions/Policy/Policy'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {Onboarding, PolicyJoinMember, PolicyReportField, Policy as PolicyType, Report, ReportAction, ReportActions, TransactionViolations} from '@src/types/onyx'; +import type {Onboarding, PolicyJoinMember, PolicyReportField, Policy as PolicyType, Report, ReportAction, ReportActions, Transaction, TransactionViolations} from '@src/types/onyx'; import type {Participant} from '@src/types/onyx/Report'; import createRandomPolicy from '../utils/collections/policies'; import {createRandomReport} from '../utils/collections/reports'; +import createRandomTransaction from '../utils/collections/transaction'; import getOnyxValue from '../utils/getOnyxValue'; import * as TestHelper from '../utils/TestHelper'; import type {MockFetch} from '../utils/TestHelper'; @@ -7093,5 +7094,56 @@ describe('actions/Policy', () => { apiWriteSpy.mockRestore(); isIOUReportUsingReportSpy.mockRestore(); }); + + it('should negate the converted transaction amounts on the optimistic data so the expense-report table total stays positive', async () => { + await Onyx.set(ONYXKEYS.SESSION, {email: ESH_EMAIL, accountID: ESH_ACCOUNT_ID}); + + const employeeAccountID = 400; + const iouReportOwnerEmail = 'employee@example.com'; + + const iouReport: Report = { + ...createRandomReport(1, undefined), + reportID: '900', + type: CONST.REPORT.TYPE.IOU, + ownerAccountID: employeeAccountID, + chatReportID: '901', + policyID: 'oldPolicyID', + currency: CONST.CURRENCY.USD, + total: 5000, + }; + + // IOU transactions are stored with a positive `convertedAmount`; the expense-report + // sign convention flips it on storage so the table total renders positive. + const transaction: Transaction = { + ...createRandomTransaction(900), + transactionID: 'transaction900', + reportID: iouReport.reportID, + amount: 5000, + modifiedAmount: '', + convertedAmount: 6000, + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${iouReport.reportID}`, iouReport); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction); + await waitForBatchedUpdates(); + + const mockTranslate = ((key: string) => key) as unknown as Parameters[8]; + Policy.createWorkspaceFromIOUPayment(iouReport, undefined, ESH_ACCOUNT_ID, ESH_EMAIL, iouReportOwnerEmail, undefined, CONST.CURRENCY.USD, undefined, mockTranslate, {}); + await waitForBatchedUpdates(); + + // Optimistic merge: read the stored transaction from Onyx + const optimisticTransaction: OnyxEntry = await new Promise((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, + callback: (value) => { + Onyx.disconnect(connection); + resolve(value); + }, + }); + }); + + expect(optimisticTransaction?.amount).toBe(-5000); + expect(optimisticTransaction?.convertedAmount).toBe(-6000); + }); }); }); From 54ed55e9b97666da8d4395596d75e6c01b7d5711 Mon Sep 17 00:00:00 2001 From: Tom Date: Fri, 29 May 2026 18:17:22 -0700 Subject: [PATCH 11/11] Remove redundant comment from negate-amounts test --- tests/actions/PolicyTest.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/actions/PolicyTest.ts b/tests/actions/PolicyTest.ts index 29c77b8569ea..1cf620943c7d 100644 --- a/tests/actions/PolicyTest.ts +++ b/tests/actions/PolicyTest.ts @@ -7112,8 +7112,6 @@ describe('actions/Policy', () => { total: 5000, }; - // IOU transactions are stored with a positive `convertedAmount`; the expense-report - // sign convention flips it on storage so the table total renders positive. const transaction: Transaction = { ...createRandomTransaction(900), transactionID: 'transaction900',