[HOLD for payment 2024-07-26][$500] When amending the amount, if the last digit is a 0, we don't show it.

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.29-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @twisterdotcom
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1705662590072039

Action Performed:

1. create an expense report
2. Open the report and edit the amount so that last decimal digit is 0 (e.g. 23.20)
3. Again open and see the amount

Expected Result:

Should see 0 at the end

Actual Result:

"0" is missing

Workaround:

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

0.gets.ignored.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0178a9407eab9689fe
• Upwork Job ID: 1749451650573455360
• Last Price Increase: 2024-01-29
• Automatic offers:
• abzokhattab | Contributor | 28129136

Issue Owner

Current Issue Owner: @eVoloshchak

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0178a9407eab9689fe

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @eVoloshchak (External)

[paultsimura]

This looks like a pretty normal behavior for decimals, isn't that expected?🤔

[abzokhattab]

Proposal

Please re-state the problem that we are trying to solve in this issue.

if the last digit is a 0 in amount we dont show it

What is the root cause of that problem?

in this function whe dividing over 100 the last digit is removed if its 0

App/src/libs/CurrencyUtils.ts

Lines 91 to 93 in 82b76c1

	function convertToFrontendAmount(amountAsInt: number): number {
	return Math.trunc(amountAsInt) / 100.0;
	}

What changes do you think we should make in order to solve the problem?

we can force it to always return two decimal digits after the comma since the max allowed numbers of digits after the comma is two:

function convertToFrontendAmount(amountAsInt: number): string {
    const amount = Math.trunc(amountAsInt) / 100.0;
    return amount.toFixed(2);
}

Or we can just check if the last digit is not then return the amount as in the old calculations but i would prefer not to have edge cases.

[cjoshidev]

Proposal

Please re-state the problem that we are trying to solve in this issue.

If the trailing digit is 0 then it is getting truncated. Show 0 digits upto 2 decimals for amounts having significant number after the decimal

What is the root cause of that problem?

in this function when dividing over 100 the last digit is removed if its 0

App/src/libs/CurrencyUtils.ts

Lines 91 to 93 in 82b76c1

	function convertToFrontendAmount(amountAsInt: number): number {
	return Math.trunc(amountAsInt) / 100.0;
	}

in the above function trailing 0 is getting truncated which is the root cause of the problem
``

What changes do you think we should make in order to solve the problem?

We can make changes in the code such it will keep the trailing zero in case there is any significant digit after decimal and not in case it is a whole number which will be the edge case scenario.

So based on the bug if it is 23.10 it should display the same in amount input without truncating.

and also in my solution for eg. 24.00 will be shown as 24 since trailing zeros after decimals don't have any significant digits.

keep the definition of the function intact without changing its return type.

function convertToFrontendAmount(amountAsInt: number): number {
    // Convert the input integer to a floating-point number by dividing it by 100.0
    const amount = Math.trunc(amountAsInt) / 100.0;
    // Check if the decimal part is zero, then remove it
    if (amount % 1 === 0) {
        return amount;
    }
    // Return the resulting floating-point number
    return Number(amount.toFixed(2));
}

What alternative solutions did you explore? (Optional)

[allgandalf]

I agree with @paultsimura and @cjoshi-zeals, thinking of UX, it would also help users to change the value more conveniently as compared to having to cancel the 0 and input a digit again :)

[melvin-bot]

📣 @Aditya-svg-code! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[Aditya-svg-code]

Proposal

Please re-state the problem that we are trying to solve in this issue.

when dividing the number by 100.0 it's unable to display the 0 after the decimal.

What is the root cause of that problem?

in this function when dividing over 100.0 the unit digit is removed if it is 0.
for eg. if amountAsInt = 2370 then 2370/100.0 = 23.7 but we want 23.70

App/src/libs/CurrencyUtils.ts

Lines 91 to 93 in 82b76c1

 function convertToFrontendAmount(amountAsInt: number): number { 
     return Math.trunc(amountAsInt) / 100.0; 
 } 

What changes do you think we should make to solve the problem?

When you perform a division operation, the result is often displayed with the minimum required precision. To format the result with a specific number of decimal places, you can use the toFixed() method.

toFixed() method returns a string, so if you need to perform further calculations, you may need to convert it back to a number using parseFloat(). Here's how you can fix the issue.

function convertToFrontendAmount(amountAsInt: number): number {
    const result = Math.trunc(amountAsInt) / 100.0;
    return parseFloat(result.toFixed(2));
}

Additionally, if amountAsInt = 2300 then the previous function was providing the result as 23.0 only, but in my solution it will return 23.00 as the result, usually in bills we should show the numbers after the decimal point even if it is 0, it helps the customer understand the visibility of the payment.

[Aditya-svg-code]

Contributor details :
Expensify account email: soni96996@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~015162f5f4b61238ea

[melvin-bot]

Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[twisterdotcom]

So, this was reported by me (which is why I added the label instead of taking it on), but I disagree that we shouldn't show the last 0. I agree 12.30 is the same as 12.3, but I found it odd that we didn't show the last character. If you asked your mate or your company to send you $12.30, you wouldn't ask them to send you $12.3.

[eVoloshchak]

I agree with @twisterdotcom, this doesn't look consistent
https://github.com/Expensify/App/assets/9059945/00e62fe4-c2b1-492e-9c34-d2495b486bb2
Banking apps also do this, if you put 12.3 it will still be displayed as 12.30 on the next step

@abzokhattab, I think this is the best proposal so far, but it has one problem - it changes the return type of convertToFrontendAmount from number to string. We need it to be number to calculate isTaxAmountInvalid

@cjoshi-zeals, your proposal doesn't work for me, could you double-check it please?

and also in my solution for eg. 24.00 will be shown as 24 since trailing zeros after decimals don't have any significant digits

We should display 24.00 in this case. 24 is displayed as 24.00 on the previous screen, those two should be identical

Screen.Recording.2024-01-24.at.23.57.22.mov

@Aditya-svg-code, your proposal also doesn't work for me, could you double-check?

[abzokhattab]

@abzokhattab, I think this is the best #34894 (comment) so far, but it has one problem - it changes the return type of convertToFrontendAmount from number to string. We need it to be number to calculate isTaxAmountInvalid

converting the output to a number will remove the leading zeros, thus we can handle this case ( isTaxAmountInvalid individually by converting it into a number

we can do that by making the following two functions:

function convertToFrontendAmountAsString(amountAsInt: number): string {
    return convertToFrontendAmount(number).toFixed(2);
}

function convertToFrontendAmount(amountAsInt: number): number {
 return Math.trunc(amountAsInt) / 100.0;
}

then in the isTaxAmountInvalid we need to use the convertToFrontendAmount method and in other occurrences we can use the other method

[cjoshidev]

We should display 24.00 in this case. 24 is displayed as 24.00 on the previous screen, those two should be identical

@eVoloshchak

In that case, we just need to remove the condition for checking if there is no significant digit after the decimal, here is my updated solution

in the below solution will keep the return type of the function the same and will also deliver the desired output.

function convertToFrontendAmount(amountAsInt: number): number {
    // Convert the input integer to a floating-point number by dividing it by 100.0
    const amount = Math.trunc(amountAsInt) / 100.0;
    
    // Return the resulting floating-point number
    return Number(amount.toFixed(2));
}

[Aditya-svg-code]

I agree with @twisterdotcom, this doesn't look consistent https://github.com/Expensify/App/assets/9059945/00e62fe4-c2b1-492e-9c34-d2495b486bb2 Banking apps also do this, if you put 12.3 it will still be displayed as 12.30 on the next step

@abzokhattab, I think this is the best proposal so far, but it has one problem - it changes the return type of convertToFrontendAmount from number to string. We need it to be number to calculate isTaxAmountInvalid

@cjoshi-zeals, your proposal doesn't work for me, could you double-check it please?

and also in my solution for eg. 24.00 will be shown as 24 since trailing zeros after decimals don't have any significant digits

We should display 24.00 in this case. 24 is displayed as 24.00 on the previous screen, those two should be identical

Screen.Recording.2024-01-24.at.23.57.22.mov
@Aditya-svg-code, your proposal also doesn't work for me, could you double-check?

@eVoloshchak I have double checked my solution and the issue is that converting it to the number and the leading zero after decimal is getting removed but if we keep it string then it is working correctly, and for the further calculation we will create a function convertToFrontendAmountAsNumber() that return a number for the calculation.

This is the function which will return string for the bill purpose.

function convertToFrontendAmount(amountAsInt: number): string { 
    const result = Math.trunc(amountAsInt) / 100.0;
    return (result.toFixed(2));              //return the value of result as string
}

This is the function which will return number for the furthur calculation.

 function convertToFrontendAmountAsNumber(amountAsInt: number): number { 
     return Math.trunc(amountAsInt) / 100.0; 
 } 

in isTaxAmountInvalid we need to use convertToFrontendAmountAsNumber() to get a number for calculation.

[sakluger]

I agree with @twisterdotcom - dollar amounts should always be reflected with two decimals.

[allgandalf]

Proposal

Please re-state the problem that we are trying to solve in this issue.

When amending the amount, if the last digit is a 0, it isn't displayed

What is the root cause of that problem?

In initializeAmount, we pass the variable newAmount to the function convertToFrontendAmount

App/src/pages/iou/steps/MoneyRequestAmountForm.js

Lines 121 to 122 in 0d0b0a8

	const initializeAmount = useCallback((newAmount) => {
	const frontendAmount = newAmount ? CurrencyUtils.convertToFrontendAmount(newAmount).toString() : '';

But here as we pass a numerical value, anything of the format 9.70, 9.700, 9.7000 will be considered as 9.7 by default by javascript variables:

What changes do you think we should make in order to solve the problem?

Instead of updating the currencyUtil, i propose that we update initializeAmount to fix the amount to 2 decimal places by using toFixed(2) as shown below:

const initializeAmount = useCallback((newAmount) => {
    const frontendAmount = newAmount ? 
        (newAmount.includes('.') ? parseFloat(CurrencyUtils.convertToFrontendAmount(newAmount)).toFixed(2) : CurrencyUtils.convertToFrontendAmount(newAmount)) 
        : '';

Result:

Screencast.from.01-26-2024.12.22.18.AM.webm

What alternative solutions did you explore? (Optional)

N/A

[allgandalf]

@abzokhattab, I think this is the best #34894 (comment) so far, but it has one problem - it changes the return type of convertToFrontendAmount from number to string. We need it to be number to calculate isTaxAmountInvalid

@eVoloshchak , By taking into consideration your comments on above proposals, i have proposed a different solution which doesn't have to deal with the CurrencyUtils, looking forward to your review :)

Proposal

[dangrous]

Yep that's correct - the bot does not do well with regressions.

We're waiting on review/merge of #43816 which should solve #43215

[sakluger]

Thanks @dangrous for clarifying! I've updated the title to be a bit more clear, and I'm going to move this back to weekly while we wait for #43816 to be merged & deployed.

[sakluger]

The PR is still not merged.

[dangrous]

PR in review! Hopefully should get merged today

[dangrous]

we're merged!

[dangrous]

No automation for some reason, but this has been merged and deployed. Should be ready for payment on 7/26

[sakluger]

Thanks @dangrous 👍 - I updated the title accordingly and will come back tomorrow to make payments.

[sakluger]

Summarizing payment on this issue:

Contributor: @abzokhattab $500, paid via Upwork
Contributor+: @eVoloshchak $500, please request on Newdot

[sakluger]

@eVoloshchak can you please complete the BZ checklist?

[sakluger]

Bump @eVoloshchak

[eVoloshchak]

• The PR that introduced the bug has been identified. Link to the PR: this was the original implementation
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A

Regression Test Proposal

1. Click on FAB
2. Select Request Money -> Manual tab
3. Enter an amount that has 0 as the last decimal digit (e.g. 23.20)
4. Click on Next > go back using the header back button
5. Ensure the amount shown to the user is the same as in step 4 (the trailing zero isn't cut off)

Do we agree 👍 or 👎

[sakluger]

Thanks!

[JmillsExpensify]

$500 approved for @eVoloshchak