Skip to content
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

feat: Edit Distance Rate flow #40021

Merged
merged 26 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
a3ea866
Init: Edit Rate flow
paultsimura Apr 10, 2024
d514b30
Merge branch 'fork/feat/36985-create-new-rate-field' into feat/36987-…
paultsimura Apr 11, 2024
967921b
Rate edit: initial commit
paultsimura Apr 12, 2024
2c07b55
Merge branch 'main' into feat/36987-rate-edit
paultsimura Apr 13, 2024
ed1f907
Merge branch 'fork/feat/36985-create-new-rate-field' into feat/36987-…
paultsimura Apr 13, 2024
180d885
Merge branch 'main' into feat/36987-rate-edit
paultsimura Apr 22, 2024
1158ffd
Merge branch 'main' into feat/36987-rate-edit
paultsimura Jun 30, 2024
21054fe
Some progress with edit rate
paultsimura Jul 2, 2024
2223b04
Merge branch 'refs/heads/main' into feat/36987-rate-edit
paultsimura Jul 16, 2024
8167592
Almost final changes for Rate Edit flow
paultsimura Jul 19, 2024
5cd792a
Use pending field indicator
paultsimura Jul 19, 2024
de51398
Merge branch 'refs/heads/main' into feat/36987-rate-edit
paultsimura Jul 19, 2024
05d522e
Move getDistanceInMeters to DistanceRequestUtils to avoid circular deps
paultsimura Jul 20, 2024
03bd854
Fix type
paultsimura Jul 20, 2024
b7fa71e
Lint
paultsimura Jul 20, 2024
42e2db9
Merge branch 'Expensify:main' into feat/36987-rate-edit
paultsimura Jul 30, 2024
a2ad2db
Move getDistanceInMeters into TransactionUtils
paultsimura Jul 30, 2024
b5544b1
Lint
paultsimura Jul 30, 2024
db7639a
Fix file name
paultsimura Jul 30, 2024
fe0e98e
Lint
paultsimura Jul 30, 2024
867e85e
Merge branch 'Expensify:main' into feat/36987-rate-edit
paultsimura Jul 31, 2024
6ccacb8
Always show wrapper for the rate page
paultsimura Jul 31, 2024
ebdbe0e
Merge branch 'refs/heads/main' into feat/36987-rate-edit
paultsimura Aug 2, 2024
65a07ff
Edit Rate: code cleanup
paultsimura Aug 3, 2024
2029ec5
Merge branch 'refs/heads/main' into feat/36987-rate-edit
paultsimura Aug 3, 2024
63865a3
Edit Rate: code cleanup
paultsimura Aug 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type {Unit} from './types/onyx/Policy';
type RateAndUnit = {
unit: Unit;
rate: number;
currency: string;
Copy link
Contributor Author

@paultsimura paultsimura Aug 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this so the RateAndUnit is more consistent with the MileageRate and we could use the currency from their union type: MileageRate | RateAndUnit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB: IMO let's delete this type and just use MileageRate, since RateAndUnit is a subset of it, and the name is not great and now inaccurate. It can be cleaned up in a follow up.

};
type CurrencyDefaultMileageRate = Record<string, RateAndUnit>;

Expand Down
6 changes: 3 additions & 3 deletions src/components/ReportActionItem/MoneyRequestView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ function MoneyRequestView({
const hasReceipt = TransactionUtils.hasReceipt(transaction);
const isReceiptBeingScanned = hasReceipt && TransactionUtils.isReceiptBeingScanned(transaction);
const didReceiptScanSucceed = hasReceipt && TransactionUtils.didReceiptScanSucceed(transaction);
const canEditDistance = ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, CONST.EDIT_REQUEST_FIELD.DISTANCE);
const canEditDistanceRate = ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, CONST.EDIT_REQUEST_FIELD.DISTANCE_RATE);
const canEditDistance = canUserPerformWriteAction && ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, CONST.EDIT_REQUEST_FIELD.DISTANCE);
const canEditDistanceRate = canUserPerformWriteAction && ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, CONST.EDIT_REQUEST_FIELD.DISTANCE_RATE);

const isAdmin = policy?.role === 'admin';
const isApprover = ReportUtils.isMoneyRequestReport(moneyRequestReport) && moneyRequestReport?.managerID !== null && session?.accountID === moneyRequestReport?.managerID;
Expand Down Expand Up @@ -225,7 +225,7 @@ function MoneyRequestView({

let amountDescription = `${translate('iou.amount')}`;

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

const currency = policy ? policy.outputCurrency : PolicyUtils.getPersonalPolicy()?.outputCurrency ?? CONST.CURRENCY.USD;
Expand Down
6 changes: 5 additions & 1 deletion src/libs/DistanceRequestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,11 @@ function getDistanceMerchant(
* @returns The rate and unit in RateAndUnit object.
*/
function getRateForP2P(currency: string): RateAndUnit {
return CONST.CURRENCY_TO_DEFAULT_MILEAGE_RATE[currency] ?? CONST.CURRENCY_TO_DEFAULT_MILEAGE_RATE.USD;
const currencyWithExistingRate = CONST.CURRENCY_TO_DEFAULT_MILEAGE_RATE[currency] ? currency : CONST.CURRENCY.USD;
return {
...CONST.CURRENCY_TO_DEFAULT_MILEAGE_RATE[currencyWithExistingRate],
currency: currencyWithExistingRate,
};
}

/**
Expand Down
13 changes: 7 additions & 6 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3237,7 +3237,6 @@ function getReportPreviewMessage(
*/
function getModifiedExpenseOriginalMessage(
oldTransaction: OnyxInputOrEntry<Transaction>,
updatedTransaction: OnyxInputOrEntry<Transaction>,
transactionChanges: TransactionChanges,
isFromExpenseReport: boolean,
policy: OnyxInputOrEntry<Policy>,
Expand Down Expand Up @@ -3305,9 +3304,12 @@ function getModifiedExpenseOriginalMessage(
originalMessage.oldCurrency = TransactionUtils.getCurrency(oldTransaction);
originalMessage.oldMerchant = TransactionUtils.getMerchant(oldTransaction);

originalMessage.amount = TransactionUtils.getAmount(updatedTransaction, isFromExpenseReport);
originalMessage.currency = TransactionUtils.getCurrency(updatedTransaction);
originalMessage.merchant = TransactionUtils.getMerchant(updatedTransaction);
const modifiedData = TransactionUtils.calculateAmountForUpdatedWaypointOrRate(oldTransaction, transactionChanges, policy, isFromExpenseReport);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB: Maybe call this modifiedDistanceFields or something similarly descriptive?


// For the originalMessage, we should use the non-negative amount, similar to what TransactionUtils.getAmount does for oldAmount
originalMessage.amount = Math.abs(modifiedData.modifiedAmount);
originalMessage.currency = modifiedData.modifiedCurrency;
originalMessage.merchant = modifiedData.modifiedMerchant;
}

return originalMessage;
Expand Down Expand Up @@ -4585,12 +4587,11 @@ function buildOptimisticActionableTrackExpenseWhisper(iouAction: OptimisticIOURe
function buildOptimisticModifiedExpenseReportAction(
transactionThread: OnyxInputOrEntry<Report>,
oldTransaction: OnyxInputOrEntry<Transaction>,
updatedTransaction: OnyxInputOrEntry<Transaction>,
transactionChanges: TransactionChanges,
isFromExpenseReport: boolean,
policy: OnyxInputOrEntry<Policy>,
): OptimisticModifiedExpenseReportAction {
const originalMessage = getModifiedExpenseOriginalMessage(oldTransaction, updatedTransaction, transactionChanges, isFromExpenseReport, policy);
const originalMessage = getModifiedExpenseOriginalMessage(oldTransaction, transactionChanges, isFromExpenseReport, policy);
return {
actionName: CONST.REPORT.ACTIONS.TYPE.MODIFIED_EXPENSE,
actorAccountID: currentUserAccountID,
Expand Down
61 changes: 59 additions & 2 deletions src/libs/TransactionUtils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ 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 getDistanceInMeters from './getDistanceInMeters';
import DistanceRequestUtils from '@libs/DistanceRequestUtils';
import * as PolicyUtils from '@libs/PolicyUtils';
import {toLocaleDigit} from '@libs/LocaleDigitUtils';
import type DeepValueOf from '@src/types/utils/DeepValueOf';

let allTransactions: OnyxCollection<Transaction> = {};
Onyx.connect({
Expand All @@ -41,6 +45,17 @@ Onyx.connect({
callback: (value) => (allTransactionViolations = value),
});

let preferredLocale: DeepValueOf<typeof CONST.LOCALES> = CONST.LOCALES.DEFAULT;
Onyx.connect({
key: ONYXKEYS.NVP_PREFERRED_LOCALE,
callback: (value) => {
if (!value) {
return;
}
preferredLocale = value;
},
});

let currentUserEmail = '';
let currentUserAccountID = -1;
Onyx.connect({
Expand Down Expand Up @@ -203,7 +218,7 @@ function areRequiredFieldsEmpty(transaction: OnyxEntry<Transaction>): boolean {
}

/**
* Given the edit made to the expnse, return an updated transaction object.
* Given the edit made to the expense, return an updated transaction object.
*/
function getUpdatedTransaction(transaction: Transaction, transactionChanges: TransactionChanges, isFromExpenseReport: boolean, shouldUpdateReceiptState = true): Transaction {
// Only changing the first level fields so no need for deep clone now
Expand Down Expand Up @@ -729,6 +744,48 @@ function calculateTaxAmount(percentage: string, amount: number, currency: string
return parseFloat(taxAmount.toFixed(decimals));
}

/**
* Calculates updated amount, currency, and merchant for a distance request with modified waypoints or customUnitRateID
*/
function calculateAmountForUpdatedWaypointOrRate(
transaction: OnyxInputOrEntry<Transaction>,
transactionChanges: TransactionChanges,
policy: OnyxInputOrEntry<Policy>,
isFromExpenseReport: boolean,
) {
if (isEmptyObject(transactionChanges?.routes?.route0?.geometry) && isEmptyObject(transactionChanges.customUnitRateID)) {
return {
amount: CONST.IOU.DEFAULT_AMOUNT,
modifiedAmount: CONST.IOU.DEFAULT_AMOUNT,
modifiedMerchant: Localize.translateLocal('iou.fieldPending'),
modifiedCurrency: Localize.translateLocal('iou.fieldPending'),
};
}

const customUnitRateID = transactionChanges.customUnitRateID ?? getRateID(transaction) ?? '';
const mileageRates = DistanceRequestUtils.getMileageRates(policy, true);
const policyCurrency = policy?.outputCurrency ?? PolicyUtils.getPersonalPolicy()?.outputCurrency ?? CONST.CURRENCY.USD;
const mileageRate = isCustomUnitRateIDForP2P(transaction)
? DistanceRequestUtils.getRateForP2P(policyCurrency)
: mileageRates?.[customUnitRateID] ?? DistanceRequestUtils.getDefaultMileageRate(policy);
const {unit, rate, currency} = mileageRate;

const distanceInMeters = getDistanceInMeters(transaction, unit);
const amount = DistanceRequestUtils.getDistanceRequestAmount(distanceInMeters, unit, rate ?? 0);
const updatedAmount = isFromExpenseReport ? -amount : amount;
const updatedCurrency = currency ?? CONST.CURRENCY.USD;
const updatedMerchant = DistanceRequestUtils.getDistanceMerchant(true, distanceInMeters, unit, rate, updatedCurrency, Localize.translateLocal, (digit) =>
toLocaleDigit(preferredLocale, digit),
);

return {
amount: updatedAmount,
modifiedAmount: updatedAmount,
modifiedMerchant: updatedMerchant,
modifiedCurrency: updatedCurrency,
};
}

/**
* Calculates count of all tax enabled options
*/
Expand Down Expand Up @@ -985,6 +1042,7 @@ function buildTransactionsMergeParams(reviewDuplicates: OnyxEntry<ReviewDuplicat
export {
buildOptimisticTransaction,
calculateTaxAmount,
calculateAmountForUpdatedWaypointOrRate,
getWorkspaceTaxesSettingsName,
getDefaultTaxCode,
transformedTaxRates,
Expand All @@ -1000,7 +1058,6 @@ export {
getTaxAmount,
getTaxCode,
getCurrency,
getDistance,
getDistanceInMeters,
getCardID,
getOriginalCurrency,
Expand Down
48 changes: 5 additions & 43 deletions src/libs/actions/IOU.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2481,44 +2481,6 @@ function calculateDiffAmount(
return 0;
}

function calculateAmountForUpdatedWaypointOrRate(
transaction: OnyxTypes.OnyxInputOrEntry<OnyxTypes.Transaction>,
transactionChanges: TransactionChanges,
policy: OnyxTypes.OnyxInputOrEntry<OnyxTypes.Policy>,
iouReport: OnyxTypes.OnyxInputOrEntry<OnyxTypes.Report>,
) {
if (isEmptyObject(transactionChanges?.routes?.route0?.geometry) && isEmptyObject(transactionChanges.customUnitRateID)) {
return {
amount: CONST.IOU.DEFAULT_AMOUNT,
modifiedAmount: CONST.IOU.DEFAULT_AMOUNT,
modifiedMerchant: Localize.translateLocal('iou.fieldPending'),
};
}

const customUnitRateID = !isEmptyObject(transactionChanges.customUnitRateID) ? transactionChanges.customUnitRateID : TransactionUtils.getRateID(transaction) ?? '';

const mileageRates = DistanceRequestUtils.getMileageRates(policy, true);
const policyCurrency = policy?.outputCurrency ?? PolicyUtils.getPersonalPolicy()?.outputCurrency ?? CONST.CURRENCY.USD;
const mileageRate = TransactionUtils.isCustomUnitRateIDForP2P(transaction)
? DistanceRequestUtils.getRateForP2P(policyCurrency)
: mileageRates?.[customUnitRateID] ?? DistanceRequestUtils.getDefaultMileageRate(policy);
const {unit, rate} = mileageRate;

const distanceInMeters = TransactionUtils.getDistanceInMeters(transaction, unit);
const amount = DistanceRequestUtils.getDistanceRequestAmount(distanceInMeters, unit, rate ?? 0);
const updatedAmount = ReportUtils.isExpenseReport(iouReport) ? -amount : amount;
const updatedMerchant = DistanceRequestUtils.getDistanceMerchant(true, distanceInMeters, unit, rate, transaction?.currency ?? CONST.CURRENCY.USD, Localize.translateLocal, (digit) =>
toLocaleDigit(preferredLocale, digit),
);

return {
amount: updatedAmount,
modifiedAmount: updatedAmount,
modifiedMerchant: updatedMerchant,
modifiedCurrency: policyCurrency,
};
}

/**
* @param transactionID
* @param transactionThreadReportID
Expand Down Expand Up @@ -2580,7 +2542,7 @@ function getUpdateMoneyRequestParams(
if (transaction && updatedTransaction && (hasPendingWaypoints || hasModifiedDistanceRate)) {
updatedTransaction = {
...updatedTransaction,
...calculateAmountForUpdatedWaypointOrRate(transaction, transactionChanges, policy, iouReport),
...TransactionUtils.calculateAmountForUpdatedWaypointOrRate(transaction, transactionChanges, policy, ReportUtils.isExpenseReport(iouReport)),
};

// Delete the draft transaction when editing waypoints when the server responds successfully and there are no errors
Expand All @@ -2607,7 +2569,7 @@ function getUpdateMoneyRequestParams(
// We don't create a modified report action if we're updating the waypoints,
// since there isn't actually any optimistic data we can create for them and the report action is created on the server
// with the response from the MapBox API
const updatedReportAction = ReportUtils.buildOptimisticModifiedExpenseReportAction(transactionThread, transaction, updatedTransaction, transactionChanges, isFromExpenseReport, policy);
const updatedReportAction = ReportUtils.buildOptimisticModifiedExpenseReportAction(transactionThread, transaction, transactionChanges, isFromExpenseReport, policy);
if (!hasPendingWaypoints) {
params.reportActionID = updatedReportAction.reportActionID;

Expand Down Expand Up @@ -2886,7 +2848,7 @@ function getUpdateTrackExpenseParams(
if (transaction && updatedTransaction && (hasPendingWaypoints || hasModifiedDistanceRate)) {
updatedTransaction = {
...updatedTransaction,
...calculateAmountForUpdatedWaypointOrRate(transaction, transactionChanges, policy, transactionThread),
...TransactionUtils.calculateAmountForUpdatedWaypointOrRate(transaction, transactionChanges, policy, ReportUtils.isExpenseReport(transactionThread)),
};

// Delete the draft transaction when editing waypoints when the server responds successfully and there are no errors
Expand All @@ -2913,7 +2875,7 @@ function getUpdateTrackExpenseParams(
// We don't create a modified report action if we're updating the waypoints,
// since there isn't actually any optimistic data we can create for them and the report action is created on the server
// with the response from the MapBox API
const updatedReportAction = ReportUtils.buildOptimisticModifiedExpenseReportAction(transactionThread, transaction, updatedTransaction, transactionChanges, false, policy);
const updatedReportAction = ReportUtils.buildOptimisticModifiedExpenseReportAction(transactionThread, transaction, transactionChanges, false, policy);
if (!hasPendingWaypoints) {
params.reportActionID = updatedReportAction.reportActionID;

Expand Down Expand Up @@ -5084,8 +5046,8 @@ function editRegularMoneyRequest(
const isFromExpenseReport = ReportUtils.isExpenseReport(iouReport);

// STEP 2: Build new modified expense report action.
const updatedReportAction = ReportUtils.buildOptimisticModifiedExpenseReportAction(transactionThread, transaction, transactionChanges, isFromExpenseReport, policy);
const updatedTransaction = transaction ? TransactionUtils.getUpdatedTransaction(transaction, transactionChanges, isFromExpenseReport) : null;
const updatedReportAction = ReportUtils.buildOptimisticModifiedExpenseReportAction(transactionThread, transaction, updatedTransaction, transactionChanges, isFromExpenseReport, policy);

// STEP 3: Compute the IOU total and update the report preview message so LHN amount owed is correct
// Should only update if the transaction matches the currency of the report, else we wait for the update
Expand Down
Loading