[HOLD for payment 2024-11-01] [$250] Bank account - User is able to edit account number but the change does not take affect

[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: 9.0.40-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5009304
Email or phone of affected tester (no customers): applausetester+vd_ios092524@applause.expensifail.com
Issue reported by: Applause - Internal Team

Action Performed:

Pre-requisite: user must have created a Workspace and have enabled Workflows

1. Go to Settings > Workspaces > Workspace > Workflows
2. Click on "Connect bank account"
3. Select "Connect manually"
4. Enter "011401533" in routing number field
5. Enter "1111222233334444" in account number field and confirm
6. Go back by clicking on the arrow on the top left of the modal
7. Verify the user is able to edit the account number
8. Delete the "4444" part and enter "1111", then confirm
9. Go back again
10. Verify the account number has not changed

Expected Result:

Behavior should be consistent, if the user is able to edit the account number, the change should take affect

Actual Result:

Behavior is not consistent, the user is able to edit the account number but the changed does not take effect

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

Bug6615499_1727311085437.bandicam_2024-09-25_20-29-30-959.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021840583568266999203
• Upwork Job ID: 1840583568266999203
• Last Price Increase: 2024-09-30
• Automatic offers:
• suneox | Reviewer | 104229719
• DylanDylann | Contributor | 104533160
• cretadn22 | Contributor | 104533363

Issue Owner

Current Issue Owner: @jliexpensify

[melvin-bot]

Triggered auto assignment to @jliexpensify (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.

[bernhardoj]

Proposal

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

Bank account number is editable but reverted to previous value when saved.

What is the root cause of that problem?

When we try to edit the bank account number, the BE will return back the previous entered value.

So, it looks like we are not allowed to change the number after we entered it and need to start over the process.

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

Disable the bank account number field if we already entered it. We can check using hasBankAccountData to disable it.

App/src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx

Lines 78 to 101 in da37289

	<InputWrapper
	InputComponent={TextInput}
	ref={inputCallbackRef}
	inputID={BANK_INFO_STEP_KEYS.ROUTING_NUMBER}
	label={translate('bankAccount.routingNumber')}
	aria-label={translate('bankAccount.routingNumber')}
	role={CONST.ROLE.PRESENTATION}
	defaultValue={defaultValues[BANK_INFO_STEP_KEYS.ROUTING_NUMBER]}
	inputMode={CONST.INPUT_MODE.NUMERIC}
	shouldSaveDraft
	shouldUseDefaultValue={hasBankAccountData}
	/>
	<InputWrapper
	InputComponent={TextInput}
	inputID={BANK_INFO_STEP_KEYS.ACCOUNT_NUMBER}
	containerStyles={[styles.mt6]}
	label={translate('bankAccount.accountNumber')}
	aria-label={translate('bankAccount.accountNumber')}
	role={CONST.ROLE.PRESENTATION}
	defaultValue={defaultValues[BANK_INFO_STEP_KEYS.ACCOUNT_NUMBER]}
	inputMode={CONST.INPUT_MODE.NUMERIC}
	shouldSaveDraft
	shouldUseDefaultValue={hasBankAccountData}
	/>

disabled={hasBankAccountData}

[cretadn22]

Proposal

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

The user can edit the account number, but the change does not take effect.

What is the root cause of that problem?

When the user enters the bank account number and routing number, we need to verify them on the backend before the user moves to the next step. If the user edits the bank account number again, the backend still retain the previous value.

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

We should disable both the routing number field and the bank account number field if hasBankAccountData is true.

Add disabled={hasBankAccountData} to two places

App/src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx

Lines 78 to 101 in da37289

	<InputWrapper
	InputComponent={TextInput}
	ref={inputCallbackRef}
	inputID={BANK_INFO_STEP_KEYS.ROUTING_NUMBER}
	label={translate('bankAccount.routingNumber')}
	aria-label={translate('bankAccount.routingNumber')}
	role={CONST.ROLE.PRESENTATION}
	defaultValue={defaultValues[BANK_INFO_STEP_KEYS.ROUTING_NUMBER]}
	inputMode={CONST.INPUT_MODE.NUMERIC}
	shouldSaveDraft
	shouldUseDefaultValue={hasBankAccountData}
	/>
	<InputWrapper
	InputComponent={TextInput}
	inputID={BANK_INFO_STEP_KEYS.ACCOUNT_NUMBER}
	containerStyles={[styles.mt6]}
	label={translate('bankAccount.accountNumber')}
	aria-label={translate('bankAccount.accountNumber')}
	role={CONST.ROLE.PRESENTATION}
	defaultValue={defaultValues[BANK_INFO_STEP_KEYS.ACCOUNT_NUMBER]}
	inputMode={CONST.INPUT_MODE.NUMERIC}
	shouldSaveDraft
	shouldUseDefaultValue={hasBankAccountData}
	/>

App/src/pages/ReimbursementAccount/BankInfo/substeps/Manual.tsx

Line 90 in da37289

<InputWrapper

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[melvin-bot]

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

[suneox]

@jliexpensify I would like to confirm if we allow users to edit their bank account numbers during the setup process. If not, we should disable the account bank number field when editing. If we do allow editing, the backend needs to be updated to handle this, and the frontend also validate the bank account number when edit to avoid submitting data with placeholders like XXXXXXXXXXXX1111.

[jliexpensify]

Great call out @suneox - I am not sure myself. It makes sense that you should be able to edit, in case you make a mistake - but actually, on Old.Dot, I don't think you can so maybe the behaviour mirrors this?

Asking - https://expensify.slack.com/archives/C036QM0SLJK/p1727690352864979

[KashifAhmed]

I’d recommend adding a "Start Over" button and disabling both fields when necessary. Otherwise, on the server side, we can update the new value and ensure the old value isn't retained. This should make the user experience smoother.

[melvin-bot]

📣 @KashifAhmed! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[KashifAhmed]

Contributor details
Your Expensify account email: imrkash@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/imrkash

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[jliexpensify]

Got a quick update @suneox - we want to mirror the behaviour of Expensify Classic for editing bank accounts, which is the ability to edit the account number prior to it being verified. Can you evaluate based off this?

(Slack thread for internal engineer's reference)

Also, I am OOO from the 3rd to 14th October, but going to keep myself assigned as I don't think payment will be needed before the 14th.

[suneox]

which is the ability to edit the account number prior to it being verified. Can you evaluate based off this?

@jliexpensify If we want to allow users to edit the account number, BE needs to accept the new value via the ConnectBankAccountManually API, instead of retaining the previous value. On the FE, we will handle preventing the submission of invalid account numbers with patterns like XXXXX....

Screen.Recording.2024-10-01.at.12.04.17.mp4

cc: @bernhardoj @cretadn22 We're waiting updated proposal for preventing the submission of invalid account numbers

[bernhardoj]

we want to mirror the behaviour of Expensify Classic for editing bank accounts, which is the ability to edit the account number prior to it being verified.

I think the point here is that we don't want to allow the user to edit the number if it's verified. In Expensify Classic, the number will be verified after submitting "Company information".

But in New Expensify, the number is immediately verified. So, if we disable it using hasBankAccountData, the behavior will be the same with Expensify Classic. This is the behavior too before this PR #48767.

[jliexpensify]

Oh interesting @bernhardoj, so are you saying if we count this issue as a bug, then this is technically a regression from #48767?

[bernhardoj]

@jliexpensify yes

[jliexpensify]

Great, thanks!

Assigning @mountiny to this issue for review - it looks like #48767 changed the behaviour of the account flow, but it now doesn't mirror Classic.

Is this intended? It sounds like we want to keep the flows the same.

[suneox]

@jliexpensify In the manual bank connection flow from oldDot users can still edit their account numbers.

Screen.Recording.2024-10-01.at.21.34.24.mp4

It seems that @bernhardoj may be using the online connection method and users cannot edit their account numbers.
So, I think the scope of this issue should handle disabling the account number edit option when connecting online and allowing edits when connecting manually.

[bernhardoj]

In Expensify Classic, the number will be verified after submitting "Company information".

I'm doing the manual flow. You need to submit the Company Information first

[mountiny]

@cretadn22 what is your eta for the PR?

[DylanDylann]

@cretadn22 Please help to raise a quick PR

[cretadn22]

I'm creating the PR, and it will be up today.

[bernhardoj]

If we only want to disable the bank account, let's go with @bernhardoj's #49786 (comment) because they go first

Hmm, I didn't explicitly mention both the account and routing number, but you can see in my proposal that I include the routing number too from the permalink.

[DylanDylann]

@bernhardoj I appreciate your contribution. If we have the same case in the future, please help to elaborate it by word for a clear and enough proposal

[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 9.0.53-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Disable bank account and routing number #51234

If no regressions arise, payment will be issued on 2024-11-01. 🎊

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

• @suneox requires payment automatic offer (Reviewer)
• @koko57 does not require payment (Contractor)
• @DylanDylann requires payment automatic offer (Contributor)
• @cretadn22 requires payment automatic offer (Contributor)

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

• [@suneox / @DylanDylann] The PR that introduced the bug has been identified. Link to the PR:
• [@suneox / @DylanDylann] 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:
• [@suneox / @DylanDylann] 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:
• [@suneox / @DylanDylann] Determine if we should create a regression test for this bug.
• [@suneox / @DylanDylann] 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.
• [@jliexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[jliexpensify]

Ok, little bit confusing - but is this Payment Summary correct?

• C: @cretadn22 $250 (to be paid via Upworks)
• C+: @DylanDylann $250 (to be paid via Upworks) - please accept

Upwork job

[DylanDylann]

@jliexpensify Done

[jliexpensify]

Thanks, any checklist needed here @DylanDylann?

[DylanDylann]

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:

[@DylanDylann] The PR that introduced the bug has been identified. Link to the PR: NA
[@DylanDylann] 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: This is more like a new requirement, not a bug
[@DylanDylann] 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: NA
[@DylanDylann] Determine if we should create a regression test for this bug. Yes
[@DylanDylann] 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.

Regression Test Proposal
Pre-requisite: user must have created a Workspace and have enabled Workflows

1. Go to Settings > Workspaces > Workspace > Workflows
2. Click on "Connect bank account"
3. Select "Connect manually"
4. Enter "011401533" in routing number field
5. Enter "1111222233334444" in account number field and confirm
6. Go back by clicking on the arrow on the top left of the modal
7. Verify both routing number and bank account are disabled

Do we agree 👍 or 👎

[DylanDylann]

@jliexpensify Posted the checklist

[jliexpensify]

Paid both, and job closed.

[melvin-bot]

@jliexpensify @mountiny Be sure to fill out the Contact List!