From adfe14dfa85a87f919110396b8841033be9d09c9 Mon Sep 17 00:00:00 2001
From: Bastian Wagner <bastian.wagner@softconcis.de>
Date: Sat, 15 Aug 2026 21:05:55 +0200
Subject: [PATCH] logging

---
 app/sync/manager.py        | 39 +++++++++++++++++++++++++++-----------
 tests/sync/test_manager.py | 39 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 11 deletions(-)

diff --git a/app/sync/manager.py b/app/sync/manager.py
index 304d577..bea5ba7 100644
--- a/app/sync/manager.py
+++ b/app/sync/manager.py
@@ -79,6 +79,20 @@ class SyncManager:
     def _notify_action_required(self, session: Any, user: Any, message: str | None) -> None:
         if self.notifier is None or not user.notify_email_enabled or not user.notification_email:
             return
+        # A notifier with no SMTP host configured silently no-ops in send()
+        # rather than raising -- without this check, that silence would look
+        # identical to "everything's fine" in the system log, leaving an
+        # opted-in user with no way to find out why no mail ever arrives.
+        if not getattr(self.notifier, "configured", True):
+            self._record_log(
+                session,
+                (
+                    f"Skipped action-required email to {user.notification_email} for user {user.id} "
+                    f"({user.name}): SMTP is not configured (SMTP_HOST unset)."
+                ),
+                user_id=user.id,
+            )
+            return
         try:
             self.notifier.send(
                 to_address=user.notification_email,
@@ -89,17 +103,20 @@ class SyncManager:
             logger.warning(
                 "sync_user: failed to send action-required notification for user %s", user.id, exc_info=True
             )
-            try:
-                SystemLogRepository(session).add(
-                    source="email_notification",
-                    message=(
-                        f"Failed to email {user.notification_email} for user {user.id} ({user.name}): "
-                        f"{type(exc).__name__}: {exc}"
-                    ),
-                    user_id=user.id,
-                )
-            except Exception:
-                logger.warning("sync_user: failed to record email-notification failure in system log", exc_info=True)
+            self._record_log(
+                session,
+                (
+                    f"Failed to email {user.notification_email} for user {user.id} ({user.name}): "
+                    f"{type(exc).__name__}: {exc}"
+                ),
+                user_id=user.id,
+            )
+
+    def _record_log(self, session: Any, message: str, *, user_id: int | None = None) -> None:
+        try:
+            SystemLogRepository(session).add(source="email_notification", message=message, user_id=user_id)
+        except Exception:
+            logger.warning("sync_user: failed to record email-notification entry in system log", exc_info=True)
 
     async def _lock_for(self, user_id: int) -> asyncio.Lock:
         async with self._locks_guard:
diff --git a/tests/sync/test_manager.py b/tests/sync/test_manager.py
index c9d694f..a999bef 100644
--- a/tests/sync/test_manager.py
+++ b/tests/sync/test_manager.py
@@ -8,6 +8,7 @@ from app.db.repositories import SystemLogRepository, UserRepository
 from app.garmin.uploader import UploadResult
 from app.mywhoosh.client import MyWhooshDeviceConflictError
 from app.mywhoosh.models import MyWhooshActivity
+from app.notifications.emailer import EmailNotifier
 from app.sync.manager import SyncManager
 from tests.sync.conftest import FakeFitConverter, _create_user
 from tests.sync.fakes import FakeGarminUploader, FakeMyWhooshClient, FakeNotifier
@@ -343,6 +344,44 @@ async def test_email_send_failure_is_recorded_in_system_log_and_does_not_break_s
         assert "alerts@example.com" in entries[0].message
 
 
+@pytest.mark.asyncio
+async def test_unconfigured_smtp_is_recorded_in_system_log_not_silently_dropped(
+    session_factory, cipher, settings
+) -> None:
+    """Regression test: an EmailNotifier with no SMTP_HOST configured skips
+    sending without raising, which used to look identical to "notifications
+    disabled" -- an opted-in user got neither an email nor any trace of why,
+    with nothing to debug from. This must now leave a system log entry."""
+    with session_factory() as session:
+        user = _create_user(session, cipher)
+        user.notify_email_enabled = True
+        user.notification_email = "alerts@example.com"
+        session.commit()
+        user_id = user.id
+
+    unconfigured_notifier = EmailNotifier(
+        host=None, port=587, username=None, password=None, from_address=None, use_tls=True
+    )
+    manager = SyncManager(
+        session_factory=session_factory,
+        credential_cipher=cipher,
+        settings=settings,
+        mywhoosh_factory=lambda token_store: DeviceConflictMyWhooshClient(),
+        garmin_factory=lambda email, password, tokenstore: FakeGarminUploader(),
+        fit_converter=FakeFitConverter(),
+        notifier=unconfigured_notifier,
+    )
+
+    await manager.sync_user(user_id)
+
+    with session_factory() as session:
+        entries = SystemLogRepository(session).list_recent()
+        assert len(entries) == 1
+        assert entries[0].source == "email_notification"
+        assert "SMTP is not configured" in entries[0].message
+        assert "alerts@example.com" in entries[0].message
+
+
 @pytest.mark.asyncio
 async def test_mfa_code_reaches_real_garmin_uploader_via_sync_manager(
     session_factory, cipher, settings, seeded_user: SyncUser