Skip to content
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

Preserve transactions amount in create IOU #40062

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
10 changes: 7 additions & 3 deletions src/components/MoneyRequestAmountInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,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 @@ -97,7 +100,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 @@ -115,6 +118,7 @@ function MoneyRequestAmountInput(
formatAmountOnBlur,
maxLength,
hideFocusedState = true,
shouldKeepUserInput = false,
autoGrow = true,
...props
}: MoneyRequestAmountInputProps,
Expand Down Expand Up @@ -191,7 +195,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 @@ -208,7 +212,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);
abzokhattab marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* 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 @@ -374,8 +374,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 @@ -150,7 +150,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
Loading