Skip to content

Commit

Permalink
Merge pull request #40062 from abzokhattab/preserve-transaction-amoun…
Browse files Browse the repository at this point in the history
…t-in-the-amount-formm

Preserve transactions amount in create IOU
  • Loading branch information
dangrous authored Jun 4, 2024
2 parents 9c091d4 + 22f8224 commit 4de7af8
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 22 deletions.
10 changes: 7 additions & 3 deletions src/components/MoneyRequestAmountInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ type MoneyRequestAmountInputProps = {
/** Hide the focus styles on TextInput */
hideFocusedState?: boolean;

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

/**
* Autogrow input container length based on the entered text.
*/
Expand All @@ -98,7 +101,7 @@ const getNewSelection = (oldSelection: Selection, prevLength: number, newLength:
return {start: cursorPosition, end: cursorPosition};
};

const defaultOnFormatAmount = (amount: number) => CurrencyUtils.convertToFrontendAmount(amount).toString();
const defaultOnFormatAmount = (amount: number) => CurrencyUtils.convertToFrontendAmountAsString(amount);

function MoneyRequestAmountInput(
{
Expand All @@ -116,6 +119,7 @@ function MoneyRequestAmountInput(
formatAmountOnBlur,
maxLength,
hideFocusedState = true,
shouldKeepUserInput = false,
autoGrow = true,
...props
}: MoneyRequestAmountInputProps,
Expand Down Expand Up @@ -192,7 +196,7 @@ function MoneyRequestAmountInput(
}));

useEffect(() => {
if (!currency || typeof amount !== 'number' || (formatAmountOnBlur && textInput.current?.isFocused())) {
if (!currency || typeof amount !== 'number' || (formatAmountOnBlur && textInput.current?.isFocused()) || shouldKeepUserInput) {
return;
}
const frontendAmount = onFormatAmount(amount, currency);
Expand All @@ -209,7 +213,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(() => {
Expand Down
22 changes: 18 additions & 4 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 @@ -128,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,
Expand Down Expand Up @@ -158,7 +171,8 @@ export {
getCurrencySymbol,
isCurrencySymbolLTR,
convertToBackendAmount,
convertToFrontendAmount,
convertToFrontendAmountAsInteger,
convertToFrontendAmountAsString,
convertToDisplayString,
convertAmountToDisplayString,
convertToDisplayStringWithoutCurrency,
Expand Down
4 changes: 2 additions & 2 deletions src/libs/actions/IOU.ts
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,8 @@ function startMoneyRequest(iouType: ValueOf<typeof CONST.IOU.TYPE>, reportID: st
}
}

function setMoneyRequestAmount(transactionID: string, amount: number, currency: string) {
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {amount, currency});
function setMoneyRequestAmount(transactionID: string, amount: number, currency: string, shouldShowOriginalAmount = false) {
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {amount, currency, shouldShowOriginalAmount});
}

function setMoneyRequestCreated(transactionID: string, created: string, isDraft: boolean) {
Expand Down
16 changes: 8 additions & 8 deletions src/pages/iou/MoneyRequestAmountForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,14 @@ type MoneyRequestAmountFormProps = {

/** The current tab we have navigated to in the expense modal. String that corresponds to the expense type. */
selectedTab?: SelectedTabRequest;

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

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 @@ -88,6 +91,7 @@ function MoneyRequestAmountForm(
onCurrencyButtonPress,
onSubmitButtonPress,
selectedTab = CONST.TAB_REQUEST.MANUAL,
shouldKeepUserInput = false,
}: MoneyRequestAmountFormProps,
forwardedRef: ForwardedRef<BaseTextInputRef>,
) {
Expand Down Expand Up @@ -144,7 +148,7 @@ function MoneyRequestAmountForm(
}, [isFocused, wasFocused]);

const initializeAmount = useCallback((newAmount: number) => {
const frontendAmount = newAmount ? CurrencyUtils.convertToFrontendAmount(newAmount).toString() : '';
const frontendAmount = newAmount ? CurrencyUtils.convertToFrontendAmountAsString(newAmount) : '';
moneyRequestAmountInput.current?.changeAmount(frontendAmount);
moneyRequestAmountInput.current?.changeSelection({
start: frontendAmount.length,
Expand Down Expand Up @@ -218,14 +222,9 @@ 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, paymentMethod: iouPaymentType});
},
[taxAmount, onSubmitButtonPress, currency, formattedTaxAmount, initializeAmount],
[taxAmount, onSubmitButtonPress, currency, formattedTaxAmount],
);

const buttonText: string = useMemo(() => {
Expand Down Expand Up @@ -287,6 +286,7 @@ function MoneyRequestAmountForm(
}
textInput.current = ref;
}}
shouldKeepUserInput={shouldKeepUserInput}
moneyRequestAmountInputRef={moneyRequestAmountInput}
inputStyle={[styles.iouAmountTextInput]}
containerStyle={[styles.iouAmountTextInputContainer]}
Expand Down
9 changes: 8 additions & 1 deletion src/pages/iou/request/IOURequestStartPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,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
7 changes: 6 additions & 1 deletion src/pages/iou/request/step/IOURequestStepAmount.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ type IOURequestStepAmountProps = IOURequestStepAmountOnyxProps &
WithWritableReportOrNotFoundProps<typeof SCREENS.MONEY_REQUEST.STEP_AMOUNT | typeof SCREENS.MONEY_REQUEST.CREATE> & {
/** The transaction object being modified in Onyx */
transaction: OnyxEntry<OnyxTypes.Transaction>;

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

function IOURequestStepAmount({
Expand All @@ -68,6 +71,7 @@ function IOURequestStepAmount({
splitDraftTransaction,
skipConfirmation,
draftTransaction,
shouldKeepUserInput = false,
}: IOURequestStepAmountProps) {
const {translate} = useLocalize();
const textInput = useRef<BaseTextInputRef | null>(null);
Expand Down Expand Up @@ -163,7 +167,7 @@ function IOURequestStepAmount({
const amountInSmallestCurrencyUnits = CurrencyUtils.convertToBackendAmount(Number.parseFloat(amount));

// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
IOU.setMoneyRequestAmount(transactionID, amountInSmallestCurrencyUnits, currency || CONST.CURRENCY.USD);
IOU.setMoneyRequestAmount(transactionID, amountInSmallestCurrencyUnits, currency || CONST.CURRENCY.USD, shouldKeepUserInput);

if (backTo) {
Navigation.goBack(backTo);
Expand Down Expand Up @@ -309,6 +313,7 @@ function IOURequestStepAmount({
policyID={policy?.id ?? ''}
bankAccountRoute={ReportUtils.getBankAccountRoute(report)}
ref={(e) => (textInput.current = e)}
shouldKeepUserInput={transaction?.shouldShowOriginalAmount}
onCurrencyButtonPress={navigateToCurrencySelectionPage}
onSubmitButtonPress={saveAmountAndCurrency}
selectedTab={iouRequestType}
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 @@ -241,6 +241,9 @@ type Transaction = OnyxCommon.OnyxValueWithOfflineFeedback<
/** Holds the accountIDs of accounts who paid the split, for now only supports a single payer */
splitPayerAccountIDs?: number[];

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

/** The actionable report action ID associated with the transaction */
actionableWhisperReportActionID?: string;

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 4de7af8

Please sign in to comment.