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

Fix payment_intent_unexpected_state error when voiding payment intents #115

Open
wants to merge 1 commit into
base: v4
Choose a base branch
from

Conversation

jfturcot
Copy link

Solves #64

If a payment intent has been created but the order is never completed, Stripe will eventually time out the intent and change its state. When a new payment intent is created, it first tries to void all the previous uncompleted payment intents of the user and this would break with the Stripe error payment_intent_unexpected_state since Stripe's state machine does not allow that change.

…xpected_state error since it was already voided by Stripe
@codecov-commenter
Copy link

codecov-commenter commented Sep 12, 2021

Codecov Report

Merging #115 (fe96667) into master (a57ff9d) will decrease coverage by 0.76%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #115      +/-   ##
==========================================
- Coverage   95.45%   94.69%   -0.77%     
==========================================
  Files          15       15              
  Lines         352      358       +6     
==========================================
+ Hits          336      339       +3     
- Misses         16       19       +3     
Impacted Files Coverage Δ
app/decorators/models/spree/payment_decorator.rb 90.90% <83.33%> (-9.10%) ⬇️
...dels/solidus_stripe/address_from_params_service.rb 96.42% <0.00%> (-3.58%) ⬇️
.../models/spree/payment_method/stripe_credit_card.rb 97.97% <0.00%> (-1.02%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a57ff9d...fe96667. Read the comment docs.

@stale
Copy link

stale bot commented Nov 11, 2022

This issue has been automatically marked as stale because it has not had recent activity. It might be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 11, 2022
@gsmendoza gsmendoza removed the stale label Nov 11, 2022
@elia elia added the v4.x Issues and PRs targeting the classic frontend (before v5) label Nov 29, 2022
@elia elia changed the base branch from master to v4 December 20, 2022 10:25
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @jfturcot!

I was wondering if there's a way to add this logic to SolidusStripe::PaymentMethod instead, in order to avoid overriding the payment class (which will be used by other payment methods as well).

The class I referenced has Spree::PaymentMethod as parent class, which implements a generic try_void method, which is called when the subclass doesn't implement one.

What do you think? Would it make sense to move this logic there?

@kennyadsl
Copy link
Member

Hello @jfturcot! Are you still interested in moving this PR forward? Otherwise, we can probably continue the work to fix the issue. Thanks!

@jfturcot
Copy link
Author

@kennyadsl Sure! Sorry I have been very busy lately, but I would love to get this merged. I will take a look at it over the weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v4.x Issues and PRs targeting the classic frontend (before v5)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants