-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Restore "#81017 Fix updates of distance and distance units when changing expense recipients" with fixes #82148
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
4a6472b
e48c017
bc5e8cc
bfc191a
9a8860c
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 |
|---|---|---|
|
|
@@ -22,7 +22,6 @@ import { | |
| setCustomUnitRateID, | ||
| setMoneyRequestAmount, | ||
| setMoneyRequestCategory, | ||
| setMoneyRequestDistance, | ||
| setMoneyRequestMerchant, | ||
| setMoneyRequestPendingFields, | ||
| setMoneyRequestTag, | ||
|
|
@@ -34,7 +33,6 @@ import {getIsMissingAttendeesViolation} from '@libs/AttendeeUtils'; | |
| import {isCategoryDescriptionRequired} from '@libs/CategoryUtils'; | ||
| import {convertToBackendAmount, convertToDisplayString, convertToDisplayStringWithoutCurrency} from '@libs/CurrencyUtils'; | ||
| import DistanceRequestUtils from '@libs/DistanceRequestUtils'; | ||
| import {calculateGPSDistance} from '@libs/GPSDraftDetailsUtils'; | ||
| import {calculateAmount, insertTagIntoTransactionTagsString, isMovingTransactionFromTrackExpense as isMovingTransactionFromTrackExpenseUtil} from '@libs/IOUUtils'; | ||
| import Log from '@libs/Log'; | ||
| import {validateAmount} from '@libs/MoneyRequestUtils'; | ||
|
|
@@ -338,21 +336,6 @@ function MoneyRequestConfirmationList({ | |
| const defaultRate = defaultMileageRate?.customUnitRateID; | ||
| const lastSelectedRate = policy?.id ? (lastSelectedDistanceRates?.[policy.id] ?? defaultRate) : defaultRate; | ||
|
|
||
| useEffect(() => { | ||
| if ( | ||
| !['-1', CONST.CUSTOM_UNITS.FAKE_P2P_ID].includes(customUnitRateID) || | ||
| !isDistanceRequest || | ||
| !isPolicyExpenseChat || | ||
| !transactionID || | ||
| !lastSelectedRate || | ||
| isMovingTransactionFromTrackExpense | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| setCustomUnitRateID(transactionID, lastSelectedRate); | ||
| }, [customUnitRateID, transactionID, lastSelectedRate, isDistanceRequest, isPolicyExpenseChat, isMovingTransactionFromTrackExpense]); | ||
|
|
||
| const mileageRate = DistanceRequestUtils.getRate({transaction, policy, policyDraft}); | ||
| const rate = mileageRate.rate; | ||
| const prevRate = usePrevious(rate); | ||
|
|
@@ -398,19 +381,6 @@ function MoneyRequestConfirmationList({ | |
|
|
||
| const distanceRequestAmount = DistanceRequestUtils.getDistanceRequestAmount(distance, unit ?? CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES, rate ?? 0); | ||
|
|
||
| // Update GPS distance whenever the current distance unit differs from the one that was used | ||
| // to calculate the distance stored in transaction.comment.customUnit.quantity | ||
| const gpsDistance = transaction?.comment?.customUnit?.quantity; | ||
| const gpsDistanceWithCurrentDistanceUnit = calculateGPSDistance(distance, unit); | ||
| const shouldUpdateGpsDistance = isGPSDistanceRequest && gpsDistance !== gpsDistanceWithCurrentDistanceUnit; | ||
| useEffect(() => { | ||
| if (!shouldUpdateGpsDistance || !transactionID || isReadOnly) { | ||
| return; | ||
| } | ||
|
|
||
| setMoneyRequestDistance(transactionID, gpsDistanceWithCurrentDistanceUnit, true); | ||
| }, [shouldUpdateGpsDistance, transactionID, isReadOnly, gpsDistanceWithCurrentDistanceUnit]); | ||
|
|
||
| let amountToBeUsed = iouAmount; | ||
|
|
||
| if (shouldCalculateDistanceAmount) { | ||
|
|
@@ -517,7 +487,7 @@ function MoneyRequestConfirmationList({ | |
| // If there is a distance rate in the policy that matches the rate and unit of the currently selected mileage rate, select it automatically | ||
| const matchingRate = Object.values(policyRates).find((policyRate) => policyRate.rate === mileageRate.rate && policyRate.unit === mileageRate.unit); | ||
| if (matchingRate?.customUnitRateID) { | ||
| setCustomUnitRateID(transactionID, matchingRate.customUnitRateID); | ||
| setCustomUnitRateID(transactionID, matchingRate.customUnitRateID, transaction, policy); | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -534,6 +504,7 @@ function MoneyRequestConfirmationList({ | |
| isMovingTransactionFromTrackExpense, | ||
| setFormError, | ||
| clearFormErrors, | ||
| transaction, | ||
| ]); | ||
|
|
||
| const routeError = Object.values(transaction?.errorFields?.route ?? {}).at(0); | ||
|
|
@@ -700,6 +671,22 @@ function MoneyRequestConfirmationList({ | |
| const payeePersonalDetails = useMemo(() => payeePersonalDetailsProp ?? currentUserPersonalDetails, [payeePersonalDetailsProp, currentUserPersonalDetails]); | ||
| const shouldShowReadOnlySplits = useMemo(() => isPolicyExpenseChat || isReadOnly || isScanRequest, [isPolicyExpenseChat, isReadOnly, isScanRequest]); | ||
|
|
||
| useEffect(() => { | ||
| if ( | ||
| !['-1', CONST.CUSTOM_UNITS.FAKE_P2P_ID].includes(customUnitRateID) || | ||
| !isDistanceRequest || | ||
| !isPolicyExpenseChat || | ||
| !transactionID || | ||
| !lastSelectedRate || | ||
| isMovingTransactionFromTrackExpense || | ||
| !selectedParticipants.some((participant) => participant.policyID === policy?.id) | ||
|
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. This is the only change added to this hook. This prevents situations where setCustomUnitRateID is called for a policy that is not a participant of the current transaction (was being called for example when for selfDM user tried to change rate and IOURequestStepUpgrade logic was used and updated transactions participants to the new workspace) |
||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| setCustomUnitRateID(transactionID, lastSelectedRate, transaction, policy); | ||
| }, [customUnitRateID, transactionID, lastSelectedRate, isDistanceRequest, isPolicyExpenseChat, isMovingTransactionFromTrackExpense, transaction, policy, selectedParticipants]); | ||
|
|
||
| const splitParticipants = useMemo(() => { | ||
| if (!isTypeSplit) { | ||
| return []; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,6 +67,7 @@ | |
| import {isOffline} from '@libs/Network/NetworkStore'; | ||
| // eslint-disable-next-line @typescript-eslint/no-deprecated | ||
| import {buildNextStepNew, buildOptimisticNextStep} from '@libs/NextStepUtils'; | ||
| import {roundToTwoDecimalPlaces} from '@libs/NumberUtils'; | ||
| import * as NumberUtils from '@libs/NumberUtils'; | ||
| import {getManagerMcTestParticipant, getPersonalDetailsForAccountIDs} from '@libs/OptionsListUtils'; | ||
| import Parser from '@libs/Parser'; | ||
|
|
@@ -251,6 +252,7 @@ | |
| import type {ErrorFields, Errors, PendingAction, PendingFields} from '@src/types/onyx/OnyxCommon'; | ||
| import type {PaymentMethodType} from '@src/types/onyx/OriginalMessage'; | ||
| import type {CurrentUserPersonalDetails} from '@src/types/onyx/PersonalDetails'; | ||
| import type {Unit} from '@src/types/onyx/Policy'; | ||
| import type {QuickActionName} from '@src/types/onyx/QuickAction'; | ||
| import type RecentlyUsedTags from '@src/types/onyx/RecentlyUsedTags'; | ||
| import type {ReportNextStep} from '@src/types/onyx/Report'; | ||
|
|
@@ -790,7 +792,7 @@ | |
| }; | ||
|
|
||
| let allPersonalDetails: OnyxTypes.PersonalDetailsList = {}; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.PERSONAL_DETAILS_LIST, | ||
| callback: (value) => { | ||
| allPersonalDetails = value ?? {}; | ||
|
|
@@ -883,7 +885,7 @@ | |
| }; | ||
|
|
||
| let allTransactions: NonNullable<OnyxCollection<OnyxTypes.Transaction>> = {}; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.TRANSACTION, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => { | ||
|
|
@@ -897,7 +899,7 @@ | |
| }); | ||
|
|
||
| let allTransactionDrafts: NonNullable<OnyxCollection<OnyxTypes.Transaction>> = {}; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.TRANSACTION_DRAFT, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => { | ||
|
|
@@ -906,7 +908,7 @@ | |
| }); | ||
|
|
||
| let allTransactionViolations: NonNullable<OnyxCollection<OnyxTypes.TransactionViolations>> = {}; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => { | ||
|
|
@@ -920,7 +922,7 @@ | |
| }); | ||
|
|
||
| let allPolicyTags: OnyxCollection<OnyxTypes.PolicyTagLists> = {}; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.POLICY_TAGS, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => { | ||
|
|
@@ -933,7 +935,7 @@ | |
| }); | ||
|
|
||
| let allReports: OnyxCollection<OnyxTypes.Report>; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.REPORT, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => { | ||
|
|
@@ -942,7 +944,7 @@ | |
| }); | ||
|
|
||
| let allReportNameValuePairs: OnyxCollection<OnyxTypes.ReportNameValuePairs>; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => { | ||
|
|
@@ -952,7 +954,7 @@ | |
|
|
||
| let userAccountID = -1; | ||
| let currentUserEmail = ''; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.SESSION, | ||
| callback: (value) => { | ||
| currentUserEmail = value?.email ?? ''; | ||
|
|
@@ -961,7 +963,7 @@ | |
| }); | ||
|
|
||
| let deprecatedCurrentUserPersonalDetails: OnyxEntry<OnyxTypes.PersonalDetails>; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.PERSONAL_DETAILS_LIST, | ||
| callback: (value) => { | ||
| deprecatedCurrentUserPersonalDetails = value?.[userAccountID] ?? undefined; | ||
|
|
@@ -969,7 +971,7 @@ | |
| }); | ||
|
|
||
| let allReportActions: OnyxCollection<OnyxTypes.ReportActions>; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, | ||
| waitForCollectionCallback: true, | ||
| callback: (actions) => { | ||
|
|
@@ -1506,15 +1508,48 @@ | |
| } | ||
|
|
||
| /** | ||
| * Set custom unit rateID for the transaction draft | ||
| * Set custom unit rateID for the transaction draft, also updates quantity and distanceUnit | ||
| * if passed transaction previously had it to make sure that transaction does not have inconsistent | ||
| * states (for example distanceUnit not matching distance unit of the new customUnitRateID) | ||
| */ | ||
| function setCustomUnitRateID(transactionID: string, customUnitRateID: string | undefined) { | ||
| function setCustomUnitRateID(transactionID: string, customUnitRateID: string | undefined, transaction: OnyxEntry<OnyxTypes.Transaction>, policy: OnyxEntry<OnyxTypes.Policy>) { | ||
| const isFakeP2PRate = customUnitRateID === CONST.CUSTOM_UNITS.FAKE_P2P_ID; | ||
|
|
||
| let newDistanceUnit: Unit | undefined; | ||
| let newQuantity: number | undefined; | ||
GCyganek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if (customUnitRateID && transaction) { | ||
| const distanceRate = isFakeP2PRate | ||
| ? DistanceRequestUtils.getRate({transaction, useTransactionDistanceUnit: false, policy}) | ||
| : DistanceRequestUtils.getRateByCustomUnitRateID({policy, customUnitRateID}); | ||
|
|
||
| const transactionDistanceUnit = transaction.comment?.customUnit?.distanceUnit; | ||
| const transactionQuantity = transaction.comment?.customUnit?.quantity; | ||
|
|
||
| const shouldUpdateDistanceUnit = !!transactionDistanceUnit && !!distanceRate?.unit; | ||
| const shouldUpdateQuantity = transactionQuantity !== null && transactionQuantity !== undefined; | ||
|
|
||
| if (shouldUpdateDistanceUnit) { | ||
| newDistanceUnit = distanceRate.unit; | ||
| } | ||
| if (shouldUpdateQuantity && !!distanceRate?.unit) { | ||
| const newQuantityInMeters = getDistanceInMeters(transaction, transactionDistanceUnit); | ||
|
|
||
| // getDistanceInMeters returns 0 only if there was not enough input to get the correct | ||
| // distance in meters or if the current transaction distance is 0 | ||
| if (newQuantityInMeters !== 0) { | ||
| newQuantity = DistanceRequestUtils.convertDistanceUnit(newQuantityInMeters, distanceRate.unit); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, { | ||
| comment: { | ||
| customUnit: { | ||
| customUnitRateID, | ||
| ...(!isFakeP2PRate && {defaultP2PRate: null}), | ||
| distanceUnit: newDistanceUnit, | ||
| quantity: newQuantity, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
@@ -1662,8 +1697,8 @@ | |
| Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transaction?.transactionID}`, newTransaction); | ||
| } | ||
|
|
||
| function setMoneyRequestDistance(transactionID: string, distanceAsFloat: number, isDraft: boolean) { | ||
| Onyx.merge(`${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, {comment: {customUnit: {quantity: distanceAsFloat}}}); | ||
| function setMoneyRequestDistance(transactionID: string, distanceAsFloat: number, isDraft: boolean, distanceUnit: Unit) { | ||
| Onyx.merge(`${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, {comment: {customUnit: {quantity: distanceAsFloat, distanceUnit}}}); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -1719,18 +1754,20 @@ | |
| Onyx.merge(ONYXKEYS.NVP_LAST_SELECTED_DISTANCE_RATES, {[policy.id]: customUnitRateID}); | ||
| } | ||
|
|
||
| const distanceRate = DistanceRequestUtils.getRateByCustomUnitRateID({policy, customUnitRateID}); | ||
| const newDistanceUnit = getDistanceRateCustomUnit(policy)?.attributes?.unit; | ||
|
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.
|
||
| const transaction = isDraft ? allTransactionDrafts[`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`] : allTransactions[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]; | ||
|
|
||
| let newDistance; | ||
| if (distanceRate?.unit && distanceRate?.unit !== transaction?.comment?.customUnit?.distanceUnit) { | ||
| newDistance = DistanceRequestUtils.convertDistanceUnit(getDistanceInMeters(transaction, transaction?.comment?.customUnit?.distanceUnit), distanceRate.unit); | ||
| if (newDistanceUnit && newDistanceUnit !== transaction?.comment?.customUnit?.distanceUnit) { | ||
| newDistance = DistanceRequestUtils.convertDistanceUnit(getDistanceInMeters(transaction, transaction?.comment?.customUnit?.distanceUnit), newDistanceUnit); | ||
| } | ||
|
|
||
| Onyx.merge(`${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, { | ||
| comment: { | ||
| customUnit: { | ||
| customUnitRateID, | ||
| ...(!!policy && {defaultP2PRate: null}), | ||
| ...(distanceRate && {distanceUnit: distanceRate.unit}), | ||
| ...(newDistanceUnit && {distanceUnit: newDistanceUnit}), | ||
| ...(newDistance && {quantity: newDistance}), | ||
| }, | ||
| }, | ||
|
|
@@ -7529,7 +7566,7 @@ | |
| attendees: attendees ? JSON.stringify(attendees) : undefined, | ||
| currency, | ||
| comment, | ||
| distance, | ||
| distance: distance !== undefined ? roundToTwoDecimalPlaces(distance) : undefined, | ||
| created, | ||
| merchant, | ||
| iouReportID: iouReport?.reportID, | ||
|
|
@@ -8374,7 +8411,7 @@ | |
| createdIOUReportActionID, | ||
| reportPreviewReportActionID: reportPreviewAction.reportActionID, | ||
| waypoints: JSON.stringify(sanitizedWaypoints), | ||
| distance, | ||
| distance: distance !== undefined ? roundToTwoDecimalPlaces(distance) : undefined, | ||
| receipt, | ||
| odometerStart, | ||
| odometerEnd, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.