-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Feat/dupe detection confirmation #42571
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
faf6155
d0e0f45
07f8709
9f66c61
b361da2
b9d088b
d44f993
6cbbcc9
e1f30d0
449aae5
573c1f0
56eeff7
75df93d
9bb346b
4821e0a
4c20950
419a1ee
9b44dec
60541d8
c7755e4
86f1a18
1b7bedb
d4214a2
47f18be
6e2ca1c
47af699
56ce616
6fa1ed8
afdc17e
4f72959
e85116e
3542f53
90299bd
d775ab0
a060f37
1a5337f
79cf046
a79f424
306a44c
0a59279
957f1ae
e3d7dd5
9c0518c
66ee120
7cb3314
6a0b99e
95f946a
38fb56b
df00611
944ea4b
4873418
a352bfb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| type TransactionMergeParams = { | ||
| transactionID: string; | ||
| transactionIDList: string[]; | ||
| created: string; | ||
| merchant: string; | ||
| amount: number; | ||
| currency: string; | ||
| category: string; | ||
| comment: string; | ||
| billable: boolean; | ||
| reimbursable: boolean; | ||
| tag: string; | ||
| receiptID: number; | ||
| reportID: string; | ||
| }; | ||
|
|
||
| export default TransactionMergeParams; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ import type {Beta, OnyxInputOrEntry, Policy, RecentWaypoint, ReviewDuplicates, T | |
| import type {Comment, Receipt, TransactionChanges, TransactionPendingFieldsKey, Waypoint, WaypointCollection} from '@src/types/onyx/Transaction'; | ||
| import {isEmptyObject} from '@src/types/utils/EmptyObject'; | ||
| import type {IOURequestType} from './actions/IOU'; | ||
| import type {TransactionMergeParams} from './API/parameters'; | ||
| import {isCorporateCard, isExpensifyCard} from './CardUtils'; | ||
| import {getCurrencyDecimals} from './CurrencyUtils'; | ||
| import DateUtils from './DateUtils'; | ||
|
|
@@ -857,7 +858,7 @@ function compareDuplicateTransactionFields(transactionID: string): {keep: Partia | |
| const change: Record<string, any[]> = {}; | ||
|
|
||
| const fieldsToCompare: FieldsToCompare = { | ||
| merchant: ['merchant', 'modifiedMerchant'], | ||
| merchant: ['modifiedMerchant', 'merchant'], | ||
| category: ['category'], | ||
| tag: ['tag'], | ||
| description: ['comment'], | ||
|
|
@@ -866,7 +867,20 @@ function compareDuplicateTransactionFields(transactionID: string): {keep: Partia | |
| reimbursable: ['reimbursable'], | ||
| }; | ||
|
|
||
| const getDifferentValues = (items: Array<OnyxEntry<Transaction>>, keys: Array<keyof Transaction>) => [...new Set(items.map((item) => keys.map((key) => item?.[key])).flat())]; | ||
| const getDifferentValues = (items: Array<OnyxEntry<Transaction>>, keys: Array<keyof Transaction>) => [ | ||
| ...new Set( | ||
| items | ||
| .map((item) => { | ||
| // Prioritize modifiedMerchant over merchant | ||
| if (keys.includes('modifiedMerchant' as keyof Transaction) && keys.includes('merchant' as keyof Transaction)) { | ||
| // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing | ||
| return item?.modifiedMerchant || item?.merchant; | ||
| } | ||
| return keys.map((key) => item?.[key]); | ||
| }) | ||
| .flat(), | ||
| ), | ||
| ]; | ||
|
|
||
| for (const fieldName in fieldsToCompare) { | ||
| if (Object.prototype.hasOwnProperty.call(fieldsToCompare, fieldName)) { | ||
|
|
@@ -913,6 +927,37 @@ function getTransactionID(threadReportID: string): string { | |
| return IOUTransactionID; | ||
| } | ||
|
|
||
| function buildNewTransactionAfterReviewingDuplicates(reviewDuplicateTransaction: OnyxEntry<ReviewDuplicates>): Partial<Transaction> { | ||
| const originalTransaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${reviewDuplicateTransaction?.transactionID}`] ?? undefined; | ||
| const {duplicates, ...restReviewDuplicateTransaction} = reviewDuplicateTransaction ?? {}; | ||
|
|
||
| return { | ||
| ...originalTransaction, | ||
| ...restReviewDuplicateTransaction, | ||
| modifiedMerchant: reviewDuplicateTransaction?.merchant, | ||
| merchant: reviewDuplicateTransaction?.merchant, | ||
| comment: {comment: reviewDuplicateTransaction?.description}, | ||
| }; | ||
| } | ||
|
|
||
| function buildTransactionsMergeParams(reviewDuplicates: OnyxEntry<ReviewDuplicates>, originalTransaction: Partial<Transaction>): TransactionMergeParams { | ||
| return { | ||
| amount: -getAmount(originalTransaction as OnyxEntry<Transaction>, false), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. {BZ CHECKLIST https://github.com/Expensify/App/issues/55404} There was regression that -ve amount was not correctly resolved in expense reports, we fixed it by passing |
||
| reportID: originalTransaction?.reportID ?? '', | ||
| receiptID: originalTransaction?.receipt?.receiptID ?? 0, | ||
| currency: getCurrency(originalTransaction as OnyxEntry<Transaction>), | ||
| created: getFormattedCreated(originalTransaction as OnyxEntry<Transaction>), | ||
| transactionID: reviewDuplicates?.transactionID ?? '', | ||
| transactionIDList: reviewDuplicates?.duplicates ?? [], | ||
| billable: reviewDuplicates?.billable ?? false, | ||
| reimbursable: reviewDuplicates?.reimbursable ?? false, | ||
| category: reviewDuplicates?.category ?? '', | ||
| tag: reviewDuplicates?.tag ?? '', | ||
| merchant: reviewDuplicates?.merchant ?? '', | ||
| comment: reviewDuplicates?.description ?? '', | ||
| }; | ||
| } | ||
|
|
||
| export { | ||
| buildOptimisticTransaction, | ||
| calculateTaxAmount, | ||
|
|
@@ -983,6 +1028,8 @@ export { | |
| getTransaction, | ||
| compareDuplicateTransactionFields, | ||
| getTransactionID, | ||
| buildNewTransactionAfterReviewingDuplicates, | ||
| buildTransactionsMergeParams, | ||
| getReimbursable, | ||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -24,6 +24,7 @@ import type { | |||||||
| StartSplitBillParams, | ||||||||
| SubmitReportParams, | ||||||||
| TrackExpenseParams, | ||||||||
| TransactionMergeParams, | ||||||||
| UnapproveExpenseReportParams, | ||||||||
| UpdateMoneyRequestParams, | ||||||||
| } from '@libs/API/parameters'; | ||||||||
|
|
@@ -7351,6 +7352,75 @@ function getIOURequestPolicyID(transaction: OnyxEntry<OnyxTypes.Transaction>, re | |||||||
| return workspaceSender?.policyID ?? report?.policyID ?? '-1'; | ||||||||
| } | ||||||||
|
|
||||||||
| /** Merge several transactions into one by updating the fields of the one we want to keep and deleting the rest */ | ||||||||
| function mergeDuplicates(params: TransactionMergeParams) { | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| const originalSelectedTransaction = allTransactions[`${ONYXKEYS.COLLECTION.TRANSACTION}${params.transactionID}`]; | ||||||||
|
|
||||||||
| const optimisticTransactionData: OnyxUpdate = { | ||||||||
| onyxMethod: Onyx.METHOD.MERGE, | ||||||||
| key: `${ONYXKEYS.COLLECTION.TRANSACTION}${params.transactionID}`, | ||||||||
| value: { | ||||||||
| ...originalSelectedTransaction, | ||||||||
| billable: params.billable, | ||||||||
| comment: { | ||||||||
| comment: params.comment, | ||||||||
| }, | ||||||||
| category: params.category, | ||||||||
| created: params.created, | ||||||||
| currency: params.currency, | ||||||||
| modifiedMerchant: params.merchant, | ||||||||
| reimbursable: params.reimbursable, | ||||||||
| tag: params.tag, | ||||||||
| }, | ||||||||
| }; | ||||||||
|
|
||||||||
| const failureTransactionData: OnyxUpdate = { | ||||||||
| onyxMethod: Onyx.METHOD.MERGE, | ||||||||
| key: `${ONYXKEYS.COLLECTION.TRANSACTION}${params.transactionID}`, | ||||||||
| // eslint-disable-next-line @typescript-eslint/non-nullable-type-assertion-style | ||||||||
| value: originalSelectedTransaction as OnyxTypes.Transaction, | ||||||||
| }; | ||||||||
|
|
||||||||
| const optimisticTransactionDuplicatesData: OnyxUpdate[] = params.transactionIDList.map((id) => ({ | ||||||||
| onyxMethod: Onyx.METHOD.SET, | ||||||||
| key: `${ONYXKEYS.COLLECTION.TRANSACTION}${id}`, | ||||||||
| value: null, | ||||||||
| })); | ||||||||
|
|
||||||||
| const failureTransactionDuplicatesData: OnyxUpdate[] = params.transactionIDList.map((id) => ({ | ||||||||
| onyxMethod: Onyx.METHOD.MERGE, | ||||||||
| key: `${ONYXKEYS.COLLECTION.TRANSACTION}${id}`, | ||||||||
| // eslint-disable-next-line @typescript-eslint/non-nullable-type-assertion-style | ||||||||
| value: allTransactions[`${ONYXKEYS.COLLECTION.TRANSACTION}${id}`] as OnyxTypes.Transaction, | ||||||||
| })); | ||||||||
|
|
||||||||
| const optimisticTransactionViolations: OnyxUpdate[] = [...params.transactionIDList, params.transactionID].map((id) => { | ||||||||
| const violations = allTransactionViolations[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${id}`] ?? []; | ||||||||
| return { | ||||||||
| onyxMethod: Onyx.METHOD.MERGE, | ||||||||
| key: `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${id}`, | ||||||||
| value: violations.filter((violation) => violation.name !== CONST.VIOLATIONS.DUPLICATED_TRANSACTION), | ||||||||
| }; | ||||||||
| }); | ||||||||
|
|
||||||||
| const failureTransactionViolations: OnyxUpdate[] = [...params.transactionIDList, params.transactionID].map((id) => { | ||||||||
| const violations = allTransactionViolations[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${id}`] ?? []; | ||||||||
| return { | ||||||||
| onyxMethod: Onyx.METHOD.MERGE, | ||||||||
| key: `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${id}`, | ||||||||
| value: violations, | ||||||||
| }; | ||||||||
| }); | ||||||||
|
|
||||||||
| const optimisticData: OnyxUpdate[] = []; | ||||||||
| const failureData: OnyxUpdate[] = []; | ||||||||
|
|
||||||||
kubabutkiewicz marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
| optimisticData.push(optimisticTransactionData, ...optimisticTransactionDuplicatesData, ...optimisticTransactionViolations); | ||||||||
| failureData.push(failureTransactionData, ...failureTransactionDuplicatesData, ...failureTransactionViolations); | ||||||||
|
|
||||||||
| API.write(WRITE_COMMANDS.TRANSACTION_MERGE, params, {optimisticData, failureData}); | ||||||||
| } | ||||||||
|
|
||||||||
| export { | ||||||||
| adjustRemainingSplitShares, | ||||||||
| approveMoneyRequest, | ||||||||
|
|
@@ -7418,5 +7488,6 @@ export { | |||||||
| updateMoneyRequestTag, | ||||||||
| updateMoneyRequestTaxAmount, | ||||||||
| updateMoneyRequestTaxRate, | ||||||||
| mergeDuplicates, | ||||||||
| }; | ||||||||
| export type {GPSPoint as GpsPoint, IOURequestType}; | ||||||||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Omitting other
reviewDuplicateTransaction.commentproperties caused an issue where if default tax is selected,Defaultlabel is not shown next to tax name. More info: #47975