fix(user-directory): #501 follow-ups — activity feed at startup, empty labels, fewer stats queries

CHANGELOG.md

@@ -271,6 +271,13 @@ All notable changes to this project are documented in this file. The format is b
   `openapi_public` (`SM_OPENAPI_PUBLIC`) is on. The bare `/openapi.json` was
   previously served, and anonymous, in production.
 
+### Fixed
+- User directory follow-ups (#501): `audit_log` installs its activity feed in
+  `on_startup` (cleared on shutdown) instead of `register_settings`, which
+  tripped `SM012`; `resolve_user_labels` omits users whose label is empty so
+  callers show the stored id; the `users` and `keycloak` directories compute
+  their stats in one query instead of two or three.
+
 ### Deprecated
 - `settings.registration`, `settings.module_registry` and `settings.hydrate`
   re-export the framework objects above and emit a `DeprecationWarning`;

framework/core/simple_module_core/user_directory.py

@@ -166,13 +166,15 @@ async def resolve_user_labels(
     """Map id -> :attr:`DirectoryUser.label` for every id the directory knows.
 
     Empty and ``None`` ids (a row with no recorded user) are skipped; ids the
-    directory cannot name are absent, and callers show the stored id instead.
+    directory cannot name (or names with an empty label, e.g. a cached account
+    with neither a name nor an email) are absent, and callers show the stored
+    id instead.
     """
     wanted = {uid for uid in ids if uid}
     if not wanted:
         return {}
     found = await directory.get_many(db, wanted)
-    return {uid: user.label for uid, user in found.items()}
+    return {uid: user.label for uid, user in found.items() if user.label}
 
 
 __all__ = [

framework/core/tests/test_core_user_directory.py

@@ -118,6 +118,10 @@ async def test_resolve_labels_uses_the_one_naming_rule(self):
         assert labels == {"a": "Ada", "b": "b@x"}
         assert directory.calls == [{"a", "b", "gone"}]
 
+    async def test_resolve_labels_drops_empty_labels(self):
+        directory = _FakeDirectory([DirectoryUser(id="a", email=""), DirectoryUser("b", "b@x")])
+        assert await resolve_user_labels(directory, None, ["a", "b"]) == {"b": "b@x"}
+
     async def test_resolve_labels_short_circuits_without_ids(self):
         directory = _FakeDirectory([])
         assert await resolve_user_labels(directory, None, ["", ""]) == {}

modules/audit_log/audit_log/module.py

@@ -33,14 +33,6 @@ class AuditLogModule(ModuleBase):
         i18n_audience="admin",
     )
 
-    def register_settings(self, app: FastAPI) -> None:
-        """Offer the audit trail as the app's activity feed (GH #501)."""
-        from simple_module_core.activity_feed import set_activity_feed
-
-        from audit_log.activity import AuditActivityFeed
-
-        set_activity_feed(app, AuditActivityFeed(app))
-
     def register_routes(self, api_router: APIRouter, view_router: APIRouter) -> None:
         from audit_log.endpoints.api import router as api
         from audit_log.endpoints.views import router as views
@@ -69,12 +61,23 @@ def register_permissions(self, registry: PermissionRegistry) -> None:
         registry.add_group(PERM_GROUP, list(ALL_PERMISSIONS))
 
     async def on_startup(self, app: FastAPI) -> None:
+        from simple_module_core.activity_feed import set_activity_feed
+
+        from audit_log.activity import AuditActivityFeed
         from audit_log.capture import audit_callback
 
         app.state.sm.db.audit_callback = audit_callback
+        # Offer the audit trail as the app's activity feed (GH #501). Installed
+        # here, not in ``register_settings``: this module has no settings, and
+        # overriding that hook without touching ``app.state.audit_log`` raises
+        # SM012.
+        set_activity_feed(app, AuditActivityFeed(app))
 
     async def on_shutdown(self, app: FastAPI) -> None:
+        from simple_module_core.activity_feed import set_activity_feed
+
         app.state.sm.db.audit_callback = None
+        set_activity_feed(app, None)
 
     def locale_dirs(self) -> dict[str, Path]:
         base = Path(str(importlib.resources.files(__package__) / "locales"))

modules/audit_log/tests/test_audit_log_activity_feed.py

@@ -58,3 +58,13 @@ async def _names(db, ids):
 
         admin_view = await feed.recent_for_actor(db, actor, limit=1, permissions=[WILDCARD])
         assert len(admin_view) == 1
+
+
+async def test_feed_is_installed_on_startup_and_cleared_on_shutdown(app):
+    from audit_log.module import AuditLogModule
+
+    assert isinstance(get_activity_feed(app), AuditActivityFeed)
+    await AuditLogModule().on_shutdown(app)
+    assert get_activity_feed(app) is None
+    await AuditLogModule().on_startup(app)
+    assert isinstance(get_activity_feed(app), AuditActivityFeed)

modules/keycloak/keycloak/directory.py

@@ -96,12 +96,15 @@ def match_ids(self, term: str) -> Any | None:
         return select(_UuidText(KeycloakUserCache.id)).where(_name_or_email_contains(term))
 
     async def stats(self, db: Any, *, active_window: timedelta) -> DirectoryStats:
-        count = select(func.count()).select_from(KeycloakUserCache)
-        total = (await db.execute(count)).scalar_one()
         cutoff = datetime.now(UTC) - active_window
-        active = (
-            await db.execute(count.where(KeycloakUserCache.last_login_at >= cutoff))
-        ).scalar_one()
+        total, active = (
+            await db.execute(
+                select(
+                    func.count(),
+                    func.count().filter(KeycloakUserCache.last_login_at >= cutoff),
+                ).select_from(KeycloakUserCache)
+            )
+        ).one()
         # The cache row is written at first sign-in, not account creation.
         return DirectoryStats(total=total, active_since=active, created_this_month=None)
 

modules/users/users/directory.py

@@ -101,12 +101,17 @@ async def stats(self, db: Any, *, active_window: timedelta) -> DirectoryStats:
         now = datetime.now(UTC)
         # Calendar month, not a rolling 30 days: the tile reads "+6 this month".
         month_start = now.replace(day=1, hour=0, minute=0, second=0, microsecond=0)
-        count = select(func.count()).select_from(User)
-        total = (await db.execute(count)).scalar_one()
-        active = (
-            await db.execute(count.where(User.last_login_at >= now - active_window))
-        ).scalar_one()
-        created = (await db.execute(count.where(User.created_at >= month_start))).scalar_one()
+        # One scan for all three figures rather than a query each.
+        row = (
+            await db.execute(
+                select(
+                    func.count(),
+                    func.count().filter(User.last_login_at >= now - active_window),
+                    func.count().filter(User.created_at >= month_start),
+                ).select_from(User)
+            )
+        ).one()
+        total, active, created = row
         return DirectoryStats(total=total, active_since=active, created_this_month=created)