Skip to content

Commit

Permalink
Merge pull request #50001 from Expensify/neil-distanceUnit
Browse files Browse the repository at this point in the history
Use transaction distanceUnit to prevent retroactive changes
  • Loading branch information
neil-marcellini authored Oct 17, 2024
2 parents 7317bd6 + f0ec762 commit 20b7017
Show file tree
Hide file tree
Showing 10 changed files with 137 additions and 70 deletions.
14 changes: 4 additions & 10 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,11 @@ import Config from 'react-native-config';
import * as KeyCommand from 'react-native-key-command';
import type {ValueOf} from 'type-fest';
import type {Video} from './libs/actions/Report';
import type {MileageRate} from './libs/DistanceRequestUtils';
import BankAccount from './libs/models/BankAccount';
import * as Url from './libs/Url';
import SCREENS from './SCREENS';
import type PlaidBankAccount from './types/onyx/PlaidBankAccount';
import type {Unit} from './types/onyx/Policy';

type RateAndUnit = {
unit: Unit;
rate: number;
currency: string;
};
type CurrencyDefaultMileageRate = Record<string, RateAndUnit>;

// Creating a default array and object this way because objects ({}) and arrays ([]) are not stable types.
// Freezing the array ensures that it cannot be unintentionally modified.
Expand Down Expand Up @@ -2466,6 +2459,8 @@ const CONST = {
DEFAULT_RATE: 'Default Rate',
RATE_DECIMALS: 3,
FAKE_P2P_ID: '_FAKE_P2P_ID_',
MILES_TO_KILOMETERS: 1.609344,
KILOMETERS_TO_MILES: 0.621371,
},

TERMS: {
Expand Down Expand Up @@ -5525,7 +5520,7 @@ const CONST = {
"rate": 2377,
"unit": "km"
}
}`) as CurrencyDefaultMileageRate,
}`) as Record<string, MileageRate>,

EXIT_SURVEY: {
REASONS: {
Expand Down Expand Up @@ -5919,7 +5914,6 @@ export type {
Country,
IOUAction,
IOUType,
RateAndUnit,
OnboardingPurposeType,
OnboardingCompanySizeType,
IOURequestType,
Expand Down
27 changes: 5 additions & 22 deletions src/components/MoneyRequestConfirmationList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {useFocusEffect, useIsFocused} from '@react-navigation/native';
import lodashIsEqual from 'lodash/isEqual';
import React, {memo, useCallback, useEffect, useMemo, useRef, useState} from 'react';
import {InteractionManager, View} from 'react-native';
import {useOnyx, withOnyx} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import useDebouncedState from '@hooks/useDebouncedState';
Expand Down Expand Up @@ -37,7 +37,6 @@ import type * as OnyxTypes from '@src/types/onyx';
import type {Participant} from '@src/types/onyx/IOU';
import type {PaymentMethodType} from '@src/types/onyx/OriginalMessage';
import type {SplitShares} from '@src/types/onyx/Transaction';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import ButtonWithDropdownMenu from './ButtonWithDropdownMenu';
import type {DropdownOption} from './ButtonWithDropdownMenu/types';
import FormHelpMessage from './FormHelpMessage';
Expand Down Expand Up @@ -66,9 +65,6 @@ type MoneyRequestConfirmationListOnyxProps = {
/** The draft policy of the report */
policyDraft: OnyxEntry<OnyxTypes.Policy>;

/** Unit and rate used for if the expense is a distance expense */
mileageRates: OnyxEntry<Record<string, MileageRate>>;

/** Mileage rate default for the policy */
defaultMileageRate: OnyxEntry<MileageRate>;

Expand Down Expand Up @@ -181,7 +177,6 @@ function MoneyRequestConfirmationList({
iouAmount,
policyCategories: policyCategoriesReal,
policyCategoriesDraft,
mileageRates: mileageRatesReal,
isDistanceRequest = false,
policy: policyReal,
policyDraft,
Expand Down Expand Up @@ -216,10 +211,6 @@ function MoneyRequestConfirmationList({
}: MoneyRequestConfirmationListProps) {
const policy = policyReal ?? policyDraft;
const policyCategories = policyCategoriesReal ?? policyCategoriesDraft;
const [mileageRatesDraft] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${policyID || '-1'}`, {
selector: (selectedPolicy: OnyxEntry<OnyxTypes.Policy>) => DistanceRequestUtils.getMileageRates(selectedPolicy),
});
const mileageRates = isEmptyObject(mileageRatesReal) ? mileageRatesDraft : mileageRatesReal;

const styles = useThemeStyles();
const {translate, toLocaleDigit} = useLocalize();
Expand Down Expand Up @@ -247,15 +238,11 @@ function MoneyRequestConfirmationList({
IOU.setCustomUnitRateID(transactionID, rateID);
}, [defaultMileageRate, customUnitRateID, lastSelectedDistanceRates, policy?.id, canUseP2PDistanceRequests, transactionID, isDistanceRequest]);

const policyCurrency = policy?.outputCurrency ?? PolicyUtils.getPersonalPolicy()?.outputCurrency ?? CONST.CURRENCY.USD;

const mileageRate = TransactionUtils.isCustomUnitRateIDForP2P(transaction) ? DistanceRequestUtils.getRateForP2P(policyCurrency) : mileageRates?.[customUnitRateID] ?? defaultMileageRate;

const {unit, rate} = mileageRate ?? {};

const mileageRate = DistanceRequestUtils.getRate({transaction, policy, policyDraft});
const rate = mileageRate.rate;
const prevRate = usePrevious(rate);

const currency = (mileageRate as MileageRate)?.currency ?? policyCurrency;
const unit = mileageRate.unit;
const currency = mileageRate.currency ?? CONST.CURRENCY.USD;
const prevCurrency = usePrevious(currency);

// A flag for showing the categories field
Expand Down Expand Up @@ -990,10 +977,6 @@ export default withOnyx<MoneyRequestConfirmationListProps, MoneyRequestConfirmat
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
selector: DistanceRequestUtils.getDefaultMileageRate,
},
mileageRates: {
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
selector: (policy: OnyxEntry<OnyxTypes.Policy>) => DistanceRequestUtils.getMileageRates(policy),
},
policy: {
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
},
Expand Down
12 changes: 1 addition & 11 deletions src/components/ReportActionItem/MoneyRequestView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,6 @@ function MoneyRequestView({report, shouldShowAnimatedBackground, readonly = fals
const [parentReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID}`, {
canEvict: false,
});
const [distanceRates = {}] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {
selector: () => DistanceRequestUtils.getMileageRates(policy, true),
});
const [transactionViolations] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${getTransactionID(report, parentReportActions)}`);

const parentReportAction = parentReportActions?.[report?.parentReportActionID ?? '-1'];
Expand Down Expand Up @@ -202,14 +199,7 @@ function MoneyRequestView({report, shouldShowAnimatedBackground, readonly = fals
let amountDescription = `${translate('iou.amount')}`;

const hasRoute = TransactionUtils.hasRoute(transactionBackup ?? transaction, isDistanceRequest);
const rateID = TransactionUtils.getRateID(transaction) ?? '-1';

const currency = transactionCurrency ?? CONST.CURRENCY.USD;

const mileageRate = TransactionUtils.isCustomUnitRateIDForP2P(transaction) ? DistanceRequestUtils.getRateForP2P(currency) : distanceRates[rateID] ?? {};
const {unit} = mileageRate;
const rate = transaction?.comment?.customUnit?.defaultP2PRate ?? mileageRate.rate;

const {unit, rate, currency} = DistanceRequestUtils.getRate({transaction, policy});
const distance = TransactionUtils.getDistanceInMeters(transactionBackup ?? transaction, unit);
const rateToDisplay = DistanceRequestUtils.getRateForDisplay(unit, rate, currency, translate, toLocaleDigit, isOffline);
const distanceToDisplay = DistanceRequestUtils.getDistanceForDisplay(hasRoute, distance, unit, rate, translate);
Expand Down
72 changes: 67 additions & 5 deletions src/libs/DistanceRequestUtils.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import type {OnyxEntry} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import type {LocaleContextProps} from '@components/LocaleContextProvider';
import type {RateAndUnit} from '@src/CONST';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {LastSelectedDistanceRates, OnyxInputOrEntry} from '@src/types/onyx';
import type {LastSelectedDistanceRates, OnyxInputOrEntry, Transaction} from '@src/types/onyx';
import type {Unit} from '@src/types/onyx/Policy';
import type Policy from '@src/types/onyx/Policy';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import * as CurrencyUtils from './CurrencyUtils';
import * as PolicyUtils from './PolicyUtils';
import * as ReportConnection from './ReportConnection';
import * as ReportUtils from './ReportUtils';
import * as TransactionUtils from './TransactionUtils';

type MileageRate = {
customUnitRateID?: string;
Expand Down Expand Up @@ -219,17 +219,30 @@ function getDistanceMerchant(
return `${distanceInUnits} @ ${ratePerUnit}`;
}

function ensureRateDefined(rate: number | undefined): asserts rate is number {
if (rate !== undefined) {
return;
}
throw new Error('All default P2P rates should have a rate defined');
}

/**
* Retrieves the rate and unit for a P2P distance expense for a given currency.
*
* @param currency
* @returns The rate and unit in RateAndUnit object.
* @returns The rate and unit in MileageRate object.
*/
function getRateForP2P(currency: string): RateAndUnit {
function getRateForP2P(currency: string, transaction: OnyxEntry<Transaction>): MileageRate {
const currencyWithExistingRate = CONST.CURRENCY_TO_DEFAULT_MILEAGE_RATE[currency] ? currency : CONST.CURRENCY.USD;
const mileageRate = CONST.CURRENCY_TO_DEFAULT_MILEAGE_RATE[currencyWithExistingRate];
ensureRateDefined(mileageRate.rate);

// Ensure the rate is updated when the currency changes, otherwise use the stored rate
const rate = TransactionUtils.getCurrency(transaction) === currency ? transaction?.comment?.customUnit?.defaultP2PRate ?? mileageRate.rate : mileageRate.rate;
return {
...CONST.CURRENCY_TO_DEFAULT_MILEAGE_RATE[currencyWithExistingRate],
...mileageRate,
currency: currencyWithExistingRate,
rate,
};
}

Expand Down Expand Up @@ -301,6 +314,52 @@ function getTaxableAmount(policy: OnyxEntry<Policy>, customUnitRateID: string, d
return amount * taxClaimablePercentage;
}

function getDistanceUnit(transaction: OnyxEntry<Transaction>, mileageRate: OnyxEntry<MileageRate>): Unit {
return transaction?.comment?.customUnit?.distanceUnit ?? mileageRate?.unit ?? CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES;
}

/**
* Get the selected rate for a transaction, from the policy or P2P default rate.
* Use the distanceUnit stored on the transaction by default to prevent policy changes modifying existing transactions. Otherwise, get the unit from the rate.
*/
function getRate({
transaction,
policy,
policyDraft,
useTransactionDistanceUnit = true,
}: {
transaction: OnyxEntry<Transaction>;
policy: OnyxEntry<Policy>;
policyDraft?: OnyxEntry<Policy>;
useTransactionDistanceUnit?: boolean;
}): MileageRate {
let mileageRates = getMileageRates(policy, true, transaction?.comment?.customUnit?.customUnitRateID);
if (isEmptyObject(mileageRates) && policyDraft) {
mileageRates = getMileageRates(policyDraft, true, transaction?.comment?.customUnit?.customUnitRateID);
}
const policyCurrency = policy?.outputCurrency ?? PolicyUtils.getPersonalPolicy()?.outputCurrency ?? CONST.CURRENCY.USD;
const defaultMileageRate = getDefaultMileageRate(policy);
const customUnitRateID = TransactionUtils.getRateID(transaction) ?? '';
const mileageRate = TransactionUtils.isCustomUnitRateIDForP2P(transaction) ? getRateForP2P(policyCurrency, transaction) : mileageRates?.[customUnitRateID] ?? defaultMileageRate;
const unit = getDistanceUnit(useTransactionDistanceUnit ? transaction : undefined, mileageRate);
return {
...mileageRate,
unit,
currency: mileageRate?.currency ?? policyCurrency,
};
}

/**
* Get the updated distance unit from the selected rate instead of the distanceUnit stored on the transaction.
* Useful for updating the transaction distance unit when the distance or rate changes.
*
* For example, if an expense is '10 mi @ $1.00 / mi' and the rate is updated to '$1.00 / km',
* then the updated distance unit should be 'km' from the updated rate, not 'mi' from the currently stored transaction distance unit.
*/
function getUpdatedDistanceUnit({transaction, policy, policyDraft}: {transaction: OnyxEntry<Transaction>; policy: OnyxEntry<Policy>; policyDraft?: OnyxEntry<Policy>}) {
return getRate({transaction, policy, policyDraft, useTransactionDistanceUnit: false}).unit;
}

export default {
getDefaultMileageRate,
getDistanceMerchant,
Expand All @@ -312,6 +371,9 @@ export default {
getCustomUnitRateID,
convertToDistanceInMeters,
getTaxableAmount,
getDistanceUnit,
getUpdatedDistanceUnit,
getRate,
};

export type {MileageRate};
38 changes: 29 additions & 9 deletions src/libs/TransactionUtils/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import lodashHas from 'lodash/has';
import lodashIsEqual from 'lodash/isEqual';
import lodashSet from 'lodash/set';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
Expand Down Expand Up @@ -139,6 +140,8 @@ function buildOptimisticTransaction(
billable = false,
pendingFields: Partial<{[K in TransactionPendingFieldsKey]: ValueOf<typeof CONST.RED_BRICK_ROAD_PENDING_ACTION>}> | undefined = undefined,
reimbursable = true,
existingTransaction: OnyxEntry<Transaction> | undefined = undefined,
policy: OnyxEntry<Policy> = undefined,
): Transaction {
// transactionIDs are random, positive, 64-bit numeric strings.
// Because JS can only handle 53-bit numbers, transactionIDs are strings in the front-end (just like reportActionID)
Expand All @@ -152,6 +155,12 @@ function buildOptimisticTransaction(
commentJSON.originalTransactionID = originalTransactionID;
}

const isDistanceTransaction = !!pendingFields?.waypoints;
if (isDistanceTransaction) {
// Set the distance unit, which comes from the policy distance unit or the P2P rate data
lodashSet(commentJSON, 'customUnit.distanceUnit', DistanceRequestUtils.getUpdatedDistanceUnit({transaction: existingTransaction, policy}));
}

return {
...(!isEmptyObject(pendingFields) ? {pendingFields} : {}),
transactionID,
Expand Down Expand Up @@ -261,15 +270,26 @@ function getUpdatedTransaction(transaction: Transaction, transactionChanges: Tra
}

if (Object.hasOwn(transactionChanges, 'customUnitRateID')) {
updatedTransaction.comment = {
...(updatedTransaction?.comment ?? {}),
customUnit: {
...updatedTransaction?.comment?.customUnit,
customUnitRateID: transactionChanges.customUnitRateID,
defaultP2PRate: null,
},
};
lodashSet(updatedTransaction, 'comment.customUnit.customUnitRateID', transactionChanges.customUnitRateID);
lodashSet(updatedTransaction, 'comment.customUnit.defaultP2PRate', null);
shouldStopSmartscan = true;

const existingDistanceUnit = transaction?.comment?.customUnit?.distanceUnit;
const allReports = ReportConnection.getAllReports();
const report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transaction.reportID}`] ?? null;
const policyID = report?.policyID ?? '';
const policy = PolicyUtils.getPolicy(policyID);

// Get the new distance unit from the rate's unit
const newDistanceUnit = DistanceRequestUtils.getUpdatedDistanceUnit({transaction: updatedTransaction, policy});

// If the distanceUnit is set and the rate is changed to one that has a different unit, convert the distance to the new unit
if (existingDistanceUnit && newDistanceUnit !== existingDistanceUnit) {
const conversionFactor = existingDistanceUnit === CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES ? CONST.CUSTOM_UNITS.MILES_TO_KILOMETERS : CONST.CUSTOM_UNITS.KILOMETERS_TO_MILES;
const distance = NumberUtils.roundToTwoDecimalPlaces((transaction?.comment?.customUnit?.quantity ?? 0) * conversionFactor);
lodashSet(updatedTransaction, 'comment.customUnit.quantity', distance);
lodashSet(updatedTransaction, 'pendingFields.waypoints', CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE);
}
}

if (Object.hasOwn(transactionChanges, 'taxAmount') && typeof transactionChanges.taxAmount === 'number') {
Expand Down Expand Up @@ -823,7 +843,7 @@ function calculateAmountForUpdatedWaypointOrRate(
const mileageRates = DistanceRequestUtils.getMileageRates(policy, true);
const policyCurrency = policy?.outputCurrency ?? PolicyUtils.getPersonalPolicy()?.outputCurrency ?? CONST.CURRENCY.USD;
const mileageRate = isCustomUnitRateIDForP2P(transaction)
? DistanceRequestUtils.getRateForP2P(policyCurrency)
? DistanceRequestUtils.getRateForP2P(policyCurrency, transaction ?? undefined)
: mileageRates?.[customUnitRateID] ?? DistanceRequestUtils.getDefaultMileageRate(policy);
const {unit, rate, currency} = mileageRate;

Expand Down
Loading

0 comments on commit 20b7017

Please sign in to comment.