From 6b0da82b45f0004b07661d1888ce4b36ff76c2ca Mon Sep 17 00:00:00 2001 From: Marcel Peterkau Date: Sat, 15 Aug 2026 04:17:40 +0200 Subject: [PATCH 1/6] Add configurable e-mail delivery: direct SMTP send or IMAP drafts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mahnungs- and SEPA-info-mails could previously only be saved as a local .eml file that still had to be manually imported into Thunderbird. Add a per-store "E-Mail-Versand" configuration (Optionen -> E-Mail-Versand, stored in repository.json alongside the rest of the club's settings, since different stores may use different mailboxes) with four delivery modes: - "Lokal speichern": today's behaviour, unchanged default for existing stores. - "Direkt versenden": sends via SMTP. - "Als Entwurf ablegen": IMAP APPENDs into a configurable drafts folder, so it shows up live in whatever mail client is already watching that account. - "Jedes Mal fragen": prompts once per generation action (not per e-mail -- a SEPA batch can cover dozens of members) with Senden/Entwürfe/Abbrechen. New ccma.services.mail_delivery module (smtplib/imaplib, no new dependency) opens one authenticated connection per batch and reuses it across all messages instead of reconnecting per recipient. Both "Verbindung testen" buttons in Options exercise the same connection path used for real delivery. The archived per-member copy of every generated e-mail is unaffected and still always written regardless of delivery mode. Co-Authored-By: Claude Sonnet 5 --- src/ccma/services/mail_delivery.py | 110 ++++++++++++++++ src/ccma/services/reminder_mail.py | 45 +++++-- src/ccma/services/sepa_mail.py | 138 ++++++++++++-------- src/ccma/storage/repository.py | 101 +++++++++++++++ src/ccma/ui/claim_tab.py | 33 +++-- src/ccma/ui/email_delivery_dialog.py | 68 ++++++++++ src/ccma/ui/labels.py | 13 ++ src/ccma/ui/options_dialog.py | 183 ++++++++++++++++++++++++++- src/ccma/ui/sepa_dialog.py | 34 +++-- tests/test_email_settings.py | 103 +++++++++++++++ tests/test_mail_delivery.py | 171 +++++++++++++++++++++++++ tests/test_reminder_mail.py | 56 ++++++++ tests/test_sepa.py | 63 +++++++++ 13 files changed, 1030 insertions(+), 88 deletions(-) create mode 100644 src/ccma/services/mail_delivery.py create mode 100644 src/ccma/ui/email_delivery_dialog.py create mode 100644 tests/test_email_settings.py create mode 100644 tests/test_mail_delivery.py diff --git a/src/ccma/services/mail_delivery.py b/src/ccma/services/mail_delivery.py new file mode 100644 index 0000000..344671a --- /dev/null +++ b/src/ccma/services/mail_delivery.py @@ -0,0 +1,110 @@ +from __future__ import annotations + +import imaplib +import smtplib +import ssl +from collections.abc import Iterator +from contextlib import contextmanager, suppress +from email.parser import BytesParser +from email.policy import default as email_default_policy + +from ccma.storage.repository import RepositoryError + + +def _ssl_context() -> ssl.SSLContext: + return ssl.create_default_context() + + +def _smtp_client(settings: dict) -> smtplib.SMTP: + host = str(settings.get("smtp_host", "")).strip() + if not host: + raise RepositoryError("Kein SMTP-Server konfiguriert.") + port = int(settings.get("smtp_port", 587)) + security = str(settings.get("smtp_security", "starttls")) + client = ( + smtplib.SMTP_SSL(host, port, context=_ssl_context(), timeout=30) + if security == "ssl" + else smtplib.SMTP(host, port, timeout=30) + ) + if security == "starttls": + client.starttls(context=_ssl_context()) + username = str(settings.get("smtp_username", "")).strip() + if username: + client.login(username, str(settings.get("smtp_password", ""))) + return client + + +def _imap_client(settings: dict) -> imaplib.IMAP4: + host = str(settings.get("imap_host", "")).strip() + if not host: + raise RepositoryError("Kein IMAP-Server konfiguriert.") + port = int(settings.get("imap_port", 993)) + security = str(settings.get("imap_security", "ssl")) + client = ( + imaplib.IMAP4_SSL(host, port, timeout=30) + if security == "ssl" + else imaplib.IMAP4(host, port, timeout=30) + ) + if security == "starttls": + client.starttls(_ssl_context()) + username = str(settings.get("imap_username", "")).strip() + if username: + client.login(username, str(settings.get("imap_password", ""))) + return client + + +@contextmanager +def smtp_session(settings: dict) -> Iterator[smtplib.SMTP]: + """Opens one authenticated SMTP connection to reuse across several sends (a SEPA + info-mail batch may cover dozens of members -- reconnecting/re-authenticating per + message would be slow and can trigger provider rate limits).""" + try: + client = _smtp_client(settings) + except (OSError, smtplib.SMTPException) as exc: + raise RepositoryError(f"SMTP-Verbindung fehlgeschlagen: {exc}") from exc + try: + yield client + finally: + with suppress(OSError, smtplib.SMTPException): + client.quit() + + +@contextmanager +def imap_session(settings: dict) -> Iterator[imaplib.IMAP4]: + try: + client = _imap_client(settings) + except (OSError, imaplib.IMAP4.error) as exc: + raise RepositoryError(f"IMAP-Verbindung fehlgeschlagen: {exc}") from exc + try: + yield client + finally: + with suppress(OSError, imaplib.IMAP4.error): + client.logout() + + +def send_via_smtp(client: smtplib.SMTP, message_bytes: bytes) -> None: + message = BytesParser(policy=email_default_policy).parsebytes(message_bytes) + try: + client.send_message(message) + except (OSError, smtplib.SMTPException) as exc: + raise RepositoryError(f"E-Mail konnte nicht versandt werden: {exc}") from exc + + +def append_to_imap_drafts(client: imaplib.IMAP4, message_bytes: bytes, *, folder: str) -> None: + try: + status, response = client.append(folder, r"(\Draft)", None, message_bytes) + except (OSError, imaplib.IMAP4.error) as exc: + raise RepositoryError(f"E-Mail konnte nicht als Entwurf abgelegt werden: {exc}") from exc + if status != "OK": + detail = response[0].decode("utf-8", "replace") if response and response[0] else status + raise RepositoryError(f"IMAP-Server hat die Ablage im Ordner „{folder}“ abgelehnt: {detail}") + + +def test_smtp_connection(settings: dict) -> None: + with smtp_session(settings): + pass + + +def test_imap_connection(settings: dict) -> None: + with imap_session(settings): + pass diff --git a/src/ccma/services/reminder_mail.py b/src/ccma/services/reminder_mail.py index 616da47..be714ff 100644 --- a/src/ccma/services/reminder_mail.py +++ b/src/ccma/services/reminder_mail.py @@ -12,12 +12,15 @@ from pathlib import Path from ccma.domain.contributions import claim_balance from ccma.domain.dates import format_date_for_display +from ccma.services.mail_delivery import append_to_imap_drafts, imap_session, send_via_smtp, smtp_session from ccma.storage.repository import MemberRepository, RepositoryError @dataclass(frozen=True, slots=True) class GeneratedReminderMail: - export_path: Path + # None for "send"/"drafts" delivery -- there is no local file, the mail was + # handed straight to the mail server. Always set for "local" delivery. + export_path: Path | None archive_path: Path sha256: str @@ -115,11 +118,14 @@ def generate_and_send_reminder_mail( claim_id: str, reminder_id: str, *, - output_path: Path | str, + delivery_mode: str, + output_path: Path | str | None = None, sender_name: str, sender_email: str, signature: str, ) -> GeneratedReminderMail: + if delivery_mode not in {"local", "send", "drafts"}: + raise RepositoryError(f"Unbekannter Versandmodus: {delivery_mode}") member = repository.get_member(member_id) data, claim = repository.get_claim(member_id, claim_id) reminder = repository._find_reminder(data, claim_id, reminder_id) @@ -129,10 +135,14 @@ def generate_and_send_reminder_mail( raise RepositoryError("Für das Mitglied ist keine E-Mail-Adresse hinterlegt.") if not sender_email.strip() or "@" not in sender_email: raise RepositoryError("Für die Mahnung ist eine gültige Absenderadresse erforderlich.") - target = Path(output_path) - target.parent.mkdir(parents=True, exist_ok=True) - if target.exists(): - raise RepositoryError(f"Die Ausgabedatei existiert bereits: {target}") + target: Path | None = None + if delivery_mode == "local": + if not output_path: + raise RepositoryError("Kein Zielpfad für die lokale Ablage angegeben.") + target = Path(output_path) + target.parent.mkdir(parents=True, exist_ok=True) + if target.exists(): + raise RepositoryError(f"Die Ausgabedatei existiert bereits: {target}") sent_reminder = repository.mark_reminder_sent(member_id, claim_id, reminder_id) updated_data, updated_claim = repository.get_claim(member_id, claim_id) @@ -155,15 +165,27 @@ def generate_and_send_reminder_mail( f"{_safe_filename(str(sent_reminder.get('name', 'Mahnung')))}.eml" ) archive_path = _available_path(archive_dir, filename) - export_temp = target.with_name(f".{target.name}.tmp") archive_temp = archive_path.with_name(f".{archive_path.name}.tmp") + export_path: Path | None = None try: - export_temp.write_bytes(content) archive_temp.write_bytes(content) - os.replace(export_temp, target) + if delivery_mode == "local": + export_temp = target.with_name(f".{target.name}.tmp") + try: + export_temp.write_bytes(content) + os.replace(export_temp, target) + finally: + export_temp.unlink(missing_ok=True) + export_path = target + elif delivery_mode == "send": + with smtp_session(repository.get_email_settings()) as client: + send_via_smtp(client, content) + else: + settings = repository.get_email_settings() + with imap_session(settings) as client: + append_to_imap_drafts(client, content, folder=settings["imap_drafts_folder"]) os.replace(archive_temp, archive_path) finally: - export_temp.unlink(missing_ok=True) archive_temp.unlink(missing_ok=True) relative_path = archive_path.relative_to( repository.members_root / member_id / "files" @@ -184,6 +206,7 @@ def generate_and_send_reminder_mail( "recipient": member.email, "sha256": digest, "payment_deadline": str(sent_reminder.get("payment_deadline", "")), + "delivery_mode": delivery_mode, }, ) - return GeneratedReminderMail(target, archive_path, digest) + return GeneratedReminderMail(export_path, archive_path, digest) diff --git a/src/ccma/services/sepa_mail.py b/src/ccma/services/sepa_mail.py index b5b791a..f067560 100644 --- a/src/ccma/services/sepa_mail.py +++ b/src/ccma/services/sepa_mail.py @@ -2,6 +2,7 @@ from __future__ import annotations import hashlib import re +from contextlib import nullcontext from dataclasses import dataclass from datetime import date, datetime from email.message import EmailMessage @@ -9,6 +10,7 @@ from email.policy import SMTP from email.utils import format_datetime, make_msgid from pathlib import Path +from ccma.services.mail_delivery import append_to_imap_drafts, imap_session, send_via_smtp, smtp_session from ccma.services.sepa import DirectDebit from ccma.storage.repository import MemberRepository, RepositoryError @@ -17,7 +19,8 @@ from ccma.storage.repository import MemberRepository, RepositoryError class GeneratedDebitMail: member_id: str recipient: str - export_path: Path + # None for "send"/"drafts" delivery -- see reminder_mail.GeneratedReminderMail. + export_path: Path | None archive_path: Path @@ -94,67 +97,94 @@ def generate_debit_mails( debits: list[DirectDebit], *, collection_date: date, - output_directory: Path | str, + delivery_mode: str, + output_directory: Path | str | None = None, sender_name: str, sender_email: str, signature: str, ) -> tuple[list[GeneratedDebitMail], list[str]]: - output = Path(output_directory) - output.mkdir(parents=True, exist_ok=True) + output: Path | None = None + email_settings: dict | None = None + if delivery_mode == "local": + if not output_directory: + raise RepositoryError("Kein Zielordner für die lokale Ablage angegeben.") + output = Path(output_directory) + output.mkdir(parents=True, exist_ok=True) + session = nullcontext(None) + elif delivery_mode == "send": + email_settings = repository.get_email_settings() + session = smtp_session(email_settings) + elif delivery_mode == "drafts": + email_settings = repository.get_email_settings() + session = imap_session(email_settings) + else: + raise RepositoryError(f"Unbekannter Versandmodus: {delivery_mode}") + organization = repository.get_configuration().get("organization") or {} creditor_id = str(organization.get("creditor_id", "")).strip() if not creditor_id: raise RepositoryError("In den Optionen fehlt die Gläubiger-ID.") generated: list[GeneratedDebitMail] = [] warnings: list[str] = [] - for debit in debits: - member = repository.get_member(debit.member_id) - if not member.email.strip(): - warnings.append(f"{member.member_number or member.display_name}: E-Mail-Adresse fehlt.") - continue - content = debit_mail_bytes( - recipient=member.email, - first_name=member.first_name, - debit=debit, - collection_date=collection_date, - creditor_id=creditor_id, - sender_name=sender_name, - sender_email=sender_email, - signature=signature, - ) - filename = ( - f"SEPA-Info-{collection_date.isoformat()}-" - f"{_safe_filename(member.member_number or member.display_name)}.eml" - ) - export_path = _available_path(output, filename) - archive_dir = repository.members_root / member.member_id / "files" / "documents" / "SEPA" - archive_dir.mkdir(parents=True, exist_ok=True) - archive_path = _available_path(archive_dir, filename) - export_path.write_bytes(content) - try: - archive_path.write_bytes(content) - except OSError: - export_path.unlink(missing_ok=True) - raise - relative_path = archive_path.relative_to( - repository.members_root / member.member_id / "files" - ).as_posix() - digest = hashlib.sha256(content).hexdigest() - repository.append_event( - member.member_id, - event_type="sepa_notification_generated", - summary=f"SEPA-Info-Mail erzeugt: {archive_path.name}", - actor_type="user", - actor_name="Vorstand", - references={"document": relative_path}, - data={ - "sha256": digest, - "recipient": member.email, - "collection_date": collection_date.isoformat(), - "amount": f"{debit.amount:.2f}", - }, - ) - generated.append( - GeneratedDebitMail(member.member_id, member.email, export_path, archive_path) - ) + # One connection is opened for the whole batch (a run can cover dozens of + # members) instead of reconnecting/re-authenticating per recipient. + with session as client: + for debit in debits: + member = repository.get_member(debit.member_id) + if not member.email.strip(): + warnings.append(f"{member.member_number or member.display_name}: E-Mail-Adresse fehlt.") + continue + content = debit_mail_bytes( + recipient=member.email, + first_name=member.first_name, + debit=debit, + collection_date=collection_date, + creditor_id=creditor_id, + sender_name=sender_name, + sender_email=sender_email, + signature=signature, + ) + filename = ( + f"SEPA-Info-{collection_date.isoformat()}-" + f"{_safe_filename(member.member_number or member.display_name)}.eml" + ) + archive_dir = repository.members_root / member.member_id / "files" / "documents" / "SEPA" + archive_dir.mkdir(parents=True, exist_ok=True) + archive_path = _available_path(archive_dir, filename) + export_path: Path | None = None + if delivery_mode == "local": + export_path = _available_path(output, filename) + export_path.write_bytes(content) + elif delivery_mode == "send": + send_via_smtp(client, content) + else: + append_to_imap_drafts(client, content, folder=email_settings["imap_drafts_folder"]) + try: + archive_path.write_bytes(content) + except OSError: + if export_path is not None: + export_path.unlink(missing_ok=True) + raise + relative_path = archive_path.relative_to( + repository.members_root / member.member_id / "files" + ).as_posix() + digest = hashlib.sha256(content).hexdigest() + repository.append_event( + member.member_id, + event_type="sepa_notification_generated", + summary=f"SEPA-Info-Mail erzeugt: {archive_path.name}", + actor_type="user", + actor_name="Vorstand", + references={"document": relative_path}, + data={ + "sha256": digest, + "recipient": member.email, + "collection_date": collection_date.isoformat(), + "amount": f"{debit.amount:.2f}", + "delivery_mode": delivery_mode, + }, + ) + generated.append( + GeneratedDebitMail(member.member_id, member.email, export_path, archive_path) + ) return generated, warnings diff --git a/src/ccma/storage/repository.py b/src/ccma/storage/repository.py index c61b98e..28ac49c 100644 --- a/src/ccma/storage/repository.py +++ b/src/ccma/storage/repository.py @@ -138,6 +138,23 @@ DEFAULT_CONFIGURATION = { ], }, "contribution_rules": DEFAULT_CONTRIBUTION_RULES, + # "local" reproduces the historical behaviour (always ask for a save location, + # never touch the network) so repositories that never configured this stay on + # exactly what they had before -- it is a first-class choice, not just a fallback. + "email_settings": { + "delivery_mode": "local", + "smtp_host": "", + "smtp_port": 587, + "smtp_security": "starttls", + "smtp_username": "", + "smtp_password": "", + "imap_host": "", + "imap_port": 993, + "imap_security": "ssl", + "imap_username": "", + "imap_password": "", + "imap_drafts_folder": "INBOX.Entwürfe", + }, } @@ -2607,6 +2624,90 @@ class MemberRepository: } write_json_atomic(self.root / "repository.json", config) + def get_email_settings(self) -> dict: + config = self.get_configuration() + settings = config.get("email_settings") or {} + defaults = DEFAULT_CONFIGURATION["email_settings"] + delivery_mode = str(settings.get("delivery_mode", defaults["delivery_mode"])) + if delivery_mode not in {"local", "send", "drafts", "ask"}: + delivery_mode = defaults["delivery_mode"] + smtp_security = str(settings.get("smtp_security", defaults["smtp_security"])) + if smtp_security not in {"starttls", "ssl", "none"}: + smtp_security = defaults["smtp_security"] + imap_security = str(settings.get("imap_security", defaults["imap_security"])) + if imap_security not in {"starttls", "ssl", "none"}: + imap_security = defaults["imap_security"] + try: + smtp_port = int(settings.get("smtp_port", defaults["smtp_port"])) + except (TypeError, ValueError): + smtp_port = defaults["smtp_port"] + try: + imap_port = int(settings.get("imap_port", defaults["imap_port"])) + except (TypeError, ValueError): + imap_port = defaults["imap_port"] + return { + "delivery_mode": delivery_mode, + "smtp_host": str(settings.get("smtp_host", "")), + "smtp_port": smtp_port, + "smtp_security": smtp_security, + "smtp_username": str(settings.get("smtp_username", "")), + "smtp_password": str(settings.get("smtp_password", "")), + "imap_host": str(settings.get("imap_host", "")), + "imap_port": imap_port, + "imap_security": imap_security, + "imap_username": str(settings.get("imap_username", "")), + "imap_password": str(settings.get("imap_password", "")), + "imap_drafts_folder": str( + settings.get("imap_drafts_folder", defaults["imap_drafts_folder"]) + ), + } + + def save_email_settings( + self, + *, + delivery_mode: str, + smtp_host: str, + smtp_port: int, + smtp_security: str, + smtp_username: str, + smtp_password: str, + imap_host: str, + imap_port: int, + imap_security: str, + imap_username: str, + imap_password: str, + imap_drafts_folder: str, + ) -> None: + if delivery_mode not in {"local", "send", "drafts", "ask"}: + raise RepositoryError("Ungültiger Versandmodus.") + if smtp_security not in {"starttls", "ssl", "none"}: + raise RepositoryError("Ungültige SMTP-Verschlüsselung.") + if imap_security not in {"starttls", "ssl", "none"}: + raise RepositoryError("Ungültige IMAP-Verschlüsselung.") + if delivery_mode in {"send", "ask"} and not smtp_host.strip(): + raise RepositoryError("Für den direkten Versand ist ein SMTP-Server erforderlich.") + if delivery_mode in {"drafts", "ask"} and not imap_host.strip(): + raise RepositoryError("Für die Ablage als Entwurf ist ein IMAP-Server erforderlich.") + for label, port in (("Der SMTP-Port", smtp_port), ("Der IMAP-Port", imap_port)): + if port < 1 or port > 65535: + raise RepositoryError(f"{label} muss zwischen 1 und 65535 liegen.") + config = self.get_configuration() + config["email_settings"] = { + "delivery_mode": delivery_mode, + "smtp_host": smtp_host.strip(), + "smtp_port": int(smtp_port), + "smtp_security": smtp_security, + "smtp_username": smtp_username.strip(), + "smtp_password": smtp_password, + "imap_host": imap_host.strip(), + "imap_port": int(imap_port), + "imap_security": imap_security, + "imap_username": imap_username.strip(), + "imap_password": imap_password, + "imap_drafts_folder": imap_drafts_folder.strip() or "INBOX.Entwürfe", + } + write_json_atomic(self.root / "repository.json", config) + def save_organization(self, values: dict[str, str]) -> None: organization = {key: str(value).strip() for key, value in values.items()} organization["iban"] = normalize_iban(organization.get("iban", "")) diff --git a/src/ccma/ui/claim_tab.py b/src/ccma/ui/claim_tab.py index d57091c..92ef2eb 100644 --- a/src/ccma/ui/claim_tab.py +++ b/src/ccma/ui/claim_tab.py @@ -25,6 +25,7 @@ from ccma.domain.dates import date_input_hint, format_date_for_display from ccma.services.reminder_mail import generate_and_send_reminder_mail from ccma.storage.repository import MemberRepository, RepositoryError from ccma.ui.document_dialog import DocumentTemplateDialog +from ccma.ui.email_delivery_dialog import resolve_delivery_mode from ccma.ui.labels import ( CLAIM_ITEM_TYPE_LABELS, REMINDER_CHANNEL_LABELS, @@ -1258,22 +1259,29 @@ class ReminderMailDialog(tk.Toplevel): self.grab_set() def _generate(self) -> None: - selected = filedialog.asksaveasfilename( - parent=self, - title="Mahnung als Thunderbird-Entwurf speichern", - defaultextension=".eml", - initialfile=f"{self.reminder_name}.eml", - filetypes=[("E-Mail-Datei", "*.eml"), ("Alle Dateien", "*.*")], - ) - if not selected: + delivery_mode = resolve_delivery_mode(self, self.repository) + if delivery_mode is None: return + output_path = None + if delivery_mode == "local": + selected = filedialog.asksaveasfilename( + parent=self, + title="Mahnung als Thunderbird-Entwurf speichern", + defaultextension=".eml", + initialfile=f"{self.reminder_name}.eml", + filetypes=[("E-Mail-Datei", "*.eml"), ("Alle Dateien", "*.*")], + ) + if not selected: + return + output_path = selected try: result = generate_and_send_reminder_mail( self.repository, self.member_id, self.claim_id, self.reminder_id, - output_path=selected, + delivery_mode=delivery_mode, + output_path=output_path, sender_name=self.sender_name_var.get(), sender_email=self.sender_email_var.get(), signature=self.signature.get("1.0", "end-1c"), @@ -1281,9 +1289,14 @@ class ReminderMailDialog(tk.Toplevel): except (OSError, RepositoryError) as exc: messagebox.showerror("Mahnung konnte nicht erzeugt werden", str(exc), parent=self) return + outcome = { + "local": f"gespeichert:\n{result.export_path}", + "send": "direkt versandt.", + "drafts": "als Entwurf im Postfach abgelegt.", + }[delivery_mode] messagebox.showinfo( "Mahnung erzeugt", - f"Die E-Mail wurde gespeichert und als versandt gebucht:\n{result.export_path}", + f"Die E-Mail wurde {outcome}\n\nDie Mahnung wurde als versandt gebucht.", parent=self, ) self.destroy() diff --git a/src/ccma/ui/email_delivery_dialog.py b/src/ccma/ui/email_delivery_dialog.py new file mode 100644 index 0000000..b470273 --- /dev/null +++ b/src/ccma/ui/email_delivery_dialog.py @@ -0,0 +1,68 @@ +from __future__ import annotations + +import tkinter as tk +from tkinter import ttk + +from ccma.storage.repository import MemberRepository + + +class EmailDeliveryChoiceDialog(tk.Toplevel): + """Shown once per generation action (not per individual e-mail) when the + repository's delivery mode is "ask" -- lets the board decide per-batch whether + to send immediately or park the e-mail(s) as a draft for later review.""" + + def __init__(self, master: tk.Misc, *, can_send: bool, can_draft: bool): + super().__init__(master) + self.result: str | None = None + self.title("E-Mail-Versand") + self.transient(master.winfo_toplevel()) + self.resizable(False, False) + frame = ttk.Frame(self, padding=18) + frame.pack(fill="both", expand=True) + ttk.Label( + frame, + text="Wie soll mit dieser E-Mail verfahren werden?", + wraplength=360, + justify="left", + ).pack(anchor="w", pady=(0, 14)) + buttons = ttk.Frame(frame) + buttons.pack(anchor="e") + ttk.Button(buttons, text="Abbrechen", command=self._cancel).pack(side="left", padx=(0, 8)) + send_button = ttk.Button(buttons, text="Senden", command=self._choose_send) + send_button.pack(side="left", padx=(0, 8)) + send_button.configure(state="normal" if can_send else "disabled") + draft_button = ttk.Button( + buttons, text="Als Entwurf ablegen", style="Accent.TButton", command=self._choose_drafts + ) + draft_button.pack(side="left") + draft_button.configure(state="normal" if can_draft else "disabled") + self.bind("", lambda _event: self._cancel()) + self.protocol("WM_DELETE_WINDOW", self._cancel) + self.after_idle(self.grab_set) + + def _choose_send(self) -> None: + self.result = "send" + self.destroy() + + def _choose_drafts(self) -> None: + self.result = "drafts" + self.destroy() + + def _cancel(self) -> None: + self.result = None + self.destroy() + + +def resolve_delivery_mode(master: tk.Misc, repository: MemberRepository) -> str | None: + """Returns the delivery mode to use for the next generated e-mail(s): "local", + "send" or "drafts" -- or None if the board cancelled out of the "ask every time" + prompt, in which case the caller should abort without generating anything.""" + settings = repository.get_email_settings() + mode = settings["delivery_mode"] + if mode in {"local", "send", "drafts"}: + return mode + dialog = EmailDeliveryChoiceDialog( + master, can_send=bool(settings["smtp_host"]), can_draft=bool(settings["imap_host"]) + ) + master.wait_window(dialog) + return dialog.result diff --git a/src/ccma/ui/labels.py b/src/ccma/ui/labels.py index 57bde48..42a2444 100644 --- a/src/ccma/ui/labels.py +++ b/src/ccma/ui/labels.py @@ -30,6 +30,19 @@ REMINDER_CHANNEL_LABELS = { "personal": "Persönlich", } +EMAIL_DELIVERY_MODE_LABELS = { + "local": "Lokal speichern (bisheriges Verhalten)", + "send": "Direkt versenden", + "drafts": "Als Entwurf im Postfach ablegen", + "ask": "Jedes Mal fragen", +} + +EMAIL_SECURITY_LABELS = { + "starttls": "STARTTLS", + "ssl": "SSL/TLS", + "none": "Keine", +} + def display_label(labels: Mapping[str, str], key: str) -> str: return labels.get(key, key) diff --git a/src/ccma/ui/options_dialog.py b/src/ccma/ui/options_dialog.py index 057790e..aa5a06c 100644 --- a/src/ccma/ui/options_dialog.py +++ b/src/ccma/ui/options_dialog.py @@ -12,10 +12,17 @@ from ccma.services.intervals import ( IntervalValidationError, normalize_anniversary_intervals, ) +from ccma.services.mail_delivery import test_imap_connection, test_smtp_connection from ccma.storage.repository import MemberRepository, RepositoryError, validate_member_number_pattern from ccma.ui.changelog_view import ChangelogView from ccma.ui.icons import IconStore -from ccma.ui.labels import THEME_LABELS, display_label, storage_key +from ccma.ui.labels import ( + EMAIL_DELIVERY_MODE_LABELS, + EMAIL_SECURITY_LABELS, + THEME_LABELS, + display_label, + storage_key, +) class OptionsDialog(tk.Toplevel): @@ -94,6 +101,7 @@ class OptionsDialog(tk.Toplevel): organization = ttk.Frame(notebook, padding=16) automation = ttk.Frame(notebook, padding=16) reminders = ttk.Frame(notebook, padding=16) + email = ttk.Frame(notebook, padding=16) changelog = ChangelogView(notebook) notebook.add(paths, text="Pfade") notebook.add(appearance, text="Darstellung") @@ -101,6 +109,7 @@ class OptionsDialog(tk.Toplevel): notebook.add(organization, text="Verein / Absender") notebook.add(automation, text="Hausmeister") notebook.add(reminders, text="Mahnungen") + notebook.add(email, text="E-Mail-Versand") notebook.add(changelog, text="Changelog") self._build_paths(paths) self._build_appearance(appearance) @@ -108,6 +117,7 @@ class OptionsDialog(tk.Toplevel): self._build_organization(organization) self._build_automation(automation) self._build_reminders(reminders) + self._build_email(email) buttons = ttk.Frame(root) buttons.grid(row=1, column=0, sticky="e", pady=(12, 0)) @@ -576,6 +586,167 @@ class OptionsDialog(tk.Toplevel): del self.standard_items[int(selected[0])] self._refresh_standard_items() + def _build_email(self, parent: ttk.Frame) -> None: + parent.columnconfigure(0, weight=1) + email_settings = self.repository.get_email_settings() + + self.email_delivery_mode_var = tk.StringVar( + value=display_label(EMAIL_DELIVERY_MODE_LABELS, email_settings["delivery_mode"]) + ) + ttk.Label(parent, text="Versandmodus").grid(row=0, column=0, sticky="w", pady=6) + ttk.Combobox( + parent, + textvariable=self.email_delivery_mode_var, + values=list(EMAIL_DELIVERY_MODE_LABELS.values()), + state="readonly", + width=42, + ).grid(row=0, column=1, sticky="w", pady=6) + ttk.Label( + parent, + text=( + "Gilt für Mahnungs- und SEPA-Info-Mails. „Jedes Mal fragen“ zeigt vor jeder " + "Erzeugung eine Auswahl zwischen Senden und Entwurf ablegen." + ), + style="Muted.TLabel", + wraplength=680, + ).grid(row=1, column=0, columnspan=2, sticky="w", pady=(0, 12)) + + smtp_frame = ttk.LabelFrame(parent, text="SMTP (zum direkten Versenden)", padding=10) + smtp_frame.grid(row=2, column=0, columnspan=2, sticky="ew", pady=(4, 0)) + smtp_frame.columnconfigure(1, weight=1) + self.smtp_host_var = tk.StringVar(value=email_settings["smtp_host"]) + self.smtp_port_var = tk.StringVar(value=str(email_settings["smtp_port"])) + self.smtp_security_var = tk.StringVar( + value=display_label(EMAIL_SECURITY_LABELS, email_settings["smtp_security"]) + ) + self.smtp_username_var = tk.StringVar(value=email_settings["smtp_username"]) + self.smtp_password_var = tk.StringVar(value=email_settings["smtp_password"]) + ttk.Label(smtp_frame, text="Server").grid(row=0, column=0, sticky="w", pady=4, padx=(0, 8)) + ttk.Entry(smtp_frame, textvariable=self.smtp_host_var).grid( + row=0, column=1, sticky="ew", pady=4, padx=(0, 8) + ) + ttk.Label(smtp_frame, text="Port").grid(row=0, column=2, sticky="w", pady=4, padx=(0, 8)) + ttk.Entry(smtp_frame, textvariable=self.smtp_port_var, width=8).grid(row=0, column=3, pady=4) + ttk.Label(smtp_frame, text="Verschlüsselung").grid( + row=1, column=0, sticky="w", pady=4, padx=(0, 8) + ) + ttk.Combobox( + smtp_frame, + textvariable=self.smtp_security_var, + values=list(EMAIL_SECURITY_LABELS.values()), + state="readonly", + width=14, + ).grid(row=1, column=1, sticky="w", pady=4) + ttk.Label(smtp_frame, text="Benutzername").grid(row=2, column=0, sticky="w", pady=4, padx=(0, 8)) + ttk.Entry(smtp_frame, textvariable=self.smtp_username_var).grid( + row=2, column=1, columnspan=3, sticky="ew", pady=4 + ) + ttk.Label(smtp_frame, text="Passwort").grid(row=3, column=0, sticky="w", pady=4, padx=(0, 8)) + ttk.Entry(smtp_frame, textvariable=self.smtp_password_var, show="•").grid( + row=3, column=1, columnspan=3, sticky="ew", pady=4 + ) + ttk.Button(smtp_frame, text="Verbindung testen", command=self._test_smtp_connection).grid( + row=4, column=0, columnspan=4, sticky="w", pady=(8, 0) + ) + + imap_frame = ttk.LabelFrame(parent, text="IMAP (für die Ablage als Entwurf)", padding=10) + imap_frame.grid(row=3, column=0, columnspan=2, sticky="ew", pady=(16, 0)) + imap_frame.columnconfigure(1, weight=1) + self.imap_host_var = tk.StringVar(value=email_settings["imap_host"]) + self.imap_port_var = tk.StringVar(value=str(email_settings["imap_port"])) + self.imap_security_var = tk.StringVar( + value=display_label(EMAIL_SECURITY_LABELS, email_settings["imap_security"]) + ) + self.imap_username_var = tk.StringVar(value=email_settings["imap_username"]) + self.imap_password_var = tk.StringVar(value=email_settings["imap_password"]) + self.imap_drafts_folder_var = tk.StringVar(value=email_settings["imap_drafts_folder"]) + ttk.Label(imap_frame, text="Server").grid(row=0, column=0, sticky="w", pady=4, padx=(0, 8)) + ttk.Entry(imap_frame, textvariable=self.imap_host_var).grid( + row=0, column=1, sticky="ew", pady=4, padx=(0, 8) + ) + ttk.Label(imap_frame, text="Port").grid(row=0, column=2, sticky="w", pady=4, padx=(0, 8)) + ttk.Entry(imap_frame, textvariable=self.imap_port_var, width=8).grid(row=0, column=3, pady=4) + ttk.Label(imap_frame, text="Verschlüsselung").grid( + row=1, column=0, sticky="w", pady=4, padx=(0, 8) + ) + ttk.Combobox( + imap_frame, + textvariable=self.imap_security_var, + values=list(EMAIL_SECURITY_LABELS.values()), + state="readonly", + width=14, + ).grid(row=1, column=1, sticky="w", pady=4) + ttk.Label(imap_frame, text="Benutzername").grid(row=2, column=0, sticky="w", pady=4, padx=(0, 8)) + ttk.Entry(imap_frame, textvariable=self.imap_username_var).grid( + row=2, column=1, columnspan=3, sticky="ew", pady=4 + ) + ttk.Label(imap_frame, text="Passwort").grid(row=3, column=0, sticky="w", pady=4, padx=(0, 8)) + ttk.Entry(imap_frame, textvariable=self.imap_password_var, show="•").grid( + row=3, column=1, columnspan=3, sticky="ew", pady=4 + ) + ttk.Label(imap_frame, text="Entwürfe-Ordner").grid(row=4, column=0, sticky="w", pady=4, padx=(0, 8)) + ttk.Entry(imap_frame, textvariable=self.imap_drafts_folder_var).grid( + row=4, column=1, columnspan=3, sticky="ew", pady=4 + ) + ttk.Button(imap_frame, text="Verbindung testen", command=self._test_imap_connection).grid( + row=5, column=0, columnspan=4, sticky="w", pady=(8, 0) + ) + + ttk.Label( + parent, + text=( + "Server, Zugangsdaten und Passwörter werden mit den übrigen Vereinseinstellungen " + "in repository.json innerhalb dieses Mitglieder-Stores gespeichert -- pro Store " + "lässt sich so ein eigenes Postfach hinterlegen." + ), + style="Muted.TLabel", + wraplength=680, + ).grid(row=4, column=0, columnspan=2, sticky="w", pady=(12, 0)) + + def _collect_email_settings(self) -> dict: + return { + "delivery_mode": storage_key(EMAIL_DELIVERY_MODE_LABELS, self.email_delivery_mode_var.get()), + "smtp_host": self.smtp_host_var.get(), + "smtp_port": self.smtp_port_var.get(), + "smtp_security": storage_key(EMAIL_SECURITY_LABELS, self.smtp_security_var.get()), + "smtp_username": self.smtp_username_var.get(), + "smtp_password": self.smtp_password_var.get(), + "imap_host": self.imap_host_var.get(), + "imap_port": self.imap_port_var.get(), + "imap_security": storage_key(EMAIL_SECURITY_LABELS, self.imap_security_var.get()), + "imap_username": self.imap_username_var.get(), + "imap_password": self.imap_password_var.get(), + "imap_drafts_folder": self.imap_drafts_folder_var.get(), + } + + def _test_smtp_connection(self) -> None: + settings = self._collect_email_settings() + try: + settings["smtp_port"] = int(settings["smtp_port"]) + except ValueError: + messagebox.showerror("Ungültiger Port", "Der SMTP-Port muss eine ganze Zahl sein.", parent=self) + return + try: + test_smtp_connection(settings) + except RepositoryError as exc: + messagebox.showerror("SMTP-Verbindung fehlgeschlagen", str(exc), parent=self) + return + messagebox.showinfo("SMTP-Verbindung", "Verbindung und Anmeldung erfolgreich.", parent=self) + + def _test_imap_connection(self) -> None: + settings = self._collect_email_settings() + try: + settings["imap_port"] = int(settings["imap_port"]) + except ValueError: + messagebox.showerror("Ungültiger Port", "Der IMAP-Port muss eine ganze Zahl sein.", parent=self) + return + try: + test_imap_connection(settings) + except RepositoryError as exc: + messagebox.showerror("IMAP-Verbindung fehlgeschlagen", str(exc), parent=self) + return + messagebox.showinfo("IMAP-Verbindung", "Verbindung und Anmeldung erfolgreich.", parent=self) + def _choose_store(self) -> None: current = Path(self.store_var.get()).expanduser() if self.store_var.get().strip() else Path.home() initial = current if current.is_dir() else current.parent @@ -621,6 +792,15 @@ class OptionsDialog(tk.Toplevel): except (ValueError, IntervalValidationError) as exc: messagebox.showerror("Ungültige Hausmeister-Einstellung", str(exc), parent=self) return + email_settings = self._collect_email_settings() + try: + email_settings["smtp_port"] = int(email_settings["smtp_port"]) + email_settings["imap_port"] = int(email_settings["imap_port"]) + except ValueError: + messagebox.showerror( + "Ungültige E-Mail-Einstellung", "SMTP-/IMAP-Port müssen ganze Zahlen sein.", parent=self + ) + return store_text = self.store_var.get().strip() if not store_text: messagebox.showerror( @@ -685,6 +865,7 @@ class OptionsDialog(tk.Toplevel): levels=self.levels, standard_fee_items=self.standard_items, ) + self.repository.save_email_settings(**email_settings) except (OSError, RepositoryError) as exc: messagebox.showerror("Optionen konnten nicht gespeichert werden", str(exc), parent=self) return diff --git a/src/ccma/ui/sepa_dialog.py b/src/ccma/ui/sepa_dialog.py index 704d470..6c95ebf 100644 --- a/src/ccma/ui/sepa_dialog.py +++ b/src/ccma/ui/sepa_dialog.py @@ -10,6 +10,7 @@ from ccma.domain.dates import DateValidationError, normalize_date_input from ccma.services.sepa import pending_direct_debits, write_csv, write_pain008 from ccma.services.sepa_mail import generate_debit_mails from ccma.storage.repository import MemberRepository, RepositoryError +from ccma.ui.email_delivery_dialog import resolve_delivery_mode def _next_weekday(selected: date) -> date: @@ -277,26 +278,33 @@ class SepaMailExportDialog(tk.Toplevel): ttk.Button(buttons, text="Abbrechen", command=self.destroy).pack(side="left") ttk.Button( buttons, - text="Ordner wählen und Entwürfe erzeugen", + text="Info-Mails erzeugen", style="Accent.TButton", command=self._generate, ).pack(side="left", padx=(8, 0)) self.grab_set() def _generate(self) -> None: - selected = filedialog.askdirectory( - parent=self, - title="Ausgabeordner für Thunderbird-Entwürfe auswählen", - mustexist=True, - ) - if not selected: + delivery_mode = resolve_delivery_mode(self, self.repository) + if delivery_mode is None: return + output_directory = None + if delivery_mode == "local": + selected = filedialog.askdirectory( + parent=self, + title="Ausgabeordner für Thunderbird-Entwürfe auswählen", + mustexist=True, + ) + if not selected: + return + output_directory = selected try: generated, warnings = generate_debit_mails( self.repository, self.debits, collection_date=self.collection_date, - output_directory=selected, + delivery_mode=delivery_mode, + output_directory=output_directory, sender_name=self.sender_name_var.get(), sender_email=self.sender_email_var.get(), signature=self.signature.get("1.0", "end-1c"), @@ -304,10 +312,12 @@ class SepaMailExportDialog(tk.Toplevel): except (OSError, RepositoryError) as exc: messagebox.showerror("Info-Mails konnten nicht erzeugt werden", str(exc), parent=self) return - detail = ( - f"{len(generated)} Entwurf/Entwürfe wurden in\n{selected}\nerzeugt und in den " - "Mitgliederakten archiviert." - ) + location = { + "local": f"in\n{output_directory}\nerzeugt", + "send": "direkt versandt", + "drafts": "als Entwurf im Postfach abgelegt", + }[delivery_mode] + detail = f"{len(generated)} Info-Mail(s) wurden {location} und in den Mitgliederakten archiviert." if warnings: detail += "\n\nÜbersprungen:\n" + "\n".join(warnings[:12]) messagebox.showinfo("Info-Mails erzeugt", detail, parent=self) diff --git a/tests/test_email_settings.py b/tests/test_email_settings.py new file mode 100644 index 0000000..b6d6b17 --- /dev/null +++ b/tests/test_email_settings.py @@ -0,0 +1,103 @@ +import pytest + +from ccma.storage.repository import MemberRepository, RepositoryError + + +def test_email_settings_default_to_local_delivery_for_new_repositories(tmp_path) -> None: + repository = MemberRepository(tmp_path) + repository.initialize() + + settings = repository.get_email_settings() + + assert settings["delivery_mode"] == "local" + assert settings["smtp_host"] == "" + assert settings["imap_host"] == "" + + +def test_email_settings_round_trip_through_save_and_get(tmp_path) -> None: + repository = MemberRepository(tmp_path) + repository.initialize() + + repository.save_email_settings( + delivery_mode="ask", + smtp_host="mail.example.org", + smtp_port=587, + smtp_security="starttls", + smtp_username="board@example.org", + smtp_password="secret", + imap_host="mail.example.org", + imap_port=993, + imap_security="ssl", + imap_username="board@example.org", + imap_password="secret", + imap_drafts_folder="INBOX.Entwürfe", + ) + + settings = repository.get_email_settings() + assert settings["delivery_mode"] == "ask" + assert settings["smtp_host"] == "mail.example.org" + assert settings["smtp_port"] == 587 + assert settings["smtp_password"] == "secret" + assert settings["imap_drafts_folder"] == "INBOX.Entwürfe" + + +@pytest.mark.parametrize( + ("overrides", "match"), + [ + ({"delivery_mode": "carrier-pigeon"}, "Ungültiger Versandmodus"), + ({"smtp_security": "rot13"}, "Ungültige SMTP-Verschlüsselung"), + ({"imap_security": "rot13"}, "Ungültige IMAP-Verschlüsselung"), + ({"delivery_mode": "send", "smtp_host": ""}, "SMTP-Server erforderlich"), + ({"delivery_mode": "drafts", "imap_host": ""}, "IMAP-Server erforderlich"), + ({"delivery_mode": "ask", "imap_host": ""}, "IMAP-Server erforderlich"), + ({"smtp_port": 0}, "SMTP-Port"), + ({"smtp_port": 70000}, "SMTP-Port"), + ({"imap_port": 0}, "IMAP-Port"), + ], +) +def test_email_settings_rejects_invalid_input(tmp_path, overrides, match) -> None: + repository = MemberRepository(tmp_path) + repository.initialize() + values = { + "delivery_mode": "send", + "smtp_host": "mail.example.org", + "smtp_port": 587, + "smtp_security": "starttls", + "smtp_username": "", + "smtp_password": "", + "imap_host": "mail.example.org", + "imap_port": 993, + "imap_security": "ssl", + "imap_username": "", + "imap_password": "", + "imap_drafts_folder": "", + } + values.update(overrides) + + with pytest.raises(RepositoryError, match=match): + repository.save_email_settings(**values) + + +def test_email_settings_local_mode_needs_no_server(tmp_path) -> None: + repository = MemberRepository(tmp_path) + repository.initialize() + + repository.save_email_settings( + delivery_mode="local", + smtp_host="", + smtp_port=587, + smtp_security="starttls", + smtp_username="", + smtp_password="", + imap_host="", + imap_port=993, + imap_security="ssl", + imap_username="", + imap_password="", + imap_drafts_folder="", + ) + + settings = repository.get_email_settings() + assert settings["delivery_mode"] == "local" + # Blank folder falls back to a sensible default instead of staying empty. + assert settings["imap_drafts_folder"] == "INBOX.Entwürfe" diff --git a/tests/test_mail_delivery.py b/tests/test_mail_delivery.py new file mode 100644 index 0000000..3c95849 --- /dev/null +++ b/tests/test_mail_delivery.py @@ -0,0 +1,171 @@ +import pytest + +from ccma.services import mail_delivery +from ccma.storage.repository import RepositoryError + + +class _FakeSmtpClient: + instances: list["_FakeSmtpClient"] = [] + + def __init__(self, host, port, **kwargs): + self.host = host + self.port = port + self.kwargs = kwargs + self.calls: list[str] = [] + self.login_args: tuple | None = None + self.sent_messages: list = [] + _FakeSmtpClient.instances.append(self) + + def starttls(self, **kwargs): + self.calls.append("starttls") + + def login(self, username, password): + self.login_args = (username, password) + self.calls.append("login") + + def send_message(self, message): + self.sent_messages.append(message) + self.calls.append("send_message") + + def quit(self): + self.calls.append("quit") + + +class _FakeImapClient: + instances: list["_FakeImapClient"] = [] + error = Exception + + def __init__(self, host, port, **kwargs): + self.host = host + self.port = port + self.kwargs = kwargs + self.calls: list[str] = [] + self.login_args: tuple | None = None + self.appended: list[tuple] = [] + self.append_result = ("OK", [b"APPEND completed"]) + _FakeImapClient.instances.append(self) + + def starttls(self, *args, **kwargs): + self.calls.append("starttls") + + def login(self, username, password): + self.login_args = (username, password) + self.calls.append("login") + + def append(self, folder, flags, date_time, message_bytes): + self.appended.append((folder, flags, date_time, message_bytes)) + self.calls.append("append") + return self.append_result + + def logout(self): + self.calls.append("logout") + + +@pytest.fixture(autouse=True) +def _reset_fakes(): + _FakeSmtpClient.instances.clear() + _FakeImapClient.instances.clear() + yield + + +def _patch_smtp(monkeypatch): + monkeypatch.setattr(mail_delivery.smtplib, "SMTP", _FakeSmtpClient) + monkeypatch.setattr(mail_delivery.smtplib, "SMTP_SSL", _FakeSmtpClient) + + +def _patch_imap(monkeypatch): + monkeypatch.setattr(mail_delivery.imaplib, "IMAP4", _FakeImapClient) + monkeypatch.setattr(mail_delivery.imaplib, "IMAP4_SSL", _FakeImapClient) + + +SMTP_SETTINGS = { + "smtp_host": "mail.example.org", + "smtp_port": 587, + "smtp_security": "starttls", + "smtp_username": "board@example.org", + "smtp_password": "secret", +} + +IMAP_SETTINGS = { + "imap_host": "mail.example.org", + "imap_port": 993, + "imap_security": "ssl", + "imap_username": "board@example.org", + "imap_password": "secret", +} + +MESSAGE = ( + b"From: Board \r\n" + b"To: member@example.org\r\n" + b"Subject: Test\r\n" + b"\r\n" + b"Hello\r\n" +) + + +def test_send_via_smtp_logs_in_and_sends_reusing_one_connection(monkeypatch) -> None: + _patch_smtp(monkeypatch) + with mail_delivery.smtp_session(SMTP_SETTINGS) as client: + mail_delivery.send_via_smtp(client, MESSAGE) + mail_delivery.send_via_smtp(client, MESSAGE) + + assert len(_FakeSmtpClient.instances) == 1 + fake = _FakeSmtpClient.instances[0] + assert fake.host == "mail.example.org" + assert fake.port == 587 + assert fake.login_args == ("board@example.org", "secret") + assert fake.calls == ["starttls", "login", "send_message", "send_message", "quit"] + assert len(fake.sent_messages) == 2 + + +def test_append_to_imap_drafts_uses_configured_folder(monkeypatch) -> None: + _patch_imap(monkeypatch) + with mail_delivery.imap_session(IMAP_SETTINGS) as client: + mail_delivery.append_to_imap_drafts(client, MESSAGE, folder="INBOX.Entwürfe") + + fake = _FakeImapClient.instances[0] + assert fake.login_args == ("board@example.org", "secret") + assert fake.appended == [("INBOX.Entwürfe", r"(\Draft)", None, MESSAGE)] + assert fake.calls == ["login", "append", "logout"] + + +def test_append_to_imap_drafts_raises_on_rejected_status(monkeypatch) -> None: + _patch_imap(monkeypatch) + with mail_delivery.imap_session(IMAP_SETTINGS) as client: + client.append_result = ("NO", [b"Mailbox does not exist"]) + with pytest.raises(RepositoryError, match="Mailbox does not exist"): + mail_delivery.append_to_imap_drafts(client, MESSAGE, folder="Missing") + + +def test_smtp_session_wraps_connection_errors(monkeypatch) -> None: + def _boom(*args, **kwargs): + raise OSError("connection refused") + + monkeypatch.setattr(mail_delivery.smtplib, "SMTP", _boom) + with pytest.raises(RepositoryError, match="SMTP-Verbindung fehlgeschlagen"): + with mail_delivery.smtp_session({**SMTP_SETTINGS, "smtp_security": "none"}): + pass + + +def test_smtp_session_requires_host() -> None: + with pytest.raises(RepositoryError, match="Kein SMTP-Server konfiguriert"): + with mail_delivery.smtp_session({}): + pass + + +def test_imap_session_requires_host() -> None: + with pytest.raises(RepositoryError, match="Kein IMAP-Server konfiguriert"): + with mail_delivery.imap_session({}): + pass + + +def test_test_smtp_connection_succeeds_and_closes(monkeypatch) -> None: + _patch_smtp(monkeypatch) + mail_delivery.test_smtp_connection(SMTP_SETTINGS) + assert _FakeSmtpClient.instances[0].calls == ["starttls", "login", "quit"] + + +def test_test_imap_connection_succeeds_and_closes(monkeypatch) -> None: + _patch_imap(monkeypatch) + mail_delivery.test_imap_connection(IMAP_SETTINGS) + assert _FakeImapClient.instances[0].calls == ["login", "logout"] diff --git a/tests/test_reminder_mail.py b/tests/test_reminder_mail.py index f385572..a910be1 100644 --- a/tests/test_reminder_mail.py +++ b/tests/test_reminder_mail.py @@ -1,3 +1,4 @@ +from contextlib import contextmanager from email.parser import BytesParser from email.policy import default @@ -35,6 +36,7 @@ def test_reminder_mail_is_archived_and_marks_reminder_sent(tmp_path): member.member_id, "claim-1", reminder["reminder_id"], + delivery_mode="local", output_path=export_path, sender_name="Verwaltung C3MA", sender_email="verwaltung@example.org", @@ -72,6 +74,7 @@ def test_reminder_mail_includes_fee_in_open_balance(tmp_path): member.member_id, "claim-1", reminder["reminder_id"], + delivery_mode="local", output_path=tmp_path / "Mahnung.eml", sender_name="Verwaltung C3MA", sender_email="verwaltung@example.org", @@ -81,3 +84,56 @@ def test_reminder_mail_includes_fee_in_open_balance(tmp_path): message = BytesParser(policy=default).parsebytes(generated.export_path.read_bytes()) assert "Offener Betrag: 105.00 Euro" in message.get_content() assert "Mahngebühr: 5.00 Euro" in message.get_content() + + +def test_reminder_mail_send_mode_delivers_via_smtp_without_local_file(tmp_path, monkeypatch): + import ccma.services.reminder_mail as reminder_mail_module + + repository, member = _overdue_claim_repository(tmp_path / "store") + member.email = "reminder@example.org" + repository.save_member(member) + repository.save_email_settings( + delivery_mode="send", + smtp_host="mail.example.org", + smtp_port=587, + smtp_security="none", + smtp_username="", + smtp_password="", + imap_host="", + imap_port=993, + imap_security="ssl", + imap_username="", + imap_password="", + imap_drafts_folder="", + ) + reminder = repository.create_reminder_draft( + member.member_id, + "claim-1", + level=1, + name="Zahlungserinnerung", + payment_deadline_days=14, + ) + + sent = [] + fake_smtp_session = contextmanager(lambda settings: iter([object()])) + monkeypatch.setattr(reminder_mail_module, "smtp_session", fake_smtp_session) + monkeypatch.setattr( + reminder_mail_module, "send_via_smtp", lambda client, content: sent.append(content) + ) + + generated = generate_and_send_reminder_mail( + repository, + member.member_id, + "claim-1", + reminder["reminder_id"], + delivery_mode="send", + sender_name="Verwaltung C3MA", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + + assert generated.export_path is None + assert generated.archive_path.is_file() + assert len(sent) == 1 + data = repository.get_contributions(member.member_id) + assert data.reminders[0]["status"] == "sent" diff --git a/tests/test_sepa.py b/tests/test_sepa.py index cc38ef5..b2d3daa 100644 --- a/tests/test_sepa.py +++ b/tests/test_sepa.py @@ -1,4 +1,5 @@ import xml.etree.ElementTree as ET +from contextlib import contextmanager from datetime import UTC, date, datetime from decimal import Decimal from email.parser import BytesParser @@ -214,6 +215,7 @@ def test_generated_mail_is_exported_archived_and_logged(tmp_path): repository, debits, collection_date=date(2026, 8, 3), + delivery_mode="local", output_directory=tmp_path / "mail-export", sender_name="Verwaltung C3MA", sender_email="verwaltung@example.org", @@ -227,3 +229,64 @@ def test_generated_mail_is_exported_archived_and_logged(tmp_path): event = repository.get_events(member.member_id)[-1] assert event.event_type == "sepa_notification_generated" assert event.references["document"].startswith("documents/SEPA/") + + +def test_generated_mail_send_mode_reuses_one_smtp_connection_for_batch(tmp_path, monkeypatch): + import ccma.services.sepa_mail as sepa_mail_module + + repository, member = _repository(tmp_path) + organization = repository.get_configuration()["organization"] + organization.update( + { + "name": "Chaos Computer Club Mannheim e.V.", + "email": "verwaltung@example.org", + "iban": "DE89370400440532013000", + "bic": "COBADEFFXXX", + "creditor_id": "DE98ZZZ09999999999", + } + ) + repository.save_organization(organization) + repository.save_email_settings( + delivery_mode="send", + smtp_host="mail.example.org", + smtp_port=587, + smtp_security="none", + smtp_username="", + smtp_password="", + imap_host="", + imap_port=993, + imap_security="ssl", + imap_username="", + imap_password="", + imap_drafts_folder="", + ) + debits, _warnings = pending_direct_debits(repository, due_until=date(2026, 12, 31)) + + connections_opened = [] + + @contextmanager + def fake_smtp_session(settings): + connections_opened.append(settings) + yield object() + + sent = [] + monkeypatch.setattr(sepa_mail_module, "smtp_session", fake_smtp_session) + monkeypatch.setattr( + sepa_mail_module, "send_via_smtp", lambda client, content: sent.append(content) + ) + + generated, warnings = generate_debit_mails( + repository, + debits, + collection_date=date(2026, 8, 3), + delivery_mode="send", + sender_name="Verwaltung C3MA", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + + assert warnings == [] + assert generated[0].export_path is None + assert generated[0].archive_path.is_file() + assert len(connections_opened) == 1, "one connection should be reused for the whole batch" + assert len(sent) == len(debits) From cc4aaef8958f7dc47ea5c561794c8e15de9d582e Mon Sep 17 00:00:00 2001 From: Marcel Peterkau Date: Sat, 15 Aug 2026 04:30:53 +0200 Subject: [PATCH 2/6] Copy directly-sent e-mails to an IMAP Sent folder, with a live folder picker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A raw SMTP send has no server-side "Sent" copy on its own (unlike IMAP drafts, which are inherently server-side) -- add an opt-in checkbox plus a configurable target folder so directly sent Mahnungen/SEPA-info-mails still show up in the account's Gesendet/Sent folder like a normal mail client would leave them. Applies only to "send" delivery; drafts already live on the server by definition. Both the Entwürfe- and Gesendet-folder fields are now editable comboboxes: a new "Ordnerliste laden" button fetches the real folder list from the IMAP server (needs working credentials first) via LIST, decoding folder names from modified UTF-7 (RFC 3501) so names like "Entwürfe" render correctly instead of as "Entw&APw-rfe". Free text still works -- ensure_imap_folder() creates the folder on first use if it doesn't exist yet, checked once per batch rather than before every single message. mail_delivery.append_to_imap_drafts() became the more general append_message(client, content, folder=, flags=), reused for both the \Draft and \Seen cases. Co-Authored-By: Claude Sonnet 5 --- src/ccma/services/mail_delivery.py | 120 ++++++++++++++++++++++++++++- src/ccma/services/reminder_mail.py | 23 +++++- src/ccma/services/sepa_mail.py | 38 ++++++--- src/ccma/storage/repository.py | 14 ++++ src/ccma/ui/options_dialog.py | 55 +++++++++++-- tests/test_email_settings.py | 29 +++++++ tests/test_mail_delivery.py | 93 ++++++++++++++++++++-- tests/test_reminder_mail.py | 63 +++++++++++++++ tests/test_sepa.py | 98 +++++++++++++++++++++++ 9 files changed, 505 insertions(+), 28 deletions(-) diff --git a/src/ccma/services/mail_delivery.py b/src/ccma/services/mail_delivery.py index 344671a..2e8c491 100644 --- a/src/ccma/services/mail_delivery.py +++ b/src/ccma/services/mail_delivery.py @@ -1,6 +1,9 @@ from __future__ import annotations +import base64 +import binascii import imaplib +import re import smtplib import ssl from collections.abc import Iterator @@ -90,16 +93,127 @@ def send_via_smtp(client: smtplib.SMTP, message_bytes: bytes) -> None: raise RepositoryError(f"E-Mail konnte nicht versandt werden: {exc}") from exc -def append_to_imap_drafts(client: imaplib.IMAP4, message_bytes: bytes, *, folder: str) -> None: +def append_message(client: imaplib.IMAP4, message_bytes: bytes, *, folder: str, flags: str) -> None: try: - status, response = client.append(folder, r"(\Draft)", None, message_bytes) + status, response = client.append(_encode_imap_utf7(folder), flags, None, message_bytes) except (OSError, imaplib.IMAP4.error) as exc: - raise RepositoryError(f"E-Mail konnte nicht als Entwurf abgelegt werden: {exc}") from exc + raise RepositoryError(f"E-Mail konnte nicht in „{folder}“ abgelegt werden: {exc}") from exc if status != "OK": detail = response[0].decode("utf-8", "replace") if response and response[0] else status raise RepositoryError(f"IMAP-Server hat die Ablage im Ordner „{folder}“ abgelehnt: {detail}") +def ensure_imap_folder(client: imaplib.IMAP4, folder: str) -> None: + """Creates the folder on the server if it doesn't exist yet -- lets someone type + a not-yet-existing Entwürfe/Gesendet folder name and have it just appear, instead + of every append failing until it's created by hand in a separate mail client.""" + encoded = _encode_imap_utf7(folder) + try: + status, _response = client.select(encoded, readonly=True) + except (OSError, imaplib.IMAP4.error) as exc: + raise RepositoryError(f"IMAP-Ordner „{folder}“ konnte nicht geprüft werden: {exc}") from exc + if status == "OK": + return + try: + status, response = client.create(encoded) + except (OSError, imaplib.IMAP4.error) as exc: + raise RepositoryError(f"IMAP-Ordner „{folder}“ konnte nicht angelegt werden: {exc}") from exc + if status != "OK": + detail = response[0].decode("utf-8", "replace") if response and response[0] else status + raise RepositoryError(f"IMAP-Ordner „{folder}“ konnte nicht angelegt werden: {detail}") + + +_LIST_RESPONSE_RE = re.compile( + rb'^\((?P[^)]*)\)\s+(?:"(?P[^"]*)"|(?P\S+))\s+(?P.+)$' +) + + +def list_imap_folders(settings: dict) -> list[str]: + """Human-readable (decoded) folder names for the Options dialog's folder picker + -- fetched live from the server, so it only works once credentials are entered.""" + with imap_session(settings) as client: + try: + status, response = client.list() + except (OSError, imaplib.IMAP4.error) as exc: + raise RepositoryError(f"Ordnerliste konnte nicht abgerufen werden: {exc}") from exc + if status != "OK": + raise RepositoryError("Ordnerliste konnte nicht abgerufen werden.") + folders = [] + for raw in response or []: + if not raw: + continue + name = _parse_list_response(raw) + if name: + folders.append(name) + return folders + + +def _parse_list_response(raw: bytes) -> str | None: + match = _LIST_RESPONSE_RE.match(raw) + if not match: + return None + name = match.group("name").decode("utf-8", "replace").strip() + if name.startswith('"') and name.endswith('"') and len(name) >= 2: + name = name[1:-1] + return _decode_imap_utf7(name) + + +def _decode_imap_utf7(value: str) -> str: + # Modified UTF-7 (RFC 3501 5.1.3): "&" takes the role of "+", and "," takes the + # role of "/" inside the base64 run; "&-" is a literal ampersand. + if "&" not in value: + return value + result: list[str] = [] + index = 0 + length = len(value) + while index < length: + char = value[index] + if char != "&": + result.append(char) + index += 1 + continue + end = value.find("-", index + 1) + if end == -1: + end = length + chunk = value[index + 1 : end] + if chunk == "": + result.append("&") + else: + base64_chunk = chunk.replace(",", "/") + base64_chunk += "=" * (-len(base64_chunk) % 4) + try: + result.append(base64.b64decode(base64_chunk).decode("utf-16-be")) + except (binascii.Error, UnicodeDecodeError): + result.append("&" + chunk + "-") + index = end + 1 + return "".join(result) + + +def _encode_imap_utf7(value: str) -> str: + if "&" not in value and all(32 <= ord(char) <= 126 for char in value): + return value + result: list[str] = [] + index = 0 + length = len(value) + while index < length: + char = value[index] + if char == "&": + result.append("&-") + index += 1 + continue + if 32 <= ord(char) <= 126: + result.append(char) + index += 1 + continue + start = index + while index < length and not (32 <= ord(value[index]) <= 126): + index += 1 + chunk = value[start:index] + encoded = base64.b64encode(chunk.encode("utf-16-be")).decode("ascii").rstrip("=") + result.append("&" + encoded.replace("/", ",") + "-") + return "".join(result) + + def test_smtp_connection(settings: dict) -> None: with smtp_session(settings): pass diff --git a/src/ccma/services/reminder_mail.py b/src/ccma/services/reminder_mail.py index be714ff..2d7413f 100644 --- a/src/ccma/services/reminder_mail.py +++ b/src/ccma/services/reminder_mail.py @@ -3,6 +3,7 @@ from __future__ import annotations import hashlib import os import re +from contextlib import nullcontext from dataclasses import dataclass from datetime import datetime from email.message import EmailMessage @@ -12,7 +13,13 @@ from pathlib import Path from ccma.domain.contributions import claim_balance from ccma.domain.dates import format_date_for_display -from ccma.services.mail_delivery import append_to_imap_drafts, imap_session, send_via_smtp, smtp_session +from ccma.services.mail_delivery import ( + append_message, + ensure_imap_folder, + imap_session, + send_via_smtp, + smtp_session, +) from ccma.storage.repository import MemberRepository, RepositoryError @@ -178,12 +185,20 @@ def generate_and_send_reminder_mail( export_temp.unlink(missing_ok=True) export_path = target elif delivery_mode == "send": - with smtp_session(repository.get_email_settings()) as client: - send_via_smtp(client, content) + settings = repository.get_email_settings() + sent_copy = imap_session(settings) if settings["imap_sent_enabled"] else nullcontext(None) + with smtp_session(settings) as smtp_client, sent_copy as imap_client: + send_via_smtp(smtp_client, content) + if imap_client is not None: + ensure_imap_folder(imap_client, settings["imap_sent_folder"]) + append_message( + imap_client, content, folder=settings["imap_sent_folder"], flags=r"(\Seen)" + ) else: settings = repository.get_email_settings() with imap_session(settings) as client: - append_to_imap_drafts(client, content, folder=settings["imap_drafts_folder"]) + ensure_imap_folder(client, settings["imap_drafts_folder"]) + append_message(client, content, folder=settings["imap_drafts_folder"], flags=r"(\Draft)") os.replace(archive_temp, archive_path) finally: archive_temp.unlink(missing_ok=True) diff --git a/src/ccma/services/sepa_mail.py b/src/ccma/services/sepa_mail.py index f067560..728da29 100644 --- a/src/ccma/services/sepa_mail.py +++ b/src/ccma/services/sepa_mail.py @@ -10,7 +10,13 @@ from email.policy import SMTP from email.utils import format_datetime, make_msgid from pathlib import Path -from ccma.services.mail_delivery import append_to_imap_drafts, imap_session, send_via_smtp, smtp_session +from ccma.services.mail_delivery import ( + append_message, + ensure_imap_folder, + imap_session, + send_via_smtp, + smtp_session, +) from ccma.services.sepa import DirectDebit from ccma.storage.repository import MemberRepository, RepositoryError @@ -105,18 +111,21 @@ def generate_debit_mails( ) -> tuple[list[GeneratedDebitMail], list[str]]: output: Path | None = None email_settings: dict | None = None + smtp_ctx = nullcontext(None) + imap_ctx = nullcontext(None) if delivery_mode == "local": if not output_directory: raise RepositoryError("Kein Zielordner für die lokale Ablage angegeben.") output = Path(output_directory) output.mkdir(parents=True, exist_ok=True) - session = nullcontext(None) elif delivery_mode == "send": email_settings = repository.get_email_settings() - session = smtp_session(email_settings) + smtp_ctx = smtp_session(email_settings) + if email_settings["imap_sent_enabled"]: + imap_ctx = imap_session(email_settings) elif delivery_mode == "drafts": email_settings = repository.get_email_settings() - session = imap_session(email_settings) + imap_ctx = imap_session(email_settings) else: raise RepositoryError(f"Unbekannter Versandmodus: {delivery_mode}") @@ -126,9 +135,14 @@ def generate_debit_mails( raise RepositoryError("In den Optionen fehlt die Gläubiger-ID.") generated: list[GeneratedDebitMail] = [] warnings: list[str] = [] - # One connection is opened for the whole batch (a run can cover dozens of - # members) instead of reconnecting/re-authenticating per recipient. - with session as client: + # One connection (or two, for "send" with a Sent-folder copy) is opened for the + # whole batch -- a run can cover dozens of members -- instead of reconnecting/ + # re-authenticating per recipient. + with smtp_ctx as smtp_client, imap_ctx as imap_client: + if delivery_mode == "drafts": + ensure_imap_folder(imap_client, email_settings["imap_drafts_folder"]) + elif delivery_mode == "send" and imap_client is not None: + ensure_imap_folder(imap_client, email_settings["imap_sent_folder"]) for debit in debits: member = repository.get_member(debit.member_id) if not member.email.strip(): @@ -156,9 +170,15 @@ def generate_debit_mails( export_path = _available_path(output, filename) export_path.write_bytes(content) elif delivery_mode == "send": - send_via_smtp(client, content) + send_via_smtp(smtp_client, content) + if imap_client is not None: + append_message( + imap_client, content, folder=email_settings["imap_sent_folder"], flags=r"(\Seen)" + ) else: - append_to_imap_drafts(client, content, folder=email_settings["imap_drafts_folder"]) + append_message( + imap_client, content, folder=email_settings["imap_drafts_folder"], flags=r"(\Draft)" + ) try: archive_path.write_bytes(content) except OSError: diff --git a/src/ccma/storage/repository.py b/src/ccma/storage/repository.py index 28ac49c..c15cfef 100644 --- a/src/ccma/storage/repository.py +++ b/src/ccma/storage/repository.py @@ -154,6 +154,10 @@ DEFAULT_CONFIGURATION = { "imap_username": "", "imap_password": "", "imap_drafts_folder": "INBOX.Entwürfe", + # Sent-copy applies only to "send" (a direct SMTP send has no server-side + # "Sent" copy on its own, unlike drafts which are inherently server-side). + "imap_sent_enabled": False, + "imap_sent_folder": "INBOX.Sent", }, } @@ -2660,6 +2664,8 @@ class MemberRepository: "imap_drafts_folder": str( settings.get("imap_drafts_folder", defaults["imap_drafts_folder"]) ), + "imap_sent_enabled": bool(settings.get("imap_sent_enabled", defaults["imap_sent_enabled"])), + "imap_sent_folder": str(settings.get("imap_sent_folder", defaults["imap_sent_folder"])), } def save_email_settings( @@ -2677,6 +2683,8 @@ class MemberRepository: imap_username: str, imap_password: str, imap_drafts_folder: str, + imap_sent_enabled: bool = False, + imap_sent_folder: str = "", ) -> None: if delivery_mode not in {"local", "send", "drafts", "ask"}: raise RepositoryError("Ungültiger Versandmodus.") @@ -2688,6 +2696,10 @@ class MemberRepository: raise RepositoryError("Für den direkten Versand ist ein SMTP-Server erforderlich.") if delivery_mode in {"drafts", "ask"} and not imap_host.strip(): raise RepositoryError("Für die Ablage als Entwurf ist ein IMAP-Server erforderlich.") + if imap_sent_enabled and not imap_host.strip(): + raise RepositoryError( + "Für die Ablage im Gesendet-Ordner ist ein IMAP-Server erforderlich." + ) for label, port in (("Der SMTP-Port", smtp_port), ("Der IMAP-Port", imap_port)): if port < 1 or port > 65535: raise RepositoryError(f"{label} muss zwischen 1 und 65535 liegen.") @@ -2705,6 +2717,8 @@ class MemberRepository: "imap_username": imap_username.strip(), "imap_password": imap_password, "imap_drafts_folder": imap_drafts_folder.strip() or "INBOX.Entwürfe", + "imap_sent_enabled": bool(imap_sent_enabled), + "imap_sent_folder": imap_sent_folder.strip() or "INBOX.Sent", } write_json_atomic(self.root / "repository.json", config) diff --git a/src/ccma/ui/options_dialog.py b/src/ccma/ui/options_dialog.py index aa5a06c..2083ad1 100644 --- a/src/ccma/ui/options_dialog.py +++ b/src/ccma/ui/options_dialog.py @@ -12,7 +12,7 @@ from ccma.services.intervals import ( IntervalValidationError, normalize_anniversary_intervals, ) -from ccma.services.mail_delivery import test_imap_connection, test_smtp_connection +from ccma.services.mail_delivery import list_imap_folders, test_imap_connection, test_smtp_connection from ccma.storage.repository import MemberRepository, RepositoryError, validate_member_number_pattern from ccma.ui.changelog_view import ChangelogView from ccma.ui.icons import IconStore @@ -649,7 +649,7 @@ class OptionsDialog(tk.Toplevel): row=4, column=0, columnspan=4, sticky="w", pady=(8, 0) ) - imap_frame = ttk.LabelFrame(parent, text="IMAP (für die Ablage als Entwurf)", padding=10) + imap_frame = ttk.LabelFrame(parent, text="IMAP (für Entwürfe & Gesendet)", padding=10) imap_frame.grid(row=3, column=0, columnspan=2, sticky="ew", pady=(16, 0)) imap_frame.columnconfigure(1, weight=1) self.imap_host_var = tk.StringVar(value=email_settings["imap_host"]) @@ -660,6 +660,8 @@ class OptionsDialog(tk.Toplevel): self.imap_username_var = tk.StringVar(value=email_settings["imap_username"]) self.imap_password_var = tk.StringVar(value=email_settings["imap_password"]) self.imap_drafts_folder_var = tk.StringVar(value=email_settings["imap_drafts_folder"]) + self.imap_sent_enabled_var = tk.BooleanVar(value=email_settings["imap_sent_enabled"]) + self.imap_sent_folder_var = tk.StringVar(value=email_settings["imap_sent_folder"]) ttk.Label(imap_frame, text="Server").grid(row=0, column=0, sticky="w", pady=4, padx=(0, 8)) ttk.Entry(imap_frame, textvariable=self.imap_host_var).grid( row=0, column=1, sticky="ew", pady=4, padx=(0, 8) @@ -685,12 +687,33 @@ class OptionsDialog(tk.Toplevel): row=3, column=1, columnspan=3, sticky="ew", pady=4 ) ttk.Label(imap_frame, text="Entwürfe-Ordner").grid(row=4, column=0, sticky="w", pady=4, padx=(0, 8)) - ttk.Entry(imap_frame, textvariable=self.imap_drafts_folder_var).grid( - row=4, column=1, columnspan=3, sticky="ew", pady=4 - ) - ttk.Button(imap_frame, text="Verbindung testen", command=self._test_imap_connection).grid( - row=5, column=0, columnspan=4, sticky="w", pady=(8, 0) + self.imap_drafts_folder_combo = ttk.Combobox(imap_frame, textvariable=self.imap_drafts_folder_var) + self.imap_drafts_folder_combo.grid(row=4, column=1, columnspan=3, sticky="ew", pady=4) + ttk.Checkbutton( + imap_frame, + text="Direkt versandte Mahnungen/Info-Mails zusätzlich in einen Gesendet-Ordner ablegen", + variable=self.imap_sent_enabled_var, + ).grid(row=5, column=0, columnspan=4, sticky="w", pady=(8, 4)) + ttk.Label(imap_frame, text="Gesendet-Ordner").grid(row=6, column=0, sticky="w", pady=4, padx=(0, 8)) + self.imap_sent_folder_combo = ttk.Combobox(imap_frame, textvariable=self.imap_sent_folder_var) + self.imap_sent_folder_combo.grid(row=6, column=1, columnspan=3, sticky="ew", pady=4) + buttons_row = ttk.Frame(imap_frame) + buttons_row.grid(row=7, column=0, columnspan=4, sticky="w", pady=(8, 0)) + ttk.Button(buttons_row, text="Verbindung testen", command=self._test_imap_connection).pack( + side="left", padx=(0, 8) ) + ttk.Button(buttons_row, text="Ordnerliste laden", command=self._load_imap_folders).pack(side="left") + ttk.Label( + imap_frame, + text=( + "„Ordnerliste laden“ ruft die Ordner live vom Server ab (Zugangsdaten oben müssen " + "dafür bereits ausgefüllt sein) und füllt beide Dropdowns; freie Eingabe bleibt " + "möglich -- ein dabei angegebener, noch nicht vorhandener Ordner wird bei der " + "nächsten Ablage automatisch angelegt." + ), + style="Muted.TLabel", + wraplength=660, + ).grid(row=8, column=0, columnspan=4, sticky="w", pady=(6, 0)) ttk.Label( parent, @@ -717,6 +740,8 @@ class OptionsDialog(tk.Toplevel): "imap_username": self.imap_username_var.get(), "imap_password": self.imap_password_var.get(), "imap_drafts_folder": self.imap_drafts_folder_var.get(), + "imap_sent_enabled": self.imap_sent_enabled_var.get(), + "imap_sent_folder": self.imap_sent_folder_var.get(), } def _test_smtp_connection(self) -> None: @@ -747,6 +772,22 @@ class OptionsDialog(tk.Toplevel): return messagebox.showinfo("IMAP-Verbindung", "Verbindung und Anmeldung erfolgreich.", parent=self) + def _load_imap_folders(self) -> None: + settings = self._collect_email_settings() + try: + settings["imap_port"] = int(settings["imap_port"]) + except ValueError: + messagebox.showerror("Ungültiger Port", "Der IMAP-Port muss eine ganze Zahl sein.", parent=self) + return + try: + folders = list_imap_folders(settings) + except RepositoryError as exc: + messagebox.showerror("Ordnerliste konnte nicht geladen werden", str(exc), parent=self) + return + self.imap_drafts_folder_combo.configure(values=folders) + self.imap_sent_folder_combo.configure(values=folders) + messagebox.showinfo("Ordnerliste geladen", f"{len(folders)} Ordner gefunden.", parent=self) + def _choose_store(self) -> None: current = Path(self.store_var.get()).expanduser() if self.store_var.get().strip() else Path.home() initial = current if current.is_dir() else current.parent diff --git a/tests/test_email_settings.py b/tests/test_email_settings.py index b6d6b17..ccd14d7 100644 --- a/tests/test_email_settings.py +++ b/tests/test_email_settings.py @@ -12,6 +12,8 @@ def test_email_settings_default_to_local_delivery_for_new_repositories(tmp_path) assert settings["delivery_mode"] == "local" assert settings["smtp_host"] == "" assert settings["imap_host"] == "" + assert settings["imap_sent_enabled"] is False + assert settings["imap_sent_folder"] == "INBOX.Sent" def test_email_settings_round_trip_through_save_and_get(tmp_path) -> None: @@ -31,6 +33,8 @@ def test_email_settings_round_trip_through_save_and_get(tmp_path) -> None: imap_username="board@example.org", imap_password="secret", imap_drafts_folder="INBOX.Entwürfe", + imap_sent_enabled=True, + imap_sent_folder="INBOX.Gesendet", ) settings = repository.get_email_settings() @@ -39,6 +43,31 @@ def test_email_settings_round_trip_through_save_and_get(tmp_path) -> None: assert settings["smtp_port"] == 587 assert settings["smtp_password"] == "secret" assert settings["imap_drafts_folder"] == "INBOX.Entwürfe" + assert settings["imap_sent_enabled"] is True + assert settings["imap_sent_folder"] == "INBOX.Gesendet" + + +def test_email_settings_sent_copy_requires_imap_host_even_in_send_only_mode(tmp_path) -> None: + repository = MemberRepository(tmp_path) + repository.initialize() + + with pytest.raises(RepositoryError, match="Gesendet-Ordner ist ein IMAP-Server erforderlich"): + repository.save_email_settings( + delivery_mode="send", + smtp_host="mail.example.org", + smtp_port=587, + smtp_security="starttls", + smtp_username="", + smtp_password="", + imap_host="", + imap_port=993, + imap_security="ssl", + imap_username="", + imap_password="", + imap_drafts_folder="", + imap_sent_enabled=True, + imap_sent_folder="", + ) @pytest.mark.parametrize( diff --git a/tests/test_mail_delivery.py b/tests/test_mail_delivery.py index 3c95849..3ab7191 100644 --- a/tests/test_mail_delivery.py +++ b/tests/test_mail_delivery.py @@ -33,6 +33,7 @@ class _FakeSmtpClient: class _FakeImapClient: instances: list["_FakeImapClient"] = [] + default_list_result: tuple = ("OK", []) error = Exception def __init__(self, host, port, **kwargs): @@ -43,6 +44,11 @@ class _FakeImapClient: self.login_args: tuple | None = None self.appended: list[tuple] = [] self.append_result = ("OK", [b"APPEND completed"]) + self.selected: list[str] = [] + self.select_result = ("OK", [b"1"]) + self.created: list[str] = [] + self.create_result = ("OK", [b"CREATE completed"]) + self.list_result = _FakeImapClient.default_list_result _FakeImapClient.instances.append(self) def starttls(self, *args, **kwargs): @@ -57,6 +63,20 @@ class _FakeImapClient: self.calls.append("append") return self.append_result + def select(self, folder, readonly=False): + self.selected.append(folder) + self.calls.append("select") + return self.select_result + + def create(self, folder): + self.created.append(folder) + self.calls.append("create") + return self.create_result + + def list(self): + self.calls.append("list") + return self.list_result + def logout(self): self.calls.append("logout") @@ -65,6 +85,7 @@ class _FakeImapClient: def _reset_fakes(): _FakeSmtpClient.instances.clear() _FakeImapClient.instances.clear() + _FakeImapClient.default_list_result = ("OK", []) yield @@ -118,23 +139,85 @@ def test_send_via_smtp_logs_in_and_sends_reusing_one_connection(monkeypatch) -> assert len(fake.sent_messages) == 2 -def test_append_to_imap_drafts_uses_configured_folder(monkeypatch) -> None: +def test_append_message_uses_configured_folder_and_flags(monkeypatch) -> None: _patch_imap(monkeypatch) with mail_delivery.imap_session(IMAP_SETTINGS) as client: - mail_delivery.append_to_imap_drafts(client, MESSAGE, folder="INBOX.Entwürfe") + mail_delivery.append_message(client, MESSAGE, folder="INBOX.Entwürfe", flags=r"(\Draft)") fake = _FakeImapClient.instances[0] assert fake.login_args == ("board@example.org", "secret") - assert fake.appended == [("INBOX.Entwürfe", r"(\Draft)", None, MESSAGE)] + # The folder name travels the wire in modified UTF-7, not raw UTF-8. + assert fake.appended == [("INBOX.Entw&APw-rfe", r"(\Draft)", None, MESSAGE)] assert fake.calls == ["login", "append", "logout"] -def test_append_to_imap_drafts_raises_on_rejected_status(monkeypatch) -> None: +def test_append_message_raises_on_rejected_status(monkeypatch) -> None: _patch_imap(monkeypatch) with mail_delivery.imap_session(IMAP_SETTINGS) as client: client.append_result = ("NO", [b"Mailbox does not exist"]) with pytest.raises(RepositoryError, match="Mailbox does not exist"): - mail_delivery.append_to_imap_drafts(client, MESSAGE, folder="Missing") + mail_delivery.append_message(client, MESSAGE, folder="Missing", flags=r"(\Seen)") + + +def test_ensure_imap_folder_skips_creation_when_folder_already_exists(monkeypatch) -> None: + _patch_imap(monkeypatch) + with mail_delivery.imap_session(IMAP_SETTINGS) as client: + mail_delivery.ensure_imap_folder(client, "INBOX.Sent") + + fake = _FakeImapClient.instances[0] + assert fake.selected == ["INBOX.Sent"] + assert fake.created == [] + + +def test_ensure_imap_folder_creates_missing_folder(monkeypatch) -> None: + _patch_imap(monkeypatch) + with mail_delivery.imap_session(IMAP_SETTINGS) as client: + client.select_result = ("NO", [b"Mailbox does not exist"]) + mail_delivery.ensure_imap_folder(client, "Archiv.Neu") + + fake = _FakeImapClient.instances[0] + assert fake.selected == ["Archiv.Neu"] + assert fake.created == ["Archiv.Neu"] + + +def test_ensure_imap_folder_raises_when_creation_fails(monkeypatch) -> None: + _patch_imap(monkeypatch) + with mail_delivery.imap_session(IMAP_SETTINGS) as client: + client.select_result = ("NO", [b"no such mailbox"]) + client.create_result = ("NO", [b"Permission denied"]) + with pytest.raises(RepositoryError, match="Permission denied"): + mail_delivery.ensure_imap_folder(client, "Verboten") + + +@pytest.mark.parametrize( + ("plain", "wire"), + [ + ("INBOX", "INBOX"), + ("Entwürfe", "Entw&APw-rfe"), + ("INBOX.Entwürfe", "INBOX.Entw&APw-rfe"), + ("Test & Foo", "Test &- Foo"), + ], +) +def test_imap_utf7_round_trips(plain, wire) -> None: + assert mail_delivery._encode_imap_utf7(plain) == wire + assert mail_delivery._decode_imap_utf7(wire) == plain + + +def test_list_imap_folders_parses_and_decodes_list_response(monkeypatch) -> None: + _patch_imap(monkeypatch) + _FakeImapClient.default_list_result = ( + "OK", + [ + rb'(\HasNoChildren) "." INBOX', + rb'(\HasNoChildren) "." INBOX.Entw&APw-rfe', + rb'(\HasNoChildren) "." "INBOX.Gesendete Objekte"', + ], + ) + + folders = mail_delivery.list_imap_folders(IMAP_SETTINGS) + + assert folders == ["INBOX", "INBOX.Entwürfe", "INBOX.Gesendete Objekte"] + assert _FakeImapClient.instances[0].calls == ["login", "list", "logout"] def test_smtp_session_wraps_connection_errors(monkeypatch) -> None: diff --git a/tests/test_reminder_mail.py b/tests/test_reminder_mail.py index a910be1..985ea43 100644 --- a/tests/test_reminder_mail.py +++ b/tests/test_reminder_mail.py @@ -137,3 +137,66 @@ def test_reminder_mail_send_mode_delivers_via_smtp_without_local_file(tmp_path, assert len(sent) == 1 data = repository.get_contributions(member.member_id) assert data.reminders[0]["status"] == "sent" + + +def test_reminder_mail_send_mode_also_copies_to_sent_folder_when_enabled(tmp_path, monkeypatch): + import ccma.services.reminder_mail as reminder_mail_module + + repository, member = _overdue_claim_repository(tmp_path / "store") + member.email = "reminder@example.org" + repository.save_member(member) + repository.save_email_settings( + delivery_mode="send", + smtp_host="mail.example.org", + smtp_port=587, + smtp_security="none", + smtp_username="", + smtp_password="", + imap_host="mail.example.org", + imap_port=993, + imap_security="ssl", + imap_username="", + imap_password="", + imap_drafts_folder="", + imap_sent_enabled=True, + imap_sent_folder="INBOX.Gesendet", + ) + reminder = repository.create_reminder_draft( + member.member_id, + "claim-1", + level=1, + name="Zahlungserinnerung", + payment_deadline_days=14, + ) + + monkeypatch.setattr( + reminder_mail_module, "smtp_session", contextmanager(lambda settings: iter(["smtp-client"])) + ) + monkeypatch.setattr(reminder_mail_module, "send_via_smtp", lambda client, content: None) + monkeypatch.setattr( + reminder_mail_module, "imap_session", contextmanager(lambda settings: iter(["imap-client"])) + ) + ensured = [] + appended = [] + monkeypatch.setattr( + reminder_mail_module, "ensure_imap_folder", lambda client, folder: ensured.append((client, folder)) + ) + monkeypatch.setattr( + reminder_mail_module, + "append_message", + lambda client, content, *, folder, flags: appended.append((client, folder, flags)), + ) + + generate_and_send_reminder_mail( + repository, + member.member_id, + "claim-1", + reminder["reminder_id"], + delivery_mode="send", + sender_name="Verwaltung C3MA", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + + assert ensured == [("imap-client", "INBOX.Gesendet")] + assert appended == [("imap-client", "INBOX.Gesendet", r"(\Seen)")] diff --git a/tests/test_sepa.py b/tests/test_sepa.py index b2d3daa..bded358 100644 --- a/tests/test_sepa.py +++ b/tests/test_sepa.py @@ -290,3 +290,101 @@ def test_generated_mail_send_mode_reuses_one_smtp_connection_for_batch(tmp_path, assert generated[0].archive_path.is_file() assert len(connections_opened) == 1, "one connection should be reused for the whole batch" assert len(sent) == len(debits) + + +def test_generated_mail_send_mode_copies_to_sent_folder_reusing_one_imap_connection( + tmp_path, monkeypatch +): + import ccma.services.sepa_mail as sepa_mail_module + + repository, ada = _repository(tmp_path) + second = repository.create_member(first_name="Grace", last_name="Hopper", member_number="C3-43") + second.status = "active" + second.email = "grace@example.org" + second.account_holder = "Grace Hopper" + second.iban = "DE89370400440532013000" + second.bic = "COBADEFFXXX" + second.mandate_reference = "MANDAT-43" + second.mandate_signed_at = "2025-01-10" + second.mandate_active = True + repository.save_member(second) + repository.save_contributions( + second.member_id, + ContributionData( + claims=[ + { + "claim_id": "due", + "title": "Mitgliedsbeitrag 2026", + "amount": "150.00", + "due_date": "2026-01-31", + "status": "open", + } + ] + ), + ) + organization = repository.get_configuration()["organization"] + organization.update( + { + "name": "Chaos Computer Club Mannheim e.V.", + "email": "verwaltung@example.org", + "iban": "DE89370400440532013000", + "bic": "COBADEFFXXX", + "creditor_id": "DE98ZZZ09999999999", + } + ) + repository.save_organization(organization) + repository.save_email_settings( + delivery_mode="send", + smtp_host="mail.example.org", + smtp_port=587, + smtp_security="none", + smtp_username="", + smtp_password="", + imap_host="mail.example.org", + imap_port=993, + imap_security="ssl", + imap_username="", + imap_password="", + imap_drafts_folder="", + imap_sent_enabled=True, + imap_sent_folder="INBOX.Gesendet", + ) + debits, _warnings = pending_direct_debits(repository, due_until=date(2026, 12, 31)) + assert len(debits) == 2 + + monkeypatch.setattr( + sepa_mail_module, "smtp_session", contextmanager(lambda settings: iter(["smtp-client"])) + ) + monkeypatch.setattr(sepa_mail_module, "send_via_smtp", lambda client, content: None) + imap_connections = [] + + @contextmanager + def fake_imap_session(settings): + imap_connections.append(settings) + yield "imap-client" + + monkeypatch.setattr(sepa_mail_module, "imap_session", fake_imap_session) + ensured = [] + appended = [] + monkeypatch.setattr( + sepa_mail_module, "ensure_imap_folder", lambda client, folder: ensured.append(folder) + ) + monkeypatch.setattr( + sepa_mail_module, + "append_message", + lambda client, content, *, folder, flags: appended.append((client, folder, flags)), + ) + + generate_debit_mails( + repository, + debits, + collection_date=date(2026, 8, 3), + delivery_mode="send", + sender_name="Verwaltung C3MA", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + + assert len(imap_connections) == 1, "one IMAP connection should be reused for the whole batch" + assert ensured == ["INBOX.Gesendet"] + assert appended == [("imap-client", "INBOX.Gesendet", r"(\Seen)")] * 2 From c68e8b7d6aa30eab53a0a783603932b893c3966e Mon Sep 17 00:00:00 2001 From: Marcel Peterkau Date: Sat, 15 Aug 2026 04:32:06 +0200 Subject: [PATCH 3/6] Document the configurable e-mail delivery and Sent-folder copy features Co-Authored-By: Claude Sonnet 5 --- src/ccma/assets/CHANGELOG.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ccma/assets/CHANGELOG.json b/src/ccma/assets/CHANGELOG.json index 30eac7d..3708993 100644 --- a/src/ccma/assets/CHANGELOG.json +++ b/src/ccma/assets/CHANGELOG.json @@ -36,7 +36,9 @@ "Eine überfällige Forderung, für die bereits eine Mahnung versandt wurde und deren Zahlungsfrist noch läuft, wird vom Hausmeister nicht mehr fälschlich als unbearbeiteter Fehler gemeldet, sondern als niedrig priorisierter Hinweis, dass die laufende Frist noch nicht abgelaufen ist.", "Geburtstags- und Mitgliedsjubiläums-Hinweise des Hausmeisters werden für Mitglieder mit beendeter Mitgliedschaft nicht mehr angezeigt.", "Eine überfällige Lastschrift, für die bereits eine Rücklastschrift-Mahnung versandt wurde und deren Zahlungsfrist noch läuft, wird vom Hausmeister nicht mehr weiterhin als überfällig gemeldet, sondern als niedrig priorisierter Hinweis, dass die Klärung noch läuft.", - "Im SEPA-Export lassen sich einzelne Lastschriften per Mehrfachauswahl von der CSV-/XML-/Info-Mail-Erstellung ausschließen (z. B. während eine Rücklastschrift geklärt wird), statt immer alle offenen Lastschriften einziehen zu müssen; standardmäßig sind alle ausgewählt." + "Im SEPA-Export lassen sich einzelne Lastschriften per Mehrfachauswahl von der CSV-/XML-/Info-Mail-Erstellung ausschließen (z. B. während eine Rücklastschrift geklärt wird), statt immer alle offenen Lastschriften einziehen zu müssen; standardmäßig sind alle ausgewählt.", + "Mahnungs- und SEPA-Info-Mails müssen nicht mehr zwingend als lokale Datei gespeichert und von Hand in Thunderbird importiert werden: unter Optionen → E-Mail-Versand kann ein Postfach mit SMTP- und IMAP-Zugangsdaten hinterlegt werden, wahlweise mit direktem Versand, Ablage als Entwurf im Postfach oder einer Rückfrage bei jeder Erzeugung; die bisherige lokale Ablage bleibt als Standard erhalten.", + "Direkt versandte Mahnungen und SEPA-Info-Mails können zusätzlich automatisch in einen Gesendet-Ordner des Postfachs abgelegt werden. Für den Entwürfe- und den Gesendet-Ordner lässt sich die tatsächliche Ordnerliste live vom Server abrufen und per Dropdown auswählen; ein noch nicht vorhandener, frei eingegebener Ordner wird bei Bedarf automatisch angelegt." ] }, { From aab43cc0fea085913b494d8d6aa38681dd0dab6b Mon Sep 17 00:00:00 2001 From: Marcel Peterkau Date: Thu, 20 Aug 2026 12:08:44 +0200 Subject: [PATCH 4/6] Roll back reminder status when direct send/draft delivery fails generate_and_send_reminder_mail marked a reminder as "sent" before attempting SMTP send/IMAP append. If the connection, login, or append failed, the reminder stayed persisted as sent even though no mail was ever delivered or filed, so it could not be corrected and resent. Wrap content generation, archiving, and delivery in a try/except that calls the existing revert_reminder_sent on any failure and re-raises, putting the reminder back into "draft" for all three delivery modes. --- src/ccma/services/reminder_mail.py | 105 +++++++++++++++-------------- tests/test_reminder_mail.py | 101 +++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 49 deletions(-) diff --git a/src/ccma/services/reminder_mail.py b/src/ccma/services/reminder_mail.py index 2d7413f..00b6ec3 100644 --- a/src/ccma/services/reminder_mail.py +++ b/src/ccma/services/reminder_mail.py @@ -152,56 +152,63 @@ def generate_and_send_reminder_mail( raise RepositoryError(f"Die Ausgabedatei existiert bereits: {target}") sent_reminder = repository.mark_reminder_sent(member_id, claim_id, reminder_id) - updated_data, updated_claim = repository.get_claim(member_id, claim_id) - organization = repository.get_configuration().get("organization") or {} - content = reminder_mail_bytes( - recipient=member.email, - first_name=member.first_name, - reminder=sent_reminder, - claim=updated_claim, - balance=f"{claim_balance(updated_data, updated_claim):.2f}", - organization=organization, - sender_name=sender_name, - sender_email=sender_email, - signature=signature, - ) - archive_dir = repository.members_root / member_id / "files" / "documents" / "Mahnungen" - archive_dir.mkdir(parents=True, exist_ok=True) - filename = ( - f"{datetime.now().date().isoformat()}-" - f"{_safe_filename(str(sent_reminder.get('name', 'Mahnung')))}.eml" - ) - archive_path = _available_path(archive_dir, filename) - archive_temp = archive_path.with_name(f".{archive_path.name}.tmp") - export_path: Path | None = None try: - archive_temp.write_bytes(content) - if delivery_mode == "local": - export_temp = target.with_name(f".{target.name}.tmp") - try: - export_temp.write_bytes(content) - os.replace(export_temp, target) - finally: - export_temp.unlink(missing_ok=True) - export_path = target - elif delivery_mode == "send": - settings = repository.get_email_settings() - sent_copy = imap_session(settings) if settings["imap_sent_enabled"] else nullcontext(None) - with smtp_session(settings) as smtp_client, sent_copy as imap_client: - send_via_smtp(smtp_client, content) - if imap_client is not None: - ensure_imap_folder(imap_client, settings["imap_sent_folder"]) - append_message( - imap_client, content, folder=settings["imap_sent_folder"], flags=r"(\Seen)" - ) - else: - settings = repository.get_email_settings() - with imap_session(settings) as client: - ensure_imap_folder(client, settings["imap_drafts_folder"]) - append_message(client, content, folder=settings["imap_drafts_folder"], flags=r"(\Draft)") - os.replace(archive_temp, archive_path) - finally: - archive_temp.unlink(missing_ok=True) + updated_data, updated_claim = repository.get_claim(member_id, claim_id) + organization = repository.get_configuration().get("organization") or {} + content = reminder_mail_bytes( + recipient=member.email, + first_name=member.first_name, + reminder=sent_reminder, + claim=updated_claim, + balance=f"{claim_balance(updated_data, updated_claim):.2f}", + organization=organization, + sender_name=sender_name, + sender_email=sender_email, + signature=signature, + ) + archive_dir = repository.members_root / member_id / "files" / "documents" / "Mahnungen" + archive_dir.mkdir(parents=True, exist_ok=True) + filename = ( + f"{datetime.now().date().isoformat()}-" + f"{_safe_filename(str(sent_reminder.get('name', 'Mahnung')))}.eml" + ) + archive_path = _available_path(archive_dir, filename) + archive_temp = archive_path.with_name(f".{archive_path.name}.tmp") + export_path: Path | None = None + try: + archive_temp.write_bytes(content) + if delivery_mode == "local": + export_temp = target.with_name(f".{target.name}.tmp") + try: + export_temp.write_bytes(content) + os.replace(export_temp, target) + finally: + export_temp.unlink(missing_ok=True) + export_path = target + elif delivery_mode == "send": + settings = repository.get_email_settings() + sent_copy = imap_session(settings) if settings["imap_sent_enabled"] else nullcontext(None) + with smtp_session(settings) as smtp_client, sent_copy as imap_client: + send_via_smtp(smtp_client, content) + if imap_client is not None: + ensure_imap_folder(imap_client, settings["imap_sent_folder"]) + append_message( + imap_client, content, folder=settings["imap_sent_folder"], flags=r"(\Seen)" + ) + else: + settings = repository.get_email_settings() + with imap_session(settings) as client: + ensure_imap_folder(client, settings["imap_drafts_folder"]) + append_message(client, content, folder=settings["imap_drafts_folder"], flags=r"(\Draft)") + os.replace(archive_temp, archive_path) + finally: + archive_temp.unlink(missing_ok=True) + except Exception: + # The mail was not (verifiably) delivered/filed -- roll the reminder back to + # "draft" so it does not sit in the "sent" state without ever having gone out, + # and so it can be corrected and sent again. + repository.revert_reminder_sent(member_id, claim_id, reminder_id) + raise relative_path = archive_path.relative_to( repository.members_root / member_id / "files" ).as_posix() diff --git a/tests/test_reminder_mail.py b/tests/test_reminder_mail.py index 985ea43..2a2fb97 100644 --- a/tests/test_reminder_mail.py +++ b/tests/test_reminder_mail.py @@ -2,9 +2,11 @@ from contextlib import contextmanager from email.parser import BytesParser from email.policy import default +import pytest from test_reminders import _overdue_claim_repository from ccma.services.reminder_mail import generate_and_send_reminder_mail +from ccma.storage.repository import RepositoryError def test_reminder_mail_is_archived_and_marks_reminder_sent(tmp_path): @@ -200,3 +202,102 @@ def test_reminder_mail_send_mode_also_copies_to_sent_folder_when_enabled(tmp_pat assert ensured == [("imap-client", "INBOX.Gesendet")] assert appended == [("imap-client", "INBOX.Gesendet", r"(\Seen)")] + + +def test_reminder_mail_send_failure_reverts_sent_status(tmp_path, monkeypatch): + import ccma.services.reminder_mail as reminder_mail_module + + repository, member = _overdue_claim_repository(tmp_path / "store") + member.email = "reminder@example.org" + repository.save_member(member) + repository.save_email_settings( + delivery_mode="send", + smtp_host="mail.example.org", + smtp_port=587, + smtp_security="none", + smtp_username="", + smtp_password="", + imap_host="", + imap_port=993, + imap_security="ssl", + imap_username="", + imap_password="", + imap_drafts_folder="", + ) + reminder = repository.create_reminder_draft( + member.member_id, + "claim-1", + level=1, + name="Zahlungserinnerung", + payment_deadline_days=14, + items=[{"description": "Mahngebühr", "amount": "5.00"}], + ) + + @contextmanager + def _fake_smtp_session(settings): + yield "smtp-client" + + monkeypatch.setattr(reminder_mail_module, "smtp_session", _fake_smtp_session) + + def _failing_send(client, content): + raise RepositoryError("E-Mail konnte nicht versandt werden: connection reset") + + monkeypatch.setattr(reminder_mail_module, "send_via_smtp", _failing_send) + + with pytest.raises(RepositoryError): + generate_and_send_reminder_mail( + repository, + member.member_id, + "claim-1", + reminder["reminder_id"], + delivery_mode="send", + sender_name="Verwaltung C3MA", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + + data = repository.get_contributions(member.member_id) + stored_reminder = data.reminders[0] + assert stored_reminder["status"] == "draft" + assert not stored_reminder.get("fee_item_ids") + assert repository.get_events(member.member_id)[-1].event_type == "reminder_sent_reverted" + _data, claim = repository.get_claim(member.member_id, "claim-1") + assert not any( + item.get("description") == "Mahngebühr" for item in claim.get("items", []) + ) + + +def test_reminder_mail_local_write_failure_reverts_sent_status(tmp_path, monkeypatch): + repository, member = _overdue_claim_repository(tmp_path / "store") + member.email = "reminder@example.org" + repository.save_member(member) + reminder = repository.create_reminder_draft( + member.member_id, + "claim-1", + level=1, + name="Zahlungserinnerung", + payment_deadline_days=14, + ) + + import ccma.services.reminder_mail as reminder_mail_module + + def _boom(*args, **kwargs): + raise OSError("disk full") + + monkeypatch.setattr(reminder_mail_module.os, "replace", _boom) + + with pytest.raises(OSError): + generate_and_send_reminder_mail( + repository, + member.member_id, + "claim-1", + reminder["reminder_id"], + delivery_mode="local", + output_path=tmp_path / "Zahlungserinnerung.eml", + sender_name="Verwaltung C3MA", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + + data = repository.get_contributions(member.member_id) + assert data.reminders[0]["status"] == "draft" From 5fc6d7aec9e9801a423d1dda7981fda664104925 Mon Sep 17 00:00:00 2001 From: Marcel Peterkau Date: Thu, 20 Aug 2026 12:22:37 +0200 Subject: [PATCH 5/6] Stop rolling back reminders/SEPA mails after a successful send The previous fix reverted the reminder to "draft" on any failure after mark_reminder_sent, including failures that happened after the mail had already been handed to send_via_smtp or appended to an IMAP folder. That made a successful send followed by a Sent-folder-copy or archive-move failure look like nothing was sent, inviting a duplicate send/fee booking on retry -- the same "sent" flag needs to be preserved once delivery is no longer reversible, per follow-up review. reminder_mail.generate_and_send_reminder_mail and sepa_mail.generate_debit_mails now track whether the mail actually left the building (SMTP accepted / IMAP append succeeded / local file written) separately from the later archiving step: - Failure before that point: reminder_mail reverts to draft (unchanged); sepa_mail now records a warning and continues with the remaining debits instead of aborting the whole batch. - Failure after that point (Sent-copy append, moving the archive file into place): the reminder stays "sent" / the debit stays in the batch's results, an event is still logged for traceability (with an archive_error note and no document reference), and the caller gets a clear error to follow up on manually -- no rollback, no silent loss of the fact that the mail already went out. --- src/ccma/services/reminder_mail.py | 129 +++++++++++-------- src/ccma/services/sepa_mail.py | 79 +++++++----- tests/test_reminder_mail.py | 69 +++++++++++ tests/test_sepa.py | 191 +++++++++++++++++++++++++++++ 4 files changed, 386 insertions(+), 82 deletions(-) diff --git a/src/ccma/services/reminder_mail.py b/src/ccma/services/reminder_mail.py index 00b6ec3..81d6ce9 100644 --- a/src/ccma/services/reminder_mail.py +++ b/src/ccma/services/reminder_mail.py @@ -166,69 +166,90 @@ def generate_and_send_reminder_mail( sender_email=sender_email, signature=signature, ) - archive_dir = repository.members_root / member_id / "files" / "documents" / "Mahnungen" - archive_dir.mkdir(parents=True, exist_ok=True) - filename = ( - f"{datetime.now().date().isoformat()}-" - f"{_safe_filename(str(sent_reminder.get('name', 'Mahnung')))}.eml" - ) - archive_path = _available_path(archive_dir, filename) - archive_temp = archive_path.with_name(f".{archive_path.name}.tmp") - export_path: Path | None = None - try: - archive_temp.write_bytes(content) - if delivery_mode == "local": - export_temp = target.with_name(f".{target.name}.tmp") - try: - export_temp.write_bytes(content) - os.replace(export_temp, target) - finally: - export_temp.unlink(missing_ok=True) - export_path = target - elif delivery_mode == "send": - settings = repository.get_email_settings() - sent_copy = imap_session(settings) if settings["imap_sent_enabled"] else nullcontext(None) - with smtp_session(settings) as smtp_client, sent_copy as imap_client: - send_via_smtp(smtp_client, content) - if imap_client is not None: - ensure_imap_folder(imap_client, settings["imap_sent_folder"]) - append_message( - imap_client, content, folder=settings["imap_sent_folder"], flags=r"(\Seen)" - ) - else: - settings = repository.get_email_settings() - with imap_session(settings) as client: - ensure_imap_folder(client, settings["imap_drafts_folder"]) - append_message(client, content, folder=settings["imap_drafts_folder"], flags=r"(\Draft)") - os.replace(archive_temp, archive_path) - finally: - archive_temp.unlink(missing_ok=True) except Exception: - # The mail was not (verifiably) delivered/filed -- roll the reminder back to - # "draft" so it does not sit in the "sent" state without ever having gone out, - # and so it can be corrected and sent again. repository.revert_reminder_sent(member_id, claim_id, reminder_id) raise - relative_path = archive_path.relative_to( - repository.members_root / member_id / "files" - ).as_posix() + archive_dir = repository.members_root / member_id / "files" / "documents" / "Mahnungen" + archive_dir.mkdir(parents=True, exist_ok=True) + filename = ( + f"{datetime.now().date().isoformat()}-" + f"{_safe_filename(str(sent_reminder.get('name', 'Mahnung')))}.eml" + ) + archive_path = _available_path(archive_dir, filename) + archive_temp = archive_path.with_name(f".{archive_path.name}.tmp") + export_path: Path | None = None + # Once the mail has actually left the building -- landed on the SMTP server, been + # filed into an IMAP folder, or been written to the local export file -- it must + # not be un-sent again: a later failure (Sent-folder copy, moving the archive file + # into place) can no longer roll the reminder back to "draft", or a retry could + # send/file the same Mahnung a second time and double-book its fee. + delivered = False + archive_failure: Exception | None = None + try: + archive_temp.write_bytes(content) + if delivery_mode == "local": + export_temp = target.with_name(f".{target.name}.tmp") + try: + export_temp.write_bytes(content) + os.replace(export_temp, target) + finally: + export_temp.unlink(missing_ok=True) + export_path = target + delivered = True + elif delivery_mode == "send": + settings = repository.get_email_settings() + sent_copy = imap_session(settings) if settings["imap_sent_enabled"] else nullcontext(None) + with smtp_session(settings) as smtp_client, sent_copy as imap_client: + send_via_smtp(smtp_client, content) + delivered = True + if imap_client is not None: + ensure_imap_folder(imap_client, settings["imap_sent_folder"]) + append_message( + imap_client, content, folder=settings["imap_sent_folder"], flags=r"(\Seen)" + ) + else: + settings = repository.get_email_settings() + with imap_session(settings) as client: + ensure_imap_folder(client, settings["imap_drafts_folder"]) + append_message(client, content, folder=settings["imap_drafts_folder"], flags=r"(\Draft)") + delivered = True + os.replace(archive_temp, archive_path) + except Exception as exc: + archive_temp.unlink(missing_ok=True) + if not delivered: + repository.revert_reminder_sent(member_id, claim_id, reminder_id) + raise + archive_failure = exc + digest = hashlib.sha256(content).hexdigest() + references = {"claim_id": claim_id, "reminder_id": reminder_id} + if archive_failure is None: + references["document"] = archive_path.relative_to( + repository.members_root / member_id / "files" + ).as_posix() + data = { + "recipient": member.email, + "sha256": digest, + "payment_deadline": str(sent_reminder.get("payment_deadline", "")), + "delivery_mode": delivery_mode, + } + if archive_failure is not None: + data["archive_error"] = str(archive_failure) + # Log the event even when archiving failed -- the mail was already delivered/filed, + # so this must stay a traceable "sent" record instead of silently vanishing. repository.append_event( member_id, event_type="reminder_email_sent", summary=f"Mahnung per E-Mail versandt: {sent_reminder.get('name', '')}", actor_type="user", actor_name="Vorstand", - references={ - "claim_id": claim_id, - "reminder_id": reminder_id, - "document": relative_path, - }, - data={ - "recipient": member.email, - "sha256": digest, - "payment_deadline": str(sent_reminder.get("payment_deadline", "")), - "delivery_mode": delivery_mode, - }, + references=references, + data=data, ) + if archive_failure is not None: + raise RepositoryError( + "Die Mahnung wurde zugestellt bzw. abgelegt, konnte aber nicht vollständig " + f"archiviert werden: {archive_failure}. Die Mahnung bleibt als versandt gebucht " + "-- bitte die Mitgliedsakte manuell prüfen." + ) from archive_failure return GeneratedReminderMail(export_path, archive_path, digest) diff --git a/src/ccma/services/sepa_mail.py b/src/ccma/services/sepa_mail.py index 728da29..c7a8c00 100644 --- a/src/ccma/services/sepa_mail.py +++ b/src/ccma/services/sepa_mail.py @@ -166,43 +166,66 @@ def generate_debit_mails( archive_dir.mkdir(parents=True, exist_ok=True) archive_path = _available_path(archive_dir, filename) export_path: Path | None = None - if delivery_mode == "local": - export_path = _available_path(output, filename) - export_path.write_bytes(content) - elif delivery_mode == "send": - send_via_smtp(smtp_client, content) - if imap_client is not None: - append_message( - imap_client, content, folder=email_settings["imap_sent_folder"], flags=r"(\Seen)" - ) - else: - append_message( - imap_client, content, folder=email_settings["imap_drafts_folder"], flags=r"(\Draft)" - ) + # Once the mail has actually left the building for this debit -- SMTP + # accepted it, it's filed in the IMAP folder, or the local export file was + # written -- a later archiving failure must not abort the whole batch and + # lose track of the fact that this one was already delivered; nor may it + # abort earlier/later debits that have nothing to do with this failure. + delivered = False + archive_failure: Exception | None = None try: + if delivery_mode == "local": + export_path = _available_path(output, filename) + export_path.write_bytes(content) + delivered = True + elif delivery_mode == "send": + send_via_smtp(smtp_client, content) + delivered = True + if imap_client is not None: + append_message( + imap_client, content, folder=email_settings["imap_sent_folder"], flags=r"(\Seen)" + ) + else: + append_message( + imap_client, content, folder=email_settings["imap_drafts_folder"], flags=r"(\Draft)" + ) + delivered = True archive_path.write_bytes(content) - except OSError: - if export_path is not None: - export_path.unlink(missing_ok=True) - raise - relative_path = archive_path.relative_to( - repository.members_root / member.member_id / "files" - ).as_posix() + except (OSError, RepositoryError) as exc: + if not delivered: + if export_path is not None: + export_path.unlink(missing_ok=True) + warnings.append(f"{member.member_number or member.display_name}: {exc}") + continue + warnings.append( + f"{member.member_number or member.display_name}: E-Mail wurde versandt/abgelegt, " + f"konnte aber nicht archiviert werden ({exc}); bitte manuell prüfen." + ) + archive_failure = exc + digest = hashlib.sha256(content).hexdigest() + references = {} + if archive_failure is None: + references["document"] = archive_path.relative_to( + repository.members_root / member.member_id / "files" + ).as_posix() + data = { + "sha256": digest, + "recipient": member.email, + "collection_date": collection_date.isoformat(), + "amount": f"{debit.amount:.2f}", + "delivery_mode": delivery_mode, + } + if archive_failure is not None: + data["archive_error"] = str(archive_failure) repository.append_event( member.member_id, event_type="sepa_notification_generated", summary=f"SEPA-Info-Mail erzeugt: {archive_path.name}", actor_type="user", actor_name="Vorstand", - references={"document": relative_path}, - data={ - "sha256": digest, - "recipient": member.email, - "collection_date": collection_date.isoformat(), - "amount": f"{debit.amount:.2f}", - "delivery_mode": delivery_mode, - }, + references=references, + data=data, ) generated.append( GeneratedDebitMail(member.member_id, member.email, export_path, archive_path) diff --git a/tests/test_reminder_mail.py b/tests/test_reminder_mail.py index 2a2fb97..c1beb8f 100644 --- a/tests/test_reminder_mail.py +++ b/tests/test_reminder_mail.py @@ -301,3 +301,72 @@ def test_reminder_mail_local_write_failure_reverts_sent_status(tmp_path, monkeyp data = repository.get_contributions(member.member_id) assert data.reminders[0]["status"] == "draft" + + +def test_reminder_mail_archiving_failure_after_smtp_success_does_not_revert(tmp_path, monkeypatch): + import ccma.services.reminder_mail as reminder_mail_module + + repository, member = _overdue_claim_repository(tmp_path / "store") + member.email = "reminder@example.org" + repository.save_member(member) + repository.save_email_settings( + delivery_mode="send", + smtp_host="mail.example.org", + smtp_port=587, + smtp_security="none", + smtp_username="", + smtp_password="", + imap_host="", + imap_port=993, + imap_security="ssl", + imap_username="", + imap_password="", + imap_drafts_folder="", + ) + reminder = repository.create_reminder_draft( + member.member_id, + "claim-1", + level=1, + name="Zahlungserinnerung", + payment_deadline_days=14, + items=[{"description": "Mahngebühr", "amount": "5.00"}], + ) + + @contextmanager + def _fake_smtp_session(settings): + yield "smtp-client" + + monkeypatch.setattr(reminder_mail_module, "smtp_session", _fake_smtp_session) + monkeypatch.setattr(reminder_mail_module, "send_via_smtp", lambda client, content: None) + + real_replace = reminder_mail_module.os.replace + + def _flaky_replace(src, dst): + # Only the final archive move (into the "Mahnungen" folder) should fail -- + # the mail has already been handed to send_via_smtp by that point. + if str(dst).endswith(".eml") and "Mahnungen" in str(dst): + raise OSError("disk full") + return real_replace(src, dst) + + monkeypatch.setattr(reminder_mail_module.os, "replace", _flaky_replace) + + with pytest.raises(RepositoryError, match="zugestellt"): + generate_and_send_reminder_mail( + repository, + member.member_id, + "claim-1", + reminder["reminder_id"], + delivery_mode="send", + sender_name="Verwaltung C3MA", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + + data = repository.get_contributions(member.member_id) + stored_reminder = data.reminders[0] + assert stored_reminder["status"] == "sent", "mail was already sent -- must not be un-sent" + assert stored_reminder.get("fee_item_ids") + events = repository.get_events(member.member_id) + assert events[-1].event_type == "reminder_email_sent" + assert events[-1].data["archive_error"] + assert "document" not in events[-1].references diff --git a/tests/test_sepa.py b/tests/test_sepa.py index bded358..d057d4e 100644 --- a/tests/test_sepa.py +++ b/tests/test_sepa.py @@ -388,3 +388,194 @@ def test_generated_mail_send_mode_copies_to_sent_folder_reusing_one_imap_connect assert len(imap_connections) == 1, "one IMAP connection should be reused for the whole batch" assert ensured == ["INBOX.Gesendet"] assert appended == [("imap-client", "INBOX.Gesendet", r"(\Seen)")] * 2 + + +def test_generated_mail_archiving_failure_after_smtp_success_keeps_batch_going(tmp_path, monkeypatch): + import ccma.services.sepa_mail as sepa_mail_module + + repository, ada = _repository(tmp_path) + second = repository.create_member(first_name="Grace", last_name="Hopper", member_number="C3-43") + second.status = "active" + second.email = "grace@example.org" + second.account_holder = "Grace Hopper" + second.iban = "DE89370400440532013000" + second.bic = "COBADEFFXXX" + second.mandate_reference = "MANDAT-43" + second.mandate_signed_at = "2025-01-10" + second.mandate_active = True + repository.save_member(second) + repository.save_contributions( + second.member_id, + ContributionData( + claims=[ + { + "claim_id": "due", + "title": "Mitgliedsbeitrag 2026", + "amount": "150.00", + "due_date": "2026-01-31", + "status": "open", + } + ] + ), + ) + organization = repository.get_configuration()["organization"] + organization.update( + { + "name": "Chaos Computer Club Mannheim e.V.", + "email": "verwaltung@example.org", + "iban": "DE89370400440532013000", + "bic": "COBADEFFXXX", + "creditor_id": "DE98ZZZ09999999999", + } + ) + repository.save_organization(organization) + repository.save_email_settings( + delivery_mode="send", + smtp_host="mail.example.org", + smtp_port=587, + smtp_security="none", + smtp_username="", + smtp_password="", + imap_host="", + imap_port=993, + imap_security="ssl", + imap_username="", + imap_password="", + imap_drafts_folder="", + ) + debits, _warnings = pending_direct_debits(repository, due_until=date(2026, 12, 31)) + assert len(debits) == 2 + + monkeypatch.setattr( + sepa_mail_module, "smtp_session", contextmanager(lambda settings: iter(["smtp-client"])) + ) + sent = [] + monkeypatch.setattr(sepa_mail_module, "send_via_smtp", lambda client, content: sent.append(content)) + + from pathlib import Path + + original_write_bytes = Path.write_bytes + + def _flaky_write_bytes(self, data): + if "documents" in str(self) and "C3-43" in self.name: + raise OSError("disk full") + return original_write_bytes(self, data) + + monkeypatch.setattr(Path, "write_bytes", _flaky_write_bytes) + + generated, warnings = generate_debit_mails( + repository, + debits, + collection_date=date(2026, 8, 3), + delivery_mode="send", + sender_name="Verwaltung C3MA", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + + # Both mails were handed to SMTP -- a broken archive write for one of them must + # not swallow the other, nor pretend the failing one was never sent. + assert len(sent) == 2 + assert len(generated) == 2 + assert len(warnings) == 1 + assert "C3-43" in warnings[0] or "Grace" in warnings[0] + + failing = next(g for g in generated if g.member_id == second.member_id) + assert not failing.archive_path.is_file() + succeeding = next(g for g in generated if g.member_id == ada.member_id) + assert succeeding.archive_path.is_file() + + grace_event = repository.get_events(second.member_id)[-1] + assert grace_event.event_type == "sepa_notification_generated" + assert grace_event.data["archive_error"] + assert "document" not in grace_event.references + ada_event = repository.get_events(ada.member_id)[-1] + assert "document" in ada_event.references + + +def test_generated_mail_smtp_failure_for_one_debit_does_not_abort_the_batch(tmp_path, monkeypatch): + import ccma.services.sepa_mail as sepa_mail_module + from ccma.storage.repository import RepositoryError + + repository, ada = _repository(tmp_path) + second = repository.create_member(first_name="Grace", last_name="Hopper", member_number="C3-43") + second.status = "active" + second.email = "grace@example.org" + second.account_holder = "Grace Hopper" + second.iban = "DE89370400440532013000" + second.bic = "COBADEFFXXX" + second.mandate_reference = "MANDAT-43" + second.mandate_signed_at = "2025-01-10" + second.mandate_active = True + repository.save_member(second) + repository.save_contributions( + second.member_id, + ContributionData( + claims=[ + { + "claim_id": "due", + "title": "Mitgliedsbeitrag 2026", + "amount": "150.00", + "due_date": "2026-01-31", + "status": "open", + } + ] + ), + ) + organization = repository.get_configuration()["organization"] + organization.update( + { + "name": "Chaos Computer Club Mannheim e.V.", + "email": "verwaltung@example.org", + "iban": "DE89370400440532013000", + "bic": "COBADEFFXXX", + "creditor_id": "DE98ZZZ09999999999", + } + ) + repository.save_organization(organization) + repository.save_email_settings( + delivery_mode="send", + smtp_host="mail.example.org", + smtp_port=587, + smtp_security="none", + smtp_username="", + smtp_password="", + imap_host="", + imap_port=993, + imap_security="ssl", + imap_username="", + imap_password="", + imap_drafts_folder="", + ) + debits, _warnings = pending_direct_debits(repository, due_until=date(2026, 12, 31)) + assert len(debits) == 2 + + monkeypatch.setattr( + sepa_mail_module, "smtp_session", contextmanager(lambda settings: iter(["smtp-client"])) + ) + sent = [] + + def _flaky_send(client, content): + if b"ada@example.org" in content: + raise RepositoryError("E-Mail konnte nicht versandt werden: connection reset") + sent.append(content) + + monkeypatch.setattr(sepa_mail_module, "send_via_smtp", _flaky_send) + + generated, warnings = generate_debit_mails( + repository, + debits, + collection_date=date(2026, 8, 3), + delivery_mode="send", + sender_name="Verwaltung C3MA", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + + assert len(sent) == 1 + assert len(generated) == 1 + assert generated[0].member_id == second.member_id + assert len(warnings) == 1 + assert "connection reset" in warnings[0] + ada_events = repository.get_events(ada.member_id) + assert all(event.event_type != "sepa_notification_generated" for event in ada_events) From a4536eedd077096aa09ef87b8b7dad4e8f4db5de Mon Sep 17 00:00:00 2001 From: Marcel Peterkau Date: Thu, 20 Aug 2026 12:51:17 +0200 Subject: [PATCH 6/6] Cover archive-directory setup in the sent/delivered rollback path archive_dir.mkdir() and the archive path lookup ran outside the try/except that decides whether to revert mark_reminder_sent (reminder mail) or keep a SEPA batch running for the other debits. A failure there (read-only store, full disk) left a reminder booked as "sent" with no mail ever having gone out, and could still abort an entire SEPA batch for one member's directory problem. Moved that setup inside the same try blocks so it's treated exactly like any other pre-delivery failure: reminder_mail reverts to draft, and sepa_mail records a warning and continues with the remaining debits. Also moved debit_mail_bytes() into the per-debit try in sepa_mail for the same reason. Added a targeted mkdir-failure test for each. --- src/ccma/services/reminder_mail.py | 42 ++++++------- src/ccma/services/sepa_mail.py | 37 ++++++------ tests/test_reminder_mail.py | 43 ++++++++++++++ tests/test_sepa.py | 94 ++++++++++++++++++++++++++++++ 4 files changed, 178 insertions(+), 38 deletions(-) diff --git a/src/ccma/services/reminder_mail.py b/src/ccma/services/reminder_mail.py index 81d6ce9..a3a0e26 100644 --- a/src/ccma/services/reminder_mail.py +++ b/src/ccma/services/reminder_mail.py @@ -152,6 +152,17 @@ def generate_and_send_reminder_mail( raise RepositoryError(f"Die Ausgabedatei existiert bereits: {target}") sent_reminder = repository.mark_reminder_sent(member_id, claim_id, reminder_id) + export_path: Path | None = None + # Once the mail has actually left the building -- landed on the SMTP server, been + # filed into an IMAP folder, or been written to the local export file -- it must + # not be un-sent again: a later failure (Sent-folder copy, moving the archive file + # into place) can no longer roll the reminder back to "draft", or a retry could + # send/file the same Mahnung a second time and double-book its fee. Everything + # before that point -- including preparing the archive directory/path -- still + # rolls back on failure, since nothing has actually gone out yet. + delivered = False + archive_failure: Exception | None = None + archive_temp: Path | None = None try: updated_data, updated_claim = repository.get_claim(member_id, claim_id) organization = repository.get_configuration().get("organization") or {} @@ -166,26 +177,14 @@ def generate_and_send_reminder_mail( sender_email=sender_email, signature=signature, ) - except Exception: - repository.revert_reminder_sent(member_id, claim_id, reminder_id) - raise - archive_dir = repository.members_root / member_id / "files" / "documents" / "Mahnungen" - archive_dir.mkdir(parents=True, exist_ok=True) - filename = ( - f"{datetime.now().date().isoformat()}-" - f"{_safe_filename(str(sent_reminder.get('name', 'Mahnung')))}.eml" - ) - archive_path = _available_path(archive_dir, filename) - archive_temp = archive_path.with_name(f".{archive_path.name}.tmp") - export_path: Path | None = None - # Once the mail has actually left the building -- landed on the SMTP server, been - # filed into an IMAP folder, or been written to the local export file -- it must - # not be un-sent again: a later failure (Sent-folder copy, moving the archive file - # into place) can no longer roll the reminder back to "draft", or a retry could - # send/file the same Mahnung a second time and double-book its fee. - delivered = False - archive_failure: Exception | None = None - try: + archive_dir = repository.members_root / member_id / "files" / "documents" / "Mahnungen" + archive_dir.mkdir(parents=True, exist_ok=True) + filename = ( + f"{datetime.now().date().isoformat()}-" + f"{_safe_filename(str(sent_reminder.get('name', 'Mahnung')))}.eml" + ) + archive_path = _available_path(archive_dir, filename) + archive_temp = archive_path.with_name(f".{archive_path.name}.tmp") archive_temp.write_bytes(content) if delivery_mode == "local": export_temp = target.with_name(f".{target.name}.tmp") @@ -215,7 +214,8 @@ def generate_and_send_reminder_mail( delivered = True os.replace(archive_temp, archive_path) except Exception as exc: - archive_temp.unlink(missing_ok=True) + if archive_temp is not None: + archive_temp.unlink(missing_ok=True) if not delivered: repository.revert_reminder_sent(member_id, claim_id, reminder_id) raise diff --git a/src/ccma/services/sepa_mail.py b/src/ccma/services/sepa_mail.py index c7a8c00..e581d58 100644 --- a/src/ccma/services/sepa_mail.py +++ b/src/ccma/services/sepa_mail.py @@ -148,24 +148,8 @@ def generate_debit_mails( if not member.email.strip(): warnings.append(f"{member.member_number or member.display_name}: E-Mail-Adresse fehlt.") continue - content = debit_mail_bytes( - recipient=member.email, - first_name=member.first_name, - debit=debit, - collection_date=collection_date, - creditor_id=creditor_id, - sender_name=sender_name, - sender_email=sender_email, - signature=signature, - ) - filename = ( - f"SEPA-Info-{collection_date.isoformat()}-" - f"{_safe_filename(member.member_number or member.display_name)}.eml" - ) - archive_dir = repository.members_root / member.member_id / "files" / "documents" / "SEPA" - archive_dir.mkdir(parents=True, exist_ok=True) - archive_path = _available_path(archive_dir, filename) export_path: Path | None = None + archive_path: Path | None = None # Once the mail has actually left the building for this debit -- SMTP # accepted it, it's filed in the IMAP folder, or the local export file was # written -- a later archiving failure must not abort the whole batch and @@ -174,6 +158,25 @@ def generate_debit_mails( delivered = False archive_failure: Exception | None = None try: + content = debit_mail_bytes( + recipient=member.email, + first_name=member.first_name, + debit=debit, + collection_date=collection_date, + creditor_id=creditor_id, + sender_name=sender_name, + sender_email=sender_email, + signature=signature, + ) + filename = ( + f"SEPA-Info-{collection_date.isoformat()}-" + f"{_safe_filename(member.member_number or member.display_name)}.eml" + ) + archive_dir = ( + repository.members_root / member.member_id / "files" / "documents" / "SEPA" + ) + archive_dir.mkdir(parents=True, exist_ok=True) + archive_path = _available_path(archive_dir, filename) if delivery_mode == "local": export_path = _available_path(output, filename) export_path.write_bytes(content) diff --git a/tests/test_reminder_mail.py b/tests/test_reminder_mail.py index c1beb8f..318c178 100644 --- a/tests/test_reminder_mail.py +++ b/tests/test_reminder_mail.py @@ -370,3 +370,46 @@ def test_reminder_mail_archiving_failure_after_smtp_success_does_not_revert(tmp_ assert events[-1].event_type == "reminder_email_sent" assert events[-1].data["archive_error"] assert "document" not in events[-1].references + + +def test_reminder_mail_archive_dir_creation_failure_reverts_sent_status(tmp_path, monkeypatch): + from pathlib import Path + + repository, member = _overdue_claim_repository(tmp_path / "store") + member.email = "reminder@example.org" + repository.save_member(member) + reminder = repository.create_reminder_draft( + member.member_id, + "claim-1", + level=1, + name="Zahlungserinnerung", + payment_deadline_days=14, + items=[{"description": "Mahngebühr", "amount": "5.00"}], + ) + + original_mkdir = Path.mkdir + + def _flaky_mkdir(self, *args, **kwargs): + if self.name == "Mahnungen": + raise OSError("permission denied") + return original_mkdir(self, *args, **kwargs) + + monkeypatch.setattr(Path, "mkdir", _flaky_mkdir) + + with pytest.raises(OSError): + generate_and_send_reminder_mail( + repository, + member.member_id, + "claim-1", + reminder["reminder_id"], + delivery_mode="local", + output_path=tmp_path / "Zahlungserinnerung.eml", + sender_name="Verwaltung C3MA", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + + data = repository.get_contributions(member.member_id) + stored_reminder = data.reminders[0] + assert stored_reminder["status"] == "draft" + assert not stored_reminder.get("fee_item_ids") diff --git a/tests/test_sepa.py b/tests/test_sepa.py index d057d4e..9469db3 100644 --- a/tests/test_sepa.py +++ b/tests/test_sepa.py @@ -579,3 +579,97 @@ def test_generated_mail_smtp_failure_for_one_debit_does_not_abort_the_batch(tmp_ assert "connection reset" in warnings[0] ada_events = repository.get_events(ada.member_id) assert all(event.event_type != "sepa_notification_generated" for event in ada_events) + + +def test_generated_mail_archive_dir_creation_failure_for_one_debit_does_not_abort_the_batch( + tmp_path, monkeypatch +): + from pathlib import Path + + repository, ada = _repository(tmp_path) + second = repository.create_member(first_name="Grace", last_name="Hopper", member_number="C3-43") + second.status = "active" + second.email = "grace@example.org" + second.account_holder = "Grace Hopper" + second.iban = "DE89370400440532013000" + second.bic = "COBADEFFXXX" + second.mandate_reference = "MANDAT-43" + second.mandate_signed_at = "2025-01-10" + second.mandate_active = True + repository.save_member(second) + repository.save_contributions( + second.member_id, + ContributionData( + claims=[ + { + "claim_id": "due", + "title": "Mitgliedsbeitrag 2026", + "amount": "150.00", + "due_date": "2026-01-31", + "status": "open", + } + ] + ), + ) + organization = repository.get_configuration()["organization"] + organization.update( + { + "name": "Chaos Computer Club Mannheim e.V.", + "email": "verwaltung@example.org", + "iban": "DE89370400440532013000", + "bic": "COBADEFFXXX", + "creditor_id": "DE98ZZZ09999999999", + } + ) + repository.save_organization(organization) + repository.save_email_settings( + delivery_mode="send", + smtp_host="mail.example.org", + smtp_port=587, + smtp_security="none", + smtp_username="", + smtp_password="", + imap_host="", + imap_port=993, + imap_security="ssl", + imap_username="", + imap_password="", + imap_drafts_folder="", + ) + debits, _warnings = pending_direct_debits(repository, due_until=date(2026, 12, 31)) + assert len(debits) == 2 + + import ccma.services.sepa_mail as sepa_mail_module + + monkeypatch.setattr( + sepa_mail_module, "smtp_session", contextmanager(lambda settings: iter(["smtp-client"])) + ) + sent = [] + monkeypatch.setattr(sepa_mail_module, "send_via_smtp", lambda client, content: sent.append(content)) + + original_mkdir = Path.mkdir + + def _flaky_mkdir(self, *args, **kwargs): + if self.name == "SEPA" and second.member_id in str(self): + raise OSError("permission denied") + return original_mkdir(self, *args, **kwargs) + + monkeypatch.setattr(Path, "mkdir", _flaky_mkdir) + + generated, warnings = generate_debit_mails( + repository, + debits, + collection_date=date(2026, 8, 3), + delivery_mode="send", + sender_name="Verwaltung C3MA", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + + # Grace's archive directory could not even be created -- nothing was sent to her, + # so she must NOT show up as delivered, but Ada's mail must still go out. + assert len(sent) == 1 + assert len(generated) == 1 + assert generated[0].member_id == ada.member_id + assert len(warnings) == 1 + assert "Grace" in warnings[0] or "C3-43" in warnings[0]