diff --git a/src/pretix/base/models/orders.py b/src/pretix/base/models/orders.py index 0902b807b5..724facd2c8 100644 --- a/src/pretix/base/models/orders.py +++ b/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 @@ -1134,12 +1134,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'): - if not op.generate_ticket: - continue - yield op + signal_response = allow_ticket_download.send(self.event, order=self) + if all([r is True for rr, r in signal_response]): + return self.positions_with_tickets_ignoring_plugins + 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): diff --git a/src/pretix/base/services/orders.py b/src/pretix/base/services/orders.py index 7791bb933d..9c192ac3bb 100644 --- a/src/pretix/base/services/orders.py +++ b/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_denied, order_expired, order_fee_calculation, order_paid, - order_placed, order_split, order_valid_if_pending, periodic_task, - validate_order, + order_approved, order_canceled, order_changed, order_denied, order_expired, + order_fee_calculation, order_paid, order_placed, order_split, + order_valid_if_pending, periodic_task, 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 = list(o.positions_with_tickets) + if not 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(): - if not p.generate_ticket: - continue - + for p in positions: if p.subevent_id: reminder_date = (p.subevent.date_from - timedelta(days=days)).replace( hour=0, minute=0, second=0, microsecond=0 diff --git a/src/pretix/base/services/tickets.py b/src/pretix/base/services/tickets.py index a372602f9b..b667c36112 100644 --- a/src/pretix/base/services/tickets.py +++ b/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)]) - if not can_download: + positions = list(order.positions_with_tickets) + 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 diff --git a/src/pretix/base/signals.py b/src/pretix/base/signals.py index bbb933bb1b..2a09b8e480 100644 --- a/src/pretix/base/signals.py +++ b/src/pretix/base/signals.py @@ -652,7 +652,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. """ diff --git a/src/pretix/base/ticketoutput.py b/src/pretix/base/ticketoutput.py index 5a8d4c618f..cd86ab9487 100644 --- a/src/pretix/base/ticketoutput.py +++ b/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] diff --git a/src/pretix/plugins/ticketoutputpdf/ticketoutput.py b/src/pretix/plugins/ticketoutputpdf/ticketoutput.py index 157a57c0f9..66e9261651 100644 --- a/src/pretix/plugins/ticketoutputpdf/ticketoutput.py +++ b/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), diff --git a/src/pretix/presale/templates/pretixpresale/event/fragment_cart.html b/src/pretix/presale/templates/pretixpresale/event/fragment_cart.html index 24210626a2..4f39027602 100644 --- a/src/pretix/presale/templates/pretixpresale/event/fragment_cart.html +++ b/src/pretix/presale/templates/pretixpresale/event/fragment_cart.html @@ -243,7 +243,7 @@ {% if download %}
- {% if line.generate_ticket %} + {% if line in tickets_with_download %} {% for b in download_buttons %}
diff --git a/src/pretix/presale/templates/pretixpresale/event/fragment_downloads.html b/src/pretix/presale/templates/pretixpresale/event/fragment_downloads.html index 8c138016e8..42d402975d 100644 --- a/src/pretix/presale/templates/pretixpresale/event/fragment_downloads.html +++ b/src/pretix/presale/templates/pretixpresale/event/fragment_downloads.html @@ -35,10 +35,10 @@

-{% elif can_download and download_buttons and order.count_positions %} +{% elif can_download and download_buttons and order.count_positions and tickets_with_download %}

{% trans "Ticket download" %}

- {% 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 #}
{% for b in download_buttons %} {% if b.multi %} diff --git a/src/pretix/presale/views/order.py b/src/pretix/presale/views/order.py index f6b220457d..d48378d61f 100644 --- a/src/pretix/presale/views/order.py +++ b/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 ( - allow_ticket_download, order_modified, register_ticket_outputs, -) +from pretix.base.signals import 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,14 @@ class TicketPageMixin: def get_context_data(self, **kwargs): ctx = super().get_context_data(**kwargs) + positions_with_tickets = list(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 and self.order.ticket_download_available and - list(self.order.positions_with_tickets) - ) + can_download = can_download and self.order.ticket_download_available 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 +188,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 +216,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 +236,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 +341,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 +1010,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 +1028,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 ( diff --git a/src/tests/base/test_orders.py b/src/tests/base/test_orders.py index d0f9ba4f9b..93565535ca 100644 --- a/src/tests/base/test_orders.py +++ b/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) diff --git a/src/tests/testdummy/ticketoutput.py b/src/tests/testdummy/ticketoutput.py index 12c496d11f..df02557ad3 100644 --- a/src/tests/testdummy/ticketoutput.py +++ b/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