Fix regression in handling gift card payments (#2936)

src/pretix/base/models/orders.py

@@ -1651,7 +1651,7 @@ class OrderPayment(models.Model):
         }, user=user, auth=auth)
 
     def confirm(self, count_waitinglist=True, send_mail=True, force=False, user=None, auth=None, mail_text='',
-                ignore_date=False, lock=True, payment_date=None):
+                ignore_date=False, lock=True, payment_date=None, generate_invoice=True):
         """
         Marks the payment as complete. If possible, this also marks the order as paid if no further
         payment is required
@@ -1714,10 +1714,11 @@ class OrderPayment(models.Model):
             ))
             return
 
-        self._mark_order_paid(count_waitinglist, send_mail, force, user, auth, mail_text, ignore_date, lock, payment_sum - refund_sum)
+        self._mark_order_paid(count_waitinglist, send_mail, force, user, auth, mail_text, ignore_date, lock, payment_sum - refund_sum,
+                              generate_invoice)
 
     def _mark_order_paid(self, count_waitinglist=True, send_mail=True, force=False, user=None, auth=None, mail_text='',
-                         ignore_date=False, lock=True, payment_refund_sum=0):
+                         ignore_date=False, lock=True, payment_refund_sum=0, allow_generate_invoice=True):
         from pretix.base.services.invoices import (
             generate_invoice, invoice_qualified,
         )
@@ -1734,7 +1735,7 @@ class OrderPayment(models.Model):
                                   ignore_date=ignore_date)
 
         invoice = None
-        if invoice_qualified(self.order):
+        if invoice_qualified(self.order) and allow_generate_invoice:
             invoices = self.order.invoices.filter(is_cancellation=False).count()
             cancellations = self.order.invoices.filter(is_cancellation=True).count()
             gen_invoice = (

src/pretix/base/payment.py

@@ -175,7 +175,9 @@ class BasePaymentProvider:
         """
         Set this to ``True`` if your ``execute_payment`` function needs to be triggered by a user request, i.e. either
         needs the ``request`` object or might require a browser redirect. If this is ``False``, you will not receive
-        a ``request`` and may not redirect since execute_payment might be called server-side.
+        a ``request`` and may not redirect since execute_payment might be called server-side. You should ensure that
+        your ``execute_payment`` method has a limited execution time (i.e. by using ``timeout`` for all external calls)
+        and handles all error cases appropriately.
         """
         return True
 
@@ -1385,7 +1387,7 @@ class GiftCardPayment(BasePaymentProvider):
         except GiftCard.MultipleObjectsReturned:
             messages.error(request, _("This gift card can not be redeemed since its code is not unique. Please contact the organizer of this event."))
 
-    def execute_payment(self, request: HttpRequest, payment: OrderPayment) -> str:
+    def execute_payment(self, request: HttpRequest, payment: OrderPayment, is_early_special_case=False) -> str:
         for p in payment.order.positions.all():
             if p.item.issue_giftcard:
                 raise PaymentException(_("You cannot pay with gift cards when buying a gift card."))
@@ -1421,7 +1423,7 @@ class GiftCardPayment(BasePaymentProvider):
                     'gift_card': gc.pk,
                     'transaction_id': trans.pk,
                 }
-                payment.confirm()
+                payment.confirm(send_mail=not is_early_special_case, generate_invoice=not is_early_special_case)
         except PaymentException as e:
             payment.fail(info={'error': str(e)})
             raise e

src/pretix/base/services/orders.py

@@ -74,7 +74,7 @@ from pretix.base.models.orders import (
 )
 from pretix.base.models.organizer import TeamAPIToken
 from pretix.base.models.tax import TAXED_ZERO, TaxedPrice, TaxRule
-from pretix.base.payment import PaymentException
+from pretix.base.payment import GiftCardPayment, PaymentException
 from pretix.base.reldate import RelativeDateWrapper
 from pretix.base.secrets import assign_ticket_secret
 from pretix.base.services import tickets
@@ -1029,6 +1029,9 @@ def _perform_order(event: Event, payment_requests: List[dict], position_ids: Lis
         locked = True
         lockfn = event.lock
 
+    warnings = []
+    any_payment_failed = False
+
     with lockfn() as now_dt:
         positions = list(
             positions.select_related('item', 'variation', 'subevent', 'seat', 'addon_to').prefetch_related('addons')
@@ -1042,25 +1045,52 @@ def _perform_order(event: Event, payment_requests: List[dict], position_ids: Lis
         order, payment_objs = _create_order(event, email, positions, now_dt, payment_requests,
                                             locale=locale, address=addr, meta_info=meta_info, sales_channel=sales_channel,
                                             shown_total=shown_total, customer=customer)
+        try:
+            for p in payment_objs:
+                if p.provider == 'free':
+                    p.confirm(send_mail=False, lock=not locked, generate_invoice=False)
+        except Quota.QuotaExceededException:
+            pass
 
-        free_order_flow = (
+    # We give special treatment to GiftCardPayment here because our invoice renderer expects gift cards to already be
-            payment_objs and
+    # processed, and because we historically treat gift card orders like free orders with regards to email texts.
-            any(p['provider'] == 'free' for p in payment_requests) and
+    # It would be great to give external gift card plugins the same special treatment, but it feels to risky for now, as
-            order.pending_sum == Decimal('0.00') and
+    # (a) there would be no email at all if the plugin fails in a weird way and (b) we'd be able to run into
-            not order.require_approval
+    # contradictions when a plugin set both execute_payment_needs_user=False as well as requires_invoice_immediately=True
-        )
+    for p in payment_objs:
-        if free_order_flow:
+        if isinstance(p.payment_provider, GiftCardPayment):
             try:
-                for p in payment_objs:
+                p.process_initiated = True
-                    if p.provider == 'free':
+                p.save(update_fields=['process_initiated'])
-                        p.confirm(send_mail=False, lock=not locked)
+                p.payment_provider.execute_payment(None, p, is_early_special_case=True)
-            except Quota.QuotaExceededException:
+            except PaymentException as e:
-                pass
+                warnings.append(str(e))
+                any_payment_failed = True
+            except Exception:
+                logger.exception('Error during payment attempt')
+
+    pending_sum = order.pending_sum
+    free_order_flow = (
+        payment_objs and
+        (
+            any(p['provider'] == 'free' for p in payment_requests) or
+            all(p['provider'] == 'giftcard' for p in payment_requests)
+        ) and
+        pending_sum == Decimal('0.00') and
+        not order.require_approval
+    )
 
     invoice = order.invoices.last()  # Might be generated by plugin already
     if not invoice and invoice_qualified(order):
-        if event.settings.get('invoice_generate') == 'True' or (
+        invoice_required = (
-                event.settings.get('invoice_generate') == 'paid' and any(p['pprov'].requires_invoice_immediately for p in payment_requests)):
+            event.settings.get('invoice_generate') == 'True' or (
+                event.settings.get('invoice_generate') == 'paid' and (
+                    any(p['pprov'].requires_invoice_immediately for p in payment_requests) or
+                    pending_sum <= Decimal('0.00')
+                )
+            )
+        )
+        if invoice_required:
             invoice = generate_invoice(
                 order,
                 trigger_pdf=not event.settings.invoice_email_attachment or not order.email
@@ -1100,23 +1130,22 @@ def _perform_order(event: Event, payment_requests: List[dict], position_ids: Lis
                         _order_placed_email_attendee(event, order, p, email_attendees_template, subject_attendees_template, log_entry,
                                                      is_free=free_order_flow)
 
-    warnings = []
+    if not any_payment_failed:
-    any_failed = False
+        for p in payment_objs:
-    for p in payment_objs:
+            if not p.payment_provider.execute_payment_needs_user and not p.process_initiated:
-        if not p.payment_provider.execute_payment_needs_user:
+                try:
-            try:
+                    p.process_initiated = True
-                p.process_initiated = True
+                    p.save(update_fields=['process_initiated'])
-                p.save(update_fields=['process_initiated'])
+                    resp = p.payment_provider.execute_payment(None, p)
-                resp = p.payment_provider.execute_payment(None, p)
+                    if isinstance(resp, str):
-                if isinstance(resp, str):
+                        logger.warning('Payment provider returned URL from execute_payment even though execute_payment_needs_user is not set')
-                    logger.warning('Payment provider returned URL from execute_payment even though execute_payment_needs_user is not set')
+                except PaymentException as e:
-            except PaymentException as e:
+                    warnings.append(str(e))
-                warnings.append(str(e))
+                    any_payment_failed = True
-                any_failed = True
+                except Exception:
-            except Exception:
+                    logger.exception('Error during payment attempt')
-                logger.exception('Error during payment attempt')
 
-    if any_failed:
+    if any_payment_failed:
         # Cancel all other payments because their amount might be wrong now.
         for p in payment_objs:
             if p.state == OrderPayment.PAYMENT_STATE_CREATED: