From aab43cc0fea085913b494d8d6aa38681dd0dab6b Mon Sep 17 00:00:00 2001 From: Marcel Peterkau Date: Thu, 20 Aug 2026 12:08:44 +0200 Subject: [PATCH 1/4] 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 2/4] 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 3/4] 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] From 1972dbabb39a5dbac8ba55378f91f2d5a6eec7bd Mon Sep 17 00:00:00 2001 From: Git-CI Date: Thu, 20 Aug 2026 14:58:51 +0200 Subject: [PATCH 4/4] ci: bump dev version to 0.1.0-dev16 [skip ci] --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 89c5dab..a2f00ca 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.1.0-dev15 +0.1.0-dev16