Merge pull request #35797 from abzokhattab/preserving-the-transaction…

…s-amounts-the-same-as-the-user-entered Corrected Currency Display: Enforce Two Decimal Places in Amounts
Expensify · Apr 3, 2024 · 4fcd8a1 · 4fcd8a1
2 parents 2b592a2 + d4b3764
commit 4fcd8a1
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 26 deletions.
diff --git a/src/components/transactionPropTypes.js b/src/components/transactionPropTypes.js
@@ -14,6 +14,9 @@ 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,
 

diff --git a/src/libs/CurrencyUtils.ts b/src/libs/CurrencyUtils.ts
@@ -87,10 +87,22 @@ function convertToBackendAmount(amountAsFloat: number): number {
  *
  * @note we do not support any currencies with more than two decimal places.
  */
-function convertToFrontendAmount(amountAsInt: number): number {
+function convertToFrontendAmountAsInteger(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)
@@ -99,7 +111,7 @@ function convertToFrontendAmount(amountAsInt: number): number {
  * @param currency - IOU currency
  */
 function convertToDisplayString(amountInCents = 0, currency: string = CONST.CURRENCY.USD): string {
-    const convertedAmount = convertToFrontendAmount(amountInCents);
+    const convertedAmount = convertToFrontendAmountAsInteger(amountInCents);
     return NumberFormatUtils.format(BaseLocaleListener.getPreferredLocale(), convertedAmount, {
         style: 'currency',
         currency,
@@ -140,7 +152,8 @@ export {
     getCurrencySymbol,
     isCurrencySymbolLTR,
     convertToBackendAmount,
-    convertToFrontendAmount,
+    convertToFrontendAmountAsInteger,
+    convertToFrontendAmountAsString,
     convertToDisplayString,
     convertAmountToDisplayString,
     isValidCurrencyCode,

diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts
@@ -338,12 +338,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) {
+function setMoneyRequestAmount_temporaryForRefactor(transactionID: string, amount: number, currency: string, removeOriginalCurrency = false, shouldShowOriginalAmount = false) {
     if (removeOriginalCurrency) {
-        Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {amount, currency, originalCurrency: null});
+        Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {amount, currency, originalCurrency: null, shouldShowOriginalAmount});
         return;
     }
-    Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {amount, currency});
+    Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {amount, currency, shouldShowOriginalAmount});
 }
 
 // eslint-disable-next-line @typescript-eslint/naming-convention

diff --git a/src/pages/iou/request/IOURequestStartPage.js b/src/pages/iou/request/IOURequestStartPage.js
@@ -165,7 +165,14 @@ function IOURequestStartPage({
                                     onTabSelected={resetIOUTypeIfChanged}
                                     tabBar={TabSelector}
                                 >
-                                    <TopTab.Screen name={CONST.TAB_REQUEST.MANUAL}>{() => <IOURequestStepAmount route={route} />}</TopTab.Screen>
+                                    <TopTab.Screen name={CONST.TAB_REQUEST.MANUAL}>
+                                        {() => (
+                                            <IOURequestStepAmount
+                                                shouldKeepUserInput
+                                                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>

diff --git a/src/pages/iou/request/step/IOURequestStepAmount.js b/src/pages/iou/request/step/IOURequestStepAmount.js
@@ -1,6 +1,7 @@
 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';
@@ -10,8 +11,8 @@ import compose from '@libs/compose';
 import * as CurrencyUtils from '@libs/CurrencyUtils';
 import Navigation from '@libs/Navigation/Navigation';
 import * as ReportUtils from '@libs/ReportUtils';
-import {getRequestType} from '@libs/TransactionUtils';
 import * as TransactionUtils from '@libs/TransactionUtils';
+import {getRequestType} from '@libs/TransactionUtils';
 import MoneyRequestAmountForm from '@pages/iou/steps/MoneyRequestAmountForm';
 import reportPropTypes from '@pages/reportPropTypes';
 import * as IOU from '@userActions/IOU';
@@ -37,6 +38,9 @@ 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,
 };
@@ -46,6 +50,7 @@ const defaultProps = {
     transaction: {},
     splitDraftTransaction: {},
     draftTransaction: {},
+    shouldKeepUserInput: false,
 };
 
 function IOURequestStepAmount({
@@ -56,6 +61,7 @@ function IOURequestStepAmount({
     transaction,
     splitDraftTransaction,
     draftTransaction,
+    shouldKeepUserInput,
 }) {
     const {translate} = useLocalize();
     const textInput = useRef(null);
@@ -125,7 +131,7 @@ function IOURequestStepAmount({
         isSaveButtonPressed.current = true;
         const amountInSmallestCurrencyUnits = CurrencyUtils.convertToBackendAmount(Number.parseFloat(amount));
 
-        IOU.setMoneyRequestAmount_temporaryForRefactor(transactionID, amountInSmallestCurrencyUnits, currency || CONST.CURRENCY.USD, true);
+        IOU.setMoneyRequestAmount_temporaryForRefactor(transactionID, amountInSmallestCurrencyUnits, currency || CONST.CURRENCY.USD, true, shouldKeepUserInput);
 
         if (backTo) {
             Navigation.goBack(backTo);
@@ -183,6 +189,7 @@ function IOURequestStepAmount({
                 currency={currency}
                 amount={Math.abs(transactionAmount)}
                 ref={(e) => (textInput.current = e)}
+                shouldKeepUserInput={transaction.shouldShowOriginalAmount}
                 onCurrencyButtonPress={navigateToCurrencySelectionPage}
                 onSubmitButtonPress={saveAmountAndCurrency}
                 selectedTab={iouRequestType}

diff --git a/src/pages/iou/steps/MoneyRequestAmountForm.tsx b/src/pages/iou/steps/MoneyRequestAmountForm.tsx
@@ -1,5 +1,5 @@
-import React, {useCallback, useEffect, useRef, useState} from 'react';
 import type {ForwardedRef} from 'react';
+import React, {useCallback, useEffect, useRef, useState} from 'react';
 import {View} from 'react-native';
 import type {NativeSyntheticEvent, TextInputSelectionChangeEventData} from 'react-native';
 import BigNumberPad from '@components/BigNumberPad';
@@ -28,6 +28,9 @@ 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;
 
@@ -62,7 +65,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.convertToFrontendAmount(Math.abs(taxAmount));
+    isTaxAmountForm && Number.parseFloat(currentAmount) > CurrencyUtils.convertToFrontendAmountAsInteger(Math.abs(taxAmount));
 
 const AMOUNT_VIEW_ID = 'amountView';
 const NUM_PAD_CONTAINER_VIEW_ID = 'numPadContainerView';
@@ -78,6 +81,7 @@ function MoneyRequestAmountForm(
         onCurrencyButtonPress,
         onSubmitButtonPress,
         selectedTab = CONST.TAB_REQUEST.MANUAL,
+        shouldKeepUserInput = false,
     }: MoneyRequestAmountFormProps,
     forwardedRef: ForwardedRef<BaseTextInputRef>,
 ) {
@@ -87,10 +91,8 @@ function MoneyRequestAmountForm(
 
     const textInput = useRef<BaseTextInputRef | null>(null);
     const isTaxAmountForm = Navigation.getActiveRoute().includes('taxAmount');
-
     const decimals = CurrencyUtils.getCurrencyDecimals(currency);
-    const selectedAmountAsString = amount ? CurrencyUtils.convertToFrontendAmount(amount).toString() : '';
-
+    const selectedAmountAsString = CurrencyUtils.convertToFrontendAmountAsString(amount);
     const [currentAmount, setCurrentAmount] = useState(selectedAmountAsString);
     const [formError, setFormError] = useState<MaybePhraseKey>('');
     const [shouldUpdateSelection, setShouldUpdateSelection] = useState(true);
@@ -123,7 +125,7 @@ function MoneyRequestAmountForm(
     };
 
     const initializeAmount = useCallback((newAmount: number) => {
-        const frontendAmount = newAmount ? CurrencyUtils.convertToFrontendAmount(newAmount).toString() : '';
+        const frontendAmount = CurrencyUtils.convertToFrontendAmountAsString(newAmount);
         setCurrentAmount(frontendAmount);
         setSelection({
             start: frontendAmount.length,
@@ -132,13 +134,13 @@ function MoneyRequestAmountForm(
     }, []);
 
     useEffect(() => {
-        if (!currency || typeof amount !== 'number') {
+        if (!currency || typeof amount !== 'number' || shouldKeepUserInput) {
             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]);
+    }, [selectedTab, amount, shouldKeepUserInput]);
 
     /**
      * Sets the selection and the amount accordingly to the value passed to the input
@@ -239,13 +241,8 @@ 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});
-    }, [onSubmitButtonPress, currentAmount, taxAmount, currency, isTaxAmountForm, formattedTaxAmount, initializeAmount]);
+    }, [currentAmount, taxAmount, isTaxAmountForm, onSubmitButtonPress, currency, formattedTaxAmount]);
 
     /**
      * Input handler to check for a forward-delete key (or keyboard shortcut) press.

diff --git a/src/types/onyx/Transaction.ts b/src/types/onyx/Transaction.ts
@@ -111,6 +111,9 @@ 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;
 

diff --git a/tests/unit/CurrencyUtilsTest.ts b/tests/unit/CurrencyUtilsTest.ts
@@ -105,18 +105,31 @@ describe('CurrencyUtils', () => {
         });
     });
 
-    describe('convertToFrontendAmount', () => {
+    describe('convertToFrontendAmountAsInteger', () => {
         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', (amount, expectedResult) => {
-            expect(CurrencyUtils.convertToFrontendAmount(amount)).toBe(expectedResult);
+        ])('Correctly converts %s to amount in units handled in frontend as an integer', (amount, expectedResult) => {
+            expect(CurrencyUtils.convertToFrontendAmountAsInteger(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'],