[HOLD for payment 2022-05-20] [$1000] Workspace - Unable delete the unlimited numbers under rate in Reimburse expenses page

[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!

---

Action Performed:

1. Go to https://staging.new.expensify.com/
2. Log in with expensifail account
3. Go to Setting - Workspace - Reimburse expenses
4. Enter unlimited numbers and unlimited numbers after point
5. Try to delete the numbers

Expected Result:

Limiting rate to 8 digits (before the decimal). We should still maintain 3 decimal places because that's the default IRS rate. E.g 12345678.123

Actual Result:

Unable to delete the numbers under rate in Reimburse expenses page

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.1.46.0

Reproducible in staging?: Y

Reproducible in production?: Y

Email or phone of affected tester (no customers): applausetester+0707abb@applause.expensifail.com / Feya87Katya

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos:

Bug5505054_Recording__115.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause

Slack conversation:

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @Gonals (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[parasharrajat]

Lol. This is a good one. We didn't notice that the browser will use the e+ scientific number format.

[MelvinBot]

@Gonals Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

Triggered auto assignment to @kadiealexander (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[Gonals]

Looks like a pretty good issue! Setting as external and weekly

[kadiealexander]

Posted to Upwork!

Internal: https://www.upwork.com/ab/applicants/1508929160732057600/job-details
External: https://www.upwork.com/jobs/~0170d8f13e30bba89f

[parasharrajat]

This is missing Exported label.

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @pecanoro (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[kadiealexander]

Good catch, thanks @parasharrajat!

[parasharrajat]

Please let us know the final expected behavior.

[trjExpensify]

Cool, so I'd vote we go for limiting to 8 digits (before the decimal) like Request money. We still need to maintain 3 decimal places because that's the default IRS rate. E.g 12345678.123. Thoughts?

[izhan]

Yeah anything less than Number.MAX_INT (which is 9007199254740991) works, so 8 digits is reasonable! Best place IMO is putting it in the regex.

Out of curiosity - is there a reason we are correcting the value in the debounce function, and not in setRate when we set it? We already enforce the format on input using the regex, so as a user I was surprised that the UI formatted my input a 2nd time after a delay. Would it be better UX if we just consolidate all formatting logic to be on input only?

[parasharrajat]

I am good with #8309 (comment). What do you think @pecanoro?

is there a reason we are correcting the value in the debounce function, and not in setRate when we set it?

So that users can type freely and modify the value if needed.

[pecanoro]

Yes! It sounds good, I doubt the rate will go to anything closer to that ever, or at least I doubt we will be here to see it hahaha

[parasharrajat]

Can someone update the Expected behavior with #8309 (comment)?

Accepting proposals now for the new behavior......

[izhan]

I'm somewhat new to this – I imagine I can just resubmit a proposal w/ a solution only (given I explained the problem above)?

Proposal

Solution

Change the regex we use in setRate to only limit to 8 characters. So changing this line:

App/src/CONST.js

Line 633 in 7029b6f

RATE_VALUE: /^\d+(\.\d*)?$/,

to /^\d{1,8}(\.\d*)?$/

This has the intended behavior, as shown below

Quick UX note: this makes the behavior more in line with that of Request Money, i.e. users cannot enter > 8 digits. We could alternatively add this check AFTER the setState (when we sent it to the server), but that leads to funky UX questions, i.e. how should we truncate long input? So IMO this is probably best, though open to other ideas

[parasharrajat]

I am also confused to determine the best solution for this problem. we recently published some rules which restrict us from manipulating the user input.

Need help from @luacmartins to determine the best course here. 🙇

[luacmartins]

@parasharrajat what part exactly do you need help with? I think it's fine to limit the rate to 8 digits before decimal and 3 decimal digits.

[parasharrajat]

Ok, Thanks. I will review the proposal tomorrow.

[izhan]

Hey @parasharrajat - no rush, but just wanted to see if there's anything I can provide here!

[parasharrajat]

Sorry, I was sick for few days past week. Looking into now.

[parasharrajat]

@izhan's proposal is looking good.

cc: @pecanoro

🎀 👀 🎀 C+ reviewed

[pecanoro]

@izhan What happens when you input more than 3 decimals? Because the original problem was that those decimals couldn't be removed and it seems the regex still let you input more than 3.

[izhan]

I think that was never an issue (here I'm adding >3 decimals on staging)

Screen.Recording.2022-04-28.at.11.57.46.AM.mov

I think the original issue was removing decimals of any large number that converted to scientific notation, which this PR should fix with the 8-digit limit

[pecanoro]

Ohh you are right, the scientific notation was the issue here. Perfect, all good then, go ahead and apply for the job!

[melvin-bot]

📣 @izhan You have been assigned to this job by @pecanoro!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[izhan]

PR is here #8838

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[kadiealexander]

@izhan - paid!
@parasharrajat - contract sent over, please let me know when you accept!

[parasharrajat]

@kadiealexander Done.

[kadiealexander]

All paid!! Thanks team!