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

Remove password requirement for setting up withdrawal account #5545

Merged
merged 3 commits into from
Sep 30, 2021

Conversation

Jag96
Copy link
Contributor

@Jag96 Jag96 commented Sep 27, 2021

Details

Per this thread we want to remove the password requirement for adding a withdrawal account. This PR updates the UI to not include the password field for the Company Step, and removes the password param for the BankAccount_SetupWithdrawal API call.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/179271

Tests

  1. If https://github.com/Expensify/Auth/pull/5973 isn't merged, check out that branch to test locally
  2. Create a new account and create a workspace
  3. Click Get Started to go through the VBA flow, follow https://stackoverflow.com/c/expensify/questions/342/525#525
  4. Confirm that when you get to the Company Information step, the password field doesn't appear

4. Confirm you can submit the form, and that there is no error regarding the password

QA Steps

Tag @Jag96 and I can run through the QA steps for this on staging

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Screenshot in the tests section

@Jag96 Jag96 requested a review from a team September 27, 2021 22:46
@Jag96 Jag96 self-assigned this Sep 27, 2021
@MelvinBot MelvinBot requested review from jasperhuangg and removed request for a team September 27, 2021 22:46
@Jag96 Jag96 requested a review from a team September 28, 2021 20:54
@MelvinBot MelvinBot requested review from pecanoro and removed request for a team September 28, 2021 20:54
@Jag96
Copy link
Contributor Author

Jag96 commented Sep 28, 2021

@jasperhuangg is at 20% this week so requesting another reviewer. The Auth PR has been merged so the code can be reviewed and tested on dev, waiting to remove the hold until after the auth deploy.

@Jag96 Jag96 requested a review from a team as a code owner September 29, 2021 16:01
@Jag96 Jag96 removed the request for review from a team September 29, 2021 16:02
@Jag96 Jag96 removed the request for review from ctkochan22 September 29, 2021 16:03
@Jag96
Copy link
Contributor Author

Jag96 commented Sep 30, 2021

Auth PR has been deployed, taking off hold

@Jag96 Jag96 changed the title [HOLD Auth 5973] Remove password requirement for setting up withdrawal account Remove password requirement for setting up withdrawal account Sep 30, 2021
@Jag96 Jag96 merged commit f189aad into main Sep 30, 2021
@Jag96 Jag96 deleted the joe-remove-vba-password branch September 30, 2021 16:50
@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @Jag96 in version: 1.1.3-4 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@isagoico
Copy link

@Jag96 Helloo! Following orders from QA steps and tagging you for QA 🙇

@chiragsalian
Copy link
Contributor

@Jag96, Is this possibly a regression? On dev, it seems like we're unable to go beyond step 2. The UI doesn't show any error for me but the network request says "401 error",
image

@luacmartins tried the same on his end and got,
image

@luacmartins
Copy link
Contributor

Adding to this, I do see the UI error Incorrect Expensify password entered.

@chiragsalian
Copy link
Contributor

Btw i just tested this on staging and it doesn't have password field,
image

and it clears through just fine on staging,
image

I'm not sure why it failed on dev for me just yet.

@chiragsalian
Copy link
Contributor

Okay, nvm me. I did an updateRepos all and makeAll and once i retested it worked fine for me on dev.

@luacmartins
Copy link
Contributor

Same here, just had to update all the repos! Sorry for the false alarm!

@OSBotify
Copy link
Contributor

OSBotify commented Oct 4, 2021

🚀 Deployed to production by @chiragsalian in version: 1.1.4-0 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants