[Payment due 3-7][$500] [MEDIUM] Connect Bank Account - No limit for entering amount in transition field

[kbecciv]

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: v1.4.12-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: Applause - Internal Team
Slack conversation:

Action Performed:

1. Open the app
2. Login with any account
3. Go to Settings -> Workspaces -> Open any WS or create it -> Bank account -> Connect manually
4. Use our Fake Test account and Save and Continue through all these steps:

• Routing 011401533
• Account Number 1111222233331111
• Company Name create any name, address, phone, tax ID, company type, incorporation date
• Personal Info create any name, address, SSN can be 3333
• Go through the Onfido step of uploading an ID/passport

5. When you finish the flow - the modal asks for 3 amounts of test transactions
6. There is no character limit on the 3 test transaction fields of the Test Transactions page

Expected Result:

Limit for entering an amount in the transition field presents

Actual Result:

No limit for entering amount in transition field

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

Bug6313132_1702563512164.Record_2023-12-14-11-27-51_4f9154176b47c00da84e32064abf1c48.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ec80ed70582379a7
• Upwork Job ID: 1735304593582518272
• Last Price Increase: 2024-01-04
• Automatic offers:
• abdulrahuman5196 | Reviewer | 0
• ZhenjaHorbach | Contributor | 0

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01ec80ed70582379a7

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[ZhenjaHorbach]

Proposal

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

Connect Bank Account - No limit for entering amount in transition field

What is the root cause of that problem?

We don't set maxLength for InputWrapper

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

We can set maxLength for InputWrapper

App/src/pages/ReimbursementAccount/ValidationStep.js

Lines 160 to 168 in 207d4ac

	<InputWrapper
	InputComponent={TextInput}
	inputID="amount1"
	shouldSaveDraft
	containerStyles={[styles.mb1]}
	placeholder="1.52"
	inputMode={CONST.INPUT_MODE.DECIMAL}
	role={CONST.ROLE.PRESENTATION}
	/>

For value, we can add a new constant
For example: VALIDATION_REIMBURSEMENT_INPUT_LIMIT: 20

And then we can update this input like

     <InputWrapper 
         InputComponent={TextInput} 
         inputID="amount1" 
         shouldSaveDraft 
         containerStyles={[styles.mb1]} 
         placeholder="1.52" 
         inputMode={CONST.INPUT_MODE.DECIMAL} 
         maxLength={CONST.VALIDATION_REIMBURSEMENT_INPUT_LIMIT}
         role={CONST.ROLE.PRESENTATION} 
     /> 

Also we need make the same for other Inputs

App/src/pages/ReimbursementAccount/ValidationStep.js

Lines 169 to 186 in 207d4ac

	<InputWrapper
	InputComponent={TextInput}
	inputID="amount2"
	shouldSaveDraft
	containerStyles={[styles.mb1]}
	placeholder="1.53"
	inputMode={CONST.INPUT_MODE.DECIMAL}
	role={CONST.ROLE.PRESENTATION}
	/>
	<InputWrapper
	InputComponent={TextInput}
	shouldSaveDraft
	inputID="amount3"
	containerStyles={[styles.mb1]}
	placeholder="1.54"
	inputMode={CONST.INPUT_MODE.DECIMAL}
	role={CONST.ROLE.PRESENTATION}
	/>

What alternative solutions did you explore? (Optional)

NA

[neonbhai]

Proposal

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

Connect Bank Account - No limit for entering amount in transition field

What is the root cause of that problem?

We don't pass the maxLength prop here:

App/src/pages/ReimbursementAccount/ValidationStep.js

Lines 159 to 187 in 207d4ac

	<View style={[styles.mv5]}>
	<InputWrapper
	InputComponent={TextInput}
	inputID="amount1"
	shouldSaveDraft
	containerStyles={[styles.mb1]}
	placeholder="1.52"
	inputMode={CONST.INPUT_MODE.DECIMAL}
	role={CONST.ROLE.PRESENTATION}
	/>
	<InputWrapper
	InputComponent={TextInput}
	inputID="amount2"
	shouldSaveDraft
	containerStyles={[styles.mb1]}
	placeholder="1.53"
	inputMode={CONST.INPUT_MODE.DECIMAL}
	role={CONST.ROLE.PRESENTATION}
	/>
	<InputWrapper
	InputComponent={TextInput}
	shouldSaveDraft
	inputID="amount3"
	containerStyles={[styles.mb1]}
	placeholder="1.54"
	inputMode={CONST.INPUT_MODE.DECIMAL}
	role={CONST.ROLE.PRESENTATION}
	/>
	</View>

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

We should pass the maxLength prop like we do in other areas of the app:

maxLength={CONST.FORM_CHARACTER_LIMIT}

Alternatively

Considering the validation transaction digit limit is very short 3/5, we can have another const for this

VALIDATION_TRANSACTION_LIMIT: 3

maxLength={CONST.VALIDATION_TRANSACTION_LIMIT}

[sfurqan2]

Dont you think we should create a new CONST to handle this? I dont think 50 is the precise limit either.

So we can add a const like CONST.BANK_ACCOUNT.MAX_LENGTH.AMOUNT and specify the value there. As these are test amounts i dont think they will be more than 6 digits.

What do you think? @ZhenjaHorbach @neonbhai

[neonbhai]

@sfurqan2 Thank you for the comment, it does seem like 50 might be too much for the verification transaction amount, but I've suggested the limit since we use this const globally for form character limit. Let's see if the C+ team has thoughts on this while reviewing.

[neonbhai]

Proposal

Updated

Added an Alternative approach with another const limiting the text field to few characters for better UX

[Christinadobrzyn]

I think this does need to be fixed (maybe a medium priority) and could be fixed as part of the VBA updates on Wave 6 - going to add to that wave for review to determine the priority of fixing this.

[melvin-bot]

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

[abdulrahuman5196]

Will review today.

[abdulrahuman5196]

Reviewing now

[abdulrahuman5196]

@Christinadobrzyn some questions on the issue

1. What is the limit we are expecting? like 10 digits or something else

2. Could you share steps to test "Pending State Withdrawal Account" ?

[Christinadobrzyn]

Great questions! working on these answers with our curators https://expensify.slack.com/archives/C02MW39LT9N/p1703024244988059

I'll follow up as soon as possible!

[Christinadobrzyn]

working on the answers to #33065 (comment)

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[Christinadobrzyn]

Hi @abdulrahuman5196 answers to your question:

• What is the limit we are expecting? like 10 digits or something else - let's make it a 20 digit limit

• Could you share steps to test "Pending State Withdrawal Account"? Yes, I added those details to the OP.

Let me know if you need anything else!

[abdulrahuman5196]

Hi, Are we tracking to fix this internally or with the already reviewed proposal?

[melvin-bot]

Current assignee @abdulrahuman5196 is eligible for the External assigner, not assigning anyone new.

[NikkiWines]

Oh, this can definitely be external. @ZhenjaHorbach's proposal you reviewed earlier looks good. @ZhenjaHorbach are you still interested in taking on this issue?

If not, we'll open this up to new proposals!

[ZhenjaHorbach]

Oh, this can definitely be external. @ZhenjaHorbach's proposal you reviewed earlier looks good. @ZhenjaHorbach are you still interested in taking on this issue?

If not, we'll open this up to new proposals!

Hello )
I'm interested )

[melvin-bot]

📣 @abdulrahuman5196 🎉 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

[melvin-bot]

📣 @ZhenjaHorbach 🎉 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 📖

[ZhenjaHorbach]

📣 @ZhenjaHorbach 🎉 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 📖

PR will be ready today or tomorrow

[Christinadobrzyn]

PR is in staging - getting close! #37106

[abdulrahuman5196]

The PR that introduced the bug has been identified. Link to the PR:
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:
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:

No a regression. Seems to be the case for sometime.

Determine if we should create a regression test for this bug.
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.

No. Wouldn't be benificial to add for this UX case bug.

@Christinadobrzyn Added BZ checklist. This is for payment due on March 7th. But seems melvin didn't auto update here.

[Christinadobrzyn]

Thanks for the heads up @abdulrahuman5196!

Can we confirm this is the payment for March 7th (assuming no regressions)?

Contributor: $500 @ZhenjaHorbach (Paid in Upwork - https://www.upwork.com/nx/wm/offer/101035377)
Contributor+: $500 @abdulrahuman5196 (Paid in Upwork - https://www.upwork.com/nx/wm/offer/101035373)

Upwork job is here.

[abdulrahuman5196]

@Christinadobrzyn I think the offer has been accepted already. Do let me know if there is some issue.

[Christinadobrzyn]

No issue with the offer - just adding them to the payment summary for tracking/easy reference.

This is paid out - closing!