From d3dbb5e96d34da076eb51026b05158bb8455745b Mon Sep 17 00:00:00 2001 From: Marcel Peterkau Date: Fri, 28 Aug 2026 21:08:23 +0200 Subject: [PATCH] Insert a working repeat block instead of its description MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The placeholder chooser showed repeat blocks as "{{#claims}} … {{/claims}}" and inserted exactly that on double-click -- the ellipsis is a label, so the saved template rendered a literal "…" per claim instead of the claim. Chooser rows are data now: each carries the text it reads as and, separately, the snippet it inserts. A block contributes a complete, ready-to-edit block with a sample line built from its own placeholders, and lands on a line of its own when the cursor sits behind existing text. Covered from both sides: the snippets are checked against the template validator and renderer, and a UI test drives the real dialog -- insert, save, send -- and asserts the mail carries actual claim lines. The Tk tests share one root (the icon library binds its images to the first one) and skip without a display. Co-Authored-By: Claude Opus 5 --- src/ccma/domain/mail_templates.py | 83 ++++++++++++++---- src/ccma/ui/options_dialog.py | 34 +++++--- tests/test_mail_templates.py | 51 +++++++++++ tests/test_options_dialog_ui.py | 138 ++++++++++++++++++++++++++++++ 4 files changed, 275 insertions(+), 31 deletions(-) create mode 100644 tests/test_options_dialog_ui.py diff --git a/src/ccma/domain/mail_templates.py b/src/ccma/domain/mail_templates.py index b68ce4e..39b27bd 100644 --- a/src/ccma/domain/mail_templates.py +++ b/src/ccma/domain/mail_templates.py @@ -26,6 +26,33 @@ class MailTemplate: body: str +@dataclass(frozen=True, slots=True) +class MailTemplateBlock: + name: str + description: str + # Placeholder name -> short explanation, valid inside this block only. + placeholders: tuple[tuple[str, str], ...] + # The line the inserted block starts out with. It makes the inserted snippet a + # working block that renders real content -- the board edits this line rather + # than having to write the whole construct by hand. + sample: str + + @property + def snippet(self) -> str: + return f"{{{{#{self.name}}}}}\n{self.sample}\n{{{{/{self.name}}}}}\n" + + +@dataclass(frozen=True, slots=True) +class PlaceholderEntry: + """One row of the options dialog's placeholder chooser: what it reads as, and + what double-clicking it puts into the template.""" + + label: str + snippet: str + description: str + indented: bool = False + + @dataclass(frozen=True, slots=True) class MailTemplateSpec: key: str @@ -36,8 +63,7 @@ class MailTemplateSpec: # dialog. The mail services must supply exactly these keys, so this doubles as # the contract a stored template is validated against. placeholders: tuple[tuple[str, str], ...] - # Repeat block name -> (explanation, item placeholders). - blocks: tuple[tuple[str, str, tuple[tuple[str, str], ...]], ...] = () + blocks: tuple[MailTemplateBlock, ...] = () BASE_PLACEHOLDERS: tuple[tuple[str, str], ...] = ( @@ -99,10 +125,11 @@ MAIL_TEMPLATES: tuple[MailTemplateSpec, ...] = ( ("payment.reference", "Vorgeschlagener Verwendungszweck"), ), blocks=( - ( - "claims", - "Wiederholt sich je ausgewählter Forderung", - CLAIM_ITEM_PLACEHOLDERS, + MailTemplateBlock( + name="claims", + description="Wiederholt sich je ausgewählter Forderung", + placeholders=CLAIM_ITEM_PLACEHOLDERS, + sample="{{claim.description}} (fällig {{claim.due_date}}): {{claim.balance}} Euro", ), ), ), @@ -128,13 +155,14 @@ MAIL_TEMPLATES: tuple[MailTemplateSpec, ...] = ( ("payment.reference", "Vorgeschlagener Verwendungszweck"), ), blocks=( - ( - "reminder.items", - "Wiederholt sich je Mahnposition", - ( + MailTemplateBlock( + name="reminder.items", + description="Wiederholt sich je Mahnposition", + placeholders=( ("item.description", "Bezeichnung der Position"), ("item.amount", "Betrag der Position"), ), + sample="{{item.description}}: {{item.amount}} Euro", ), ), ), @@ -157,6 +185,28 @@ MAIL_TEMPLATES: tuple[MailTemplateSpec, ...] = ( ) +def placeholder_entries(key: str) -> list[PlaceholderEntry]: + """Rows for the placeholder chooser. Repeat blocks contribute a complete, + ready-to-edit block -- inserting one has to leave a working template behind, + not a description of one.""" + spec = template_spec(key) + entries = [ + PlaceholderEntry(f"{{{{{name}}}}}", f"{{{{{name}}}}}", description) + for name, description in spec.placeholders + ] + for block in spec.blocks: + entries.append( + PlaceholderEntry( + f"{{{{#{block.name}}}}} … {{{{/{block.name}}}}}", block.snippet, block.description + ) + ) + entries.extend( + PlaceholderEntry(f"{{{{{name}}}}}", f"{{{{{name}}}}}", description, indented=True) + for name, description in block.placeholders + ) + return entries + + def template_spec(key: str) -> MailTemplateSpec: for spec in MAIL_TEMPLATES: if spec.key == key: @@ -209,15 +259,12 @@ def validate_mail_template(key: str, subject: str, body: str) -> None: while editing the template instead of during a send run.""" spec = template_spec(key) known = {name for name, _help in spec.placeholders} - block_names = {name for name, _help, _items in spec.blocks} + block_names = {block.name for block in spec.blocks} unknown: set[str] = set() - for block_name, _help, item_placeholders in spec.blocks: - for match in _blocks_of(body, block_name): - unknown.update( - name - for name in _placeholder_names(match) - if name not in known and name not in {item for item, _text in item_placeholders} - ) + for block in spec.blocks: + block_known = known | {name for name, _help in block.placeholders} + for match in _blocks_of(body, block.name): + unknown.update(name for name in _placeholder_names(match) if name not in block_known) remaining = body for block_name in block_names: remaining = _strip_blocks(remaining, block_name) diff --git a/src/ccma/ui/options_dialog.py b/src/ccma/ui/options_dialog.py index 649becd..215a434 100644 --- a/src/ccma/ui/options_dialog.py +++ b/src/ccma/ui/options_dialog.py @@ -11,6 +11,7 @@ from ccma.domain.mail_templates import ( MAIL_TEMPLATES, MailTemplateError, default_mail_template, + placeholder_entries, template_spec, ) from ccma.domain.models import HOUSEKEEPER_MEMBER_FIELD_LABELS @@ -81,6 +82,7 @@ class OptionsDialog(tk.Toplevel): self.mail_template_var = tk.StringVar(value=MAIL_TEMPLATES[0].label) self.mail_template_subject_var = tk.StringVar() self.mail_template_hint_var = tk.StringVar() + self.mail_template_snippets: dict[str, str] = {} self.title("Optionen") self.transient(master.winfo_toplevel()) self.grab_set() @@ -1031,18 +1033,15 @@ class OptionsDialog(tk.Toplevel): self.mail_template_body.delete("1.0", "end") self.mail_template_body.insert("1.0", body) self.mail_template_placeholders.delete(*self.mail_template_placeholders.get_children()) - for name, description in spec.placeholders: - self.mail_template_placeholders.insert( - "", "end", values=(f"{{{{{name}}}}}", description) + # The row text is a readable label; what actually gets inserted is the entry's + # snippet, which for a repeat block is the whole block and not its description. + self.mail_template_snippets = {} + for entry in placeholder_entries(key): + label = f" {entry.label}" if entry.indented else entry.label + row = self.mail_template_placeholders.insert( + "", "end", values=(label, entry.description) ) - for name, description, items in spec.blocks: - self.mail_template_placeholders.insert( - "", "end", values=(f"{{{{#{name}}}}} … {{{{/{name}}}}}", description) - ) - for item_name, item_description in items: - self.mail_template_placeholders.insert( - "", "end", values=(f" {{{{{item_name}}}}}", item_description) - ) + self.mail_template_snippets[row] = entry.snippet def _capture_mail_template(self) -> None: if not hasattr(self, "mail_template_body"): @@ -1081,10 +1080,19 @@ class OptionsDialog(tk.Toplevel): selected = self.mail_template_placeholders.selection() if not selected: return - value = str(self.mail_template_placeholders.item(selected[0], "values")[0]).strip() - self.mail_template_body.insert("insert", value) + snippet = self.mail_template_snippets.get(selected[0]) + if not snippet: + return + # A multi-line block only works on lines of its own, so it starts on a fresh + # one instead of being appended to whatever the cursor sat behind. + if "\n" in snippet.strip() and not self._mail_cursor_at_line_start(): + snippet = "\n" + snippet + self.mail_template_body.insert("insert", snippet) self.mail_template_body.focus_set() + def _mail_cursor_at_line_start(self) -> bool: + return str(self.mail_template_body.index("insert")).split(".")[1] == "0" + def _save_mail_templates(self) -> None: self._capture_mail_template() for key, (subject, body) in self.mail_template_edits.items(): diff --git a/tests/test_mail_templates.py b/tests/test_mail_templates.py index 1ee6b33..aae0ba7 100644 --- a/tests/test_mail_templates.py +++ b/tests/test_mail_templates.py @@ -6,6 +6,7 @@ from ccma.domain.mail_templates import ( MailTemplateError, default_mail_template, parse_mail_template, + placeholder_entries, render_mail_template, serialize_mail_template, validate_mail_template, @@ -103,3 +104,53 @@ def test_saving_a_broken_template_is_refused(tmp_path) -> None: repository.save_mail_template("reminder", subject="Mahnung", body="Hallo {{member.nonsense}}") with pytest.raises(RepositoryError, match="Betreff"): repository.save_mail_template("reminder", subject=" ", body="Hallo") + + +def test_block_rows_insert_a_working_block_not_its_description() -> None: + entries = placeholder_entries("welcome") + block = next(entry for entry in entries if entry.label.startswith("{{#claims}}")) + + # The label may abbreviate the block, the inserted snippet may not. + assert block.label == "{{#claims}} … {{/claims}}" + assert block.snippet.startswith("{{#claims}}\n") + assert block.snippet.rstrip().endswith("{{/claims}}") + assert "{{claim.description}}" in block.snippet + assert "…" not in block.snippet + assert all("…" not in entry.snippet for entry in entries) + + +def test_inserted_block_validates_and_renders_one_line_per_claim() -> None: + block = next( + entry for entry in placeholder_entries("welcome") if entry.label.startswith("{{#claims}}") + ) + body = f"Hallo {{{{member.first_name}}}},\n\n{block.snippet}\nSumme: {{{{claims.total}}}}" + + validate_mail_template("welcome", "Betreff", body) + rendered = render_mail_template( + MailTemplate("Betreff", body), + {"member.first_name": "Ada", "claims.total": "75.00"}, + { + "claims": [ + { + "claim.description": "Aufnahmegebühr", + "claim.due_date": "17.09.2026", + "claim.balance": "15.00", + }, + { + "claim.description": "Mitgliedsbeitrag 2. Halbjahr 2026", + "claim.due_date": "17.09.2026", + "claim.balance": "60.00", + }, + ] + }, + ) + + assert "Aufnahmegebühr (fällig 17.09.2026): 15.00 Euro" in rendered.body + assert "Mitgliedsbeitrag 2. Halbjahr 2026 (fällig 17.09.2026): 60.00 Euro" in rendered.body + assert "…" not in rendered.body + + +def test_every_block_snippet_is_valid_in_its_own_template() -> None: + for spec in MAIL_TEMPLATES: + for block in spec.blocks: + validate_mail_template(spec.key, "Betreff", block.snippet) diff --git a/tests/test_options_dialog_ui.py b/tests/test_options_dialog_ui.py new file mode 100644 index 0000000..1ce34c4 --- /dev/null +++ b/tests/test_options_dialog_ui.py @@ -0,0 +1,138 @@ +from email.parser import BytesParser +from email.policy import default + +import pytest + +tk = pytest.importorskip("tkinter") + +from ccma.config import AppConfig # noqa: E402 +from ccma.domain.mail_templates import placeholder_entries # noqa: E402 +from ccma.services.welcome_mail import generate_and_send_welcome_mail # noqa: E402 +from ccma.storage.repository import MemberRepository # noqa: E402 + + +# One root for the whole session: the icon library binds its images to the first +# Tk instance, so tearing a root down between tests would invalidate them. +@pytest.fixture(scope="session") +def tk_root(): + from ccma.ui.theme import load_theme + + try: + root = tk.Tk() + except tk.TclError as exc: # headless CI has no display to build widgets on + pytest.skip(f"kein Display verfügbar: {exc}") + root.withdraw() + load_theme(root, "dark") + yield root + root.destroy() + + +@pytest.fixture +def repository(tmp_path): + repository = MemberRepository(tmp_path / "store") + repository.initialize() + organization = repository.get_configuration()["organization"] + organization.update( + { + "name": "Chaos Computer Club Mannheim e.V.", + "email": "verwaltung@example.org", + "iban": "DE98670505050038907751", + "bic": "MANSDE66XXX", + } + ) + repository.save_organization(organization) + return repository + + +def _open_options(tk_root, repository): + from ccma.ui.options_dialog import OptionsDialog + + dialog = OptionsDialog(tk_root, AppConfig(store_path=str(repository.root)), repository) + tk_root.update() + return dialog + + +def _block_row(dialog): + for row in dialog.mail_template_placeholders.get_children(): + label = str(dialog.mail_template_placeholders.item(row, "values")[0]) + if label.startswith("{{#claims}}"): + return row + raise AssertionError("Der Wiederholungsblock fehlt in der Platzhalterliste.") + + +def test_double_click_on_a_block_is_wired_up(tk_root, repository): + dialog = _open_options(tk_root, repository) + try: + assert dialog.mail_template_placeholders.bind("") + assert dialog.mail_template_snippets[_block_row(dialog)] == next( + entry.snippet + for entry in placeholder_entries("welcome") + if entry.label.startswith("{{#claims}}") + ) + finally: + dialog.grab_release() + dialog.destroy() + + +def test_inserting_a_block_puts_editable_template_code_into_the_body(tk_root, repository): + dialog = _open_options(tk_root, repository) + try: + dialog.mail_template_body.delete("1.0", "end") + dialog.mail_template_body.insert("1.0", "Offen sind aktuell:") + dialog.mail_template_placeholders.selection_set(_block_row(dialog)) + dialog._insert_mail_placeholder() + tk_root.update() + body = dialog.mail_template_body.get("1.0", "end-1c") + finally: + dialog.grab_release() + dialog.destroy() + + assert "…" not in body + assert "{{#claims}}" in body and "{{/claims}}" in body + assert "{{claim.description}}" in body + # The block was appended behind existing text and still starts on its own line. + assert body.splitlines()[0] == "Offen sind aktuell:" + assert body.splitlines()[1] == "{{#claims}}" + + +def test_edited_template_survives_saving_and_produces_real_lines_in_the_mail( + tk_root, repository, tmp_path +): + member = repository.create_member( + first_name="Ada", last_name="Lovelace", birth_date="1990-01-01" + ) + member.email = "ada@example.org" + member.accepted_at = "2026-08-20" + repository.save_member(member) + repository.create_manual_claim( + member.member_id, title="Aufnahmegebühr", amount="15.00", due_date="2026-09-17" + ) + + dialog = _open_options(tk_root, repository) + try: + dialog.mail_template_subject_var.set("Willkommen, {{member.first_name}}") + dialog.mail_template_body.delete("1.0", "end") + dialog.mail_template_body.insert("1.0", "Offen sind aktuell:\n") + dialog.mail_template_placeholders.selection_set(_block_row(dialog)) + dialog._insert_mail_placeholder() + tk_root.update() + dialog._save_mail_templates() + finally: + dialog.grab_release() + dialog.destroy() + + generated = generate_and_send_welcome_mail( + repository, + member.member_id, + delivery_mode="local", + output_path=tmp_path / "Willkommen.eml", + sender_name="Verwaltung C3MA", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + + content = BytesParser(policy=default).parsebytes( + generated.export_path.read_bytes() + ).get_content() + assert "Aufnahmegebühr (fällig 17.09.2026): 15.00 Euro" in content + assert "…" not in content