Address review comments

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(

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(

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,)}