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

[Hold until 2021-10-14] IOU - Unable to enter some combination of decimal numbers #5579

Closed
kavimuru opened this issue Sep 29, 2021 · 21 comments
Assignees
Labels
Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review

Comments

@kavimuru
Copy link

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. Navigate to a conversation
  3. Click on the + button in the compose box.
  4. Select Request Money
  5. Try to enter 19.1, 19.4, 19.6, 19.9 or try to enter 1, 4, 6, 9 after decimal point for the numbers 16, 17, 18 (18.4 or 17.9)

Expected Result:

User should be able to enter any combination of decimal numbers

Actual Result:

Unable to enter decimal after 16, 17, 18, 19 which start 1,4,6,9

Workaround:

Unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.1.3-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

https://user-images.githubusercontent.com/43996225/135196204-e369f854-5f66-4d11-af31-6938cfb07aba.mp4
Bug5257495_19

Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:

View all open jobs on GitHub

@MelvinBot
Copy link

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

@PrashantMangukiya
Copy link
Contributor

PrashantMangukiya commented Sep 29, 2021

If considered as external, this is Proposed Solution:

Problem is with line 104 in IOUAmountPage.js
We are doing parseFloat and multiply with 100. Whenever we parse float it can results with 16 decimal places depending on amount value. So whenever it returns long decimal places it failed CONST.IOU.AMOUNT_MAX_LENGTH condition, so we can not enter specific amount.

This article shows how float works: Click here to read

validateAmount(amount) {
const decimalNumberRegex = new RegExp(/^\d+(,\d+)*(\.\d{0,3})?$/, 'i');
return amount === '' || (decimalNumberRegex.test(amount) && ((parseFloat(amount) * 100).toString().length <= CONST.IOU.AMOUNT_MAX_LENGTH));
}

Here is solution: We have to fix (parseFloat(amount) * 100) result to 3 decimal places. Below is updated code.

validateAmount(amount) { 
  const decimalNumberRegex = new RegExp(/^\d+(,\d+)*(\.\d{0,3})?$/, 'i'); 
  return amount === '' || (decimalNumberRegex.test(amount) && (parseFloat((amount * 100).toFixed(3)).toString().length <= CONST.IOU.AMOUNT_MAX_LENGTH)); // *** UPDATED CODE
} 

Note: At present I am not sure why parse float multiplied with 100 i.e. ((parseFloat(amount) * 100) , may be to take care for cents handling etc. So I am not touching that part. This ((parseFloat(amount) * 100) logic added in this pr commit list
https://github.com/Expensify/App/pull/5243/commits

Below is screen record, It works proper after correction:

Web

Web.mov

Desktop

Desktop.mov

Mobile Web

Mobile.Web.mov

iOS

iOS.mov

Android

Android.mov

I understand this bug is important to correct asap. If considered external then please message me, I will submit PR immediately. Thanks.

@deetergp
Copy link
Contributor

deetergp commented Sep 29, 2021

Note: At present I am not sure why parse float multiplied with 100

@PrashantMangukiya it is because we store amounts in cents on the back end. (API reference)

@deetergp deetergp added the External Added to denote the issue can be worked on by a contributor label Sep 29, 2021
@MelvinBot
Copy link

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

@PrashantMangukiya
Copy link
Contributor

@deetergp ok go It. So it multiply with 100 is needed in this case. Also thanks for api reference link.

If my solution accepted then please message me. I will submit pr soon.

@deetergp
Copy link
Contributor

deetergp commented Sep 29, 2021

I just went on a journey into currencies and decimal places! TIL about the Chilean Unidad de Fomento which can have four decimal places (but is a non-circulating currency).

@puneetlath: @PrashantMangukiya's proposal is solid 👍

@deetergp deetergp added Weekly KSv2 Daily KSv2 and removed Daily KSv2 Weekly KSv2 labels Sep 29, 2021
@isagoico
Copy link

isagoico commented Oct 4, 2021

Issue reproducible during KI retests

@puneetlath
Copy link
Contributor

Sounds good. @PrashantMangukiya I've invited you to the Upwork job!

@MelvinBot MelvinBot removed the Overdue label Oct 4, 2021
@ritusharma4
Copy link

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. Navigate to a conversation
  3. Click on the + button in the compose box.
  4. Select Request Money
  5. Try to enter 19.1, 19.4, 19.6, 19.9 or try to enter 1, 4, 6, 9 after decimal point for the numbers 16, 17, 18 (18.4 or 17.9)

Expected Result:

User should be able to enter any combination of decimal numbers

Actual Result:

Unable to enter decimal after 16, 17, 18, 19 which start 1,4,6,9

Workaround:

Unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.1.3-0 Reproducible in staging?: Yes Reproducible in production?: Yes Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos:

https://user-images.githubusercontent.com/43996225/135196204-e369f854-5f66-4d11-af31-6938cfb07aba.mp4 Bug5257495_19

Expensify/Expensify Issue URL: Issue reported by: Slack conversation:

View all open jobs on GitHub

I have Experienced with this.

@ritusharma4
Copy link

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. Navigate to a conversation
  3. Click on the + button in the compose box.
  4. Select Request Money
  5. Try to enter 19.1, 19.4, 19.6, 19.9 or try to enter 1, 4, 6, 9 after decimal point for the numbers 16, 17, 18 (18.4 or 17.9)

Expected Result:

User should be able to enter any combination of decimal numbers

Actual Result:

Unable to enter decimal after 16, 17, 18, 19 which start 1,4,6,9

Workaround:

Unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.1.3-0 Reproducible in staging?: Yes Reproducible in production?: Yes Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos:

https://user-images.githubusercontent.com/43996225/135196204-e369f854-5f66-4d11-af31-6938cfb07aba.mp4 Bug5257495_19

Expensify/Expensify Issue URL: Issue reported by: Slack conversation:

View all open jobs on GitHub

I have invited to you on Upwork.

@PrashantMangukiya
Copy link
Contributor

@puneetlath thank you. Just submitted proposal on upwork. Kindly accept it so I can proceed to submit pr.

@puneetlath
Copy link
Contributor

@PrashantMangukiya hired!

@PrashantMangukiya
Copy link
Contributor

@puneetlath Thank you. Preparing the pr now and submit soon.

@PrashantMangukiya
Copy link
Contributor

@deetergp PR #5628 is ready for review.

@deetergp deetergp added the Reviewing Has a PR in review label Oct 5, 2021
@PrashantMangukiya
Copy link
Contributor

PR for this issue is deployed to production version: 1.1.6-0 but awaiting payment label not applied to this issue. I think this may be GitHub automation glitch etc.

@puneetlath puneetlath changed the title IOU - Unable to enter some combination of decimal numbers [Hold until 2021-10-14] IOU - Unable to enter some combination of decimal numbers Oct 8, 2021
@puneetlath
Copy link
Contributor

Thanks @PrashantMangukiya. I updated the title manually and will pay on Oct 14 if there are no regressions.

@mvtglobally
Copy link

Issue not reproducible during KI retests (First Week)

@PrashantMangukiya
Copy link
Contributor

Ping for
upwork

@puneetlath
Copy link
Contributor

Whoops sorry for the delay. Created the job and invited @PrashantMangukiya.

@PrashantMangukiya
Copy link
Contributor

@puneetlath thanks for the message.

I think Job for this issue was created and assigned to me before. So I think no need for new job now.

You can just release payment from job assigned to me for this issue. Here is assigned job url. https://www.upwork.com/jobs/IOU-Unable-enter-some-combination-decimal-numbers-5579-Expensify_~014448c7e8eb9c5eea

Thanks.

@puneetlath
Copy link
Contributor

Thanks for the clarification @PrashantMangukiya. I was moving a bit too fast 😅. Paid!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

8 participants