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

Payments - Generic error message appears when deleting bank card #8325

Closed
kavimuru opened this issue Mar 25, 2022 · 32 comments
Closed

Payments - Generic error message appears when deleting bank card #8325

kavimuru opened this issue Mar 25, 2022 · 32 comments
Assignees
Labels
Daily KSv2 Engineering FirstPick Engineering only, please! Only add when there is an identified code solution. Improvement Item broken or needs improvement. Internal Requires API changes or must be handled by Expensify staff

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:

  1. Navigate to new staging.
  2. Open the Payment Page from Settings => Payments.
  3. Select Delete option for the bank card added

Expected Result:

User is able to delete any payment method

Actual Result:

Error message "oops...something went wrong and your request could not be completed. Please try again later". is displayed when try to delete CC.

Workaround:

Unknown

Platform:

Where is this issue occurring?

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

Version Number: v1.1.46-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:

Bug5506623_new_delete.mp4

Bug5506623_Screen_Shot_2022-03-25_at_4 22 57_PM

Expensify/Expensify Issue URL:
Issue reported by: Applause
Slack conversation:

View all open jobs on GitHub

@melvin-bot
Copy link

melvin-bot bot commented Mar 25, 2022

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

@bondydaa bondydaa added the Improvement Item broken or needs improvement. label Mar 25, 2022
@bondydaa bondydaa removed their assignment Mar 25, 2022
@bondydaa
Copy link
Contributor

hmm this is probably internal since it seems something changed with the API and is breaking if it's happening on production too.

@bondydaa bondydaa added the Internal Requires API changes or must be handled by Expensify staff label Mar 25, 2022
@MariaHCD MariaHCD self-assigned this Mar 29, 2022
@MariaHCD
Copy link
Contributor

Thanks for finding the logs, @aldo-expensify!

The error 401 Cannot delete billing cards is expected. I think we'll just need to catch the exception on NewDot and show a better error message to the user here. cc: @stitesExpensify

@stitesExpensify
Copy link
Contributor

Yep makes sense to me. Maybe we should make a PR like this that covers all error cases and not just 401?

@MariaHCD
Copy link
Contributor

Thanks, @stitesExpensify! Of the two errors that markAsDeleted throws, I think 401 Cannot delete billing cards is the only one that we should worry about?

Making this a first pick! We'll need to:

  1. Get the error message we will show to the user (use the Waiting for copy label)
  2. Get the Spanish translation for the message on Slack via #espanol
  3. Catch the exception on NewDot here and show the new error message

@MariaHCD MariaHCD added Weekly KSv2 and removed Daily KSv2 labels Mar 30, 2022
@MariaHCD MariaHCD removed their assignment Mar 30, 2022
@MariaHCD MariaHCD added the FirstPick Engineering only, please! Only add when there is an identified code solution. label Mar 30, 2022
@MariaHCD MariaHCD changed the title Payments - Error message appears when deleting bank card Payments - Generic error message appears when deleting bank card Mar 30, 2022
@techievivek
Copy link
Contributor

Assigning it to myself as I am just getting started with the new Dot. Thanks

@techievivek techievivek self-assigned this Mar 30, 2022
@stitesExpensify
Copy link
Contributor

stitesExpensify commented Mar 30, 2022

401 Cannot delete billing cards is the only one that we should worry about?

Yep you're right!

@techievivek techievivek added the Waiting for copy User facing verbiage needs polishing label Apr 1, 2022
@melvin-bot
Copy link

melvin-bot bot commented Apr 1, 2022

Triggered auto assignment to @zsgreenwald (Waiting for copy), see https://stackoverflow.com/c/expensify/questions/7025/ for more details.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Apr 1, 2022
@techievivek
Copy link
Contributor

Hi Zach could you please share a well-formatted error message that we can show to the user for this issue. Thanks

@MelvinBot
Copy link

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

@zsgreenwald zsgreenwald self-assigned this Apr 5, 2022
@zsgreenwald
Copy link
Contributor

Removing Ryan, and pulling myself back in. Just to summarize, this error occurs when a user tries to delete a payment method, although they should be able to do that either way. What's a scenario where the user wouldn't be able to delete a payment method?

I feel like "Oops...something went wrong and your request could not be completed. Please try again later". is just fine, but I'll gut check this with others on the Copy side to confirm.

@MariaHCD
Copy link
Contributor

MariaHCD commented Apr 5, 2022

Thanks, @zsgreenwald!

What's a scenario where the user wouldn't be able to delete a payment method?

This particular scenario is when a user tries to delete a billing card. We currently throw an error from the backend and show the user a generic error but would like to display a more clear error message.

@zsgreenwald
Copy link
Contributor

I'd suggest something like this:

"Oops! Something went wrong. Our team is on it, but please contact Concierge if you have any questions!"

This way there's direct action to contact Concierge in case they have any questions. Thoughts?

@sylveawong
Copy link

@zsgreenwald In general, I think we've tried to move away from generic errors as much as possible so that they can address the issue immediately vs add more support volume.

@MariaHCD To confirm that i'm understanding this particular issue correctly though, this error is showing up being we specifically don't allow users to delete billing cards, right? I know its clunky to go back and forth but would something like this work in allowing them to resolve the issue?

You cannot remove this payment method because it is currently set up as your default billing card. Please change your billing card details here before trying again.

@techievivek
Copy link
Contributor

@sylveawong Yes this get shown when one tries to delete billing card.

@sylveawong
Copy link

hrmm but actually, if they update with a new billing card, the card should be removed all together. This should remove it from showing up in newdot as well, correct? If that's the case, we can update the copy to tell them to remove the card by changing billing (vs "change billing before trying again")

@zsgreenwald
Copy link
Contributor

You cannot remove this payment method because it is currently set up as your default billing card. Please change your billing card details here before trying again.

Yeah I like the direction you took here. Maybe to shorten it:

You cannot remove an active billing card. Please change your billing card details here before trying again.

Thoughts @sylveawong ?

@techievivek
Copy link
Contributor

techievivek commented Apr 7, 2022

@zsgreenwald

You cannot remove an active billing card. Please change your billing card details here before trying again.

This one looks useful but currently growl doesn't support listening to onclick events so not sure we can add a link.

@aldo-expensify
Copy link
Contributor

This one looks useful but currently growl doesn't support can't listen to onclick events so not sure we can add a link.

I know that sometimes we inline the error (no growl), that would allow you to put a link... but I'm not sure about our guidelines on when to growl and when to inline the error.

@sylveawong
Copy link

Hrmm is displaying a growl the right action to take here then?

The user doesn't know that it's being used as a billing card, and dont know that they cant remove billing cards. Once a billing card is changed, it gets removed anyway so it wont show up anymore.

So maybe the solution here is just to remove the "delete" option from payments that are currently used for billing, and possibly adding a label to indicate that its used for billing (similar to the "default" label). Thoughts?

@techievivek
Copy link
Contributor

Any update on this as to what we will be moving forward with? So that I can look into it.

@sylveawong
Copy link

What do you think about the idea of just not showing billing cards at all?

After testing the flow in newdot, I don' think it makes sense for us to have the billing card appear in the payments section since both of the dropdown options are not applicable. Users can't:

  1. set billing cards as their default payment method
  2. delete billing cards

@melvin-bot melvin-bot bot added the Overdue label Apr 13, 2022
@techievivek
Copy link
Contributor

I think we can still show the billing card but let's not show any of the dropdown options because none of the options is applicable with the billing card.

@melvin-bot melvin-bot bot removed the Overdue label Apr 14, 2022
@stitesExpensify
Copy link
Contributor

I think that not showing the card makes more sense than not showing the dropdown. If all of your other buttons have a dropdown and when you click the billing card nothing happens that feels like a bug to the user

@techievivek
Copy link
Contributor

Just started a conversation here on slack and as per suggestion I think it will be a good idea not to show the delete option at all there.

@melvin-bot melvin-bot bot added the Overdue label Apr 18, 2022
@techievivek
Copy link
Contributor

Hi 👋 just reminding so that we can close the discussion on slack here thanks.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Apr 18, 2022
@techievivek
Copy link
Contributor

Not overdue.

@melvin-bot melvin-bot bot removed the Overdue label Apr 21, 2022
@stitesExpensify
Copy link
Contributor

Bumped the slack convo again too

@stitesExpensify
Copy link
Contributor

Following up on this, we decided that the best path forward is to just not show billing cards!

@sylveawong sylveawong removed the Waiting for copy User facing verbiage needs polishing label Apr 22, 2022
@sylveawong
Copy link

Removing the waiting for copy here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering FirstPick Engineering only, please! Only add when there is an identified code solution. Improvement Item broken or needs improvement. Internal Requires API changes or must be handled by Expensify staff
Projects
None yet
Development

No branches or pull requests

10 participants