diff --git a/payments/core.py b/payments/core.py index e81bfab6e..bc3a60be2 100644 --- a/payments/core.py +++ b/payments/core.py @@ -26,8 +26,8 @@ def get_base_url(): """ Returns host url according to project settings. Protocol is chosen by checking PAYMENT_USES_SSL variable. - If PAYMENT_HOST is not specified, gets domain from Sites. - Otherwise checks if it's callable and returns it's result. If it's not a + If PAYMENT_HOST is not specified, gets domain from Sites. + Otherwise checks if it's callable and returns it's result. If it's not a callable treats it as domain. """ protocol = 'https' if PAYMENT_USES_SSL else 'http' @@ -92,13 +92,16 @@ def get_return_url(self, payment, extra_data=None): return url + '?' + qs return url - def capture(self, payment, amount=None): + def capture(self, payment, amount=None, final=True): + ''' Capture a fraction of the total amount of a payment. Return amount captured or None ''' raise NotImplementedError() def release(self, payment): + ''' Annilates captured payment ''' raise NotImplementedError() def refund(self, payment, amount=None): + ''' Refund payment, return amount which was refunded or None ''' raise NotImplementedError() diff --git a/payments/cybersource/__init__.py b/payments/cybersource/__init__.py index 6b119ebf2..40125796a 100644 --- a/payments/cybersource/__init__.py +++ b/payments/cybersource/__init__.py @@ -161,7 +161,7 @@ def charge(self, payment, data): self._set_proper_payment_status_from_reason_code( payment, response.reasonCode) - def capture(self, payment, amount=None): + def capture(self, payment, amount=None, final=True): if amount is None: amount = payment.total params = self._prepare_capture(payment, amount=amount) diff --git a/payments/dummy/__init__.py b/payments/dummy/__init__.py index 0a693c701..1d1788be5 100644 --- a/payments/dummy/__init__.py +++ b/payments/dummy/__init__.py @@ -63,7 +63,7 @@ def process_data(self, payment, request): return HttpResponseRedirect(payment.get_success_url()) return HttpResponseRedirect(payment.get_failure_url()) - def capture(self, payment, amount=None): + def capture(self, payment, amount=None, final=True): payment.change_status(PaymentStatus.CONFIRMED) return amount diff --git a/payments/models.py b/payments/models.py index 2cbd8206a..dc23a7a09 100644 --- a/payments/models.py +++ b/payments/models.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals import json from uuid import uuid4 +import logging from django.conf import settings from django.core.urlresolvers import reverse @@ -10,6 +11,8 @@ from .core import provider_factory from . import FraudStatus, PaymentStatus +# Get an instance of a logger +logger = logging.getLogger(__name__) class PaymentAttributeProxy(object): @@ -82,11 +85,16 @@ def change_status(self, status, message=''): ''' Updates the Payment status and sends the status_changed signal. ''' - from .signals import status_changed - self.status = status - self.message = message - self.save() - status_changed.send(sender=type(self), instance=self) + if self.status != status: + from .signals import status_changed + self.status = status + self.message = message + self.save() + for receiver, result in status_changed.send_robust(sender=type(self), instance=self): + if isinstance(result, Exception): + logger.critical(result) + else: + self.save() def change_fraud_status(self, status, message='', commit=True): available_statuses = [choice[0] for choice in FraudStatus.CHOICES] @@ -133,17 +141,21 @@ def get_success_url(self): def get_process_url(self): return reverse('process_payment', kwargs={'token': self.token}) - def capture(self, amount=None): + def capture(self, amount=None, final=True): + ''' Capture a fraction of the total amount of a payment. Return amount captured or None ''' if self.status != PaymentStatus.PREAUTH: raise ValueError( 'Only pre-authorized payments can be captured.') provider = provider_factory(self.variant) - amount = provider.capture(self, amount) + amount = provider.capture(self, amount, final) if amount: - self.captured_amount = amount - self.change_status(PaymentStatus.CONFIRMED) + self.captured_amount += amount + if final: + self.change_status(PaymentStatus.CONFIRMED) + return amount def release(self): + ''' Annilates captured payment ''' if self.status != PaymentStatus.PREAUTH: raise ValueError( 'Only pre-authorized payments can be released.') @@ -152,6 +164,7 @@ def release(self): self.change_status(PaymentStatus.REFUNDED) def refund(self, amount=None): + ''' Refund payment, return amount which was refunded or None ''' if self.status != PaymentStatus.CONFIRMED: raise ValueError( 'Only charged payments can be refunded.') @@ -159,12 +172,14 @@ def refund(self, amount=None): if amount > self.captured_amount: raise ValueError( 'Refund amount can not be greater then captured amount') - provider = provider_factory(self.variant) - amount = provider.refund(self, amount) + provider = provider_factory(self.variant) + amount = provider.refund(self, amount) + if amount: self.captured_amount -= amount - if self.captured_amount == 0 and self.status != PaymentStatus.REFUNDED: - self.change_status(PaymentStatus.REFUNDED) - self.save() + if self.captured_amount == 0 and self.status != PaymentStatus.REFUNDED: + self.change_status(PaymentStatus.REFUNDED) + self.save() + return amount @property def attrs(self): diff --git a/payments/paypal/__init__.py b/payments/paypal/__init__.py index fbeb2421e..8b82a49b2 100644 --- a/payments/paypal/__init__.py +++ b/payments/paypal/__init__.py @@ -252,13 +252,13 @@ def get_amount_data(self, payment, amount=None): 'total': str(amount.quantize( CENTS, rounding=ROUND_HALF_UP))} - def capture(self, payment, amount=None): + def capture(self, payment, amount=None, final=True): if amount is None: amount = payment.total amount_data = self.get_amount_data(payment, amount) capture_data = { 'amount': amount_data, - 'is_final_capture': True + 'is_final_capture': final } links = self._get_links(payment) url = links['capture']['href'] diff --git a/payments/stripe/__init__.py b/payments/stripe/__init__.py index f4a7fb5c5..a1b6faaa3 100644 --- a/payments/stripe/__init__.py +++ b/payments/stripe/__init__.py @@ -31,7 +31,7 @@ def get_form(self, payment, data=None): raise RedirectNeeded(payment.get_success_url()) return form - def capture(self, payment, amount=None): + def capture(self, payment, amount=None, final=True): amount = int((amount or payment.total) * 100) charge = stripe.Charge.retrieve(payment.transaction_id) try: diff --git a/payments/stripe/test_stripe.py b/payments/stripe/test_stripe.py index dd08ccfde..9a67db297 100644 --- a/payments/stripe/test_stripe.py +++ b/payments/stripe/test_stripe.py @@ -35,7 +35,7 @@ def change_fraud_status(self, status, message='', commit=True): self.fraud_status = status self.fraud_message = message - def capture(self, amount=None): + def capture(self, amount=None, final=True): amount = amount or self.total self.captured_amount = amount self.change_status(PaymentStatus.CONFIRMED) diff --git a/payments/test_core.py b/payments/test_core.py index f6f3001fe..4a09c33ed 100644 --- a/payments/test_core.py +++ b/payments/test_core.py @@ -3,6 +3,8 @@ from unittest import TestCase from mock import patch, NonCallableMock +from django.dispatch import Signal + from payments import core from .forms import CreditCardPaymentFormWithName, PaymentForm from .models import BasePayment @@ -42,6 +44,28 @@ def test_capture_with_wrong_status(self): payment = BasePayment(variant='default', status=PaymentStatus.WAITING) self.assertRaises(ValueError, payment.capture) + @patch('payments.signals.status_changed', new_callable=Signal) + def test_robust_signals(self, mocked_signal): + with patch.object(BasePayment, 'save') as mocked_save_method: + mocked_save_method.return_value = None + def rogue_handler(sender, instance, **kwargs): + raise Exception("Here be dragons") + def benign_handler(sender, instance, **kwargs): + pass + class UnrelatedClass(object): + pass + def unrelated_handler(sender, instance, **kwargs): + raise Exception("Should not be called") + mocked_signal.connect(rogue_handler, sender=BasePayment) + mocked_signal.connect(benign_handler, sender=BasePayment) + mocked_signal.connect(unrelated_handler, sender=UnrelatedClass) + payment = BasePayment(variant='default', status=PaymentStatus.PREAUTH) + # python < 3.4 has no asserLogs + if hasattr(self, "assertLogs"): + with self.assertLogs("payments.models", "CRITICAL") as logs: + payment.change_status(PaymentStatus.WAITING, "fooo") + self.assertEqual(logs.output, ['CRITICAL:payments.models:Here be dragons']) + @patch('payments.dummy.DummyProvider.capture') def test_capture_preauth_successfully(self, mocked_capture_method): amount = Decimal('20') @@ -49,7 +73,9 @@ def test_capture_preauth_successfully(self, mocked_capture_method): mocked_save_method.return_value = None mocked_capture_method.return_value = amount - payment = BasePayment(variant='default', status=PaymentStatus.PREAUTH) + captured_amount = Decimal('0') + payment = BasePayment(variant='default', captured_amount=captured_amount, + status=PaymentStatus.PREAUTH) payment.capture(amount) self.assertEqual(payment.status, PaymentStatus.CONFIRMED) @@ -63,7 +89,7 @@ def test_capture_preauth_without_amount(self, mocked_capture_method): mocked_save_method.return_value = None mocked_capture_method.return_value = amount - captured_amount = Decimal('100') + captured_amount = Decimal('0') status = PaymentStatus.PREAUTH payment = BasePayment(variant='default', status=status, captured_amount=captured_amount) @@ -110,7 +136,7 @@ def test_refund_without_amount(self, mocked_refund_method): payment.refund(refund_amount) self.assertEqual(payment.status, status) self.assertEqual(payment.captured_amount, captured_amount) - self.assertEqual(mocked_refund_method.call_count, 0) + self.assertEqual(mocked_refund_method.call_count, 1) @patch('payments.dummy.DummyProvider.refund') def test_refund_partial_success(self, mocked_refund_method):