diff --git a/src/pretix/base/migrations/0297_outgoingmail.py b/src/pretix/base/migrations/0297_outgoingmail.py index 6f52877391..7d80a92d67 100644 --- a/src/pretix/base/migrations/0297_outgoingmail.py +++ b/src/pretix/base/migrations/0297_outgoingmail.py @@ -40,6 +40,7 @@ class Migration(migrations.Migration): ("to", models.JSONField(default=list)), ("cc", models.JSONField(default=list)), ("bcc", models.JSONField(default=list)), + ("recipient_count", models.IntegerField(default=1)), ("should_attach_tickets", models.BooleanField(default=False)), ("should_attach_ical", models.BooleanField(default=False)), ("should_attach_other_files", models.JSONField(default=list)), diff --git a/src/pretix/base/models/mail.py b/src/pretix/base/models/mail.py index 257125d165..edb686c8d2 100644 --- a/src/pretix/base/models/mail.py +++ b/src/pretix/base/models/mail.py @@ -42,26 +42,48 @@ def CASCADE_IF_QUEUED(collector, field, sub_objs, using): class OutgoingMail(models.Model): STATUS_QUEUED = "queued" + STATUS_WITHHELD = "withheld" STATUS_INFLIGHT = "inflight" - STATUS_AWAWITING_RETRY = "awaiting_retry" + STATUS_AWAITING_RETRY = "awaiting_retry" STATUS_FAILED = "failed" STATUS_SENT = "sent" STATUS_BOUNCED = "bounced" + STATUS_ABORTED = "aborted" STATUS_CHOICES = ( (STATUS_QUEUED, _("queued")), (STATUS_INFLIGHT, _("being sent")), - (STATUS_AWAWITING_RETRY, _("awaiting retry")), + (STATUS_AWAITING_RETRY, _("awaiting retry")), + (STATUS_WITHHELD, _("withheld")), # for plugin use (STATUS_FAILED, _("failed")), + (STATUS_ABORTED, _("aborted")), (STATUS_SENT, _("sent")), - (STATUS_BOUNCED, _("bounced")), + (STATUS_BOUNCED, _("bounced")), # for plugin use ) + STATUS_LIST_ABORTABLE = { + STATUS_QUEUED, + STATUS_WITHHELD, + STATUS_AWAITING_RETRY, + } + STATUS_LIST_RETRYABLE = { + STATUS_FAILED, + STATUS_WITHHELD, + } + # The GUID is a globally unique ID for the email added to a header of the email for later tracing + # in bug reports etc. We could theoretically also use this as a basis for the Message-ID header, but + # we currently don't since we are unsure if some intermediary SMTP servers have opinions on setting + # their own Message-ID headers. guid = models.UUIDField(db_index=True, default=uuid.uuid4) + status = models.CharField(max_length=200, choices=STATUS_CHOICES, default=STATUS_QUEUED) created = models.DateTimeField(auto_now_add=True) + + # sent will be the time the email was sent or the email failed sent = models.DateTimeField(null=True, blank=True) + inflight_since = models.DateTimeField(null=True, blank=True) retry_after = models.DateTimeField(null=True, blank=True) + error = models.TextField(null=True, blank=True) error_detail = models.TextField(null=True, blank=True) @@ -122,6 +144,7 @@ class OutgoingMail(models.Model): to = models.JSONField(default=list) cc = models.JSONField(default=list) bcc = models.JSONField(default=list) + recipient_count = models.IntegerField() # We don't store the actual invoices, tickets or calendar invites, so if the email is re-sent at a later time, a # newer version of the files might be used. We accept that risk to save on storage and also because the new @@ -143,6 +166,7 @@ class OutgoingMail(models.Model): # if the email is sent. Otherwise, they will be skipped. We accept that risk. should_attach_other_files = models.JSONField(default=list) + # [{name, type size}] of the attachments we actually setn actual_attachments = models.JSONField(default=list) class Meta: @@ -164,7 +188,11 @@ class OutgoingMail(models.Model): @property def is_failed(self): - return self.status in (OutgoingMail.STATUS_FAILED, OutgoingMail.STATUS_AWAWITING_RETRY, OutgoingMail.STATUS_BOUNCED) + return self.status in ( + OutgoingMail.STATUS_FAILED, + OutgoingMail.STATUS_AWAITING_RETRY, + OutgoingMail.STATUS_BOUNCED, + ) def save(self, *args, **kwargs): if self.orderposition_id and not self.order_id: @@ -175,6 +203,7 @@ class OutgoingMail(models.Model): self.organizer = self.event.organizer if self.customer_id and not self.organizer_id: self.organizer = self.customer.organizer + self.recipient_count = len(self.to) + len(self.cc) + len(self.bcc) super().save(*args, **kwargs) def log_parameters(self): diff --git a/src/pretix/base/services/cleanup.py b/src/pretix/base/services/cleanup.py index 0142f95752..41376b920f 100644 --- a/src/pretix/base/services/cleanup.py +++ b/src/pretix/base/services/cleanup.py @@ -56,7 +56,7 @@ def clean_cached_files(sender, **kwargs): status__in=( OutgoingMail.STATUS_QUEUED, OutgoingMail.STATUS_INFLIGHT, - OutgoingMail.STATUS_AWAWITING_RETRY, + OutgoingMail.STATUS_AWAITING_RETRY, OutgoingMail.STATUS_FAILED, ), ) diff --git a/src/pretix/base/services/mail.py b/src/pretix/base/services/mail.py index 72753003b8..3316ecb04f 100644 --- a/src/pretix/base/services/mail.py +++ b/src/pretix/base/services/mail.py @@ -103,6 +103,12 @@ class SendMailException(Exception): pass +class WithholdMailException(Exception): + def __init__(self, error, error_detail): + self.error = error + self.error_detail = error_detail + + def clean_sender_name(sender_name: str) -> str: # Even though we try to properly escape sender names, some characters seem to cause problems when the escaping # fails due to some forwardings, etc. @@ -366,13 +372,13 @@ def mail_send_task(self, *args, outgoing_mail: int) -> bool: try: outgoing_mail = OutgoingMail.objects.select_for_update(of=OF_SELF).get(pk=outgoing_mail) except OutgoingMail.DoesNotExist: - logger.info(f"Ignoring job for non existing email {outgoing_mail}") + logger.info(f"Ignoring job for non existing email {outgoing_mail.guid}") return False if outgoing_mail.status == OutgoingMail.STATUS_INFLIGHT: - logger.info(f"Ignoring job for inflight email {outgoing_mail.pk}") + logger.info(f"Ignoring job for inflight email {outgoing_mail.guid}") return False - elif outgoing_mail.status in (OutgoingMail.STATUS_SENT, OutgoingMail.STATUS_FAILED): - logger.info(f"Ignoring job for email {outgoing_mail.pk} in final state {outgoing_mail.status}") + elif outgoing_mail.status not in (OutgoingMail.STATUS_AWAITING_RETRY, OutgoingMail.STATUS_QUEUED): + logger.info(f"Ignoring job for email {outgoing_mail.guid} in final state {outgoing_mail.status}") return False outgoing_mail.status = OutgoingMail.STATUS_INFLIGHT outgoing_mail.inflight_since = now() @@ -387,7 +393,7 @@ def mail_send_task(self, *args, outgoing_mail: int) -> bool: to=outgoing_mail.to, cc=outgoing_mail.cc, bcc=outgoing_mail.bcc, - headers=outgoing_mail.headers, + headers=headers, ) # Rewrite all tags from real URLs or data URLs to inline attachments referred to by content ID @@ -420,7 +426,7 @@ def mail_send_task(self, *args, outgoing_mail: int) -> bool: except MaxRetriesExceededError: # Well then, something is really wrong, let's send it without attachment before we # don't sent at all - logger.exception(f'Could not attach tickets to email {outgoing_mail.pk}') + logger.exception(f'Could not attach tickets to email {outgoing_mail.guid}') pass if attach_size * 1.37 < settings.FILE_UPLOAD_MAX_SIZE_EMAIL_ATTACHMENT - 1024 * 1024: @@ -479,7 +485,7 @@ def mail_send_task(self, *args, outgoing_mail: int) -> bool: ) invoices_attached.append(inv) except Exception: - logger.exception(f'Could not attach invoice to email {outgoing_mail.pk}') + logger.exception(f'Could not attach invoice to email {outgoing_mail.guid}') pass else: if inv.transmission_type == "email": @@ -509,7 +515,7 @@ def mail_send_task(self, *args, outgoing_mail: int) -> bool: ftype ) except: - logger.exception(f'Could not attach file to email {outgoing_mail.pk}') + logger.exception(f'Could not attach file to email {outgoing_mail.guid}') pass for cf in outgoing_mail.should_attach_cached_files.all(): @@ -521,20 +527,9 @@ def mail_send_task(self, *args, outgoing_mail: int) -> bool: cf.type, ) except: - logger.exception(f'Could not attach file to email {outgoing_mail.pk}') + logger.exception(f'Could not attach file to email {outgoing_mail.guid}') pass - if outgoing_mail.event: - with outgoing_mail.scope_manager(): - email = email_filter.send_chained( - outgoing_mail.event, 'message', message=email, order=outgoing_mail.order, user=outgoing_mail.user - ) - - email = global_email_filter.send_chained( - outgoing_mail.event, 'message', message=email, user=outgoing_mail.user, order=outgoing_mail.order, - organizer=outgoing_mail.organizer, customer=outgoing_mail.customer - ) - outgoing_mail.actual_attachments = [ { "name": a[0], @@ -543,11 +538,58 @@ def mail_send_task(self, *args, outgoing_mail: int) -> bool: } for a in email.attachments ] + try: + if outgoing_mail.event: + with outgoing_mail.scope_manager(): + email = email_filter.send_chained( + sender=outgoing_mail.event, + chain_kwarg_name='message', + message=email, + order=outgoing_mail.order, + user=outgoing_mail.user, + outgoing_mail=outgoing_mail, + ) + + email = global_email_filter.send_chained( + sender=outgoing_mail.event, + chain_kwarg_name='message', + message=email, + user=outgoing_mail.user, + order=outgoing_mail.order, + organizer=outgoing_mail.organizer, + customer=outgoing_mail.customer, + outgoing_mail=outgoing_mail, + ) + except WithholdMailException as e: + outgoing_mail.status = OutgoingMail.STATUS_WITHHELD + outgoing_mail.error = e.error + outgoing_mail.error_detail = e.error_detail + outgoing_mail.sent = now() + outgoing_mail.retry_after = None + outgoing_mail.actual_attachments = [ + { + "name": a[0], + "size": len(a[1]), + "type": a[2], + } for a in email.attachments + ] + outgoing_mail.save(update_fields=["status", "error", "error_detail", "sent", "retry_after", "actual_attachments"]) + logger.info(f"Email {outgoing_mail.guid} withheld") + return False + + # Seems duplicate, but needs to be in this order since plugins might change this + outgoing_mail.actual_attachments = [ + { + "name": a[0], + "size": len(a[1]), + "type": a[2], + } for a in email.attachments + ] backend = outgoing_mail.get_mail_backend() try: backend.send_messages([email]) except Exception as e: - logger.exception(f'Error sending email {outgoing_mail.pk}') + logger.exception(f'Error sending email {outgoing_mail.guid}') retry_strategy = _retry_strategy(e) err, err_detail = _format_error(e) @@ -568,21 +610,21 @@ def mail_send_task(self, *args, outgoing_mail: int) -> bool: max_retries = 10 retry_after = min(30 + cnt * 10, 1800) - outgoing_mail.status = OutgoingMail.STATUS_AWAWITING_RETRY + outgoing_mail.status = OutgoingMail.STATUS_AWAITING_RETRY outgoing_mail.retry_after = now() + timedelta(seconds=retry_after) outgoing_mail.save(update_fields=["status", "error", "error_detail", "sent", "retry_after", "actual_attachments"]) self.retry(max_retries=max_retries, countdown=retry_after) # throws RetryException, ends function flow elif retry_strategy in ("microsoft_concurrency", "quick"): max_retries = 5 retry_after = [10, 30, 60, 300, 900, 900][self.request.retries] - outgoing_mail.status = OutgoingMail.STATUS_AWAWITING_RETRY + outgoing_mail.status = OutgoingMail.STATUS_AWAITING_RETRY outgoing_mail.retry_after = now() + timedelta(seconds=retry_after) outgoing_mail.save(update_fields=["status", "error", "error_detail", "sent", "retry_after", "actual_attachments"]) self.retry(max_retries=max_retries, countdown=retry_after) # throws RetryException, ends function flow elif retry_strategy == "slow": retry_after = [60, 300, 600, 1200, 1800, 1800][self.request.retries] - outgoing_mail.status = OutgoingMail.STATUS_AWAWITING_RETRY + outgoing_mail.status = OutgoingMail.STATUS_AWAITING_RETRY outgoing_mail.retry_after = now() + timedelta(seconds=retry_after) outgoing_mail.save(update_fields=["status", "error", "error_detail", "sent", "retry_after", "actual_attachments"]) self.retry(max_retries=5, countdown=retry_after) # throws RetryException, ends function flow @@ -681,7 +723,7 @@ def mail_send_task(self, *args, outgoing_mail: int) -> bool: ) -def mail_send(to: List[str], subject: str, body: str, html: str, sender: str, +def mail_send(to: List[str], subject: str, body: str, html: Optional[str], sender: str, event: int = None, position: int = None, headers: dict = None, cc: List[str] = None, bcc: List[str] = None, invoices: List[int] = None, order: int = None, attach_tickets=False, user=None, organizer=None, customer=None, attach_ical=False, attach_cached_files: List[int] = None, @@ -708,6 +750,15 @@ def mail_send(to: List[str], subject: str, body: str, html: str, sender: str, should_attach_ical=attach_ical, should_attach_other_files=attach_other_files or [], ) + if invoices and not position: + m.should_attach_invoices.add(*invoices) + if attach_cached_files: + for cf in attach_cached_files: + if not isinstance(cf, CachedFile): + m.should_attach_cached_files.add(CachedFile.objects.get(pk=cf)) + else: + m.should_attach_cached_files.add(cf) + mail_send_task.apply_async(kwargs={"outgoing_mail": m.pk}) @@ -1007,7 +1058,7 @@ def retry_stuck_queued_mails(sender, **kwargs): status=OutgoingMail.STATUS_QUEUED, created__lt=now() - timedelta(hours=1), ) | Q( - status=OutgoingMail.STATUS_AWAWITING_RETRY, + status=OutgoingMail.STATUS_AWAITING_RETRY, retry_after__lt=now() - timedelta(hours=1), ) ): diff --git a/src/pretix/base/signals.py b/src/pretix/base/signals.py index 3ab54bf0eb..c917f318bd 100644 --- a/src/pretix/base/signals.py +++ b/src/pretix/base/signals.py @@ -944,32 +944,40 @@ As with all event-plugin signals, the ``sender`` keyword argument will contain t email_filter = EventPluginSignal() """ -Arguments: ``message``, ``order``, ``user`` +Arguments: ``message``, ``order``, ``user``, ``outgoing_mail`` This signal allows you to implement a middleware-style filter on all outgoing emails. You are expected to return a (possibly modified) copy of the message object passed to you. As with all event-plugin signals, the ``sender`` keyword argument will contain the event. The ``message`` argument will contain an ``EmailMultiAlternatives`` object. +The ``outgoing_mail`` argument will contain the ``OutgoingMail`` model instance. Note that the ``message`` object +might have newer information if a previous plugin already modified the email. If the email is associated with a specific order, the ``order`` argument will be passed as well, otherwise it will be ``None``. If the email is associated with a specific user, e.g. a notification email, the ``user`` argument will be passed as well, otherwise it will be ``None``. + +You can raise ``WithholdMailException`` to prevent the email from being sent, e.g. when implementing rate limiting. """ global_email_filter = GlobalSignal() """ -Arguments: ``message``, ``order``, ``user``, ``customer``, ``organizer`` +Arguments: ``message``, ``order``, ``user``, ``customer``, ``organizer``, ``outgoing_mail`` This signal allows you to implement a middleware-style filter on all outgoing emails. You are expected to return a (possibly modified) copy of the message object passed to you. This signal is called on all events and even if there is no known event. ``sender`` is an event or None. The ``message`` argument will contain an ``EmailMultiAlternatives`` object. +The ``outgoing_mail`` argument will contain the ``OutgoingMail`` model instance. Note that the ``message`` object +might have newer information if a previous plugin already modified the email. If the email is associated with a specific order, the ``order`` argument will be passed as well, otherwise it will be ``None``. If the email is associated with a specific user, e.g. a notification email, the ``user`` argument will be passed as well, otherwise it will be ``None``. + +You can raise ``WithholdMailException`` to prevent the email from being sent, e.g. when implementing rate limiting. """ diff --git a/src/pretix/control/logdisplay.py b/src/pretix/control/logdisplay.py index c34cfa104a..8f8f5b291e 100644 --- a/src/pretix/control/logdisplay.py +++ b/src/pretix/control/logdisplay.py @@ -700,6 +700,7 @@ class CoreUserImpersonatedLogEntryType(UserImpersonatedLogEntryType): 'pretix.organizer.export.schedule.executed': _('A scheduled export has been executed.'), 'pretix.organizer.export.schedule.failed': _('A scheduled export has failed: {reason}.'), 'pretix.organizer.outgoingmails.retried': _('Failed emails have been scheduled to be retried.'), + 'pretix.organizer.outgoingmails.aborted': _('Queued emails have been aborted.'), 'pretix.giftcards.acceptance.added': _('Gift card acceptance for another organizer has been added.'), 'pretix.giftcards.acceptance.removed': _('Gift card acceptance for another organizer has been removed.'), 'pretix.giftcards.acceptance.acceptor.invited': _('A new gift card acceptor has been invited.'), diff --git a/src/pretix/control/templates/pretixcontrol/organizers/outgoing_mail.html b/src/pretix/control/templates/pretixcontrol/organizers/outgoing_mail.html index ae033f3043..5183459db0 100644 --- a/src/pretix/control/templates/pretixcontrol/organizers/outgoing_mail.html +++ b/src/pretix/control/templates/pretixcontrol/organizers/outgoing_mail.html @@ -35,7 +35,7 @@
{% trans "Status" %}
{% if mail.status == "queued" %} - {% icon "clock" %} {% trans "queued" %} + {% icon "clock-o" %} {% trans "queued" %} {% elif mail.status == "inflight" %} {% icon "send" %} {% trans "being sent" %} {% elif mail.status == "awaiting_retry" %} @@ -43,7 +43,11 @@ {% elif mail.status == "failed" %} {% icon "warning" %} {% trans "failed" %} {% elif mail.status == "bounced" %} - {% icon "ban" %} {% trans "bounced" %} + {% icon "exclamation-circle" %} {% trans "bounced" %} + {% elif mail.status == "withheld" %} + {% icon "ban" %} {% trans "withheld" %} + {% elif mail.status == "aborted" %} + {% icon "ban" %} {% trans "aborted" %} {% elif mail.status == "sent" %} {% icon "check" %} {% trans "sent" %} {% endif %} diff --git a/src/pretix/control/templates/pretixcontrol/organizers/outgoing_mails.html b/src/pretix/control/templates/pretixcontrol/organizers/outgoing_mails.html index 1384e2a440..b3bdc9b2d0 100644 --- a/src/pretix/control/templates/pretixcontrol/organizers/outgoing_mails.html +++ b/src/pretix/control/templates/pretixcontrol/organizers/outgoing_mails.html @@ -133,7 +133,7 @@ {% if m.status == "queued" %} - {% icon "clock" %} {% trans "queued" %} + {% icon "clock-o" %} {% trans "queued" %} {% elif m.status == "inflight" %} {% icon "send" %} {% trans "being sent" %} {% elif m.status == "awaiting_retry" %} @@ -141,7 +141,11 @@ {% elif m.status == "failed" %} {% icon "warning" %} {% trans "failed" %} {% elif m.status == "bounced" %} - {% icon "ban" %} {% trans "bounced" %} + {% icon "exclamation-circle" %} {% trans "bounced" %} + {% elif m.status == "withheld" %} + {% icon "ban" %} {% trans "withheld" %} + {% elif m.status == "aborted" %} + {% icon "ban" %} {% trans "aborted" %} {% elif m.status == "sent" %} {% icon "check" %} {% trans "sent" %} {% endif %} @@ -165,7 +169,11 @@
+
diff --git a/src/pretix/control/views/mail.py b/src/pretix/control/views/mail.py index a8a439bb41..6b775961ea 100644 --- a/src/pretix/control/views/mail.py +++ b/src/pretix/control/views/mail.py @@ -131,7 +131,7 @@ class OutgoingMailBulkAction(OutgoingMailQueryMixin, OrganizerPermissionRequired def post(self, request, *args, **kwargs): if request.POST.get('action') == 'retry': ids = set( - self.get_queryset().filter(status=OutgoingMail.STATUS_FAILED).values_list("pk", flat=True) + self.get_queryset().filter(status__in=OutgoingMail.STATUS_LIST_RETRYABLE).values_list("pk", flat=True) ) with transaction.atomic(): OutgoingMail.objects.filter(pk__in=ids).update( @@ -151,6 +151,30 @@ class OutgoingMailBulkAction(OutgoingMailQueryMixin, OrganizerPermissionRequired "A retry of {num} emails was scheduled.", len(ids), ).format(num=len(ids))) + elif request.POST.get('action') == 'abort': + ids = set( + self.get_queryset().filter( + status__in=(OutgoingMail.STATUS_QUEUED, OutgoingMail.STATUS_AWAITING_RETRY) + ).values_list("pk", flat=True) + ) + with transaction.atomic(): + OutgoingMail.objects.filter(pk__in=ids).update( + status=OutgoingMail.STATUS_ABORTED, + sent=None, + ) + self.request.organizer.log_action( + 'pretix.organizer.outgoingmails.aborted', user=self.request.user, data={ + 'mails': list(ids) + }, save=False + ) + for i in ids: + mail_send_task.apply_async(kwargs={"outgoing_mail": i}) + + messages.success(request, ngettext( + "One email was aborted and will not be sent.", + "{num} emails were aborted and will not be sent.", + len(ids), + ).format(num=len(ids))) return redirect(self.get_success_url()) def get_success_url(self) -> str: diff --git a/src/pretix/static/pretixcontrol/js/ui/outgoingmail.js b/src/pretix/static/pretixcontrol/js/ui/outgoingmail.js index d82803bafb..f6457440ad 100644 --- a/src/pretix/static/pretixcontrol/js/ui/outgoingmail.js +++ b/src/pretix/static/pretixcontrol/js/ui/outgoingmail.js @@ -4,6 +4,7 @@ function is_sandbox_supported() { } function safe_render(url, parent) { + // Estimate the height that prevents the user from having to scroll on two levels to see the full email const height = ( window.innerHeight - parent.parent().get(0).getBoundingClientRect().top - document.querySelector("footer").getBoundingClientRect().height - 20 ) + "px"; diff --git a/src/tests/base/test_mail.py b/src/tests/base/test_mail.py index d68a02ac04..2d94dcb378 100644 --- a/src/tests/base/test_mail.py +++ b/src/tests/base/test_mail.py @@ -217,7 +217,7 @@ def test_queue_state_retry_failure(env, monkeypatch): 'outgoing_mail': m.pk, }, max_retries=0) m.refresh_from_db() - assert m.status == OutgoingMail.STATUS_AWAWITING_RETRY + assert m.status == OutgoingMail.STATUS_AWAITING_RETRY assert m.retry_after > now()