mirror of
https://git.hiabuto.net/C3MA/CCMA.git
synced 2026-08-24 22:45:18 +02:00
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user