diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 01561ac69..daea9e7d0 100644 --- a/CHANGELOG.rst +++ b/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 ` +- Fixed ``base_payment.refund()`` not making any refund v2.0.0 ------ diff --git a/payments/models.py b/payments/models.py index 52e7e76da..fe5203aee 100644 --- a/payments/models.py +++ b/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() diff --git a/payments/test_core.py b/payments/test_core.py index 8c9190dd6..0ff939452 100644 --- a/payments/test_core.py +++ b/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):