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

VBA - Missing text "Step 5 of 5" under Test transactions #7322

Closed
kbecciv opened this issue Jan 19, 2022 · 7 comments
Closed

VBA - Missing text "Step 5 of 5" under Test transactions #7322

kbecciv opened this issue Jan 19, 2022 · 7 comments
Assignees
Labels
Daily KSv2 Engineering Improvement Item broken or needs improvement. Reviewing Has a PR in review

Comments

@kbecciv
Copy link

kbecciv commented Jan 19, 2022

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Launch the app
  2. Log in with expensifail account
  3. Enable the Staging Server under Setting
  4. Tap on any workspace
  5. Tap Connect with Bank Account
  6. Log in with Wells Fargo using username: user_good / Password - pass_good
  7. Fill out all necessary information for the VBA flow and hit Onfido Flow

Expected Result:

Text "Step 5 of 5" should present under Test transactions

Actual Result:

Missing text "Step 5 of 5" under Test transactions page

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.1.31.0

Reproducible in staging?: Yes

Reproducible in production?: n/a ( unable to verify VBA flow in Prod)

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:

Issue reported by: Applause

Slack conversation:

View all open jobs on GitHub

@MelvinBot
Copy link

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

@bondydaa bondydaa added Improvement Item broken or needs improvement. External Added to denote the issue can be worked on by a contributor labels Jan 19, 2022
@MelvinBot
Copy link

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

@bondydaa
Copy link
Contributor

Hmm looks like we have a prop here that is saying if we should show the step counter or not https://github.com/PrashantMangukiya/ExpensifyApp/blob/b9ef450aa586285073af061a87418867d124c9d6/src/pages/ReimbursementAccount/ValidationStep.js#L171

I see these somewhat recent PRs but it doesn't look like they are the cause of this #6477 #5255

Since it's also not showing the test amount inputs, I'm guessing this bank account is already in the validated state and that's most likely why the step counter isn't showing.

We could fix by always showing the step counter no matter what even if the bank account is validated. Or we could do nothing. I don't have a strong opinion.

I'm going to add the external label so @Christinadobrzyn please weigh in since you're more familiar with the app - if you think it's worth changing to always show the step counter then post it, if not then lets close it.

@ctkochan22 ctkochan22 self-assigned this Jan 20, 2022
@ctkochan22 ctkochan22 removed the External Added to denote the issue can be worked on by a contributor label Jan 20, 2022
@ctkochan22
Copy link
Contributor

The validation step here, has two states.

  • Verifying (Ops is verifying the VBA)
  • Pending (User needs to add test transactions or validation amounts as its said in old-dot)

We used to just have it set to "Connect Bank Account", then it changed to "Test Transactions" indiscriminately of either VBA state for this issue and in this pr
cc @kevinksullivan

Going to update this so that if we the VBA is VERIFYING, it'll say "Connect Bank Account" (do we want to change this to "validate" or something?)
image

And if the VBA is in the PENDING state, it'll say as the issue requested, "Test Transactions"
image

@kevinksullivan
Copy link
Contributor

Sorry just catching up on this @ctkochan22 . So going forward there is still a generic Connect bank account rather than Test transactions when we're following up in chat (i.e. in Verifying state), and then Test transactions when in Pending? That sounds good to me! We could add the additional Validate bank account for the verifying state as well.

@ctkochan22
Copy link
Contributor

So going forward there is still a generic Connect bank account rather than Test transactions when we're following up in chat (i.e. in Verifying state), and then Test transactions when in Pending

Yes exactly

@ctkochan22
Copy link
Contributor

Yeah, happy to change it to Validate bank account. It sounds better to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering Improvement Item broken or needs improvement. Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

7 participants