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 all 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
2 changes: 2 additions & 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 Expand Up @@ -2507,6 +2508,7 @@ const CONST = {
CATEGORY: 'category',
RECEIPT: 'receipt',
DISTANCE: 'distance',
DISTANCE_RATE: 'distanceRate',
TAG: 'tag',
TAX_RATE: 'taxRate',
TAX_AMOUNT: 'taxAmount',
Expand Down
16 changes: 8 additions & 8 deletions src/components/ReportActionItem/MoneyRequestView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ function MoneyRequestView({
const isReceiptBeingScanned = hasReceipt && TransactionUtils.isReceiptBeingScanned(transaction);
const didReceiptScanSucceed = hasReceipt && TransactionUtils.didReceiptScanSucceed(transaction);
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 +226,7 @@ function MoneyRequestView({
let amountDescription = `${translate('iou.amount')}`;

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

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

Expand Down Expand Up @@ -344,17 +345,16 @@ function MoneyRequestView({
}
/>
</OfflineWithFeedback>
{/* TODO: correct the pending field action https://github.com/Expensify/App/issues/36987 */}
<OfflineWithFeedback pendingAction={getPendingFieldAction('waypoints')}>
<OfflineWithFeedback pendingAction={getPendingFieldAction('customUnitRateID')}>
<MenuItemWithTopDescription
description={translate('common.rate')}
title={rateToDisplay}
// TODO: https://github.com/Expensify/App/issues/36987 make it interactive and show right icon when EditRatePage is ready
interactive={false}
shouldShowRightIcon={false}
interactive={canEditDistanceRate}
shouldShowRightIcon={canEditDistanceRate}
titleStyle={styles.flex1}
// TODO: https://github.com/Expensify/App/issues/36987 Add route for editing rate
onPress={() => {}}
onPress={() =>
Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_DISTANCE_RATE.getRoute(CONST.IOU.ACTION.EDIT, iouType, transaction?.transactionID ?? '-1', report?.reportID ?? '-1'))
}
/>
</OfflineWithFeedback>
</>
Expand Down
2 changes: 2 additions & 0 deletions src/libs/API/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ const WRITE_COMMANDS = {
UPDATE_MONEY_REQUEST_TAX_AMOUNT: 'UpdateMoneyRequestTaxAmount',
UPDATE_MONEY_REQUEST_TAX_RATE: 'UpdateMoneyRequestTaxRate',
UPDATE_MONEY_REQUEST_DISTANCE: 'UpdateMoneyRequestDistance',
UPDATE_MONEY_REQUEST_DISTANCE_RATE: 'UpdateMoneyRequestDistanceRate',
UPDATE_MONEY_REQUEST_CATEGORY: 'UpdateMoneyRequestCategory',
UPDATE_MONEY_REQUEST_DESCRIPTION: 'UpdateMoneyRequestDescription',
UPDATE_MONEY_REQUEST_AMOUNT_AND_CURRENCY: 'UpdateMoneyRequestAmountAndCurrency',
Expand Down Expand Up @@ -475,6 +476,7 @@ type WriteCommandParameters = {
[WRITE_COMMANDS.UPDATE_MONEY_REQUEST_TAX_AMOUNT]: Parameters.UpdateMoneyRequestParams;
[WRITE_COMMANDS.UPDATE_MONEY_REQUEST_TAX_RATE]: Parameters.UpdateMoneyRequestParams;
[WRITE_COMMANDS.UPDATE_MONEY_REQUEST_DISTANCE]: Parameters.UpdateMoneyRequestParams;
[WRITE_COMMANDS.UPDATE_MONEY_REQUEST_DISTANCE_RATE]: Parameters.UpdateMoneyRequestParams;
[WRITE_COMMANDS.UPDATE_MONEY_REQUEST_CATEGORY]: Parameters.UpdateMoneyRequestParams;
[WRITE_COMMANDS.UPDATE_MONEY_REQUEST_DESCRIPTION]: Parameters.UpdateMoneyRequestParams;
[WRITE_COMMANDS.HOLD_MONEY_REQUEST]: Parameters.HoldMoneyRequestParams;
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
1 change: 1 addition & 0 deletions src/libs/Navigation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,7 @@ type MoneyRequestNavigatorParamList = {
currency?: string;
};
[SCREENS.MONEY_REQUEST.STEP_DISTANCE_RATE]: {
action: IOUAction;
iouType: ValueOf<typeof CONST.IOU.TYPE>;
transactionID: string;
backTo: Routes;
Expand Down
21 changes: 21 additions & 0 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ type TransactionDetails = {
currency: string;
merchant: string;
waypoints?: WaypointCollection | string;
customUnitRateID?: string;
comment: string;
category: string;
billable: boolean;
Expand Down Expand Up @@ -2719,6 +2720,7 @@ function getTransactionDetails(transaction: OnyxInputOrEntry<Transaction>, creat
comment: TransactionUtils.getDescription(transaction),
merchant: TransactionUtils.getMerchant(transaction),
waypoints: TransactionUtils.getWaypoints(transaction),
customUnitRateID: TransactionUtils.getRateID(transaction),
category: TransactionUtils.getCategory(transaction),
billable: TransactionUtils.getBillable(transaction),
tag: TransactionUtils.getTag(transaction),
Expand Down Expand Up @@ -2813,6 +2815,7 @@ function canEditFieldOfMoneyRequest(reportAction: OnyxInputOrEntry<ReportAction>
CONST.EDIT_REQUEST_FIELD.DATE,
CONST.EDIT_REQUEST_FIELD.RECEIPT,
CONST.EDIT_REQUEST_FIELD.DISTANCE,
CONST.EDIT_REQUEST_FIELD.DISTANCE_RATE,
];

if (!ReportActionsUtils.isMoneyRequestAction(reportAction) || !canEditMoneyRequest(reportAction)) {
Expand Down Expand Up @@ -2852,6 +2855,11 @@ function canEditFieldOfMoneyRequest(reportAction: OnyxInputOrEntry<ReportAction>
return !isInvoiceReport(moneyRequestReport) && !TransactionUtils.isReceiptBeingScanned(transaction) && !TransactionUtils.isDistanceRequest(transaction) && isRequestor;
}

if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.DISTANCE_RATE) {
// The distance rate can be modified only on the distance expense reports
return isExpenseReport(moneyRequestReport) && TransactionUtils.isDistanceRequest(transaction);
}

return true;
}

Expand Down Expand Up @@ -3291,6 +3299,19 @@ function getModifiedExpenseOriginalMessage(
originalMessage.billable = transactionChanges?.billable ? Localize.translateLocal('common.billable').toLowerCase() : Localize.translateLocal('common.nonBillable').toLowerCase();
}

if ('customUnitRateID' in transactionChanges) {
originalMessage.oldAmount = TransactionUtils.getAmount(oldTransaction, isFromExpenseReport);
originalMessage.oldCurrency = TransactionUtils.getCurrency(oldTransaction);
originalMessage.oldMerchant = TransactionUtils.getMerchant(oldTransaction);

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
4 changes: 3 additions & 1 deletion src/libs/TransactionUtils/getDistanceInMeters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ import DistanceRequestUtils from '@libs/DistanceRequestUtils';
import type {OnyxInputOrEntry, Transaction} from '@src/types/onyx';
import type {Unit} from '@src/types/onyx/Policy';

// Get the distance in meters from the transaction.
// This function is placed in a separate file to avoid circular dependencies.
function getDistanceInMeters(transaction: OnyxInputOrEntry<Transaction>, unit: Unit | undefined) {
// If we are creating a new distance request, the distance is available in routes.route0.distance and it's already in meter.
// If we are creating a new distance request, the distance is available in routes.route0.distance and it's already in meters.
if (transaction?.routes?.route0?.distance) {
return transaction.routes.route0.distance;
}
Expand Down
69 changes: 66 additions & 3 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 @@ -240,6 +255,11 @@ function getUpdatedTransaction(transaction: Transaction, transactionChanges: Tra
shouldStopSmartscan = true;
}

if (Object.hasOwn(transactionChanges, 'customUnitRateID')) {
updatedTransaction.modifiedCustomUnitRateID = transactionChanges.customUnitRateID;
neil-marcellini marked this conversation as resolved.
Show resolved Hide resolved
shouldStopSmartscan = true;
}

if (Object.hasOwn(transactionChanges, 'taxAmount') && typeof transactionChanges.taxAmount === 'number') {
updatedTransaction.taxAmount = isFromExpenseReport ? -transactionChanges.taxAmount : transactionChanges.taxAmount;
}
Expand Down Expand Up @@ -724,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 All @@ -743,10 +805,10 @@ function hasReservationList(transaction: Transaction | undefined | null): boolea
}

/**
* Get rate ID from the transaction object
* Get custom unit rate (distance rate) ID from the transaction object
*/
function getRateID(transaction: OnyxInputOrEntry<Transaction>): string | undefined {
return transaction?.comment?.customUnit?.customUnitRateID?.toString();
return transaction?.modifiedCustomUnitRateID ?? transaction?.comment?.customUnit?.customUnitRateID?.toString();
}

/**
Expand Down Expand Up @@ -980,6 +1042,7 @@ function buildTransactionsMergeParams(reviewDuplicates: OnyxEntry<ReviewDuplicat
export {
buildOptimisticTransaction,
calculateTaxAmount,
calculateAmountForUpdatedWaypointOrRate,
getWorkspaceTaxesSettingsName,
getDefaultTaxCode,
transformedTaxRates,
Expand Down
Loading
Loading