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 #5829 (Voiding an initial payment (i.e. a full refund) after partially refunding the order is not possible with Stripe-SCA) #6453

Merged

Conversation

andrewpbrett
Copy link
Contributor

What? Why?

Closes #5829

This adds a method to calculate the refundable amount by looking at the charges property of the returned PaymentIntent from Stripe. A charge is created when a partial refund is issued. If there are no partial refunds issued, the full amount will still be refunded.

What should we test?

See issue's Steps to Reproduce

Release notes

Fixed a bug that prevented payments from being refunded if a partial refund had already been issued.

Changelog Category: User facing changes

Dependencies

Documentation updates

@andrewpbrett andrewpbrett force-pushed the fix-void-payment-error branch from 61ea144 to 9646232 Compare December 5, 2020 02:14
@@ -79,6 +79,11 @@ def create_profile(payment)

private

def refundable_amount(payment_intent_response)
payment_intent_response.amount_received -
payment_intent_response.charges.data.map(&:amount_refunded).sum
Copy link
Contributor

Choose a reason for hiding this comment

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

What if there are unsuccessful charges? Looking at the docs it seems that they might not even include them all?

This list only contains the latest charge, even if there were previously multiple unsuccessful charges. To view all previous charges for a PaymentIntent, you can filter the charges list using the payment_intent parameter.

source: https://stripe.com/docs/api/payment_intents/object#payment_intent_object-charges.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this still works, because each time we try to make a payment for an order it creates a new PaymentIntent. I tried a couple things:

At first, I thought you meant "what if there are multiple partial refunds" so I tried that: I placed an order, then in the backend I made two adjustments to the order and issued a refund each time. The amount_refunded returns the sum across all refunds.

Then I realized you might be asking "what if the customer or an admin creates unsuccessful charges?" For the admin side, I placed an order through the front end and chose Cash. Then on the backend, I created two unsuccessful charges and one successful one. I was still able to do the partial refunds on the successful charge. I found the same when the unsuccessful charges were created by the customer on the front end.

The option to do the refund on the failed charge never appears in the first place, only on the successful one:
Screen Shot 2020-12-10 at 2 37 57 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

But does that mean that the payment_intent_response.charges includes only successful charges then? Sorry, I'm still wrapping my head around this Stripe API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I'm saying is that for our case, the PaymentIntent that we're querying Stripe about will only have the single successful charge. It's definitely complex! Especially since our mapping is not as direct as it could be if we were only dealing with Stripe as a provider.

@filipefurtad0 filipefurtad0 self-assigned this Dec 13, 2020
@filipefurtad0 filipefurtad0 added the pr-staged-uk staging.openfoodnetwork.org.uk label Dec 13, 2020
@filipefurtad0
Copy link
Contributor

filipefurtad0 commented Dec 13, 2020

Hey @andrewpbrett ,

I took the initial issue and reproduced those steps, after staging the PR. Tried this for 3D-auth and non-3D-auth cards (not sure this would make any difference). I associated a transaction fee for Stripe-SCA.

Issuing one refund, and voiding the initial payment
After placing the order as a customer, the admin can remove some items from that order, and issue one, or several partial refunds - this is working now, awesome.

Issuing several refunds and voiding the initial payment - with transaction fees
Below are the pics of an example, in which the admin successively removes items from an order, one by one, issuing refunds for each change. The final one, is a total refund of the initial payment:

image

The dashboard from Stripe shows all these changes correctly:

image

The values above are not correct - this is because of issue #3584, so we're all good here 👍
This is shown below, on the repeated transaction fees:

image

Failing payments (unsuccessful charges), issuing partial refunds and voiding the initial payment

This case works fine as well. All correct in the BO and in the Stripe-dashboard:

image

Ready to go!

PS: also bumped on a known issue related to the removal of the last item from an order - #5546

@filipefurtad0 filipefurtad0 removed the pr-staged-uk staging.openfoodnetwork.org.uk label Dec 13, 2020
@sauloperez sauloperez merged commit fa81236 into openfoodfoundation:master Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants