Fix order cancel with Payment Intents captured payment#57
Conversation
|
|
||
| def try_void(payment) | ||
| if payment.completed? | ||
| reason = Spree::RefundReason.find_by(name: self.class::PAYMENT_INTENTS_REFUND_REASON) || Spree::RefundReason.first |
There was a problem hiding this comment.
I think is just necessary PAYMENTS_INTENTS_REFUND_REASON, since the constant is in the same class.
Also, instead to fallback to first reason maybe we can use find_or_create?
There was a problem hiding this comment.
@ccarruitero thanks for spotting the issue with the constant, what was I thinking when I wrote that?
I thought about using find_or_create, but then I went for the migration (the record is explicitly created in a more "expected" place) and the fallback to Spree::RefundReason.first for making the custom refund reason optional.
But I think that your options are good as well. I'm wondering if a rake task for adding the refund reason might be a better option... does anybody want to share opinions on this?
There was a problem hiding this comment.
I have some concerns about creating a migration for this RefundReason. Can't we use an already existing one as default, like Spree::Payment::Cancellation::DEFAULT_REASON? Maybe also defined in a separate method so that people can customize it and use something else if needed?
spaghetticode
force-pushed
the
spaghetticode/payment-intents-order-cancel
branch
from
07fc6ef to
32b2a22
Compare
A decorator is added in order to be able to properly access the ivar `@response` that is set when the refund is performed. That variable points to a `ActiveMerchant::Billing::Response` object.
spaghetticode
force-pushed
the
spaghetticode/payment-intents-order-cancel
branch
2 times, most recently
from
4476e81 to
e2b3863
Compare
| if v3_intents? && payment.completed? | ||
| Spree::Refund.create!( | ||
| payment: payment, | ||
| amount: payment.amount, |
There was a problem hiding this comment.
I think you can use credit_allowed here (like in Spree::Payment::Cancellation#cancel)
I don't think it's a problem for Stripe but, if there are some partial refunds already done, credit_allowed should give the correct amount to be refunded. WDYT?
This method is now the preferred way in Solidus for canceling a payment and supersedes the old #cancel method. If a payment for a Payment Intent was captured, it cannot be cancelled anymore, but needs to be refunded. For this refund we're using the default order cancel reason named after `Spree::Payment::Cancellation::DEFAULT_REASON`.
Due to Payment Intents customizations, we need to verify that order cancels work properly with both a newly generated Payment Intent payment and with one created from an already saved credit card, when the payment has already been captured.
spaghetticode
force-pushed
the
spaghetticode/payment-intents-order-cancel
branch
from
e2b3863 to
b9f2e7b
Compare
| end | ||
|
|
||
| def try_void(payment) | ||
| if v3_intents? && payment.completed? |
There was a problem hiding this comment.
Why don't you just return false here? It seems like solidus already handles the refund here 🤔
There was a problem hiding this comment.
@ChristianRimondi that way the payment would remain to its original state, instead of becomingvoid, as this line would not run. That line executes this method.
5 participants
Fixes #56
This PR adds the
Spree::PaymentMethod::StripeCreditCard#try_voidmethod that is now called when an order is canceled in the admin area (Solidus used to call the now deprecated methodSpree::PaymentMethod#cancel).If we're dealing with a Payment Intents payment, and the payment is already captured, Stripe will not allow canceling it, we can only refund it.
Also, a custom refund reason record is added via migration, in order to be able to clearly determine those payments that were refunded because there was no alternative.