[HOLD for payment 2024-07-24] [$250] Connect bank - "-1" is pre-filled for the tax ID number when connecting a new bank account

[lanitochka17]

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: 9.0.5-2
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. Navigate to staging.new.expensify.com
2. Create a workspace and enable workflows
3. Go through connecting a bank account flow up until the fourth step where it requires you for the tax ID number

Expected Result:

The input field should be empty

Actual Result:

The input field has a pre-filled value of "-1"

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016ff97d1a1bc0b4b8
• Upwork Job ID: 1811441687713769949
• Last Price Increase: 2024-07-11
• Automatic offers:
• suneox | Reviewer | 103085245
• Krishna2323 | Contributor | 103085246

Issue Owner

Current Issue Owner: @RachCHopkins

[melvin-bot]

Triggered auto assignment to @RachCHopkins (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]

@RachCHopkins 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

[Krishna2323]

Proposal

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

Connect bank - "-1" is pre-filled for the tax ID number when connecting a new bank account

What is the root cause of that problem?

The default value of defaultCompanyTaxId is -1.

App/src/pages/ReimbursementAccount/BusinessInfo/substeps/TaxIdBusiness.tsx

Lines 31 to 33 in 9c32293

	const defaultCompanyTaxId = reimbursementAccount?.achData?.companyTaxID ?? '-1';
	const bankAccountID = reimbursementAccount?.achData?.bankAccountID ?? -1;
	const shouldDisableCompanyTaxID = !!(bankAccountID && defaultCompanyTaxId && reimbursementAccount?.achData?.state !== 'SETUP');

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

I think this PR mistakenly changed the default value of defaultCompanyTaxId and bankAccountID. We should revert the changes in TaxIdBusiness and also look for other files which might have been affected. I think we should only default to -1 when accessing from a value from onyx.

What alternative solutions did you explore? (Optional)

1. If we need -1 as default for determining shouldDisableCompanyTaxID value, we can directly default it when accessing it in shouldDisableCompanyTaxID or

2. We can use reimbursementAccount?.achData?.companyTaxID for defaultValue prop value.

3. Remove -1 and add empty string if needed.

4. Remove the default value (??) from bankAccountID & defaultCompanyTaxId.

[RachCHopkins]

Clearly reproducible!

@blimpich can I get you to take a look at this? Looks like you were on that PR where this crept in? Should this be an internal or external fix?

[blimpich]

@kubabutkiewicz can you take a look at this and see if there was a mistake that we made in #42634?

[kubabutkiewicz]

@blimpich yeah this comes from #42634 PR

[blimpich]

Sweet, lets make this open to contributors then and get it fixed.

[melvin-bot]

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

[melvin-bot]

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

[suneox]

@Krishna2323 proposal using default empty string instead of '-1' value is make sense for defaultCompanyTaxId and 0 for bankAccountID. I think we can go with this solution.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Current assignee @blimpich is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot]

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

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

[Krishna2323]

Opening a PR today.

[Krishna2323]

@suneox, PR ready for review ^

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

• fix: Connect bank - -1 is pre-filled for the tax ID number when connecting a new bank account. #45402

If no regressions arise, payment will be issued on 2024-07-24. 🎊

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

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

[suneox]

• [@suneox] The PR that introduced the bug has been identified. Link to the PR: [TS Migration] Define a concise way to access or default to an inexistent record #42634
• [@suneox] 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: https://github.com/Expensify/App/pull/42634/files#r1685307458
• [@suneox] 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: N/A
• [@suneox] Determine if we should create a regression test for this bug. No, due to the changes from the offending PR being large and missing test and we only revert invalid change

[RachCHopkins]

All contracts paid via Upwork!

[RachCHopkins]

Payment Summary:

Contributor: @Krishna2323 paid $250 via Upwork

Contributor+: @suneox paid $250 via UpWork

Upwork job here

Contributor has been paid, the contract has been completed, and the Upwork post has been closed.