refactor notifications to MVC: extract templates, context, and renderer
- Move all email layout to Jinja2 templates (4 files: admin_new, admin_update, parent_confirmation html+txt) - Extract pure helper/context-builder functions to context.py (format_types, calculate_age, format_dob, calculate_monthly_fee, format_days, build_admin_new_context, build_admin_update_context, build_parent_context) - Move i18n label strings out of Python into YAML locale files (i18n/de.yaml, i18n/en.yaml) — admin-editable without code changes - Add renderer.py as thin Jinja2 wrapper (render_template) - Reduce notifier.py to routing + SMTP dispatch only - Update tests to call context functions directly and render templates via renderer instead of calling private builder methods - Add jinja2 and pyyaml as explicit dependencies in pyproject.toml https://claude.ai/code/session_01LjjK7RjKVnC8bETtccfgna
This commit is contained in:
Claude
+43
-32
@@ -1,11 +1,18 @@
|
||||
"""Tests for AdminNotifier helper methods."""
|
||||
"""Tests for AdminNotifier and notification helper functions."""
|
||||
|
||||
import email
|
||||
from email.header import decode_header
|
||||
|
||||
import pytest
|
||||
|
||||
from src.notifications.notifier import AdminNotifier, _STRINGS_DE, _STRINGS_EN
|
||||
from src.notifications.notifier import AdminNotifier
|
||||
from src.notifications.context import (
|
||||
calculate_age,
|
||||
calculate_monthly_fee,
|
||||
format_types,
|
||||
load_strings,
|
||||
)
|
||||
from src.notifications.renderer import render_template
|
||||
from src.models.registration import RegistrationData, Booking, BookingDay
|
||||
|
||||
|
||||
@@ -36,53 +43,55 @@ def notifier():
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _format_types
|
||||
# format_types
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestFormatTypes:
|
||||
def test_indoor_label(self, notifier):
|
||||
assert "Innen" in notifier._format_types(["indoor"]) or "indoor" in notifier._format_types(["indoor"]).lower()
|
||||
def test_indoor_label(self):
|
||||
result = format_types(["indoor"])
|
||||
assert "Innen" in result or "indoor" in result.lower()
|
||||
|
||||
def test_outdoor_label(self, notifier):
|
||||
assert "Wald" in notifier._format_types(["outdoor"]) or "outdoor" in notifier._format_types(["outdoor"]).lower()
|
||||
def test_outdoor_label(self):
|
||||
result = format_types(["outdoor"])
|
||||
assert "Wald" in result or "outdoor" in result.lower()
|
||||
|
||||
def test_both_labels(self, notifier):
|
||||
result = notifier._format_types(["indoor", "outdoor"])
|
||||
def test_both_labels(self):
|
||||
result = format_types(["indoor", "outdoor"])
|
||||
assert len(result) > 0
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _calculate_age
|
||||
# calculate_age
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestCalculateAge:
|
||||
def test_returns_age_string(self, notifier):
|
||||
result = notifier._calculate_age("2022-01-01")
|
||||
def test_returns_age_string(self):
|
||||
result = calculate_age("2022-01-01")
|
||||
assert isinstance(result, str)
|
||||
assert len(result) > 0
|
||||
|
||||
def test_invalid_dob_returns_original_string(self, notifier):
|
||||
result = notifier._calculate_age("not-a-date")
|
||||
def test_invalid_dob_returns_original_string(self):
|
||||
result = calculate_age("not-a-date")
|
||||
assert result == "not-a-date"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _calculate_monthly_fee
|
||||
# calculate_monthly_fee
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestCalculateMonthlyFee:
|
||||
def test_indoor_one_day(self, notifier, complete_registration):
|
||||
def test_indoor_one_day(self, complete_registration):
|
||||
complete_registration.booking = Booking(
|
||||
playgroup_types=["indoor"],
|
||||
selected_days=[BookingDay(day="monday", type="indoor")],
|
||||
)
|
||||
fee = notifier._calculate_monthly_fee(complete_registration)
|
||||
fee = calculate_monthly_fee(complete_registration)
|
||||
assert "130" in fee
|
||||
|
||||
def test_indoor_two_days(self, notifier, complete_registration):
|
||||
def test_indoor_two_days(self, complete_registration):
|
||||
complete_registration.booking = Booking(
|
||||
playgroup_types=["indoor"],
|
||||
selected_days=[
|
||||
@@ -90,10 +99,10 @@ class TestCalculateMonthlyFee:
|
||||
BookingDay(day="wednesday", type="indoor"),
|
||||
],
|
||||
)
|
||||
fee = notifier._calculate_monthly_fee(complete_registration)
|
||||
fee = calculate_monthly_fee(complete_registration)
|
||||
assert "260" in fee
|
||||
|
||||
def test_indoor_three_days(self, notifier, complete_registration):
|
||||
def test_indoor_three_days(self, complete_registration):
|
||||
complete_registration.booking = Booking(
|
||||
playgroup_types=["indoor"],
|
||||
selected_days=[
|
||||
@@ -102,15 +111,15 @@ class TestCalculateMonthlyFee:
|
||||
BookingDay(day="thursday", type="indoor"),
|
||||
],
|
||||
)
|
||||
fee = notifier._calculate_monthly_fee(complete_registration)
|
||||
fee = calculate_monthly_fee(complete_registration)
|
||||
assert "390" in fee
|
||||
|
||||
def test_outdoor_one_day(self, notifier, complete_registration):
|
||||
def test_outdoor_one_day(self, complete_registration):
|
||||
complete_registration.booking = Booking(
|
||||
playgroup_types=["outdoor"],
|
||||
selected_days=[BookingDay(day="monday", type="outdoor")],
|
||||
)
|
||||
fee = notifier._calculate_monthly_fee(complete_registration)
|
||||
fee = calculate_monthly_fee(complete_registration)
|
||||
assert "250" in fee
|
||||
|
||||
|
||||
@@ -121,7 +130,6 @@ class TestCalculateMonthlyFee:
|
||||
|
||||
class TestSend:
|
||||
def test_send_calls_smtp(self, notifier, mocker):
|
||||
# _send uses smtplib.SMTP directly (not as context manager)
|
||||
mock_smtp_cls = mocker.patch("smtplib.SMTP")
|
||||
mock_server = mock_smtp_cls.return_value
|
||||
|
||||
@@ -146,7 +154,7 @@ class TestSend:
|
||||
)
|
||||
|
||||
call_args = mock_server.sendmail.call_args
|
||||
recipients = call_args[0][1] # positional arg: to_addrs
|
||||
recipients = call_args[0][1]
|
||||
assert "a@example.com" in recipients
|
||||
assert "b@example.com" in recipients
|
||||
|
||||
@@ -235,16 +243,20 @@ class TestNotifyParent:
|
||||
|
||||
mock_smtp_cls.assert_not_called()
|
||||
|
||||
def test_notify_parent_text_body_contains_iban(self, notifier_no_smtp, complete_registration):
|
||||
def test_notify_parent_text_body_contains_iban(self, complete_registration):
|
||||
"""Plain-text body includes the IBAN so payment is possible without the QR image."""
|
||||
text = notifier_no_smtp._build_parent_text(complete_registration, _STRINGS_DE)
|
||||
strings = load_strings("de")
|
||||
from src.notifications.context import build_parent_context
|
||||
ctx = build_parent_context(complete_registration, strings, has_qr=False)
|
||||
text = render_template("parent_confirmation.txt.j2", ctx)
|
||||
assert "CH14" in text
|
||||
|
||||
def test_notify_parent_text_body_english_contains_iban(
|
||||
self, notifier_no_smtp, complete_registration
|
||||
):
|
||||
def test_notify_parent_text_body_english_contains_iban(self, complete_registration):
|
||||
"""English plain-text body also includes the IBAN."""
|
||||
text = notifier_no_smtp._build_parent_text(complete_registration, _STRINGS_EN)
|
||||
strings = load_strings("en")
|
||||
from src.notifications.context import build_parent_context
|
||||
ctx = build_parent_context(complete_registration, strings, has_qr=False)
|
||||
text = render_template("parent_confirmation.txt.j2", ctx)
|
||||
assert "CH14" in text
|
||||
|
||||
|
||||
@@ -263,5 +275,4 @@ class TestGenerateQrBillPng:
|
||||
def test_returns_png_signature(self, notifier):
|
||||
"""Output starts with the PNG magic bytes."""
|
||||
png = notifier._generate_qr_bill_png()
|
||||
# PNG files start with the 8-byte signature \x89PNG\r\n\x1a\n
|
||||
assert png[:4] == b"\x89PNG"
|
||||
|
||||
Reference in New Issue
Block a user