diff --git a/functions-python/tasks_executor/src/tasks/notifications/dispatch_monitor.py b/functions-python/tasks_executor/src/tasks/notifications/dispatch_monitor.py index bac975194..d564f6330 100644 --- a/functions-python/tasks_executor/src/tasks/notifications/dispatch_monitor.py +++ b/functions-python/tasks_executor/src/tasks/notifications/dispatch_monitor.py @@ -45,8 +45,16 @@ TaskExecutionTracker, TaskInProgressError, ) -from shared.notifications.notification_constants import NotificationLogStatus -from shared.users_database_gen.sqlacodegen_models import NotificationLog +from sqlalchemy import or_ + +from shared.notifications.notification_constants import ( + NotificationLogStatus, + NotificationTypeId, +) +from shared.users_database_gen.sqlacodegen_models import ( + NotificationEvent, + NotificationLog, +) from tasks.notifications.dispatch_notifications import ( DISPATCH_TASK_NAME, emit_admin_summary, @@ -132,8 +140,23 @@ def _aggregate_delivery_stats( """Aggregate notification_log outcomes for this run from the users DB. Scoped by ``sent_at >= run_started_at`` so it reflects only this run's sends. + ``admin.event_summary`` deliveries are excluded: counting the summary email + itself as a "sent notification" is misleading to admins. """ - q = db_session.query(NotificationLog) + q = ( + db_session.query(NotificationLog) + .outerjoin( + NotificationEvent, + NotificationLog.notification_event_id == NotificationEvent.id, + ) + .filter( + or_( + NotificationEvent.notification_type_id.is_(None), + NotificationEvent.notification_type_id + != NotificationTypeId.ADMIN_EVENT_SUMMARY, + ) + ) + ) if since is not None: q = q.filter(NotificationLog.sent_at >= since) rows = q.with_entities(NotificationLog.status).all() diff --git a/functions-python/tasks_executor/tests/tasks/notifications/test_dispatch_notifications.py b/functions-python/tasks_executor/tests/tasks/notifications/test_dispatch_notifications.py index eb9363a7c..c199cedaf 100644 --- a/functions-python/tasks_executor/tests/tasks/notifications/test_dispatch_notifications.py +++ b/functions-python/tasks_executor/tests/tasks/notifications/test_dispatch_notifications.py @@ -45,6 +45,7 @@ NotificationLog, NotificationSubscription, ) +from tasks.notifications.dispatch_monitor import _aggregate_delivery_stats from tasks.notifications.dispatch_notifications import ( apply_filter_params, emit_admin_summary, @@ -464,6 +465,54 @@ def test_creates_notification_event(self, db_session: Session = None): self.assertEqual(event.payload["cadence"], "weekly") +# --------------------------------------------------------------------------- +# _aggregate_delivery_stats — admin.event_summary deliveries are not counted +# --------------------------------------------------------------------------- + + +class TestAggregateDeliveryStats(unittest.TestCase): + def tearDown(self): + _cleanup_notifications() + + def _make_log(self, db_session, event_id, subscription_id, status): + db_session.add( + NotificationLog( + id=_uid(), + notification_event_id=event_id, + subscription_id=subscription_id, + channel="email", + status=status, + retry_count=0, + sent_at=datetime.now(timezone.utc), + ) + ) + db_session.flush() + + @with_users_db_session(db_url=default_users_db_url) + def test_excludes_admin_summary_deliveries(self, db_session: Session = None): + since = datetime.now(timezone.utc) - timedelta(minutes=5) + sub = _make_subscription(db_session, "user-alice") + + feed_event = _make_event(db_session) + self._make_log(db_session, feed_event.id, sub.id, NotificationLogStatus.SENT) + + # The admin summary email itself must not be counted as a sent + # notification — it is misleading for admins. + admin_event = _make_event( + db_session, + feed_stable_id=None, + notification_type_id=NotificationTypeId.ADMIN_EVENT_SUMMARY, + update_type=AdminEventUpdateType.DISPATCH_SUMMARY, + ) + self._make_log(db_session, admin_event.id, sub.id, NotificationLogStatus.SENT) + + stats = _aggregate_delivery_stats(since=since, db_session=db_session) + + self.assertEqual(stats["emails_sent"], 1) + self.assertEqual(stats["events_found"], 1) + self.assertEqual(stats["emails_failed"], 0) + + # --------------------------------------------------------------------------- # _resolve_scheduled_cadences — day-of-week gating for the single daily job # ---------------------------------------------------------------------------