You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[HOLD for payment 2024-07-24] [$250] Connect bank - "-1" is pre-filled for the tax ID number when connecting a new bank account #45073
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:
Navigate to staging.new.expensify.com
Create a workspace and enable workflows
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?
@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
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)
If we need -1 as default for determining shouldDisableCompanyTaxID value, we can directly default it when accessing it in shouldDisableCompanyTaxID or
We can use reimbursementAccount?.achData?.companyTaxID for defaultValue prop value.
Remove -1 and add empty string if needed.
Remove the default value (??) from bankAccountID & defaultCompanyTaxId.
changed the title [-]Connect bank - "-1" is pre-filled for the tax ID number when connecting a new bank account[/-][+][$250] Connect bank - "-1" is pre-filled for the tax ID number when connecting a new bank account[/+]on Jul 11, 2024
changed the title [-][$250] Connect bank - "-1" is pre-filled for the tax ID number when connecting a new bank account[/-][+][HOLD for payment 2024-07-24] [$250] Connect bank - "-1" is pre-filled for the tax ID number when connecting a new bank account[/+]on Jul 17, 2024
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:
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] 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
Activity
melvin-bot commentedon Jul 9, 2024
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 commentedon Jul 9, 2024
@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 commentedon Jul 9, 2024
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
defaultCompanyTaxIdis-1.App/src/pages/ReimbursementAccount/BusinessInfo/substeps/TaxIdBusiness.tsx
Lines 31 to 33 in 9c32293
What changes do you think we should make in order to solve the problem?
I think this PR mistakenly changed the default value of
defaultCompanyTaxIdandbankAccountID. We should revert the changes inTaxIdBusinessand also look for other files which might have been affected. I think we should only default to-1when accessing from a value from onyx.What alternative solutions did you explore? (Optional)
If we need
-1as default for determiningshouldDisableCompanyTaxIDvalue, we can directly default it when accessing it inshouldDisableCompanyTaxIDorWe can use
reimbursementAccount?.achData?.companyTaxIDfordefaultValueprop value.Remove
-1and add empty string if needed.Remove the default value (??) from
bankAccountID&defaultCompanyTaxId.RachCHopkins commentedon Jul 10, 2024
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 commentedon Jul 10, 2024
@kubabutkiewicz can you take a look at this and see if there was a mistake that we made in #42634?
kubabutkiewicz commentedon Jul 11, 2024
@blimpich yeah this comes from #42634 PR
blimpich commentedon Jul 11, 2024
Sweet, lets make this open to contributors then and get it fixed.
[-]Connect bank - "-1" is pre-filled for the tax ID number when connecting a new bank account[/-][+][$250] Connect bank - "-1" is pre-filled for the tax ID number when connecting a new bank account[/+]melvin-bot commentedon Jul 11, 2024
Job added to Upwork: https://www.upwork.com/jobs/~016ff97d1a1bc0b4b8
16 remaining items
[-][$250] Connect bank - "-1" is pre-filled for the tax ID number when connecting a new bank account[/-][+][HOLD for payment 2024-07-24] [$250] Connect bank - "-1" is pre-filled for the tax ID number when connecting a new bank account[/+]melvin-bot commentedon Jul 17, 2024
Reviewinglabel has been removed, please complete the "BugZero Checklist".melvin-bot commentedon Jul 17, 2024
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:
If no regressions arise, payment will be issued on 2024-07-24. 🎊
For reference, here are some details about the assignees on this issue:
melvin-bot commentedon Jul 17, 2024
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 commentedon Jul 20, 2024
RachCHopkins commentedon Jul 25, 2024
All contracts paid via Upwork!
RachCHopkins commentedon Aug 12, 2024
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.