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 App PR#12181] [$1000] [BUG] Pay with Expensify is NOT selecting ACH automatically #12004

Closed
mvtglobally opened this issue Oct 19, 2022 · 30 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering

Comments

@mvtglobally
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:

  1. Login
  2. Set your default payment method in Settings > Payment
  3. Get IOU from another user
  4. Click Pay in chat
  5. Note the

Expected Result:

If user has Bank account setup, we should not be asking again for the new payment

Actual Result:

Pay with Expensify is NOT selecting ACH automatically

Workaround:

unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.2.18-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
image - 2022-10-19T155743 782

image - 2022-10-19T155919 500

Recording.549.mp4

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

View all open jobs on GitHub

@mvtglobally mvtglobally added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Oct 19, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 19, 2022

Triggered auto assignment to @ntrepanier (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 19, 2022
@michaelhaxhiu
Copy link
Contributor

@ntrepanier can you please prioritize this one, as it was initially missed by mistake (in our bug reporting flow)? Refer to the slack thread above for context.

@puneetlath puneetlath added the Bug Something is broken. Auto assigns a BugZero manager. label Oct 19, 2022
@mvtglobally mvtglobally changed the title Pay with Expensify is NOT selecting ACH automatically [BUG] Pay with Expensify is NOT selecting ACH automatically Oct 19, 2022
@michaelhaxhiu
Copy link
Contributor

Note for the C+ that gets assigned, this GH should include a Root Cause Analysis (RCA) investigation since it's clearly a regression.

@melvin-bot melvin-bot bot added the Overdue label Oct 21, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 25, 2022

@ntrepanier Huh... This is 4 days overdue. Who can take care of this?

@ntrepanier
Copy link

Sorry. It assigned while I was OOO. On it.

@melvin-bot
Copy link

melvin-bot bot commented Oct 25, 2022

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

@ntrepanier
Copy link

@johnmlee101 Can you help me determine if this would be external or internal?

@johnmlee101
Copy link
Contributor

{triggerKYCFlow => (

seems to be self-referencial

{triggerKYCFlow => (

I'm a bit confused, but I'm pretty sure this is external so marking it

@johnmlee101 johnmlee101 added the External Added to denote the issue can be worked on by a contributor label Oct 26, 2022
@johnmlee101 johnmlee101 removed their assignment Oct 26, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 26, 2022

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

@melvin-bot
Copy link

melvin-bot bot commented Oct 26, 2022

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

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

melvin-bot bot commented Oct 26, 2022

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

@melvin-bot melvin-bot bot changed the title [BUG] Pay with Expensify is NOT selecting ACH automatically [$250] [BUG] Pay with Expensify is NOT selecting ACH automatically Oct 26, 2022
@smrutiparida
Copy link
Contributor

@mvtglobally The bank account added is still not a default account as I can see from the screenshot attached. Underlined below.

Screen Shot 2022-10-27 at 10 33 00

As an external developer though I have not reproduced the steps but from the code it seems evident that though bank account is found, the isDefaultCredit() is returning false as the account is not a default account to receive payment.

Screen Shot 2022-10-27 at 10 36 35

May I hence suggest to make the account default and see one more. I hope that is what is the expected result.

@puneetlath
Copy link
Contributor

puneetlath commented Oct 28, 2022

@smrutiparida the issue exists even when a default payment method is set. Here's an example from my account:
Screen Shot 2022-10-28 at 1 12 45 PM

Screen Shot 2022-10-28 at 1 12 54 PM

I have a default payment method set, but it's still asking me how I want to pay when I click "pay with Expensify". If I choose bank account, it takes me to the add a bank account page. So in some way, this page is not registering that I already have a bank account set.

@puneetlath
Copy link
Contributor

Doubled to $1000.

Job post is here: https://www.upwork.com/jobs/~01744f6f4e5bb3bfe4

@puneetlath puneetlath changed the title [$500] [BUG] Pay with Expensify is NOT selecting ACH automatically [$1000] [BUG] Pay with Expensify is NOT selecting ACH automatically Nov 3, 2022
@melvin-bot melvin-bot bot added the Overdue label Nov 7, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 7, 2022

@puneetlath, @rushatgabhane Whoops! This issue is 2 days overdue. Let's get this updated quick!

@thomas-coldwell
Copy link
Contributor

Trying to reproduce this issue, but unable to add a bank account as the default payment method - getting the following 404 output in console when I attempt it:
Finished API request - {"command":"MakeDefaultPaymentMethod","jsonCode":404,"requestID":"76665b147b9f7505-SJC"}

@MariaHCD
Copy link
Contributor

MariaHCD commented Nov 8, 2022

Just a heads up here, I actually have the fix for this included in this PR: #12181

bankAccount.isDefaultCredit() is currently returning false because the this.json.defaultCredit property has now moved to this.json.accountData.defaultCredit as part of #10447

@MariaHCD MariaHCD changed the title [$1000] [BUG] Pay with Expensify is NOT selecting ACH automatically [HOLD] [$1000] [BUG] Pay with Expensify is NOT selecting ACH automatically Nov 8, 2022
@MariaHCD MariaHCD self-assigned this Nov 8, 2022
@mountiny mountiny removed 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 labels Nov 8, 2022
@melvin-bot melvin-bot bot removed the Overdue label Nov 8, 2022
@mountiny
Copy link
Contributor

mountiny commented Nov 8, 2022

Updated this issue to reflect it is internal. Sorry for the confusion. It is easy to overlook that some issue overlap and miss this. Hopefully this issue will be resolved. with the aforementioned PR.

@puneetlath
Copy link
Contributor

Ok cool, good to know. Thanks @mountiny and @MariaHCD!

@melvin-bot
Copy link

melvin-bot bot commented Nov 9, 2022

BugZero Checklist: The PR fixing this issue has been merged! The following checklist will need to be completed before the issue can be closed:

  • A regression test has been added or updated so that the same bug will not reach production again. Link to the updated test here:
  • The PR that introduced the bug has been identified. Link to the PR:
  • The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • A discussion in #contributor-plus has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • Payment has been made to the issue reporter (if applicable)
  • Payment has been made to the contributor that fixed the issue (if applicable)
  • Payment has been made to the contributor+ that helped on the issue (if applicable)

@puneetlath
Copy link
Contributor

@MariaHCD would you be able to help me out with figuring out which PR caused the bug?

@MariaHCD
Copy link
Contributor

@puneetlath It was due to this backend change: https://github.com/Expensify/Web-Expensify/pull/34592

The format of the bank account data in Onyx changed but the corresponding model in the App was not. cc: @Justicea83

There was also a RCA here but we didn't catch this particular bug because features related to the Expensify wallet are not being QA'd right now. That's going to change with the completion of https://github.com/Expensify/Expensify/issues/232687

@melvin-bot melvin-bot bot added the Overdue label Nov 14, 2022
@puneetlath
Copy link
Contributor

Ok awesome, thanks for the info @MariaHCD.

And just checking, this is still on hold, is that right? If so, mind linking what we are holding for? Thanks!

@melvin-bot melvin-bot bot removed the Overdue label Nov 14, 2022
@mountiny mountiny changed the title [HOLD] [$1000] [BUG] Pay with Expensify is NOT selecting ACH automatically [HOLD App PR#12181] [$1000] [BUG] Pay with Expensify is NOT selecting ACH automatically Nov 14, 2022
@mountiny
Copy link
Contributor

I think Maria is ooo til Friday so stepping in, from the previous comments this is on hold for #12181

It is in staging now so we should be bale to retest there, but we can wait for another deploy I assume

@mountiny
Copy link
Contributor

I have read and reviewed this Project Manager Application!

@puneetlath
Copy link
Contributor

Just tested on staging and looks like it's working so I think we're good to go here. Thanks @MariaHCD!

@mountiny
Copy link
Contributor

mountiny commented Nov 14, 2022

I have read and reviewed this Manager Application!

Oops, luckily i dont have that chore. Wondering if it created some bugbot issue 🤔

nah, nothing, it will just add save me but it will not matter for the autoassigner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering
Projects
None yet
Development

No branches or pull requests

10 participants