Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD for payment 2022-11-04] [$250] BUG: Enter password field shows on add bank account page while trying to add bank accounts consecutively reported by @adeel0202 #11816

Closed
kavimuru opened this issue Oct 13, 2022 · 30 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@kavimuru
Copy link

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

  1. Go to Settings > Payments
  2. Click Add payment method > Bank account
  3. Complete the process on Plaid to add a new bank account
  4. On Add bank account page, choose an account
  5. Enter your password
  6. Click save & continue
  7. Click Continue
  8. Again click Add payment method > Bank account (Notice Enter Expensify password field shows on add bank account page)
  9. Again complete the process on Plaid to add another bank account (Notice Enter Expensify password field shows on add bank account page without choosing an account)

Expected Result:

Enter Expensify password field doesn't show until an account is chosen

Actual Result:

Enter Expensify password field shows even when the account is not chosen

Workaround:

unknown

Platform:

Where is this issue occurring?

  • Web

Version Number: v1.2.14-0
Reproducible in staging?: y
Reproducible in production?: Could not test in Production
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Recording.687.mp4

Expensify/Expensify Issue URL:
Issue reported by: @adeel0202
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1665648921463259

View all open jobs on GitHub

Screen.Recording.2022-10-13.at.12.54.51.PM.mov
@kavimuru kavimuru added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Oct 13, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 13, 2022

Triggered auto assignment to @sakluger (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@melvin-bot melvin-bot bot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Oct 13, 2022
@sakluger
Copy link
Contributor

I was able to reproduce this issue, although it's easy to miss, since the password field only shows in the RHN briefly before the Plaid modal pops up. Passing along to engineering.

@sakluger sakluger removed their assignment Oct 14, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 14, 2022

Triggered auto assignment to @mountiny (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@mountiny
Copy link
Contributor

I see so the problem is that the password page flashes there for a second when doing it second time. I think this issue can be External

@mountiny mountiny added the External Added to denote the issue can be worked on by a contributor label Oct 15, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 15, 2022

Triggered auto assignment to @alexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Oct 15, 2022

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 15, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 15, 2022

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

@melvin-bot melvin-bot bot changed the title BUG: Enter password field shows on add bank account page while trying to add bank accounts consecutively reported by @adeel0202 [$250] BUG: Enter password field shows on add bank account page while trying to add bank accounts consecutively reported by @adeel0202 Oct 15, 2022
@sobitneupane
Copy link
Contributor

Proposal
Reason:

this.state = {
selectedPlaidAccountID: this.props.personalBankAccount.plaidAccountID,
};
}
componentDidMount() {
BankAccounts.clearPersonalBankAccount();
}

Here, componentDidMount clears personalBankAccount currently being setup. But before that, state selectedPlaidAccountID gets its value from this.props.personalBankAccount.

Solution:

this.state = {
-         selectedPlaidAccountID: this.props.personalBankAccount.plaidAccountID, 
+         selectedPlaidAccountID: '', 
}

This change was recently made in this PR by @nkuoch. Might be an intended change.

cc: @nkuoch

@parasharrajat
Copy link
Member

parasharrajat commented Oct 16, 2022

Hmmmmm, my intuition says that we can't just do this change as above. I will take a look at that PR and try to understand the matter.

@nkuoch
Copy link
Contributor

nkuoch commented Oct 17, 2022

I accept this solution. I'm the one who initially added it in state, but thinking more about it, I agree we should reset it back to empty.

@mountiny
Copy link
Contributor

Great! Thanks for looking into this @nkuoch!

@parasharrajat Can you confirm this proposal before I would assign @sobitneupane? Thank you! 🙇

@parasharrajat
Copy link
Member

Let me review it first.

@parasharrajat
Copy link
Member

Ok, it seems that we can remove the state initialization. We are clearing the data on componentDidMount thus this does not make sense together. Also, every time we open the connect account page/add a bank account, the process is started again. So the user will select the bank account each time.

@sobitneupane's proposal looks good to me.

cc: @mountiny

🎀 👀 🎀 C+ reviewed

@mountiny
Copy link
Contributor

Thank you @parasharrajat! @sobitneupane Feel free to start the PR!

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Help Wanted Apply this label when an issue is open to proposals by contributors Daily KSv2 labels Oct 17, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 21, 2022

Triggered auto assignment to @alexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Oct 21, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 21, 2022

Current assignees @parasharrajat and @sobitneupane are eligible for the External assigner, not assigning anyone new.

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 21, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 21, 2022

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

@thesahindia
Copy link
Member

EEP! None of us noticed that we have a dupe issue #11874. We also have a PR for it.

cc: @MonilBhavsar

@mountiny
Copy link
Contributor

Thanks for the ping, gonna look into that

@parasharrajat
Copy link
Member

parasharrajat commented Oct 23, 2022

Waiting on the signal from Vit for PR review.

@mountiny
Copy link
Contributor

Signal sent @parasharrajat

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Oct 28, 2022
@melvin-bot melvin-bot bot changed the title [$250] BUG: Enter password field shows on add bank account page while trying to add bank accounts consecutively reported by @adeel0202 [HOLD for payment 2022-11-04] [$250] BUG: Enter password field shows on add bank account page while trying to add bank accounts consecutively reported by @adeel0202 Oct 28, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 28, 2022

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.20-3 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 2022-11-04. 🎊

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Nov 3, 2022
@alexpensify
Copy link
Contributor

Based on the hold notice, I'll check tomorrow!

@mallenexpensify
Copy link
Contributor

@sobitneupane @parasharrajat @adeel0202 can you apply for the job here then comment on this issue? https://www.upwork.com/jobs/~01f174bbce556e4071

@sobitneupane
Copy link
Contributor

@mallenexpensify Applied.

@melvin-bot melvin-bot bot added the Overdue label Nov 7, 2022
@mountiny
Copy link
Contributor

mountiny commented Nov 7, 2022

@mallenexpensify will pay this out today.

@melvin-bot melvin-bot bot removed the Overdue label Nov 7, 2022
@mallenexpensify
Copy link
Contributor

mallenexpensify commented Nov 7, 2022

All hired, please accept and I'll pay out
https://www.upwork.com/jobs/~01f174bbce556e4071

@mallenexpensify
Copy link
Contributor

Everyone's paid!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests