Avoid duplicated Solidus refunds

waiting-for-dev commented 1 year ago

Summary

Stripe doesn't guarantee that a webhook event will be delivered only once, so we need to be prepared to handle duplicated incoming refund events. Just creating a Solidus copy whenever we receive a refund event is not enough.

On top of that, Stripe won't identify the refund that triggered the event. Both the charge.refunded and payment_intent.succeeded (for partial captures, which generates a refund for the remaining amount under the hood) webhooks only give us information to retrieve the list of all refunds associated with a charge or payment intent.

Because of this, we are now syncing all the refunds associated with a payment intent at every webhook event. We keep track of the refunds already present on Solidus by leveraging the transaction_id field on Spree::Refund, copying the Stripe refund id as value.

On top of that, we need to account for refunds created from Solidus admin panel, which calls the Stripe API. In this case, we need to avoid syncing the refund back to Solidus on the subsequent webhook. We're marking those refunds with a metadata field on Stripe, so we can exclude them from the sync.

Closes #262

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

[x] I have written a thorough PR description.
[x] I have kept my commits small and atomic.
[x] I have used clear, explanatory commit messages.

The following are not always needed:

📖 I have updated the README to account for my changes.
📑 I have documented new code with YARD.
🛣️ I have opened a PR to update the guides.
✅ I have added automated tests to cover my changes.
📸 I have attached screenshots to demo visual changes.

codecov[bot] commented 1 year ago

Codecov Report

Merging #281 (062e69a) into master (d86549b) will increase coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #281      +/-   ##
==========================================
+ Coverage   99.58%   99.59%   +0.01%     
==========================================
  Files          26       27       +1     
  Lines         477      489      +12     
==========================================
+ Hits          475      487      +12     
  Misses          2        2

Impacted Files	Coverage Δ
app/models/solidus_stripe/gateway.rb	`100.00% <100.00%> (ø)`
...ribers/solidus_stripe/webhook/charge_subscriber.rb	`100.00% <100.00%> (ø)`
...olidus_stripe/webhook/payment_intent_subscriber.rb	`100.00% <100.00%> (ø)`
lib/solidus_stripe/refunds_synchronizer.rb	`100.00% <100.00%> (ø)`

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

elia commented 1 year ago

@waiting-for-dev sorry if the review took long, maybe some of the comments can be addressed in a follow up if they make sense to you

waiting-for-dev commented 1 year ago

Thanks for your review @elia. Please, see https://github.com/solidusio/solidus_stripe/pull/290

solidusio / solidus_stripe