Fix #248 -- Failed Payment error handling (#333)

* [WIP] Failed Payment error handling

When finished, this should fix #248

* rename PaymentFailedException to PaymentException\nimported Exception where neccessary

* comments fixed

* minor style fixes

* Fixed a name error
This commit is contained in:
Jakob Schnell
2017-02-24 14:11:41 +01:00
committed by Raphael Michel
parent 8e4b71eb19
commit c251a48e31
5 changed files with 43 additions and 26 deletions

View File

@@ -337,9 +337,7 @@ class BasePaymentProvider:
The default implementation just returns ``None`` and therefore leaves the
order unpaid. The user will be redirected to the order's detail page by default.
On errors, you should use Django's message framework to display an error message
to the user.
On errors, you should raise a ``PaymentException``.
:param order: The order object
"""
return None
@@ -483,6 +481,10 @@ class BasePaymentProvider:
'back to the buyer manually.'))
class PaymentException(Exception):
pass
class FreeOrderProvider(BasePaymentProvider):
@property
@@ -511,7 +513,7 @@ class FreeOrderProvider(BasePaymentProvider):
try:
mark_order_paid(order, 'free', send_mail=False)
except Quota.QuotaExceededException as e:
messages.error(request, str(e))
raise PaymentException(str(e))
@property
def settings_form_fields(self) -> dict:

View File

@@ -9,7 +9,7 @@ from django.template.loader import get_template
from django.utils.translation import ugettext as __, ugettext_lazy as _
from pretix.base.models import Order, Quota, RequiredAction
from pretix.base.payment import BasePaymentProvider
from pretix.base.payment import BasePaymentProvider, PaymentException
from pretix.base.services.mail import SendMailException
from pretix.base.services.orders import mark_order_paid, mark_order_refunded
from pretix.multidomain.urlreverse import build_absolute_uri
@@ -145,17 +145,16 @@ class Paypal(BasePaymentProvider):
"""
if (request.session.get('payment_paypal_id', '') == ''
or request.session.get('payment_paypal_payer', '') == ''):
messages.error(request, _('We were unable to process your payment. See below for details on how to '
'proceed.'))
raise PaymentException(_('We were unable to process your payment. See below for details on how to '
'proceed.'))
self.init_api()
payment = paypalrestsdk.Payment.find(request.session.get('payment_paypal_id'))
if str(payment.transactions[0].amount.total) != str(order.total) or payment.transactions[0].amount.currency != \
self.event.currency:
messages.error(request, _('We were unable to process your payment. See below for details on how to '
'proceed.'))
logger.error('Value mismatch: Order %s vs payment %s' % (order.id, str(payment)))
return
raise PaymentException(_('We were unable to process your payment. See below for details on how to '
'proceed.'))
return self._execute_payment(payment, request, order)
@@ -196,9 +195,9 @@ class Paypal(BasePaymentProvider):
return
if payment.state != 'approved':
messages.error(request, _('We were unable to process your payment. See below for details on how to '
'proceed.'))
logger.error('Invalid state: %s' % str(payment))
raise PaymentException(_('We were unable to process your payment. See below for details on how to '
'proceed.'))
return
if order.status == Order.STATUS_PAID:
@@ -208,13 +207,14 @@ class Paypal(BasePaymentProvider):
try:
mark_order_paid(order, 'paypal', json.dumps(payment.to_dict()))
except Quota.QuotaExceededException as e:
messages.error(request, str(e))
RequiredAction.objects.create(
event=request.event, action_type='pretix.plugins.paypal.overpaid', data=json.dumps({
'order': order.code,
'payment': payment.id
})
)
raise PaymentException(str(e))
except SendMailException:
messages.warning(request, _('There was an error sending the confirmation mail.'))
return None

View File

@@ -9,7 +9,7 @@ from django.template.loader import get_template
from django.utils.translation import ugettext_lazy as _
from pretix.base.models import Quota, RequiredAction
from pretix.base.payment import BasePaymentProvider
from pretix.base.payment import BasePaymentProvider, PaymentException
from pretix.base.services.mail import SendMailException
from pretix.base.services.orders import mark_order_paid, mark_order_refunded
from pretix.multidomain.urlreverse import build_absolute_uri
@@ -110,13 +110,14 @@ class Stripe(BasePaymentProvider):
else:
err = {'message': str(e)}
logger.exception('Stripe error: %s' % str(e))
messages.error(request, _('Stripe reported an error with your card: %s' % err['message']))
logger.info('Stripe card error: %s' % str(err))
order.payment_info = json.dumps({
'error': True,
'message': err['message'],
})
order.save()
raise PaymentException(_('Stripe reported an error with your card:%s') % err['message'])
except stripe.error.StripeError as e:
if e.json_body:
err = e.json_body['error']
@@ -124,33 +125,33 @@ class Stripe(BasePaymentProvider):
else:
err = {'message': str(e)}
logger.exception('Stripe error: %s' % str(e))
messages.error(request, _('We had trouble communicating with Stripe. Please try again and get in touch '
'with us if this problem persists.'))
order.payment_info = json.dumps({
'error': True,
'message': err['message'],
})
order.save()
raise PaymentException(_('We had trouble communicating with Stripe. Please try again and get in touch '
'with us if this problem persists.'))
else:
if charge.status == 'succeeded' and charge.paid:
try:
mark_order_paid(order, 'stripe', str(charge))
except Quota.QuotaExceededException as e:
messages.error(request, str(e))
RequiredAction.objects.create(
event=request.event, action_type='pretix.plugins.stripe.overpaid', data=json.dumps({
'order': order.code,
'charge': charge.id
})
)
except SendMailException:
messages.warning(request, _('There was an error sending the confirmation mail.'))
raise PaymentException(str(e))
except SendMailException:
raise PaymentException(_('There was an error sending the confirmation mail.'))
else:
messages.warning(request, _('Stripe reported an error: %s' % charge.failure_message))
logger.info('Charge failed: %s' % str(charge))
order.payment_info = str(charge)
order.save()
raise PaymentException(_('Stripe reported an error: %s') % charge.failure_message)
del request.session['payment_stripe_token']
def order_pending_render(self, request, order) -> str:

View File

@@ -12,6 +12,7 @@ from django.views.generic import TemplateView, View
from pretix.base.models import CachedTicket, Invoice, Order, OrderPosition
from pretix.base.models.orders import CachedCombinedTicket, InvoiceAddress
from pretix.base.payment import PaymentException
from pretix.base.services.invoices import (
generate_cancellation, generate_invoice, invoice_pdf, invoice_qualified,
)
@@ -199,7 +200,11 @@ class OrderPaymentConfirm(EventViewMixin, OrderDetailMixin, TemplateView):
return super().dispatch(request, *args, **kwargs)
def post(self, request, *args, **kwargs):
resp = self.payment_provider.payment_perform(request, self.order)
try:
resp = self.payment_provider.payment_perform(request, self.order)
except PaymentException as e:
messages.error(request, str(e))
return redirect(self.get_order_url())
if 'payment_change_{}'.format(self.order.pk) in request.session:
del request.session['payment_change_{}'.format(self.order.pk)]
return redirect(resp or self.get_order_url())
@@ -241,7 +246,12 @@ class OrderPaymentComplete(EventViewMixin, OrderDetailMixin, View):
return super().dispatch(request, *args, **kwargs)
def get(self, request, *args, **kwargs):
resp = self.payment_provider.payment_perform(request, self.order)
try:
resp = self.payment_provider.payment_perform(request, self.order)
except PaymentException as e:
messages.error(request, str(e))
return redirect(self.get_order_url())
if self.order.status == Order.STATUS_PAID:
return redirect(resp or self.get_order_url() + '?paid=yes')
else:

View File

@@ -8,6 +8,7 @@ from django.utils.timezone import now
from stripe import APIConnectionError, CardError, StripeError
from pretix.base.models import Event, Order, Organizer
from pretix.base.payment import PaymentException
from pretix.plugins.stripe.payment import Stripe
@@ -92,7 +93,8 @@ def test_perform_card_error(env, factory, monkeypatch):
req.session = {}
prov.checkout_prepare(req, {})
assert 'payment_stripe_token' in req.session
prov.payment_perform(req, order)
with pytest.raises(PaymentException):
prov.payment_perform(req, order)
order.refresh_from_db()
assert order.status == Order.STATUS_PENDING
@@ -114,7 +116,8 @@ def test_perform_stripe_error(env, factory, monkeypatch):
req.session = {}
prov.checkout_prepare(req, {})
assert 'payment_stripe_token' in req.session
prov.payment_perform(req, order)
with pytest.raises(PaymentException):
prov.payment_perform(req, order)
order.refresh_from_db()
assert order.status == Order.STATUS_PENDING
@@ -140,7 +143,8 @@ def test_perform_failed(env, factory, monkeypatch):
req.session = {}
prov.checkout_prepare(req, {})
assert 'payment_stripe_token' in req.session
prov.payment_perform(req, order)
with pytest.raises(PaymentException):
prov.payment_perform(req, order)
order.refresh_from_db()
assert order.status == Order.STATUS_PENDING