Skip to content

Revert "35797-preserving-the-transactions-amounts-the-same-as-entered" #39610

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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