From fc4c6444cb77c7e0e8d83878ef2e9f69351cd213 Mon Sep 17 00:00:00 2001 From: pajowu Date: Mon, 21 Oct 2019 14:06:11 +0200 Subject: [PATCH] Sendmail plugin: Allow filtering for check-ins (#1382) * Allow filtering mass-emails for Checkin Lists * Allow filtering mass emails for not checked in * Fix email filtering logic issue * Use Select2 for checkin lists selection * sendmail plugin: Make checkin list filtering optional * Remove unused constant * Re-size panel to only fit the right column * Revert incorrect JavaScript change * Change semantics of not_checked_in * Introduce a subquery to filter on position properties --- src/pretix/plugins/sendmail/forms.py | 26 +++- src/pretix/plugins/sendmail/tasks.py | 12 +- .../pretixplugins/sendmail/history.html | 8 + .../pretixplugins/sendmail/send_form.html | 17 +++ src/pretix/plugins/sendmail/views.py | 46 +++++- src/pretix/static/pretixcontrol/js/ui/main.js | 4 +- src/tests/plugins/test_sendmail.py | 140 ++++++++++++++++-- 7 files changed, 227 insertions(+), 26 deletions(-) diff --git a/src/pretix/plugins/sendmail/forms.py b/src/pretix/plugins/sendmail/forms.py index f70a5b9b2..fb7a4be86 100644 --- a/src/pretix/plugins/sendmail/forms.py +++ b/src/pretix/plugins/sendmail/forms.py @@ -1,12 +1,13 @@ from django import forms from django.urls import reverse from django.utils.translation import pgettext_lazy, ugettext_lazy as _ +from django_scopes.forms import SafeModelMultipleChoiceField from i18nfield.forms import I18nFormField, I18nTextarea, I18nTextInput from pretix.base.email import get_available_placeholders from pretix.base.forms import PlaceholderValidator -from pretix.base.models import Item, Order, SubEvent -from pretix.control.forms.widgets import Select2 +from pretix.base.models import CheckinList, Item, Order, SubEvent +from pretix.control.forms.widgets import Select2, Select2Multiple class MailForm(forms.Form): @@ -27,6 +28,12 @@ class MailForm(forms.Form): required=True, queryset=Item.objects.none() ) + filter_checkins = forms.BooleanField( + label=_('Filter check-in status'), + required=False + ) + checkin_lists = SafeModelMultipleChoiceField(queryset=CheckinList.objects.none(), required=False) # overridden later + not_checked_in = forms.BooleanField(label=_("Send to customers not checked in"), required=False) subevent = forms.ModelChoiceField( SubEvent.objects.none(), label=_('Only send to customers of'), @@ -96,6 +103,21 @@ class MailForm(forms.Form): if not self.initial.get('items'): self.initial['items'] = event.items.all() + self.fields['checkin_lists'].queryset = event.checkin_lists.all() + self.fields['checkin_lists'].widget = Select2Multiple( + attrs={ + 'data-model-select2': 'generic', + 'data-select2-url': reverse('control:event.orders.checkinlists.select2', kwargs={ + 'event': event.slug, + 'organizer': event.organizer.slug, + }), + 'data-placeholder': _('Send to customers checked in on list'), + 'data-inverse-dependency': '#id_not_checked_in' + } + ) + self.fields['checkin_lists'].widget.choices = self.fields['checkin_lists'].choices + self.fields['checkin_lists'].label = _('Send to customers checked in on list') + if event.has_subevents: self.fields['subevent'].queryset = event.subevents.all() self.fields['subevent'].widget = Select2( diff --git a/src/pretix/plugins/sendmail/tasks.py b/src/pretix/plugins/sendmail/tasks.py index bb5e7e3e5..19a03345f 100644 --- a/src/pretix/plugins/sendmail/tasks.py +++ b/src/pretix/plugins/sendmail/tasks.py @@ -9,7 +9,8 @@ from pretix.celery_app import app @app.task(base=ProfiledEventTask) -def send_mails(event: Event, user: int, subject: dict, message: dict, orders: list, items: list, recipients: str) -> None: +def send_mails(event: Event, user: int, subject: dict, message: dict, orders: list, items: list, + recipients: str, filter_checkins: bool, not_checked_in: bool, checkin_lists: list) -> None: failures = [] user = User.objects.get(pk=user) if user else None orders = Order.objects.filter(pk__in=orders, event=event) @@ -32,6 +33,15 @@ def send_mails(event: Event, user: int, subject: dict, message: dict, orders: li if p.item_id not in items and not any(a.item_id in items for a in p.addons.all()): continue + if filter_checkins: + checkins = list(p.checkins.all()) + allowed = ( + (not_checked_in and not checkins) + or (any(c.list_id in checkin_lists for c in checkins)) + ) + if not allowed: + continue + if not p.attendee_email: if recipients == 'attendees': send_to_order = True diff --git a/src/pretix/plugins/sendmail/templates/pretixplugins/sendmail/history.html b/src/pretix/plugins/sendmail/templates/pretixplugins/sendmail/history.html index 6f21eab81..6c132dba5 100644 --- a/src/pretix/plugins/sendmail/templates/pretixplugins/sendmail/history.html +++ b/src/pretix/plugins/sendmail/templates/pretixplugins/sendmail/history.html @@ -23,6 +23,14 @@ {% if log.pdata.items %}
{{ log.pdata.items|join:", " }} {% endif %} + {% if log.pdata.filter_checkins %} + {% if log.pdata.not_checked_in %} +
{% trans "All customers not checked in" %} + {% endif %} + {% if log.pdata.checkin_lists %} +
{{ log.pdata.checkin_lists|join:", " }} + {% endif %} + {% endif %} {% if log.pdata.subevent_obj %}
{{ log.pdata.subevent_obj }} {% endif %} diff --git a/src/pretix/plugins/sendmail/templates/pretixplugins/sendmail/send_form.html b/src/pretix/plugins/sendmail/templates/pretixplugins/sendmail/send_form.html index f6f43ac95..76aa4093d 100644 --- a/src/pretix/plugins/sendmail/templates/pretixplugins/sendmail/send_form.html +++ b/src/pretix/plugins/sendmail/templates/pretixplugins/sendmail/send_form.html @@ -13,6 +13,23 @@ {% bootstrap_field form.subevent layout='horizontal' %} {% endif %} {% bootstrap_field form.items layout='horizontal' %} +
+
+
+
+
+ +
+
+ {% bootstrap_field form.not_checked_in layout='horizontal' %} + {% bootstrap_field form.checkin_lists layout='horizontal' %} +
+
+
+
+
{% bootstrap_field form.subject layout='horizontal' %} {% bootstrap_field form.message layout='horizontal' %} {% if request.method == "POST" %} diff --git a/src/pretix/plugins/sendmail/views.py b/src/pretix/plugins/sendmail/views.py index 1f7b1e552..ded0c3e72 100644 --- a/src/pretix/plugins/sendmail/views.py +++ b/src/pretix/plugins/sendmail/views.py @@ -2,7 +2,7 @@ import logging import bleach from django.contrib import messages -from django.db.models import Q +from django.db.models import Exists, OuterRef, Q from django.http import Http404 from django.shortcuts import redirect from django.utils.timezone import now @@ -11,7 +11,7 @@ from django.views.generic import FormView, ListView from pretix.base.email import get_available_placeholders from pretix.base.i18n import LazyI18nString, language -from pretix.base.models import LogEntry, Order +from pretix.base.models import LogEntry, Order, OrderPosition from pretix.base.models.event import SubEvent from pretix.base.services.mail import TolerantDict from pretix.base.templatetags.rich_text import markdown_compile_email @@ -53,6 +53,9 @@ class SenderView(EventPermissionRequiredMixin, FormView): kwargs['initial']['items'] = self.request.event.items.filter( id=logentry.parsed_data['item']['id'] ) + if 'checkin_lists' in logentry.parsed_data: + kwargs['initial']['checkin_lists'] = logentry.parsed_data['checkin_lists'] + kwargs['initial']['filter_checkins'] = logentry.parsed_data.get('filter_checkins', False) if logentry.parsed_data.get('subevent'): try: kwargs['initial']['subevent'] = self.request.event.subevents.get( @@ -74,12 +77,30 @@ class SenderView(EventPermissionRequiredMixin, FormView): if 'overdue' in form.cleaned_data['sendto']: statusq |= Q(status=Order.STATUS_PENDING, expires__lt=now()) orders = qs.filter(statusq) - orders = orders.filter(all_positions__item_id__in=[i.pk for i in form.cleaned_data.get('items')], - all_positions__canceled=False) + + opq = OrderPosition.objects.filter( + order=OuterRef('pk'), + canceled=False, + item_id__in=[i.pk for i in form.cleaned_data.get('items')], + ) + + if form.cleaned_data.get('filter_checkins'): + ql = [] + if form.cleaned_data.get('not_checked_in'): + ql.append(Q(checkins__list_id=None)) + if form.cleaned_data.get('checkin_lists'): + ql.append(Q( + checkins__list_id__in=[i.pk for i in form.cleaned_data.get('checkin_lists', [])], + )) + if len(ql) == 2: + opq = opq.filter(ql[0] | ql[1]) + else: + opq = opq.filter(ql[0]) + if form.cleaned_data.get('subevent'): - orders = orders.filter(all_positions__subevent__in=(form.cleaned_data.get('subevent'),), - all_positions__canceled=False) - orders = orders.distinct() + opq = opq.filter(subevent=form.cleaned_data.get('subevent')) + + orders = orders.annotate(match_pos=Exists(opq)).filter(match_pos=True).distinct() self.output = {} if not orders: @@ -118,7 +139,10 @@ class SenderView(EventPermissionRequiredMixin, FormView): 'subject': form.cleaned_data['subject'].data, 'message': form.cleaned_data['message'].data, 'orders': [o.pk for o in orders], - 'items': [i.pk for i in form.cleaned_data.get('items')] + 'items': [i.pk for i in form.cleaned_data.get('items')], + 'not_checked_in': form.cleaned_data.get('not_checked_in'), + 'checkin_lists': [i.pk for i in form.cleaned_data.get('checkin_lists')], + 'filter_checkins': form.cleaned_data.get('filter_checkins'), } ) self.request.event.log_action('pretix.plugins.sendmail.sent', @@ -159,6 +183,9 @@ class EmailHistoryView(EventPermissionRequiredMixin, ListView): itemcache = { i.pk: str(i) for i in self.request.event.items.all() } + checkin_list_cache = { + i.pk: str(i) for i in self.request.event.checkin_lists.all() + } status = dict(Order.STATUS_CHOICE) status['overdue'] = _('pending with payment overdue') status['r'] = status['c'] @@ -176,6 +203,9 @@ class EmailHistoryView(EventPermissionRequiredMixin, ListView): log.pdata['items'] = [ itemcache[i['id']] for i in log.pdata.get('items', []) ] + log.pdata['checkin_lists'] = [ + checkin_list_cache[i] for i in log.pdata.get('checkin_lists', []) if i in checkin_list_cache + ] if log.pdata.get('subevent'): try: log.pdata['subevent_obj'] = self.request.event.subevents.get(pk=log.pdata['subevent']['id']) diff --git a/src/pretix/static/pretixcontrol/js/ui/main.js b/src/pretix/static/pretixcontrol/js/ui/main.js index 7ab1bc2dc..38aebf02e 100644 --- a/src/pretix/static/pretixcontrol/js/ui/main.js +++ b/src/pretix/static/pretixcontrol/js/ui/main.js @@ -256,7 +256,7 @@ var form_handlers = function (el) { dependency.on("change", update); }); - el.find("input[data-inverse-dependency]").each(function () { + el.find("select[data-inverse-dependency], input[data-inverse-dependency]").each(function () { var dependency = $(this).attr("data-inverse-dependency"); if (dependency.substr(0, 1) === '<') { dependency = $(this).closest("form, .form-horizontal").find(dependency.substr(1)); @@ -273,7 +273,7 @@ var form_handlers = function (el) { dependency.on("change", update); }); - $("div[data-display-dependency], input[data-display-dependency]").each(function () { + el.find("div[data-display-dependency], input[data-display-dependency]").each(function () { var dependent = $(this), dependency = $($(this).attr("data-display-dependency")), update = function (ev) { diff --git a/src/tests/plugins/test_sendmail.py b/src/tests/plugins/test_sendmail.py index 0c7c984b2..d2255cf14 100644 --- a/src/tests/plugins/test_sendmail.py +++ b/src/tests/plugins/test_sendmail.py @@ -6,7 +6,7 @@ from django.utils.timezone import now from django_scopes import scopes_disabled from pretix.base.models import ( - Event, Item, Order, OrderPosition, Organizer, Team, User, + Checkin, Event, Item, Order, OrderPosition, Organizer, Team, User, ) @@ -28,6 +28,12 @@ def item(event): return Item.objects.create(name='Test item', event=event, default_price=13) +@pytest.fixture +def checkin_list(event): + """Returns an checkin list instance""" + return event.checkin_lists.create(name="Test Checkinlist", all_products=True) + + @pytest.fixture def order(item): """Returns an order instance""" @@ -78,7 +84,7 @@ def test_sendmail_simple_case(logged_in_client, sendmail_url, event, order, pos) 'recipients': 'orders', 'items': pos.item_id, 'subject_0': 'Test subject', - 'message_0': 'This is a test file for sending mails.' + 'message_0': 'This is a test file for sending mails.', }, follow=True) assert response.status_code == 200 @@ -104,7 +110,7 @@ def test_sendmail_email_not_sent_if_order_not_match(logged_in_client, sendmail_u 'recipients': 'orders', 'items': pos.item_id, 'subject_0': 'Test subject', - 'message_0': 'This is a test file for sending mails.' + 'message_0': 'This is a test file for sending mails.', }, follow=True) assert 'alert-danger' in response.rendered_content @@ -121,7 +127,7 @@ def test_sendmail_preview(logged_in_client, sendmail_url, event, order, pos): 'items': pos.item_id, 'subject_0': 'Test subject', 'message_0': 'This is a test file for sending mails.', - 'action': 'preview' + 'action': 'preview', }, follow=True) assert response.status_code == 200 @@ -167,7 +173,7 @@ def test_sendmail_multi_locales(logged_in_client, sendmail_url, event, item): 'subject_0': 'Test subject', 'message_0': 'Test message', 'subject_1': 'Benutzer', - 'message_1': 'Test nachricht' + 'message_1': 'Test nachricht', }, follow=True) assert response.status_code == 200 @@ -204,7 +210,7 @@ def test_sendmail_subevents(logged_in_client, sendmail_url, event, order, pos): 'items': pos.item_id, 'subject_0': 'Test subject', 'message_0': 'This is a test file for sending mails.', - 'subevent': se1.pk + 'subevent': se1.pk, }, follow=True) assert response.status_code == 200 @@ -218,7 +224,7 @@ def test_sendmail_subevents(logged_in_client, sendmail_url, event, order, pos): 'items': pos.item_id, 'subject_0': 'Test subject', 'message_0': 'This is a test file for sending mails.', - 'subevent': se2.pk + 'subevent': se2.pk, }, follow=True) assert len(djmail.outbox) == 0 @@ -239,7 +245,7 @@ def test_sendmail_placeholder(logged_in_client, sendmail_url, event, order, pos) 'items': pos.item_id, 'subject_0': '{code} Test subject', 'message_0': 'This is a test file for sending mails.', - 'action': 'preview' + 'action': 'preview', }, follow=True) @@ -262,7 +268,7 @@ def test_sendmail_attendee_mails(logged_in_client, sendmail_url, event, order, p 'recipients': 'attendees', 'items': pos.item_id, 'subject_0': 'Test subject', - 'message_0': 'This is a test file for sending mails.' + 'message_0': 'This is a test file for sending mails.', }, follow=True) assert response.status_code == 200 @@ -286,7 +292,7 @@ def test_sendmail_both_mails(logged_in_client, sendmail_url, event, order, pos): 'recipients': 'both', 'items': pos.item_id, 'subject_0': 'Test subject', - 'message_0': 'This is a test file for sending mails.' + 'message_0': 'This is a test file for sending mails.', }, follow=True) assert response.status_code == 200 @@ -313,7 +319,7 @@ def test_sendmail_both_but_same_address(logged_in_client, sendmail_url, event, o 'recipients': 'both', 'items': pos.item_id, 'subject_0': 'Test subject', - 'message_0': 'This is a test file for sending mails.' + 'message_0': 'This is a test file for sending mails.', }, follow=True) assert response.status_code == 200 @@ -337,7 +343,7 @@ def test_sendmail_attendee_fallback(logged_in_client, sendmail_url, event, order 'recipients': 'attendees', 'items': pos.item_id, 'subject_0': 'Test subject', - 'message_0': 'This is a test file for sending mails.' + 'message_0': 'This is a test file for sending mails.', }, follow=True) assert response.status_code == 200 @@ -360,11 +366,41 @@ def test_sendmail_attendee_product_filter(logged_in_client, sendmail_url, event, item=i2, price=0, attendee_email='attendee2@dummy.test' ) + djmail.outbox = [] + response = logged_in_client.post(sendmail_url, + {'sendto': 'n', + 'recipients': 'attendees', + 'items': i2.pk, + 'subject_0': 'Test subject', + 'message_0': 'This is a test file for sending mails.', + }, + follow=True) + assert response.status_code == 200 + assert 'alert-success' in response.rendered_content + assert len(djmail.outbox) == 1 + assert djmail.outbox[0].to == ['attendee2@dummy.test'] + assert '/ticket/' in djmail.outbox[0].body + assert '/order/' not in djmail.outbox[0].body + + +@pytest.mark.django_db +def test_sendmail_attendee_checkin_filter(logged_in_client, sendmail_url, event, order, checkin_list, item, pos): + event.settings.attendee_emails_asked = True + with scopes_disabled(): + chkl2 = event.checkin_lists.create(name="Test Checkinlist 2", all_products=True) + p = pos + p.attendee_email = 'attendee1@dummy.test' + p.save() + pos2 = order.positions.create(item=item, price=0, attendee_email='attendee2@dummy.test') + Checkin.objects.create(position=pos2, list=chkl2) + djmail.outbox = [] response = logged_in_client.post(sendmail_url, {'sendto': 'n', 'recipients': 'attendees', - 'items': i2.pk, + 'items': pos2.item_id, + 'filter_checkins': 'on', + 'checkin_lists': [chkl2.id], 'subject_0': 'Test subject', 'message_0': 'This is a test file for sending mails.' }, @@ -375,3 +411,81 @@ def test_sendmail_attendee_product_filter(logged_in_client, sendmail_url, event, assert djmail.outbox[0].to == ['attendee2@dummy.test'] assert '/ticket/' in djmail.outbox[0].body assert '/order/' not in djmail.outbox[0].body + + djmail.outbox = [] + response = logged_in_client.post(sendmail_url, + {'sendto': 'n', + 'recipients': 'attendees', + 'items': pos2.item_id, + 'subject_0': 'Test subject', + 'message_0': 'This is a test file for sending mails.', + 'filter_checkins': 'on', + 'not_checked_in': 'on', + }, + follow=True) + assert response.status_code == 200 + assert 'alert-success' in response.rendered_content + assert len(djmail.outbox) == 1 + assert djmail.outbox[0].to == ['attendee1@dummy.test'] + assert '/ticket/' in djmail.outbox[0].body + assert '/order/' not in djmail.outbox[0].body + + djmail.outbox = [] + response = logged_in_client.post(sendmail_url, + {'sendto': 'n', + 'recipients': 'attendees', + 'items': pos2.item_id, + 'subject_0': 'Test subject', + 'message_0': 'This is a test file for sending mails.', + 'filter_checkins': 'on', + 'checkin_lists': [chkl2.id], + 'not_checked_in': 'on', + }, + follow=True) + assert response.status_code == 200 + assert 'alert-success' in response.rendered_content + assert len(djmail.outbox) == 2 + assert djmail.outbox[0].to == ['attendee1@dummy.test'] + assert djmail.outbox[1].to == ['attendee2@dummy.test'] + + # Test that filtering is ignored if filter_checkins is not set + djmail.outbox = [] + response = logged_in_client.post(sendmail_url, + {'sendto': 'n', + 'recipients': 'attendees', + 'items': pos2.item_id, + 'subject_0': 'Test subject', + 'message_0': 'This is a test file for sending mails.', + 'not_checked_in': 'on', + }, + follow=True) + assert response.status_code == 200 + assert 'alert-success' in response.rendered_content + assert len(djmail.outbox) == 2 + assert '/ticket/' in djmail.outbox[0].body + assert '/order/' not in djmail.outbox[0].body + assert '/ticket/' in djmail.outbox[1].body + assert '/order/' not in djmail.outbox[1].body + to_emails = set(*zip(*[mail.to for mail in djmail.outbox])) + assert to_emails == {'attendee1@dummy.test', 'attendee2@dummy.test'} + + # Test that filtering is ignored if filter_checkins is not set + djmail.outbox = [] + response = logged_in_client.post(sendmail_url, + {'sendto': 'n', + 'recipients': 'attendees', + 'items': pos2.item_id, + 'subject_0': 'Test subject', + 'message_0': 'This is a test file for sending mails.', + 'checkin_lists': [chkl2.id], + }, + follow=True) + assert response.status_code == 200 + assert 'alert-success' in response.rendered_content + assert len(djmail.outbox) == 2 + assert '/ticket/' in djmail.outbox[0].body + assert '/order/' not in djmail.outbox[0].body + assert '/ticket/' in djmail.outbox[1].body + assert '/order/' not in djmail.outbox[1].body + to_emails = set(*zip(*[mail.to for mail in djmail.outbox])) + assert to_emails == {'attendee1@dummy.test', 'attendee2@dummy.test'}