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

Payment - User can't continue after reaching "Save & Continue" #9085

Closed
kavimuru opened this issue May 19, 2022 · 29 comments
Closed

Payment - User can't continue after reaching "Save & Continue" #9085

kavimuru opened this issue May 19, 2022 · 29 comments
Assignees

Comments

@kavimuru
Copy link

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:

Pre-Condition: Have a workspace created

  1. Open the Expensify iOS app
  2. Log in to a applause.expensifail.com account
  3. Tap on "Profile" icon > "Payments" > "Add payment method" > "Bank Account"
  4. Proceed with the flow until the "Account type & Password" page
  5. Tap "Save & Continue"

Expected Result:

The user expects to be able to add a deposit bank account through this flow

Actual Result:

The user wont be able to proceed beyond this "Save & continue' page, the button will "Load" and return to its initial state

Workaround:

Unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.1.63-2

Reproducible in staging?: Y

Reproducible in production?: No (Not able to check in production)

Email or phone of affected tester (no customers):

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

Notes/Photos/Videos:

Bug5577168_Cant_continue_beyond_Save___Continue.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause

Slack conversation:

View all open jobs on GitHub

@kavimuru kavimuru added the DeployBlockerCash This issue or pull request should block deployment label May 19, 2022
@OSBotify
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@melvin-bot
Copy link

melvin-bot bot commented May 19, 2022

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

@techievivek
Copy link
Contributor

I looked into the issue and can see that it throws the error and the reason being the account that you are trying to add is already previously added.

bankaccount? [hmmm] [mobile] API threw ExpError Expensify\Libs\Error\ExpError - 15fc249e-1f89-49be-aa78-1d87132bf9c5 ~~ exceptionJsonCode: '666' exceptionUserMessage: 'Bank account can't be created, it looks like a bank account with the same information already exists in your account.' exceptionMessage: 'Bank account can't be created, it looks like a bank account with the same information already exists in your account.' exceptionFile: '/vagrant/Web-Secure/lib/BankAccountAPI.php' exceptionLine: '883' exceptionCode: '666'

I also tested this with a new account where no bank accounts are added and it worked without any errors.
So a solution would be to show the error message to the user in the app.

@techievivek
Copy link
Contributor

techievivek commented May 19, 2022

@kavimuru Can you confirm if you had already added this bank account before? or did you try adding this as a bank account in one of the workspaces?

@techievivek
Copy link
Contributor

@AndrewGable Can you please cross-check this. Thanks

@parasharrajat
Copy link
Member

parasharrajat commented May 19, 2022

I am already fixing this here #9023 as part of #7185

But I think #9085 (comment) is not the best solution due to the reason.

@techievivek
Copy link
Contributor

So I think we can close this one then and yeah #9085 (comment) would be the best solution here.

@parasharrajat
Copy link
Member

parasharrajat commented May 19, 2022

But I think we should fix where we check for the duplicate accounts so that it does not show duplicate accounts for selection instead of showing the error. This logic should successfully filter the existing accounts. But currently, it is not. It may be due to testing accounts.

plaidBankAccounts = _.filter(response.accounts, account => !account.alreadyExists);

Sorry, I meant to say that #9085 (comment) is not the best solution. Mistyping...

@techievivek
Copy link
Contributor

techievivek commented May 19, 2022

Did you request a copy for your fix? I think we can just go with whatever they suggest.

And about the other concern while trying to reproduce this locally I saw an inconsistency when I added the same saving account by going through Profile -> Payment -> Add payment method it did filter out the already added account when I reattempted to add a new bank account. But when I added the same bank account by going into the workspace and left it incomplete and then came back to the previous flow Profile -> Payment -> Add payment method it got stuck at Save & Continue.

@parasharrajat
Copy link
Member

Thanks. I am also concerned about the filtering of existing accounts. It is clearly an issue with that filtration. Code is designed to hide existing accounts from the dropdown so we should improve this logic to successfully filter all existing accounts. But if there is a possibility of an edge case where we are getting this error, then we can go with showing the error on the UI.

At last, we can keep this open for tracking and I can add this to my PR in the fixed issues list.

@techievivek
Copy link
Contributor

techievivek commented May 19, 2022

It indeed is related to filtration logic, I think the current logic only hides the bank accounts which has been successfully added but for bank accounts that are in process of being added(You can mimic this by trying to add a bank account under one of the workspaces but leave it incomplete) are shown in the dropdown. But when you click on Save & Continue it reports the bank account is already added. Strange!

This is what the progress of adding a bank account in my workspace looks like.
Screenshot 2022-05-19 at 3 25 23 PM

Anyway making this a Daily for now.

@techievivek techievivek added Daily KSv2 and removed Hourly KSv2 labels May 19, 2022
@AndrewGable
Copy link
Contributor

Since this issue sounds like it's on production as part of #7185 - Going to remove deploy blocker label.

@techievivek
Copy link
Contributor

Not overdue.

@melvin-bot melvin-bot bot removed the Overdue label May 23, 2022
@melvin-bot
Copy link

melvin-bot bot commented May 26, 2022

@techievivek Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label May 26, 2022
@techievivek
Copy link
Contributor

Not overdue

@melvin-bot melvin-bot bot removed the Overdue label May 26, 2022
@techievivek
Copy link
Contributor

@parasharrajat Hi 👋 Is this getting fixed in your PR?

@parasharrajat
Copy link
Member

@techievivek No yet. I am waiting on update from @chiragsalian #7185 (comment).

@melvin-bot melvin-bot bot added the Overdue label May 31, 2022
@techievivek
Copy link
Contributor

Not overdue.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jun 1, 2022
@techievivek
Copy link
Contributor

Not overdue. Work is in progress.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jun 4, 2022
@techievivek
Copy link
Contributor

Not overdue. Updating this to Weekly.

@melvin-bot melvin-bot bot removed the Overdue label Jun 7, 2022
@techievivek techievivek added Weekly KSv2 and removed Daily KSv2 labels Jun 7, 2022
@melvin-bot melvin-bot bot added the Overdue label Jun 15, 2022
@techievivek
Copy link
Contributor

Not overdue.

@melvin-bot melvin-bot bot removed the Overdue label Jun 16, 2022
@melvin-bot melvin-bot bot added the Overdue label Jun 24, 2022
@techievivek
Copy link
Contributor

Not overdue.

@melvin-bot melvin-bot bot removed the Overdue label Jun 27, 2022
@melvin-bot melvin-bot bot added the Overdue label Jul 5, 2022
@techievivek
Copy link
Contributor

Not overdue, @parasharrajat Can you give an update for this one 🙌 .

@melvin-bot melvin-bot bot removed the Overdue label Jul 6, 2022
@parasharrajat
Copy link
Member

The other PR is is about to be deployed on production and then I will complete my changes.

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@melvin-bot melvin-bot bot added the Overdue label Jul 14, 2022
@techievivek
Copy link
Contributor

Not overdue.

@melvin-bot melvin-bot bot removed the Overdue label Jul 15, 2022
@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

@AndrewGable
Copy link
Contributor

If it's not reproducible 2x let's close

@parasharrajat
Copy link
Member

Yes, we fixed it from the backend #7185 (comment)

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

No branches or pull requests

6 participants