refactoring

src/pretix/base/models/orders.py

@@ -44,7 +44,7 @@ from datetime import datetime, time, timedelta
 from decimal import Decimal
 from functools import reduce
 from time import sleep
-from typing import Any, Dict, List, Union
+from typing import Any, Dict, Iterable, List, Union
 from zoneinfo import ZoneInfo
 
 import dateutil
@@ -79,7 +79,7 @@ from pretix.base.i18n import language
 from pretix.base.models import Customer, User
 from pretix.base.reldate import RelativeDateWrapper
 from pretix.base.settings import PERSON_NAME_SCHEMES
-from pretix.base.signals import order_gracefully_delete
+from pretix.base.signals import allow_ticket_download, order_gracefully_delete
 
 from ...helpers import OF_SELF
 from ...helpers.countries import CachedCountries, FastCountryField
@@ -1137,12 +1137,19 @@ class Order(LockModel, LoggedModel):
                 attach_tickets=True,
             )
 
+    @property
+    def positions_with_tickets_ignoring_plugins(self):
+        return (op for op in self.positions.select_related('item') if op.generate_ticket)
+
     @property
     def positions_with_tickets(self):
-        for op in self.positions.select_related('item'):
+        signal_response = allow_ticket_download.send(self.event, order=self)
-            if not op.generate_ticket:
+        if all([r is True for rr, r in signal_response]):
-                continue
+            return self.positions_with_tickets_ignoring_plugins
-            yield op
+        elif any([r is False for rr, r in signal_response]):
+            return []
+        else:
+            return set.intersection(set(self.positions_with_tickets_ignoring_plugins), *[set(r) for rr, r in signal_response if isinstance(r, Iterable)])
 
     def create_transactions(self, is_new=False, positions=None, fees=None, dt_now=None, migrated=False,
                             _backfill_before_cancellation=False, save=True):

src/pretix/base/services/orders.py

@@ -98,10 +98,9 @@ from pretix.base.services.pricing import (
 from pretix.base.services.quotas import QuotaAvailability
 from pretix.base.services.tasks import ProfiledEventTask, ProfiledTask
 from pretix.base.signals import (
-    allow_ticket_download, order_approved, order_canceled, order_changed,
+    order_approved, order_canceled, order_changed, order_denied, order_expired,
-    order_denied, order_expired, order_fee_calculation, order_paid,
+    order_fee_calculation, order_paid, order_placed, order_split,
-    order_placed, order_split, order_valid_if_pending, periodic_task,
+    order_valid_if_pending, periodic_task, validate_order,
-    validate_order,
 )
 from pretix.celery_app import app
 from pretix.helpers import OF_SELF
@@ -1408,23 +1407,16 @@ def send_download_reminders(sender, **kwargs):
             if o.download_reminder_sent:
                 # Race condition
                 continue
-            if not all([r for rr, r in allow_ticket_download.send(event, order=o)]):
+            positions = o.positions_with_tickets
+            if not list(positions):
                 continue
 
             if not o.ticket_download_available:
                 continue
-            positions = o.positions.select_related('item')
 
             if o.status != Order.STATUS_PAID:
                 if o.status != Order.STATUS_PENDING or o.require_approval or (not o.valid_if_pending and not o.event.settings.ticket_download_pending):
                     continue
-            send = False
-            for p in positions:
-                if p.generate_ticket:
-                    send = True
-                    break
-            if not send:
-                continue
 
             with language(o.locale, o.event.settings.region):
                 o.download_reminder_sent = True
@@ -1442,10 +1434,7 @@ def send_download_reminders(sender, **kwargs):
                     logger.exception('Reminder email could not be sent')
 
                 if event.settings.mail_send_download_reminder_attendee:
-                    for p in o.positions.all():
+                    for p in o.positions_with_tickets:
-                        if not p.generate_ticket:
-                            continue
-
                         if p.subevent_id:
                             reminder_date = (p.subevent.date_from - timedelta(days=days)).replace(
                                 hour=0, minute=0, second=0, microsecond=0

src/pretix/base/services/tickets.py

@@ -34,7 +34,7 @@ from pretix.base.models import (
 )
 from pretix.base.services.tasks import EventTask, ProfiledTask
 from pretix.base.settings import PERSON_NAME_SCHEMES
-from pretix.base.signals import allow_ticket_download, register_ticket_outputs
+from pretix.base.signals import register_ticket_outputs
 from pretix.celery_app import app
 from pretix.helpers.database import rolledback_transaction
 
@@ -124,8 +124,8 @@ def preview(event: int, provider: str):
 
 
 def get_tickets_for_order(order, base_position=None):
-    can_download = all([r for rr, r in allow_ticket_download.send(order.event, order=order)])
+    positions = list(order.positions_with_tickets)
-    if not can_download:
+    if not positions:
         return []
     if not order.ticket_download_available:
         return []
@@ -135,10 +135,8 @@ def get_tickets_for_order(order, base_position=None):
         for receiver, response
         in register_ticket_outputs.send(order.event)
     ]
-
     tickets = []
 
-    positions = list(order.positions_with_tickets)
     if base_position:
         # Only the given position and its children
         positions = [
@@ -202,7 +200,6 @@ def get_tickets_for_order(order, base_position=None):
                     ))
                 except:
                     logger.exception('Failed to generate ticket.')
-
     return tickets
 
 

src/pretix/base/signals.py

@@ -646,7 +646,7 @@ allow_ticket_download = EventPluginSignal()
 Arguments: ``order``
 
 This signal is sent out to check if tickets for an order can be downloaded. If any receiver returns false,
-a download will not be offered.
+a download will not be offered. If a receiver returns a list of OrderPositions, only those will be downloadable.
 
 As with all event-plugin signals, the ``sender`` keyword argument will contain the event.
 """

src/pretix/base/ticketoutput.py

@@ -96,6 +96,9 @@ class BaseTicketOutput:
         """
         raise NotImplementedError()
 
+    def get_tickets_to_print(self, order):
+        return order.positions_with_tickets
+
     def generate_order(self, order: Order) -> Tuple[str, str, str]:
         """
         This method is the same as order() but should not generate one file per order position
@@ -116,7 +119,7 @@ class BaseTicketOutput:
         """
         with tempfile.TemporaryDirectory() as d:
             with ZipFile(os.path.join(d, 'tmp.zip'), 'w') as zipf:
-                for pos in order.positions_with_tickets:
+                for pos in self.get_tickets_to_print(order):
                     fname, __, content = self.generate(pos)
                     zipf.writestr('{}-{}{}'.format(
                         order.code, pos.positionid, os.path.splitext(fname)[1]

src/pretix/plugins/ticketoutputpdf/ticketoutput.py

@@ -115,7 +115,7 @@ class PdfTicketOutput(BaseTicketOutput):
     def generate_order(self, order: Order):
         merger = PdfWriter()
         with language(order.locale, self.event.settings.region):
-            for op in order.positions_with_tickets:
+            for op in self.get_tickets_to_print(order):
                 layout = override_layout.send_chained(
                     order.event, 'layout', orderposition=op, layout=self.layout_map.get(
                         (op.item_id, self.override_channel or order.sales_channel),

src/pretix/presale/templates/pretixpresale/event/fragment_cart.html

@@ -243,7 +243,7 @@
 
             {% if download %}
                 <div role="cell" class="download-desktop">
-                {% if line.generate_ticket %}
+                {% if line in tickets_with_download %}
                     {% for b in download_buttons %}
                         <form action="{% if position_page and line.addon_to %}{% eventurl event "presale:event.order.position.download" secret=line.addon_to.web_secret order=order.code output=b.identifier pid=line.pk position=line.addon_to.positionid %}{% elif position_page %}{% eventurl event "presale:event.order.position.download" secret=line.web_secret order=order.code output=b.identifier pid=line.pk position=line.positionid %}{% else %}{% eventurl event "presale:event.order.download" secret=order.secret order=order.code output=b.identifier position=line.pk %}{% endif %}"
                                 method="post" data-asynctask data-asynctask-download class="download-btn-form{% if b.javascript_required %} requirejs{% endif %}">

src/pretix/presale/templates/pretixpresale/event/fragment_downloads.html

@@ -35,10 +35,10 @@
             </p>
         </div>
     </div>
-{% elif can_download and download_buttons and order.count_positions %}
+{% elif can_download and download_buttons and order.count_positions and tickets_with_download  %}
     <div class="info-download">
         <h3 class="sr-only">{% trans "Ticket download" %}</h3>
-        {% if cart.positions|length > 1 and can_download_multi %} {# never True on ticket page #}
+        {% if tickets_with_download|length > 1 and can_download_multi %} {# never True on ticket page #}
             <div>
             {% for b in download_buttons %}
                 {% if b.multi %}

src/pretix/presale/views/order.py

@@ -80,9 +80,7 @@ from pretix.base.services.orders import (
 )
 from pretix.base.services.pricing import get_price
 from pretix.base.services.tickets import generate, invalidate_cache
-from pretix.base.signals import (
+from pretix.base.signals import order_modified, register_ticket_outputs
-    allow_ticket_download, order_modified, register_ticket_outputs,
-)
 from pretix.base.templatetags.money import money_filter
 from pretix.base.views.mixins import OrderQuestionsViewMixin
 from pretix.base.views.tasks import AsyncAction
@@ -175,16 +173,15 @@ class TicketPageMixin:
     def get_context_data(self, **kwargs):
         ctx = super().get_context_data(**kwargs)
 
+        positions_with_tickets = self.order.positions_with_tickets
+
         ctx['order'] = self.order
 
-        can_download = all([r for rr, r in allow_ticket_download.send(self.request.event, order=self.order)])
+        can_download = bool(positions_with_tickets)
         ctx['plugins_allow_ticket_download'] = can_download
         if self.request.event.settings.ticket_download_date:
             ctx['ticket_download_date'] = self.order.ticket_download_date
-        can_download = (
+        can_download = can_download and self.order.ticket_download_available
-            can_download and self.order.ticket_download_available and
-            list(self.order.positions_with_tickets)
-        )
         ctx['download_email_required'] = can_download and (
             self.request.event.settings.ticket_download_require_validated_email and
             self.order.sales_channel == 'web' and
@@ -192,6 +189,26 @@ class TicketPageMixin:
         )
         ctx['can_download'] = can_download and not ctx['download_email_required']
 
+        qs = self.context_query_set
+        if self.request.event.settings.show_checkin_number_user:
+            qs = qs.annotate(
+                checkin_count=Subquery(
+                    Checkin.objects.filter(
+                        successful=True,
+                        type=Checkin.TYPE_ENTRY,
+                        position_id=OuterRef('pk'),
+                        list__consider_tickets_used=True,
+                    ).order_by().values('position').annotate(c=Count('*')).values('c')
+                )
+            )
+        ctx['cart'] = self.get_cart(
+            answers=True, downloads=ctx['can_download'],
+            queryset=qs,
+            order=self.order
+        )
+
+        ctx['tickets_with_download'] = [p for p in ctx['cart']['positions'] if p in positions_with_tickets]
+
         ctx['download_buttons'] = self.download_buttons
 
         ctx['backend_user'] = (
@@ -200,25 +217,6 @@ class TicketPageMixin:
         )
         return ctx
 
-
-@method_decorator(xframe_options_exempt, 'dispatch')
-class OrderDetails(EventViewMixin, OrderDetailMixin, CartMixin, TicketPageMixin, TemplateView):
-    template_name = "pretixpresale/event/order.html"
-
-    def get(self, request, *args, **kwargs):
-        self.kwargs = kwargs
-        if not self.order:
-            raise Http404(_('Unknown order code or not authorized to access this order.'))
-        if self.order.status == Order.STATUS_PENDING:
-            payment_to_complete = self.order.payments.filter(state=OrderPayment.PAYMENT_STATE_CREATED, process_initiated=False).first()
-            if payment_to_complete:
-                return redirect(eventreverse(self.request.event, 'presale:event.order.pay.complete', kwargs={
-                    'order': self.order.code,
-                    'secret': self.order.secret,
-                    'payment': payment_to_complete.pk
-                }))
-        return super().get(request, *args, **kwargs)
-
     @cached_property
     def download_buttons(self):
         buttons = []
@@ -239,29 +237,31 @@ class OrderDetails(EventViewMixin, OrderDetailMixin, CartMixin, TicketPageMixin,
             })
         return buttons
 
+
+@method_decorator(xframe_options_exempt, 'dispatch')
+class OrderDetails(EventViewMixin, OrderDetailMixin, CartMixin, TicketPageMixin, TemplateView):
+    template_name = "pretixpresale/event/order.html"
+
+    def get(self, request, *args, **kwargs):
+        self.kwargs = kwargs
+        if not self.order:
+            raise Http404(_('Unknown order code or not authorized to access this order.'))
+        if self.order.status == Order.STATUS_PENDING:
+            payment_to_complete = self.order.payments.filter(state=OrderPayment.PAYMENT_STATE_CREATED, process_initiated=False).first()
+            if payment_to_complete:
+                return redirect(eventreverse(self.request.event, 'presale:event.order.pay.complete', kwargs={
+                    'order': self.order.code,
+                    'secret': self.order.secret,
+                    'payment': payment_to_complete.pk
+                }))
+        return super().get(request, *args, **kwargs)
+
     def get_context_data(self, **kwargs):
+        self.context_query_set = (
+            self.order.positions.prefetch_related('issued_gift_cards', 'owned_gift_cards').select_related('tax_rule')
+        )
         ctx = super().get_context_data(**kwargs)
 
-        qs = self.order.positions.prefetch_related('issued_gift_cards', 'owned_gift_cards').select_related('tax_rule')
-        if self.request.event.settings.show_checkin_number_user:
-            qs = qs.annotate(
-                checkin_count=Subquery(
-                    Checkin.objects.filter(
-                        successful=True,
-                        type=Checkin.TYPE_ENTRY,
-                        position_id=OuterRef('pk'),
-                        list__consider_tickets_used=True,
-                    ).order_by().values('position').annotate(c=Count('*')).values('c')
-                )
-            )
-
-        ctx['cart'] = self.get_cart(
-            answers=True,
-            downloads=ctx['can_download'],
-            queryset=qs,
-            order=self.order
-        )
-        ctx['tickets_with_download'] = [p for p in ctx['cart']['positions'] if p.generate_ticket]
         ctx['can_download_multi'] = any([b['multi'] for b in self.download_buttons]) and (
             [p.generate_ticket for p in ctx['cart']['positions']].count(True) > 1
         )
@@ -342,50 +342,13 @@ class OrderPositionDetails(EventViewMixin, OrderPositionDetailMixin, CartMixin,
             raise Http404(_('Unknown order code or not authorized to access this order.'))
         return super().get(request, *args, **kwargs)
 
-    @cached_property
-    def download_buttons(self):
-        buttons = []
-
-        responses = register_ticket_outputs.send(self.request.event)
-        for receiver, response in responses:
-            provider = response(self.request.event)
-            if not provider.is_enabled:
-                continue
-            buttons.append({
-                'text': provider.download_button_text or 'Download',
-                'icon': provider.download_button_icon or 'fa-download',
-                'identifier': provider.identifier,
-                'multi': provider.multi_download_enabled,
-                'multi_text': provider.multi_download_button_text or 'Download',
-                'long_text': provider.long_download_button_text or 'Download',
-                'javascript_required': provider.javascript_required
-            })
-        return buttons
-
     def get_context_data(self, **kwargs):
-        qs = self.order.positions.select_related('tax_rule').filter(
+        self.context_query_set = self.order.positions.select_related('tax_rule').filter(
             Q(pk=self.position.pk) | Q(addon_to__id=self.position.pk)
         )
-        if self.request.event.settings.show_checkin_number_user:
-            qs = qs.annotate(
-                checkin_count=Subquery(
-                    Checkin.objects.filter(
-                        successful=True,
-                        type=Checkin.TYPE_ENTRY,
-                        position_id=OuterRef('pk'),
-                        list__consider_tickets_used=True,
-                    ).order_by().values('position').annotate(c=Count('*')).values('c')
-                )
-            )
         ctx = super().get_context_data(**kwargs)
         ctx['can_download_multi'] = False
         ctx['position'] = self.position
-        ctx['cart'] = self.get_cart(
-            answers=True, downloads=ctx['can_download'],
-            queryset=qs,
-            order=self.order
-        )
-        ctx['tickets_with_download'] = [p for p in ctx['cart']['positions'] if p.generate_ticket]
         ctx['attendee_change_allowed'] = self.position.attendee_change_allowed
         return ctx
 
@@ -1048,8 +1011,6 @@ class OrderDownloadMixin:
 
     @cached_property
     def output(self):
-        if not all([r for rr, r in allow_ticket_download.send(self.request.event, order=self.order)]):
-            return None
         responses = register_ticket_outputs.send(self.request.event)
         for receiver, response in responses:
             provider = response(self.request.event)
@@ -1068,9 +1029,10 @@ class OrderDownloadMixin:
             return self.error(OrderError(_('You requested an invalid ticket output type.')))
         if not self.order or ('position' in kwargs and not self.order_position):
             raise Http404(_('Unknown order code or not authorized to access this order.'))
-        if not self.order.ticket_download_available:
+        positions = list(self.order.positions_with_tickets)
+        if not self.order.ticket_download_available or not positions:
             return self.error(OrderError(_('Ticket download is not (yet) enabled for this order.')))
-        if 'position' in kwargs and not self.order_position.generate_ticket:
+        if 'position' in kwargs and self.order_position not in positions:
             return self.error(OrderError(_('Ticket download is not enabled for this product.')))
 
         if (

src/tests/base/test_orders.py

@@ -55,6 +55,7 @@ from pretix.base.services.orders import (
     send_expiry_warnings,
 )
 from pretix.plugins.banktransfer.payment import BankTransfer
+from pretix.testutils.mock import mocker_context
 from pretix.testutils.scope import classscope
 
 
@@ -820,7 +821,7 @@ class DownloadReminderTests(TestCase):
             self.event = Event.objects.create(
                 organizer=self.o, name='Dummy', slug='dummy',
                 date_from=now() + timedelta(days=2),
-                plugins='pretix.plugins.banktransfer'
+                plugins='pretix.plugins.banktransfer,tests.testdummy'
             )
             self.order = Order.objects.create(
                 code='FOO', event=self.event, email='dummy@dummy.test',
@@ -853,6 +854,58 @@ class DownloadReminderTests(TestCase):
         send_download_reminders(sender=self.event)
         assert len(djmail.outbox) == 0
 
+    @classscope(attr='o')
+    def test_downloads_disabled_by_plugin(self):
+        with mocker_context() as mocker:
+            self.event.settings.mail_days_download_reminder = 2
+
+            from pretix.base.signals import allow_ticket_download
+            mocker.patch('pretix.base.signals.allow_ticket_download.send')
+            allow_ticket_download.send.return_value = [(None, [])]
+
+            send_download_reminders(sender=self.event)
+            assert len(djmail.outbox) == 0
+
+    @classscope(attr='o')
+    def test_downloads_all_allowed_by_plugin(self):
+        with mocker_context() as mocker:
+            self.event.settings.mail_days_download_reminder = 2
+            self.event.settings.mail_attach_tickets = True
+            self.event.settings.ticketoutput_testdummy__enabled = True
+
+            self.op2 = OrderPosition.objects.create(
+                order=self.order, item=self.ticket, variation=None,
+                price=Decimal("42.00"), attendee_name_parts={"full_name": "Mary"}, positionid=2
+            )
+
+            from pretix.base.signals import allow_ticket_download
+            mocker.patch('pretix.base.signals.allow_ticket_download.send')
+            allow_ticket_download.send.return_value = [(None, True)]
+
+            send_download_reminders(sender=self.event)
+            assert len(djmail.outbox) == 1
+            assert len(djmail.outbox[0].attachments) == 2
+
+    @classscope(attr='o')
+    def test_downloads_partially_disabled_by_plugin(self):
+        with mocker_context() as mocker:
+            self.event.settings.mail_days_download_reminder = 2
+            self.event.settings.mail_attach_tickets = True
+            self.event.settings.ticketoutput_testdummy__enabled = True
+
+            self.op2 = OrderPosition.objects.create(
+                order=self.order, item=self.ticket, variation=None,
+                price=Decimal("42.00"), attendee_name_parts={"full_name": "Mary"}, positionid=2
+            )
+
+            from pretix.base.signals import allow_ticket_download
+            mocker.patch('pretix.base.signals.allow_ticket_download.send')
+            allow_ticket_download.send.return_value = [(None, [self.op2])]
+
+            send_download_reminders(sender=self.event)
+            assert len(djmail.outbox) == 1
+            assert len(djmail.outbox[0].attachments) == 1
+
     @classscope(attr='o')
     def test_disabled(self):
         send_download_reminders(sender=self.event)

src/tests/testdummy/ticketoutput.py

@@ -33,3 +33,7 @@ class DummyTicketOutput(BaseTicketOutput):
 
     def generate(self, op):
         return 'test.txt', 'text/plain', str(op.order.id)
+
+    @property
+    def multi_download_enabled(self) -> bool:
+        return False