From aab43cc0fea085913b494d8d6aa38681dd0dab6b Mon Sep 17 00:00:00 2001 From: Marcel Peterkau Date: Thu, 20 Aug 2026 12:08:44 +0200 Subject: [PATCH] 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"