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]