[Pay 4/29][$500] Wallet - No error when clicking Save & continue button after selecting BA with existing draft

[kbecciv]

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.54-0
Reproducible in staging?: y
Reproducible in production?: y
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com.
2. Go to Profile > Workspaces > any workspace.
3. Go to Bank account > Connect manually.
4. Add the following bank account (important):
   Routing number - 011401533
   Account number - 1111222233331111
5. Proceed the setup and quit in the middle of it (by clicking outside RHP).
6. Go to Wallet > Add bank account > Personal bank account.
7. Add any bank account and select Plaid Saving account (ends with 1111).
8. Click Save & continue.

Expected Result:

App should present an error about unable to proceed with Plaid Saving account (ends with 1111) since there is draft in Step 5.

Actual Result:

There is no error when clicking Save & continue. Nothing happens when clicking on the button.

Workaround:

n/a

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

Bug6418307_1710791246953.20240319_033905.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0183d2c7f9a0e1aa53
• Upwork Job ID: 1772399631405613056
• Last Price Increase: 2024-03-25
• Automatic offers:
• cubuspl42 | Reviewer | 0
• bernhardoj | Contributor | 0

[melvin-bot]

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

[kbecciv]

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

[kbecciv]

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

The add personal bank account page doesn't show an error and just looks like do nothing when pressing the button.

What is the root cause of that problem?

When we press the save button, we actually request the BE to try saving it, and BE already returns the correct error message for us to show.

However, the add personal bank account form has a different key than the BE response. The BE response merges the errors to personalBankAccount, but the form has a key of personalBankAccountForm.

App/src/pages/AddPersonalBankAccountPage.tsx

Lines 82 to 83 in 22bd9eb

	<FormProvider
	formID={ONYXKEYS.FORMS.PERSONAL_BANK_ACCOUNT}

App/src/ONYXKEYS.ts

Line 408 in 22bd9eb

PERSONAL_BANK_ACCOUNT: 'personalBankAccountForm',

This happens after this TS migration PR which unnecessarily changes the key.

If we look at the bank account step pages, we can see that they don't add form suffix to the key.

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

Lines 63 to 64 in 22bd9eb

	<FormProvider
	formID={ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM}

App/src/ONYXKEYS.ts

Line 406 in 22bd9eb

REIMBURSEMENT_ACCOUNT_FORM: 'reimbursementAccount',

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

Remove the form from ONYXKEYS.FORMS.PERSONAL_BANK_ACCOUNT and PERSONAL_BANK_ACCOUNT_DRAFT

But after fixing this, we will see the error message shown is still incorrect and this is a separate issue that is being handled here

Result after fixing all:

Screen.Recording.2024-03-19.at.13.10.58.mov

[MitchExpensify]

Assigning to VSB as this is a personal account issue more in line with off workspace use

[melvin-bot]

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

[melvin-bot]

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

[cubuspl42]

@bernhardoj

This happens after this TS migration PR which unnecessarily changes the key.

Ouch, I reviewed that one.

Honestly, I don't understand how this is a regression. The old JS code didn't include the 'personalBankAccount' string. The whole diff doesn't include such a string. Could you explain it in more depth?

[bernhardoj]

Sure, in the old code, we use ONYXKEYS.PERSONAL_BANK_ACCOUNT as the form ID.

App/src/ONYXKEYS.ts

Line 206 in d8361ee

PERSONAL_BANK_ACCOUNT: 'personalBankAccount',

But the PR replaces it with ONYXKEYS.FORMS.PERSONAL_BANK_ACCOUNT which has the Form suffix.

[cubuspl42]

I'm not defending the code that introduces the regression, but it's worth noting that all keys in the ONYXKEYS.FORMS subobject have the same fooForm / fooFormDraft naming.

Could you comment on that? Is this a bigger problem (affecting the whole ONYXKEYS.FORMS), or do we have a single case that should opt-out from the naming convention because of the existing, older backend code?

[cubuspl42]

We should definitely figure this out. Right now, both the ONYXKEYS.PERSONAL_BANK_ACCOUNT and ONYXKEYS.FORMS.PERSONAL_BANK_ACCOUNT keys exist.

[bernhardoj]

Not all follow the form/formDraft naming, for example, these two:

App/src/ONYXKEYS.ts

Lines 358 to 359 in d8361ee

	CLOSE_ACCOUNT_FORM: 'closeAccount',
	CLOSE_ACCOUNT_FORM_DRAFT: 'closeAccountDraft',

App/src/ONYXKEYS.ts

Lines 352 to 353 in d8361ee

	WORKSPACE_TAX_CUSTOM_NAME: 'workspaceTaxCustomName',
	WORKSPACE_TAX_CUSTOM_NAME_DRAFT: 'workspaceTaxCustomNameDraft',

App/src/ONYXKEYS.ts

Lines 418 to 419 in d8361ee

	REIMBURSEMENT_ACCOUNT_FORM: 'reimbursementAccount',
	REIMBURSEMENT_ACCOUNT_FORM_DRAFT: 'reimbursementAccountDraft',

(I think the tax name could use form naming though)

My guess is, that we can't use the form naming if the form relies on BE validation before able to continue, for example, close account page and our case.

[cubuspl42]

You're right. Thanks for the extra clarification.

I approve the proposal by @bernhardoj

[melvin-bot]

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

[MitchExpensify]

I'm not sure why automation is not assigning an internal engineer to review the assignment of @bernhardoj

[bernhardoj]

I think @cubuspl42 forget to put the "magic" word

[cubuspl42]

Sorry!

I approve the proposal by @bernhardoj

Wait for it... 🥁

C+ reviewed 🎀 👀 🎀

[MitchExpensify]

Friendly bump @Gonals :)

[Gonals]

Looks good!

[melvin-bot]

📣 @cubuspl42 🎉 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]

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

[bernhardoj]

PR is ready

cc: @cubuspl42

[melvin-bot]

This issue has not been updated in over 15 days. @cubuspl42, @Gonals, @MitchExpensify, @bernhardoj eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[MitchExpensify]

Reminder set to pay on 4/29

Payment summary:

$500 Contributor @bernhardoj
$500 C+ @cubuspl42

[melvin-bot]

@cubuspl42, @Gonals, @MitchExpensify, @bernhardoj Whoops! This issue is 2 days overdue. Let's get this updated quick!

[MitchExpensify]

Paid and contract ended, apologies for the delay