mirror of
https://git.hiabuto.net/C3MA/CCMA.git
synced 2026-08-24 22:45:18 +02:00
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.
This commit is contained in:
@@ -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,9 +177,6 @@ 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 = (
|
||||
@@ -177,15 +185,6 @@ def generate_and_send_reminder_mail(
|
||||
)
|
||||
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")
|
||||
@@ -215,6 +214,7 @@ def generate_and_send_reminder_mail(
|
||||
delivered = True
|
||||
os.replace(archive_temp, archive_path)
|
||||
except Exception as exc:
|
||||
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)
|
||||
|
||||
@@ -148,6 +148,16 @@ def generate_debit_mails(
|
||||
if not member.email.strip():
|
||||
warnings.append(f"{member.member_number or member.display_name}: E-Mail-Adresse fehlt.")
|
||||
continue
|
||||
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
|
||||
# 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:
|
||||
content = debit_mail_bytes(
|
||||
recipient=member.email,
|
||||
first_name=member.first_name,
|
||||
@@ -162,18 +172,11 @@ def generate_debit_mails(
|
||||
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 = (
|
||||
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
|
||||
# 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)
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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]
|
||||
|
||||
Reference in New Issue
Block a user