Skip to content

Commit f82a8db

Browse files
authored
Merge pull request #73934 from dominictb/feat/70373
[Merge] Merge split expenses
2 parents 964f725 + 0fe6719 commit f82a8db

8 files changed

Lines changed: 249 additions & 37 deletions

File tree

src/libs/MergeTransactionUtils.ts

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {getIOUActionForReportID} from './ReportActionsUtils';
1616
import {findSelfDMReportID, getReportName, getReportOrDraftReport, getTransactionDetails} from './ReportUtils';
1717
import type {TransactionDetails} from './ReportUtils';
1818
import StringUtils from './StringUtils';
19-
import {getAttendeesListDisplayString, getCurrency, getReimbursable, getWaypoints, isDistanceRequest, isManagedCardTransaction, isMerchantMissing} from './TransactionUtils';
19+
import {getAttendeesListDisplayString, getCurrency, getReimbursable, getWaypoints, isDistanceRequest, isExpenseSplit, isManagedCardTransaction, isMerchantMissing} from './TransactionUtils';
2020

2121
const RECEIPT_SOURCE_URL = 'https://www.expensify.com/receipts/';
2222

@@ -190,9 +190,16 @@ function getMergeFields(targetTransaction: OnyxEntry<Transaction>) {
190190
* Get mergeableData data if one is missing, and conflict fields that need to be resolved by the user
191191
* @param targetTransaction - The target transaction
192192
* @param sourceTransaction - The source transaction
193+
* @param originalTargetTransaction - The original transaction of target transaction
194+
* @param localeCompare - The localize compare function
193195
* @returns mergeableData and conflictFields
194196
*/
195-
function getMergeableDataAndConflictFields(targetTransaction: OnyxEntry<Transaction>, sourceTransaction: OnyxEntry<Transaction>, localeCompare: (a: string, b: string) => number) {
197+
function getMergeableDataAndConflictFields(
198+
targetTransaction: OnyxEntry<Transaction>,
199+
sourceTransaction: OnyxEntry<Transaction>,
200+
originalTargetTransaction: OnyxEntry<Transaction>,
201+
localeCompare: (a: string, b: string) => number,
202+
) {
196203
const conflictFields: string[] = [];
197204
const mergeableData: Record<string, unknown> = {};
198205

@@ -207,11 +214,16 @@ function getMergeableDataAndConflictFields(targetTransaction: OnyxEntry<Transact
207214
const isSourceValueEmpty = isEmptyMergeValue(sourceValue);
208215

209216
if (field === 'amount') {
210-
// If target transaction is a card transaction, always preserve the target transaction's amount and currency
217+
// If target transaction is a card or split expense, always preserve the target transaction's amount and currency
218+
// Card takes precedence over split expense
211219
// See https://github.com/Expensify/App/issues/68189#issuecomment-3167156907
212-
if (isManagedCardTransaction(targetTransaction)) {
220+
const isTargetExpenseSplit = isExpenseSplit(targetTransaction, originalTargetTransaction);
221+
if (isManagedCardTransaction(targetTransaction) || isTargetExpenseSplit) {
213222
mergeableData[field] = targetValue;
214223
mergeableData.currency = getCurrency(targetTransaction);
224+
if (isTargetExpenseSplit) {
225+
mergeableData.originalTransactionID = targetTransaction?.comment?.originalTransactionID;
226+
}
215227
continue;
216228
}
217229

@@ -346,24 +358,27 @@ function buildMergedTransactionData(targetTransaction: OnyxEntry<Transaction>, m
346358
}
347359

348360
/**
349-
* Determines the correct target and source transaction IDs for merging based on transaction types.
361+
* Determines the correct target and source transactions for merging based on transaction types.
350362
*
351363
* Rules:
352-
* - If one transaction is a card transaction, it becomes the target (card transactions take priority)
353-
* - If both are cash transactions, the first parameter becomes the target
354-
* - Users can only merge two cash expenses or one cash/one card expense
364+
* - The target transaction (transaction to keep) is selected based on the following priority: card transaction > split expense > cash transaction
355365
* - Users cannot merge two card expenses
366+
* - Users cannot merge two split expenses
367+
* - Users can merge any other combinations
356368
*
357-
* @param targetTransaction - The first transaction in the merge operation
358-
* @param sourceTransaction - The second transaction in the merge operation
359-
* @returns An object containing the determined targetTransactionID and sourceTransactionID
369+
* @param targetTransaction - The transaction where the merge action is started from
370+
* @param sourceTransaction - The selected transaction to be merged with the target transaction
371+
* @param originalSourceTransaction - The original transaction of the source transaction
372+
* @returns An object containing the determined targetTransaction and sourceTransaction
360373
*/
361-
function selectTargetAndSourceTransactionsForMerge(originalTargetTransaction: OnyxEntry<Transaction>, originalSourceTransaction: OnyxEntry<Transaction>) {
362-
if (isManagedCardTransaction(originalSourceTransaction)) {
363-
return {targetTransaction: originalSourceTransaction, sourceTransaction: originalTargetTransaction};
374+
function selectTargetAndSourceTransactionsForMerge(targetTransaction: OnyxEntry<Transaction>, sourceTransaction: OnyxEntry<Transaction>, originalSourceTransaction?: OnyxEntry<Transaction>) {
375+
// If target transaction is a card or split expense, always preserve the target transaction
376+
// Card takes precedence over split expense
377+
if (isManagedCardTransaction(sourceTransaction) || (isExpenseSplit(sourceTransaction, originalSourceTransaction) && !isManagedCardTransaction(targetTransaction))) {
378+
return {targetTransaction: sourceTransaction, sourceTransaction: targetTransaction};
364379
}
365380

366-
return {targetTransaction: originalTargetTransaction, sourceTransaction: originalSourceTransaction};
381+
return {targetTransaction, sourceTransaction};
367382
}
368383

369384
/**

src/libs/actions/MergeTransaction.ts

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,15 @@ import {
1818
isReportManager,
1919
} from '@libs/ReportUtils';
2020
import CONST from '@src/CONST';
21-
import {getAmount, getTransactionViolationsOfTransaction, isDistanceRequest, isManagedCardTransaction, isPerDiemRequest, isTransactionPendingDelete} from '@src/libs/TransactionUtils';
21+
import {
22+
getAmount,
23+
getTransactionViolationsOfTransaction,
24+
isDistanceRequest,
25+
isExpenseSplit,
26+
isManagedCardTransaction,
27+
isPerDiemRequest,
28+
isTransactionPendingDelete,
29+
} from '@src/libs/TransactionUtils';
2230
import ONYXKEYS from '@src/ONYXKEYS';
2331
import type {MergeTransaction, Policy, PolicyCategories, PolicyTagLists, Report, Transaction} from '@src/types/onyx';
2432
import {getUpdateMoneyRequestParams, getUpdateTrackExpenseParams} from './IOU';
@@ -49,12 +57,17 @@ function getTransactionsForMergingFromAPI(transactionID: string) {
4957
API.read(READ_COMMANDS.GET_TRANSACTIONS_FOR_MERGING, parameters);
5058
}
5159

52-
function areTransactionsEligibleForMerge(transaction1: Transaction, transaction2: Transaction) {
60+
function areTransactionsEligibleForMerge(transaction1: Transaction, transaction2: Transaction, originalTransaction1?: Transaction, originalTransaction2?: Transaction) {
5361
// Do not allow merging two card transactions
5462
if (isManagedCardTransaction(transaction1) && isManagedCardTransaction(transaction2)) {
5563
return false;
5664
}
5765

66+
// Do not allow merging two split expenses
67+
if (isExpenseSplit(transaction1, originalTransaction1) && isExpenseSplit(transaction2, originalTransaction2)) {
68+
return false;
69+
}
70+
5871
// Do not allow merging two $0 transactions
5972
if (getAmount(transaction1, false, false) === 0 && getAmount(transaction2, false, false) === 0) {
6073
return false;
@@ -84,15 +97,18 @@ function areTransactionsEligibleForMerge(transaction1: Transaction, transaction2
8497
*/
8598
function getTransactionsForMergingLocally(transactionID: string, targetTransaction: Transaction, transactions: OnyxCollection<Transaction>) {
8699
const transactionsArray = Object.values(transactions ?? {});
100+
const targetOriginalTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${targetTransaction?.comment?.originalTransactionID}`];
87101

88102
const eligibleTransactions = transactionsArray.filter((transaction): transaction is Transaction => {
89103
if (!transaction || transaction.transactionID === targetTransaction.transactionID) {
90104
return false;
91105
}
92106

107+
const originalTransactionID = transaction.comment?.originalTransactionID;
108+
const originalTransaction = originalTransactionID ? transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${originalTransactionID}`] : undefined;
93109
const isUnreportedExpense = !transaction?.reportID || transaction?.reportID === CONST.REPORT.UNREPORTED_REPORT_ID;
94110
return (
95-
areTransactionsEligibleForMerge(targetTransaction, transaction) &&
111+
areTransactionsEligibleForMerge(targetTransaction, transaction, targetOriginalTransaction, originalTransaction) &&
96112
!isTransactionPendingDelete(transaction) &&
97113
(isUnreportedExpense || (!!transaction.reportID && isMoneyRequestReportEligibleForMerge(transaction.reportID, false)))
98114
);
@@ -131,12 +147,15 @@ function getTransactionsForMerging({
131147

132148
if (isPaidGroupPolicy(policy) && (isAdmin || isManager) && !isCurrentUserSubmitter(report)) {
133149
const reportTransactions = getReportTransactions(report?.reportID);
150+
const targetOriginalTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${targetTransaction?.comment?.originalTransactionID}`];
134151
const eligibleTransactions = reportTransactions.filter((transaction): transaction is Transaction => {
135152
if (!transaction || transaction.transactionID === transactionID) {
136153
return false;
137154
}
138155

139-
return areTransactionsEligibleForMerge(targetTransaction, transaction);
156+
const originalTransactionID = transaction.comment?.originalTransactionID;
157+
const originalTransaction = originalTransactionID ? transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${originalTransactionID}`] : undefined;
158+
return areTransactionsEligibleForMerge(targetTransaction, transaction, targetOriginalTransaction, originalTransaction);
140159
});
141160

142161
Onyx.merge(`${ONYXKEYS.COLLECTION.MERGE_TRANSACTION}${transactionID}`, {
@@ -277,6 +296,7 @@ function mergeTransactionRequest({
277296
customUnit: mergeTransaction.customUnit,
278297
waypoints: mergeTransaction.waypoints ?? null,
279298
attendees: mergeTransaction.attendees,
299+
originalTransactionID: mergeTransaction.originalTransactionID,
280300
}),
281301
billable: mergeTransaction.billable,
282302
reimbursable: mergeTransaction.reimbursable,
@@ -428,4 +448,4 @@ function mergeTransactionRequest({
428448
API.write(WRITE_COMMANDS.MERGE_TRANSACTION, params, {optimisticData, failureData, successData});
429449
}
430450

431-
export {setupMergeTransactionData, setMergeTransactionKey, getTransactionsForMerging, mergeTransactionRequest, areTransactionsEligibleForMerge};
451+
export {areTransactionsEligibleForMerge, setupMergeTransactionData, setMergeTransactionKey, getTransactionsForMerging, mergeTransactionRequest};

src/pages/TransactionMerge/DetailsReviewPage.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,9 @@ function DetailsReviewPage({route}: DetailsReviewPageProps) {
8282
const [sourceTransaction = getSourceTransactionFromMergeTransaction(mergeTransaction)] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${mergeTransaction?.sourceTransactionID}`, {
8383
canBeMissing: true,
8484
});
85+
const [originalTargetTransaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${targetTransaction?.comment?.originalTransactionID}`, {canBeMissing: true});
8586
const [targetTransactionThreadReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionThreadReportID}`, {canBeMissing: true});
8687
const [currentUserEmail] = useOnyx(ONYXKEYS.SESSION, {selector: emailSelector, canBeMissing: false});
87-
8888
const [hasErrors, setHasErrors] = useState<Partial<Record<MergeFieldKey, boolean>>>({});
8989
const [conflictFields, setConflictFields] = useState<MergeFieldKey[]>([]);
9090
const [isCheckingDataBeforeGoNext, setIsCheckingDataBeforeGoNext] = useState<boolean>(false);
@@ -94,11 +94,11 @@ function DetailsReviewPage({route}: DetailsReviewPageProps) {
9494
return;
9595
}
9696

97-
const {conflictFields: detectedConflictFields, mergeableData} = getMergeableDataAndConflictFields(targetTransaction, sourceTransaction, localeCompare);
97+
const {conflictFields: detectedConflictFields, mergeableData} = getMergeableDataAndConflictFields(targetTransaction, sourceTransaction, originalTargetTransaction, localeCompare);
9898

9999
setMergeTransactionKey(transactionID, mergeableData);
100100
setConflictFields(detectedConflictFields as MergeFieldKey[]);
101-
}, [targetTransaction, sourceTransaction, transactionID, localeCompare]);
101+
}, [targetTransaction, sourceTransaction, originalTargetTransaction, transactionID, localeCompare]);
102102

103103
useEffect(() => {
104104
if (!isCheckingDataBeforeGoNext) {

src/pages/TransactionMerge/MergeTransactionsListContent.tsx

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,13 @@ function MergeTransactionsListContent({transactionID, mergeTransaction}: MergeTr
4848
const [transactions] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION, {canBeMissing: false});
4949
const {isOffline} = useNetwork();
5050
const [targetTransaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, {canBeMissing: false});
51+
const [originalTargetTransaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${targetTransaction?.comment?.originalTransactionID}`, {canBeMissing: true});
5152
const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${targetTransaction?.reportID}`, {canBeMissing: true});
5253
const [transactionThreadReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getTransactionThreadReportID(targetTransaction)}`, {canBeMissing: true});
5354
const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`, {canBeMissing: true});
5455
const eligibleTransactions = mergeTransaction?.eligibleTransactions;
56+
const sourceTransaction = getSourceTransactionFromMergeTransaction(mergeTransaction);
57+
const [originalSourceTransaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${sourceTransaction?.comment?.originalTransactionID}`, {canBeMissing: true});
5558
const currentUserLogin = session?.email;
5659

5760
useEffect(() => {
@@ -109,13 +112,15 @@ function MergeTransactionsListContent({transactionID, mergeTransaction}: MergeTr
109112
}, [translate, styles.renderHTML, styles.textNormal]);
110113

111114
const handleConfirm = useCallback(() => {
112-
const sourceTransaction = getSourceTransactionFromMergeTransaction(mergeTransaction);
113-
114115
if (!sourceTransaction || !targetTransaction) {
115116
return;
116117
}
117118

118-
const {targetTransaction: newTargetTransaction, sourceTransaction: newSourceTransaction} = selectTargetAndSourceTransactionsForMerge(targetTransaction, sourceTransaction);
119+
const {targetTransaction: newTargetTransaction, sourceTransaction: newSourceTransaction} = selectTargetAndSourceTransactionsForMerge(
120+
targetTransaction,
121+
sourceTransaction,
122+
originalSourceTransaction,
123+
);
119124
if (shouldNavigateToReceiptReview([newTargetTransaction, newSourceTransaction])) {
120125
setMergeTransactionKey(transactionID, {
121126
targetTransactionID: newTargetTransaction?.transactionID,
@@ -130,7 +135,7 @@ function MergeTransactionsListContent({transactionID, mergeTransaction}: MergeTr
130135
receipt: mergedReceipt,
131136
});
132137

133-
const {conflictFields, mergeableData} = getMergeableDataAndConflictFields(newTargetTransaction, newSourceTransaction, localeCompare);
138+
const {conflictFields, mergeableData} = getMergeableDataAndConflictFields(newTargetTransaction, newSourceTransaction, originalTargetTransaction, localeCompare);
134139
if (!conflictFields.length) {
135140
// If there are no conflict fields, we should set mergeable data and navigate to the confirmation page
136141
setMergeTransactionKey(transactionID, mergeableData);
@@ -139,7 +144,7 @@ function MergeTransactionsListContent({transactionID, mergeTransaction}: MergeTr
139144
}
140145
Navigation.navigate(ROUTES.MERGE_TRANSACTION_DETAILS_PAGE.getRoute(transactionID, Navigation.getActiveRoute()));
141146
}
142-
}, [mergeTransaction, transactionID, targetTransaction, localeCompare]);
147+
}, [transactionID, targetTransaction, sourceTransaction, originalSourceTransaction, originalTargetTransaction, localeCompare]);
143148

144149
if (eligibleTransactions?.length === 0) {
145150
return (

src/pages/TransactionMerge/ReceiptReviewPage.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ function ReceiptReviewPage({route}: ReceiptReviewPageProps) {
3838
const [sourceTransaction = getSourceTransactionFromMergeTransaction(mergeTransaction)] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${mergeTransaction?.sourceTransactionID}`, {
3939
canBeMissing: true,
4040
});
41-
41+
const [originalTargetTransaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${targetTransaction?.comment?.originalTransactionID}`, {canBeMissing: true});
4242
const transactions = [targetTransaction, sourceTransaction].filter((transaction): transaction is Transaction => !!transaction);
4343

4444
const handleSelect = (receipt: Receipt | undefined) => {
@@ -50,7 +50,7 @@ function ReceiptReviewPage({route}: ReceiptReviewPageProps) {
5050
return;
5151
}
5252

53-
const {conflictFields, mergeableData} = getMergeableDataAndConflictFields(targetTransaction, sourceTransaction, localeCompare);
53+
const {conflictFields, mergeableData} = getMergeableDataAndConflictFields(targetTransaction, sourceTransaction, originalTargetTransaction, localeCompare);
5454
if (!conflictFields.length) {
5555
// If there are no conflict fields, we should set mergeable data and navigate to the confirmation page
5656
setMergeTransactionKey(transactionID, mergeableData);

src/types/onyx/MergeTransaction.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ type MergeTransaction = {
7070

7171
/** The attendees of the transaction */
7272
attendees?: Attendee[];
73+
74+
/** ID of the original transaction */
75+
originalTransactionID?: string;
7376
};
7477

7578
export default MergeTransaction;

0 commit comments

Comments
 (0)