Screen wireframes: 15 UX rethinks across the admin app

.gitignore

@@ -63,3 +63,4 @@ Thumbs.db
 host/client_app/.playwright-cli/*
 .superpowers/
 .qa/
+.verify/

framework/core/simple_module_core/__init__.py

@@ -1,5 +1,6 @@
 """SimpleModule Core - Module system, menu, permissions, events, and diagnostics."""
 
+from simple_module_core.audit_links import AuditLink, AuditLinkRegistry
 from simple_module_core.design_packs import DesignPack, DesignPackRegistry
 from simple_module_core.diagnostics import (
     DiagnosticLevel,
@@ -44,6 +45,8 @@
 __all__ = [
     "DEFAULT_AUTH_PROVIDER",
     "FRAMEWORK_API_VERSION",
+    "AuditLink",
+    "AuditLinkRegistry",
     "CircularDependencyError",
     "DesignPack",
     "DesignPackRegistry",

framework/core/simple_module_core/audit_links.py

@@ -0,0 +1,84 @@
+"""Audit-link registry — modules teach the audit log how to reach their records.
+
+An audit entry stores the *model class name* and primary key of the row that
+changed (``StoredFile``, ``a91f3c2b…`` — ``snapshot_changes`` records
+``type(obj).__name__``, never ``__tablename__``). That is enough to prove what
+happened and useless for doing anything about it: the reader has an id and no
+way to open the record it names.
+
+A module declares where its rows live via
+:meth:`~simple_module_core.module.ModuleBase.register_audit_links`; the host
+collects them into one registry at boot and stores it on
+``app.state.sm.audit_links``.
+
+**The registry maps model class names to URL templates, nothing more.** It does
+not verify the row exists or that the reader may open it — following a link to a
+deleted record lands on that screen's own 404, and permissions are enforced by
+the target route as usual.
+"""
+
+from __future__ import annotations
+
+from dataclasses import dataclass
+
+_ID_PLACEHOLDER = "{id}"
+
+
+@dataclass(frozen=True)
+class AuditLink:
+    """Where the records of one audited model can be viewed.
+
+    Args:
+        entity_type: The **model class name**, matching
+            ``AuditEntry.entity_type`` (e.g. ``"User"``, not ``"users_user"``).
+            ``snapshot_changes`` records ``type(obj).__name__``, so keying this
+            off ``__tablename__`` silently never matches — and because an
+            unmatched lookup falls back to showing ``entity_type`` as the
+            label, a table-name key looks like it worked.
+        url_template: Path containing ``{id}``, substituted with the entity id
+            (e.g. ``"/admin/users/{id}/edit"``).
+        label: Human-readable name for the entity kind, shown instead of the
+            raw class name (e.g. ``"User account"``).
+    """
+
+    entity_type: str
+    url_template: str
+    label: str = ""
+
+    def __post_init__(self) -> None:
+        if _ID_PLACEHOLDER not in self.url_template:
+            raise ValueError(
+                f"AuditLink for {self.entity_type!r} has url_template "
+                f"{self.url_template!r}, which contains no {_ID_PLACEHOLDER} — "
+                f"every row would link to the same page"
+            )
+
+    def url_for(self, entity_id: str) -> str:
+        return self.url_template.replace(_ID_PLACEHOLDER, entity_id)
+
+
+class AuditLinkRegistry:
+    """Aggregates every module's :class:`AuditLink` declarations.
+
+    Populated once during boot (``register_audit_links`` hook) and read
+    thereafter by the audit log view when it renders each row.
+    """
+
+    def __init__(self) -> None:
+        self._links: dict[str, AuditLink] = {}
+
+    def register(self, link: AuditLink) -> None:
+        existing = self._links.get(link.entity_type)
+        if existing is not None and existing != link:
+            raise ValueError(
+                f"Two modules claim audit links for {link.entity_type!r}: "
+                f"{existing.url_template!r} and {link.url_template!r}"
+            )
+        self._links[link.entity_type] = link
+
+    def get(self, entity_type: str) -> AuditLink | None:
+        return self._links.get(entity_type)
+
+    @property
+    def all_links(self) -> dict[str, AuditLink]:
+        return dict(self._links)

framework/core/simple_module_core/health.py

@@ -30,17 +30,55 @@ class HealthCheck:
 
     name: str
     check: HealthCheckFn
+    module: str = ""
+    """Module that contributed the check. Stamped by the registry during
+    ``register_health_checks``; module authors never set it by hand."""
+    probe: bool = True
+    """Whether automatic pollers may run this check.
+
+    Set ``False`` for checks that reach a third party — an SMTP login, an S3
+    request. Readiness is asked on a timer (a Kubernetes probe every 10s), and
+    a check that authenticates against a mail provider on that schedule earns
+    a rate-limit and binds probe latency to someone else's uptime. Such a
+    dependency is also not a readiness signal: the app serves pages fine while
+    its mailer is down.
+
+    ``False`` checks still run when explicitly requested — that is what the
+    "Test connection" action on the module-settings screen invokes.
+    """
 
 
 class HealthRegistry:
     """Collects health checks contributed by modules."""
 
     def __init__(self) -> None:
         self._checks: list[HealthCheck] = []
+        self._current_owner: str = ""
+
+    def set_owner(self, module_name: str) -> None:
+        """Attribute subsequently-added checks to ``module_name``.
+
+        The host calls this around each ``register_health_checks`` hook so a
+        check knows which module it belongs to without changing the ``add``
+        signature module authors already use. Attribution is what lets the
+        dashboard show health per module rather than one global number.
+        """
+        self._current_owner = module_name
 
     def add(self, check: HealthCheck) -> None:
+        if not check.module:
+            check.module = self._current_owner
         self._checks.append(check)
 
     @property
     def all_checks(self) -> list[HealthCheck]:
+        """Every registered check, including on-demand ones.
+
+        Callers that poll on a timer want :attr:`probe_checks` instead.
+        """
         return list(self._checks)
+
+    @property
+    def probe_checks(self) -> list[HealthCheck]:
+        """Checks safe to run automatically, on a timer."""
+        return [c for c in self._checks if c.probe]

framework/core/simple_module_core/menu.py

@@ -30,6 +30,15 @@ class MenuItem:
     requires_auth: bool = True
     roles: list[str] = field(default_factory=list)
     """Empty list = visible to all authenticated users."""
+    permissions: list[str] = field(default_factory=list)
+    """Permission keys required to see this entry. Empty = no permission check.
+
+    Declare the same permission the target route enforces. Roles alone cannot
+    express this: a custom role holding ``settings.view`` should see Settings,
+    and hard-coding ``roles=["admin"]`` would hide it from them while still
+    showing it to any admin-adjacent role that cannot actually open it. An
+    entry that 403s on click is worse than no entry at all.
+    """
     method: MenuItemMethod = "get"
     """HTTP method used when the item is activated. ``"post"`` renders as an
     Inertia form submission so the target endpoint can be POST-only (e.g. logout)."""
@@ -69,19 +78,29 @@ def get_for_user(
         *,
         is_authenticated: bool,
         roles: list[str] | None = None,
+        permissions: list[str] | None = None,
     ) -> dict[str, list[dict]]:
-        """Return menu items grouped by section, filtered by auth/roles.
+        """Return menu items grouped by section, filtered by auth/roles/permissions.
+
+        ``permissions`` is the caller's already-expanded permission list (no
+        wildcards). Items declaring permissions the caller lacks are dropped,
+        so the sidebar never offers a screen that will 403 on click.
 
         Returns a dict ready to be serialized into Inertia shared props.
         """
         roles = roles or []
+        granted = set(permissions or [])
         result: dict[str, list[dict]] = {s.value: [] for s in MenuSection}
 
         for item in self.all_items:
             if item.requires_auth and not is_authenticated:
                 continue
             if item.roles and not any(r in item.roles for r in roles):
                 continue
+            # All declared permissions must be held: an entry naming several is
+            # asking for all of them, matching how route guards compose.
+            if item.permissions and not granted.issuperset(item.permissions):
+                continue
             result[item.section.value].append(
                 {
                     "label": item.label,

framework/core/simple_module_core/module.py

@@ -10,6 +10,7 @@
 if TYPE_CHECKING:
     from fastapi import APIRouter, FastAPI
 
+    from simple_module_core.audit_links import AuditLinkRegistry
     from simple_module_core.design_packs import DesignPackRegistry
     from simple_module_core.events import EventBus
     from simple_module_core.feature_flags import FeatureFlagRegistry
@@ -163,6 +164,28 @@ def register_design_packs(self, registry):
         order.
         """
 
+    def register_audit_links(self, registry: AuditLinkRegistry) -> None:
+        """Declare where this module's audited records can be viewed.
+
+        An audit entry stores a table name and a primary key, which tells the
+        reader what changed but gives them no way to open it. Override this
+        hook to make your module's rows reachable from the audit log::
+
+            def register_audit_links(self, registry):
+                registry.register(
+                    AuditLink(
+                        entity_type="users_user",
+                        url_template="/admin/users/{id}/edit",
+                        label="User",
+                    )
+                )
+
+        ``entity_type`` is the ``__tablename__`` the rows are audited under.
+        Registering only supplies the URL — the target route still enforces
+        its own permissions, so linking never widens access. Called once at
+        boot, in dependency order.
+        """
+
     def register_middleware(self, app: FastAPI) -> None:
         """Add middleware to the application.
 

framework/core/simple_module_core/services.py

@@ -20,6 +20,7 @@
     from simple_module_db.session import DatabaseState
     from simple_module_hosting.settings import Settings
 
+    from simple_module_core.audit_links import AuditLinkRegistry
     from simple_module_core.design_packs import DesignPackRegistry
     from simple_module_core.events import EventBus
     from simple_module_core.feature_flags import FeatureFlagRegistry
@@ -44,6 +45,7 @@ class Services:
     health_registry: HealthRegistry
     public_routes: PublicRouteRegistry
     design_packs: DesignPackRegistry
+    audit_links: AuditLinkRegistry
     i18n_registry: I18nRegistry
     inertia_config: InertiaConfig
     modules: tuple[ModuleBase, ...]

framework/core/tests/test_audit_links.py

@@ -0,0 +1,57 @@
+"""Tests for AuditLink / AuditLinkRegistry.
+
+The audit log stores a table name and a primary key. This registry is how a
+module says where those rows can be opened, so the log stops being a wall of
+unactionable uuids.
+"""
+
+from __future__ import annotations
+
+import pytest
+from simple_module_core.audit_links import AuditLink, AuditLinkRegistry
+
+
+class TestAuditLink:
+    def test_url_for_substitutes_the_id(self):
+        link = AuditLink(entity_type="User", url_template="/users/admin/{id}")
+        assert link.url_for("a91f3c2b") == "/users/admin/a91f3c2b"
+
+    def test_template_without_placeholder_is_rejected(self):
+        """Every row would otherwise link to the same page."""
+        with pytest.raises(ValueError, match="contains no"):
+            AuditLink(entity_type="User", url_template="/users/admin")
+
+    def test_label_defaults_to_empty(self):
+        assert AuditLink(entity_type="t", url_template="/t/{id}").label == ""
+
+
+class TestAuditLinkRegistry:
+    def test_get_returns_none_for_unclaimed_tables(self):
+        """Join tables and blob stores have no screen; that is not an error."""
+        assert AuditLinkRegistry().get("UserPermission") is None
+
+    def test_register_and_get(self):
+        reg = AuditLinkRegistry()
+        link = AuditLink(entity_type="User", url_template="/users/admin/{id}", label="User")
+        reg.register(link)
+        assert reg.get("User") is link
+
+    def test_conflicting_claims_raise(self):
+        """Two modules mapping one table means one of them silently loses."""
+        reg = AuditLinkRegistry()
+        reg.register(AuditLink(entity_type="User", url_template="/a/{id}"))
+        with pytest.raises(ValueError, match="Two modules claim"):
+            reg.register(AuditLink(entity_type="User", url_template="/b/{id}"))
+
+    def test_registering_the_same_link_twice_is_allowed(self):
+        """Idempotent re-registration must not break a re-entrant boot."""
+        reg = AuditLinkRegistry()
+        for _ in range(2):
+            reg.register(AuditLink(entity_type="User", url_template="/a/{id}"))
+        assert len(reg.all_links) == 1
+
+    def test_all_links_is_a_copy(self):
+        reg = AuditLinkRegistry()
+        reg.register(AuditLink(entity_type="User", url_template="/a/{id}"))
+        reg.all_links.clear()
+        assert reg.get("User") is not None

framework/core/tests/test_health_registry.py

@@ -33,6 +33,41 @@ async def check_b() -> HealthCheckResult:
         reg.add(HealthCheck(name="b", check=check_b))
         assert len(reg.all_checks) == 2
 
+    async def test_checks_are_attributed_to_the_owning_module(self):
+        """The dashboard shows health per module, which needs this attribution."""
+        reg = HealthRegistry()
+
+        async def check() -> HealthCheckResult:
+            return HealthCheckResult(status=HealthStatus.HEALTHY)
+
+        reg.set_owner("FileStorage")
+        reg.add(HealthCheck(name="s3", check=check))
+        reg.set_owner("BackgroundTasks")
+        reg.add(HealthCheck(name="broker", check=check))
+
+        owners = {c.name: c.module for c in reg.all_checks}
+        assert owners == {"s3": "FileStorage", "broker": "BackgroundTasks"}
+
+    async def test_explicit_module_survives_the_current_owner(self):
+        reg = HealthRegistry()
+
+        async def check() -> HealthCheckResult:
+            return HealthCheckResult(status=HealthStatus.HEALTHY)
+
+        reg.set_owner("Dashboard")
+        reg.add(HealthCheck(name="db", check=check, module="Users"))
+        assert reg.all_checks[0].module == "Users"
+
+    async def test_unowned_checks_have_no_module(self):
+        """Checks added outside a register_health_checks hook belong to nobody."""
+        reg = HealthRegistry()
+
+        async def check() -> HealthCheckResult:
+            return HealthCheckResult(status=HealthStatus.HEALTHY)
+
+        reg.add(HealthCheck(name="db", check=check))
+        assert reg.all_checks[0].module == ""
+
     async def test_check_result_defaults(self):
         result = HealthCheckResult(status=HealthStatus.HEALTHY)
         assert result.detail is None