[HOLD for Payment 5-16][$250] Distance rate - Value ending with decimal point in distance rate editor is not accepted

[lanitochka17]

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.65-5
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to FAB > Submit expense > Manual
3. Enter amount ending with decimal point (eg, 8.) > Next
4. Note that app proceeds to next step
5. Go to taxes section in Collect workspace
6. Click Add rate > Value
7. Enter amount ending with decimal point (eg, 8.) > Save
8. Note that the value is accepted
9. Go to Distance rates section > Add rate
10. Enter amount ending with decimal point (eg, 8.) > Save

Expected Result:

The value ending with decimal point in distance rate editor should be accepted

Actual Result:

The value ending with decimal point in distance rate editor is not accepted

Workaround:

Unknown

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

Bug6460730_1713988278794.20240425_034651.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010c566adc64c516d8
• Upwork Job ID: 1783752626404245504
• Last Price Increase: 2024-04-26
• Automatic offers:
• ikevin127 | Reviewer | 0
• GandalfGwaihir | Contributor | 0

[melvin-bot]

Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

We think that this bug might be related to #wave-collect - Release 1

[lanitochka17]

@Christinadobrzyn FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[allgandalf]

Proposal

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

Validation fails for values with only decimal pointer and no digits after that.

What is the root cause of that problem?

For distance rates, We have a validation function which calls validateRateValue:

App/src/pages/workspace/distanceRates/CreateDistanceRatePage.tsx

Line 45 in 9a727de

(values: FormOnyxValues<typeof ONYXKEYS.FORMS.POLICY_CREATE_DISTANCE_RATE_FORM>) => validateRateValue(values, currency, toLocaleDigit),

This validation function has a regex which doesn't allow us to have a value with only decimal point and no digit after that, this causes the validation logic to fail:

App/src/libs/PolicyDistanceRatesUtils.ts

Line 18 in 9a727de

const rateValueRegex = RegExp(String.raw`^-?\d{0,8}([${getPermittedDecimalSeparator(decimalSeparator)}]\d{1,${CurrencyUtils.getCurrencyDecimals(currency) + 1}})?$`, 'i');

Note: Same doesn't happen with tax, we allow the user to enter only decimal point in tax, so we should fix it here.

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

We need to update the regex to:

const rateValueRegex = RegExp(String.raw`^-?\d{0,8}([${getPermittedDecimalSeparator(decimalSeparator)}]\d{0,${CurrencyUtils.getCurrencyDecimals(currency) + 1}})?$`, 'i');

Well update {1, to {0, after the decimal separator part, allowing for zero or more digits after the decimal point.

What alternative solutions did you explore? (Optional)

We also need to update the edit page as well, the bug occurs there too.

[Christinadobrzyn]

I think this might be vsb-split (as I see other BZ Distance GHs linked to that project). Let's see if this can be external

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~010c566adc64c516d8

[melvin-bot]

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

[Christinadobrzyn]

However, I'm also wondering if we want to fix this as explained in the OP or if we want to throw an error. Do we want users to be able to enter a "3." value?

Asking the team what we want as an expected behaviour - https://expensify.slack.com/archives/C05RECHFBEW/p1714115140280529

[allgandalf]

Do we want users to be able to enter a "3." value?

@Christinadobrzyn , we allow the user to enter values like 3. 34. 543., for other fields like, entering amount for IOU, tax rates as well, so I guess we should be consistent with distance rates as well:

Screen.Recording.2024-04-26.at.1.19.10.PM.mov

(We also allow the values like 3. when we edit distance rates as well

[ikevin127]

♻️ Reviewing proposal.

@Christinadobrzyn Please let me know what the team decided based on #40946 (comment), as I don't have access to the Slack 🧵.

[ikevin127]

@GandalfGwaihir's proposal looks good to me. The RCA was correctly identified and the proposed solution fixes the issue accordingly.

🎀👀🎀 C+ reviewed

@lakchote Let's hold assignment until @Christinadobrzyn confirms whether we should go with this fix according to Slack discussion mentioned in #40946 (comment).

[melvin-bot]

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

[Christinadobrzyn]

nudging the team for some thoughts - https://expensify.slack.com/archives/C05RECHFBEW/p1714384313394889

[lakchote]

nudging the team for some thoughts - https://expensify.slack.com/archives/C05RECHFBEW/p1714384313394889

Looks like we want for consistency to treat cases where we're ending with a decimal as 0, can you confirm?

[Christinadobrzyn]

Thanks @lakchote - yes that's my understanding. A user should be able to enter a number (aka amount) ending with a decimal without error. Thank you!

[allgandalf]

I guess we're good to proceed with the PR @lakchote ?

[melvin-bot]

📣 @ikevin127 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[lakchote]

I guess we're good to proceed with the PR @lakchote ?

Yes, your proposal LGTM.

[melvin-bot]

📣 @GandalfGwaihir 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer 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 📖

[allgandalf]

PR ready for review c.c. @ikevin127

[Christinadobrzyn]

reviewing PR - #41503

[ikevin127]

⚠️ Automation failed -> this should be on [HOLD for Payment [2024-05-15]] according to today's production deploy from #41503 (comment).

cc @lakchote @Christinadobrzyn

[Christinadobrzyn]

Payouts due:

• Contributor: $250 @GandalfGwaihir (Upwork offer here - https://www.upwork.com/nx/wm/workroom/37146800/overview)
• Contributor+: $250 @ikevin127 (Upwork offer here - https://www.upwork.com/nx/wm/workroom/37146801/overview)

Upwork job is here.

We'll pay this in a few days! Do we need a regression test?

[Christinadobrzyn]

Paid out based on this payment summary.

I assume we'll create a regression test for this at the end of the polish process? @lakchote?

[ikevin127]

Regression Test Proposal

Preconditions:

We need to have a Collect workspace with Distance rates enabled.

1. Navigate to https://staging.new.expensify.com.
2. Go to Distance rates section in Collect workspace.
3. Click on [+ Add rate] and enter amount ending with decimal point eg. 8. then Save.
4. Verify that you are able to Save the value ending with decimal point.

Do we agree 👍 or 👎.

[Christinadobrzyn]

Regression test - https://github.com/Expensify/Expensify/issues/397047

Payment summary here - #40946 (comment)

Closing as complete!