From 2f0d98ba19fc0d1f307469f42653b03c4699b234 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 15 Nov 2025 16:58:41 +0100 Subject: [PATCH 01/33] Write basic move out reminder mail --- pycroft/templates/mail/move_out_reminder.html | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 pycroft/templates/mail/move_out_reminder.html diff --git a/pycroft/templates/mail/move_out_reminder.html b/pycroft/templates/mail/move_out_reminder.html new file mode 100644 index 000000000..cee367a91 --- /dev/null +++ b/pycroft/templates/mail/move_out_reminder.html @@ -0,0 +1,40 @@ +{%- extends "base.html.j2" -%} +{%- block body -%} +* English version below * + +Hallo {{ user_name }}, + +Unserer Kenntnis nach endet dein Mietvertrag am {{ contract_end }}. +Deine Mitgliedschaft bei der AG DSN endet *nicht automatisch*. +Bitte denke also daran, sie zu beenden, +sofern du nicht über deine Wohndauer hinaus bei uns Mitglied bleiben möchtest. + +Dein Mitgliedschaftsende kannst du unter folgendem Link vormerken: [1] + +Bei Fragen oder Problemen antworte gerne auf diese Mail. + +Viele Grüße +Deine AG DSN + +[1] https://agdsn.de/sipa/usersuite/terminate-membership + + +* English version * + +Hello {{ user_name }}, + +To our knowledge, your rental agreement ends on {{ contract_end }}. +Your membership with AG DSN does *not* end automatically. +Please remember to cancel your membership +if you do not wish to remain a member beyond the duration of your stay. + +You can register your termination at the following link: [1] + +If you have any questions or problems, please reply to this email. + +Best regards, +Your AG DSN + +[1] https://agdsn.de/sipa/usersuite/terminate-membership + +{%- endblock -%} From 75051b4de710fb7227af97fc2544deda5afd6931 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 15 Nov 2025 16:58:41 +0100 Subject: [PATCH 02/33] TODOs --- pycroft/lib/mail.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pycroft/lib/mail.py b/pycroft/lib/mail.py index dfe029036..c817b2d93 100644 --- a/pycroft/lib/mail.py +++ b/pycroft/lib/mail.py @@ -60,6 +60,7 @@ class MailTemplate: def __init__(self, **kwargs: t.Any) -> None: self.args = kwargs + # TODO don't put this as a method on the template… We want a separate render method for each template. def render(self, **kwargs: t.Any) -> tuple[str, str]: plain = self.jinja_template.render(mode="plain", **self.args, **kwargs) html = self.jinja_template.render(mode="html", **self.args, **kwargs) From cdda7559fc1c1eb70ae97b81ac546c397fadca57 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Tue, 24 Jun 2025 20:54:39 +0200 Subject: [PATCH 03/33] Fix error path in uninitialized config access --- pycroft/lib/mail.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pycroft/lib/mail.py b/pycroft/lib/mail.py index c817b2d93..6d2c6e067 100644 --- a/pycroft/lib/mail.py +++ b/pycroft/lib/mail.py @@ -74,9 +74,10 @@ def jinja_template(self) -> jinja2.Template: @lru_cache(maxsize=None) def _get_template(template_location: str) -> jinja2.Template: - if config is None: - raise RuntimeError("`mail.config` not set up!") - return config.template_env.get_template(template_location) + try: + return config.template_env.get_template(template_location) + except RuntimeError as e: + raise RuntimeError("`mail.config` not set up!") from e def compose_mail(mail: Mail, from_: str, default_reply_to: str | None) -> MIMEMultipart: From 62dfcad5087a75598d749ea3822315526f734bf9 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 15 Nov 2025 16:58:41 +0100 Subject: [PATCH 04/33] Turn `pycroft.lib.mail` into package --- pycroft/lib/{mail.py => mail/__init__.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename pycroft/lib/{mail.py => mail/__init__.py} (100%) diff --git a/pycroft/lib/mail.py b/pycroft/lib/mail/__init__.py similarity index 100% rename from pycroft/lib/mail.py rename to pycroft/lib/mail/__init__.py From 7f22eaf9e7edeeb04f19eef43cee2462efd0c8cc Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 22 Nov 2025 14:34:55 +0100 Subject: [PATCH 05/33] Turn `compose_mail` into method to be consistent with mime methods --- pycroft/lib/mail/__init__.py | 37 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/pycroft/lib/mail/__init__.py b/pycroft/lib/mail/__init__.py index 6d2c6e067..150ac2e6d 100644 --- a/pycroft/lib/mail/__init__.py +++ b/pycroft/lib/mail/__init__.py @@ -52,6 +52,23 @@ def body_html_mime(self) -> MIMEText | None: return MIMEText(self.body_html, "html", _charset="utf-8") + def compose(self, from_: str, default_reply_to: str | None) -> MIMEMultipart: + msg = MIMEMultipart("alternative", _charset="utf-8") + msg["Message-Id"] = make_msgid() + msg["From"] = from_ + msg["To"] = str(Header(self.to_address)) + msg["Subject"] = self.subject + msg["Date"] = formatdate(localtime=True) + msg.attach(self.body_plain_mime) + if (html := self.body_html_mime) is not None: + msg.attach(html) + if reply_to := self.reply_to or default_reply_to: + msg["Reply-To"] = reply_to + + print(msg) + + return msg + class MailTemplate: template: str subject: str @@ -80,24 +97,6 @@ def _get_template(template_location: str) -> jinja2.Template: raise RuntimeError("`mail.config` not set up!") from e -def compose_mail(mail: Mail, from_: str, default_reply_to: str | None) -> MIMEMultipart: - msg = MIMEMultipart("alternative", _charset="utf-8") - msg["Message-Id"] = make_msgid() - msg["From"] = from_ - msg["To"] = str(Header(mail.to_address)) - msg["Subject"] = mail.subject - msg["Date"] = formatdate(localtime=True) - msg.attach(mail.body_plain_mime) - if (html := mail.body_html_mime) is not None: - msg.attach(html) - if reply_to := mail.reply_to or default_reply_to: - msg["Reply-To"] = reply_to - - print(msg) - - return msg - - class RetryableException(PycroftLibException): pass @@ -176,7 +175,7 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: for mail in mails: try: - mime_mail = compose_mail(mail, from_=mail_from, default_reply_to=mail_reply_to) + mime_mail = mail.compose(from_=mail_from, default_reply_to=mail_reply_to) assert mail_envelope_from is not None smtp.sendmail(from_addr=mail_envelope_from, to_addrs=mail.to_address, msg=mime_mail.as_string()) From c77e7e55e28667709a8f6b2ed70d216e5f57f02d Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 22 Nov 2025 14:59:26 +0100 Subject: [PATCH 06/33] move `Mail` class into `pycroft.lib.mail.concepts` --- pycroft/lib/mail/__init__.py | 41 +--------------------------------- pycroft/lib/mail/concepts.py | 43 ++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 40 deletions(-) create mode 100644 pycroft/lib/mail/concepts.py diff --git a/pycroft/lib/mail/__init__.py b/pycroft/lib/mail/__init__.py index 150ac2e6d..82e8fb068 100644 --- a/pycroft/lib/mail/__init__.py +++ b/pycroft/lib/mail/__init__.py @@ -12,10 +12,6 @@ import typing as t from contextvars import ContextVar from dataclasses import dataclass, field, InitVar -from email.header import Header -from email.mime.multipart import MIMEMultipart -from email.mime.text import MIMEText -from email.utils import make_msgid, formatdate from functools import lru_cache import jinja2 @@ -32,42 +28,7 @@ logger.setLevel(logging.INFO) -@dataclass -class Mail: - to_name: str - to_address: str - subject: str - body_plain: str - body_html: str | None = None - reply_to: str | None = None - - @property - def body_plain_mime(self) -> MIMEText: - return MIMEText(self.body_plain, "plain", _charset="utf-8") - - @property - def body_html_mime(self) -> MIMEText | None: - if not self.body_html: - return None - return MIMEText(self.body_html, "html", _charset="utf-8") - - - def compose(self, from_: str, default_reply_to: str | None) -> MIMEMultipart: - msg = MIMEMultipart("alternative", _charset="utf-8") - msg["Message-Id"] = make_msgid() - msg["From"] = from_ - msg["To"] = str(Header(self.to_address)) - msg["Subject"] = self.subject - msg["Date"] = formatdate(localtime=True) - msg.attach(self.body_plain_mime) - if (html := self.body_html_mime) is not None: - msg.attach(html) - if reply_to := self.reply_to or default_reply_to: - msg["Reply-To"] = reply_to - - print(msg) - - return msg +from .concepts import Mail class MailTemplate: template: str diff --git a/pycroft/lib/mail/concepts.py b/pycroft/lib/mail/concepts.py new file mode 100644 index 000000000..9ac03f048 --- /dev/null +++ b/pycroft/lib/mail/concepts.py @@ -0,0 +1,43 @@ +from dataclasses import dataclass +from email.header import Header +from email.mime.multipart import MIMEMultipart +from email.mime.text import MIMEText +from email.utils import make_msgid, formatdate + + +@dataclass +class Mail: + to_name: str + to_address: str + subject: str + body_plain: str + body_html: str | None = None + reply_to: str | None = None + + @property + def body_plain_mime(self) -> MIMEText: + return MIMEText(self.body_plain, "plain", _charset="utf-8") + + @property + def body_html_mime(self) -> MIMEText | None: + if not self.body_html: + return None + return MIMEText(self.body_html, "html", _charset="utf-8") + + + def compose(self, from_: str, default_reply_to: str | None) -> MIMEMultipart: + msg = MIMEMultipart("alternative", _charset="utf-8") + msg["Message-Id"] = make_msgid() + msg["From"] = from_ + msg["To"] = str(Header(self.to_address)) + msg["Subject"] = self.subject + msg["Date"] = formatdate(localtime=True) + msg.attach(self.body_plain_mime) + if (html := self.body_html_mime) is not None: + msg.attach(html) + if reply_to := self.reply_to or default_reply_to: + msg["Reply-To"] = reply_to + + print(msg) + + return msg From 1e9264678050e19980082a0aa6fbfede8e19cfc4 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 22 Nov 2025 14:59:26 +0100 Subject: [PATCH 07/33] extract `pycroft.lib.mail.config` --- pycroft/lib/mail/__init__.py | 56 +------------------------------- pycroft/lib/mail/config.py | 63 ++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 55 deletions(-) create mode 100644 pycroft/lib/mail/config.py diff --git a/pycroft/lib/mail/__init__.py b/pycroft/lib/mail/__init__.py index 82e8fb068..f147a58c1 100644 --- a/pycroft/lib/mail/__init__.py +++ b/pycroft/lib/mail/__init__.py @@ -21,14 +21,12 @@ # TODO proxy and DI; set at app init -_config_var: ContextVar[MailConfig] = ContextVar("config") -config: MailConfig = LocalProxy(_config_var) # type: ignore[assignment] - logger = logging.getLogger('mail') logger.setLevel(logging.INFO) from .concepts import Mail +from .config import MailConfig, config class MailTemplate: template: str @@ -225,55 +223,3 @@ def send_template_mails( from pycroft.task import send_mails_async send_mails_async.delay(mails) - - -@dataclass -class MailConfig: - mail_envelope_from: str - mail_from: str - mail_reply_to: str | None - smtp_host: str - smtp_user: str - smtp_password: str - smtp_port: int = field(default=465) - smtp_ssl: str = field(default="ssl") - - template_path_type: InitVar[str | None] = None - template_path: InitVar[str | None] = None - template_env: jinja2.Environment = field(init=False) - - @classmethod - def from_env(cls) -> t.Self: - env = os.environ - config = cls( - mail_envelope_from=env["PYCROFT_MAIL_ENVELOPE_FROM"], - mail_from=env["PYCROFT_MAIL_FROM"], - mail_reply_to=env.get("PYCROFT_MAIL_REPLY_TO"), - smtp_host=env["PYCROFT_SMTP_HOST"], - smtp_user=env["PYCROFT_SMTP_USER"], - smtp_password=env["PYCROFT_SMTP_PASSWORD"], - template_path_type=env.get("PYCROFT_TEMPLATE_PATH_TYPE"), - template_path=env.get("PYCROFT_TEMPLATE_PATH"), - ) - if (smtp_port := env.get("PYCROFT_SMTP_PORT")) is not None: - config.smtp_port = int(smtp_port) - if (smtp_ssl := env.get("PYCROFT_SMTP_SSL")) is not None: - config.smtp_ssl = smtp_ssl - - return config - - def __post_init__(self, template_path_type: str | None, template_path: str | None) -> None: - template_loader: jinja2.BaseLoader - if template_path_type is None: - template_path_type = "filesystem" - if template_path is None: - template_path = "pycroft/templates" - - if template_path_type == "filesystem": - template_loader = jinja2.FileSystemLoader(searchpath=f"{template_path}/mail") - else: - template_loader = jinja2.PackageLoader( - package_name="pycroft", package_path=f"{template_path}/mail" - ) - - self.template_env = jinja2.Environment(loader=template_loader) diff --git a/pycroft/lib/mail/config.py b/pycroft/lib/mail/config.py new file mode 100644 index 000000000..e742176c2 --- /dev/null +++ b/pycroft/lib/mail/config.py @@ -0,0 +1,63 @@ +import os +import typing as t +from contextvars import ContextVar +from dataclasses import dataclass, field, InitVar + +import jinja2 +from werkzeug.local import LocalProxy + +@dataclass +class MailConfig: + mail_envelope_from: str + mail_from: str + mail_reply_to: str | None + smtp_host: str + smtp_user: str + smtp_password: str + smtp_port: int = field(default=465) + smtp_ssl: str = field(default="ssl") + + template_path_type: InitVar[str | None] = None + template_path: InitVar[str | None] = None + template_env: jinja2.Environment = field(init=False) + + @classmethod + def from_env(cls) -> t.Self: + env = os.environ + config = cls( + mail_envelope_from=env["PYCROFT_MAIL_ENVELOPE_FROM"], + mail_from=env["PYCROFT_MAIL_FROM"], + mail_reply_to=env.get("PYCROFT_MAIL_REPLY_TO"), + smtp_host=env["PYCROFT_SMTP_HOST"], + smtp_user=env["PYCROFT_SMTP_USER"], + smtp_password=env["PYCROFT_SMTP_PASSWORD"], + template_path_type=env.get("PYCROFT_TEMPLATE_PATH_TYPE"), + template_path=env.get("PYCROFT_TEMPLATE_PATH"), + ) + if (smtp_port := env.get("PYCROFT_SMTP_PORT")) is not None: + config.smtp_port = int(smtp_port) + if (smtp_ssl := env.get("PYCROFT_SMTP_SSL")) is not None: + config.smtp_ssl = smtp_ssl + + return config + + def __post_init__(self, template_path_type: str | None, template_path: str | None) -> None: + template_loader: jinja2.BaseLoader + if template_path_type is None: + template_path_type = "filesystem" + if template_path is None: + template_path = "pycroft/templates" + + if template_path_type == "filesystem": + template_loader = jinja2.FileSystemLoader(searchpath=f"{template_path}/mail") + else: + template_loader = jinja2.PackageLoader( + package_name="pycroft", package_path=f"{template_path}/mail" + ) + + self.template_env = jinja2.Environment(loader=template_loader) + + +_config_var: ContextVar[MailConfig] = ContextVar("config") +config: MailConfig = LocalProxy(_config_var) # type: ignore[assignment] + From 4e1df625a69d61be581f35b5d7e4ec6a1e3149c7 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 22 Nov 2025 14:59:26 +0100 Subject: [PATCH 08/33] Allow specifying custom template loader in MailTemplate constructor --- pycroft/lib/mail/__init__.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/pycroft/lib/mail/__init__.py b/pycroft/lib/mail/__init__.py index f147a58c1..3e4f26aec 100644 --- a/pycroft/lib/mail/__init__.py +++ b/pycroft/lib/mail/__init__.py @@ -33,7 +33,8 @@ class MailTemplate: subject: str args: dict - def __init__(self, **kwargs: t.Any) -> None: + def __init__(self, loader: t.Callable[[str], jinja2.Template] | None = None, **kwargs: t.Any) -> None: + self.jinja_template: jinja2.Template = (loader or _get_template)(self.template) self.args = kwargs # TODO don't put this as a method on the template… We want a separate render method for each template. @@ -43,10 +44,6 @@ def render(self, **kwargs: t.Any) -> tuple[str, str]: return plain, html - @property - def jinja_template(self) -> jinja2.Template: - return _get_template(self.template) - @lru_cache(maxsize=None) def _get_template(template_location: str) -> jinja2.Template: From 6b1f8af6450579aeb918827d4d131e8cb13acf4b Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 22 Nov 2025 15:04:50 +0100 Subject: [PATCH 09/33] Move `MailTemplate` to `pycroft.lib.mail.concepts` --- pycroft/lib/mail/__init__.py | 27 +-------------------------- pycroft/lib/mail/concepts.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/pycroft/lib/mail/__init__.py b/pycroft/lib/mail/__init__.py index 3e4f26aec..26456331a 100644 --- a/pycroft/lib/mail/__init__.py +++ b/pycroft/lib/mail/__init__.py @@ -25,34 +25,9 @@ logger.setLevel(logging.INFO) -from .concepts import Mail +from .concepts import Mail, MailTemplate from .config import MailConfig, config -class MailTemplate: - template: str - subject: str - args: dict - - def __init__(self, loader: t.Callable[[str], jinja2.Template] | None = None, **kwargs: t.Any) -> None: - self.jinja_template: jinja2.Template = (loader or _get_template)(self.template) - self.args = kwargs - - # TODO don't put this as a method on the template… We want a separate render method for each template. - def render(self, **kwargs: t.Any) -> tuple[str, str]: - plain = self.jinja_template.render(mode="plain", **self.args, **kwargs) - html = self.jinja_template.render(mode="html", **self.args, **kwargs) - - return plain, html - - -@lru_cache(maxsize=None) -def _get_template(template_location: str) -> jinja2.Template: - try: - return config.template_env.get_template(template_location) - except RuntimeError as e: - raise RuntimeError("`mail.config` not set up!") from e - - class RetryableException(PycroftLibException): pass diff --git a/pycroft/lib/mail/concepts.py b/pycroft/lib/mail/concepts.py index 9ac03f048..074e93e11 100644 --- a/pycroft/lib/mail/concepts.py +++ b/pycroft/lib/mail/concepts.py @@ -1,9 +1,14 @@ +import typing as t from dataclasses import dataclass from email.header import Header from email.mime.multipart import MIMEMultipart from email.mime.text import MIMEText from email.utils import make_msgid, formatdate +from functools import lru_cache +import jinja2 + +from .config import config @dataclass class Mail: @@ -41,3 +46,30 @@ def compose(self, from_: str, default_reply_to: str | None) -> MIMEMultipart: print(msg) return msg + + +class MailTemplate: + template: str + subject: str + args: dict + + def __init__(self, loader: t.Callable[[str], jinja2.Template] | None = None, **kwargs: t.Any) -> None: + self.jinja_template: jinja2.Template = (loader or _get_template)(self.template) + self.args = kwargs + + # TODO don't put this as a method on the template… We want a separate render method for each template. + def render(self, **kwargs: t.Any) -> tuple[str, str]: + plain = self.jinja_template.render(mode="plain", **self.args, **kwargs) + html = self.jinja_template.render(mode="html", **self.args, **kwargs) + + return plain, html + + +@lru_cache(maxsize=None) +def _get_template(template_location: str) -> jinja2.Template: + try: + return config.template_env.get_template(template_location) + except RuntimeError as e: + raise RuntimeError("`mail.config` not set up!") from e + + From e919165c37735d0b20a1908c44079a65ba3b2be8 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 22 Nov 2025 15:15:00 +0100 Subject: [PATCH 10/33] Extract `pycroft.lib.mail.submission` --- pycroft/lib/mail/__init__.py | 116 +------------------------------- pycroft/lib/mail/submission.py | 118 +++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 115 deletions(-) create mode 100644 pycroft/lib/mail/submission.py diff --git a/pycroft/lib/mail/__init__.py b/pycroft/lib/mail/__init__.py index 26456331a..1c522e497 100644 --- a/pycroft/lib/mail/__init__.py +++ b/pycroft/lib/mail/__init__.py @@ -4,11 +4,6 @@ """ from __future__ import annotations -import logging -import os -import smtplib -import ssl -import traceback import typing as t from contextvars import ContextVar from dataclasses import dataclass, field, InitVar @@ -17,119 +12,10 @@ import jinja2 from werkzeug.local import LocalProxy -from pycroft.lib.exc import PycroftLibException - - -# TODO proxy and DI; set at app init -logger = logging.getLogger('mail') -logger.setLevel(logging.INFO) - from .concepts import Mail, MailTemplate from .config import MailConfig, config - -class RetryableException(PycroftLibException): - pass - - -def send_mails(mails: list[Mail]) -> tuple[bool, int]: - """Send MIME text mails - - Returns False, if sending fails. Else returns True. - - :param mails: A list of mails - - :returns: Whether the transmission succeeded - :context: config - """ - if config is None: - raise RuntimeError("`mail.config` not set up!") - - mail_envelope_from = config.mail_envelope_from - mail_from = config.mail_from - mail_reply_to = config.mail_reply_to - smtp_host = config.smtp_host - smtp_port = config.smtp_port - smtp_user = config.smtp_user - smtp_password = config.smtp_password - smtp_ssl = config.smtp_ssl - - use_ssl = smtp_ssl == 'ssl' - use_starttls = smtp_ssl == 'starttls' - ssl_context = None - - if use_ssl or use_starttls: - try: - ssl_context = ssl.create_default_context() - ssl_context.verify_mode = ssl.VerifyMode.CERT_REQUIRED - ssl_context.check_hostname = True - except ssl.SSLError as e: - # smtp.connect failed to connect - logger.critical('Unable to create ssl context', extra={ - 'trace': True, - 'data': {'exception_arguments': e.args} - }) - raise RetryableException from e - - try: - smtp: smtplib.SMTP - if use_ssl: - assert ssl_context is not None - smtp = smtplib.SMTP_SSL(host=smtp_host, port=smtp_port, - context=ssl_context) - else: - smtp = smtplib.SMTP(host=smtp_host, port=smtp_port) - - if use_starttls: - smtp.starttls(context=ssl_context) - - if smtp_user: - assert smtp_password is not None - smtp.login(smtp_user, smtp_password) - except (OSError, smtplib.SMTPException) as e: - traceback.print_exc() - - # smtp.connect failed to connect - logger.critical( - "Unable to connect to SMTP server: %s", - e, - extra={ - "trace": True, - "tags": {"mailserver": f"{smtp_host}:{smtp_host}"}, - "data": {"exception_arguments": e.args}, - }, - ) - - raise RetryableException from e - else: - failures: int = 0 - - for mail in mails: - try: - mime_mail = mail.compose(from_=mail_from, default_reply_to=mail_reply_to) - assert mail_envelope_from is not None - smtp.sendmail(from_addr=mail_envelope_from, to_addrs=mail.to_address, - msg=mime_mail.as_string()) - except smtplib.SMTPException as e: - traceback.print_exc() - logger.critical( - 'Unable to send mail: "%s" to "%s": %s', mail.subject, mail.to_address, e, - extra={ - 'trace': True, - 'tags': {'mailserver': f"{smtp_host}:{smtp_host}"}, - 'data': {'exception_arguments': e.args, 'to': mail.to_address, - 'subject': mail.subject} - } - ) - failures += 1 - - smtp.close() - - logger.info('Tried to send mails (%i/%i succeeded)', len(mails) - failures, len(mails), extra={ - 'tags': {'mailserver': f"{smtp_host}:{smtp_host}"} - }) - - return failures == 0, failures +from .submission import send_mails, RetryableException class UserConfirmEmailTemplate(MailTemplate): diff --git a/pycroft/lib/mail/submission.py b/pycroft/lib/mail/submission.py new file mode 100644 index 000000000..263dc810e --- /dev/null +++ b/pycroft/lib/mail/submission.py @@ -0,0 +1,118 @@ +import logging +import smtplib +import ssl +import traceback + +from pycroft.lib.exc import PycroftLibException +from .concepts import Mail +from .config import config + + +logger = logging.getLogger(__name__) +logger.setLevel(logging.INFO) + + +def send_mails(mails: list[Mail]) -> tuple[bool, int]: + """Send MIME text mails + + Returns False, if sending fails. Else returns True. + + :param mails: A list of mails + + :returns: Whether the transmission succeeded + :context: config + """ + if config is None: + raise RuntimeError("`mail.config` not set up!") + + mail_envelope_from = config.mail_envelope_from + mail_from = config.mail_from + mail_reply_to = config.mail_reply_to + smtp_host = config.smtp_host + smtp_port = config.smtp_port + smtp_user = config.smtp_user + smtp_password = config.smtp_password + smtp_ssl = config.smtp_ssl + + use_ssl = smtp_ssl == 'ssl' + use_starttls = smtp_ssl == 'starttls' + ssl_context = None + + if use_ssl or use_starttls: + try: + ssl_context = ssl.create_default_context() + ssl_context.verify_mode = ssl.VerifyMode.CERT_REQUIRED + ssl_context.check_hostname = True + except ssl.SSLError as e: + # smtp.connect failed to connect + logger.critical('Unable to create ssl context', extra={ + 'trace': True, + 'data': {'exception_arguments': e.args} + }) + raise RetryableException from e + + try: + smtp: smtplib.SMTP + if use_ssl: + assert ssl_context is not None + smtp = smtplib.SMTP_SSL(host=smtp_host, port=smtp_port, + context=ssl_context) + else: + smtp = smtplib.SMTP(host=smtp_host, port=smtp_port) + + if use_starttls: + smtp.starttls(context=ssl_context) + + if smtp_user: + assert smtp_password is not None + smtp.login(smtp_user, smtp_password) + except (OSError, smtplib.SMTPException) as e: + traceback.print_exc() + + # smtp.connect failed to connect + logger.critical( + "Unable to connect to SMTP server: %s", + e, + extra={ + "trace": True, + "tags": {"mailserver": f"{smtp_host}:{smtp_host}"}, + "data": {"exception_arguments": e.args}, + }, + ) + + raise RetryableException from e + else: + failures: int = 0 + + for mail in mails: + try: + mime_mail = mail.compose(from_=mail_from, default_reply_to=mail_reply_to) + assert mail_envelope_from is not None + smtp.sendmail(from_addr=mail_envelope_from, to_addrs=mail.to_address, + msg=mime_mail.as_string()) + except smtplib.SMTPException as e: + traceback.print_exc() + logger.critical( + 'Unable to send mail: "%s" to "%s": %s', mail.subject, mail.to_address, e, + extra={ + 'trace': True, + 'tags': {'mailserver': f"{smtp_host}:{smtp_host}"}, + 'data': {'exception_arguments': e.args, 'to': mail.to_address, + 'subject': mail.subject} + } + ) + failures += 1 + + smtp.close() + + logger.info('Tried to send mails (%i/%i succeeded)', len(mails) - failures, len(mails), extra={ + 'tags': {'mailserver': f"{smtp_host}:{smtp_host}"} + }) + + return failures == 0, failures + + + +class RetryableException(PycroftLibException): + pass + From fcb92d83af7534825c44bdeb603b13c0114c3672 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 22 Nov 2025 15:18:41 +0100 Subject: [PATCH 11/33] Removed unused imports --- pycroft/lib/mail/__init__.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pycroft/lib/mail/__init__.py b/pycroft/lib/mail/__init__.py index 1c522e497..473273672 100644 --- a/pycroft/lib/mail/__init__.py +++ b/pycroft/lib/mail/__init__.py @@ -5,12 +5,6 @@ from __future__ import annotations import typing as t -from contextvars import ContextVar -from dataclasses import dataclass, field, InitVar -from functools import lru_cache - -import jinja2 -from werkzeug.local import LocalProxy from .concepts import Mail, MailTemplate From c0b21a01bba7f7e8dc3c8309949f97afa4885aee Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 22 Nov 2025 15:20:28 +0100 Subject: [PATCH 12/33] Extract templates to `pycroft.lib.mail.templates` --- pycroft/lib/mail/__init__.py | 56 +++++++---------------------------- pycroft/lib/mail/templates.py | 47 +++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 45 deletions(-) create mode 100644 pycroft/lib/mail/templates.py diff --git a/pycroft/lib/mail/__init__.py b/pycroft/lib/mail/__init__.py index 473273672..70f67b137 100644 --- a/pycroft/lib/mail/__init__.py +++ b/pycroft/lib/mail/__init__.py @@ -10,51 +10,17 @@ from .concepts import Mail, MailTemplate from .config import MailConfig, config from .submission import send_mails, RetryableException - - -class UserConfirmEmailTemplate(MailTemplate): - template = "user_confirm_email.html" - subject = "Bitte bestätige deine E-Mail Adresse // Please confirm your email address" - - -class UserResetPasswordTemplate(MailTemplate): - template = "user_reset_password.html" - subject = "Neues Passwort setzen // Set a new password" - - -class UserMovedInTemplate(MailTemplate): - template = "user_moved_in.html" - subject = "Wohnortänderung // Change of residence" - - -class UserCreatedTemplate(MailTemplate): - template = "user_created.html" - subject = "Willkommen bei der AG DSN // Welcome to the AG DSN" - - -class MemberRequestPendingTemplate(MailTemplate): - template = "member_request_pending.html" - subject = "Deine Mitgliedschaftsanfrage // Your member request" - - -class MemberRequestDeniedTemplate(MailTemplate): - template = "member_request_denied.html" - subject = "Mitgliedschaftsanfrage abgelehnt // Member request denied" - - -class MemberRequestMergedTemplate(MailTemplate): - template = "member_request_merged.html" - subject = "Mitgliedskonto zusammengeführt // Member account merged" - - -class TaskFailedTemplate(MailTemplate): - template = "task_failed.html" - subject = "Aufgabe fehlgeschlagen // Task failed" - - -class MemberNegativeBalance(MailTemplate): - template = "member_negative_balance.html" - subject = "Deine ausstehenden Zahlungen // Your due payments" +from .templates import ( + UserConfirmEmailTemplate, + UserResetPasswordTemplate, + UserMovedInTemplate, + UserCreatedTemplate, + MemberRequestPendingTemplate, + MemberRequestDeniedTemplate, + MemberRequestMergedTemplate, + TaskFailedTemplate, + MemberNegativeBalance, +) def send_template_mails( diff --git a/pycroft/lib/mail/templates.py b/pycroft/lib/mail/templates.py new file mode 100644 index 000000000..6fa482a27 --- /dev/null +++ b/pycroft/lib/mail/templates.py @@ -0,0 +1,47 @@ +from .concepts import MailTemplate + + +class UserConfirmEmailTemplate(MailTemplate): + template = "user_confirm_email.html" + subject = "Bitte bestätige deine E-Mail Adresse // Please confirm your email address" + + +class UserResetPasswordTemplate(MailTemplate): + template = "user_reset_password.html" + subject = "Neues Passwort setzen // Set a new password" + + +class UserMovedInTemplate(MailTemplate): + template = "user_moved_in.html" + subject = "Wohnortänderung // Change of residence" + + +class UserCreatedTemplate(MailTemplate): + template = "user_created.html" + subject = "Willkommen bei der AG DSN // Welcome to the AG DSN" + + +class MemberRequestPendingTemplate(MailTemplate): + template = "member_request_pending.html" + subject = "Deine Mitgliedschaftsanfrage // Your member request" + + +class MemberRequestDeniedTemplate(MailTemplate): + template = "member_request_denied.html" + subject = "Mitgliedschaftsanfrage abgelehnt // Member request denied" + + +class MemberRequestMergedTemplate(MailTemplate): + template = "member_request_merged.html" + subject = "Mitgliedskonto zusammengeführt // Member account merged" + + +class TaskFailedTemplate(MailTemplate): + template = "task_failed.html" + subject = "Aufgabe fehlgeschlagen // Task failed" + + +class MemberNegativeBalance(MailTemplate): + template = "member_negative_balance.html" + subject = "Deine ausstehenden Zahlungen // Your due payments" + From e8b9324dec1f0ea3bea5110b863a11357ef57d72 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 22 Nov 2025 15:20:53 +0100 Subject: [PATCH 13/33] Mark templates as final --- pycroft/lib/mail/templates.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pycroft/lib/mail/templates.py b/pycroft/lib/mail/templates.py index 6fa482a27..609ae3afe 100644 --- a/pycroft/lib/mail/templates.py +++ b/pycroft/lib/mail/templates.py @@ -1,46 +1,56 @@ +import typing as t from .concepts import MailTemplate +@t.final class UserConfirmEmailTemplate(MailTemplate): template = "user_confirm_email.html" subject = "Bitte bestätige deine E-Mail Adresse // Please confirm your email address" +@t.final class UserResetPasswordTemplate(MailTemplate): template = "user_reset_password.html" subject = "Neues Passwort setzen // Set a new password" +@t.final class UserMovedInTemplate(MailTemplate): template = "user_moved_in.html" subject = "Wohnortänderung // Change of residence" +@t.final class UserCreatedTemplate(MailTemplate): template = "user_created.html" subject = "Willkommen bei der AG DSN // Welcome to the AG DSN" +@t.final class MemberRequestPendingTemplate(MailTemplate): template = "member_request_pending.html" subject = "Deine Mitgliedschaftsanfrage // Your member request" +@t.final class MemberRequestDeniedTemplate(MailTemplate): template = "member_request_denied.html" subject = "Mitgliedschaftsanfrage abgelehnt // Member request denied" +@t.final class MemberRequestMergedTemplate(MailTemplate): template = "member_request_merged.html" subject = "Mitgliedskonto zusammengeführt // Member account merged" +@t.final class TaskFailedTemplate(MailTemplate): template = "task_failed.html" subject = "Aufgabe fehlgeschlagen // Task failed" +@t.final class MemberNegativeBalance(MailTemplate): template = "member_negative_balance.html" subject = "Deine ausstehenden Zahlungen // Your due payments" From a55ded5caf1b21c46eb0d9bb0ed8abdc0184de3a Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 23 Nov 2025 13:59:41 +0100 Subject: [PATCH 14/33] Extract ssl context creation into separate method --- pycroft/lib/mail/submission.py | 43 +++++++++++++++++----------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/pycroft/lib/mail/submission.py b/pycroft/lib/mail/submission.py index 263dc810e..5f6e35ffc 100644 --- a/pycroft/lib/mail/submission.py +++ b/pycroft/lib/mail/submission.py @@ -34,34 +34,16 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: smtp_password = config.smtp_password smtp_ssl = config.smtp_ssl - use_ssl = smtp_ssl == 'ssl' - use_starttls = smtp_ssl == 'starttls' - ssl_context = None - - if use_ssl or use_starttls: - try: - ssl_context = ssl.create_default_context() - ssl_context.verify_mode = ssl.VerifyMode.CERT_REQUIRED - ssl_context.check_hostname = True - except ssl.SSLError as e: - # smtp.connect failed to connect - logger.critical('Unable to create ssl context', extra={ - 'trace': True, - 'data': {'exception_arguments': e.args} - }) - raise RetryableException from e - try: smtp: smtplib.SMTP - if use_ssl: - assert ssl_context is not None + if smtp_ssl == 'ssl': smtp = smtplib.SMTP_SSL(host=smtp_host, port=smtp_port, - context=ssl_context) + context=try_create_ssl_context()) else: smtp = smtplib.SMTP(host=smtp_host, port=smtp_port) - if use_starttls: - smtp.starttls(context=ssl_context) + if smtp_ssl == 'starttls': + smtp.starttls(context=try_create_ssl_context()) if smtp_user: assert smtp_password is not None @@ -112,6 +94,23 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: return failures == 0, failures +def try_create_ssl_context() -> ssl.SSLContext: + """ + :raises RetryableException: + """ + try: + ssl_context = ssl.create_default_context() + ssl_context.verify_mode = ssl.VerifyMode.CERT_REQUIRED + ssl_context.check_hostname = True + except ssl.SSLError as e: + # smtp.connect failed to connect + logger.critical('Unable to create ssl context', extra={ + 'trace': True, + 'data': {'exception_arguments': e.args} + }) + raise RetryableException from e + return ssl_context + class RetryableException(PycroftLibException): pass From 25996dc14fd8d4fb82619eb1a78923a05b4f16bc Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 23 Nov 2025 13:59:41 +0100 Subject: [PATCH 15/33] Promote `else` branch to outer scope --- pycroft/lib/mail/submission.py | 56 +++++++++++++++++----------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/pycroft/lib/mail/submission.py b/pycroft/lib/mail/submission.py index 5f6e35ffc..73b8b1bae 100644 --- a/pycroft/lib/mail/submission.py +++ b/pycroft/lib/mail/submission.py @@ -63,35 +63,35 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: ) raise RetryableException from e - else: - failures: int = 0 - - for mail in mails: - try: - mime_mail = mail.compose(from_=mail_from, default_reply_to=mail_reply_to) - assert mail_envelope_from is not None - smtp.sendmail(from_addr=mail_envelope_from, to_addrs=mail.to_address, - msg=mime_mail.as_string()) - except smtplib.SMTPException as e: - traceback.print_exc() - logger.critical( - 'Unable to send mail: "%s" to "%s": %s', mail.subject, mail.to_address, e, - extra={ - 'trace': True, - 'tags': {'mailserver': f"{smtp_host}:{smtp_host}"}, - 'data': {'exception_arguments': e.args, 'to': mail.to_address, - 'subject': mail.subject} - } - ) - failures += 1 - - smtp.close() - - logger.info('Tried to send mails (%i/%i succeeded)', len(mails) - failures, len(mails), extra={ - 'tags': {'mailserver': f"{smtp_host}:{smtp_host}"} - }) - return failures == 0, failures + failures: int = 0 + + for mail in mails: + try: + mime_mail = mail.compose(from_=mail_from, default_reply_to=mail_reply_to) + assert mail_envelope_from is not None + smtp.sendmail(from_addr=mail_envelope_from, to_addrs=mail.to_address, + msg=mime_mail.as_string()) + except smtplib.SMTPException as e: + traceback.print_exc() + logger.critical( + 'Unable to send mail: "%s" to "%s": %s', mail.subject, mail.to_address, e, + extra={ + 'trace': True, + 'tags': {'mailserver': f"{smtp_host}:{smtp_host}"}, + 'data': {'exception_arguments': e.args, 'to': mail.to_address, + 'subject': mail.subject} + } + ) + failures += 1 + + smtp.close() + + logger.info('Tried to send mails (%i/%i succeeded)', len(mails) - failures, len(mails), extra={ + 'tags': {'mailserver': f"{smtp_host}:{smtp_host}"} + }) + + return failures == 0, failures def try_create_ssl_context() -> ssl.SSLContext: From f99ba82cc0d54811db574310262b97a72010d67f Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 23 Nov 2025 13:59:41 +0100 Subject: [PATCH 16/33] Use `t.Literal` for smtp ssl type config string --- pycroft/lib/mail/config.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pycroft/lib/mail/config.py b/pycroft/lib/mail/config.py index e742176c2..a7299ccb3 100644 --- a/pycroft/lib/mail/config.py +++ b/pycroft/lib/mail/config.py @@ -6,6 +6,8 @@ import jinja2 from werkzeug.local import LocalProxy +type SmtpSslType = t.Literal['ssl', 'starttls'] | None + @dataclass class MailConfig: mail_envelope_from: str @@ -15,7 +17,7 @@ class MailConfig: smtp_user: str smtp_password: str smtp_port: int = field(default=465) - smtp_ssl: str = field(default="ssl") + smtp_ssl: SmtpSslType = field(default="ssl") template_path_type: InitVar[str | None] = None template_path: InitVar[str | None] = None @@ -37,6 +39,10 @@ def from_env(cls) -> t.Self: if (smtp_port := env.get("PYCROFT_SMTP_PORT")) is not None: config.smtp_port = int(smtp_port) if (smtp_ssl := env.get("PYCROFT_SMTP_SSL")) is not None: + if smtp_ssl not in ("ssl", "starttls"): + raise ValueError( + "PYCROFT_SMTP_SSL must be either 'ssl' or 'starttls' if set" + ) config.smtp_ssl = smtp_ssl return config From 8b02e886b939b457269e60288f11ec0f266a59c3 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 23 Nov 2025 13:59:41 +0100 Subject: [PATCH 17/33] Extract smtp setup into own method --- pycroft/lib/mail/submission.py | 71 ++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 30 deletions(-) diff --git a/pycroft/lib/mail/submission.py b/pycroft/lib/mail/submission.py index 73b8b1bae..52eb47b2c 100644 --- a/pycroft/lib/mail/submission.py +++ b/pycroft/lib/mail/submission.py @@ -5,7 +5,7 @@ from pycroft.lib.exc import PycroftLibException from .concepts import Mail -from .config import config +from .config import config, SmtpSslType logger = logging.getLogger(__name__) @@ -34,35 +34,7 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: smtp_password = config.smtp_password smtp_ssl = config.smtp_ssl - try: - smtp: smtplib.SMTP - if smtp_ssl == 'ssl': - smtp = smtplib.SMTP_SSL(host=smtp_host, port=smtp_port, - context=try_create_ssl_context()) - else: - smtp = smtplib.SMTP(host=smtp_host, port=smtp_port) - - if smtp_ssl == 'starttls': - smtp.starttls(context=try_create_ssl_context()) - - if smtp_user: - assert smtp_password is not None - smtp.login(smtp_user, smtp_password) - except (OSError, smtplib.SMTPException) as e: - traceback.print_exc() - - # smtp.connect failed to connect - logger.critical( - "Unable to connect to SMTP server: %s", - e, - extra={ - "trace": True, - "tags": {"mailserver": f"{smtp_host}:{smtp_host}"}, - "data": {"exception_arguments": e.args}, - }, - ) - - raise RetryableException from e + smtp = try_create_smtp(smtp_ssl, smtp_host, smtp_port, smtp_user, smtp_password) failures: int = 0 @@ -94,6 +66,45 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: return failures == 0, failures +def try_create_smtp( + smtp_ssl: SmtpSslType, smtp_host: str, smtp_port: int, smtp_user: str, smtp_password: str +) -> smtplib.SMTP: + """ + :raises RetryableException: + """ + try: + smtp: smtplib.SMTP + if smtp_ssl == "ssl": + smtp = smtplib.SMTP_SSL( + host=smtp_host, port=smtp_port, context=try_create_ssl_context() + ) + else: + smtp = smtplib.SMTP(host=smtp_host, port=smtp_port) + + if smtp_ssl == "starttls": + smtp.starttls(context=try_create_ssl_context()) + + if smtp_user: + assert smtp_password is not None + smtp.login(smtp_user, smtp_password) + return smtp + except (OSError, smtplib.SMTPException) as e: + traceback.print_exc() + + # smtp.connect failed to connect + logger.critical( + "Unable to connect to SMTP server: %s", + e, + extra={ + "trace": True, + "tags": {"mailserver": f"{smtp_host}:{smtp_host}"}, + "data": {"exception_arguments": e.args}, + }, + ) + + raise RetryableException from e + + def try_create_ssl_context() -> ssl.SSLContext: """ :raises RetryableException: From 974f780fff4b25954eeb6a1fc1fd9c4ba141ae27 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 23 Nov 2025 13:59:41 +0100 Subject: [PATCH 18/33] Inline variables only used once --- pycroft/lib/mail/submission.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pycroft/lib/mail/submission.py b/pycroft/lib/mail/submission.py index 52eb47b2c..5b35ac68c 100644 --- a/pycroft/lib/mail/submission.py +++ b/pycroft/lib/mail/submission.py @@ -30,11 +30,10 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: mail_reply_to = config.mail_reply_to smtp_host = config.smtp_host smtp_port = config.smtp_port - smtp_user = config.smtp_user - smtp_password = config.smtp_password - smtp_ssl = config.smtp_ssl - smtp = try_create_smtp(smtp_ssl, smtp_host, smtp_port, smtp_user, smtp_password) + smtp = try_create_smtp( + config.smtp_ssl, smtp_host, smtp_port, config.smtp_user, config.smtp_password + ) failures: int = 0 From 9d01534650bdb11e5f6cb17000fd82da0d359447 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 23 Nov 2025 13:59:41 +0100 Subject: [PATCH 19/33] Move assertion to outer scope --- pycroft/lib/mail/submission.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pycroft/lib/mail/submission.py b/pycroft/lib/mail/submission.py index 5b35ac68c..9135687e9 100644 --- a/pycroft/lib/mail/submission.py +++ b/pycroft/lib/mail/submission.py @@ -26,6 +26,7 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: raise RuntimeError("`mail.config` not set up!") mail_envelope_from = config.mail_envelope_from + assert mail_envelope_from is not None mail_from = config.mail_from mail_reply_to = config.mail_reply_to smtp_host = config.smtp_host @@ -40,7 +41,6 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: for mail in mails: try: mime_mail = mail.compose(from_=mail_from, default_reply_to=mail_reply_to) - assert mail_envelope_from is not None smtp.sendmail(from_addr=mail_envelope_from, to_addrs=mail.to_address, msg=mime_mail.as_string()) except smtplib.SMTPException as e: From 902fa8524c17147ebb223746da7da736901a747f Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 23 Nov 2025 13:59:41 +0100 Subject: [PATCH 20/33] Fix typos in log message (host:host makes no sense) --- pycroft/lib/mail/submission.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pycroft/lib/mail/submission.py b/pycroft/lib/mail/submission.py index 9135687e9..e57329ea4 100644 --- a/pycroft/lib/mail/submission.py +++ b/pycroft/lib/mail/submission.py @@ -49,7 +49,7 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: 'Unable to send mail: "%s" to "%s": %s', mail.subject, mail.to_address, e, extra={ 'trace': True, - 'tags': {'mailserver': f"{smtp_host}:{smtp_host}"}, + 'tags': {'mailserver': f"{smtp_host}:{smtp_port}"}, 'data': {'exception_arguments': e.args, 'to': mail.to_address, 'subject': mail.subject} } @@ -59,7 +59,7 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: smtp.close() logger.info('Tried to send mails (%i/%i succeeded)', len(mails) - failures, len(mails), extra={ - 'tags': {'mailserver': f"{smtp_host}:{smtp_host}"} + 'tags': {'mailserver': f"{smtp_host}:{smtp_port}"} }) return failures == 0, failures @@ -96,7 +96,7 @@ def try_create_smtp( e, extra={ "trace": True, - "tags": {"mailserver": f"{smtp_host}:{smtp_host}"}, + "tags": {"mailserver": f"{smtp_host}:{smtp_port}"}, "data": {"exception_arguments": e.args}, }, ) From 410fd0c3e52be919b8288b1f5d73fc1d7930d79f Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 23 Nov 2025 15:16:05 +0100 Subject: [PATCH 21/33] black pycroft.lib.mail --- pycroft/lib/mail/__init__.py | 12 +++++---- pycroft/lib/mail/concepts.py | 8 +++--- pycroft/lib/mail/config.py | 8 +++--- pycroft/lib/mail/submission.py | 45 ++++++++++++++++++++-------------- pycroft/lib/mail/templates.py | 3 +-- 5 files changed, 42 insertions(+), 34 deletions(-) diff --git a/pycroft/lib/mail/__init__.py b/pycroft/lib/mail/__init__.py index 70f67b137..4eab7d935 100644 --- a/pycroft/lib/mail/__init__.py +++ b/pycroft/lib/mail/__init__.py @@ -31,11 +31,13 @@ def send_template_mails( for addr in email_addresses: body_plain, body_html = template.render(**kwargs) - mail = Mail(to_name='', - to_address=addr, - subject=template.subject, - body_plain=body_plain, - body_html=body_html) + mail = Mail( + to_name="", + to_address=addr, + subject=template.subject, + body_plain=body_plain, + body_html=body_html, + ) mails.append(mail) from pycroft.task import send_mails_async diff --git a/pycroft/lib/mail/concepts.py b/pycroft/lib/mail/concepts.py index 074e93e11..753c15e21 100644 --- a/pycroft/lib/mail/concepts.py +++ b/pycroft/lib/mail/concepts.py @@ -10,6 +10,7 @@ from .config import config + @dataclass class Mail: to_name: str @@ -29,7 +30,6 @@ def body_html_mime(self) -> MIMEText | None: return None return MIMEText(self.body_html, "html", _charset="utf-8") - def compose(self, from_: str, default_reply_to: str | None) -> MIMEMultipart: msg = MIMEMultipart("alternative", _charset="utf-8") msg["Message-Id"] = make_msgid() @@ -53,7 +53,9 @@ class MailTemplate: subject: str args: dict - def __init__(self, loader: t.Callable[[str], jinja2.Template] | None = None, **kwargs: t.Any) -> None: + def __init__( + self, loader: t.Callable[[str], jinja2.Template] | None = None, **kwargs: t.Any + ) -> None: self.jinja_template: jinja2.Template = (loader or _get_template)(self.template) self.args = kwargs @@ -71,5 +73,3 @@ def _get_template(template_location: str) -> jinja2.Template: return config.template_env.get_template(template_location) except RuntimeError as e: raise RuntimeError("`mail.config` not set up!") from e - - diff --git a/pycroft/lib/mail/config.py b/pycroft/lib/mail/config.py index a7299ccb3..57e633ff7 100644 --- a/pycroft/lib/mail/config.py +++ b/pycroft/lib/mail/config.py @@ -6,7 +6,8 @@ import jinja2 from werkzeug.local import LocalProxy -type SmtpSslType = t.Literal['ssl', 'starttls'] | None +type SmtpSslType = t.Literal["ssl", "starttls"] | None + @dataclass class MailConfig: @@ -40,9 +41,7 @@ def from_env(cls) -> t.Self: config.smtp_port = int(smtp_port) if (smtp_ssl := env.get("PYCROFT_SMTP_SSL")) is not None: if smtp_ssl not in ("ssl", "starttls"): - raise ValueError( - "PYCROFT_SMTP_SSL must be either 'ssl' or 'starttls' if set" - ) + raise ValueError("PYCROFT_SMTP_SSL must be either 'ssl' or 'starttls' if set") config.smtp_ssl = smtp_ssl return config @@ -66,4 +65,3 @@ def __post_init__(self, template_path_type: str | None, template_path: str | Non _config_var: ContextVar[MailConfig] = ContextVar("config") config: MailConfig = LocalProxy(_config_var) # type: ignore[assignment] - diff --git a/pycroft/lib/mail/submission.py b/pycroft/lib/mail/submission.py index e57329ea4..dda9b8d7b 100644 --- a/pycroft/lib/mail/submission.py +++ b/pycroft/lib/mail/submission.py @@ -41,26 +41,36 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: for mail in mails: try: mime_mail = mail.compose(from_=mail_from, default_reply_to=mail_reply_to) - smtp.sendmail(from_addr=mail_envelope_from, to_addrs=mail.to_address, - msg=mime_mail.as_string()) + smtp.sendmail( + from_addr=mail_envelope_from, to_addrs=mail.to_address, msg=mime_mail.as_string() + ) except smtplib.SMTPException as e: traceback.print_exc() logger.critical( - 'Unable to send mail: "%s" to "%s": %s', mail.subject, mail.to_address, e, + 'Unable to send mail: "%s" to "%s": %s', + mail.subject, + mail.to_address, + e, extra={ - 'trace': True, - 'tags': {'mailserver': f"{smtp_host}:{smtp_port}"}, - 'data': {'exception_arguments': e.args, 'to': mail.to_address, - 'subject': mail.subject} - } + "trace": True, + "tags": {"mailserver": f"{smtp_host}:{smtp_port}"}, + "data": { + "exception_arguments": e.args, + "to": mail.to_address, + "subject": mail.subject, + }, + }, ) failures += 1 smtp.close() - logger.info('Tried to send mails (%i/%i succeeded)', len(mails) - failures, len(mails), extra={ - 'tags': {'mailserver': f"{smtp_host}:{smtp_port}"} - }) + logger.info( + "Tried to send mails (%i/%i succeeded)", + len(mails) - failures, + len(mails), + extra={"tags": {"mailserver": f"{smtp_host}:{smtp_port}"}}, + ) return failures == 0, failures @@ -69,7 +79,7 @@ def try_create_smtp( smtp_ssl: SmtpSslType, smtp_host: str, smtp_port: int, smtp_user: str, smtp_password: str ) -> smtplib.SMTP: """ - :raises RetryableException: + :raises RetryableException: """ try: smtp: smtplib.SMTP @@ -106,7 +116,7 @@ def try_create_smtp( def try_create_ssl_context() -> ssl.SSLContext: """ - :raises RetryableException: + :raises RetryableException: """ try: ssl_context = ssl.create_default_context() @@ -114,14 +124,13 @@ def try_create_ssl_context() -> ssl.SSLContext: ssl_context.check_hostname = True except ssl.SSLError as e: # smtp.connect failed to connect - logger.critical('Unable to create ssl context', extra={ - 'trace': True, - 'data': {'exception_arguments': e.args} - }) + logger.critical( + "Unable to create ssl context", + extra={"trace": True, "data": {"exception_arguments": e.args}}, + ) raise RetryableException from e return ssl_context class RetryableException(PycroftLibException): pass - diff --git a/pycroft/lib/mail/templates.py b/pycroft/lib/mail/templates.py index 609ae3afe..34c33b6d6 100644 --- a/pycroft/lib/mail/templates.py +++ b/pycroft/lib/mail/templates.py @@ -53,5 +53,4 @@ class TaskFailedTemplate(MailTemplate): @t.final class MemberNegativeBalance(MailTemplate): template = "member_negative_balance.html" - subject = "Deine ausstehenden Zahlungen // Your due payments" - + subject = "Deine ausstehenden Zahlungen // Your due payments" From 0475515335b9a75fe75dc7260bc0148066c1e644 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 23 Nov 2025 15:23:03 +0100 Subject: [PATCH 22/33] Use match statement for improved type inference --- pycroft/lib/mail/config.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pycroft/lib/mail/config.py b/pycroft/lib/mail/config.py index 57e633ff7..4216ff6e3 100644 --- a/pycroft/lib/mail/config.py +++ b/pycroft/lib/mail/config.py @@ -40,9 +40,11 @@ def from_env(cls) -> t.Self: if (smtp_port := env.get("PYCROFT_SMTP_PORT")) is not None: config.smtp_port = int(smtp_port) if (smtp_ssl := env.get("PYCROFT_SMTP_SSL")) is not None: - if smtp_ssl not in ("ssl", "starttls"): - raise ValueError("PYCROFT_SMTP_SSL must be either 'ssl' or 'starttls' if set") - config.smtp_ssl = smtp_ssl + match smtp_ssl: + case "ssl" | "starttls": + config.smtp_ssl = smtp_ssl + case _: + raise ValueError("PYCROFT_SMTP_SSL must be either 'ssl' or 'starttls' if set") return config From 27654ea7e91cd962cc51c3bf657ccc5d0e089f05 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 23 Nov 2025 15:28:42 +0100 Subject: [PATCH 23/33] Strengthen isolated `smtp.close()` to context manager usage --- pycroft/lib/mail/submission.py | 56 ++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/pycroft/lib/mail/submission.py b/pycroft/lib/mail/submission.py index dda9b8d7b..977d77bb3 100644 --- a/pycroft/lib/mail/submission.py +++ b/pycroft/lib/mail/submission.py @@ -32,38 +32,40 @@ def send_mails(mails: list[Mail]) -> tuple[bool, int]: smtp_host = config.smtp_host smtp_port = config.smtp_port - smtp = try_create_smtp( - config.smtp_ssl, smtp_host, smtp_port, config.smtp_user, config.smtp_password - ) failures: int = 0 - for mail in mails: - try: + with ( + smtp := try_create_smtp( + config.smtp_ssl, smtp_host, smtp_port, config.smtp_user, config.smtp_password + ) + ): + for mail in mails: mime_mail = mail.compose(from_=mail_from, default_reply_to=mail_reply_to) - smtp.sendmail( - from_addr=mail_envelope_from, to_addrs=mail.to_address, msg=mime_mail.as_string() - ) - except smtplib.SMTPException as e: - traceback.print_exc() - logger.critical( - 'Unable to send mail: "%s" to "%s": %s', - mail.subject, - mail.to_address, - e, - extra={ - "trace": True, - "tags": {"mailserver": f"{smtp_host}:{smtp_port}"}, - "data": { - "exception_arguments": e.args, - "to": mail.to_address, - "subject": mail.subject, + try: + smtp.sendmail( + from_addr=mail_envelope_from, + to_addrs=mail.to_address, + msg=mime_mail.as_string(), + ) + except smtplib.SMTPException as e: + traceback.print_exc() + logger.critical( + 'Unable to send mail: "%s" to "%s": %s', + mail.subject, + mail.to_address, + e, + extra={ + "trace": True, + "tags": {"mailserver": f"{smtp_host}:{smtp_port}"}, + "data": { + "exception_arguments": e.args, + "to": mail.to_address, + "subject": mail.subject, + }, }, - }, - ) - failures += 1 - - smtp.close() + ) + failures += 1 logger.info( "Tried to send mails (%i/%i succeeded)", From 7c6cdd002cb0ce6e4654a64f3590958ff2b71cb6 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 23 Nov 2025 15:38:10 +0100 Subject: [PATCH 24/33] Communicate parameter intent via assertions --- pycroft/lib/user/mail.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pycroft/lib/user/mail.py b/pycroft/lib/user/mail.py index 3af2ac19b..db1543ee0 100644 --- a/pycroft/lib/user/mail.py +++ b/pycroft/lib/user/mail.py @@ -68,6 +68,11 @@ def user_send_mails( :return: """ + assert (template is not None) ^ (body_plain is not None), \ + "user_send_mails should be called with either template or plain body" + assert (body_plain is not None) == (subject is not None), \ + "subject must be passed if and only if body is passed" + mails = [] for user in users: From 9bb513a7500c2970561e2bd7ad36e28009909aca Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 23 Nov 2025 15:48:47 +0100 Subject: [PATCH 25/33] Implement celery stubs for move_out reminder mail --- pycroft/lib/mail/templates.py | 6 +++++ pycroft/task.py | 26 ++++++++++++++++++- pycroft/templates/mail/move_out_reminder.html | 4 +-- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/pycroft/lib/mail/templates.py b/pycroft/lib/mail/templates.py index 34c33b6d6..a45f546e5 100644 --- a/pycroft/lib/mail/templates.py +++ b/pycroft/lib/mail/templates.py @@ -54,3 +54,9 @@ class TaskFailedTemplate(MailTemplate): class MemberNegativeBalance(MailTemplate): template = "member_negative_balance.html" subject = "Deine ausstehenden Zahlungen // Your due payments" + + +@t.final +class MoveOutReminder(MailTemplate): + template = "move_out_reminder.html" + subject = "Deine ausstehenden Zahlungen // Your due payments" diff --git a/pycroft/task.py b/pycroft/task.py index 013be3b1a..f0a3b1beb 100644 --- a/pycroft/task.py +++ b/pycroft/task.py @@ -11,7 +11,7 @@ import os import sys import typing as t -from datetime import timedelta +from datetime import timedelta, date import sentry_sdk from celery import Celery, Task as CeleryTask @@ -34,6 +34,7 @@ _config_var, MailConfig, ) +from pycroft.lib.mail.templates import MoveOutReminder from pycroft.lib.task import get_task_implementation, get_scheduled_tasks from pycroft.lib.traffic import delete_old_traffic_data from pycroft.model import session @@ -42,6 +43,7 @@ from pycroft.model.task import TaskStatus from scripts.connection import try_create_connection + if dsn := os.getenv('PYCROFT_SENTRY_DSN'): logging_integration = LoggingIntegration( level=logging.INFO, # INFO / WARN create breadcrumbs, just as SQL queries @@ -204,6 +206,28 @@ def mail_negative_members(): send_mails_async.delay([mail]) +@app.task(base=DBTask) +def mail_soon_to_move_out_members(): + from pycroft.lib.user import user_send_mails + contract_end = contract_end_reminder_date() + user_send_mails( + get_members_with_contract_end_at(contract_end), + template=MoveOutReminder(), + contract_end=contract_end + ) + + +# TODO move to pycroft.lib.user or somwhere else suitable +def get_members_with_contract_end_at(date: date): + # TODO implement + pass + + +# TODO move to pycroft.lib.user or somwhere else suitable +def contract_end_reminder_date(): + return date.today() + timedelta(days=7) + + @app.task(ignore_result=True, rate_limit=1, bind=True) def send_mails_async(self, mails: list[Mail]): success = False diff --git a/pycroft/templates/mail/move_out_reminder.html b/pycroft/templates/mail/move_out_reminder.html index cee367a91..f2830ef35 100644 --- a/pycroft/templates/mail/move_out_reminder.html +++ b/pycroft/templates/mail/move_out_reminder.html @@ -2,7 +2,7 @@ {%- block body -%} * English version below * -Hallo {{ user_name }}, +Hallo {{ user.name }}, Unserer Kenntnis nach endet dein Mietvertrag am {{ contract_end }}. Deine Mitgliedschaft bei der AG DSN endet *nicht automatisch*. @@ -21,7 +21,7 @@ * English version * -Hello {{ user_name }}, +Hello {{ user.name }}, To our knowledge, your rental agreement ends on {{ contract_end }}. Your membership with AG DSN does *not* end automatically. From c6565b7412ac1642855f34522cb8e9e0bf6b0014 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 23 Nov 2025 15:48:47 +0100 Subject: [PATCH 26/33] Add helpful assertion to docstring Ideally, all of this would be typed sufficiently so that an error like this could not happen. --- pycroft/lib/user/mail.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pycroft/lib/user/mail.py b/pycroft/lib/user/mail.py index db1543ee0..f0ad1595c 100644 --- a/pycroft/lib/user/mail.py +++ b/pycroft/lib/user/mail.py @@ -58,6 +58,7 @@ def user_send_mails( :param users: Users who should receive the mail :param template: The template that should be used. Can be None if body_plain is supplied. + if supplied, must take a `user` parameter. :param soft_fail: Do not raise an exception if a user does not have an email and use_internal is set to True :param use_internal: If internal mail addresses can be used (@agdsn.me) From ea57b5ffc10ae833c1ceba162f2afcf3ef4573b2 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Tue, 25 Nov 2025 19:38:30 +0100 Subject: [PATCH 27/33] Move stub functions to lib.user.mail --- pycroft/lib/user/mail.py | 13 +++++++++++++ pycroft/task.py | 18 +++++------------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/pycroft/lib/user/mail.py b/pycroft/lib/user/mail.py index f0ad1595c..a426ea616 100644 --- a/pycroft/lib/user/mail.py +++ b/pycroft/lib/user/mail.py @@ -1,5 +1,6 @@ import os import typing as t +from datetime import date, timedelta from sqlalchemy import select, ScalarResult from sqlalchemy.orm import Session @@ -212,3 +213,15 @@ def send_password_reset_mail(user: User) -> bool: return False return True + + +# TODO move to pycroft.lib.user or somwhere else suitable +def get_members_with_contract_end_at(session: Session, date: date): + # TODO implement + pass + + +# TODO move to pycroft.lib.user or somwhere else suitable +def contract_end_reminder_date(session: Session): + return date.today() + timedelta(days=7) + diff --git a/pycroft/task.py b/pycroft/task.py index f0a3b1beb..7d3e67ca6 100644 --- a/pycroft/task.py +++ b/pycroft/task.py @@ -209,25 +209,17 @@ def mail_negative_members(): @app.task(base=DBTask) def mail_soon_to_move_out_members(): from pycroft.lib.user import user_send_mails - contract_end = contract_end_reminder_date() + from pycroft.lib.user.mail import contract_end_reminder_date, get_members_with_contract_end_at + sess = t.cast(Session, session.session) + contract_end = contract_end_reminder_date(sess) + user_send_mails( - get_members_with_contract_end_at(contract_end), + get_members_with_contract_end_at(sess, contract_end), template=MoveOutReminder(), contract_end=contract_end ) -# TODO move to pycroft.lib.user or somwhere else suitable -def get_members_with_contract_end_at(date: date): - # TODO implement - pass - - -# TODO move to pycroft.lib.user or somwhere else suitable -def contract_end_reminder_date(): - return date.today() + timedelta(days=7) - - @app.task(ignore_result=True, rate_limit=1, bind=True) def send_mails_async(self, mails: list[Mail]): success = False From bb3f99e360a6c69d4ff414b679a43546e32953a4 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Tue, 25 Nov 2025 19:38:30 +0100 Subject: [PATCH 28/33] Make celery task implementation testable and move to lib --- pycroft/lib/user/mail.py | 19 +++++++++++++++++-- pycroft/task.py | 12 ++---------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/pycroft/lib/user/mail.py b/pycroft/lib/user/mail.py index a426ea616..56ae41e9a 100644 --- a/pycroft/lib/user/mail.py +++ b/pycroft/lib/user/mail.py @@ -9,9 +9,12 @@ from pycroft.lib.mail import ( MailTemplate, Mail, +) +from pycroft.lib.mail.templates import ( + MemberRequestMergedTemplate, + MoveOutReminder, UserConfirmEmailTemplate, UserResetPasswordTemplate, - MemberRequestMergedTemplate, ) from pycroft.model import session from pycroft.model.session import with_transaction @@ -52,6 +55,7 @@ def user_send_mails( use_internal: bool = True, body_plain: str | None = None, subject: str | None = None, + send_mails: t.Callable[[list[Mail]], None] | None = None, **kwargs: t.Any, ) -> None: """ @@ -120,7 +124,7 @@ def user_send_mails( ) mails.append(mail) - send_mails_async.delay(mails) + (send_mails or send_mails_async.delay)(mails) def user_send_mail( @@ -215,6 +219,17 @@ def send_password_reset_mail(user: User) -> bool: return True +def mail_soon_to_move_out_members(session: Session, send_mails: t.Callable[[list[Mail]], None]): + """Dependency-free implementation of the celery task of the same name.""" + contract_end = contract_end_reminder_date(session) + user_send_mails( + get_members_with_contract_end_at(session, contract_end), + template=MoveOutReminder(), + contract_end=contract_end, + send_mails=send_mails, + ) + + # TODO move to pycroft.lib.user or somwhere else suitable def get_members_with_contract_end_at(session: Session, date: date): # TODO implement diff --git a/pycroft/task.py b/pycroft/task.py index 7d3e67ca6..682446049 100644 --- a/pycroft/task.py +++ b/pycroft/task.py @@ -208,16 +208,8 @@ def mail_negative_members(): @app.task(base=DBTask) def mail_soon_to_move_out_members(): - from pycroft.lib.user import user_send_mails - from pycroft.lib.user.mail import contract_end_reminder_date, get_members_with_contract_end_at - sess = t.cast(Session, session.session) - contract_end = contract_end_reminder_date(sess) - - user_send_mails( - get_members_with_contract_end_at(sess, contract_end), - template=MoveOutReminder(), - contract_end=contract_end - ) + from pycroft.lib.user.mail import mail_soon_to_move_out_members as impl + impl(t.cast(Session, session.session), send_mails_async.delay) @app.task(ignore_result=True, rate_limit=1, bind=True) From 875ad2d8d3d9584057102afd4128988dc5bd2595 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Tue, 25 Nov 2025 20:24:43 +0100 Subject: [PATCH 29/33] Naively implement get_members_with_contract_end_at --- pycroft/lib/user/mail.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pycroft/lib/user/mail.py b/pycroft/lib/user/mail.py index 56ae41e9a..c9b3ebb73 100644 --- a/pycroft/lib/user/mail.py +++ b/pycroft/lib/user/mail.py @@ -26,6 +26,7 @@ PropertyGroup, ) from pycroft.model.facilities import Building, Room +from pycroft.model.swdd import Tenancy from pycroft.task import send_mails_async from .user_id import ( @@ -230,13 +231,11 @@ def mail_soon_to_move_out_members(session: Session, send_mails: t.Callable[[list ) -# TODO move to pycroft.lib.user or somwhere else suitable -def get_members_with_contract_end_at(session: Session, date: date): - # TODO implement - pass +def get_members_with_contract_end_at(session: Session, date: date) -> ScalarResult[User]: + stmt = select(User).outerjoin(User.tenancies).where(Tenancy.mietende == date) + return session.scalars(stmt) -# TODO move to pycroft.lib.user or somwhere else suitable def contract_end_reminder_date(session: Session): return date.today() + timedelta(days=7) From ac27985d1a09a4999e3e93ab36bc83be16c00714 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 29 Nov 2025 11:08:16 +0100 Subject: [PATCH 30/33] Add missing `_config_var` import in mail package This was used somewhere; pytest startup failed otherwise. --- pycroft/lib/mail/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pycroft/lib/mail/__init__.py b/pycroft/lib/mail/__init__.py index 4eab7d935..23aa2863c 100644 --- a/pycroft/lib/mail/__init__.py +++ b/pycroft/lib/mail/__init__.py @@ -8,7 +8,7 @@ from .concepts import Mail, MailTemplate -from .config import MailConfig, config +from .config import MailConfig, config, _config_var from .submission import send_mails, RetryableException from .templates import ( UserConfirmEmailTemplate, From e7c43a840413046f63425aa98cbab3f23caabb1e Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 29 Nov 2025 11:08:16 +0100 Subject: [PATCH 31/33] Only show tenancies of users whose last eom is None --- pycroft/lib/user/mail.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/pycroft/lib/user/mail.py b/pycroft/lib/user/mail.py index c9b3ebb73..177b256d2 100644 --- a/pycroft/lib/user/mail.py +++ b/pycroft/lib/user/mail.py @@ -16,6 +16,7 @@ UserConfirmEmailTemplate, UserResetPasswordTemplate, ) +from pycroft.lib.membership import select_user_and_last_mem from pycroft.model import session from pycroft.model.session import with_transaction from pycroft.model.user import ( @@ -232,7 +233,18 @@ def mail_soon_to_move_out_members(session: Session, send_mails: t.Callable[[list def get_members_with_contract_end_at(session: Session, date: date) -> ScalarResult[User]: - stmt = select(User).outerjoin(User.tenancies).where(Tenancy.mietende == date) + """Select members whose contract ends at a given date. + + We only show users with an unbounded membership in the member_group. + """ + last_mem = select_user_and_last_mem().cte("last_mem") + stmt = ( + select(last_mem) + .join(User, last_mem.c.user_id == User.id) + .outerjoin(User.tenancies) + .where(Tenancy.mietende == date, last_mem.c.mem_end.is_(None)) + .with_only_columns(User) + ) return session.scalars(stmt) From 969c3ea65f045e8988b312caa1ba90c528ebeaa0 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 29 Nov 2025 11:08:16 +0100 Subject: [PATCH 32/33] Add POC unit test for tenancy end reminder selection (one positive case) --- tests/lib/user/test_mail.py | 83 ++++++++++++++++++++++++++++++++++++- 1 file changed, 81 insertions(+), 2 deletions(-) diff --git a/tests/lib/user/test_mail.py b/tests/lib/user/test_mail.py index 0ec9b6d68..0eaf3d414 100644 --- a/tests/lib/user/test_mail.py +++ b/tests/lib/user/test_mail.py @@ -1,12 +1,17 @@ # Copyright (c) 2025. The Pycroft Authors. See the AUTHORS file. # This file is part of the Pycroft project and licensed under the terms of # the Apache License, Version 2.0. See the LICENSE file for details +import typing as t +from datetime import datetime, date +from itertools import combinations + import pytest +from sqlalchemy.orm import Session +from pycroft import config +from pycroft.helpers.interval import closedopen from pycroft.lib.user import get_active_users_with_building from pycroft.model.facilities import Room, Building -from itertools import combinations - from pycroft.model.user import User from ...factories import UserFactory, RoomFactory, BuildingFactory, AddressFactory @@ -83,3 +88,77 @@ def test_not_found_in_building(self, session, config, build_map, user, building_ assert len(users.all()) == 1 users = get_active_users_with_building(session, [config.violation_group], building_list) assert len(users.all()) == 0 + + +class UserWithTenancyGen(t.Protocol): + def __call__( + self, + session: Session, + tenancy_begin: date, + tenancy_end: date, + mem_end: datetime | None = None, + ) -> User: ... + +@pytest.fixture(scope="function") +def gen_user(config) -> UserWithTenancyGen: + def gen( + session: Session, tenancy_begin: date, tenancy_end: date, mem_end: datetime | None = None + ) -> User: + u = t.cast( + User, + UserFactory.create( + with_membership=True, + registered_at=tenancy_begin, + membership__active_during=closedopen(tenancy_begin, mem_end), + membership__group=config.member_group, + ), + ) + u.swdd_person_id = u.id + 10000 + session.add(u) + session.flush() + from sqlalchemy import Table, MetaData, Column, Integer, Text, Date + from sqlalchemy.sql import insert, text + + swdd_vv = Table( + "swdd_vv", + MetaData(), + Column("persvv_id", Integer, nullable=False), + Column("person_id", Integer, nullable=False), + Column("vo_suchname", Text, nullable=False), + Column("person_hash", Text, nullable=False), + Column("mietbeginn", Date, nullable=False), + Column("mietende", Date, nullable=False), + Column("status_id", Integer, nullable=False), + schema="swdd", + ) + assert u.room + session.execute( + swdd_vv.insert().values( + persvv_id=u.id + 10000, + person_id=u.id + 10000, + vo_suchname=u.room.swdd_vo_suchname, + person_hash="", + mietbeginn=tenancy_begin.isoformat(), + mietende=tenancy_end.isoformat(), + status_id=1, + ) + ) + session.execute(text("refresh materialized view swdd_vv")) + return u + + return gen + +def test_move_out_reminder(session, config, gen_user: UserWithTenancyGen): + u = gen_user( + session, + date(2019, 10, 1), + date(2020, 9, 30), + None, + ) + session.flush() + + # TODO fixture: member with contract end at 2020-01-08 + # TODO fixture: member with the former, but also + from pycroft.lib.user.mail import get_members_with_contract_end_at + m = get_members_with_contract_end_at(session, date(2020, 9, 30)) + assert m.all() == [u] From 03f553e91270ff1570eba47e6c4c84618cda9bf5 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 29 Nov 2025 11:08:16 +0100 Subject: [PATCH 33/33] Move test to separate test file --- tests/lib/user/test_mail.py | 79 -------------------- tests/lib/user/test_move_out_reminder.py | 92 ++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 79 deletions(-) create mode 100644 tests/lib/user/test_move_out_reminder.py diff --git a/tests/lib/user/test_mail.py b/tests/lib/user/test_mail.py index 0eaf3d414..b3eda28e4 100644 --- a/tests/lib/user/test_mail.py +++ b/tests/lib/user/test_mail.py @@ -1,15 +1,10 @@ # Copyright (c) 2025. The Pycroft Authors. See the AUTHORS file. # This file is part of the Pycroft project and licensed under the terms of # the Apache License, Version 2.0. See the LICENSE file for details -import typing as t -from datetime import datetime, date from itertools import combinations import pytest -from sqlalchemy.orm import Session -from pycroft import config -from pycroft.helpers.interval import closedopen from pycroft.lib.user import get_active_users_with_building from pycroft.model.facilities import Room, Building from pycroft.model.user import User @@ -88,77 +83,3 @@ def test_not_found_in_building(self, session, config, build_map, user, building_ assert len(users.all()) == 1 users = get_active_users_with_building(session, [config.violation_group], building_list) assert len(users.all()) == 0 - - -class UserWithTenancyGen(t.Protocol): - def __call__( - self, - session: Session, - tenancy_begin: date, - tenancy_end: date, - mem_end: datetime | None = None, - ) -> User: ... - -@pytest.fixture(scope="function") -def gen_user(config) -> UserWithTenancyGen: - def gen( - session: Session, tenancy_begin: date, tenancy_end: date, mem_end: datetime | None = None - ) -> User: - u = t.cast( - User, - UserFactory.create( - with_membership=True, - registered_at=tenancy_begin, - membership__active_during=closedopen(tenancy_begin, mem_end), - membership__group=config.member_group, - ), - ) - u.swdd_person_id = u.id + 10000 - session.add(u) - session.flush() - from sqlalchemy import Table, MetaData, Column, Integer, Text, Date - from sqlalchemy.sql import insert, text - - swdd_vv = Table( - "swdd_vv", - MetaData(), - Column("persvv_id", Integer, nullable=False), - Column("person_id", Integer, nullable=False), - Column("vo_suchname", Text, nullable=False), - Column("person_hash", Text, nullable=False), - Column("mietbeginn", Date, nullable=False), - Column("mietende", Date, nullable=False), - Column("status_id", Integer, nullable=False), - schema="swdd", - ) - assert u.room - session.execute( - swdd_vv.insert().values( - persvv_id=u.id + 10000, - person_id=u.id + 10000, - vo_suchname=u.room.swdd_vo_suchname, - person_hash="", - mietbeginn=tenancy_begin.isoformat(), - mietende=tenancy_end.isoformat(), - status_id=1, - ) - ) - session.execute(text("refresh materialized view swdd_vv")) - return u - - return gen - -def test_move_out_reminder(session, config, gen_user: UserWithTenancyGen): - u = gen_user( - session, - date(2019, 10, 1), - date(2020, 9, 30), - None, - ) - session.flush() - - # TODO fixture: member with contract end at 2020-01-08 - # TODO fixture: member with the former, but also - from pycroft.lib.user.mail import get_members_with_contract_end_at - m = get_members_with_contract_end_at(session, date(2020, 9, 30)) - assert m.all() == [u] diff --git a/tests/lib/user/test_move_out_reminder.py b/tests/lib/user/test_move_out_reminder.py new file mode 100644 index 000000000..a512064bb --- /dev/null +++ b/tests/lib/user/test_move_out_reminder.py @@ -0,0 +1,92 @@ +# Copyright (c) 2025. The Pycroft Authors. See the AUTHORS file. +# This file is part of the Pycroft project and licensed under the terms of +# the Apache License, Version 2.0. See the LICENSE file for details +from __future__ import annotations + +import typing as t +from datetime import datetime, date + +import pytest +from sqlalchemy.orm import Session + +from pycroft.helpers.interval import closedopen +from pycroft.lib.user.mail import get_members_with_contract_end_at +from pycroft.model.user import User +from ...factories import UserFactory + +def test_move_out_reminder(session, config, gen_user: UserWithTenancyGen): + u = gen_user( + session, + date(2019, 10, 1), + date(2020, 9, 30), + None, + ) + session.flush() + + # TODO fixture: member with contract end at 2020-01-08 + # TODO fixture: member with the former, but also + + m = get_members_with_contract_end_at(session, date(2020, 9, 30)) + assert m.all() == [u] + + + +class UserWithTenancyGen(t.Protocol): + def __call__( + self, + session: Session, + tenancy_begin: date, + tenancy_end: date, + mem_end: datetime | None = None, + ) -> User: ... + + +@pytest.fixture(scope="function") +def gen_user(config) -> UserWithTenancyGen: + def gen( + session: Session, tenancy_begin: date, tenancy_end: date, mem_end: datetime | None = None + ) -> User: + u = t.cast( + User, + UserFactory.create( + with_membership=True, + registered_at=tenancy_begin, + membership__active_during=closedopen(tenancy_begin, mem_end), + membership__group=config.member_group, + ), + ) + u.swdd_person_id = u.id + 10000 + session.add(u) + session.flush() + from sqlalchemy import Table, MetaData, Column, Integer, Text, Date + from sqlalchemy.sql import text + + swdd_vv = Table( + "swdd_vv", + MetaData(), + Column("persvv_id", Integer, nullable=False), + Column("person_id", Integer, nullable=False), + Column("vo_suchname", Text, nullable=False), + Column("person_hash", Text, nullable=False), + Column("mietbeginn", Date, nullable=False), + Column("mietende", Date, nullable=False), + Column("status_id", Integer, nullable=False), + schema="swdd", + ) + assert u.room + session.execute( + swdd_vv.insert().values( + persvv_id=u.id + 10000, + person_id=u.id + 10000, + vo_suchname=u.room.swdd_vo_suchname, + person_hash="", + mietbeginn=tenancy_begin.isoformat(), + mietende=tenancy_end.isoformat(), + status_id=1, + ) + ) + session.execute(text("refresh materialized view swdd_vv")) + return u + + return gen +