-
Notifications
You must be signed in to change notification settings - Fork 3.8k
fix: [GPS]Transactions - Always use iouRequestType to check for request type #88289
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
d944b76
12c3d5b
8fb5b4d
3688075
bd93a70
f3a7663
6f81363
18931b3
70301d8
44ce9d1
3eff827
d67f364
aacb30c
40d4fc6
6af4347
af222be
b477c85
6f41c46
4e80f6b
270b2f9
c1d6235
0173d32
27fc427
8258c14
cbbd526
a82c0cd
e2ee7a3
a5e69fd
402ae74
e75a524
4fead9e
fbba0c5
d86efe5
fb05eca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -141,53 +141,13 @@ function isDeletedTransaction(transaction: {reportID?: string}): boolean { | |
| return transaction.reportID === CONST.REPORT.TRASH_REPORT_ID; | ||
| } | ||
|
|
||
| function hasDistanceCustomUnit(transaction: OnyxEntry<Transaction> | Partial<Transaction>): boolean { | ||
| return transaction?.comment?.type === CONST.TRANSACTION.TYPE.CUSTOM_UNIT && transaction?.comment?.customUnit?.name === CONST.CUSTOM_UNITS.NAME_DISTANCE; | ||
| } | ||
|
|
||
| function isDistanceRequest(transaction: OnyxEntry<Transaction>): boolean { | ||
| // This is used during the expense creation flow before the transaction has been saved to the server | ||
| if (transaction && Object.hasOwn(transaction, 'iouRequestType')) { | ||
| return ( | ||
| transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE || | ||
| transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_MAP || | ||
| transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_ODOMETER || | ||
| transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_GPS || | ||
| transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_MANUAL | ||
| ); | ||
| } | ||
|
|
||
| // This is the case for transaction objects once they have been saved to the server | ||
| return hasDistanceCustomUnit(transaction); | ||
| const requestType = transaction?.iouRequestType; | ||
| return requestType === CONST.IOU.REQUEST_TYPE.DISTANCE || isDistanceExpenseType(requestType); | ||
| } | ||
|
|
||
| function isDistanceTypeRequest(transaction: OnyxEntry<Transaction>): boolean { | ||
| // This is used during the expense creation flow before the transaction has been saved to the server | ||
| if (transaction && Object.hasOwn(transaction, 'iouRequestType')) { | ||
| return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE; | ||
| } | ||
|
|
||
| // This is the case for transaction objects once they have been saved to the server | ||
| return hasDistanceCustomUnit(transaction); | ||
| } | ||
|
|
||
| /** | ||
| * todo: Currently there is no way to tell server map transaction object from | ||
| * server GPS transaction object, this will be discussed and updated later. | ||
| * To fix this temporarily we set keyForList of GPS waypoints to 'gps_start' and 'gps_end' | ||
| * and use that to determine if it's a GPS or Map transaction. This should be changed before | ||
| * the first GPS release. | ||
| */ | ||
| function hasGPSWaypoints(transaction: OnyxEntry<Transaction>) { | ||
| const waypoints = transaction?.comment?.waypoints; | ||
|
|
||
| if (!waypoints) { | ||
| return false; | ||
| } | ||
|
|
||
| const waypoint = Object.values(waypoints).at(0); | ||
|
|
||
| return !!waypoint?.keyForList?.startsWith('gps'); | ||
| return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -202,90 +162,31 @@ function haveWaypointAddressesChanged(oldWaypoints: WaypointCollection | undefin | |
| } | ||
|
|
||
| function isMapDistanceRequest(transaction: OnyxEntry<Transaction>): boolean { | ||
| // This is used during the expense creation flow before the transaction has been saved to the server | ||
| if (transaction && Object.hasOwn(transaction, 'iouRequestType')) { | ||
| return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_MAP; | ||
| } | ||
|
|
||
| // This is the case for transaction objects once they have been saved to the server | ||
| return hasDistanceCustomUnit(transaction) && !hasGPSWaypoints(transaction); | ||
| return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_MAP; | ||
| } | ||
|
|
||
| function isGPSDistanceRequest(transaction: OnyxEntry<Transaction>): boolean { | ||
| // This is used during the expense creation flow before the transaction has been saved to the server | ||
| if (transaction && Object.hasOwn(transaction, 'iouRequestType')) { | ||
| return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_GPS; | ||
| } | ||
|
|
||
| // This is the case for transaction objects once they have been saved to the server | ||
| return hasGPSWaypoints(transaction); | ||
| return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_GPS; | ||
| } | ||
|
|
||
| function isManualDistanceRequest(transaction: OnyxEntry<Transaction>, isUpdatedMergeTransaction = false): boolean { | ||
| // This is used during the expense creation flow before the transaction has been saved to the server | ||
| if (transaction && Object.hasOwn(transaction, 'iouRequestType') && !isUpdatedMergeTransaction) { | ||
| return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_MANUAL; | ||
| } | ||
|
|
||
| // This is the case for transaction objects once they have been saved to the server | ||
| // Exclude odometer requests which also have no waypoints but have odometer readings | ||
| return ( | ||
| hasDistanceCustomUnit(transaction) && | ||
| isEmptyObject(transaction?.comment?.waypoints) && | ||
| transaction?.comment?.odometerStart === undefined && | ||
| transaction?.comment?.odometerEnd === undefined | ||
| ); | ||
| function isManualDistanceRequest(transaction: OnyxEntry<Transaction>): boolean { | ||
| return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_MANUAL; | ||
| } | ||
|
|
||
| function isOdometerDistanceRequest(transaction: OnyxEntry<Transaction>): boolean { | ||
| // This is used during the expense creation flow before the transaction has been saved to the server | ||
| if (transaction && Object.hasOwn(transaction, 'iouRequestType')) { | ||
| return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_ODOMETER; | ||
| } | ||
|
|
||
| // This is the case for transaction objects once they have been saved to the server | ||
| // Odometer requests have odometerStart and odometerEnd in comment, and no waypoints | ||
| return ( | ||
| hasDistanceCustomUnit(transaction) && | ||
| isEmptyObject(transaction?.comment?.waypoints) && | ||
| (transaction?.comment?.odometerStart !== undefined || transaction?.comment?.odometerEnd !== undefined) | ||
| ); | ||
| return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_ODOMETER; | ||
| } | ||
|
|
||
| function isScanRequest(transaction: OnyxEntry<Transaction> | Partial<Transaction>): boolean { | ||
| // This is used during the expense creation flow before the transaction has been saved to the server | ||
| if (transaction && Object.hasOwn(transaction, 'iouRequestType')) { | ||
| return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.SCAN; | ||
| } | ||
|
|
||
| // Distance requests can have a receipt source (for the map), so we need to exclude them | ||
| if (hasDistanceCustomUnit(transaction)) { | ||
| return false; | ||
| } | ||
|
|
||
| return !!transaction?.receipt?.source && transaction?.amount === 0; | ||
| function isScanRequest(transaction: OnyxEntry<Transaction>): boolean { | ||
| return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.SCAN; | ||
|
TaduJR marked this conversation as resolved.
|
||
| } | ||
|
|
||
| function isPerDiemRequest(transaction: OnyxEntry<Transaction>): boolean { | ||
| // This is used during the expense creation flow before the transaction has been saved to the server | ||
| if (transaction && Object.hasOwn(transaction, 'iouRequestType')) { | ||
| return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.PER_DIEM; | ||
| } | ||
|
|
||
| // This is the case for transaction objects once they have been saved to the server | ||
| const type = transaction?.comment?.type; | ||
| const customUnitName = transaction?.comment?.customUnit?.name; | ||
| return type === CONST.TRANSACTION.TYPE.CUSTOM_UNIT && customUnitName === CONST.CUSTOM_UNITS.NAME_PER_DIEM_INTERNATIONAL; | ||
| return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.PER_DIEM; | ||
| } | ||
|
|
||
| function isTimeRequest(transaction: OnyxEntry<Transaction>): boolean { | ||
| // This is used during the expense creation flow before the transaction has been saved to the server | ||
| if (transaction && Object.hasOwn(transaction, 'iouRequestType')) { | ||
| return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.TIME; | ||
| } | ||
|
|
||
| // This is the case for transaction objects once they have been saved to the server | ||
| return transaction?.comment?.type === CONST.TRANSACTION.TYPE.TIME; | ||
| return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.TIME; | ||
| } | ||
|
|
||
| function isDistanceExpenseType(requestType: IOURequestType | undefined): requestType is DistanceExpenseType { | ||
|
|
@@ -302,32 +203,7 @@ function isCorporateCardTransaction(transaction: OnyxEntry<Transaction>): boolea | |
| } | ||
|
|
||
| function getRequestType(transaction: OnyxEntry<Transaction>): IOURequestType { | ||
| if (isOdometerDistanceRequest(transaction)) { | ||
| return CONST.IOU.REQUEST_TYPE.DISTANCE_ODOMETER; | ||
| } | ||
| if (isManualDistanceRequest(transaction)) { | ||
| return CONST.IOU.REQUEST_TYPE.DISTANCE_MANUAL; | ||
| } | ||
| if (isMapDistanceRequest(transaction)) { | ||
| return CONST.IOU.REQUEST_TYPE.DISTANCE_MAP; | ||
| } | ||
| if (isDistanceTypeRequest(transaction)) { | ||
| return CONST.IOU.REQUEST_TYPE.DISTANCE; | ||
| } | ||
| if (isScanRequest(transaction)) { | ||
| return CONST.IOU.REQUEST_TYPE.SCAN; | ||
| } | ||
| if (isPerDiemRequest(transaction)) { | ||
| return CONST.IOU.REQUEST_TYPE.PER_DIEM; | ||
| } | ||
| if (isTimeRequest(transaction)) { | ||
| return CONST.IOU.REQUEST_TYPE.TIME; | ||
| } | ||
| if (isGPSDistanceRequest(transaction)) { | ||
| return CONST.IOU.REQUEST_TYPE.DISTANCE_GPS; | ||
| } | ||
|
|
||
| return CONST.IOU.REQUEST_TYPE.MANUAL; | ||
| return transaction?.iouRequestType ?? CONST.IOU.REQUEST_TYPE.MANUAL; | ||
|
TaduJR marked this conversation as resolved.
TaduJR marked this conversation as resolved.
TaduJR marked this conversation as resolved.
|
||
| } | ||
|
|
||
| /** | ||
|
|
@@ -552,7 +428,6 @@ function buildOptimisticTransaction(params: BuildOptimisticTransactionParams): T | |
| cardID: existingTransaction?.cardID, | ||
| cardName: existingTransaction?.cardName, | ||
| cardNumber: existingTransaction?.cardNumber, | ||
| // Use conditional spread to avoid creating the key if it's undefined, which would break lodashHas checks. | ||
| ...(existingTransaction?.iouRequestType ? {iouRequestType: existingTransaction.iouRequestType} : {}), | ||
|
Collaborator
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. Optimistic transactions won’t get iouRequestType unless the caller seeded it ? Can you confirm my assumption please?
Contributor
Author
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.
But the caller reliably seeds it: the draft created by |
||
| routes, | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -615,6 +615,15 @@ function buildDuplicateTransactionParams(transaction: OnyxTypes.Transaction, tra | |
| return {transactionParams, waypoints}; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the request type the duplicate should be created with. SCAN sources become MANUAL because | ||
| * `buildDuplicateTransactionParams` strips the receipt — without one, the duplicate cannot be a scan request. | ||
| */ | ||
| function getDuplicateRequestType(transaction: OnyxTypes.Transaction) { | ||
| const sourceRequestType = getRequestType(transaction); | ||
| return sourceRequestType === CONST.IOU.REQUEST_TYPE.SCAN ? CONST.IOU.REQUEST_TYPE.MANUAL : sourceRequestType; | ||
| } | ||
|
|
||
| /** | ||
| * Routes a duplicate expense to the correct creation function based on transaction type. | ||
| * Shared between duplicateExpenseTransaction and duplicateReport. | ||
|
|
@@ -654,18 +663,20 @@ function createExpenseByType({ | |
| currentUserLogin: params.currentUserEmailParam, | ||
| currentUserAccountID: params.currentUserAccountIDParam, | ||
| existingTransaction: { | ||
| ...(params.transactionParams ?? {}), | ||
| iouRequestType: getDuplicateRequestType(transaction), | ||
|
Collaborator
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. I can see this being duplicated quite a few times. |
||
| amount: 0, | ||
| currency: '', | ||
| created: '', | ||
| merchant: '', | ||
| reportID: '1', | ||
| transactionID: '1', | ||
| comment: { | ||
| ...transaction.comment, | ||
| hold: undefined, | ||
| originalTransactionID: undefined, | ||
| source: undefined, | ||
| waypoints, | ||
| }, | ||
| iouRequestType: getRequestType(transaction), | ||
| modifiedCreated: '', | ||
| reportID: '1', | ||
| transactionID: '1', | ||
| }, | ||
| transactionParams: { | ||
| ...(params.transactionParams ?? {}), | ||
|
|
@@ -787,6 +798,15 @@ function duplicateExpenseTransaction({ | |
| policyRecentlyUsedCurrencies, | ||
| quickAction, | ||
| existingTransactionDraft, | ||
| existingTransaction: { | ||
| iouRequestType: getDuplicateRequestType(transaction), | ||
| amount: 0, | ||
| currency: '', | ||
| created: '', | ||
| merchant: '', | ||
| reportID: '1', | ||
| transactionID: '1', | ||
| }, | ||
| draftTransactionIDs, | ||
| isSelfTourViewed, | ||
| betas, | ||
|
|
@@ -803,18 +823,20 @@ function duplicateExpenseTransaction({ | |
| participant: {accountID: currentUserAccountID, selected: true}, | ||
| }, | ||
| existingTransaction: { | ||
| ...(params.transactionParams ?? {}), | ||
| iouRequestType: getDuplicateRequestType(transaction), | ||
| amount: 0, | ||
| currency: '', | ||
| created: '', | ||
| merchant: '', | ||
| reportID: '1', | ||
| transactionID: '1', | ||
| comment: { | ||
| ...transaction.comment, | ||
| hold: undefined, | ||
| originalTransactionID: undefined, | ||
| source: undefined, | ||
| waypoints, | ||
| }, | ||
| iouRequestType: getRequestType(transaction), | ||
| modifiedCreated: '', | ||
| reportID: '1', | ||
| transactionID: '1', | ||
| }, | ||
| transactionParams: { | ||
| ...(params.transactionParams ?? {}), | ||
|
|
@@ -981,6 +1003,15 @@ function duplicateReport({ | |
| quickAction, | ||
| policyRecentlyUsedCurrencies, | ||
| existingTransactionDraft: undefined, | ||
| existingTransaction: { | ||
| iouRequestType: getDuplicateRequestType(transaction), | ||
| amount: 0, | ||
| currency: '', | ||
| created: '', | ||
| merchant: '', | ||
| reportID: '1', | ||
| transactionID: '1', | ||
| }, | ||
| draftTransactionIDs, | ||
| isSelfTourViewed, | ||
| betas, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -488,6 +488,8 @@ function startSplitBill({ | |
| }, | ||
| }); | ||
|
|
||
| splitTransaction.iouRequestType = receipt?.source ? CONST.IOU.REQUEST_TYPE.SCAN : CONST.IOU.REQUEST_TYPE.MANUAL; | ||
|
Collaborator
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. We're building splitTransaction just before this line. Any reason why this isn't added there? |
||
|
|
||
| const filename = splitTransaction.receipt?.filename; | ||
|
|
||
| // Note: The created action must be optimistically generated before the IOU action so there's no chance that the created action appears after the IOU action in the chat | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.