From 774f127c273a41e037bfa9bb612cb131cb3bb62c Mon Sep 17 00:00:00 2001 From: Kara Engelhardt Date: Wed, 15 Jul 2026 16:58:46 +0200 Subject: [PATCH] Address review comments --- src/pretix/base/models/orders.py | 46 +++++++++------------ src/pretix/plugins/sendmail/views.py | 35 ++++++++-------- src/tests/plugins/sendmail/test_sendmail.py | 32 ++++++++++++++ 3 files changed, 70 insertions(+), 43 deletions(-) diff --git a/src/pretix/base/models/orders.py b/src/pretix/base/models/orders.py index 26472be687..bcfc4e1e31 100644 --- a/src/pretix/base/models/orders.py +++ b/src/pretix/base/models/orders.py @@ -155,9 +155,16 @@ class OrderQuerySet(models.QuerySet): filter |= Q(status__in=[Order.STATUS_PENDING, Order.STATUS_EXPIRED]) if 'pv' in status: filter |= Q(status=Order.STATUS_PAID) | Q(status=Order.STATUS_PENDING, valid_if_pending=True) - for s in ('p', 'n', 'e', 'c'): + for s in ('n', 'e', 'c'): if s in status: filter |= Q(status=s) + if 'p' in status: + has_pc = OrderPosition.objects.filter( + order=OuterRef('pk') + ) + filter |= ( + Q(Exists(has_pc), status=Order.STATUS_PAID) + ) if 'overpaid' in status: qs = Order.annotate_overpayments(qs, refunds=False, results=True, sums=False) filter |= Q(is_overpaid=True) @@ -215,6 +222,12 @@ class OrderQuerySet(models.QuerySet): require_approval=True ) if 'na' in status: + filter |= Q( + status=Order.STATUS_PENDING, + require_approval=False, + valid_if_pending=False + ) + if 'na_all' in status: filter |= Q( status=Order.STATUS_PENDING, require_approval=False @@ -346,9 +359,10 @@ class Order(LockModel, LoggedModel): ('underpaid', _('Underpaid (but confirmed)')), ('pendingpaid', _('Pending (but fully paid)')), ('pendingnopayment', _('Pending (but no current payment)')), + ('na', _('Payment pending (except unapproved or already confirmed)')), )), (_('Approval process'), ( - ('na', _('Approved, payment pending')), + ('na_all', _('Approved, payment pending')), ('pa', _('Approval pending')), )), (_('Follow-up date'), ( @@ -358,30 +372,10 @@ class Order(LockModel, LoggedModel): ('testmode', _('Test mode')), ) - STATUS_FILTER_OPTIONS = ( - (STATUS_PAID, _('Paid (or canceled with paid fee)')), - (STATUS_PAID + 'v', _('Paid or confirmed')), - ('valid_if_confirmed', _('Pending but already confirmed')), - (STATUS_PENDING, _('Pending')), - (STATUS_PENDING + STATUS_PAID, _('Pending or paid')), - (STATUS_CANCELED, _('Canceled (fully)')), - ('cp', _('Canceled (fully or with paid fee)')), - ('cany', _('Canceled (at least one position)')), - ('rc', _('Cancellation requested')), - ('cni', _('Fully canceled but invoice not canceled')), - (STATUS_EXPIRED, _('Expired')), - (STATUS_PENDING + STATUS_EXPIRED, _('Pending or expired')), - ('o', _('Pending (overdue)')), - ('overpaid', _('Overpaid')), - ('partially_paid', _('Partially paid')), - ('underpaid', _('Underpaid (but confirmed)')), - ('pendingpaid', _('Pending (but fully paid)')), - ('pendingnopayment', _('Pending (but no current payment)')), - ('na', _('Approved, payment pending')), - ('pa', _('Approval pending')), - ('custom_followup_at', _('Follow-up configured')), - ('custom_followup_due', _('Follow-up due')), - ('testmode', _('Test mode')), + STATUS_FILTER_OPTIONS = tuple( + c + for cs in STATUS_FILTERS + for c in (cs[1] if isinstance(cs[1], tuple) else [cs]) ) code = models.CharField( diff --git a/src/pretix/plugins/sendmail/views.py b/src/pretix/plugins/sendmail/views.py index bef6120c66..1dbfa65727 100644 --- a/src/pretix/plugins/sendmail/views.py +++ b/src/pretix/plugins/sendmail/views.py @@ -338,8 +338,13 @@ class OrderSendView(BaseSenderView): return initial def get_object_queryset(self, form): + # Filtering here is a bit intricate, as some filters relate to order positions, while others relate to the order itself qs = Order.objects.filter(event=self.request.event) orders = qs.filter_by_status(qs, form.cleaned_data['sendto']) + if form.cleaned_data.get('created_from'): + orders = orders.filter(datetime_gte=form.cleaned_data.get('created_to')) + if form.cleaned_data.get('created_to'): + orders = orders.filter(datetime__lt=form.cleaned_data.get('created_to')) opq = OrderPosition.objects.filter( Q(item_id__in=[i.pk for i in form.cleaned_data.get('items')]) | Q(Exists( @@ -353,7 +358,7 @@ class OrderSendView(BaseSenderView): ) if form.cleaned_data.get('filter_checkins'): - ql = [] + ci_filter = Q(pk__in=[]) # return nothing if form.cleaned_data.get('not_checked_in'): opq = opq.alias( @@ -365,7 +370,7 @@ class OrderSendView(BaseSenderView): ) ) ) - ql.append(Q(any_checkins=False)) + ci_filter |= Q(any_checkins=False) if form.cleaned_data.get('checkin_lists'): opq = opq.alias( matching_checkins=Exists( @@ -376,13 +381,8 @@ class OrderSendView(BaseSenderView): ) ) ) - ql.append(Q(matching_checkins=True)) - if len(ql) == 2: - opq = opq.filter(ql[0] | ql[1]) - elif ql: - opq = opq.filter(ql[0]) - else: - opq = opq.none() + ci_filter |= Q(matching_checkins=True) + opq.filter(ci_filter) if form.cleaned_data.get('subevent'): opq = opq.filter(subevent=form.cleaned_data.get('subevent')) @@ -390,15 +390,16 @@ class OrderSendView(BaseSenderView): opq = opq.filter(subevent__date_from__gte=form.cleaned_data.get('subevents_from')) if form.cleaned_data.get('subevents_to'): opq = opq.filter(subevent__date_from__lt=form.cleaned_data.get('subevents_to')) - if form.cleaned_data.get('created_from'): - opq = opq.filter(order__datetime__gte=form.cleaned_data.get('created_from')) - if form.cleaned_data.get('created_to'): - opq = opq.filter(order__datetime__lt=form.cleaned_data.get('created_to')) - orders_without_positions = Order.objects.filter(~Exists(OrderPosition.objects.filter(canceled=False, order_id=OuterRef('pk')))) - # pk__in turns out to be faster than Exists(subquery) in many cases since we often filter on a large subset - # of orderpositions - return orders.filter(Q(pk__in=opq.values_list('order_id')) | Q(pk__in=orders_without_positions)) + if form.cleaned_data.get('recipients') in ['orders', 'both']: + orders_without_positions = Order.objects.filter(~Exists(OrderPosition.objects.filter(canceled=False, order_id=OuterRef('pk')))) + # pk__in turns out to be faster than Exists(subquery) in many cases since we often filter on a large subset + # of orderpositions + return orders.filter(Q(pk__in=opq.values_list('order_id')) | Q(pk__in=orders_without_positions)) + else: + # pk__in turns out to be faster than Exists(subquery) in many cases since we often filter on a large subset + # of orderpositions + return orders.filter(pk__in=opq.values_list('order_id')) def describe_match_size(self, cnt): return ngettext( diff --git a/src/tests/plugins/sendmail/test_sendmail.py b/src/tests/plugins/sendmail/test_sendmail.py index 9b0e781a00..d990fb8c0e 100644 --- a/src/tests/plugins/sendmail/test_sendmail.py +++ b/src/tests/plugins/sendmail/test_sendmail.py @@ -722,6 +722,22 @@ def test_sendmail_canceled(logged_in_client, sendmail_url, event, item): assert len(djmail.outbox) == 1 assert set(tuple(x.to) for x in djmail.outbox) == {(o1_p1.attendee_email,), } + djmail.outbox = [] + response = logged_in_client.post(sendmail_url + 'orders/', + {'sendto': 'cp', + 'action': 'send', + 'recipients': 'both', + 'items': item.pk, + 'subject_0': 'Test subject', + 'message_0': 'This is a test file for sending mails.', + }, + follow=True) + + assert 'alert-success' in response.rendered_content + # o2 has no active attendees + assert len(djmail.outbox) == 3 + assert set(tuple(x.to) for x in djmail.outbox) == {(o1.email,), (o2.email,), (o1_p1.attendee_email,), } + djmail.outbox = [] response = logged_in_client.post(sendmail_url + 'orders/', {'sendto': 'cany', @@ -736,3 +752,19 @@ def test_sendmail_canceled(logged_in_client, sendmail_url, event, item): assert 'alert-success' in response.rendered_content assert len(djmail.outbox) == 2 assert set(tuple(x.to) for x in djmail.outbox) == {(o1_p1.attendee_email,), (o3_p1.attendee_email,)} + + # Order cancelled with fee, should not receive mails for paid + djmail.outbox = [] + response = logged_in_client.post(sendmail_url + 'orders/', + {'sendto': 'p', + 'action': 'send', + 'recipients': 'orders', + 'items': item.pk, + 'subject_0': 'Test subject', + 'message_0': 'This is a test file for sending mails.', + }, + follow=True) + + assert 'alert-success' in response.rendered_content + assert len(djmail.outbox) == 1 + assert set(tuple(x.to) for x in djmail.outbox) == {(o3.email,)}