Skip to content

Commit

Permalink
Preserve transactions amount in create IOU
Browse files Browse the repository at this point in the history
  • Loading branch information
abzokhattab committed Apr 11, 2024
1 parent 8ae5491 commit 894c310
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 22 deletions.
3 changes: 3 additions & 0 deletions src/components/transactionPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
20 changes: 17 additions & 3 deletions src/libs/CurrencyUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand Down Expand Up @@ -139,7 +152,8 @@ export {
getCurrencySymbol,
isCurrencySymbolLTR,
convertToBackendAmount,
convertToFrontendAmount,
convertToFrontendAmountAsInteger,
convertToFrontendAmountAsString,
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 @@ -357,12 +357,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
Expand Down
9 changes: 8 additions & 1 deletion src/pages/iou/request/IOURequestStartPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,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>
Expand Down
9 changes: 8 additions & 1 deletion src/pages/iou/request/step/IOURequestStepAmount.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -39,13 +40,17 @@ 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 = {
report: {},
transaction: {},
splitDraftTransaction: {},
draftTransaction: {},
shouldKeepUserInput: false,
};

function IOURequestStepAmount({
Expand All @@ -56,6 +61,7 @@ function IOURequestStepAmount({
transaction,
splitDraftTransaction,
draftTransaction,
shouldKeepUserInput,
}) {
const {translate} = useLocalize();
const textInput = useRef(null);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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}
Expand Down
21 changes: 10 additions & 11 deletions src/pages/iou/steps/MoneyRequestAmountForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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';
Expand All @@ -82,6 +85,7 @@ function MoneyRequestAmountForm(
onCurrencyButtonPress,
onSubmitButtonPress,
selectedTab = CONST.TAB_REQUEST.MANUAL,
shouldKeepUserInput = false,
}: MoneyRequestAmountFormProps,
forwardedRef: ForwardedRef<BaseTextInputRef>,
) {
Expand All @@ -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<MaybePhraseKey>('');
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions src/types/onyx/Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,9 @@ type Transaction = OnyxCommon.OnyxValueWithOfflineFeedback<

/** Indicates transaction loading */
isLoading?: boolean;

/** Whether the user input should be kept */
shouldShowOriginalAmount?: boolean;
},
keyof Comment
>;
Expand Down
20 changes: 17 additions & 3 deletions tests/unit/CurrencyUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

Expand Down

0 comments on commit 894c310

Please sign in to comment.