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

Reset instead of deleting achData #6257

Merged
merged 1 commit into from
Nov 6, 2021
Merged

Reset instead of deleting achData #6257

merged 1 commit into from
Nov 6, 2021

Conversation

NikkiWines
Copy link
Contributor

cc: @marcaaron related to #5828

Details

Sets basic ACHData, including currentStep, in case the user deletes their VBA and then immediately goes to add another.
Previously we'd just wipe out the reimbursementAccount onyx key so we'd get an error when trying to call API.BankAccount_SetupWithdrawal because currentStep was missing.

Fixed Issues

$ #6246

Tests

Same as QA steps

QA Steps

  1. Go to any Workspace and select Connect bank account
  2. Add a bank account following VBA flow
  3. After the bank account is validated - tap disconnect bank account
  4. Go to any Workspace and select Connect bank account
  5. Continue VBA flow
  6. After entering chase bank credentials select saving accounts
  7. Confirm you're successfully routed to the company step after selecting the savings account

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

web.mp4

Mobile Web

Desktop

iOS

ios.mov

Android

android.mp4

@NikkiWines NikkiWines requested a review from a team as a code owner November 5, 2021 22:12
@NikkiWines NikkiWines self-assigned this Nov 5, 2021
@MelvinBot MelvinBot requested review from aldo-expensify and removed request for a team November 5, 2021 22:13
Copy link
Contributor

@aldo-expensify aldo-expensify left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on web, works. Looks good to me.

Leaving un-merged in case @marcaaron wants to have a look. @NikkiWines feel free to merge if you want.

isInSetup: true,
domainLimit: 0,
currentStep: CONST.BANK_ACCOUNT.STEP.BANK_ACCOUNT,
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this!!!

@marcaaron marcaaron merged commit b12eb5f into main Nov 6, 2021
@marcaaron marcaaron deleted the nikki-fix-removevba branch November 6, 2021 00:06
@OSBotify
Copy link
Contributor

OSBotify commented Nov 6, 2021

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions bot pushed a commit that referenced this pull request Nov 8, 2021
Reset instead of deleting achData

(cherry picked from commit b12eb5f)
@OSBotify
Copy link
Contributor

OSBotify commented Nov 8, 2021

🚀 Cherry-picked to staging by @francoisl in version: 1.1.14-4 🚀

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

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @marcaaron in version: 1.1.14-5 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @Jag96 in version: 1.1.14-4 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @AndrewGable in version: 1.1.15-15 🚀

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.

4 participants