Fix not refunding the total amount by default

Fixes jazzband#401

Author: radekholy24

CHANGELOG.rst

@@ -13,6 +13,7 @@ v3.0.0
 - Added support for Python 3.11, Django 4.1 and Django 4.2.
 - Stripe backends now supports webhooks
 - New :ref:`webhook settings <webhooks>`
+- Fixed ``base_payment.refund()`` not making any refund
 
 v2.0.0
 ------

payments/models.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import json
+import logging
 from typing import Iterable
 from uuid import uuid4
 
@@ -14,6 +15,8 @@
 from . import PurchasedItem
 from .core import provider_factory
 
+logger = logging.getLogger(__name__)
+
 
 class PaymentAttributeProxy:
     def __init__(self, payment):
@@ -212,15 +215,23 @@ def release(self):
     def refund(self, amount=None):
         if self.status != PaymentStatus.CONFIRMED:
             raise ValueError("Only charged payments can be refunded.")
-        if amount:
-            if amount > self.captured_amount:
-                raise ValueError(
-                    "Refund amount can not be greater then captured amount"
-                )
-            provider = provider_factory(self.variant, self)
-            amount = provider.refund(self, amount)
-            self.captured_amount -= amount
-        if self.captured_amount == 0 and self.status != PaymentStatus.REFUNDED:
+        if amount and amount > self.captured_amount:
+            raise ValueError("Refund amount can not be greater then captured amount")
+        provider = provider_factory(self.variant, self)
+        amount = provider.refund(self, amount)
+        # If the initial amount is None, the code above has no chance to check whether
+        # the actual amount is greater than the captured amount before actually
+        # performing the refund. But since the refund has been performed already,
+        # raising an exception would just cause inconsistencies. Thus, logging an error.
+        if amount > self.captured_amount:
+            logger.error(
+                "Refund amount of payment %s greater than captured amount: %f > %f",
+                self.id,
+                amount,
+                self.captured_amount,
+            )
+        self.captured_amount -= amount
+        if self.captured_amount <= 0 and self.status != PaymentStatus.REFUNDED:
             self.change_status(PaymentStatus.REFUNDED)
         self.save()
 

payments/test_core.py

@@ -113,20 +113,20 @@ def test_refund_too_high_amount(self):
 
     @patch("payments.dummy.DummyProvider.refund")
     def test_refund_without_amount(self, mocked_refund_method):
-        refund_amount = None
+        captured_amount = Decimal("200")
         with patch.object(BasePayment, "save") as mocked_save_method:
             mocked_save_method.return_value = None
-            mocked_refund_method.return_value = refund_amount
+            mocked_refund_method.return_value = captured_amount
 
-            captured_amount = Decimal("200")
-            status = PaymentStatus.CONFIRMED
             payment = Payment(
-                variant="default", status=status, captured_amount=captured_amount
+                variant="default",
+                status=PaymentStatus.CONFIRMED,
+                captured_amount=captured_amount,
             )
-            payment.refund(refund_amount)
-            self.assertEqual(payment.status, status)
-            self.assertEqual(payment.captured_amount, captured_amount)
-        self.assertEqual(mocked_refund_method.call_count, 0)
+            payment.refund()
+            self.assertEqual(payment.status, PaymentStatus.REFUNDED)
+            self.assertEqual(payment.captured_amount, Decimal(0))
+        self.assertEqual(mocked_refund_method.call_count, 1)
 
     @patch("payments.dummy.DummyProvider.refund")
     def test_refund_partial_success(self, mocked_refund_method):