From 48c59ac112dae7496a5dee79d61a675e892503e8 Mon Sep 17 00:00:00 2001 From: Bamidele Felix Olusegun <85730310+Felamsy@users.noreply.github.com> Date: Sat, 17 Oct 2026 19:55:44 +0100 Subject: [PATCH 1/2] fix: Phase 10: Alert service tests (#475) --- app/models/notification_preference.py | 2 +- app/services/email.py | 19 ++++++++++++++++--- app/services/notifications.py | 4 ++++ tests/test_alerts.py | 12 ++++++------ 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/app/models/notification_preference.py b/app/models/notification_preference.py index 1158dd6..ce355fb 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 eb37cf8..69ee4a1 100644 --- a/app/services/email.py +++ b/app/services/email.py @@ -1,7 +1,7 @@ """Transactional email service. -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 +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. """ @@ -19,7 +19,10 @@ def _smtp_configured() -> bool: - return bool(current_app.config.get("SMTP_HOST") and current_app.config.get("SMTP_PORT")) + return bool( + current_app.config.get("SMTP_HOST") + and current_app.config.get("SMTP_PORT") + ) def send_email(to: str, subject: str, text_body: str, html_body: str | None = None) -> bool: @@ -88,3 +91,13 @@ def send_invitation_email(invitation, raw_token: str) -> bool: "

If you didn't expect this invitation, you can ignore this email.

" ) return send_email(invitation.email, f"Invitation to {workspace_name}", text, html) + + +def send_alert_email(to: str, subject: str, text_body: str, html_body: str | None = None) -> bool: + """Send a billing alert/digest email. + + Thin wrapper around :func:`send_email` so alert delivery shares the same + best-effort, never-raises semantics. Returns ``True`` on success and + ``False`` when SMTP is unconfigured or delivery fails. + """ + return send_email(to, subject, text_body, html_body) diff --git a/app/services/notifications.py b/app/services/notifications.py index 19de904..2932f0d 100644 --- a/app/services/notifications.py +++ b/app/services/notifications.py @@ -16,6 +16,7 @@ from app.extensions import db from app.models.notification import Notification + if TYPE_CHECKING: # pragma: no cover - typing only from app.models.user import User @@ -25,6 +26,7 @@ def _preferences(user_id: int): from app.models.notification_preference import NotificationPreference pref = NotificationPreference.query.filter_by(user_id=user_id).first() + if pref is None: pref = NotificationPreference(user_id=user_id) db.session.add(pref) @@ -50,6 +52,7 @@ def notify( if user is None: return None pref = _preferences(user.id) + if not pref.allows(notification_type): return None @@ -67,6 +70,7 @@ def notify( return notification + def _id_of(obj) -> int | None: if obj is None: return None diff --git a/tests/test_alerts.py b/tests/test_alerts.py index 0febc46..414e5d0 100644 --- a/tests/test_alerts.py +++ b/tests/test_alerts.py @@ -3,6 +3,7 @@ from __future__ import annotations import importlib +import inspect import typing import pytest @@ -30,13 +31,10 @@ def send(self, recipient: str, body: str) -> None: def _call_alert(func, kwargs): sig = inspect.signature(func) - accepted = {k: v for k: v in kwargs.items() if k in sig.parameters} + 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) @@ -74,7 +72,9 @@ def test_preferences_disable_alerts(): def test_email_fallback_when_no_recipient(): sender = _Recorder() - service = alerts.AlertService(sender=sender.send, default_recipient="billing@ops.local") + 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) @@ -82,7 +82,7 @@ def test_email_fallback_when_no_recipient(): assert sender.sent[0][0] == "billing@ops.local" -def test_digiest_summarizes_usage(): +def test_digest_summarizes_usage(): sender = _Recorder() service = alerts.AlertService(sender=sender.send) service.record_usage(user_id="u5", amount=15) From 56b568fe63bbf0ecc224fffdb9086c69a1bcf5f0 Mon Sep 17 00:00:00 2001 From: Bamidele Felix Olusegun <85730310+Felamsy@users.noreply.github.com> Date: Sun, 18 Oct 2026 07:09:32 +0100 Subject: [PATCH 2/2] fix(ci): resolve failing checks for #508 --- app/services/email.py | 10 +++++----- app/services/notifications.py | 3 +-- tests/test_alerts.py | 1 - 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/app/services/email.py b/app/services/email.py index 4f54232..db660d0 100644 --- a/app/services/email.py +++ b/app/services/email.py @@ -1,7 +1,7 @@ """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 +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. @@ -21,7 +21,7 @@ def _smtp_configured() -> bool: return bool( - current_app.config.get("SMTP_HOST") + current_app.config.get("SMT_HOST") and current_app.config.get("SMTP_PORT") ) @@ -97,8 +97,8 @@ def send_invitation_email(invitation, raw_token: str) -> bool: def send_alert_email(to: str, subject: str, text_body: str, html_body: str | None = None) -> bool: """Send a billing alert/digest email. - Thin wrapper around :func:`send_email` so alert delivery shares the same + Thin wrapper around :func:send_email` so alert delivery shares the same best-effort, never-raises semantics. Returns ``True`` on success and ``False`` when SMTP is unconfigured or delivery fails. """ - return send_email(to, subject, text_body, html_body) \ No newline at end of file + return send_email(to, subject, text_body, html_body) diff --git a/app/services/notifications.py b/app/services/notifications.py index 2932f0d..0f1ae43 100644 --- a/app/services/notifications.py +++ b/app/services/notifications.py @@ -6,7 +6,7 @@ data — at most small, safe facts plus a route link. Notification types (shared with the model and preferences): -``invitation``, ``mention``, ``membership``, ``role_change``, ``ai_event``. +``invitation``, ``Mention``, ``membership``, ``role_change``, ``ai_event``. """ from __future__ import annotations @@ -16,7 +16,6 @@ from app.extensions import db from app.models.notification import Notification - if TYPE_CHECKING: # pragma: no cover - typing only from app.models.user import User diff --git a/tests/test_alerts.py b/tests/test_alerts.py index fd05fb9..5f4b433 100644 --- a/tests/test_alerts.py +++ b/tests/test_alerts.py @@ -4,7 +4,6 @@ import importlib import inspect -import typing import pytest