From 186af530b08fcc75a16a0b4080fb8d3a39e6f94b Mon Sep 17 00:00:00 2001 From: Marcel Peterkau Date: Sat, 15 Aug 2026 02:50:46 +0200 Subject: [PATCH] =?UTF-8?q?Allow=20editing=20reminders=20(incl.=20undoing?= =?UTF-8?q?=20"sent")=20and=20prefill=20a=20R=C3=BCcklastschrift=20reason?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A generated/sent Mahnung email couldn't be corrected before: sent reminders were locked, so a wording mistake meant living with it. Add edit_reminder_draft() for draft/generated reminders and revert_reminder_sent() to safely undo a "sent" reminder (removing the fee items it booked, refusing if a payment already covers them or a higher Mahnstufe already exists) so it can be edited and resent. ReminderDialog now supports an edit mode (reminder=... prefills name/detail/ items/deadline/channel, level stays fixed) and claim_tab gained a "Mahnung bearbeiten" button that reverts-then-edits for sent reminders automatically, asking for confirmation first since it un-books the fee. The Rücklastschrift preset now also prefills "Details" with a short explanation ("Leider konnte die Lastschrift ... nicht eingelöst werden.") since that field goes straight into the reminder email -- the board can append the concrete reason (mangels Deckung, Konto ungültig, ...) right there instead of the field starting empty. Co-Authored-By: Claude Sonnet 5 --- src/ccma/storage/repository.py | 93 ++++++++++++++++++++++ src/ccma/ui/claim_tab.py | 116 ++++++++++++++++++++++------ tests/test_reminders.py | 136 +++++++++++++++++++++++++++++++++ 3 files changed, 322 insertions(+), 23 deletions(-) diff --git a/src/ccma/storage/repository.py b/src/ccma/storage/repository.py index c78b0a2..c61b98e 100644 --- a/src/ccma/storage/repository.py +++ b/src/ccma/storage/repository.py @@ -17,6 +17,8 @@ from ccma.domain.contributions import ( CONTRIBUTION_OVERRIDE_KIND_LABELS, allocated_total, claim_balance, + claim_items, + claim_settled_total, claim_total, contribution_override_ranges_overlap, credit_allocated_total, @@ -2108,6 +2110,97 @@ class MemberRepository: references={"claim_id": claim_id, "reminder_id": reminder_id}, ) + def edit_reminder_draft( + self, + member_id: str, + claim_id: str, + reminder_id: str, + *, + name: str, + payment_deadline_days: int, + detail: str = "", + items: list[dict[str, str]] | None = None, + channel: str = "email", + ) -> dict: + try: + normalized_items = _normalize_reminder_items(items) + except ValueError as exc: + raise RepositoryError(str(exc)) from exc + if payment_deadline_days < 1 or payment_deadline_days > 365: + raise RepositoryError("Die Zahlungsfrist muss zwischen 1 und 365 Tagen liegen.") + data, _claim = self.get_claim(member_id, claim_id) + reminder = self._find_reminder(data, claim_id, reminder_id) + if str(reminder.get("status", "draft")) not in {"draft", "generated"}: + raise RepositoryError("Nur ein Entwurf kann bearbeitet werden.") + reminder["name"] = name.strip() or reminder["name"] + reminder["detail"] = detail.strip() + reminder["channel"] = channel.strip() or "email" + reminder["payment_deadline_days"] = payment_deadline_days + reminder["items"] = normalized_items + self.save_contributions(member_id, data) + self.append_event( + member_id, + event_type="reminder_draft_edited", + summary=f"Mahnungsentwurf bearbeitet: {reminder['name']}", + references={"claim_id": claim_id, "reminder_id": str(reminder["reminder_id"])}, + data={"fee": money_text(reminder_items_total(reminder))}, + ) + return reminder + + def revert_reminder_sent(self, member_id: str, claim_id: str, reminder_id: str) -> dict: + """Undoes mark_reminder_sent: removes the fee items it booked onto the claim + and puts the reminder back into "draft" so its text/items can be edited and it + can be sent again -- e.g. after generating a Mahnung email and noticing the + wording needs a fix. Refuses if a later Mahnstufe already exists (would break + the level ordering) or if payments already booked against the claim would no + longer fit once the fee is removed.""" + data, claim = self.get_claim(member_id, claim_id) + reminder = self._find_reminder(data, claim_id, reminder_id) + if str(reminder.get("status", "")) != "sent": + raise RepositoryError("Nur eine bereits versandte Mahnung kann zurückgesetzt werden.") + if any( + str(item.get("claim_id", "")) == claim_id + and int(item.get("level", 0)) > int(reminder.get("level", 0)) + and str(item.get("status", "draft")) in {"draft", "generated", "sent"} + for item in data.reminders + ): + raise RepositoryError( + "Es existiert bereits eine höhere Mahnstufe; diese Mahnung kann nicht " + "zurückgesetzt werden." + ) + fee_item_ids = set(reminder.get("fee_item_ids") or []) + if fee_item_ids: + removed_total = sum( + ( + decimal_value(item.get("amount", "0")) + for item in claim_items(claim) + if str(item.get("item_id", "")) in fee_item_ids + ), + Decimal("0"), + ) + new_total = claim_total(claim) - removed_total + if claim_settled_total(data, claim) > new_total: + raise RepositoryError( + "Für diese Forderung wurden bereits Zahlungen verbucht, die die " + "Mahngebühr abdecken; der Versand kann nicht zurückgesetzt werden." + ) + claim["items"] = [ + item for item in claim_items(claim) if str(item.get("item_id", "")) not in fee_item_ids + ] + claim["amount"] = money_text(claim_total(claim)) + reminder["status"] = "draft" + reminder["sent_at"] = None + reminder["payment_deadline"] = None + reminder["fee_item_ids"] = [] + self.save_contributions(member_id, data) + self.append_event( + member_id, + event_type="reminder_sent_reverted", + summary=f"Versand zurückgesetzt: {reminder.get('name', '')}", + references={"claim_id": claim_id, "reminder_id": str(reminder["reminder_id"])}, + ) + return reminder + def register_reminder_document( self, member_id: str, diff --git a/src/ccma/ui/claim_tab.py b/src/ccma/ui/claim_tab.py index 99cd081..d57091c 100644 --- a/src/ccma/ui/claim_tab.py +++ b/src/ccma/ui/claim_tab.py @@ -167,6 +167,10 @@ class ClaimTab(ttk.Frame): state="disabled", ) self.email_reminder_button.pack(side="right", padx=(8, 0)) + self.edit_reminder_button = ttk.Button( + buttons, text="Mahnung bearbeiten", command=self._edit_reminder, state="disabled" + ) + self.edit_reminder_button.pack(side="right", padx=(8, 0)) ttk.Button(buttons, text="Mahnung vorbereiten", command=self._add_reminder).pack(side="right") ttk.Button(buttons, text="Dokument erzeugen", command=self._create_document).pack( side="right", padx=(0, 8) @@ -438,6 +442,9 @@ class ClaimTab(ttk.Frame): self.send_reminder_button.configure(state=state) self.email_reminder_button.configure(state=state) self.discard_reminder_button.configure(state=state) + # Sent reminders can also be edited (revert-then-edit), just not cancelled ones. + can_edit = bool(reminder and str(reminder.get("status", "draft")) != "cancelled") + self.edit_reminder_button.configure(state="normal" if can_edit else "disabled") def _email_reminder(self) -> None: reminder = self._selected_reminder() @@ -467,6 +474,32 @@ class ClaimTab(ttk.Frame): return self._changed() + def _edit_reminder(self) -> None: + reminder = self._selected_reminder() + if not reminder: + return + if str(reminder.get("status", "")) == "sent": + if not messagebox.askyesno( + "Versand zurücksetzen", + "Diese Mahnung wurde bereits versandt. Zum Bearbeiten wird der Versand " + "zurückgesetzt (eine bereits gebuchte Mahngebühr wird von der Forderung " + "entfernt und die Zahlungsfrist gelöscht) -- anschließend muss die Mahnung " + "erneut versandt werden. Fortfahren?", + parent=self, + ): + return + try: + reminder = self.repository.revert_reminder_sent( + self.member_id, self.claim_id, str(reminder["reminder_id"]) + ) + except RepositoryError as exc: + messagebox.showerror("Versand konnte nicht zurückgesetzt werden", str(exc), parent=self) + return + self.refresh() + ReminderDialog( + self, self.repository, self.member_id, self.claim_id, self._changed, reminder=reminder + ) + def _discard_reminder(self) -> None: reminder = self._selected_reminder() if not reminder or not messagebox.askyesno( @@ -876,21 +909,25 @@ class ReminderDialog(_Dialog): provide cover/valid bank details), or a blank template for manual entry. The fee items table itself is always free to edit no matter which preset was picked.""" - def __init__(self, master, repository, member_id, claim_id, on_saved): - super().__init__(master, "Mahnung vorbereiten", on_saved) + def __init__(self, master, repository, member_id, claim_id, on_saved, *, reminder: dict | None = None): + super().__init__(master, "Mahnung bearbeiten" if reminder else "Mahnung vorbereiten", on_saved) # Unlike the small fixed-size dialogs sharing _Dialog, this one holds a table # of freely-editable fee items whose content varies a lot -- make it resizable # so a long/multi-item Mahnung doesn't get cramped. self.resizable(True, True) self.repository, self.member_id, self.claim_id = repository, member_id, claim_id - data, _claim = repository.get_claim(member_id, claim_id) - existing_levels = { - int(item.get("level", 0)) - for item in data.reminders - if str(item.get("claim_id", "")) == claim_id - and str(item.get("status", "draft")) in {"draft", "generated", "sent"} - } - self.level = max(existing_levels, default=0) + 1 + self.editing_reminder = reminder + if reminder: + self.level = int(reminder.get("level", 1)) + else: + data, _claim = repository.get_claim(member_id, claim_id) + existing_levels = { + int(item.get("level", 0)) + for item in data.reminders + if str(item.get("claim_id", "")) == claim_id + and str(item.get("status", "draft")) in {"draft", "generated", "sent"} + } + self.level = max(existing_levels, default=0) + 1 self.policy = repository.get_reminder_policy() self.items: list[dict[str, str]] = [] @@ -902,7 +939,10 @@ class ReminderDialog(_Dialog): ) self.preset_by_label = self._build_presets() - self.preset_var = tk.StringVar(value=next(iter(self.preset_by_label))) + default_preset = ( + next(reversed(self.preset_by_label)) if reminder else next(iter(self.preset_by_label)) + ) + self.preset_var = tk.StringVar(value=default_preset) ttk.Label(self.frame, text="Vorlage").grid(row=1, column=0, sticky="w", pady=5, padx=(0, 12)) preset_combo = ttk.Combobox( self.frame, @@ -942,7 +982,17 @@ class ReminderDialog(_Dialog): ).grid(row=5, column=1, sticky="ew", pady=5) self._build_items_table(row=6) - self._apply_preset() + if reminder: + self.name_var.set(str(reminder.get("name", ""))) + self.detail_var.set(str(reminder.get("detail", ""))) + self.deadline_var.set(str(reminder.get("payment_deadline_days", 14))) + self.channel_var.set( + display_label(REMINDER_CHANNEL_LABELS, str(reminder.get("channel", "email"))) + ) + self.items = [dict(item) for item in reminder.get("items") or []] + self._refresh_items() + else: + self._apply_preset() self._buttons(7, self._save) # Size to fit the fully built content (incl. the preset-populated items table) # instead of whatever size Tk guessed before everything was in place -- the @@ -963,16 +1013,23 @@ class ReminderDialog(_Dialog): fee = decimal_value(matching["fee"]) presets[f"Stufe {self.level}: {matching['name']}"] = { "name": matching["name"], + "detail": "", "payment_deadline_days": matching["payment_deadline_days"], "items": [{"description": "Mahngebühr", "amount": money_text(fee)}] if fee > 0 else [], } failed_debit_amount = self._default_amount_for("Rücklastschrift") presets[f"Stufe {self.level}: Rücklastschrift"] = { "name": "Rücklastschrift", + # Prefilled so the member sees why they're being charged; the board can + # still append/edit a concrete reason (mangels Deckung, Konto ungültig, ...) + # right here since "Details" goes straight into the reminder email as-is. + "detail": "Leider konnte die Lastschrift von Ihrem Konto nicht eingelöst werden.", "payment_deadline_days": 14, "items": [{"description": "Rücklastschriftgebühr", "amount": failed_debit_amount}], } - presets["Frei (eigene Eingabe)"] = {"name": "", "payment_deadline_days": 14, "items": []} + presets["Frei (eigene Eingabe)"] = { + "name": "", "detail": "", "payment_deadline_days": 14, "items": [] + } return presets def _default_amount_for(self, needle: str) -> str: @@ -984,6 +1041,7 @@ class ReminderDialog(_Dialog): def _apply_preset(self) -> None: preset = self.preset_by_label[self.preset_var.get()] self.name_var.set(preset["name"]) + self.detail_var.set(preset["detail"]) self.deadline_var.set(str(preset["payment_deadline_days"])) self.items = [dict(item) for item in preset["items"]] self._refresh_items() @@ -1112,16 +1170,28 @@ class ReminderDialog(_Dialog): ) return try: - self.repository.create_reminder_draft( - self.member_id, - self.claim_id, - level=self.level, - name=self.name_var.get(), - payment_deadline_days=deadline_days, - detail=self.detail_var.get(), - items=self.items, - channel=storage_key(REMINDER_CHANNEL_LABELS, self.channel_var.get()), - ) + if self.editing_reminder: + self.repository.edit_reminder_draft( + self.member_id, + self.claim_id, + str(self.editing_reminder["reminder_id"]), + name=self.name_var.get(), + payment_deadline_days=deadline_days, + detail=self.detail_var.get(), + items=self.items, + channel=storage_key(REMINDER_CHANNEL_LABELS, self.channel_var.get()), + ) + else: + self.repository.create_reminder_draft( + self.member_id, + self.claim_id, + level=self.level, + name=self.name_var.get(), + payment_deadline_days=deadline_days, + detail=self.detail_var.get(), + items=self.items, + channel=storage_key(REMINDER_CHANNEL_LABELS, self.channel_var.get()), + ) except (ValueError, RepositoryError) as exc: messagebox.showerror("Mahnungsentwurf konnte nicht gespeichert werden", str(exc), parent=self) return diff --git a/tests/test_reminders.py b/tests/test_reminders.py index ecca884..9e2aa35 100644 --- a/tests/test_reminders.py +++ b/tests/test_reminders.py @@ -281,6 +281,142 @@ def test_reminder_policy_rejects_invalid_input(tmp_path) -> None: ) +def test_edit_reminder_draft_updates_fields_but_not_level(tmp_path) -> None: + repository, member = _overdue_claim_repository(tmp_path) + draft = repository.create_reminder_draft( + member.member_id, + "claim-1", + level=1, + name="Zahlungserinnerung", + payment_deadline_days=14, + items=[], + ) + + edited = repository.edit_reminder_draft( + member.member_id, + "claim-1", + draft["reminder_id"], + name="Erste Mahnung", + payment_deadline_days=10, + detail="Bitte um Ausgleich.", + items=[{"description": "Mahngebühr", "amount": "5.00"}], + ) + assert edited["level"] == 1 + assert edited["name"] == "Erste Mahnung" + assert edited["payment_deadline_days"] == 10 + assert edited["detail"] == "Bitte um Ausgleich." + assert edited["items"] == [{"description": "Mahngebühr", "amount": "5.00"}] + + +def test_edit_reminder_draft_rejects_already_sent_reminder(tmp_path) -> None: + repository, member = _overdue_claim_repository(tmp_path) + draft = repository.create_reminder_draft( + member.member_id, + "claim-1", + level=1, + name="Zahlungserinnerung", + payment_deadline_days=14, + items=[], + ) + repository.mark_reminder_sent(member.member_id, "claim-1", draft["reminder_id"]) + + with pytest.raises(RepositoryError, match="Nur ein Entwurf"): + repository.edit_reminder_draft( + member.member_id, + "claim-1", + draft["reminder_id"], + name="Andere Bezeichnung", + payment_deadline_days=14, + ) + + +def test_revert_reminder_sent_removes_booked_fee_and_reopens_draft(tmp_path) -> None: + repository, member = _overdue_claim_repository(tmp_path) + draft = repository.create_reminder_draft( + member.member_id, + "claim-1", + level=1, + name="Rücklastschrift", + payment_deadline_days=14, + items=[{"description": "Rücklastschriftgebühr", "amount": "5.00"}], + ) + sent = repository.mark_reminder_sent(member.member_id, "claim-1", draft["reminder_id"]) + assert len(sent["fee_item_ids"]) == 1 + _data, claim = repository.get_claim(member.member_id, "claim-1") + assert claim_total(claim) == Decimal("105.00") + + reverted = repository.revert_reminder_sent(member.member_id, "claim-1", draft["reminder_id"]) + assert reverted["status"] == "draft" + assert reverted["sent_at"] is None + assert reverted["payment_deadline"] is None + assert reverted["fee_item_ids"] == [] + + _data, claim = repository.get_claim(member.member_id, "claim-1") + assert claim_total(claim) == Decimal("100.00") + descriptions = {item["description"] for item in claim["items"]} + assert "Rücklastschriftgebühr" not in descriptions + + # Now editable and re-sendable, e.g. after fixing the wording. + repository.edit_reminder_draft( + member.member_id, + "claim-1", + draft["reminder_id"], + name="Rücklastschrift", + payment_deadline_days=14, + items=[{"description": "Rücklastschriftgebühr", "amount": "7.50"}], + ) + repository.mark_reminder_sent(member.member_id, "claim-1", draft["reminder_id"]) + _data, claim = repository.get_claim(member.member_id, "claim-1") + assert claim_total(claim) == Decimal("107.50") + + +def test_revert_reminder_sent_rejects_when_higher_level_exists(tmp_path) -> None: + repository, member = _overdue_claim_repository(tmp_path) + first = repository.create_reminder_draft( + member.member_id, + "claim-1", + level=1, + name="Zahlungserinnerung", + payment_deadline_days=14, + items=[], + ) + repository.mark_reminder_sent(member.member_id, "claim-1", first["reminder_id"]) + repository.create_reminder_draft( + member.member_id, + "claim-1", + level=2, + name="Erste Mahnung", + payment_deadline_days=14, + items=[{"description": "Mahngebühr", "amount": "5.00"}], + ) + + with pytest.raises(RepositoryError, match="höhere Mahnstufe"): + repository.revert_reminder_sent(member.member_id, "claim-1", first["reminder_id"]) + + +def test_revert_reminder_sent_rejects_when_payment_already_covers_fee(tmp_path) -> None: + repository, member = _overdue_claim_repository(tmp_path) + draft = repository.create_reminder_draft( + member.member_id, + "claim-1", + level=1, + name="Rücklastschrift", + payment_deadline_days=14, + items=[{"description": "Rücklastschriftgebühr", "amount": "5.00"}], + ) + repository.mark_reminder_sent(member.member_id, "claim-1", draft["reminder_id"]) + repository.record_payment( + member.member_id, + "claim-1", + payment_date="2026-02-10", + amount="105.00", + allocation_amount="105.00", + ) + + with pytest.raises(RepositoryError, match="bereits Zahlungen verbucht"): + repository.revert_reminder_sent(member.member_id, "claim-1", draft["reminder_id"]) + + def test_reminder_policy_backfills_standard_fee_items_for_old_repositories(tmp_path) -> None: repository = MemberRepository(tmp_path) repository.initialize()