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

PayPal: Perform full refund if amount=None #411

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

radekholy24
Copy link
Contributor

@radekholy24 radekholy24 commented Apr 25, 2024

Since PayPal does support refunding without an amount specified and django-payment's API as well, it makes sense to me.

The benefit is that for various reasons the local copy of the payment (BasePayment.captured_amount) may get easily desynchronized with the payment gateway (e.g. bugs like this #309 (see #412)). This performs the full refund regardless of the BasePayment.captured_amount.

This change will have an effect only if #401 gets fixed (see #410) but it won't hurt otherwise too.

Feel free to request any necessary changes 🙏

@radekholy24 radekholy24 force-pushed the paypal_full_refund branch 2 times, most recently from 38f5a69 to 95969ce Compare April 25, 2024 17:23
@WhyNotHugo
Copy link
Member

Can you rebase and fix merge conflicts?

(sorry, the permissions on this repo won't allow me to just perform the merge myself)

@radekholy24
Copy link
Contributor Author

@WhyNotHugo, no problem. Rebased.

@WhyNotHugo
Copy link
Member

Needs to be rebased due to #410

Copy link

codecov bot commented Jun 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.41%. Comparing base (8303e09) to head (b456e33).
Report is 15 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #411      +/-   ##
==========================================
- Coverage   78.42%   78.41%   -0.01%     
==========================================
  Files          29       29              
  Lines        1979     1983       +4     
  Branches      244      244              
==========================================
+ Hits         1552     1555       +3     
- Misses        310      311       +1     
  Partials      117      117              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@radekholy24
Copy link
Contributor Author

Sure. Rebased, @WhyNotHugo

@WhyNotHugo WhyNotHugo merged commit 39d6564 into jazzband:main Jun 12, 2024
23 of 24 checks passed
@WhyNotHugo
Copy link
Member

Thanks!

Goury pushed a commit to Goury/django-payments that referenced this pull request Jul 6, 2024
Goury added a commit to Goury/django-payments that referenced this pull request Jul 6, 2024
fix jazzband#411 Manually following process URL results in a crash
Goury pushed a commit to Goury/django-payments that referenced this pull request Jul 6, 2024
Goury added a commit to Goury/django-payments that referenced this pull request Jul 6, 2024
fix jazzband#411 Manually following process URL results in a crash
Goury pushed a commit to Goury/django-payments that referenced this pull request Jul 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants