From 5865e216f7babbe48cffe74f5c615d2710e1ffe1 Mon Sep 17 00:00:00 2001 From: yauhenihorbach Date: Thu, 30 Jul 2026 15:29:50 +0200 Subject: [PATCH 1/5] Fix selft dm split issues --- src/hooks/useDeleteTransactions.ts | 32 ++- src/libs/TransactionPreviewUtils.ts | 11 +- src/libs/actions/IOU/SplitExpenseItems.ts | 5 +- .../actions/IOU/SplitTransactionUpdate.ts | 31 ++- src/libs/actions/IOU/TrackExpense.ts | 66 +++-- src/libs/actions/Transaction.ts | 4 + tests/actions/IOUTest/SplitTest.ts | 232 +++++++++++++++++- tests/actions/IOUTest/TrackExpenseTest.ts | 68 ++++- tests/actions/TransactionTest.ts | 81 +++++- tests/unit/TransactionPreviewUtils.test.ts | 19 ++ .../unit/hooks/useDeleteTransactions.test.ts | 110 +++++++++ 11 files changed, 624 insertions(+), 35 deletions(-) create mode 100644 tests/unit/hooks/useDeleteTransactions.test.ts diff --git a/src/hooks/useDeleteTransactions.ts b/src/hooks/useDeleteTransactions.ts index 6dc488f740c1..f2348969abc5 100644 --- a/src/hooks/useDeleteTransactions.ts +++ b/src/hooks/useDeleteTransactions.ts @@ -5,10 +5,11 @@ import {getIOUActionForTransactions} from '@libs/actions/IOU/Duplicate'; import {getIOURequestPolicyID} from '@libs/actions/IOU/MoneyRequest'; import {initSplitExpenseItemData} from '@libs/actions/IOU/SplitExpenseItems'; import {updateSplitTransactions} from '@libs/actions/IOU/SplitTransactionUpdate'; +import {deleteTrackExpense} from '@libs/actions/IOU/TrackExpense'; import initSplitExpense from '@libs/actions/SplitExpenses'; import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import {calculateAmount as calculateIOUAmount} from '@libs/IOUUtils'; -import {getOriginalMessage, isMoneyRequestAction} from '@libs/ReportActionsUtils'; +import {getOriginalMessage, isMoneyRequestAction, isTrackExpenseAction} from '@libs/ReportActionsUtils'; import {isArchivedReport, isExpenseReport, isIOUReport, isSelfDM} from '@libs/ReportUtils'; import {getActiveGroupSearchHashes} from '@libs/SearchUIUtils'; import { @@ -346,6 +347,35 @@ function useDeleteTransactions({report, reportActions, policy}: UseDeleteTransac const iouReport = isIOUReport(candidateIOUReport) || isExpenseReport(candidateIOUReport) ? candidateIOUReport : undefined; const chatReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${iouReport?.chatReportID}`]; const transactionThreadReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${action?.childReportID}`]; + + // A self-DM expense is a tracked expense: it has no IOU report to key the cleanup on, so it goes + // through the track-expense flow, which resolves the self-DM report actions and the whisper. + if (isSelfDM(candidateIOUReport) && isTrackExpenseAction(action)) { + deleteTrackExpense({ + chatReportID: candidateIOUReport?.reportID, + chatReport: candidateIOUReport, + chatReportActions: allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${candidateIOUReport?.reportID}`], + transactionID, + reportAction: action, + iouReport: undefined, + chatIOUReport: undefined, + transactions: duplicateTransactions, + violations: duplicateTransactionViolations, + isSingleTransactionView, + isChatReportArchived: isArchivedReport(allReportNameValuePairs?.[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${candidateIOUReport?.reportID}`]), + isChatIOUReportArchived: false, + allTransactionViolationsParam: transactionViolations, + currentUserAccountID: currentUserPersonalDetails.accountID, + currentUserEmail: currentUserPersonalDetails.email ?? '', + policy: undefined, + }); + deletedTransactionIDs.push(transactionID); + if (action.childReportID) { + deletedTransactionThreadReportIDs.add(action.childReportID); + } + continue; + } + const chatIOUReportID = chatReport?.reportID; const isChatIOUReportArchived = isArchivedReport(allReportNameValuePairs?.[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${chatIOUReportID}`]); const iouPolicy = iouReport?.policyID ? allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${iouReport.policyID}`] : undefined; diff --git a/src/libs/TransactionPreviewUtils.ts b/src/libs/TransactionPreviewUtils.ts index fe22cfde1635..d930be5685ec 100644 --- a/src/libs/TransactionPreviewUtils.ts +++ b/src/libs/TransactionPreviewUtils.ts @@ -18,7 +18,14 @@ import {isCategoryMissing} from './CategoryUtils'; import DateUtils from './DateUtils'; import createDynamicRoute from './Navigation/helpers/dynamicRoutesUtils/createDynamicRoute'; import {hasDynamicExternalWorkflow, isGroupPolicy as isGroupPolicyUtil} from './PolicyUtils'; -import {getMostRecentActiveDEWSubmitFailedAction, getOriginalMessage, isDynamicExternalWorkflowSubmitFailedAction, isMessageDeleted, isMoneyRequestAction} from './ReportActionsUtils'; +import { + getMostRecentActiveDEWSubmitFailedAction, + getOriginalMessage, + isDeletedAction, + isDynamicExternalWorkflowSubmitFailedAction, + isMessageDeleted, + isMoneyRequestAction, +} from './ReportActionsUtils'; import {hasActionWithErrorsForTransaction, hasReceiptError, isExpenseReport, isReportApproved, isSettled} from './ReportUtils'; import StringUtils from './StringUtils'; import { @@ -418,7 +425,7 @@ function createTransactionPreviewConditionals({ const isFullySettled = isMoneyRequestSettled && !isSettlementOrApprovalPartial; const isFullyApproved = isApproved && !isSettlementOrApprovalPartial; - const shouldShowSkeleton = isEmptyObject(transaction) && !isMessageDeleted(action) && action?.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE; + const shouldShowSkeleton = isEmptyObject(transaction) && !isMessageDeleted(action) && !isDeletedAction(action) && action?.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE; const shouldShowTag = !!tag && isReportAPolicyExpenseChat; const categoryForDisplay = isCategoryMissing(category) ? '' : category; diff --git a/src/libs/actions/IOU/SplitExpenseItems.ts b/src/libs/actions/IOU/SplitExpenseItems.ts index 6a85aeea9905..5b573b1c26f4 100644 --- a/src/libs/actions/IOU/SplitExpenseItems.ts +++ b/src/libs/actions/IOU/SplitExpenseItems.ts @@ -686,7 +686,10 @@ function updateSplitExpenseAmountField( // the original-transaction rate (covers the P2P and deleted-rate cases). const useSplitSelectedRate = !isSplitP2PRate && !!splitSelectedRate?.rate && splitSelectedRate.rate > 0 && splitSelectedRate.enabled !== false; const rate = useSplitSelectedRate ? (splitSelectedRate?.rate ?? 0) : mileageRate.rate; - const unit = useSplitSelectedRate ? (splitSelectedRate?.unit ?? mileageRate.unit) : mileageRate.unit; + + // The unit comes from the split's own `distanceUnit`, kept in sync by `getUpdatedTransaction` on every + // rate change, so the merchant matches the Distance field instead of the policy's current unit. + const unit = splitExpense.customUnit?.distanceUnit ?? (useSplitSelectedRate ? (splitSelectedRate?.unit ?? mileageRate.unit) : mileageRate.unit); if (rate && rate > 0) { const {customUnit: updatedCustomUnit, merchant} = updateSplitExpenseDistanceFromAmount( diff --git a/src/libs/actions/IOU/SplitTransactionUpdate.ts b/src/libs/actions/IOU/SplitTransactionUpdate.ts index b63a1160807f..c0222c53cda7 100644 --- a/src/libs/actions/IOU/SplitTransactionUpdate.ts +++ b/src/libs/actions/IOU/SplitTransactionUpdate.ts @@ -1327,6 +1327,23 @@ function updateSplitTransactions({ onyxData.successData?.push(...(moneyRequestInformationOnyxData.successData ?? [])); onyxData.failureData?.push(...(moneyRequestInformationOnyxData.failureData ?? [])); } + // A revert restores an expense that already existed, so it isn't pending creation — + // `buildOptimisticIOUReportAction` stamps ADD on every action it builds. + if (isReverseSplitOperation && reportActionsReportID) { + onyxData.optimisticData?.push( + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportActionsReportID}`, + value: {[iouAction.reportActionID]: {pendingAction: null}}, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.TRANSACTION}${existingTransactionID}`, + value: {pendingAction: null}, + }, + ); + } + onyxData.optimisticData?.push(...(updateMoneyRequestParamsOnyxData.optimisticData ?? []), ...optimisticDataComments); onyxData.successData?.push(...(updateMoneyRequestParamsOnyxData.successData ?? []), ...successDataComments); onyxData.failureData?.push(...(updateMoneyRequestParamsOnyxData.failureData ?? []), ...failureDataComments); @@ -1358,12 +1375,10 @@ function updateSplitTransactions({ continue; } - // For a reverse split operation (i.e. deleting one transaction from a 2-split), the other split(undeleted) - // transaction also gets marked for deletion optimistically. This causes the undeleted split to remain visible, - // resulting in 3 transactions(deleted, undeleted, and original) being shown at the same time when offline. - // Since original transaction will be reverted and both splits will eventually be deleted, we remove - // the undeleted split entirely instead of marking it for deletion. - const forceDeleteSplitTransactionID = isReverseSplitOperation ? splitExpenses.at(0)?.transactionID : undefined; + // A reverse split brings the original transaction back in place of every split, so all of them are removed + // outright rather than marked for deletion. A split left marked keeps its report action visible (the offline + // surfaces render a pending deletion on purpose), and its pending state has nothing left to resolve it. + const shouldForceDeleteSplit = isReverseSplitOperation; const { optimisticData: deleteExpenseOptimisticData, @@ -1379,7 +1394,7 @@ function updateSplitTransactions({ undefined, undefined, undefined, - isReportArchived || undeletedTransaction?.transactionID === forceDeleteSplitTransactionID, + isReportArchived || shouldForceDeleteSplit, ); // getDeleteTrackExpenseInformation only handles deleting the transaction report thread, so we need to update the report preview action here @@ -1398,7 +1413,7 @@ function updateSplitTransactions({ onyxData.successData?.push(...(deleteExpenseSuccessData ?? [])); onyxData.failureData?.push(...(deleteExpenseFailureData ?? [])); - if (undeletedTransaction?.transactionID && undeletedTransaction.transactionID === forceDeleteSplitTransactionID) { + if (undeletedTransaction?.transactionID && shouldForceDeleteSplit) { onyxData.optimisticData?.push({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportActionsReportID}`, diff --git a/src/libs/actions/IOU/TrackExpense.ts b/src/libs/actions/IOU/TrackExpense.ts index b0378ebf0c5f..0ae6b2250c63 100644 --- a/src/libs/actions/IOU/TrackExpense.ts +++ b/src/libs/actions/IOU/TrackExpense.ts @@ -29,6 +29,7 @@ import { getReportActionText, getTrackExpenseActionableWhisper, isActionableTrackExpense, + isDeletedAction, isMoneyRequestAction, } from '@libs/ReportActionsUtils'; import type {OptimisticChatReport, OptimisticCreatedReportAction, OptimisticIOUReportAction} from '@libs/ReportUtils'; @@ -666,24 +667,41 @@ function getDeleteTrackExpenseInformation( const shouldShowDeletedRequestMessage = !isMovingTransactionFromTrackExpense && !!transactionThreadID && !shouldDeleteTransactionThread; // STEP 3: Update the IOU reportAction. - const updatedReportAction = { - [reportAction.reportActionID]: { - pendingAction: shouldShowDeletedRequestMessage ? CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE : CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, - previousMessage: reportAction.message, - message: [ - { - type: 'COMMENT', - html: '', - text: '', - isEdited: true, - isDeletedParentAction: shouldShowDeletedRequestMessage, - }, - ], - originalMessage: { - IOUTransactionID: shouldRemoveIOUTransaction ? null : transactionID, + const deletedActionValue = { + pendingAction: shouldShowDeletedRequestMessage ? CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE : CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, + previousMessage: reportAction.message, + message: [ + { + type: 'COMMENT', + html: '', + text: '', + isEdited: true, + isDeletedParentAction: shouldShowDeletedRequestMessage, }, - errors: undefined, + ], + originalMessage: { + IOUTransactionID: shouldRemoveIOUTransaction ? null : transactionID, }, + errors: undefined, + }; + + // A report is expected to hold a single live IOU action per transaction, but the chat can end up with more than one + // (an optimistic action plus the one the backend created for the same transaction), so all of them are emptied out + // here. Only create/track actions are swept — a payment action referencing the transaction stays untouched. + const siblingIOUActions = Object.values(getAllReportActions(chatReport?.reportID)).filter((chatAction) => { + if (chatAction.reportActionID === reportAction.reportActionID || !isMoneyRequestAction(chatAction) || isDeletedAction(chatAction)) { + return false; + } + const siblingOriginalMessage = getOriginalMessage(chatAction); + return ( + siblingOriginalMessage?.IOUTransactionID === transactionID && + (siblingOriginalMessage?.type === CONST.IOU.REPORT_ACTION_TYPE.TRACK || siblingOriginalMessage?.type === CONST.IOU.REPORT_ACTION_TYPE.CREATE) + ); + }); + + const updatedReportAction = { + [reportAction.reportActionID]: deletedActionValue, + ...Object.fromEntries(siblingIOUActions.map((siblingAction) => [siblingAction.reportActionID, {...deletedActionValue, previousMessage: siblingAction.message}])), ...(actionableWhisperReportActionID && {[actionableWhisperReportActionID]: {originalMessage: {resolution}}}), } as OnyxTypes.ReportActions; let canUserPerformWriteAction = true; @@ -745,7 +763,7 @@ function getDeleteTrackExpenseInformation( }, ); - const successData: Array> = [ + const successData: Array> = [ { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport?.reportID}`, @@ -754,10 +772,21 @@ function getDeleteTrackExpenseInformation( pendingAction: null, errors: null, }, + ...Object.fromEntries(siblingIOUActions.map((siblingAction) => [siblingAction.reportActionID, {pendingAction: null, errors: null}])), }, }, ]; + // When the transaction is only marked as pending deletion above, drop it once the delete goes through. Left in + // place it keeps surfacing in the lists that include pending-delete transactions while offline. + if (shouldDeleteTransactionFromOnyx && !isMovingTransactionFromTrackExpense) { + successData.push({ + onyxMethod: Onyx.METHOD.SET, + key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, + value: null, + }); + } + // Ensure that any remaining data is removed upon successful completion, even if the server sends a report removal response. // This is done to prevent the removal update from lingering in the applyHTTPSOnyxUpdates function. successData.push(...cleanUpTransactionThreadReportOnyxData.successData); @@ -815,6 +844,7 @@ function getDeleteTrackExpenseInformation( pendingAction: null, errors: getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericDeleteFailureMessage'), }, + ...Object.fromEntries(siblingIOUActions.map((siblingAction) => [siblingAction.reportActionID, {...siblingAction, pendingAction: null}])), }, }, { @@ -1214,7 +1244,7 @@ const getConvertTrackedExpenseInformation = ( const optimisticData: Array< OnyxUpdate > = []; - const successData: Array> = []; + const successData: Array> = []; const failureData: Array< OnyxUpdate > = []; diff --git a/src/libs/actions/Transaction.ts b/src/libs/actions/Transaction.ts index 6e075ccc6327..66ba20cfb447 100644 --- a/src/libs/actions/Transaction.ts +++ b/src/libs/actions/Transaction.ts @@ -1532,6 +1532,10 @@ function getChangeTransactionsReportOnyxData({ IOUTransactionID: null, }, errors: undefined, + // The expense gets its own action on the new report, so this one is retired: clear its pending + // state too, since `shouldReportActionBeVisible` keeps a pending action on screen. + // `failureData` below restores the whole action. + pendingAction: null, }, ...(trackExpenseActionableWhisper ? {[trackExpenseActionableWhisper.reportActionID]: null} : {}), }, diff --git a/tests/actions/IOUTest/SplitTest.ts b/tests/actions/IOUTest/SplitTest.ts index 7e8e434393de..ef0d06796d9b 100644 --- a/tests/actions/IOUTest/SplitTest.ts +++ b/tests/actions/IOUTest/SplitTest.ts @@ -3891,9 +3891,9 @@ describe('updateSplitTransactionsFromSplitExpensesFlow', () => { const deletedSplit1 = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${splitTransactionID1}`); expect(deletedSplit1).toBeFalsy(); - // The other split (split2) is marked pendingAction: delete + // The other split (split2) is marked pendingAction: delete optimistically and dropped once the revert succeeds const deletedSplit2 = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${splitTransactionID2}`); - expect(deletedSplit2?.pendingAction).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); + expect(deletedSplit2).toBeFalsy(); // Step 5: Verify the new IOU action for the restored original transaction const revertExpenseReportID = reports.expenseReport?.reportID; @@ -6459,6 +6459,138 @@ describe('updateSplitTransactions', () => { expect(isDeletedAction(updatedStaleAction1)).toBe(true); expect(isDeletedAction(updatedStaleAction2)).toBe(true); }); + + it('should remove the report actions of every split when reverting, instead of leaving them pending deletion', async () => { + // Given a selfDM expense split into two children, each with its own IOU action in the selfDM + const selfDMReport = createSelfDM(2, RORY_ACCOUNT_ID); + const originalTransactionID = 'revert-actions-original'; + const childTransactionID1 = 'revert-actions-child-1'; + const childTransactionID2 = 'revert-actions-child-2'; + const created = DateUtils.getDBTime(); + + const buildChild = (transactionID: string): Transaction => ({ + transactionID, + amount: -5000, + currency: 'USD', + merchant: 'Test Merchant', + comment: {originalTransactionID, source: CONST.IOU.TYPE.SPLIT}, + created, + reportID: CONST.REPORT.UNREPORTED_REPORT_ID, + }); + const buildTrackAction = (transactionID: string): ReportAction => ({ + ...buildOptimisticIOUReportAction({ + type: CONST.IOU.REPORT_ACTION_TYPE.TRACK, + amount: 5000, + currency: 'USD', + comment: '', + participants: [{accountID: RORY_ACCOUNT_ID, login: RORY_EMAIL}], + transactionID, + isPersonalTrackingExpense: true, + }), + reportID: selfDMReport.reportID, + }); + + const child1Action = buildTrackAction(childTransactionID1); + const child2Action = buildTrackAction(childTransactionID2); + + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${selfDMReport.reportID}`, selfDMReport); + await Onyx.merge(ONYXKEYS.SELF_DM_REPORT_ID, selfDMReport.reportID); + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${originalTransactionID}`, { + transactionID: originalTransactionID, + amount: -10000, + currency: 'USD', + merchant: 'Test Merchant', + comment: {comment: ''}, + created, + reportID: CONST.REPORT.SPLIT_REPORT_ID, + }); + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${childTransactionID1}`, buildChild(childTransactionID1)); + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${childTransactionID2}`, buildChild(childTransactionID2)); + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`, { + [child1Action.reportActionID]: child1Action, + [child2Action.reportActionID]: child2Action, + }); + await waitForBatchedUpdates(); + + let allTransactions: OnyxCollection; + let allReports: OnyxCollection; + let allReportNameValuePairs: OnyxCollection; + let allReportActions: OnyxCollection; + let allSnapshots: OnyxCollection; + await getOnyxData({key: ONYXKEYS.COLLECTION.TRANSACTION, callback: (value) => (allTransactions = value)}); + await getOnyxData({key: ONYXKEYS.COLLECTION.REPORT, callback: (value) => (allReports = value)}); + await getOnyxData({key: ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS, callback: (value) => (allReportNameValuePairs = value)}); + await getOnyxData({key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, callback: (value) => (allReportActions = value)}); + await getOnyxData({key: ONYXKEYS.COLLECTION.SNAPSHOT, callback: (value) => (allSnapshots = value)}); + + // When one of the two splits is deleted, which reverts the split back to the original transaction + mockFetch?.pause?.(); + updateSplitTransactions({ + allTransactionsList: allTransactions, + allReportsList: allReports, + allReportActionsList: allReportActions, + allReportNameValuePairsList: allReportNameValuePairs, + allSnapshots, + allPolicyTags: {}, + transactionData: { + reportID: selfDMReport.reportID, + originalTransactionID, + splitExpenses: [{transactionID: childTransactionID1, amount: 10000, created, reportID: selfDMReport.reportID}], + splitExpensesTotal: 10000, + }, + searchContext: {currentSearchHash: undefined, activeGroupSearchHashes: []}, + policyCategories: undefined, + policy: undefined, + policyRecentlyUsedCategories: [], + iouReport: undefined, + firstIOU: undefined, + isASAPSubmitBetaEnabled: false, + currentUserPersonalDetails, + transactionViolations: {}, + policyRecentlyUsedCurrencies: [], + quickAction: undefined, + iouReportNextStep: undefined, + betas: [CONST.BETAS.ALL], + personalDetails: {[RORY_ACCOUNT_ID]: {accountID: RORY_ACCOUNT_ID, login: RORY_EMAIL}}, + transactionReport: selfDMReport, + expenseReport: undefined, + isOffline: false, + delegateAccountID: undefined, + isTrackIntentUser: false, + }); + await waitForBatchedUpdates(); + + // Then the restored expense isn't marked as pending creation — it already existed before the split + const optimisticActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`); + const restoredAction = Object.values(optimisticActions ?? {}).find( + (action) => isMoneyRequestAction(action) && getOriginalMessage(action)?.IOUTransactionID === originalTransactionID && !isDeletedAction(action), + ); + expect(restoredAction).toBeTruthy(); + expect(restoredAction?.pendingAction).toBeFalsy(); + const inFlightTransaction = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${originalTransactionID}`); + expect(inFlightTransaction?.pendingAction).toBeFalsy(); + + await mockFetch?.resume?.(); + await waitForBatchedUpdates(); + + // And it stays that way after the request goes through + const settledActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`); + const settledRestoredAction = Object.values(settledActions ?? {}).find( + (action) => isMoneyRequestAction(action) && getOriginalMessage(action)?.IOUTransactionID === originalTransactionID && !isDeletedAction(action), + ); + expect(settledRestoredAction?.pendingAction).toBeFalsy(); + const restoredTransaction = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${originalTransactionID}`); + expect(restoredTransaction?.pendingAction).toBeFalsy(); + + // And neither split is left holding a transaction or a pending state, which the offline surfaces would render + const updatedActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`); + for (const childAction of [updatedActions?.[child1Action.reportActionID], updatedActions?.[child2Action.reportActionID]]) { + expect(childAction?.pendingAction).toBeFalsy(); + expect(isMoneyRequestAction(childAction) && getOriginalMessage(childAction)?.IOUTransactionID).toBeFalsy(); + } + await expect(getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${childTransactionID1}`)).resolves.toBeFalsy(); + await expect(getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${childTransactionID2}`)).resolves.toBeFalsy(); + }); }); describe('initSplitExpense', () => { @@ -8007,6 +8139,102 @@ describe('updateSplitExpenseAmountField', () => { expect(splitExpenses?.[0].merchant).toBeTruthy(); expect(splitExpenses?.[0].merchant).toContain('150'); }); + + it('should keep the unit stored on the split when the workspace distance unit changed after the split was created', async () => { + // Given a distance split created in miles, whose workspace unit was later switched to kilometers + const customUnitRateID = 'rate-unit-change'; + const customUnitID = 'distance-unit'; + const originalTransactionID = '321'; + const currentTransactionID = '987'; + const policy: Policy = { + ...createRandomPolicy(2), + customUnits: { + [customUnitID]: { + customUnitID, + name: CONST.CUSTOM_UNITS.NAME_DISTANCE, + enabled: true, + attributes: { + unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_KILOMETERS, + }, + rates: { + [customUnitRateID]: { + customUnitRateID, + currency: CONST.CURRENCY.USD, + rate: 100, + enabled: true, + name: 'Default Rate', + subRates: [], + }, + }, + }, + }, + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`, policy); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${originalTransactionID}`, { + transactionID: originalTransactionID, + amount: -20000, + currency: 'USD', + iouRequestType: CONST.IOU.REQUEST_TYPE.DISTANCE_MAP, + comment: { + type: CONST.TRANSACTION.TYPE.CUSTOM_UNIT, + customUnit: { + name: CONST.CUSTOM_UNITS.NAME_DISTANCE, + customUnitID, + customUnitRateID, + distanceUnit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES, + quantity: 200, + }, + }, + }); + await waitForBatchedUpdates(); + + const draftTransaction: Transaction = { + transactionID: '654', + amount: 20000, + currency: 'USD', + merchant: 'Test Merchant', + iouRequestType: CONST.IOU.REQUEST_TYPE.DISTANCE_MAP, + comment: { + comment: 'Test comment', + originalTransactionID, + splitExpenses: [ + { + transactionID: currentTransactionID, + amount: 10000, + description: 'Test comment', + category: 'Car', + tags: [], + created: DateUtils.getDBTime(), + customUnit: { + name: CONST.CUSTOM_UNITS.NAME_DISTANCE, + customUnitID, + customUnitRateID, + distanceUnit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES, + quantity: 100, + }, + }, + ], + attendees: [], + type: CONST.TRANSACTION.TYPE.CUSTOM_UNIT, + }, + category: 'Car', + created: DateUtils.getDBTime(), + reportID: '456', + }; + + // When the amount of the split is edited + updateSplitExpenseAmountField(draftTransaction, currentTransactionID, 15000, policy, false, undefined); + await waitForBatchedUpdates(); + + // Then the merchant is rebuilt with the unit stored on the split, which is the unit the Distance field renders + const updatedDraftTransaction = await getOnyxValue(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${originalTransactionID}`); + const splitExpenses = updatedDraftTransaction?.comment?.splitExpenses; + expect(splitExpenses?.[0].customUnit?.distanceUnit).toBe(CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES); + expect(splitExpenses?.[0].customUnit?.quantity).toBe(150); + expect(splitExpenses?.[0].merchant).toContain(`150.00 ${CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES}`); + expect(splitExpenses?.[0].merchant).not.toContain(CONST.CUSTOM_UNITS.DISTANCE_UNIT_KILOMETERS); + }); }); describe('setDraftSplitTransaction', () => { diff --git a/tests/actions/IOUTest/TrackExpenseTest.ts b/tests/actions/IOUTest/TrackExpenseTest.ts index 6836b903a7c9..3fd998baf749 100644 --- a/tests/actions/IOUTest/TrackExpenseTest.ts +++ b/tests/actions/IOUTest/TrackExpenseTest.ts @@ -15,7 +15,7 @@ import {subscribeToUserEvents} from '@libs/actions/User'; import {WRITE_COMMANDS} from '@libs/API/types'; import {getLoginsByAccountIDs} from '@libs/PersonalDetailsUtils'; import type * as PolicyUtils from '@libs/PolicyUtils'; -import {getOriginalMessage, isActionableTrackExpense, isMoneyRequestAction} from '@libs/ReportActionsUtils'; +import {getOriginalMessage, isActionableTrackExpense, isDeletedAction, isMoneyRequestAction} from '@libs/ReportActionsUtils'; import type {OptimisticChatReport} from '@libs/ReportUtils'; import {createDraftTransactionAndNavigateToParticipantSelector} from '@libs/ReportUtils'; import SidebarUtils from '@libs/SidebarUtils'; @@ -2596,6 +2596,8 @@ describe('actions/IOU/TrackExpense', () => { beforeEach(async () => { jest.clearAllMocks(); + // Spies installed by a previous test would otherwise keep `API.write` stubbed for this setup + jest.restoreAllMocks(); PusherHelper.setup(); await signInWithTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN); @@ -2727,6 +2729,70 @@ describe('actions/IOU/TrackExpense', () => { }), ); }); + + it('empties out every live IOU action pointing at the transaction, not only the one it was called with', async () => { + // Given a second live IOU action in the self-DM for the same transaction + const duplicateIOUActionID = 'duplicate-iou-action'; + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`, { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + [duplicateIOUActionID]: {...iouReportAction!, reportActionID: duplicateIOUActionID}, + }); + await waitForBatchedUpdates(); + + // When the expense is deleted through one of the two actions + deleteTrackExpense({ + chatReportID: selfDMReport.reportID, + chatReport: selfDMReport, + chatReportActions: undefined, + transactionID: transaction?.transactionID, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + reportAction: iouReportAction!, + iouReport: undefined, + chatIOUReport: undefined, + transactions: {}, + violations: {}, + isSingleTransactionView: false, + isChatReportArchived: false, + isChatIOUReportArchived: false, + allTransactionViolationsParam: {}, + currentUserAccountID: TEST_USER_ACCOUNT_ID, + currentUserEmail: TEST_USER_LOGIN, + }); + await waitForBatchedUpdates(); + + // Then both actions read as deleted, so neither keeps the expense reachable in the chat + const updatedActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + expect(isDeletedAction(updatedActions?.[iouReportAction!.reportActionID])).toBe(true); + expect(isDeletedAction(updatedActions?.[duplicateIOUActionID])).toBe(true); + }); + + it('drops the transaction from Onyx once the delete goes through', async () => { + // When a tracked expense is deleted + deleteTrackExpense({ + chatReportID: selfDMReport.reportID, + chatReport: selfDMReport, + chatReportActions: undefined, + transactionID: transaction?.transactionID, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + reportAction: iouReportAction!, + iouReport: undefined, + chatIOUReport: undefined, + transactions: {}, + violations: {}, + isSingleTransactionView: false, + isChatReportArchived: false, + isChatIOUReportArchived: false, + allTransactionViolationsParam: {}, + currentUserAccountID: TEST_USER_ACCOUNT_ID, + currentUserEmail: TEST_USER_LOGIN, + }); + await waitForBatchedUpdates(); + + // Then it is gone rather than left behind marked as pending deletion, which the lists that include + // pending-delete transactions while offline would keep showing + await expect(getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction?.transactionID}`)).resolves.toBeFalsy(); + }); }); describe('convertBulkTrackedExpensesToIOU', () => { diff --git a/tests/actions/TransactionTest.ts b/tests/actions/TransactionTest.ts index 403b241fbce3..d483abc943d6 100644 --- a/tests/actions/TransactionTest.ts +++ b/tests/actions/TransactionTest.ts @@ -11,8 +11,8 @@ import '@libs/actions/IOU/MoneyRequest'; import {createWorkspace, generatePolicyID, setWorkspaceApprovalMode} from '@libs/actions/Policy/Policy'; import {createNewReport} from '@libs/actions/Report'; import type * as PolicyUtils from '@libs/PolicyUtils'; -import {getOriginalMessage, isMoneyRequestAction} from '@libs/ReportActionsUtils'; -import {getReportOrDraftReport} from '@libs/ReportUtils'; +import {getOriginalMessage, isMoneyRequestAction, shouldReportActionBeVisible} from '@libs/ReportActionsUtils'; +import {buildOptimisticIOUReportAction, getReportOrDraftReport} from '@libs/ReportUtils'; import CONST from '@src/CONST'; import IntlStore from '@src/languages/IntlStore'; @@ -351,6 +351,83 @@ describe('actions/Transaction', () => { expect(updatedExpenseReport?.unheldNonReimbursableTotal).toBe(-amount); }); + it('stops rendering the self-DM report action once its expense moves to a report, even when the action still carries a pending state', async () => { + // Given an unreported expense in the self-DM whose IOU action is still marked as pending, which happens when + // an earlier optimistic write on that action was never resolved + const selfDMReport: Report = {...createRandomReport(77, CONST.REPORT.CHAT_TYPE.SELF_DM), reportID: '77'}; + const movePolicy: Policy = {...createRandomPolicy(78, CONST.POLICY.TYPE.TEAM, 'Move Workspace'), id: 'policy-for-move'}; + const workspaceChat: Report = {...createRandomReport(79, CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT), reportID: '79', policyID: movePolicy.id}; + const destinationReport: Report = { + ...createRandomReport(80), + reportID: '80', + type: CONST.REPORT.TYPE.EXPENSE, + policyID: movePolicy.id, + chatReportID: workspaceChat.reportID, + ownerAccountID: CARLOS_ACCOUNT_ID, + }; + const movedTransaction: Transaction = { + transactionID: 'transaction-to-move', + amount: -5000, + currency: CONST.CURRENCY.USD, + merchant: 'merchant', + created: format(new Date(), CONST.DATE.FNS_FORMAT_STRING), + comment: {comment: ''}, + reportID: CONST.REPORT.UNREPORTED_REPORT_ID, + }; + const selfDMIOUAction: ReportAction = { + ...buildOptimisticIOUReportAction({ + type: CONST.IOU.REPORT_ACTION_TYPE.TRACK, + amount: 5000, + currency: CONST.CURRENCY.USD, + comment: '', + participants: [{accountID: CARLOS_ACCOUNT_ID, login: CARLOS_EMAIL}], + transactionID: movedTransaction.transactionID, + isPersonalTrackingExpense: true, + }), + reportID: selfDMReport.reportID, + pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, + }; + + await Onyx.merge(ONYXKEYS.SESSION, {email: CARLOS_EMAIL, accountID: CARLOS_ACCOUNT_ID}); + await Onyx.merge(ONYXKEYS.SELF_DM_REPORT_ID, selfDMReport.reportID); + await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${movePolicy.id}`, movePolicy); + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${selfDMReport.reportID}`, selfDMReport); + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${workspaceChat.reportID}`, workspaceChat); + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${destinationReport.reportID}`, destinationReport); + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${movedTransaction.transactionID}`, movedTransaction); + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`, {[selfDMIOUAction.reportActionID]: selfDMIOUAction}); + await waitForBatchedUpdates(); + + let allTransactions: OnyxCollection; + let allReports: OnyxCollection; + await getOnyxData({key: ONYXKEYS.COLLECTION.TRANSACTION, callback: (value) => (allTransactions = value)}); + await getOnyxData({key: ONYXKEYS.COLLECTION.REPORT, callback: (value) => (allReports = value)}); + + // When the expense is moved to a workspace report + changeTransactionsReport({ + transactionIDs: [movedTransaction.transactionID], + isASAPSubmitBetaEnabled: false, + accountID: CARLOS_ACCOUNT_ID, + email: CARLOS_EMAIL, + newReport: destinationReport, + policy: movePolicy, + allTransactions, + policyTagList: {}, + transactionViolations: {}, + allReports, + selfDMReportActions: {[selfDMIOUAction.reportActionID]: selfDMIOUAction}, + isTrackIntentUser: false, + }); + await waitForBatchedUpdates(); + + // Then the action left behind in the self-DM holds no transaction, no pending state, and is not rendered + const selfDMActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`); + const retiredAction = selfDMActions?.[selfDMIOUAction.reportActionID]; + expect(isMoneyRequestAction(retiredAction) ? getOriginalMessage(retiredAction)?.IOUTransactionID : undefined).toBeFalsy(); + expect(retiredAction?.pendingAction).toBeFalsy(); + expect(shouldReportActionBeVisible(retiredAction, selfDMIOUAction.reportActionID, true, CARLOS_ACCOUNT_ID)).toBe(false); + }); + it('recomputes a distance expense amount/merchant/currency from the destination workspace rate when moved', async () => { // Given a destination workspace whose default distance rate is defined in GBP (200/mi) const policyID = generatePolicyID(); diff --git a/tests/unit/TransactionPreviewUtils.test.ts b/tests/unit/TransactionPreviewUtils.test.ts index 8d0eca4093bf..353fe66b9cfc 100644 --- a/tests/unit/TransactionPreviewUtils.test.ts +++ b/tests/unit/TransactionPreviewUtils.test.ts @@ -471,6 +471,25 @@ describe('TransactionPreviewUtils', () => { expect(result.shouldShowSkeleton).toBeTruthy(); }); + it('should not show skeleton for an action the backend marked deleted, whose transaction will never arrive', () => { + // Given a money request action deleted the way the backend reports it — `deleted` timestamps on the + // message and the original message rather than the `isDeletedParentAction` flag — and no transaction + const functionArgs = { + ...basicProps, + transaction: undefined, + action: { + ...basicProps.action, + message: [{type: 'TEXT', text: '', deleted: '2026-07-30 10:31:05.644'}], + }, + }; + + // When the preview conditionals are computed + const result = createTransactionPreviewConditionals(functionArgs); + + // Then the preview stays out of the loading state instead of waiting for a transaction that is gone + expect(result.shouldShowSkeleton).toBeFalsy(); + }); + it('should show merchant if merchant data is valid and significant', () => { const functionArgs = {...basicProps, transactionDetails: {merchant: 'Valid Merchant'}}; const result = createTransactionPreviewConditionals(functionArgs); diff --git a/tests/unit/hooks/useDeleteTransactions.test.ts b/tests/unit/hooks/useDeleteTransactions.test.ts new file mode 100644 index 000000000000..bbd0e936586c --- /dev/null +++ b/tests/unit/hooks/useDeleteTransactions.test.ts @@ -0,0 +1,110 @@ +import {act, renderHook} from '@testing-library/react-native'; + +import OnyxListItemProvider from '@components/OnyxListItemProvider'; + +import useDeleteTransactions from '@hooks/useDeleteTransactions'; + +import {buildOptimisticIOUReportAction} from '@libs/ReportUtils'; + +import CONST from '@src/CONST'; +import DateUtils from '@src/libs/DateUtils'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {ReportActions} from '@src/types/onyx'; +import type ReportAction from '@src/types/onyx/ReportAction'; +import type Transaction from '@src/types/onyx/Transaction'; + +import Onyx from 'react-native-onyx'; + +import {createSelfDM} from '../../utils/collections/reports'; +import getOnyxValue from '../../utils/getOnyxValue'; +import {getGlobalFetchMock} from '../../utils/TestHelper'; +import waitForBatchedUpdates from '../../utils/waitForBatchedUpdates'; + +jest.mock('@libs/Navigation/Navigation', () => ({ + navigate: jest.fn(), + dismissModal: jest.fn(), + goBack: jest.fn(), + getTopmostReportId: jest.fn(() => '1'), + setNavigationActionToMicrotaskQueue: jest.fn(), + isNavigationReady: jest.fn(() => Promise.resolve()), + getReportRouteByID: jest.fn(), + getActiveRoute: jest.fn(), + navigationRef: {getRootState: jest.fn(), isReady: jest.fn(() => true)}, +})); +jest.mock('@react-navigation/native'); +jest.mock('@libs/Navigation/helpers/isSearchTopmostFullScreenRoute', () => jest.fn()); +jest.mock('@libs/Navigation/helpers/isReportTopmostSplitNavigator', () => jest.fn()); +jest.mock('@src/libs/actions/Report', () => ({ + ...jest.requireActual>('@src/libs/actions/Report'), + notifyNewAction: jest.fn(), + setDeleteTransactionNavigateBackUrl: jest.fn(), +})); + +const RORY_EMAIL = 'rory@expensifail.com'; +const RORY_ACCOUNT_ID = 3; + +beforeAll(() => { + Onyx.init({keys: ONYXKEYS}); + return waitForBatchedUpdates(); +}); + +beforeEach(async () => { + global.fetch = getGlobalFetchMock(); + await Onyx.clear(); + await Onyx.multiSet({ + [ONYXKEYS.SESSION]: {accountID: RORY_ACCOUNT_ID, email: RORY_EMAIL}, + [ONYXKEYS.PERSONAL_DETAILS_LIST]: {[RORY_ACCOUNT_ID]: {accountID: RORY_ACCOUNT_ID, login: RORY_EMAIL}}, + }); +}); + +describe('useDeleteTransactions', () => { + it('deletes the self-DM IOU action along with an unreported expense instead of leaving an orphaned preview', async () => { + // Given an unreported (self-DM) expense whose IOU action lives in the self-DM chat, with no IOU report + const selfDMReport = createSelfDM(2, RORY_ACCOUNT_ID); + const transactionID = 'unreported-transaction'; + const transaction: Transaction = { + transactionID, + amount: -10000, + currency: 'USD', + merchant: 'Test Merchant', + comment: {comment: 'Test comment'}, + created: DateUtils.getDBTime(), + reportID: CONST.REPORT.UNREPORTED_REPORT_ID, + }; + const iouAction: ReportAction = { + ...buildOptimisticIOUReportAction({ + type: CONST.IOU.REPORT_ACTION_TYPE.TRACK, + amount: 10000, + currency: 'USD', + comment: '', + participants: [{accountID: RORY_ACCOUNT_ID, login: RORY_EMAIL}], + transactionID, + isPersonalTrackingExpense: true, + }), + reportID: selfDMReport.reportID, + }; + + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${selfDMReport.reportID}`, selfDMReport); + await Onyx.merge(ONYXKEYS.SELF_DM_REPORT_ID, selfDMReport.reportID); + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, transaction); + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`, {[iouAction.reportActionID]: iouAction}); + await waitForBatchedUpdates(); + + const {result} = renderHook(() => useDeleteTransactions({report: selfDMReport, reportActions: [iouAction]}), {wrapper: OnyxListItemProvider}); + await waitForBatchedUpdates(); + + // When the expense is deleted (e.g. from Search / a bulk selection) + act(() => { + result.current.deleteTransactions([transactionID], {}, {}); + }); + await waitForBatchedUpdates(); + + // Then the IOU action in the self-DM report is emptied out along with the transaction + const selfDMActions = (await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`)) as ReportActions | undefined; + const deletedAction = selfDMActions?.[iouAction.reportActionID]; + expect(Array.isArray(deletedAction?.message) ? deletedAction?.message.at(0)?.html : undefined).toBe(''); + + // And the report actions are keyed on the self-DM report, not on a missing IOU report + await expect(getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}undefined`)).resolves.toBeFalsy(); + }); +}); From 0dfb8d87b0a34ee27a6845d7abcd903824606553 Mon Sep 17 00:00:00 2001 From: yauhenihorbach Date: Thu, 30 Jul 2026 15:59:09 +0200 Subject: [PATCH 2/5] Fix spellcheck and codex comment --- src/hooks/useDeleteTransactions.ts | 13 +++++++++++-- tests/actions/TransactionTest.ts | 10 +++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/hooks/useDeleteTransactions.ts b/src/hooks/useDeleteTransactions.ts index f2348969abc5..236a6fd3493e 100644 --- a/src/hooks/useDeleteTransactions.ts +++ b/src/hooks/useDeleteTransactions.ts @@ -22,7 +22,7 @@ import { import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {Policy, Report, ReportAction, Transaction, TransactionViolations} from '@src/types/onyx'; +import type {Policy, Report, ReportAction, ReportActions, Transaction, TransactionViolations} from '@src/types/onyx'; import type {SplitExpense} from '@src/types/onyx/IOU'; import type {OnyxCollection} from 'react-native-onyx'; @@ -351,10 +351,19 @@ function useDeleteTransactions({report, reportActions, policy}: UseDeleteTransac // A self-DM expense is a tracked expense: it has no IOU report to key the cleanup on, so it goes // through the track-expense flow, which resolves the self-DM report actions and the whisper. if (isSelfDM(candidateIOUReport) && isTrackExpenseAction(action)) { + // The Onyx collection can be missing the self-DM actions when the delete comes from Search, where + // they are read from the search snapshot instead, so the ones passed in fill the gaps. + const selfDMReportActions: ReportActions = { + ...Object.fromEntries( + reportActions.filter((chatAction) => chatAction.reportID === candidateIOUReport?.reportID).map((chatAction) => [chatAction.reportActionID, chatAction]), + ), + ...allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${candidateIOUReport?.reportID}`], + }; + deleteTrackExpense({ chatReportID: candidateIOUReport?.reportID, chatReport: candidateIOUReport, - chatReportActions: allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${candidateIOUReport?.reportID}`], + chatReportActions: selfDMReportActions, transactionID, reportAction: action, iouReport: undefined, diff --git a/tests/actions/TransactionTest.ts b/tests/actions/TransactionTest.ts index d483abc943d6..77d411d1287f 100644 --- a/tests/actions/TransactionTest.ts +++ b/tests/actions/TransactionTest.ts @@ -374,7 +374,7 @@ describe('actions/Transaction', () => { comment: {comment: ''}, reportID: CONST.REPORT.UNREPORTED_REPORT_ID, }; - const selfDMIOUAction: ReportAction = { + const trackedExpenseAction: ReportAction = { ...buildOptimisticIOUReportAction({ type: CONST.IOU.REPORT_ACTION_TYPE.TRACK, amount: 5000, @@ -395,7 +395,7 @@ describe('actions/Transaction', () => { await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${workspaceChat.reportID}`, workspaceChat); await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${destinationReport.reportID}`, destinationReport); await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${movedTransaction.transactionID}`, movedTransaction); - await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`, {[selfDMIOUAction.reportActionID]: selfDMIOUAction}); + await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`, {[trackedExpenseAction.reportActionID]: trackedExpenseAction}); await waitForBatchedUpdates(); let allTransactions: OnyxCollection; @@ -415,17 +415,17 @@ describe('actions/Transaction', () => { policyTagList: {}, transactionViolations: {}, allReports, - selfDMReportActions: {[selfDMIOUAction.reportActionID]: selfDMIOUAction}, + selfDMReportActions: {[trackedExpenseAction.reportActionID]: trackedExpenseAction}, isTrackIntentUser: false, }); await waitForBatchedUpdates(); // Then the action left behind in the self-DM holds no transaction, no pending state, and is not rendered const selfDMActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`); - const retiredAction = selfDMActions?.[selfDMIOUAction.reportActionID]; + const retiredAction = selfDMActions?.[trackedExpenseAction.reportActionID]; expect(isMoneyRequestAction(retiredAction) ? getOriginalMessage(retiredAction)?.IOUTransactionID : undefined).toBeFalsy(); expect(retiredAction?.pendingAction).toBeFalsy(); - expect(shouldReportActionBeVisible(retiredAction, selfDMIOUAction.reportActionID, true, CARLOS_ACCOUNT_ID)).toBe(false); + expect(shouldReportActionBeVisible(retiredAction, trackedExpenseAction.reportActionID, true, CARLOS_ACCOUNT_ID)).toBe(false); }); it('recomputes a distance expense amount/merchant/currency from the destination workspace rate when moved', async () => { From be0581471c6a0d478f5d62c44e9c956da68f85cb Mon Sep 17 00:00:00 2001 From: yauhenihorbach Date: Fri, 31 Jul 2026 17:41:36 +0200 Subject: [PATCH 3/5] Fix comments and revert fix for 97400 --- src/hooks/useDeleteTransactions.ts | 9 +- .../actions/IOU/SplitTransactionUpdate.ts | 31 ++-- src/libs/actions/IOU/TrackExpense.ts | 66 +++------ tests/actions/IOUTest/SplitTest.ts | 136 +----------------- tests/actions/IOUTest/TrackExpenseTest.ts | 68 +-------- 5 files changed, 35 insertions(+), 275 deletions(-) diff --git a/src/hooks/useDeleteTransactions.ts b/src/hooks/useDeleteTransactions.ts index 236a6fd3493e..e176bdbe0aa3 100644 --- a/src/hooks/useDeleteTransactions.ts +++ b/src/hooks/useDeleteTransactions.ts @@ -9,7 +9,7 @@ import {deleteTrackExpense} from '@libs/actions/IOU/TrackExpense'; import initSplitExpense from '@libs/actions/SplitExpenses'; import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import {calculateAmount as calculateIOUAmount} from '@libs/IOUUtils'; -import {getOriginalMessage, isMoneyRequestAction, isTrackExpenseAction} from '@libs/ReportActionsUtils'; +import {getOriginalMessage, isActionableTrackExpense, isMoneyRequestAction, isTrackExpenseAction} from '@libs/ReportActionsUtils'; import {isArchivedReport, isExpenseReport, isIOUReport, isSelfDM} from '@libs/ReportUtils'; import {getActiveGroupSearchHashes} from '@libs/SearchUIUtils'; import { @@ -352,10 +352,13 @@ function useDeleteTransactions({report, reportActions, policy}: UseDeleteTransac // through the track-expense flow, which resolves the self-DM report actions and the whisper. if (isSelfDM(candidateIOUReport) && isTrackExpenseAction(action)) { // The Onyx collection can be missing the self-DM actions when the delete comes from Search, where - // they are read from the search snapshot instead, so the ones passed in fill the gaps. + // they are read from the search snapshot instead, so the ones passed in fill the gaps. Actionable + // track expense whispers are matched on their own since they are built without a `reportID`. const selfDMReportActions: ReportActions = { ...Object.fromEntries( - reportActions.filter((chatAction) => chatAction.reportID === candidateIOUReport?.reportID).map((chatAction) => [chatAction.reportActionID, chatAction]), + reportActions + .filter((chatAction) => chatAction.reportID === candidateIOUReport?.reportID || isActionableTrackExpense(chatAction)) + .map((chatAction) => [chatAction.reportActionID, chatAction]), ), ...allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${candidateIOUReport?.reportID}`], }; diff --git a/src/libs/actions/IOU/SplitTransactionUpdate.ts b/src/libs/actions/IOU/SplitTransactionUpdate.ts index c0222c53cda7..b63a1160807f 100644 --- a/src/libs/actions/IOU/SplitTransactionUpdate.ts +++ b/src/libs/actions/IOU/SplitTransactionUpdate.ts @@ -1327,23 +1327,6 @@ function updateSplitTransactions({ onyxData.successData?.push(...(moneyRequestInformationOnyxData.successData ?? [])); onyxData.failureData?.push(...(moneyRequestInformationOnyxData.failureData ?? [])); } - // A revert restores an expense that already existed, so it isn't pending creation — - // `buildOptimisticIOUReportAction` stamps ADD on every action it builds. - if (isReverseSplitOperation && reportActionsReportID) { - onyxData.optimisticData?.push( - { - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportActionsReportID}`, - value: {[iouAction.reportActionID]: {pendingAction: null}}, - }, - { - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.TRANSACTION}${existingTransactionID}`, - value: {pendingAction: null}, - }, - ); - } - onyxData.optimisticData?.push(...(updateMoneyRequestParamsOnyxData.optimisticData ?? []), ...optimisticDataComments); onyxData.successData?.push(...(updateMoneyRequestParamsOnyxData.successData ?? []), ...successDataComments); onyxData.failureData?.push(...(updateMoneyRequestParamsOnyxData.failureData ?? []), ...failureDataComments); @@ -1375,10 +1358,12 @@ function updateSplitTransactions({ continue; } - // A reverse split brings the original transaction back in place of every split, so all of them are removed - // outright rather than marked for deletion. A split left marked keeps its report action visible (the offline - // surfaces render a pending deletion on purpose), and its pending state has nothing left to resolve it. - const shouldForceDeleteSplit = isReverseSplitOperation; + // For a reverse split operation (i.e. deleting one transaction from a 2-split), the other split(undeleted) + // transaction also gets marked for deletion optimistically. This causes the undeleted split to remain visible, + // resulting in 3 transactions(deleted, undeleted, and original) being shown at the same time when offline. + // Since original transaction will be reverted and both splits will eventually be deleted, we remove + // the undeleted split entirely instead of marking it for deletion. + const forceDeleteSplitTransactionID = isReverseSplitOperation ? splitExpenses.at(0)?.transactionID : undefined; const { optimisticData: deleteExpenseOptimisticData, @@ -1394,7 +1379,7 @@ function updateSplitTransactions({ undefined, undefined, undefined, - isReportArchived || shouldForceDeleteSplit, + isReportArchived || undeletedTransaction?.transactionID === forceDeleteSplitTransactionID, ); // getDeleteTrackExpenseInformation only handles deleting the transaction report thread, so we need to update the report preview action here @@ -1413,7 +1398,7 @@ function updateSplitTransactions({ onyxData.successData?.push(...(deleteExpenseSuccessData ?? [])); onyxData.failureData?.push(...(deleteExpenseFailureData ?? [])); - if (undeletedTransaction?.transactionID && shouldForceDeleteSplit) { + if (undeletedTransaction?.transactionID && undeletedTransaction.transactionID === forceDeleteSplitTransactionID) { onyxData.optimisticData?.push({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportActionsReportID}`, diff --git a/src/libs/actions/IOU/TrackExpense.ts b/src/libs/actions/IOU/TrackExpense.ts index 0ae6b2250c63..b0378ebf0c5f 100644 --- a/src/libs/actions/IOU/TrackExpense.ts +++ b/src/libs/actions/IOU/TrackExpense.ts @@ -29,7 +29,6 @@ import { getReportActionText, getTrackExpenseActionableWhisper, isActionableTrackExpense, - isDeletedAction, isMoneyRequestAction, } from '@libs/ReportActionsUtils'; import type {OptimisticChatReport, OptimisticCreatedReportAction, OptimisticIOUReportAction} from '@libs/ReportUtils'; @@ -667,41 +666,24 @@ function getDeleteTrackExpenseInformation( const shouldShowDeletedRequestMessage = !isMovingTransactionFromTrackExpense && !!transactionThreadID && !shouldDeleteTransactionThread; // STEP 3: Update the IOU reportAction. - const deletedActionValue = { - pendingAction: shouldShowDeletedRequestMessage ? CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE : CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, - previousMessage: reportAction.message, - message: [ - { - type: 'COMMENT', - html: '', - text: '', - isEdited: true, - isDeletedParentAction: shouldShowDeletedRequestMessage, + const updatedReportAction = { + [reportAction.reportActionID]: { + pendingAction: shouldShowDeletedRequestMessage ? CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE : CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, + previousMessage: reportAction.message, + message: [ + { + type: 'COMMENT', + html: '', + text: '', + isEdited: true, + isDeletedParentAction: shouldShowDeletedRequestMessage, + }, + ], + originalMessage: { + IOUTransactionID: shouldRemoveIOUTransaction ? null : transactionID, }, - ], - originalMessage: { - IOUTransactionID: shouldRemoveIOUTransaction ? null : transactionID, + errors: undefined, }, - errors: undefined, - }; - - // A report is expected to hold a single live IOU action per transaction, but the chat can end up with more than one - // (an optimistic action plus the one the backend created for the same transaction), so all of them are emptied out - // here. Only create/track actions are swept — a payment action referencing the transaction stays untouched. - const siblingIOUActions = Object.values(getAllReportActions(chatReport?.reportID)).filter((chatAction) => { - if (chatAction.reportActionID === reportAction.reportActionID || !isMoneyRequestAction(chatAction) || isDeletedAction(chatAction)) { - return false; - } - const siblingOriginalMessage = getOriginalMessage(chatAction); - return ( - siblingOriginalMessage?.IOUTransactionID === transactionID && - (siblingOriginalMessage?.type === CONST.IOU.REPORT_ACTION_TYPE.TRACK || siblingOriginalMessage?.type === CONST.IOU.REPORT_ACTION_TYPE.CREATE) - ); - }); - - const updatedReportAction = { - [reportAction.reportActionID]: deletedActionValue, - ...Object.fromEntries(siblingIOUActions.map((siblingAction) => [siblingAction.reportActionID, {...deletedActionValue, previousMessage: siblingAction.message}])), ...(actionableWhisperReportActionID && {[actionableWhisperReportActionID]: {originalMessage: {resolution}}}), } as OnyxTypes.ReportActions; let canUserPerformWriteAction = true; @@ -763,7 +745,7 @@ function getDeleteTrackExpenseInformation( }, ); - const successData: Array> = [ + const successData: Array> = [ { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport?.reportID}`, @@ -772,21 +754,10 @@ function getDeleteTrackExpenseInformation( pendingAction: null, errors: null, }, - ...Object.fromEntries(siblingIOUActions.map((siblingAction) => [siblingAction.reportActionID, {pendingAction: null, errors: null}])), }, }, ]; - // When the transaction is only marked as pending deletion above, drop it once the delete goes through. Left in - // place it keeps surfacing in the lists that include pending-delete transactions while offline. - if (shouldDeleteTransactionFromOnyx && !isMovingTransactionFromTrackExpense) { - successData.push({ - onyxMethod: Onyx.METHOD.SET, - key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, - value: null, - }); - } - // Ensure that any remaining data is removed upon successful completion, even if the server sends a report removal response. // This is done to prevent the removal update from lingering in the applyHTTPSOnyxUpdates function. successData.push(...cleanUpTransactionThreadReportOnyxData.successData); @@ -844,7 +815,6 @@ function getDeleteTrackExpenseInformation( pendingAction: null, errors: getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericDeleteFailureMessage'), }, - ...Object.fromEntries(siblingIOUActions.map((siblingAction) => [siblingAction.reportActionID, {...siblingAction, pendingAction: null}])), }, }, { @@ -1244,7 +1214,7 @@ const getConvertTrackedExpenseInformation = ( const optimisticData: Array< OnyxUpdate > = []; - const successData: Array> = []; + const successData: Array> = []; const failureData: Array< OnyxUpdate > = []; diff --git a/tests/actions/IOUTest/SplitTest.ts b/tests/actions/IOUTest/SplitTest.ts index ef0d06796d9b..775ce793a081 100644 --- a/tests/actions/IOUTest/SplitTest.ts +++ b/tests/actions/IOUTest/SplitTest.ts @@ -3891,9 +3891,9 @@ describe('updateSplitTransactionsFromSplitExpensesFlow', () => { const deletedSplit1 = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${splitTransactionID1}`); expect(deletedSplit1).toBeFalsy(); - // The other split (split2) is marked pendingAction: delete optimistically and dropped once the revert succeeds + // The other split (split2) is marked pendingAction: delete const deletedSplit2 = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${splitTransactionID2}`); - expect(deletedSplit2).toBeFalsy(); + expect(deletedSplit2?.pendingAction).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); // Step 5: Verify the new IOU action for the restored original transaction const revertExpenseReportID = reports.expenseReport?.reportID; @@ -6459,138 +6459,6 @@ describe('updateSplitTransactions', () => { expect(isDeletedAction(updatedStaleAction1)).toBe(true); expect(isDeletedAction(updatedStaleAction2)).toBe(true); }); - - it('should remove the report actions of every split when reverting, instead of leaving them pending deletion', async () => { - // Given a selfDM expense split into two children, each with its own IOU action in the selfDM - const selfDMReport = createSelfDM(2, RORY_ACCOUNT_ID); - const originalTransactionID = 'revert-actions-original'; - const childTransactionID1 = 'revert-actions-child-1'; - const childTransactionID2 = 'revert-actions-child-2'; - const created = DateUtils.getDBTime(); - - const buildChild = (transactionID: string): Transaction => ({ - transactionID, - amount: -5000, - currency: 'USD', - merchant: 'Test Merchant', - comment: {originalTransactionID, source: CONST.IOU.TYPE.SPLIT}, - created, - reportID: CONST.REPORT.UNREPORTED_REPORT_ID, - }); - const buildTrackAction = (transactionID: string): ReportAction => ({ - ...buildOptimisticIOUReportAction({ - type: CONST.IOU.REPORT_ACTION_TYPE.TRACK, - amount: 5000, - currency: 'USD', - comment: '', - participants: [{accountID: RORY_ACCOUNT_ID, login: RORY_EMAIL}], - transactionID, - isPersonalTrackingExpense: true, - }), - reportID: selfDMReport.reportID, - }); - - const child1Action = buildTrackAction(childTransactionID1); - const child2Action = buildTrackAction(childTransactionID2); - - await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${selfDMReport.reportID}`, selfDMReport); - await Onyx.merge(ONYXKEYS.SELF_DM_REPORT_ID, selfDMReport.reportID); - await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${originalTransactionID}`, { - transactionID: originalTransactionID, - amount: -10000, - currency: 'USD', - merchant: 'Test Merchant', - comment: {comment: ''}, - created, - reportID: CONST.REPORT.SPLIT_REPORT_ID, - }); - await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${childTransactionID1}`, buildChild(childTransactionID1)); - await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${childTransactionID2}`, buildChild(childTransactionID2)); - await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`, { - [child1Action.reportActionID]: child1Action, - [child2Action.reportActionID]: child2Action, - }); - await waitForBatchedUpdates(); - - let allTransactions: OnyxCollection; - let allReports: OnyxCollection; - let allReportNameValuePairs: OnyxCollection; - let allReportActions: OnyxCollection; - let allSnapshots: OnyxCollection; - await getOnyxData({key: ONYXKEYS.COLLECTION.TRANSACTION, callback: (value) => (allTransactions = value)}); - await getOnyxData({key: ONYXKEYS.COLLECTION.REPORT, callback: (value) => (allReports = value)}); - await getOnyxData({key: ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS, callback: (value) => (allReportNameValuePairs = value)}); - await getOnyxData({key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, callback: (value) => (allReportActions = value)}); - await getOnyxData({key: ONYXKEYS.COLLECTION.SNAPSHOT, callback: (value) => (allSnapshots = value)}); - - // When one of the two splits is deleted, which reverts the split back to the original transaction - mockFetch?.pause?.(); - updateSplitTransactions({ - allTransactionsList: allTransactions, - allReportsList: allReports, - allReportActionsList: allReportActions, - allReportNameValuePairsList: allReportNameValuePairs, - allSnapshots, - allPolicyTags: {}, - transactionData: { - reportID: selfDMReport.reportID, - originalTransactionID, - splitExpenses: [{transactionID: childTransactionID1, amount: 10000, created, reportID: selfDMReport.reportID}], - splitExpensesTotal: 10000, - }, - searchContext: {currentSearchHash: undefined, activeGroupSearchHashes: []}, - policyCategories: undefined, - policy: undefined, - policyRecentlyUsedCategories: [], - iouReport: undefined, - firstIOU: undefined, - isASAPSubmitBetaEnabled: false, - currentUserPersonalDetails, - transactionViolations: {}, - policyRecentlyUsedCurrencies: [], - quickAction: undefined, - iouReportNextStep: undefined, - betas: [CONST.BETAS.ALL], - personalDetails: {[RORY_ACCOUNT_ID]: {accountID: RORY_ACCOUNT_ID, login: RORY_EMAIL}}, - transactionReport: selfDMReport, - expenseReport: undefined, - isOffline: false, - delegateAccountID: undefined, - isTrackIntentUser: false, - }); - await waitForBatchedUpdates(); - - // Then the restored expense isn't marked as pending creation — it already existed before the split - const optimisticActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`); - const restoredAction = Object.values(optimisticActions ?? {}).find( - (action) => isMoneyRequestAction(action) && getOriginalMessage(action)?.IOUTransactionID === originalTransactionID && !isDeletedAction(action), - ); - expect(restoredAction).toBeTruthy(); - expect(restoredAction?.pendingAction).toBeFalsy(); - const inFlightTransaction = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${originalTransactionID}`); - expect(inFlightTransaction?.pendingAction).toBeFalsy(); - - await mockFetch?.resume?.(); - await waitForBatchedUpdates(); - - // And it stays that way after the request goes through - const settledActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`); - const settledRestoredAction = Object.values(settledActions ?? {}).find( - (action) => isMoneyRequestAction(action) && getOriginalMessage(action)?.IOUTransactionID === originalTransactionID && !isDeletedAction(action), - ); - expect(settledRestoredAction?.pendingAction).toBeFalsy(); - const restoredTransaction = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${originalTransactionID}`); - expect(restoredTransaction?.pendingAction).toBeFalsy(); - - // And neither split is left holding a transaction or a pending state, which the offline surfaces would render - const updatedActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`); - for (const childAction of [updatedActions?.[child1Action.reportActionID], updatedActions?.[child2Action.reportActionID]]) { - expect(childAction?.pendingAction).toBeFalsy(); - expect(isMoneyRequestAction(childAction) && getOriginalMessage(childAction)?.IOUTransactionID).toBeFalsy(); - } - await expect(getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${childTransactionID1}`)).resolves.toBeFalsy(); - await expect(getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${childTransactionID2}`)).resolves.toBeFalsy(); - }); }); describe('initSplitExpense', () => { diff --git a/tests/actions/IOUTest/TrackExpenseTest.ts b/tests/actions/IOUTest/TrackExpenseTest.ts index 3fd998baf749..6836b903a7c9 100644 --- a/tests/actions/IOUTest/TrackExpenseTest.ts +++ b/tests/actions/IOUTest/TrackExpenseTest.ts @@ -15,7 +15,7 @@ import {subscribeToUserEvents} from '@libs/actions/User'; import {WRITE_COMMANDS} from '@libs/API/types'; import {getLoginsByAccountIDs} from '@libs/PersonalDetailsUtils'; import type * as PolicyUtils from '@libs/PolicyUtils'; -import {getOriginalMessage, isActionableTrackExpense, isDeletedAction, isMoneyRequestAction} from '@libs/ReportActionsUtils'; +import {getOriginalMessage, isActionableTrackExpense, isMoneyRequestAction} from '@libs/ReportActionsUtils'; import type {OptimisticChatReport} from '@libs/ReportUtils'; import {createDraftTransactionAndNavigateToParticipantSelector} from '@libs/ReportUtils'; import SidebarUtils from '@libs/SidebarUtils'; @@ -2596,8 +2596,6 @@ describe('actions/IOU/TrackExpense', () => { beforeEach(async () => { jest.clearAllMocks(); - // Spies installed by a previous test would otherwise keep `API.write` stubbed for this setup - jest.restoreAllMocks(); PusherHelper.setup(); await signInWithTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN); @@ -2729,70 +2727,6 @@ describe('actions/IOU/TrackExpense', () => { }), ); }); - - it('empties out every live IOU action pointing at the transaction, not only the one it was called with', async () => { - // Given a second live IOU action in the self-DM for the same transaction - const duplicateIOUActionID = 'duplicate-iou-action'; - await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`, { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - [duplicateIOUActionID]: {...iouReportAction!, reportActionID: duplicateIOUActionID}, - }); - await waitForBatchedUpdates(); - - // When the expense is deleted through one of the two actions - deleteTrackExpense({ - chatReportID: selfDMReport.reportID, - chatReport: selfDMReport, - chatReportActions: undefined, - transactionID: transaction?.transactionID, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - reportAction: iouReportAction!, - iouReport: undefined, - chatIOUReport: undefined, - transactions: {}, - violations: {}, - isSingleTransactionView: false, - isChatReportArchived: false, - isChatIOUReportArchived: false, - allTransactionViolationsParam: {}, - currentUserAccountID: TEST_USER_ACCOUNT_ID, - currentUserEmail: TEST_USER_LOGIN, - }); - await waitForBatchedUpdates(); - - // Then both actions read as deleted, so neither keeps the expense reachable in the chat - const updatedActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReport.reportID}`); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - expect(isDeletedAction(updatedActions?.[iouReportAction!.reportActionID])).toBe(true); - expect(isDeletedAction(updatedActions?.[duplicateIOUActionID])).toBe(true); - }); - - it('drops the transaction from Onyx once the delete goes through', async () => { - // When a tracked expense is deleted - deleteTrackExpense({ - chatReportID: selfDMReport.reportID, - chatReport: selfDMReport, - chatReportActions: undefined, - transactionID: transaction?.transactionID, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - reportAction: iouReportAction!, - iouReport: undefined, - chatIOUReport: undefined, - transactions: {}, - violations: {}, - isSingleTransactionView: false, - isChatReportArchived: false, - isChatIOUReportArchived: false, - allTransactionViolationsParam: {}, - currentUserAccountID: TEST_USER_ACCOUNT_ID, - currentUserEmail: TEST_USER_LOGIN, - }); - await waitForBatchedUpdates(); - - // Then it is gone rather than left behind marked as pending deletion, which the lists that include - // pending-delete transactions while offline would keep showing - await expect(getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction?.transactionID}`)).resolves.toBeFalsy(); - }); }); describe('convertBulkTrackedExpensesToIOU', () => { From 4238173b6d6b2a3eaf402f1d21108410a3f78ba3 Mon Sep 17 00:00:00 2001 From: yauhenihorbach Date: Mon, 3 Aug 2026 11:08:38 +0200 Subject: [PATCH 4/5] Improve logic for using different units for splits --- src/libs/actions/IOU/SplitExpenseItems.ts | 78 ++++--- .../iou/SplitExpenseCreateDateRagePage.tsx | 2 + src/pages/iou/SplitExpenseEditPage.tsx | 1 + src/pages/iou/SplitExpensePage.tsx | 4 +- tests/actions/IOUTest/SplitTest.ts | 195 +++++++++++++++++- 5 files changed, 239 insertions(+), 41 deletions(-) diff --git a/src/libs/actions/IOU/SplitExpenseItems.ts b/src/libs/actions/IOU/SplitExpenseItems.ts index 5b573b1c26f4..549206ecd9b0 100644 --- a/src/libs/actions/IOU/SplitExpenseItems.ts +++ b/src/libs/actions/IOU/SplitExpenseItems.ts @@ -72,6 +72,7 @@ function updateSplitExpenseDistanceFromAmount( const customUnit: TransactionCustomUnit = { ...existingCustomUnit, quantity, + distanceUnit: unit, }; const merchant = getDistanceMerchantFromDistance(distanceInUnits, unit, rate, transactionCurrency ?? mileageRate?.currency ?? CONST.CURRENCY.USD); @@ -144,6 +145,39 @@ function resolveSplitMileageRate({ return baseMileageRate; } +/** + * Resolve the rate and unit a split item is calculated with: its own selected rate when that rate still + * resolves (it can live on another workspace), and the rate of the expense being split otherwise. + * + * The unit stays the one the expense is stored with — a workspace switching between miles and kilometers + * doesn't re-express expenses that already exist, so the splits follow the same unit as their expense. + */ +function resolveSplitItemRate({ + customUnit, + fallbackMileageRate, + policy, + policies, +}: { + customUnit: TransactionCustomUnit | undefined; + fallbackMileageRate: ReturnType; + policy: OnyxEntry; + policies?: OnyxCollection; +}): {rate: number | undefined; unit: Unit | undefined} { + const unit = customUnit?.distanceUnit ?? fallbackMileageRate.unit; + const customUnitRateID = customUnit?.customUnitRateID; + if (!customUnitRateID || customUnitRateID === CONST.CUSTOM_UNITS.FAKE_P2P_ID) { + return {rate: fallbackMileageRate.rate, unit}; + } + + const selectedRate = + DistanceRequestUtils.getRateByCustomUnitRateID({policy, customUnitRateID}) ?? DistanceRequestUtils.getEnabledRateByCustomUnitRateIDFromAnyPolicy(customUnitRateID, policies); + if (!selectedRate?.rate || selectedRate.rate <= 0 || selectedRate.enabled === false) { + return {rate: fallbackMileageRate.rate, unit}; + } + + return {rate: selectedRate.rate, unit}; +} + function resolveSplitItemReportID({ childTransaction, allReports, @@ -297,6 +331,7 @@ function addSplitExpenseField( policy: OnyxEntry, isSelfDMSplit: boolean, personalPolicyOutputCurrency: string | undefined, + policies?: OnyxCollection, ) { if (!transaction || !draftTransaction) { return; @@ -318,11 +353,12 @@ function addSplitExpenseField( : undefined; const mileageRate = resolveSplitMileageRate({transaction, policy, isSelfDMSplit, personalPolicyOutputCurrency}); - const {unit, rate} = mileageRate; + const {unit, rate} = resolveSplitItemRate({customUnit, fallbackMileageRate: mileageRate, policy, policies}); if (rate && rate > 0 && customUnit) { // For amount = 0, distance = 0, but we still calculate merchant format - const {merchant: calculatedMerchant} = updateSplitExpenseDistanceFromAmount(0, rate, unit, customUnit, mileageRate, transaction.currency); + const {customUnit: updatedCustomUnit, merchant: calculatedMerchant} = updateSplitExpenseDistanceFromAmount(0, rate, unit, customUnit, mileageRate, transaction.currency); + customUnit = updatedCustomUnit; merchant = calculatedMerchant; } } @@ -380,6 +416,7 @@ function evenlyDistributeSplitExpenseAmounts( policy: OnyxEntry, isSelfDMSplit: boolean, personalPolicyOutputCurrency: string | undefined, + policies?: OnyxCollection, ) { if (!draftTransaction) { return; @@ -399,15 +436,14 @@ function evenlyDistributeSplitExpenseAmounts( const isDistanceRequest = transaction && isDistanceRequestTransactionUtils(transaction); - // Floor-allocation with full remainder added to the last split so the last is always the largest + // Floor-allocation with the full remainder added to the first split, the way the amounts are allocated when + // the splits are created, so distributing them evenly doesn't move the remainder from one split to another const splitCount = splitExpenses.length; - const lastIndex = splitCount - 1; const mileageRate = resolveSplitMileageRate({transaction, policy, isSelfDMSplit, personalPolicyOutputCurrency}); - const {unit, rate} = mileageRate; const updatedSplitExpenses = splitExpenses.map((splitExpense, index) => { - const amount = calculateIOUAmount(splitCount - 1, total, currency, index === lastIndex, true); + const amount = calculateIOUAmount(splitCount - 1, total, currency, index === 0, true); let updatedSplitExpense: SplitExpense = { ...splitExpense, amount, @@ -417,6 +453,7 @@ function evenlyDistributeSplitExpenseAmounts( // Update distance for distance transactions based on new amount and rate if (isDistanceRequest && transaction && splitExpense.customUnit && amount !== 0) { + const {unit, rate} = resolveSplitItemRate({customUnit: splitExpense.customUnit, fallbackMileageRate: mileageRate, policy, policies}); if (rate && rate > 0) { const {customUnit: updatedCustomUnit, merchant} = updateSplitExpenseDistanceFromAmount(amount, rate, unit, splitExpense.customUnit, mileageRate, transaction.currency); @@ -457,6 +494,7 @@ function resetSplitExpensesByDateRange( policy: OnyxEntry, isSelfDMSplit: boolean, personalPolicyOutputCurrency: string | undefined, + policies?: OnyxCollection, ) { if (!transaction || !draftTransaction || !startDate || !endDate) { return; @@ -475,12 +513,10 @@ function resetSplitExpensesByDateRange( const isDistanceRequest = isDistanceRequestTransactionUtils(transaction); const mileageRate = resolveSplitMileageRate({transaction, policy, isSelfDMSplit, personalPolicyOutputCurrency}); - const {unit, rate} = mileageRate; - // Create split expenses for each date with proportional amounts - const lastIndex = dates.length - 1; + // Create split expenses for each date with proportional amounts, the remainder going to the first one const newSplitExpenses: SplitExpense[] = dates.map((date, index) => { - const amount = calculateIOUAmount(lastIndex, total, currency, index === lastIndex, true); + const amount = calculateIOUAmount(dates.length - 1, total, currency, index === 0, true); let splitExpense = initSplitExpenseItemData(transaction, transactionReport, { amount, transactionID: rand64(), @@ -490,6 +526,7 @@ function resetSplitExpensesByDateRange( // Update distance for distance transactions based on new amount and rate if (isDistanceRequest && splitExpense.customUnit && amount !== 0) { + const {unit, rate} = resolveSplitItemRate({customUnit: splitExpense.customUnit, fallbackMileageRate: mileageRate, policy, policies}); if (rate && rate > 0) { const {customUnit: updatedCustomUnit, merchant} = updateSplitExpenseDistanceFromAmount(amount, rate, unit, splitExpense.customUnit, mileageRate, transaction.currency); @@ -555,6 +592,7 @@ function updateSplitExpenseField( policy: OnyxEntry, isSelfDMSplit: boolean, personalPolicyOutputCurrency: string | undefined, + policies?: OnyxCollection, ) { if (!splitExpenseDraftTransaction || !splitExpenseTransactionID || !originalTransactionDraft) { return; @@ -598,7 +636,7 @@ function updateSplitExpenseField( // Recalculate amount for distance transactions when rate or distance changes if (isDistanceRequest && originalTransaction) { const mileageRate = resolveSplitMileageRate({transaction: splitExpenseDraftTransaction, policy, isSelfDMSplit, personalPolicyOutputCurrency}); - const {unit, rate} = mileageRate; + const {unit, rate} = resolveSplitItemRate({customUnit: splitExpenseDraftTransaction?.comment?.customUnit, fallbackMileageRate: mileageRate, policy, policies}); if (rate && rate > 0) { // Get distance from routes or customUnit.quantity (same logic as in initSplitExpense) @@ -673,23 +711,7 @@ function updateSplitExpenseAmountField( // Update distance for distance transactions based on new amount and rate if (isDistanceRequest && originalTransaction && splitExpense.customUnit) { const mileageRate = resolveSplitMileageRate({transaction: originalTransaction, policy, isSelfDMSplit, personalPolicyOutputCurrency}); - const splitRateID = splitExpense.customUnit?.customUnitRateID ?? String(CONST.DEFAULT_NUMBER_ID); - - // `policy` is undefined for a self-DM split on the personal rate, so also resolve the split's - // picked rate across all policies, so the selection isn't lost. - const splitSelectedRate = - DistanceRequestUtils.getRateByCustomUnitRateID({policy, customUnitRateID: splitRateID}) ?? - DistanceRequestUtils.getEnabledRateByCustomUnitRateIDFromAnyPolicy(splitRateID, policies); - const isSplitP2PRate = splitRateID === CONST.CUSTOM_UNITS.FAKE_P2P_ID; - - // Prefer the split's own selected rate when it's a real enabled rate; otherwise fall back to - // the original-transaction rate (covers the P2P and deleted-rate cases). - const useSplitSelectedRate = !isSplitP2PRate && !!splitSelectedRate?.rate && splitSelectedRate.rate > 0 && splitSelectedRate.enabled !== false; - const rate = useSplitSelectedRate ? (splitSelectedRate?.rate ?? 0) : mileageRate.rate; - - // The unit comes from the split's own `distanceUnit`, kept in sync by `getUpdatedTransaction` on every - // rate change, so the merchant matches the Distance field instead of the policy's current unit. - const unit = splitExpense.customUnit?.distanceUnit ?? (useSplitSelectedRate ? (splitSelectedRate?.unit ?? mileageRate.unit) : mileageRate.unit); + const {unit, rate} = resolveSplitItemRate({customUnit: splitExpense.customUnit, fallbackMileageRate: mileageRate, policy, policies}); if (rate && rate > 0) { const {customUnit: updatedCustomUnit, merchant} = updateSplitExpenseDistanceFromAmount( diff --git a/src/pages/iou/SplitExpenseCreateDateRagePage.tsx b/src/pages/iou/SplitExpenseCreateDateRagePage.tsx index f46da0aae4f4..90aa1496e514 100644 --- a/src/pages/iou/SplitExpenseCreateDateRagePage.tsx +++ b/src/pages/iou/SplitExpenseCreateDateRagePage.tsx @@ -47,6 +47,7 @@ function SplitExpenseCreateDateRagePage({route}: SplitExpenseCreateDateRagePageP const [draftTransaction] = useOnyx(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${transactionID}`); const allTransactions = useAllTransactions(); const [allReports] = useOnyx(ONYXKEYS.COLLECTION.REPORT); + const [allPolicies] = useOnyx(ONYXKEYS.COLLECTION.POLICY); const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${getNonEmptyStringOnyxID(transactionID)}`]; const originalTransaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${getNonEmptyStringOnyxID(transaction?.comment?.originalTransactionID)}`]; @@ -70,6 +71,7 @@ function SplitExpenseCreateDateRagePage({route}: SplitExpenseCreateDateRagePageP effectivePolicy, isSelfDM(currentReport) || isSelfDM(parentReport), personalPolicy?.outputCurrency, + allPolicies, ); Navigation.goBack(backTo); }; diff --git a/src/pages/iou/SplitExpenseEditPage.tsx b/src/pages/iou/SplitExpenseEditPage.tsx index d6ea36caf1a9..6b367a7d35b9 100644 --- a/src/pages/iou/SplitExpenseEditPage.tsx +++ b/src/pages/iou/SplitExpenseEditPage.tsx @@ -479,6 +479,7 @@ function SplitExpenseEditPage({route}: SplitExpensePageProps) { effectivePolicy, isSelfDMSplit, personalPolicy?.outputCurrency, + allPolicies, ); Navigation.goBack(backTo); }} diff --git a/src/pages/iou/SplitExpensePage.tsx b/src/pages/iou/SplitExpensePage.tsx index 9bbd04a3196f..f9f4971bc91b 100644 --- a/src/pages/iou/SplitExpensePage.tsx +++ b/src/pages/iou/SplitExpensePage.tsx @@ -294,14 +294,14 @@ function SplitExpensePage({route}: SplitExpensePageProps) { if (draftTransaction?.errors) { clearSplitTransactionDraftErrors(transactionID); } - addSplitExpenseField(transaction, draftTransaction, transactionReport, effectivePolicy, isDraftSelfDMContext, personalPolicy?.outputCurrency); + addSplitExpenseField(transaction, draftTransaction, transactionReport, effectivePolicy, isDraftSelfDMContext, personalPolicy?.outputCurrency, allPolicies); }; const onMakeSplitsEven = () => { if (!draftTransaction) { return; } - evenlyDistributeSplitExpenseAmounts(draftTransaction, transaction, effectivePolicy, isDraftSelfDMContext, personalPolicy?.outputCurrency); + evenlyDistributeSplitExpenseAmounts(draftTransaction, transaction, effectivePolicy, isDraftSelfDMContext, personalPolicy?.outputCurrency, allPolicies); }; const [allPolicyTags] = useOnyx(ONYXKEYS.COLLECTION.POLICY_TAGS, {selector: passthroughPolicyTagListSelector}); diff --git a/tests/actions/IOUTest/SplitTest.ts b/tests/actions/IOUTest/SplitTest.ts index 775ce793a081..cb1110ed1b11 100644 --- a/tests/actions/IOUTest/SplitTest.ts +++ b/tests/actions/IOUTest/SplitTest.ts @@ -7418,6 +7418,78 @@ describe('addSplitExpenseField', () => { expect(splitExpenses?.[1].merchant).toBeDefined(); }); + it('should build a new split on the unit the expense is stored with', async () => { + // Given a distance expense split in miles, whose workspace unit was later switched to kilometers + const customUnitRateID = 'rate-unit-switch'; + const customUnitID = 'distance-unit'; + const policy: Policy = { + ...createRandomPolicy(4), + customUnits: { + [customUnitID]: { + customUnitID, + name: CONST.CUSTOM_UNITS.NAME_DISTANCE, + enabled: true, + attributes: {unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_KILOMETERS}, + rates: { + [customUnitRateID]: {customUnitRateID, currency: CONST.CURRENCY.USD, rate: 100, enabled: true, name: 'Default Rate', subRates: []}, + }, + }, + }, + }; + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`, policy); + await waitForBatchedUpdates(); + + const customUnit: TransactionCustomUnit = { + name: CONST.CUSTOM_UNITS.NAME_DISTANCE, + customUnitID, + customUnitRateID, + distanceUnit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES, + quantity: 200, + }; + const transaction: Transaction = { + transactionID: 'unit-switch-original', + amount: -20000, + currency: 'USD', + merchant: '', + iouRequestType: CONST.IOU.REQUEST_TYPE.DISTANCE_MAP, + comment: {comment: '', splitExpenses: [], attendees: [], type: CONST.TRANSACTION.TYPE.CUSTOM_UNIT, customUnit}, + created: DateUtils.getDBTime(), + reportID: '456', + }; + const draftTransaction: Transaction = { + ...transaction, + amount: 20000, + comment: { + comment: '', + splitExpenses: [ + { + transactionID: 'unit-switch-split', + amount: 10000, + description: '', + category: '', + tags: [], + created: DateUtils.getDBTime(), + customUnit: {...customUnit, quantity: 100}, + }, + ], + attendees: [], + type: CONST.TRANSACTION.TYPE.CUSTOM_UNIT, + }, + }; + const transactionReport: Report = {reportID: '456', type: CONST.REPORT.TYPE.EXPENSE, total: 20000, currency: 'USD'}; + + // When another split is added + addSplitExpenseField(transaction, draftTransaction, transactionReport, policy, false, undefined); + await waitForBatchedUpdates(); + + // Then it is stored on that same unit, so the merchant and the Distance field agree + const updatedDraftTransaction = await getOnyxValue(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${transaction.transactionID}`); + const splitExpenses = updatedDraftTransaction?.comment?.splitExpenses; + expect(splitExpenses).toHaveLength(2); + expect(splitExpenses?.[1].customUnit?.distanceUnit).toBe(CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES); + expect(splitExpenses?.[1].merchant).toContain(CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES); + }); + it('should use EUR P2P mileage rate and EUR currency in merchant when personalPolicyOutputCurrency is EUR', async () => { // For a P2P distance transaction in EUR: personalPolicyOutputCurrency selects the EUR P2P rate (30¢/mi). // transaction.currency drives the merchant currency symbol, so both must be EUR for €0.30/mi to appear. @@ -7524,7 +7596,7 @@ describe('addSplitExpenseField', () => { }); describe('evenlyDistributeSplitExpenseAmounts', () => { - it('distributes evenly across 3 splits with remainder on last split', async () => { + it('distributes evenly across 3 splits with remainder on the first split', async () => { const originalTransactionID = 'orig-last'; const draftTransaction: Transaction = { transactionID: 'draft-2', @@ -7552,7 +7624,7 @@ describe('evenlyDistributeSplitExpenseAmounts', () => { const updatedDraft = await getOnyxValue(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${originalTransactionID}`); expect(updatedDraft).toBeTruthy(); const amounts = (updatedDraft?.comment?.splitExpenses ?? []).map((x) => x.amount); - expect(amounts).toEqual([33, 33, 34]); + expect(amounts).toEqual([34, 33, 33]); }); it('assigns full amount when there is only one split', async () => { @@ -7641,7 +7713,7 @@ describe('evenlyDistributeSplitExpenseAmounts', () => { expect(amounts).toEqual([50, 50]); }); - it('2-way split with remainder (odd cents) -> 50¢ / 51¢', async () => { + it('2-way split with remainder (odd cents) -> 51¢ / 50¢', async () => { const originalTransactionID = 'orig-2-rem'; const draftTransaction: Transaction = { transactionID: 'draft-6', @@ -7667,10 +7739,10 @@ describe('evenlyDistributeSplitExpenseAmounts', () => { const updatedDraft = await getOnyxValue(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${originalTransactionID}`); const amounts = (updatedDraft?.comment?.splitExpenses ?? []).map((x) => x.amount); - expect(amounts).toEqual([50, 51]); + expect(amounts).toEqual([51, 50]); }); - it('3-way split of $1001 with remainder -> [$333.66, $333.66, $333.68]', async () => { + it('3-way split of $1001 with remainder -> [$333.68, $333.66, $333.66]', async () => { const originalTransactionID = 'orig-1001-3-last'; const draftTransaction: Transaction = { transactionID: 'draft-7', @@ -7697,11 +7769,11 @@ describe('evenlyDistributeSplitExpenseAmounts', () => { const updatedDraft = await getOnyxValue(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${originalTransactionID}`); const amounts = (updatedDraft?.comment?.splitExpenses ?? []).map((x) => x.amount); - expect(amounts).toEqual([33366, 33366, 33368]); + expect(amounts).toEqual([33368, 33366, 33366]); expect(amounts.reduce((a, b) => a + b, 0)).toBe(100100); }); - it('preserves negative sign and evenly distributes with remainder on last for 3-way split', async () => { + it('preserves negative sign and evenly distributes with remainder on the first split for a 3-way split', async () => { const originalTransactionID = 'orig-neg-3'; const draftTransaction: Transaction = { transactionID: 'draft-neg-3', @@ -7728,7 +7800,7 @@ describe('evenlyDistributeSplitExpenseAmounts', () => { const updatedDraft = await getOnyxValue(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${originalTransactionID}`); const amounts = (updatedDraft?.comment?.splitExpenses ?? []).map((x) => x.amount); - expect(amounts).toEqual([-33, -33, -34]); + expect(amounts).toEqual([-34, -33, -33]); expect(amounts.reduce((a, b) => a + b, 0)).toBe(-100); }); @@ -7758,7 +7830,7 @@ describe('evenlyDistributeSplitExpenseAmounts', () => { const updatedDraft = await getOnyxValue(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${originalTransactionID}`); const amounts = (updatedDraft?.comment?.splitExpenses ?? []).map((x) => x.amount); - expect(amounts).toEqual([-50, -51]); + expect(amounts).toEqual([-51, -50]); expect(amounts.reduce((a, b) => a + b, 0)).toBe(-101); }); @@ -7870,6 +7942,106 @@ describe('evenlyDistributeSplitExpenseAmounts', () => { expect(splitExpenses.at(1)?.merchant).toBeTruthy(); expect(splitExpenses.at(1)?.merchant).toContain('100'); }); + + it('should distribute the splits on the rate each one is calculated with', async () => { + // Given an expense stored in miles whose splits were switched to a rate that another workspace keeps in kilometers + const originalTransactionID = 'even-selected-rate-original'; + const expenseCustomUnitID = 'even-selected-rate-expense-unit'; + const expenseRateID = 'even-selected-rate-expense-rate'; + const selectedCustomUnitID = 'even-selected-rate-selected-unit'; + const selectedRateID = 'even-selected-rate-selected-rate'; + const expensePolicy: Policy = { + ...createRandomPolicy(11), + customUnits: { + [expenseCustomUnitID]: { + customUnitID: expenseCustomUnitID, + name: CONST.CUSTOM_UNITS.NAME_DISTANCE, + enabled: true, + attributes: {unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES}, + rates: { + [expenseRateID]: {customUnitRateID: expenseRateID, currency: CONST.CURRENCY.USD, rate: 100, enabled: true, name: 'Expense Rate', subRates: []}, + }, + }, + }, + }; + const selectedRatePolicy: Policy = { + ...createRandomPolicy(12), + customUnits: { + [selectedCustomUnitID]: { + customUnitID: selectedCustomUnitID, + name: CONST.CUSTOM_UNITS.NAME_DISTANCE, + enabled: true, + attributes: {unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_KILOMETERS}, + rates: { + [selectedRateID]: {customUnitRateID: selectedRateID, currency: CONST.CURRENCY.USD, rate: 50, enabled: true, name: 'Selected Rate', subRates: []}, + }, + }, + }, + }; + + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${expensePolicy.id}`, expensePolicy); + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${selectedRatePolicy.id}`, selectedRatePolicy); + await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${originalTransactionID}`, { + transactionID: originalTransactionID, + amount: -20000, + currency: 'USD', + iouRequestType: CONST.IOU.REQUEST_TYPE.DISTANCE_MAP, + comment: { + type: CONST.TRANSACTION.TYPE.CUSTOM_UNIT, + customUnit: { + name: CONST.CUSTOM_UNITS.NAME_DISTANCE, + customUnitID: expenseCustomUnitID, + customUnitRateID: expenseRateID, + distanceUnit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES, + quantity: 200, + }, + }, + }); + await waitForBatchedUpdates(); + + const splitCustomUnit = { + name: CONST.CUSTOM_UNITS.NAME_DISTANCE, + customUnitID: selectedCustomUnitID, + customUnitRateID: selectedRateID, + distanceUnit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_KILOMETERS, + quantity: 200, + }; + const draftTransaction: Transaction = { + transactionID: 'even-selected-rate-draft', + amount: 20000, + currency: 'USD', + merchant: 'Test Merchant', + iouRequestType: CONST.IOU.REQUEST_TYPE.DISTANCE_MAP, + comment: { + comment: '', + originalTransactionID, + splitExpenses: [ + {transactionID: 'even-selected-rate-first', amount: 15000, description: '', category: '', tags: [], created: DateUtils.getDBTime(), customUnit: splitCustomUnit}, + {transactionID: 'even-selected-rate-second', amount: 5000, description: '', category: '', tags: [], created: DateUtils.getDBTime(), customUnit: splitCustomUnit}, + ], + attendees: [], + type: CONST.TRANSACTION.TYPE.CUSTOM_UNIT, + }, + created: DateUtils.getDBTime(), + reportID: 'even-selected-rate-report', + }; + + // When the splits are distributed evenly + const originalTransaction = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${originalTransactionID}`); + evenlyDistributeSplitExpenseAmounts(draftTransaction, originalTransaction, expensePolicy, false, undefined, { + [`${ONYXKEYS.COLLECTION.POLICY}${selectedRatePolicy.id}`]: selectedRatePolicy, + }); + await waitForBatchedUpdates(); + + // Then every split is measured with the rate it carries, on the unit it is stored with + const updatedDraft = await getOnyxValue(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${originalTransactionID}`); + const splitExpenses = updatedDraft?.comment?.splitExpenses ?? []; + expect(splitExpenses.at(0)?.customUnit?.quantity).toBe(200); + expect(splitExpenses.at(0)?.customUnit?.distanceUnit).toBe(CONST.CUSTOM_UNITS.DISTANCE_UNIT_KILOMETERS); + expect(splitExpenses.at(0)?.merchant).toContain(`200.00 ${CONST.CUSTOM_UNITS.DISTANCE_UNIT_KILOMETERS}`); + expect(splitExpenses.at(1)?.customUnit?.quantity).toBe(200); + expect(splitExpenses.at(1)?.customUnit?.distanceUnit).toBe(CONST.CUSTOM_UNITS.DISTANCE_UNIT_KILOMETERS); + }); }); describe('updateSplitExpenseAmountField', () => { @@ -8008,7 +8180,7 @@ describe('updateSplitExpenseAmountField', () => { expect(splitExpenses?.[0].merchant).toContain('150'); }); - it('should keep the unit stored on the split when the workspace distance unit changed after the split was created', async () => { + it('should keep the unit the expense is stored with when the workspace distance unit changed', async () => { // Given a distance split created in miles, whose workspace unit was later switched to kilometers const customUnitRateID = 'rate-unit-change'; const customUnitID = 'distance-unit'; @@ -8095,7 +8267,8 @@ describe('updateSplitExpenseAmountField', () => { updateSplitExpenseAmountField(draftTransaction, currentTransactionID, 15000, policy, false, undefined); await waitForBatchedUpdates(); - // Then the merchant is rebuilt with the unit stored on the split, which is the unit the Distance field renders + // Then the split stays on the unit the expense is stored with, and the distance it stores is expressed in that + // same unit, so the merchant and the Distance field agree const updatedDraftTransaction = await getOnyxValue(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${originalTransactionID}`); const splitExpenses = updatedDraftTransaction?.comment?.splitExpenses; expect(splitExpenses?.[0].customUnit?.distanceUnit).toBe(CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES); From 5b7076242902e96fae28d709e98cc0861913c39c Mon Sep 17 00:00:00 2001 From: yauhenihorbach Date: Mon, 3 Aug 2026 18:01:01 +0200 Subject: [PATCH 5/5] Pass distanceUnit to BE --- src/libs/API/parameters/SplitTransactionParams.ts | 2 ++ src/libs/actions/IOU/SplitTransactionUpdate.ts | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/libs/API/parameters/SplitTransactionParams.ts b/src/libs/API/parameters/SplitTransactionParams.ts index e7e9c45870dd..3ea3e266873c 100644 --- a/src/libs/API/parameters/SplitTransactionParams.ts +++ b/src/libs/API/parameters/SplitTransactionParams.ts @@ -1,3 +1,4 @@ +import type {Unit} from '@src/types/onyx/Policy'; import type {Comment, WaypointCollection} from '@src/types/onyx/Transaction'; type SplitTransactionSplitParam = { @@ -17,6 +18,7 @@ type SplitTransactionSplitParam = { reportID?: string; quantity?: number; customUnitRateID?: string; + distanceUnit?: Unit; odometerStart?: number; odometerEnd?: number; waypoints?: WaypointCollection; diff --git a/src/libs/actions/IOU/SplitTransactionUpdate.ts b/src/libs/actions/IOU/SplitTransactionUpdate.ts index b63a1160807f..23bd6ceff9cc 100644 --- a/src/libs/actions/IOU/SplitTransactionUpdate.ts +++ b/src/libs/actions/IOU/SplitTransactionUpdate.ts @@ -345,6 +345,7 @@ function updateSplitTransactions({ billable: split?.billable, quantity: split.customUnit?.quantity ?? undefined, customUnitRateID: split.customUnit?.customUnitRateID, + distanceUnit: split.customUnit?.distanceUnit, odometerStart: split.odometerStart, odometerEnd: split.odometerEnd, waypoints: split.waypoints, @@ -802,6 +803,7 @@ function updateSplitTransactions({ const oldTransactionChanges = { ...existing, quantity: splitTransaction.comment?.customUnit?.quantity ?? existing?.distance, + distanceUnit: splitTransaction.comment?.customUnit?.distanceUnit, } as TransactionChanges; if (currentSplit) {