[HOLD for payment 2024-04-09] [$500] On Validate your bank account page, no invalid amount error if the lasts digits are "0"

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.49-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: n/a
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: applause internal team
Slack conversation:

Action Performed:

Pre-requisite: user must be logged in and have created a workspace.

1. Go to Workspace > Bank account.
2. Select "Connect manually".
3. Go trough the add BA flow with the testing credentials until you reach the "Validate your bank account page".
4. On a transaction field, enter any 8 digits.
5. Verify no error is displayed.
6. Verify that if you enter any other digit than "0", an error for invalid amount is displayed.
7. Clear the field.
8. Enter any 8 digits.
9. After the 8 digits, enter "0" many times.
10. Verify no error is displayed.

Expected Result:

The invalid amount error should be displayed if the number entered has more than 8 digits.

Actual Result:

The invalid amount error is not displayed if the last digits are "0".

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

Bug6406504_1709873247666.Fjxr0832_1_.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015f8c244843935a8f
• Upwork Job ID: 1770500300003213312
• Last Price Increase: 2024-03-20

[melvin-bot]

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

[m-natarajan]

@alexpensify 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.

[bernhardoj]

Proposal

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

User can input the test transaction amount with more than 8 digit if the leading and trailing amount is 0s.

What is the root cause of that problem?

The input is validated using the regex as shown below where the max digit is 8 (and 2 digit for decimal).

App/src/pages/ReimbursementAccount/ConnectBankAccount/components/BankAccountValidationForm.tsx

Lines 67 to 75 in a2c24e1

	const amountRegex = RegExp(String.raw`^-?\d{0,8}([${getPermittedDecimalSeparator(decimalSeparator)}]\d{0,${CurrencyUtils.getCurrencyDecimals(outputCurrency)}})?$`, 'i');
	Object.keys(amountValues).forEach((key) => {
	const value = amountValues[key as keyof AmountValues];
	const filteredValue = filterInput(value, amountRegex);
	if (ValidationUtils.isRequiredFulfilled(filteredValue.toString())) {
	return;
	}
	errors[key as keyof AmountValues] = 'common.error.invalidAmount';

It's validating using filterInput function where it will returns an empty string if it's not a valid input.

App/src/pages/ReimbursementAccount/ConnectBankAccount/components/BankAccountValidationForm.tsx

Lines 46 to 53 in a2c24e1

	const filterInput = (amount: string, amountRegex?: RegExp) => {
	let value = amount ? amount.toString().trim() : '';
	value = value.replace(/^0+|0+$/g, '');
	if (value === '' || Number.isNaN(Number(value)) || !Math.abs(Str.fromUSDToNumber(value, false)) || (amountRegex && !amountRegex.test(value))) {
	return '';
	}
	return value;

The problem here is that we test the regex with value variable which omits all leading and trailing 0s from the user input.

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

We should test the regex with the untouched user input, that is amount.

amountRegex && !amountRegex.test(amount)

[allgandalf]

Proposal

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

On Validate your bank account page, no invalid amount error if the lasts digits are "0"

What is the root cause of that problem?

We have current regex which doesn't account for the trailing zeros if there is no decimal value

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

We need to update the amountRegex to consider the cases where there are tailing zeros but no decimal places, this can be done with the following update:

const amountRegex = RegExp(String.raw`^-?(0|[1-9]\d{0,7})([${getPermittedDecimalSeparator(decimalSeparator)}]\d*?[1-9](0{0,${CurrencyUtils.getCurrencyDecimals(outputCurrency)}})?)?$`, 'i');

Also updated filterInput function would be:

const filterInput = (amount: string, amountRegex?: RegExp) => {
    let value = amount ? amount.toString().trim() : '';
    // Remove leading zeros except when the value is '0'
    value = value.replace(/^0+(?!=\.|$)/, '');
    if (value === '' || Number.isNaN(Number(value)) || !Math.abs(Str.fromUSDToNumber(value, false)) || (amountRegex && !amountRegex.test(value))) {
        return '';
    }

    return value;
};

Result Video:

simplescreenrecorder-2024-03-22_19.49.44.mp4

[dragnoir]

@m-natarajan can you pls explain from where did you got the Expected Result: you mentioned!!

The invalid amount error should be displayed if the number entered has more than 8 digits.?

[melvin-bot]

@alexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[alexpensify]

No update yet, I need to test.

[alexpensify]

I'll test this one tomorrow.

[alexpensify]

No update

[alexpensify]

Still on my testing list.

[allgandalf]

bump @alexpensify

[alexpensify]

Perfect timing since I was testing this one right now. Ok, so I confirmed that if you input a all zeros, there will be an error. The issue is isolated to when you input numbers then have trailing zeros.

Error

No Error

---

I believe we should fix this one. If you input a valid number and a trailing zero, the process will try to save the account.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~015f8c244843935a8f

[melvin-bot]

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

[alexpensify]

@hungvu193 - can you please review the proposals to identify if one fixes this issue? Thanks!

[allgandalf]

@alexpensify , you tested this on the bank account page, does this work same as the amount validation page?

[hungvu193]

What do you think about my proposal @hungvu193 , won’t it be better to modify the regex instead?

I think that's fine. However you can check the testcases on your proposal, it failed on these cases where we have trailing zeros or only dot without any number.

[tienifr]

What do you think about my #37974 (comment) @hungvu193 , won’t it be better to modify the regex instead?

@GandalfGwaihir RegEx is expensive. This RegEx you're trying to modify is already a complex one, we shouldn't add more to it, especially when there's much simpler solution that works for all cases and has no disadvantage

[allgandalf]

Thanks for the feedback @hungvu193 @tienifr , will try to improve proposal next time 😄

[melvin-bot]

❌ There was an error making the offer to @hungvu193 for the Reviewer role. The BZ member will need to manually hire the contributor.

[melvin-bot]

❌ There was an error making the offer to @tienifr for the Contributor role. The BZ member will need to manually hire the contributor.

[tienifr]

@hungvu193 PR #39039 is ready to review

[alexpensify]

Cool, now we wait for automation here.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.58-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: no invalid amount error if lasts digit are 0 #39039

If no regressions arise, payment will be issued on 2024-04-09. 🎊

For reference, here are some details about the assignees on this issue:

• @hungvu193 requires payment (Needs manual offer from BZ)
• @tienifr requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@hungvu193] The PR that introduced the bug has been identified. Link to the PR:
• [@hungvu193] 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:
• [@hungvu193] 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:
• [@hungvu193] Determine if we should create a regression test for this bug.
• [@hungvu193] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@alexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[alexpensify]

@tienifr and @hungvu193 - to prepare for the payment process, please accept the invitation for the following Upwork job:

https://www.upwork.com/jobs/~015f8c244843935a8f

Thanks! I'll complete the process tomorrow.

[hungvu193]

@tienifr and @hungvu193 - to prepare for the payment process, please accept the invitation for the following Upwork job:

https://www.upwork.com/jobs/~015f8c244843935a8f

Thanks! I'll complete the process tomorrow.

I've just accepted. Thank you.

[tienifr]

@alexpensify I've accepted as well, thanks!

[alexpensify]

Thanks! Everyone has been paid in Upwork.

Payouts due: 2024-04-09

• Contributor: $500 @tienifr
• Contributor+: $500 @hungvu193

Upwork job is here.