From 894c310bcd560b6135f4d10246bfbd472d225128 Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab Date: Thu, 11 Apr 2024 04:14:11 +0200 Subject: [PATCH 1/6] Preserve transactions amount in create IOU --- src/components/transactionPropTypes.js | 3 +++ src/libs/CurrencyUtils.ts | 20 +++++++++++++++--- src/libs/actions/IOU.ts | 6 +++--- src/pages/iou/request/IOURequestStartPage.js | 9 +++++++- .../iou/request/step/IOURequestStepAmount.js | 9 +++++++- .../iou/steps/MoneyRequestAmountForm.tsx | 21 +++++++++---------- src/types/onyx/Transaction.ts | 3 +++ tests/unit/CurrencyUtilsTest.ts | 20 +++++++++++++++--- 8 files changed, 69 insertions(+), 22 deletions(-) diff --git a/src/components/transactionPropTypes.js b/src/components/transactionPropTypes.js index 7eb1b776358c..46d246948460 100644 --- a/src/components/transactionPropTypes.js +++ b/src/components/transactionPropTypes.js @@ -93,4 +93,7 @@ export default PropTypes.shape({ /** Server side errors keyed by microtime */ errorFields: PropTypes.objectOf(PropTypes.objectOf(translatableTextPropTypes)), + + /** Whether the original input should be shown */ + shouldShowOriginalAmount: PropTypes.bool, }); diff --git a/src/libs/CurrencyUtils.ts b/src/libs/CurrencyUtils.ts index 95970d2a9582..4530dc105e03 100644 --- a/src/libs/CurrencyUtils.ts +++ b/src/libs/CurrencyUtils.ts @@ -87,9 +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) @@ -98,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, @@ -139,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 index dab0eed1653a..8e3ead229a61 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -357,12 +357,12 @@ function startMoneyRequest(iouType: ValueOf, 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 index e9057fef9226..2a96e5b1e1eb 100644 --- a/src/pages/iou/request/IOURequestStartPage.js +++ b/src/pages/iou/request/IOURequestStartPage.js @@ -166,7 +166,14 @@ function IOURequestStartPage({ onTabSelected={resetIOUTypeIfChanged} tabBar={TabSelector} > - {() => } + + {() => ( + + )} + {() => } {shouldDisplayDistanceRequest && {() => }} diff --git a/src/pages/iou/request/step/IOURequestStepAmount.js b/src/pages/iou/request/step/IOURequestStepAmount.js index b392ee310205..0dd4d0247629 100644 --- 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'; @@ -39,6 +40,9 @@ const propTypes = { /** The draft transaction object being modified in Onyx */ draftTransaction: transactionPropTypes, + + /** Whether the user input should be kept or not */ + shouldKeepUserInput: PropTypes.bool, }; const defaultProps = { @@ -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 index c8b418f301c9..77a953dada90 100644 --- a/src/pages/iou/steps/MoneyRequestAmountForm.tsx +++ b/src/pages/iou/steps/MoneyRequestAmountForm.tsx @@ -49,6 +49,9 @@ type MoneyRequestAmountFormProps = { /** The current tab we have navigated to in the request modal. String that corresponds to the request type. */ selectedTab?: SelectedTabRequest; + + /** Whether the user input should be kept or not */ + shouldKeepUserInput?: boolean; }; type Selection = { @@ -66,7 +69,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'; @@ -82,6 +85,7 @@ function MoneyRequestAmountForm( onCurrencyButtonPress, onSubmitButtonPress, selectedTab = CONST.TAB_REQUEST.MANUAL, + shouldKeepUserInput = false, }: MoneyRequestAmountFormProps, forwardedRef: ForwardedRef, ) { @@ -93,7 +97,7 @@ function MoneyRequestAmountForm( const isTaxAmountForm = Navigation.getActiveRoute().includes('taxAmount'); const decimals = CurrencyUtils.getCurrencyDecimals(currency); - const selectedAmountAsString = amount ? CurrencyUtils.convertToFrontendAmount(amount).toString() : ''; + const selectedAmountAsString = amount ? CurrencyUtils.convertToFrontendAmountAsString(amount) : ''; const [currentAmount, setCurrentAmount] = useState(selectedAmountAsString); const [formError, setFormError] = useState(''); @@ -135,7 +139,7 @@ function MoneyRequestAmountForm( }; const initializeAmount = useCallback((newAmount: number) => { - const frontendAmount = newAmount ? CurrencyUtils.convertToFrontendAmount(newAmount).toString() : ''; + const frontendAmount = newAmount ? CurrencyUtils.convertToFrontendAmountAsString(newAmount) : ''; setCurrentAmount(frontendAmount); setSelection({ start: frontendAmount.length, @@ -144,13 +148,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 @@ -264,13 +268,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}); - }, [currentAmount, taxAmount, isTaxAmountForm, onSubmitButtonPress, currency, 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 index 281b6b4228ce..6b4bd1fc0d19 100644 --- a/src/types/onyx/Transaction.ts +++ b/src/types/onyx/Transaction.ts @@ -215,6 +215,9 @@ type Transaction = OnyxCommon.OnyxValueWithOfflineFeedback< /** Indicates transaction loading */ isLoading?: boolean; + + /** Whether the user input should be kept */ + shouldShowOriginalAmount?: boolean; }, keyof Comment >; diff --git a/tests/unit/CurrencyUtilsTest.ts b/tests/unit/CurrencyUtilsTest.ts index a1e4b03fa715..089cdf8426a8 100644 --- a/tests/unit/CurrencyUtilsTest.ts +++ b/tests/unit/CurrencyUtilsTest.ts @@ -105,15 +105,29 @@ 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); }); }); From 5f0df225b999649f6a7c7cccefe138e66ccc3071 Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab Date: Mon, 15 Apr 2024 20:42:38 +0200 Subject: [PATCH 2/6] fixing types --- src/pages/iou/request/step/IOURequestStepAmount.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pages/iou/request/step/IOURequestStepAmount.tsx b/src/pages/iou/request/step/IOURequestStepAmount.tsx index a747084ac2b3..ca662d1a2a66 100644 --- a/src/pages/iou/request/step/IOURequestStepAmount.tsx +++ b/src/pages/iou/request/step/IOURequestStepAmount.tsx @@ -33,15 +33,15 @@ type IOURequestStepAmountOnyxProps = { /** The draft transaction object being modified in Onyx */ draftTransaction: OnyxEntry; - - /** Whether the user input should be kept or not */ - shouldKeepUserInput: PropTypes.bool; }; type IOURequestStepAmountProps = IOURequestStepAmountOnyxProps & WithWritableReportOrNotFoundProps & { /** The transaction object being modified in Onyx */ transaction: OnyxEntry; + + /** Whether the user input should be kept or not */ + shouldKeepUserInput?: boolean; }; function IOURequestStepAmount({ @@ -169,7 +169,7 @@ function IOURequestStepAmount({ currency={currency} amount={Math.abs(transactionAmount)} ref={(e) => (textInput.current = e)} - shouldKeepUserInput={transaction.shouldShowOriginalAmount} + shouldKeepUserInput={transaction?.shouldShowOriginalAmount} onCurrencyButtonPress={navigateToCurrencySelectionPage} onSubmitButtonPress={saveAmountAndCurrency} selectedTab={iouRequestType} From 2d5e6168bb025ede400775448fd65f7ddadef00a Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab Date: Wed, 8 May 2024 13:02:29 +0200 Subject: [PATCH 3/6] moving shouldKeepUserInput to the MoneyRequestAmountInput component --- src/components/MoneyRequestAmountInput.tsx | 12 ++++++++---- src/pages/iou/MoneyRequestAmountForm.tsx | 5 +++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/components/MoneyRequestAmountInput.tsx b/src/components/MoneyRequestAmountInput.tsx index fcc156db4a96..2a47d147e05a 100644 --- a/src/components/MoneyRequestAmountInput.tsx +++ b/src/components/MoneyRequestAmountInput.tsx @@ -62,6 +62,9 @@ type MoneyRequestAmountInputProps = { /** Style for the touchable input wrapper */ touchableInputWrapperStyle?: StyleProp; + + /** Whether the user input should be kept or not */ + shouldKeepUserInput?: boolean; }; type Selection = { @@ -88,6 +91,7 @@ function MoneyRequestAmountInput( hideCurrencySymbol = false, shouldUpdateSelection = true, moneyRequestAmountInputRef, + shouldKeepUserInput = false, ...props }: MoneyRequestAmountInputProps, forwardedRef: ForwardedRef, @@ -97,7 +101,7 @@ function MoneyRequestAmountInput( const textInput = useRef(null); const decimals = CurrencyUtils.getCurrencyDecimals(currency); - const selectedAmountAsString = amount ? CurrencyUtils.convertToFrontendAmount(amount).toString() : ''; + const selectedAmountAsString = amount ? CurrencyUtils.convertToFrontendAmountAsString(amount).toString() : ''; const [currentAmount, setCurrentAmount] = useState(selectedAmountAsString); @@ -160,10 +164,10 @@ function MoneyRequestAmountInput( })); useEffect(() => { - if (!currency || typeof amount !== 'number') { + if (!currency || typeof amount !== 'number' || shouldKeepUserInput) { return; } - const frontendAmount = amount ? CurrencyUtils.convertToFrontendAmount(amount).toString() : ''; + const frontendAmount = amount ? CurrencyUtils.convertToFrontendAmountAsString(amount).toString() : ''; setCurrentAmount(frontendAmount); setSelection({ start: frontendAmount.length, @@ -171,7 +175,7 @@ function MoneyRequestAmountInput( }); // we want to re-initialize the state only when the amount changes // eslint-disable-next-line react-hooks/exhaustive-deps - }, [amount]); + }, [amount, shouldKeepUserInput]); // Modifies the amount to match the decimals for changed currency. useEffect(() => { diff --git a/src/pages/iou/MoneyRequestAmountForm.tsx b/src/pages/iou/MoneyRequestAmountForm.tsx index 291cf458d498..4de70ec96fdf 100644 --- a/src/pages/iou/MoneyRequestAmountForm.tsx +++ b/src/pages/iou/MoneyRequestAmountForm.tsx @@ -157,13 +157,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 // eslint-disable-next-line react-hooks/exhaustive-deps - }, [selectedTab, shouldKeepUserInput]); + }, [selectedTab]); /** * Update amount with number or Backspace pressed for BigNumberPad. @@ -286,6 +286,7 @@ function MoneyRequestAmountForm( } textInput.current = ref; }} + shouldKeepUserInput={shouldKeepUserInput} moneyRequestAmountInputRef={moneyRequestAmountInput} inputStyle={[styles.iouAmountTextInput]} containerStyle={[styles.iouAmountTextInputContainer]} From 2a6f9ef52af48c73f9afbb7ad6e8fdbd1f79916c Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab Date: Wed, 8 May 2024 13:09:56 +0200 Subject: [PATCH 4/6] Minor edit --- src/libs/CurrencyUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/CurrencyUtils.ts b/src/libs/CurrencyUtils.ts index 920d49c8ceb0..d3660c5f16f4 100644 --- a/src/libs/CurrencyUtils.ts +++ b/src/libs/CurrencyUtils.ts @@ -141,7 +141,7 @@ function convertAmountToDisplayString(amount = 0, currency: string = CONST.CURRE * Acts the same as `convertAmountToDisplayString` but the result string does not contain currency */ function convertToDisplayStringWithoutCurrency(amountInCents: number, currency: string = CONST.CURRENCY.USD) { - const convertedAmount = convertToFrontendAmount(amountInCents); + const convertedAmount = convertToFrontendAmountAsInteger(amountInCents); return NumberFormatUtils.formatToParts(BaseLocaleListener.getPreferredLocale(), convertedAmount, { style: 'currency', currency, From 4ce6e266af438c302e286c0802e4d1616d10b026 Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab Date: Wed, 8 May 2024 17:24:28 +0200 Subject: [PATCH 5/6] cleaning --- src/pages/iou/MoneyRequestAmountForm.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/iou/MoneyRequestAmountForm.tsx b/src/pages/iou/MoneyRequestAmountForm.tsx index 4de70ec96fdf..b3b00ab37bf6 100644 --- a/src/pages/iou/MoneyRequestAmountForm.tsx +++ b/src/pages/iou/MoneyRequestAmountForm.tsx @@ -224,7 +224,7 @@ function MoneyRequestAmountForm( onSubmitButtonPress({amount: currentAmount, currency, paymentMethod: iouPaymentType}); }, - [taxAmount, onSubmitButtonPress, currency, formattedTaxAmount, initializeAmount], + [taxAmount, onSubmitButtonPress, currency, formattedTaxAmount], ); const buttonText: string = useMemo(() => { From e202ba916c92d7e52eafc22888e59772d27567a1 Mon Sep 17 00:00:00 2001 From: Abdelrahman Khattab Date: Sun, 19 May 2024 19:05:05 +0200 Subject: [PATCH 6/6] Refactoring --- src/components/MoneyRequestAmountInput.tsx | 5 ++--- src/pages/iou/MoneyRequestAmountForm.tsx | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/components/MoneyRequestAmountInput.tsx b/src/components/MoneyRequestAmountInput.tsx index d7cb10cb2f23..22336f8c3a78 100644 --- a/src/components/MoneyRequestAmountInput.tsx +++ b/src/components/MoneyRequestAmountInput.tsx @@ -108,7 +108,6 @@ function MoneyRequestAmountInput( maxLength, hideFocusedState = true, shouldKeepUserInput = false, - ...props }: MoneyRequestAmountInputProps, forwardedRef: ForwardedRef, @@ -118,7 +117,7 @@ function MoneyRequestAmountInput( const textInput = useRef(null); const decimals = CurrencyUtils.getCurrencyDecimals(currency); - const selectedAmountAsString = amount ? CurrencyUtils.convertToFrontendAmountAsString(amount).toString() : ''; + const selectedAmountAsString = CurrencyUtils.convertToFrontendAmountAsString(amount); const [currentAmount, setCurrentAmount] = useState(selectedAmountAsString); @@ -187,7 +186,7 @@ function MoneyRequestAmountInput( if (!currency || typeof amount !== 'number' || (formatAmountOnBlur && textInput.current?.isFocused()) || shouldKeepUserInput) { return; } - const frontendAmount = formatAmountOnBlur ? CurrencyUtils.convertToDisplayStringWithoutCurrency(amount, currency) : CurrencyUtils.convertToFrontendAmountAsString(amount).toString(); + const frontendAmount = formatAmountOnBlur ? CurrencyUtils.convertToDisplayStringWithoutCurrency(amount, currency) : CurrencyUtils.convertToFrontendAmountAsString(amount); setCurrentAmount(frontendAmount); // Only update selection if the amount prop was changed from the outside and is not the same as the current amount we just computed diff --git a/src/pages/iou/MoneyRequestAmountForm.tsx b/src/pages/iou/MoneyRequestAmountForm.tsx index b3b00ab37bf6..5bbc9d22a97f 100644 --- a/src/pages/iou/MoneyRequestAmountForm.tsx +++ b/src/pages/iou/MoneyRequestAmountForm.tsx @@ -148,7 +148,7 @@ function MoneyRequestAmountForm( }, [isFocused, wasFocused]); const initializeAmount = useCallback((newAmount: number) => { - const frontendAmount = newAmount ? CurrencyUtils.convertToFrontendAmountAsString(newAmount).toString() : ''; + const frontendAmount = newAmount ? CurrencyUtils.convertToFrontendAmountAsString(newAmount) : ''; moneyRequestAmountInput.current?.changeAmount(frontendAmount); moneyRequestAmountInput.current?.changeSelection({ start: frontendAmount.length,