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

Pay invoice manually #1900

Merged
merged 20 commits into from
Feb 8, 2022
Merged

Pay invoice manually #1900

merged 20 commits into from
Feb 8, 2022

Conversation

FSM1
Copy link
Contributor

@FSM1 FSM1 commented Feb 1, 2022

closes #1884
closes #1891

Testing the card payment flow for failed payments has become more difficult as no invoice or upgrade is processed if the initial payment fails. This PR handles the case where initial payment fails, displaying an error message to the user.

Submission checklist:

Layout

  • Change looks good in the desktop web ui
  • Change looks good in the mobile web ui

Theme

  • Components / elements inspected in light mode
  • Components / elements inspected in dark mode

@render
Copy link

render bot commented Feb 1, 2022

@render
Copy link

render bot commented Feb 1, 2022

@render
Copy link

render bot commented Feb 1, 2022

@github-actions github-actions bot added the Type: Feature Added to PRs to identify that the change is a new feature. label Feb 1, 2022
@lgtm-com
Copy link

lgtm-com bot commented Feb 1, 2022

This pull request introduces 2 alerts when merging 65a8b0d into 3d0d0a7 - view on LGTM.com

new alerts:

  • 2 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Feb 4, 2022

This pull request introduces 4 alerts when merging 59236db into 872cd2f - view on LGTM.com

new alerts:

  • 4 for Unused variable, import, function or class

@FSM1 FSM1 changed the title initial testing Pay invoice manually Feb 7, 2022
@FSM1 FSM1 marked this pull request as ready for review February 7, 2022 13:19
@FSM1 FSM1 requested review from Tbaut, tanmoyAtb and asnaith February 7, 2022 13:19
@FSM1 FSM1 self-assigned this Feb 7, 2022
@FSM1 FSM1 added the Status: Review Needed 👀 Added to PRs when they need more review label Feb 7, 2022
@lgtm-com
Copy link

lgtm-com bot commented Feb 7, 2022

This pull request introduces 1 alert when merging 7dbfc42 into 012f0ca - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@Tbaut
Copy link
Collaborator

Tbaut commented Feb 7, 2022

lint and lgtm-com are complaining a little :D

@FSM1
Copy link
Contributor Author

FSM1 commented Feb 7, 2022

lint and lgtm-com are complaining a little :D

not anymore >.<

Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Found some conflicts resolution gone bad!

FSM1 and others added 2 commits February 7, 2022 17:04
@FSM1 FSM1 requested a review from Tbaut February 7, 2022 17:45
@asnaith
Copy link
Contributor

asnaith commented Feb 7, 2022

Nice. The error message looks good and pay now button on the billing history page now returns the user to the payment flow.

I noticed an issue with the error message being retained after leaving and returning to the modal but I realized it was a pre-existing issue so I've created a separate issue for that (#1922)

Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Just looked at the code, found a couple nits, I'll play with it now and check for #1922

FSM1 and others added 2 commits February 8, 2022 13:27
@FSM1 FSM1 requested a review from Tbaut February 8, 2022 11:50
@lgtm-com
Copy link

lgtm-com bot commented Feb 8, 2022

This pull request introduces 1 alert when merging 1019ade into b9c4feb - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@FSM1 FSM1 enabled auto-merge (squash) February 8, 2022 13:32
Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

looks good, I'll push a PR right away for what Andrew found

@FSM1 FSM1 merged commit a08e413 into dev Feb 8, 2022
@FSM1 FSM1 deleted the feat/pay-stripe-invoice-1884 branch February 8, 2022 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Review Needed 👀 Added to PRs when they need more review Type: Feature Added to PRs to identify that the change is a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Pay Now" link does not return user to "Pay with crypto" modal Handle failed credit card payments gracefully
4 participants