Skip to content

Commit

Permalink
Merge pull request Expensify#39610 from eVoloshchak/revert-35797-pres…
Browse files Browse the repository at this point in the history
…erving-the-transactions-amounts-the-same-as-the-user-entered

Revert "35797-preserving-the-transactions-amounts-the-same-as-entered"
  • Loading branch information
iwiznia authored Apr 4, 2024
2 parents 819df53 + 789beb2 commit a1f9cec
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 68 deletions.
3 changes: 0 additions & 3 deletions src/components/transactionPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ export default PropTypes.shape({
/** The original transaction amount */
amount: PropTypes.number,

/** Whether the original input should be shown */
shouldShowOriginalAmount: PropTypes.bool,

/** The edited transaction amount */
modifiedAmount: PropTypes.number,

Expand Down
20 changes: 3 additions & 17 deletions src/libs/CurrencyUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,22 +87,9 @@ function convertToBackendAmount(amountAsFloat: number): number {
*
* @note we do not support any currencies with more than two decimal places.
*/
function convertToFrontendAmountAsInteger(amountAsInt: number): number {
function convertToFrontendAmount(amountAsInt: number): number {
return Math.trunc(amountAsInt) / 100.0;
}

/**
* Takes an amount in "cents" as an integer and converts it to a string amount used in the frontend.
*
* @note we do not support any currencies with more than two decimal places.
*/
function convertToFrontendAmountAsString(amountAsInt: number | null | undefined): string {
if (amountAsInt === null || amountAsInt === undefined) {
return '';
}
return convertToFrontendAmountAsInteger(amountAsInt).toFixed(2);
}

/**
* Given an amount in the "cents", convert it to a string for display in the UI.
* The backend always handle things in "cents" (subunit equal to 1/100)
Expand All @@ -111,7 +98,7 @@ function convertToFrontendAmountAsString(amountAsInt: number | null | undefined)
* @param currency - IOU currency
*/
function convertToDisplayString(amountInCents = 0, currency: string = CONST.CURRENCY.USD): string {
const convertedAmount = convertToFrontendAmountAsInteger(amountInCents);
const convertedAmount = convertToFrontendAmount(amountInCents);
return NumberFormatUtils.format(BaseLocaleListener.getPreferredLocale(), convertedAmount, {
style: 'currency',
currency,
Expand Down Expand Up @@ -152,8 +139,7 @@ export {
getCurrencySymbol,
isCurrencySymbolLTR,
convertToBackendAmount,
convertToFrontendAmountAsInteger,
convertToFrontendAmountAsString,
convertToFrontendAmount,
convertToDisplayString,
convertAmountToDisplayString,
isValidCurrencyCode,
Expand Down
6 changes: 3 additions & 3 deletions src/libs/actions/IOU.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,12 +356,12 @@ function startMoneyRequest(iouType: ValueOf<typeof CONST.IOU.TYPE>, reportID: st
}

// eslint-disable-next-line @typescript-eslint/naming-convention
function setMoneyRequestAmount_temporaryForRefactor(transactionID: string, amount: number, currency: string, removeOriginalCurrency = false, shouldShowOriginalAmount = false) {
function setMoneyRequestAmount_temporaryForRefactor(transactionID: string, amount: number, currency: string, removeOriginalCurrency = false) {
if (removeOriginalCurrency) {
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {amount, currency, originalCurrency: null, shouldShowOriginalAmount});
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {amount, currency, originalCurrency: null});
return;
}
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {amount, currency, shouldShowOriginalAmount});
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {amount, currency});
}

// eslint-disable-next-line @typescript-eslint/naming-convention
Expand Down
9 changes: 1 addition & 8 deletions src/pages/iou/request/IOURequestStartPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,14 +165,7 @@ function IOURequestStartPage({
onTabSelected={resetIOUTypeIfChanged}
tabBar={TabSelector}
>
<TopTab.Screen name={CONST.TAB_REQUEST.MANUAL}>
{() => (
<IOURequestStepAmount
shouldKeepUserInput
route={route}
/>
)}
</TopTab.Screen>
<TopTab.Screen name={CONST.TAB_REQUEST.MANUAL}>{() => <IOURequestStepAmount route={route} />}</TopTab.Screen>
<TopTab.Screen name={CONST.TAB_REQUEST.SCAN}>{() => <IOURequestStepScan route={route} />}</TopTab.Screen>
{shouldDisplayDistanceRequest && <TopTab.Screen name={CONST.TAB_REQUEST.DISTANCE}>{() => <IOURequestStepDistance route={route} />}</TopTab.Screen>}
</OnyxTabNavigator>
Expand Down
9 changes: 1 addition & 8 deletions src/pages/iou/request/step/IOURequestStepAmount.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {useFocusEffect} from '@react-navigation/native';
import lodashGet from 'lodash/get';
import lodashIsEmpty from 'lodash/isEmpty';
import PropTypes from 'prop-types';
import React, {useCallback, useEffect, useRef} from 'react';
import {withOnyx} from 'react-native-onyx';
import transactionPropTypes from '@components/transactionPropTypes';
Expand Down Expand Up @@ -38,9 +37,6 @@ const propTypes = {
/** The draft transaction that holds data to be persisted on the current transaction */
splitDraftTransaction: transactionPropTypes,

/** Whether the user input should be kept or not */
shouldKeepUserInput: PropTypes.bool,

/** The draft transaction object being modified in Onyx */
draftTransaction: transactionPropTypes,
};
Expand All @@ -50,7 +46,6 @@ const defaultProps = {
transaction: {},
splitDraftTransaction: {},
draftTransaction: {},
shouldKeepUserInput: false,
};

function IOURequestStepAmount({
Expand All @@ -61,7 +56,6 @@ function IOURequestStepAmount({
transaction,
splitDraftTransaction,
draftTransaction,
shouldKeepUserInput,
}) {
const {translate} = useLocalize();
const textInput = useRef(null);
Expand Down Expand Up @@ -131,7 +125,7 @@ function IOURequestStepAmount({
isSaveButtonPressed.current = true;
const amountInSmallestCurrencyUnits = CurrencyUtils.convertToBackendAmount(Number.parseFloat(amount));

IOU.setMoneyRequestAmount_temporaryForRefactor(transactionID, amountInSmallestCurrencyUnits, currency || CONST.CURRENCY.USD, true, shouldKeepUserInput);
IOU.setMoneyRequestAmount_temporaryForRefactor(transactionID, amountInSmallestCurrencyUnits, currency || CONST.CURRENCY.USD, true);

if (backTo) {
Navigation.goBack(backTo);
Expand Down Expand Up @@ -189,7 +183,6 @@ function IOURequestStepAmount({
currency={currency}
amount={Math.abs(transactionAmount)}
ref={(e) => (textInput.current = e)}
shouldKeepUserInput={transaction.shouldShowOriginalAmount}
onCurrencyButtonPress={navigateToCurrencySelectionPage}
onSubmitButtonPress={saveAmountAndCurrency}
selectedTab={iouRequestType}
Expand Down
23 changes: 13 additions & 10 deletions src/pages/iou/steps/MoneyRequestAmountForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ type MoneyRequestAmountFormProps = {
/** Calculated tax amount based on selected tax rate */
taxAmount?: number;

/** Whether the user input should be kept or not */
shouldKeepUserInput?: boolean;

/** Currency chosen by user or saved in Onyx */
currency?: string;

Expand Down Expand Up @@ -67,7 +64,7 @@ const getNewSelection = (oldSelection: Selection, prevLength: number, newLength:

const isAmountInvalid = (amount: string) => !amount.length || parseFloat(amount) < 0.01;
const isTaxAmountInvalid = (currentAmount: string, taxAmount: number, isTaxAmountForm: boolean) =>
isTaxAmountForm && Number.parseFloat(currentAmount) > CurrencyUtils.convertToFrontendAmountAsInteger(Math.abs(taxAmount));
isTaxAmountForm && Number.parseFloat(currentAmount) > CurrencyUtils.convertToFrontendAmount(Math.abs(taxAmount));

const AMOUNT_VIEW_ID = 'amountView';
const NUM_PAD_CONTAINER_VIEW_ID = 'numPadContainerView';
Expand All @@ -83,7 +80,6 @@ function MoneyRequestAmountForm(
onCurrencyButtonPress,
onSubmitButtonPress,
selectedTab = CONST.TAB_REQUEST.MANUAL,
shouldKeepUserInput = false,
}: MoneyRequestAmountFormProps,
forwardedRef: ForwardedRef<BaseTextInputRef>,
) {
Expand All @@ -93,8 +89,10 @@ function MoneyRequestAmountForm(

const textInput = useRef<BaseTextInputRef | null>(null);
const isTaxAmountForm = Navigation.getActiveRoute().includes('taxAmount');

const decimals = CurrencyUtils.getCurrencyDecimals(currency);
const selectedAmountAsString = CurrencyUtils.convertToFrontendAmountAsString(amount);
const selectedAmountAsString = amount ? CurrencyUtils.convertToFrontendAmount(amount).toString() : '';

const [currentAmount, setCurrentAmount] = useState(selectedAmountAsString);
const [formError, setFormError] = useState<MaybePhraseKey>('');
const [shouldUpdateSelection, setShouldUpdateSelection] = useState(true);
Expand Down Expand Up @@ -127,7 +125,7 @@ function MoneyRequestAmountForm(
};

const initializeAmount = useCallback((newAmount: number) => {
const frontendAmount = CurrencyUtils.convertToFrontendAmountAsString(newAmount);
const frontendAmount = newAmount ? CurrencyUtils.convertToFrontendAmount(newAmount).toString() : '';
setCurrentAmount(frontendAmount);
setSelection({
start: frontendAmount.length,
Expand All @@ -136,13 +134,13 @@ function MoneyRequestAmountForm(
}, []);

useEffect(() => {
if (!currency || typeof amount !== 'number' || shouldKeepUserInput) {
if (!currency || typeof amount !== 'number') {
return;
}
initializeAmount(amount);
// we want to re-initialize the state only when the selected tab or amount changes
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [selectedTab, amount, shouldKeepUserInput]);
}, [selectedTab, amount]);

/**
* Sets the selection and the amount accordingly to the value passed to the input
Expand Down Expand Up @@ -243,8 +241,13 @@ function MoneyRequestAmountForm(
return;
}

// Update display amount string post-edit to ensure consistency with backend amount
// Reference: https://github.com/Expensify/App/issues/30505
const backendAmount = CurrencyUtils.convertToBackendAmount(Number.parseFloat(currentAmount));
initializeAmount(backendAmount);

onSubmitButtonPress({amount: currentAmount, currency});
}, [currentAmount, taxAmount, isTaxAmountForm, onSubmitButtonPress, currency, formattedTaxAmount]);
}, [currentAmount, taxAmount, isTaxAmountForm, onSubmitButtonPress, currency, formattedTaxAmount, initializeAmount]);

/**
* Input handler to check for a forward-delete key (or keyboard shortcut) press.
Expand Down
3 changes: 0 additions & 3 deletions src/types/onyx/Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,6 @@ type Transaction = OnyxCommon.OnyxValueWithOfflineFeedback<
/** Whether the request is billable */
billable?: boolean;

/** Whether the user input should be kept */
shouldShowOriginalAmount?: boolean;

/** The category name */
category?: string;

Expand Down
19 changes: 3 additions & 16 deletions tests/unit/CurrencyUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,31 +105,18 @@ describe('CurrencyUtils', () => {
});
});

describe('convertToFrontendAmountAsInteger', () => {
describe('convertToFrontendAmount', () => {
test.each([
[2500, 25],
[2550, 25.5],
[25, 0.25],
[2500, 25],
[2500.5, 25], // The backend should never send a decimal .5 value
])('Correctly converts %s to amount in units handled in frontend as an integer', (amount, expectedResult) => {
expect(CurrencyUtils.convertToFrontendAmountAsInteger(amount)).toBe(expectedResult);
])('Correctly converts %s to amount in units handled in frontend', (amount, expectedResult) => {
expect(CurrencyUtils.convertToFrontendAmount(amount)).toBe(expectedResult);
});
});

describe('convertToFrontendAmountAsString', () => {
test.each([
[2500, '25.00'],
[2550, '25.50'],
[25, '0.25'],
[2500.5, '25.00'],
[null, ''],
[undefined, ''],
[0, '0.00'],
])('Correctly converts %s to amount in units handled in frontend as a string', (input, expectedResult) => {
expect(CurrencyUtils.convertToFrontendAmountAsString(input)).toBe(expectedResult);
});
});
describe('convertToDisplayString', () => {
test.each([
[CONST.CURRENCY.USD, 25, '$0.25'],
Expand Down

0 comments on commit a1f9cec

Please sign in to comment.