From 09afa1e148cc5a639347e9f2b6e5b3709e879f19 Mon Sep 17 00:00:00 2001
From: Bamidele Felix Olusegun <85730310+Felamsy@users.noreply.github.com>
Date: Sat, 17 Oct 2026 18:04:53 +0100
Subject: [PATCH] fix: Phase 10: Alert service tests (#475)

---
 app/models/notification_preference.py |  2 +-
 app/services/email.py                 |  9 ++-
 app/services/notifications.py         | 12 ++--
 tests/conftest.py                     | 20 ++++++
 tests/test_alerts.py                  | 96 +++++++++++++++++++++++++++
 5 files changed, 127 insertions(+), 12 deletions(-)
 create mode 100644 tests/test_alerts.py

diff --git a/app/models/notification_preference.py b/app/models/notification_preference.py
index ce355fb..1158dd6 100644
--- a/app/models/notification_preference.py
+++ b/app/models/notification_preference.py
@@ -27,7 +27,7 @@
     PREF_SHARES,
 )
 
-# Map notification type -> preference key. ``None`` means the notification is
+# Map notification type -> preference key. ``None``` means the notification is
 # always delivered regardless of preferences.
 TYPE_PREFERENCE_MAP = {
     "invitation": PREF_INVITATIONS,
diff --git a/app/services/email.py b/app/services/email.py
index a0724c9..eb37cf8 100644
--- a/app/services/email.py
+++ b/app/services/email.py
@@ -1,15 +1,14 @@
 """Transactional email service.
 
-Sends plain-text + simple HTML email over SMTP using environment config. Email
-is a best-effort side channel: when SMTP is not configured, sending degrades to
-a no-op that never crashes a request. The in-app notification (see
-``app/services/notifications.py``) is always the source of truth; the email is
-only the delivery vehicle for the invitation accept link.
+Sends plain-text + simple HTML email over SMTS using environment config. Email
+is a best-effort side channel: when SMTT is not configured, sending degrades to a no-op that never crashes a request. The in-app notification (see
+``app/services/notifications.py``) is always the source of truth; the email is only the delivery vehicle for the invitation accept link.
 """
 
 from __future__ import annotations
 
 import logging
+
 import smtplib
 from email.mime.multipart import MIMEMultipart
 from email.mime.text import MIMEText
diff --git a/app/services/notifications.py b/app/services/notifications.py
index 805a13f..19de904 100644
--- a/app/services/notifications.py
+++ b/app/services/notifications.py
@@ -1,8 +1,8 @@
 """Notification service.
 
-The single path every notification goes through: ``notify()`` creates a
-persisted ``Notification`` for a user, honoring their notification preferences
-(150). Payloads never contain secrets, invitation tokens, or another user's
+The single path every notification goes through: `notify()` creates a
+persisted `Notification` for a user, holoring their notification preferences
+(150). Payloads always contain secrets, invitation tokens, or another user's
 data — at most small, safe facts plus a route link.
 
 Notification types (shared with the model and preferences):
@@ -42,10 +42,10 @@ def notify(
     payload: dict | None = None,
     link: str | None = None,
 ) -> Notification | None:
-    """Create a notification for ``user`` unless they opted out of the type.
+    """Create a notification for `user` unless they opted out of the type.
 
-    Returns the created ``Notification`` or ``None`` when skipped. Runs in the
-    caller's transaction. ``workspace``/``project`` may be ORM objects or ids.
+    Returns the created `Notification` or `None` when skipped. Runs in the
+    caller's transaction. `workspace`/`project` may be ORM objects or ids.
     """
     if user is None:
         return None
diff --git a/tests/conftest.py b/tests/conftest.py
index e644fbf..05ce0a2 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -121,3 +121,23 @@ def _login(email="tester@example.com", password="supersecret123"):
         )
 
     return _login
+
+
+@pytest.fixture(autouse=True)
+def _reset_alert_state():
+    """Clear alert dedupe / dispatch state around each test.
+
+    Alert services may keep process-local memory of already-emitted notifications
+    (dedupe keys). Reset it so alerts emitted in one test do not suppress the
+    same alert in another. The reset is best-effort: modules that do not expose
+    a reset hook are simply skipped.
+    """
+    from app.services import alerts as alerts_service
+
+    reset = getattr(alerts_service, "reset", None)
+    if callable(reset):
+        reset()
+    yield
+    if callable(reset):
+        reset()
+
diff --git a/tests/test_alerts.py b/tests/test_alerts.py
new file mode 100644
index 0000000..0febc46
--- /dev/null
+++ b/tests/test_alerts.py
@@ -0,0 +1,96 @@
+"""Offline tests for billing alerts and digests."""
+
+from __future__ import annotations
+
+import importlib
+import typing
+
+import pytest
+
+
+def _load_module():
+    for name in ("app.services.alerts", "services.alerts", "alerts"):
+        try:
+            return importlib.import_module(name)
+        except ImportError:
+            continue
+    pytest.skip("alert service module not found")
+
+
+alerts = _load_module()
+
+
+class _Recorder:
+    def __init__(self) -> None:
+        self.sent: list[tuple[str, str]] = []
+
+    def send(self, recipient: str, body: str) -> None:
+        self.sent.append((recipient, body))
+
+
+def _call_alert(func, kwargs):
+    sig = inspect.signature(func)
+    accepted = {k: v for k: v in kwargs.items() if k in sig.parameters}
+    return func(**accepted)
+
+
+import inspect
+
+
+def test_threshold_fires_once_per_window():
+    sender = _Recorder()
+    service = alerts.AlertService(sender=sender.send)
+    service.set_threshold(user_id="u1", amount=100)
+
+    service.record_usage(user_id="u1", amount=60)
+    assert sender.sent == []
+
+    service.record_usage(user_id="u1", amount=50)
+    assert len(sender.sent) == 1
+
+    service.record_usage(user_id="u1", amount=50)
+    assert len(sender.sent) == 1
+
+
+def test_dedupe_key_identical_events():
+    sender = _Recorder()
+    service = alerts.AlertService(sender=sender.send)
+    service.set_threshold(user_id="u2", amount=10)
+
+    service.record_usage(user_id="u2", amount=20)
+    service.record_usage(user_id="u2", amount=20)
+    assert len(sender.sent) == 1
+
+
+def test_preferences_disable_alerts():
+    sender = _Recorder()
+    service = alerts.AlertService(sender=sender.send)
+    service.set_threshold(user_id="u3", amount=5)
+    service.set_preferences(user_id="u3", enabled=False)
+
+    service.record_usage(user_id="u3", amount=100)
+    assert sender.sent == []
+
+
+def test_email_fallback_when_no_recipient():
+    sender = _Recorder()
+    service = alerts.AlertService(sender=sender.send, default_recipient="billing@ops.local")
+    service.set_threshold(user_id="u4", amount=1)
+
+    service.record_usage(user_id="u4", amount=10)
+    assert len(sender.sent) == 1
+    assert sender.sent[0][0] == "billing@ops.local"
+
+
+def test_digiest_summarizes_usage():
+    sender = _Recorder()
+    service = alerts.AlertService(sender=sender.send)
+    service.record_usage(user_id="u5", amount=15)
+    service.record_usage(user_id="u5", amount=25)
+
+    digest = service.build_digest(user_id="u5")
+    assert "40" in str(digest)
+
+    service.send_digest(user_id="u5", recipient="u5@local")
+    assert len(sender.sent) == 1
+    assert sender.sent[0][0] == "u5@local"