diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 3f98f120..78aa7109 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -158,7 +158,10 @@ jobs: SM_USERS_BOOTSTRAP_PASSWORD: admin # Exclude Keycloak module — SM020 prevents both users and keycloak # from running simultaneously. E2E tests use the users module. - SM_MODULES_ENABLED: '["Auth","Users","Dashboard","Permissions","Settings","BackgroundTasks","FileStorage","FeatureFlags","AuditLog"]' + # Branding is included so the admin section's "edited on its own page, + # never in the generic module editor" rule is actually exercised — the + # server-side half of that is a 409 guard, which is worth CI coverage. + SM_MODULES_ENABLED: '["Auth","Users","Dashboard","Permissions","Settings","BackgroundTasks","FileStorage","FeatureFlags","AuditLog","Branding"]' E2E_BASE_URL: http://localhost:8000 steps: - uses: actions/checkout@v7 diff --git a/framework/cli/tests/test_cli_package_update.py b/framework/cli/tests/test_cli_package_update.py index ae944542..d64e830d 100644 --- a/framework/cli/tests/test_cli_package_update.py +++ b/framework/cli/tests/test_cli_package_update.py @@ -2,6 +2,7 @@ from __future__ import annotations +import importlib from pathlib import Path from urllib.error import HTTPError @@ -156,17 +157,30 @@ def fetcher(url: str) -> dict: assert "simple_module_core>=2.0.0rc1" in pyproject.read_text(encoding="utf-8") +def _exit_exception_types() -> tuple[type[BaseException], ...]: + """Every ``Exit`` class this typer build might raise. + + typer's layout is not fixed by its version number: some 0.27.1 installs + vendor click as ``typer._click`` (whose ``Exit`` does not inherit from + click's) and ship no ``typer.exceptions``, while others ship + ``typer.exceptions`` and no ``typer._click``. Naming either path directly + is what made this test pass on one machine and fail on another, so probe + both and keep whatever exists. + """ + found: list[type[BaseException]] = [click.exceptions.Exit] + for module_path in ("typer._click.exceptions", "typer.exceptions"): + try: + module = importlib.import_module(module_path) + except ImportError: + continue + exit_cls = getattr(module, "Exit", None) + if isinstance(exit_cls, type) and issubclass(exit_cls, BaseException): + found.append(exit_cls) + return tuple(found) + + def test_missing_pyproject_exits_nonzero(tmp_path: Path) -> None: - # typer >= 0.26 vendors click as ``typer._click``; the raised ``Exit`` - # no longer inherits from ``click.exceptions.Exit``. Catch both. - _exit_types: tuple[type[BaseException], ...] = (click.exceptions.Exit,) - try: - from typer._click.exceptions import Exit as _TyExit - - _exit_types = (*_exit_types, _TyExit) - except ImportError: - pass - with pytest.raises(_exit_types) as exc: + with pytest.raises(_exit_exception_types()) as exc: pu.run_update( path=tmp_path, dry_run=False, diff --git a/framework/core/simple_module_core/diagnostics/_pages.py b/framework/core/simple_module_core/diagnostics/_pages.py index 2023b94c..f805236f 100644 --- a/framework/core/simple_module_core/diagnostics/_pages.py +++ b/framework/core/simple_module_core/diagnostics/_pages.py @@ -12,17 +12,62 @@ from simple_module_core.module import ModuleBase +def _assignment(s: ast.stmt) -> tuple[ast.Name, ast.expr] | None: + """Return ``(target, value)`` for a single-target top-level assignment. + + Covers both ``NAME = ...`` and the annotated ``NAME: Final = ...`` form + module constants are conventionally written in. + """ + if isinstance(s, ast.Assign) and len(s.targets) == 1 and isinstance(s.targets[0], ast.Name): + return s.targets[0], s.value + if isinstance(s, ast.AnnAssign) and isinstance(s.target, ast.Name) and s.value is not None: + return s.target, s.value + return None + + def _module_level_str_consts(tree: ast.Module) -> dict[str, str]: """Return ``{name: literal}`` for top-level ``NAME = "string"`` assignments.""" - return { - s.targets[0].id: s.value.value - for s in tree.body - if isinstance(s, ast.Assign) - and len(s.targets) == 1 - and isinstance(s.targets[0], ast.Name) - and isinstance(s.value, ast.Constant) - and isinstance(s.value.value, str) - } + consts: dict[str, str] = {} + for s in tree.body: + assignment = _assignment(s) + if assignment is None: + continue + target, value = assignment + if isinstance(value, ast.Constant) and isinstance(value.value, str): + consts[target.id] = value.value + return consts + + +def _resolve_fstring_consts(tree: ast.Module, consts: dict[str, str]) -> dict[str, str]: + """Resolve top-level ``NAME = f"{CONST}/lit"`` against already-known consts. + + Only plain interpolations of known string constants count — a conversion, + format spec, or unknown name makes the value non-static, so it is skipped + rather than guessed at. + """ + resolved: dict[str, str] = {} + for s in tree.body: + assignment = _assignment(s) + if assignment is None or not isinstance(assignment[1], ast.JoinedStr): + continue + target, value = assignment + parts: list[str] = [] + for piece in value.values: + if isinstance(piece, ast.Constant) and isinstance(piece.value, str): + parts.append(piece.value) + elif ( + isinstance(piece, ast.FormattedValue) + and isinstance(piece.value, ast.Name) + and piece.value.id in consts + and piece.conversion == -1 + and piece.format_spec is None + ): + parts.append(consts[piece.value.id]) + else: + break + else: + resolved[target.id] = "".join(parts) + return resolved def _iter_render_components(tree: ast.Module, consts: dict[str, str]) -> list[str]: @@ -79,6 +124,19 @@ def find_render_calls(mod: ModuleBase, src_dir: Path) -> set[str]: consts: dict[str, str] = {} for tree in trees: consts.update(_module_level_str_consts(tree)) + # Then f-string constants built from the plain ones above + # (``PAGE = f"{MODULE_NAME}/Browse"`` is the conventional shape). Repeat to + # a fixed point so a chain — ``PREFIX = f"{NAME}/sub"`` then + # ``PAGE = f"{PREFIX}/Browse"`` — resolves too; one pass would only learn + # the first hop and report the page as an SM003 orphan. + while True: + resolved: dict[str, str] = {} + for tree in trees: + resolved.update(_resolve_fstring_consts(tree, consts)) + new = {k: v for k, v in resolved.items() if consts.get(k) != v} + if not new: + break + consts.update(new) prefix = f"{mod.meta.name}/" rendered: set[str] = set() diff --git a/framework/core/simple_module_core/redirect_safety.py b/framework/core/simple_module_core/redirect_safety.py index 82e09546..62af4bff 100644 --- a/framework/core/simple_module_core/redirect_safety.py +++ b/framework/core/simple_module_core/redirect_safety.py @@ -53,4 +53,26 @@ def safe_next_or_none(raw: str | None) -> str | None: return result or None -__all__ = ["DEFAULT_FALLBACK", "SESSION_NEXT_KEY", "safe_next", "safe_next_or_none"] +def non_empty_redirect(value: str, *, default: str) -> str: + """Normalise a configured redirect destination, never returning ``""``. + + Any auth provider can expose a ``login_redirect_url``-style setting, and + nothing stops an admin clearing it in the generic module-settings editor. + Every consumer treats the value as a destination — Inertia's + ``router.visit("")`` silently reloads the current page, and an empty + ``Location`` header is a broken redirect — so providers normalise on their + settings class, where hydration and ``apply_changes_and_reload`` both run. + It lives here rather than in one provider because the providers must not + import each other (cross-module coupling), and this is the same concern as + the rest of this module. + """ + return value.strip() or default + + +__all__ = [ + "DEFAULT_FALLBACK", + "SESSION_NEXT_KEY", + "non_empty_redirect", + "safe_next", + "safe_next_or_none", +] diff --git a/framework/core/tests/test_module_diagnostics.py b/framework/core/tests/test_module_diagnostics.py index 6b103d77..c84e62eb 100644 --- a/framework/core/tests/test_module_diagnostics.py +++ b/framework/core/tests/test_module_diagnostics.py @@ -33,42 +33,6 @@ def _mk_module_tree(root: Path, name: str, *, with_pkg_json: bool, with_tsconfig return src_dir -class TestSm003PageRenderResolution: - """SM003 must resolve PAGE_X constants imported from sibling files.""" - - def _diags(self, src_dir: Path, mod_name: str): - from simple_module_core.diagnostics._pages import check_pages, find_render_calls - - mod = _FakeModule(meta=_FakeMeta(name=mod_name)) - rendered = find_render_calls(mod, src_dir) # pyright: ignore[reportArgumentType] - return [d for d in check_pages(mod, src_dir, rendered) if d.code == "SM003"] # pyright: ignore[reportArgumentType] - - async def test_resolves_constant_imported_from_sibling_file(self, tmp_path: Path): - src_dir = tmp_path / "feature_flags" / "feature_flags" - (src_dir / "pages").mkdir(parents=True) - (src_dir / "pages" / "Browse.tsx").write_text("export default function B() {}") - (src_dir / "constants.py").write_text('PAGE_BROWSE = "FeatureFlags/Browse"\n') - endpoints = src_dir / "endpoints" - endpoints.mkdir() - (endpoints / "views.py").write_text( - "from feature_flags.constants import PAGE_BROWSE\n" - "async def view(inertia):\n" - " return await inertia.render(PAGE_BROWSE, {})\n" - ) - assert self._diags(src_dir, "FeatureFlags") == [] - - async def test_still_flags_truly_orphan_pages(self, tmp_path: Path): - src_dir = tmp_path / "m" / "m" - (src_dir / "pages").mkdir(parents=True) - (src_dir / "pages" / "Ghost.tsx").write_text("export default function G() {}") - (src_dir / "endpoints.py").write_text( - 'async def view(inertia):\n return await inertia.render("M/Other", {})\n' - ) - results = self._diags(src_dir, "M") - assert [r.code for r in results] == ["SM003"] - assert "Ghost.tsx" in results[0].message - - class TestSm017JsWorkspaceFiles: async def test_fires_when_both_missing(self, tmp_path: Path): src_dir = _mk_module_tree(tmp_path, "orders", with_pkg_json=False, with_tsconfig=False) diff --git a/framework/core/tests/test_sm003_page_render.py b/framework/core/tests/test_sm003_page_render.py new file mode 100644 index 00000000..91a7426f --- /dev/null +++ b/framework/core/tests/test_sm003_page_render.py @@ -0,0 +1,122 @@ +"""SM003 page-render resolution tests, split from test_module_diagnostics. + +The resolver reads inertia.render() targets out of module source; these tests +cover the constant shapes modules actually use (plain, imported, annotated +f-string) and the dynamic shapes it must refuse to guess at. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from pathlib import Path + + +@dataclass +class _FakeMeta: + name: str + + +@dataclass +class _FakeModule: + meta: _FakeMeta + + +class TestSm003PageRenderResolution: + """SM003 must resolve PAGE_X constants imported from sibling files.""" + + def _diags(self, src_dir: Path, mod_name: str): + from simple_module_core.diagnostics._pages import check_pages, find_render_calls + + mod = _FakeModule(meta=_FakeMeta(name=mod_name)) + rendered = find_render_calls(mod, src_dir) # pyright: ignore[reportArgumentType] + return [d for d in check_pages(mod, src_dir, rendered) if d.code == "SM003"] # pyright: ignore[reportArgumentType] + + async def test_resolves_constant_imported_from_sibling_file(self, tmp_path: Path): + src_dir = tmp_path / "feature_flags" / "feature_flags" + (src_dir / "pages").mkdir(parents=True) + (src_dir / "pages" / "Browse.tsx").write_text("export default function B() {}") + (src_dir / "constants.py").write_text('PAGE_BROWSE = "FeatureFlags/Browse"\n') + endpoints = src_dir / "endpoints" + endpoints.mkdir() + (endpoints / "views.py").write_text( + "from feature_flags.constants import PAGE_BROWSE\n" + "async def view(inertia):\n" + " return await inertia.render(PAGE_BROWSE, {})\n" + ) + assert self._diags(src_dir, "FeatureFlags") == [] + + async def test_still_flags_truly_orphan_pages(self, tmp_path: Path): + src_dir = tmp_path / "m" / "m" + (src_dir / "pages").mkdir(parents=True) + (src_dir / "pages" / "Ghost.tsx").write_text("export default function G() {}") + (src_dir / "endpoints.py").write_text( + 'async def view(inertia):\n return await inertia.render("M/Other", {})\n' + ) + results = self._diags(src_dir, "M") + assert [r.code for r in results] == ["SM003"] + assert "Ghost.tsx" in results[0].message + + async def test_resolves_annotated_fstring_constant(self, tmp_path: Path): + """The conventional shape: ``PAGE: Final = f"{MODULE_NAME}/Browse"``. + + Regression test for a false SM003 against audit_log — the resolver + skipped both annotated assignments and f-strings, so every module + writing its page name this way was flagged as an orphan. + """ + src_dir = tmp_path / "audit_log" / "audit_log" + (src_dir / "pages").mkdir(parents=True) + (src_dir / "pages" / "Browse.tsx").write_text("export default function B() {}") + (src_dir / "constants.py").write_text( + "from typing import Final\n" + 'MODULE_NAME: Final = "AuditLog"\n' + 'PAGE_BROWSE: Final = f"{MODULE_NAME}/Browse"\n' + ) + endpoints = src_dir / "endpoints" + endpoints.mkdir() + (endpoints / "views.py").write_text( + "from audit_log.constants import PAGE_BROWSE\n" + "async def view(inertia):\n" + " return await inertia.render(PAGE_BROWSE, {})\n" + ) + assert self._diags(src_dir, "AuditLog") == [] + + async def test_resolves_a_chained_fstring_constant(self, tmp_path: Path): + """``PREFIX = f"{NAME}/sub"`` then ``PAGE = f"{PREFIX}/Browse"``. + + Resolution runs to a fixed point; a single pass would learn only + ``PREFIX`` and wrongly report the page as an orphan. + """ + src_dir = tmp_path / "m" / "m" + (src_dir / "pages").mkdir(parents=True) + (src_dir / "pages" / "Browse.tsx").write_text("export default function B() {}") + (src_dir / "constants.py").write_text( + "from typing import Final\n" + 'MODULE_NAME: Final = "M"\n' + 'SECTION: Final = f"{MODULE_NAME}/admin"\n' + 'PAGE_BROWSE: Final = f"{SECTION}/Browse"\n' + ) + (src_dir / "views.py").write_text( + "from m.constants import PAGE_BROWSE\n" + "async def view(inertia):\n" + " return await inertia.render(PAGE_BROWSE, {})\n" + ) + # The rendered component is "M/admin/Browse" while the page file is + # pages/Browse.tsx, so this asserts the constant resolved at all — + # an unresolved chain reports Browse.tsx as an SM003 orphan. + from simple_module_core.diagnostics._pages import find_render_calls + + mod = _FakeModule(meta=_FakeMeta(name="M")) + assert "admin/Browse" in find_render_calls(mod, src_dir) # pyright: ignore[reportArgumentType] + + async def test_fstring_with_unknown_name_stays_flagged(self, tmp_path: Path): + """An f-string over a runtime value is not static — don't guess.""" + src_dir = tmp_path / "m" / "m" + (src_dir / "pages").mkdir(parents=True) + (src_dir / "pages" / "Browse.tsx").write_text("export default function B() {}") + (src_dir / "endpoints.py").write_text( + 'PAGE = f"{dynamic()}/Browse"\n' + "async def view(inertia):\n" + " return await inertia.render(PAGE, {})\n" + ) + results = self._diags(src_dir, "M") + assert [r.code for r in results] == ["SM003"] diff --git a/framework/hosting/simple_module_hosting/migrations.py b/framework/hosting/simple_module_hosting/migrations.py index 6291baf4..c96b0933 100644 --- a/framework/hosting/simple_module_hosting/migrations.py +++ b/framework/hosting/simple_module_hosting/migrations.py @@ -7,15 +7,26 @@ logger = logging.getLogger(__name__) +def script_directory(alembic_ini_path: str = "host/alembic.ini"): + """Build the ``ScriptDirectory`` for ``alembic_ini_path``. + + Shared by every caller that needs to walk or query alembic revisions + (boot-time head check, the in-app Doctor screen) so the config/ini-path + construction has exactly one source of truth. + """ + from alembic.config import Config as AlembicConfig + from alembic.script import ScriptDirectory + + return ScriptDirectory.from_config(AlembicConfig(alembic_ini_path)) + + def resolve_head_revision(alembic_ini_path: str = "host/alembic.ini") -> str | None: """Return the current head revision string, or ``None`` if alembic isn't configured at ``alembic_ini_path`` or has no revisions.""" - from alembic.config import Config as AlembicConfig - from alembic.script import ScriptDirectory from alembic.util.exc import CommandError try: - return ScriptDirectory.from_config(AlembicConfig(alembic_ini_path)).get_current_head() + return script_directory(alembic_ini_path).get_current_head() except (CommandError, FileNotFoundError) as exc: logger.debug("Alembic not available: %s", exc) return None @@ -26,9 +37,7 @@ async def check_migrations(engine, alembic_ini_path: str = "host/alembic.ini") - Returns a dict with migration status for storage on app.state. """ - from alembic.config import Config as AlembicConfig from alembic.runtime.migration import MigrationContext - from alembic.script import ScriptDirectory _no_migrations = { "current_revision": None, @@ -40,7 +49,7 @@ async def check_migrations(engine, alembic_ini_path: str = "host/alembic.ini") - head = resolve_head_revision(alembic_ini_path) if head is None: return _no_migrations - script = ScriptDirectory.from_config(AlembicConfig(alembic_ini_path)) + script = script_directory(alembic_ini_path) async with engine.connect() as conn: diff --git a/host/client_app/components/CopyCommand.tsx b/host/client_app/components/CopyCommand.tsx index c007dd22..45e85eb8 100644 --- a/host/client_app/components/CopyCommand.tsx +++ b/host/client_app/components/CopyCommand.tsx @@ -23,11 +23,15 @@ export function CopyCommand({ command }: { command: string }) { const { t } = useT(); const [copied, setCopied] = useState(false); const timer = useRef | null>(null); + const mounted = useRef(true); // Clear on unmount so the reset can't fire into a gone component, and so a // rapid second click restarts the window rather than stacking timeouts. + // `mounted` guards the state updates below it for the same reason: a click + // right before navigating away can have `writeText` resolve after unmount. useEffect( () => () => { + mounted.current = false; if (timer.current) clearTimeout(timer.current); }, [], @@ -42,6 +46,7 @@ export function CopyCommand({ command }: { command: string }) { // selectable, which is the fallback either way. return; } + if (!mounted.current) return; setCopied(true); if (timer.current) clearTimeout(timer.current); timer.current = setTimeout(() => setCopied(false), RESET_AFTER_MS); diff --git a/modules/audit_log/audit_log/locales/en.json b/modules/audit_log/audit_log/locales/en.json index 629536e0..5e4894f8 100644 --- a/modules/audit_log/audit_log/locales/en.json +++ b/modules/audit_log/audit_log/locales/en.json @@ -50,7 +50,9 @@ "show_less": "Show less", "system_user": "System", "no_changes": "—", - "unresolved_user": "No matching account for this id" + "unresolved_user": "No matching account for this id", + "fields_set_one": "{count} field set", + "fields_set_other": "{count} fields set" }, "nav": { "audit_log": "Audit Log" diff --git a/modules/background_tasks/background_tasks/pages/Index.tsx b/modules/background_tasks/background_tasks/pages/Index.tsx index 056b0525..ffa90124 100644 --- a/modules/background_tasks/background_tasks/pages/Index.tsx +++ b/modules/background_tasks/background_tasks/pages/Index.tsx @@ -47,6 +47,9 @@ interface Props { /** Task, Status, Queue, Queued, Duration, Worker, Actions. */ const COLUMN_COUNT = 7; +// Same header treatment as the other admin tables (users, audit log, flags). +const TH = 'text-[11px] font-semibold uppercase tracking-[0.08em] text-muted-foreground'; + const STATUS_ALL = '__all__'; function pushFilters(filters: { status: string; task_name: string }, page: number): void { @@ -181,23 +184,23 @@ function Index() { - + - {t(keys.background_tasks.table.task)} - {t(keys.background_tasks.table.status)} - + {t(keys.background_tasks.table.task)} + {t(keys.background_tasks.table.status)} + - + - + - + - + {t(keys.background_tasks.table.actions)} diff --git a/modules/branding/branding/module.py b/modules/branding/branding/module.py index e92693be..4a0d1477 100644 --- a/modules/branding/branding/module.py +++ b/modules/branding/branding/module.py @@ -11,7 +11,7 @@ from pathlib import Path from fastapi import APIRouter, FastAPI -from simple_module_core.menu import MenuItem, MenuRegistry +from simple_module_core.menu import MenuItem, MenuRegistry, MenuSection from simple_module_core.module import ModuleBase, ModuleMeta from simple_module_core.permissions import PermissionRegistry from simple_module_core.public_routes import PublicRouteRegistry @@ -42,6 +42,9 @@ def register_settings(self, app: FastAPI) -> None: constants.PACKAGE, BrandingSettings, lambda s: BrandingServices(settings=s), + # Branding ships its own management page; the generic module-settings + # editor links there instead of double-editing the same fields. + manage_url=MENU_URL, ) def register_permissions(self, registry: PermissionRegistry) -> None: @@ -68,6 +71,10 @@ def register_menu_items(self, registry: MenuRegistry) -> None: label_key="branding.nav.branding", url=MENU_URL, icon="palette", + # Between Access (100) and System (110) so the admin sidebar + # reads Access → Appearance → System. + order=105, + section=MenuSection.ADMIN_SIDEBAR, group="Appearance", group_key="ui.nav_groups.appearance", roles=["admin"], diff --git a/modules/branding/branding/pages/Manage.tsx b/modules/branding/branding/pages/Manage.tsx index 359f9510..82530043 100644 --- a/modules/branding/branding/pages/Manage.tsx +++ b/modules/branding/branding/pages/Manage.tsx @@ -2,13 +2,7 @@ import { Head, router, usePage } from '@inertiajs/react'; import { keys, useT } from '@simple-module-py/i18n'; import { PageShell } from '@simple-module-py/ui/components/PageShell'; import { Button } from '@simple-module-py/ui/components/ui/button'; -import { - Card, - CardContent, - CardDescription, - CardHeader, - CardTitle, -} from '@simple-module-py/ui/components/ui/card'; +import { Card, CardContent } from '@simple-module-py/ui/components/ui/card'; import { Input } from '@simple-module-py/ui/components/ui/input'; import { Label } from '@simple-module-py/ui/components/ui/label'; import { AdminLayout } from '@simple-module-py/ui/layouts/AdminLayout'; @@ -136,12 +130,10 @@ function Manage() { description={t(keys.branding.manage.description)} >
+ {/* The PageShell above already carries the title + description; + repeating them inside the card read as a glitch. */} - - {t(keys.branding.manage.title)} - {t(keys.branding.manage.description)} - - +
str | None: + """Trim the working directory off absolute finding paths for display.""" + if not file: + return file + try: + return str(Path(file).relative_to(Path.cwd())) + except ValueError: + return file + + +_RECENT_LIMIT = 5 + + +def collect_diagnostics(app: FastAPI) -> list[dict[str, Any]]: + """Run the module + i18n diagnostics and serialize the findings. + + Mirrors the dev-boot run in ``app_builder.create_app`` (minus the host/ui + locale extras, which only the builder can see). Sorted errors-first so the + screen leads with what needs fixing. + """ + sm = app.state.sm + diagnostics = run_diagnostics( + list(sm.modules), + i18n_supported_locales=sm.settings.i18n_supported_locales, + i18n_default_locale=sm.settings.i18n_default_locale, + ) + order = {"error": 0, "warning": 1, "info": 2} + diagnostics.sort(key=lambda d: (order.get(d.level.value, 3), d.code)) + return [ + { + "level": d.level.value, + "code": d.code, + "message": d.message, + "module": d.module_name, + "file": _relative(d.file), + "suggestion": d.suggestion, + } + for d in diagnostics + ] + + +def migration_overview(app: FastAPI) -> dict[str, Any]: + """Migration state from the boot check, plus the most recent revisions. + + ``app.state.migration`` exists because the lifespan refuses to start a + behind-head app, so a running app is at head by construction — the value + of this panel is showing *which* head, and what recently changed. + """ + state = getattr(app.state, "migration", None) or {} + return { + "current_revision": state.get("current_revision"), + "head_revision": state.get("head_revision"), + "is_current": state.get("is_current", True), + "recent": _recent_revisions(), + } + + +def _recent_revisions(limit: int = _RECENT_LIMIT) -> list[dict[str, Any]]: + """Newest ``limit`` alembic revisions (head first), or ``[]`` when the + script directory isn't present (e.g. a deployment without host/).""" + try: + script = script_directory() + revisions = [] + for rev in script.walk_revisions(): + revisions.append( + { + "revision": rev.revision[:12], + "message": rev.doc or "", + "modules": sorted(rev.branch_labels or ()), + } + ) + if len(revisions) >= limit: + break + return revisions + except Exception as exc: # pragma: no cover - depends on deploy layout + logger.debug("Alembic script directory unavailable: %s", exc) + return [] + + +def environment_info(app: FastAPI) -> dict[str, Any]: + """Live environment facts: mode, database backend, locales.""" + sm = app.state.sm + return { + "environment": sm.settings.environment, + "database": sm.db.engine.dialect.name, + "locales": list(sm.settings.i18n_supported_locales), + "default_locale": sm.settings.i18n_default_locale, + } diff --git a/modules/dashboard/dashboard/endpoints/views.py b/modules/dashboard/dashboard/endpoints/views.py index 1c72319d..c5948c6d 100644 --- a/modules/dashboard/dashboard/endpoints/views.py +++ b/modules/dashboard/dashboard/endpoints/views.py @@ -6,6 +6,8 @@ from __future__ import annotations +import asyncio + from fastapi import APIRouter, Depends, HTTPException, Request from inertia import InertiaResponse from simple_module_core.permissions import is_admin @@ -60,12 +62,26 @@ async def doctor( inertia: InertiaDep, db: AsyncSession = Depends(get_db), ) -> InertiaResponse: - """`make doctor` mirror — static checks, modules, dev server, env.""" - stats = await fetch_dashboard_stats(db, request.app) + """`make doctor` mirror — live diagnostics, migrations, modules, env.""" + from dashboard.doctor import collect_diagnostics, environment_info, migration_overview + + # collect_diagnostics/migration_overview do blocking filesystem walks + + # AST parses (module coupling/page checks, the alembic script directory) — + # run them off the event loop and alongside the DB stats fetch instead of + # serially after it, so one doctor request doesn't stall every other + # coroutine on this worker for the duration of a full framework scan. + stats, diagnostics, migration = await asyncio.gather( + fetch_dashboard_stats(db, request.app), + asyncio.to_thread(collect_diagnostics, request.app), + asyncio.to_thread(migration_overview, request.app), + ) return await inertia.render( _PAGE_DOCTOR, { "module_count": stats["module_count"], "system_info": stats["system_info"], + "diagnostics": diagnostics, + "migration": migration, + "environment": environment_info(request.app), }, ) diff --git a/modules/dashboard/dashboard/locales/en.json b/modules/dashboard/dashboard/locales/en.json index cc9ccae2..7a5660a6 100644 --- a/modules/dashboard/dashboard/locales/en.json +++ b/modules/dashboard/dashboard/locales/en.json @@ -31,30 +31,33 @@ }, "doctor": { "title": "Doctor", - "description": "Static checks, migrations, dev server, and module health. Mirrors `make doctor` output.", + "description": "Live module diagnostics, migration state and environment. Mirrors `make doctor`.", "rerun": "Re-run", - "stat_checks_passed": "Checks passed", "stat_modules": "Modules", - "stat_pending_migrations": "Pending mig.", "stat_health": "Health", "ok": "OK", "review": "review", - "clean": "clean", "alert": "{count} alert", - "static_checks": "Static checks", - "just_now": "just now", "recent_migrations": "Recent migrations", - "generate": "Generate", - "apply": "Apply", "applied": "applied", - "pending": "pending", "installed_modules": "Installed modules", "loaded": "loaded", "active": "active", - "dev_server": "Dev server", - "running": "running", "run_command": "Run a command", - "environment": "Environment" + "environment": "Environment", + "stat_errors": "Errors", + "stat_warnings": "Warnings", + "diagnostics": "Diagnostics", + "all_clear_title": "All checks pass", + "all_clear_hint": "No findings from the module, i18n and migration checks.", + "at_head": "DB at head", + "behind": "behind", + "suggestion": "Suggestion", + "env_mode": "Mode", + "env_database": "Database", + "env_python": "Python", + "env_locales": "Locales", + "env_revision": "Revision" }, "nav": { "dashboard": "Dashboard", diff --git a/modules/dashboard/dashboard/pages/Doctor.tsx b/modules/dashboard/dashboard/pages/Doctor.tsx index b18fedfc..db2af827 100644 --- a/modules/dashboard/dashboard/pages/Doctor.tsx +++ b/modules/dashboard/dashboard/pages/Doctor.tsx @@ -1,4 +1,4 @@ -import { Head, usePage } from '@inertiajs/react'; +import { Head, router, usePage } from '@inertiajs/react'; import { keys, useT } from '@simple-module-py/i18n'; import { PageShell } from '@simple-module-py/ui/components/PageShell'; import { SectionTitle } from '@simple-module-py/ui/components/SectionTitle'; @@ -7,20 +7,11 @@ import { Badge } from '@simple-module-py/ui/components/ui/badge'; import { Button } from '@simple-module-py/ui/components/ui/button'; import { Card, CardContent } from '@simple-module-py/ui/components/ui/card'; import { AdminLayout } from '@simple-module-py/ui/layouts/AdminLayout'; -import { - Activity, - AlertTriangle, - CheckCircle2, - Database, - GitBranch, - Package, - Play, - RefreshCw, - Stethoscope, - Terminal, - XCircle, -} from 'lucide-react'; -import { DEV_SERVER, ENV_VARS, MIGRATIONS, STATIC_CHECKS, TONE } from './components/doctor-data'; +import { TONE } from '@simple-module-py/ui/lib/tone'; +import { Activity, AlertTriangle, Package, RefreshCw, Stethoscope, XCircle } from 'lucide-react'; +import type React from 'react'; +import { type Diagnostic, DiagnosticsCard } from './components/DiagnosticsCard'; +import { type Migration, MigrationsCard } from './components/MigrationsCard'; interface SystemModule { name: string; @@ -32,52 +23,44 @@ interface HealthCheck { status: 'healthy' | 'degraded' | 'unhealthy'; } +interface Environment { + environment: string; + database: string; + locales: string[]; + default_locale: string; +} + interface Props { - total_users: number; - active_users_7d: number; module_count: number; system_info: { modules: SystemModule[]; python_version: string; health_checks: HealthCheck[]; }; -} - -const STATUS_VISUALS = { - pass: { Icon: CheckCircle2, color: 'text-primary-600', tone: TONE.success }, - warn: { Icon: AlertTriangle, color: 'text-amber-600', tone: TONE.warning }, - fail: { Icon: XCircle, color: 'text-red-600', tone: TONE.destructive }, -} as const; - -function CheckRow({ check }: { check: (typeof STATIC_CHECKS)[number] }) { - const { Icon, color, tone } = STATUS_VISUALS[check.status]; - return ( -
-
- ); + diagnostics: Diagnostic[]; + migration: Migration; + environment: Environment; } function Doctor() { - const { system_info, module_count } = usePage<{ props: Props }>().props as unknown as Props; + const props = usePage<{ props: Props }>().props as unknown as Props; + const { system_info, module_count, diagnostics, migration, environment } = props; const { t } = useT(); - const passed = STATIC_CHECKS.filter((c) => c.status === 'pass').length; - const pending = MIGRATIONS.filter((m) => !m.applied).length; + const errors = diagnostics.filter((d) => d.level === 'error').length; + const warnings = diagnostics.filter((d) => d.level === 'warning').length; const unhealthy = system_info.health_checks.filter((c) => c.status !== 'healthy').length; + const envRows: [string, string][] = [ + [t(keys.dashboard.doctor.env_mode), environment.environment], + [t(keys.dashboard.doctor.env_database), environment.database], + [t(keys.dashboard.doctor.env_python), system_info.python_version], + [t(keys.dashboard.doctor.env_locales), environment.locales.join(', ')], + ]; + if (migration.head_revision) { + envRows.push([t(keys.dashboard.doctor.env_revision), migration.head_revision.slice(0, 12)]); + } + return ( <> @@ -85,40 +68,31 @@ function Doctor() { title={t(keys.dashboard.doctor.title)} description={t(keys.dashboard.doctor.description)} actions={ - <> - - - + } >
+ -
- - - - {t(keys.dashboard.doctor.just_now)} - - } - > - {t(keys.dashboard.doctor.static_checks)} - -
- {STATIC_CHECKS.map((c) => ( - - ))} -
- - - - - - - - -
- } - > - {t(keys.dashboard.doctor.recent_migrations)} - -
- {MIGRATIONS.map((m) => ( -
- - {m.id} - - - {m.module} - -
{m.msg}
- {m.when} - - {m.applied - ? t(keys.dashboard.doctor.applied) - : t(keys.dashboard.doctor.pending)} - -
- ))} -
- - + + @@ -226,24 +143,18 @@ function Doctor() {
- - - {t(keys.dashboard.doctor.running)} - - } - > - {t(keys.dashboard.doctor.dev_server)} + +
- {DEV_SERVER.map(([k, v, tone]) => ( + {envRows.map(([k, v]) => (
{k} - + {v}
@@ -267,25 +178,6 @@ function Doctor() {
- - - - - -
- {ENV_VARS.map(([k, v]) => ( -
- {k} - - {v} - -
- ))} -
- -
diff --git a/modules/dashboard/dashboard/pages/components/DemoPlaceholders.tsx b/modules/dashboard/dashboard/pages/components/DemoPlaceholders.tsx index edea57fb..a588ba6b 100644 --- a/modules/dashboard/dashboard/pages/components/DemoPlaceholders.tsx +++ b/modules/dashboard/dashboard/pages/components/DemoPlaceholders.tsx @@ -6,6 +6,7 @@ import { SectionTitle } from '@simple-module-py/ui/components/SectionTitle'; import { Badge } from '@simple-module-py/ui/components/ui/badge'; import { Card, CardContent } from '@simple-module-py/ui/components/ui/card'; +import { TONE } from '@simple-module-py/ui/lib/tone'; import { Box, ChevronRight, @@ -15,7 +16,6 @@ import { ShoppingCart, Users, } from 'lucide-react'; -import { TONE } from './doctor-data'; type Tone = keyof typeof TONE; diff --git a/modules/dashboard/dashboard/pages/components/DiagnosticsCard.tsx b/modules/dashboard/dashboard/pages/components/DiagnosticsCard.tsx new file mode 100644 index 00000000..fff58e85 --- /dev/null +++ b/modules/dashboard/dashboard/pages/components/DiagnosticsCard.tsx @@ -0,0 +1,87 @@ +import { keys, useT } from '@simple-module-py/i18n'; +import { SectionTitle } from '@simple-module-py/ui/components/SectionTitle'; +import { Badge } from '@simple-module-py/ui/components/ui/badge'; +import { Card, CardContent } from '@simple-module-py/ui/components/ui/card'; +import { TONE } from '@simple-module-py/ui/lib/tone'; +import { AlertTriangle, CheckCircle2, Info, XCircle } from 'lucide-react'; + +export interface Diagnostic { + level: 'error' | 'warning' | 'info'; + code: string; + message: string; + module: string; + file: string | null; + suggestion: string | null; +} + +const LEVEL_VISUALS = { + error: { Icon: XCircle, color: 'text-red-600', tone: TONE.destructive }, + warning: { Icon: AlertTriangle, color: 'text-amber-600', tone: TONE.warning }, + info: { Icon: Info, color: 'text-muted-foreground', tone: TONE.default }, +} as const; + +function DiagnosticRow({ d, suggestionLabel }: { d: Diagnostic; suggestionLabel: string }) { + const { Icon, color, tone } = LEVEL_VISUALS[d.level]; + return ( +
+
+ ); +} + +export function DiagnosticsCard({ diagnostics }: { diagnostics: Diagnostic[] }) { + const { t } = useT(); + return ( + + + {t(keys.dashboard.doctor.diagnostics)} + {diagnostics.length === 0 ? ( +
+
+ ) : ( +
+ {diagnostics.map((d) => ( + + ))} +
+ )} + + + ); +} diff --git a/modules/dashboard/dashboard/pages/components/MigrationsCard.tsx b/modules/dashboard/dashboard/pages/components/MigrationsCard.tsx new file mode 100644 index 00000000..c9339a7d --- /dev/null +++ b/modules/dashboard/dashboard/pages/components/MigrationsCard.tsx @@ -0,0 +1,55 @@ +import { keys, useT } from '@simple-module-py/i18n'; +import { SectionTitle } from '@simple-module-py/ui/components/SectionTitle'; +import { Badge } from '@simple-module-py/ui/components/ui/badge'; +import { Card, CardContent } from '@simple-module-py/ui/components/ui/card'; +import { TONE } from '@simple-module-py/ui/lib/tone'; + +export interface Migration { + current_revision: string | null; + head_revision: string | null; + is_current: boolean; + recent: { revision: string; message: string; modules: string[] }[]; +} + +export function MigrationsCard({ migration }: { migration: Migration }) { + const { t } = useT(); + if (migration.recent.length === 0) return null; + return ( + + + + {migration.is_current + ? t(keys.dashboard.doctor.at_head) + : t(keys.dashboard.doctor.behind)} + + } + > + {t(keys.dashboard.doctor.recent_migrations)} + +
+ {migration.recent.map((m) => ( +
+ + {m.revision} + + {m.modules.map((mod) => ( + + {mod} + + ))} +
{m.message}
+ + {t(keys.dashboard.doctor.applied)} + +
+ ))} +
+ + + ); +} diff --git a/modules/dashboard/dashboard/pages/components/doctor-data.ts b/modules/dashboard/dashboard/pages/components/doctor-data.ts deleted file mode 100644 index 4ad83348..00000000 --- a/modules/dashboard/dashboard/pages/components/doctor-data.ts +++ /dev/null @@ -1,68 +0,0 @@ -export const STATIC_CHECKS = [ - { name: 'Module imports', status: 'pass' as const, hint: 'All ModuleBase subclasses load.' }, - { name: 'Migration drift', status: 'pass' as const, hint: 'Alembic head matches DB.' }, - { name: 'Orphan pages', status: 'pass' as const, hint: 'Every Inertia page has a route.' }, - { - name: 'Permission registry', - status: 'pass' as const, - hint: 'All declared perms reachable from a role.', - }, - { - name: 'Coupling check', - status: 'warn' as const, - hint: 'Cross-module imports detected — modules should depend via the registry.', - file: 'modules/billing/router.py:14', - }, - { - name: 'Schema isolation', - status: 'pass' as const, - hint: 'No cross-schema foreign keys on Postgres.', - }, -]; - -export const MIGRATIONS = [ - { - id: '0024', - module: 'billing', - msg: 'create subscriptions table', - when: 'just now', - applied: false, - }, - { - id: '0023', - module: 'orders', - msg: 'add fulfilled_at column', - when: '3h ago', - applied: true, - }, - { - id: '0022', - module: 'users', - msg: 'add invited_by foreign key', - when: '1d ago', - applied: true, - }, - { - id: '0021', - module: 'audit', - msg: 'partition events by month', - when: '2d ago', - applied: true, - }, -]; - -export const ENV_VARS: [string, string][] = [ - ['SM_ENVIRONMENT', 'development'], - ['SM_DATABASE_URL', 'sqlite+aiosqlite'], - ['SM_USERS_MAILER', 'console'], - ['SM_USERS_ALLOW_SIGNUP', 'false'], -]; - -export const DEV_SERVER: [string, string, 'success' | 'default'][] = [ - ['FastAPI', ':8000', 'success'], - ['Vite HMR', ':5173', 'success'], - ['Postgres', ':5432', 'success'], - ['Worker', 'idle', 'default'], -]; - -export { TONE } from '@simple-module-py/ui/lib/tone'; diff --git a/modules/dashboard/tests/test_view_routes.py b/modules/dashboard/tests/test_view_routes.py index fd8c979c..969ab53a 100644 --- a/modules/dashboard/tests/test_view_routes.py +++ b/modules/dashboard/tests/test_view_routes.py @@ -94,3 +94,32 @@ async def test_dashboard_index_redirects_anon_to_login(client): resp = await client.get("/dashboard/", follow_redirects=False) assert resp.status_code == 302 assert "/users/login" in resp.headers["location"] + + +@pytest.mark.anyio +async def test_doctor_reports_real_diagnostics(authenticated_client): + """The doctor page ships live diagnostics, migration state and env facts. + + Guards against the panel regressing to hardcoded demo data: the values + asserted here can only come from the running app. + """ + resp = await authenticated_client.get( + "/admin/doctor/", + headers={"X-Inertia": "true", "X-Inertia-Version": "1.0"}, + ) + assert resp.status_code == 200, resp.text + props = resp.json()["props"] + + assert isinstance(props["diagnostics"], list) + for finding in props["diagnostics"]: + assert finding["code"].startswith("SM") + assert finding["level"] in {"error", "warning", "info"} + + migration = props["migration"] + # The test fixtures stamp alembic at head, so the page must agree. + assert migration["is_current"] is True + assert migration["current_revision"] == migration["head_revision"] + + env = props["environment"] + assert env["database"] == "sqlite" + assert env["default_locale"] in env["locales"] diff --git a/modules/feature_flags/feature_flags/module.py b/modules/feature_flags/feature_flags/module.py index ae3a85dd..ffe5d3c5 100644 --- a/modules/feature_flags/feature_flags/module.py +++ b/modules/feature_flags/feature_flags/module.py @@ -7,7 +7,7 @@ from pathlib import Path from fastapi import APIRouter, FastAPI -from simple_module_core.menu import MenuItem, MenuRegistry +from simple_module_core.menu import MenuItem, MenuRegistry, MenuSection from simple_module_core.module import ModuleBase, ModuleMeta from simple_module_core.permissions import PermissionRegistry @@ -48,6 +48,7 @@ def register_menu_items(self, registry: MenuRegistry) -> None: url=MENU_URL, icon=MENU_ICON, order=MENU_ORDER, + section=MenuSection.ADMIN_SIDEBAR, group="System", group_key="ui.nav_groups.system", # Mirrors the view router's guard. Without it the entry shows diff --git a/modules/feature_flags/feature_flags/pages/Browse.tsx b/modules/feature_flags/feature_flags/pages/Browse.tsx index 7fb23eb3..20e00045 100644 --- a/modules/feature_flags/feature_flags/pages/Browse.tsx +++ b/modules/feature_flags/feature_flags/pages/Browse.tsx @@ -105,22 +105,23 @@ function Browse() { title={t(keys.feature_flags.browse.title)} description={t(keys.feature_flags.browse.description)} > - - router.visit(buildPath(next))} - /> -

- {tenant_id - ? t(keys.feature_flags.browse.viewing_tenant, { tenant_id }) - : t(keys.feature_flags.browse.viewing_system)} -

- - -
+ {/* One toolbar row instead of a near-empty card: picker, its hint, + and the flag count share the line the table sits under. */} +
+
+ router.visit(buildPath(next))} + /> +

+ {tenant_id + ? t(keys.feature_flags.browse.viewing_tenant, { tenant_id }) + : t(keys.feature_flags.browse.viewing_system)} +

+
{flags.length > 0 && ( -

+

{t(keys.feature_flags.browse.count, { count: flags.length })}

)} diff --git a/modules/keycloak/keycloak/settings.py b/modules/keycloak/keycloak/settings.py index e9e9745c..b9f2b286 100644 --- a/modules/keycloak/keycloak/settings.py +++ b/modules/keycloak/keycloak/settings.py @@ -2,10 +2,13 @@ from __future__ import annotations -from pydantic import Field, model_validator +from pydantic import Field, field_validator, model_validator from pydantic_settings import BaseSettings, SettingsConfigDict from simple_module_core.dotenv import env_str from simple_module_core.environments import NON_PROD_ENVIRONMENTS +from simple_module_core.redirect_safety import non_empty_redirect + +DEFAULT_LOGIN_REDIRECT_URL = "/dashboard/" class KeycloakSettings(BaseSettings): @@ -20,13 +23,25 @@ class KeycloakSettings(BaseSettings): roles_claim_path: str = "realm_access.roles" admin_role: str = "admin" - login_redirect_url: str = "/dashboard/" + login_redirect_url: str = DEFAULT_LOGIN_REDIRECT_URL jwks_cache_ttl_seconds: int = 3600 role_mapping: dict[str, str] = Field( default_factory=lambda: {"admin": "admin", "user": "user"}, ) + @field_validator("login_redirect_url") + @classmethod + def _non_empty_redirect(cls, value: str) -> str: + """Blank is never a usable navigation target. + + The callback puts this straight into a ``Location`` header, and an + admin can clear it in the generic module-settings editor. Normalising + on the class covers hydration and ``apply_changes_and_reload`` alike. + Users' provider has its own copy of this field and does the same. + """ + return non_empty_redirect(value, default=DEFAULT_LOGIN_REDIRECT_URL) + @model_validator(mode="after") def _check_required_in_production(self) -> KeycloakSettings: import os diff --git a/modules/keycloak/tests/test_keycloak_module.py b/modules/keycloak/tests/test_keycloak_module.py index 7b073d11..2b5164e2 100644 --- a/modules/keycloak/tests/test_keycloak_module.py +++ b/modules/keycloak/tests/test_keycloak_module.py @@ -20,3 +20,28 @@ def test_keycloak_provider_satisfies_protocol(): provider = KeycloakAuthProvider() assert isinstance(provider, AuthProvider) assert provider.name == "keycloak" + + +class TestKeycloakLoginRedirectUrl: + """A blanked ``login_redirect_url`` must never reach the OIDC callback. + + ``endpoints/api.py`` puts this value straight into a ``Location`` header, + and an admin can clear it in the generic module-settings editor. This is a + second copy of the field — ``users`` has its own — so it needs its own + guard; the two provider modules must not import each other. + """ + + def test_blank_falls_back_to_the_default(self): + from keycloak.settings import KeycloakSettings + + assert KeycloakSettings(login_redirect_url="").login_redirect_url == "/dashboard/" + + def test_whitespace_only_falls_back_to_the_default(self): + from keycloak.settings import KeycloakSettings + + assert KeycloakSettings(login_redirect_url=" ").login_redirect_url == "/dashboard/" + + def test_a_real_value_is_left_alone(self): + from keycloak.settings import KeycloakSettings + + assert KeycloakSettings(login_redirect_url="/home/").login_redirect_url == "/home/" diff --git a/modules/keycloak/tsconfig.json b/modules/keycloak/tsconfig.json index d479e6d7..fb09977b 100644 --- a/modules/keycloak/tsconfig.json +++ b/modules/keycloak/tsconfig.json @@ -1,4 +1,9 @@ { "extends": "../../host/client_app/tsconfig.json", - "include": ["keycloak/**/*.ts", "keycloak/**/*.tsx"] + "include": ["keycloak/**/*.ts", "keycloak/**/*.tsx"], + // The parent's "exclude" is a relative glob resolved against the config file + // it was declared in (host/client_app), so it does not reach files under + // this module — override it here so a future keycloak/**/*.test.tsx is + // excluded from tsc the same way host/client_app's specs are. + "exclude": ["keycloak/**/*.test.ts", "keycloak/**/*.test.tsx"] } diff --git a/modules/settings/settings/_module_settings.py b/modules/settings/settings/_module_settings.py index 01e936be..54524f41 100644 --- a/modules/settings/settings/_module_settings.py +++ b/modules/settings/settings/_module_settings.py @@ -12,7 +12,6 @@ from typing import Any from fastapi import FastAPI -from fastapi.encoders import jsonable_encoder from pydantic_settings import BaseSettings from settings.env_vars import env_prefix_for @@ -27,6 +26,10 @@ ) SECRET_MASK = "••••••••" +# Value types that cannot carry credential material, so a secret-ish *name* +# on one of them is a false positive rather than something to hide. +_NEVER_SECRET_TYPES = frozenset({"int", "float", "bool"}) + def is_secret_field(name: str) -> bool: """True if a field name suggests it holds credential material.""" @@ -84,6 +87,9 @@ class ModuleSettingsView: env_prefix: str class_name: str fields: list[ModuleSettingField] + manage_url: str | None = None + """The module's own management page. When set, the generic editor renders + a link there instead of a second editor for the same fields.""" def _mask(value: Any) -> Any: @@ -156,7 +162,15 @@ def _field_view( cls = type(settings) info = cls.model_fields[name] raw_value = getattr(settings, name) - secret = is_secret_field(name) + value_type = value_type_for_field(cls, name) + # A numeric field whose name merely contains a secret-ish word was being + # masked and made uneditable — `reset_password_token_lifetime_seconds` is + # an int, but it matches on "password" exactly as the real secrets do. + # Phrased as "exempt the types that cannot hold a credential" rather than + # "mask only strings" so the failure direction is safe: an unexpected type + # (e.g. `str | None`, which resolves to "json") stays masked instead of + # silently exposing a secret. + secret = value_type not in _NEVER_SECRET_TYPES and is_secret_field(name) extra = info.json_schema_extra if isinstance(info.json_schema_extra, dict) else {} default = _resolve_default(info) env_var = f"{prefix}{name.upper()}" @@ -168,7 +182,7 @@ def _field_view( default=_mask(default) if secret else default, description=info.description or "", is_secret=secret, - type=value_type_for_field(cls, name), + type=value_type, requires_restart=bool(extra.get("requires_restart", False)), group=extra.get("group"), env_set=live_env_var is not None and live_env_var in os.environ, @@ -194,16 +208,22 @@ def collect_module_settings( views: list[ModuleSettingsView] = [] seen: set[str] = set() + settings_services = getattr(app.state, "settings", None) + registry = getattr(settings_services, "module_registry", None) + + def _manage_url(package: str) -> str | None: + return registry.manage_url(package) if registry is not None else None + for mod in getattr(app.state.sm, "modules", ()): package = _package_of(mod) settings = _extract_settings(app, package) if settings is None: continue - views.append(_build_view(mod.meta.name, package, settings, by_package)) + views.append( + _build_view(mod.meta.name, package, settings, by_package, _manage_url(package)) + ) seen.add(package) - settings_services = getattr(app.state, "settings", None) - registry = getattr(settings_services, "module_registry", None) if registry is not None: for package in registry.all_packages(): if package in seen: @@ -211,7 +231,9 @@ def collect_module_settings( settings = _extract_settings(app, package) if settings is None: continue - views.append(_build_view(package.title(), package, settings, by_package)) + views.append( + _build_view(package.title(), package, settings, by_package, _manage_url(package)) + ) seen.add(package) views.sort(key=lambda v: v.module_name) @@ -223,6 +245,7 @@ def _build_view( package: str, settings: BaseSettings, overrides: dict[str, frozenset[str]] | None = None, + manage_url: str | None = None, ) -> ModuleSettingsView: prefix = env_prefix_for(package) overridden = (overrides or {}).get(package, frozenset()) @@ -235,6 +258,7 @@ def _build_view( env_prefix=prefix, class_name=type(settings).__name__, fields=fields, + manage_url=manage_url, ) @@ -249,41 +273,3 @@ async def overrides_by_package(service: SettingService) -> dict[str, frozenset[s from settings.store import SettingsStore return await SettingsStore(service).all_override_fields() - - -def serialize(views: list[ModuleSettingsView]) -> list[dict[str, Any]]: - """Convert dataclass views to plain dicts for Inertia props. - - Field values arrive as whatever type the module declared — pydantic has - already coerced ``media_root: Path`` to a ``PosixPath``, ``timeout: - timedelta`` to a ``timedelta`` — and this screen reflects every installed - module's settings, so the set of types is open-ended by design. They are - encoded here rather than handed on as-is: this is the boundary where a - settings object stops being Python and becomes a prop. - """ - return [ - { - "module_name": v.module_name, - "package": v.package, - "env_prefix": v.env_prefix, - "class_name": v.class_name, - "fields": [ - { - "name": f.name, - "env_var": f.env_var, - "value": jsonable_encoder(f.value), - "default": jsonable_encoder(f.default), - "description": f.description, - "is_secret": f.is_secret, - "type": f.type, - "requires_restart": f.requires_restart, - "group": f.group, - "env_set": f.env_set, - "db_override": f.db_override, - "source": f.source, - } - for f in v.fields - ], - } - for v in views - ] diff --git a/modules/settings/settings/_module_settings_props.py b/modules/settings/settings/_module_settings_props.py new file mode 100644 index 00000000..8e626d82 --- /dev/null +++ b/modules/settings/settings/_module_settings_props.py @@ -0,0 +1,52 @@ +"""Serialize module-settings views into Inertia props. + +Split from ``_module_settings`` (collection) so each file keeps one +responsibility: that one discovers and shapes the views, this one is the +boundary where a settings object stops being Python and becomes a prop. +""" + +from __future__ import annotations + +from typing import Any + +from fastapi.encoders import jsonable_encoder + +from settings._module_settings import ModuleSettingsView + + +def serialize(views: list[ModuleSettingsView]) -> list[dict[str, Any]]: + """Convert dataclass views to plain dicts for Inertia props. + + Field values arrive as whatever type the module declared — pydantic has + already coerced ``media_root: Path`` to a ``PosixPath``, ``timeout: + timedelta`` to a ``timedelta`` — and this screen reflects every installed + module's settings, so the set of types is open-ended by design. They are + encoded here rather than handed on as-is. + """ + return [ + { + "module_name": v.module_name, + "package": v.package, + "env_prefix": v.env_prefix, + "class_name": v.class_name, + "manage_url": v.manage_url, + "fields": [ + { + "name": f.name, + "env_var": f.env_var, + "value": jsonable_encoder(f.value), + "default": jsonable_encoder(f.default), + "description": f.description, + "is_secret": f.is_secret, + "type": f.type, + "requires_restart": f.requires_restart, + "group": f.group, + "env_set": f.env_set, + "db_override": f.db_override, + "source": f.source, + } + for f in v.fields + ], + } + for v in views + ] diff --git a/modules/settings/settings/endpoints/module_api.py b/modules/settings/settings/endpoints/module_api.py index adab6709..7e70d7a4 100644 --- a/modules/settings/settings/endpoints/module_api.py +++ b/modules/settings/settings/endpoints/module_api.py @@ -18,8 +18,8 @@ collect_module_settings, is_secret_field, overrides_by_package, - serialize, ) +from settings._module_settings_props import serialize from settings.constants import MODULE_PACKAGE, PERM_DELETE, PERM_EDIT, PERM_VIEW from settings.contracts.events import SettingsReloaded from settings.deps import get_setting_service @@ -70,6 +70,15 @@ async def update_module( registry = getattr(request.app.state, MODULE_PACKAGE).module_registry if registry.get(package) is None: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Unknown module package") + if registry.manage_url(package) is not None: + # Modules with a purpose-built settings screen (e.g. Branding) own + # their own validation/preview flow — the generic editor only ever + # links out to it (ModulesEdit.tsx), it must not also accept writes + # that bypass that flow. + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="This module has a dedicated settings page; edit it there instead.", + ) cleaned = _strip_mask_sentinels(changes) if not cleaned: @@ -102,6 +111,11 @@ async def clear_module_field( cls = registry.get(package) if cls is None: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Unknown module package") + if registry.manage_url(package) is not None: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="This module has a dedicated settings page; edit it there instead.", + ) if field not in cls.model_fields: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Unknown field") diff --git a/modules/settings/settings/endpoints/views.py b/modules/settings/settings/endpoints/views.py index 3509b078..bc3abe85 100644 --- a/modules/settings/settings/endpoints/views.py +++ b/modules/settings/settings/endpoints/views.py @@ -23,8 +23,8 @@ _package_of, collect_module_settings, overrides_by_package, - serialize, ) +from settings._module_settings_props import serialize from settings.constants import ( ERR_SETTING_NOT_FOUND, PERM_CREATE, diff --git a/modules/settings/settings/locales/en.json b/modules/settings/settings/locales/en.json index 13562939..542c212d 100644 --- a/modules/settings/settings/locales/en.json +++ b/modules/settings/settings/locales/en.json @@ -83,7 +83,10 @@ "source_env": "From environment", "source_env_hint": "{env_var} is set in this deployment", "source_default": "Default", - "head_title": "Modules" + "head_title": "Modules", + "managed_title": "Managed on its own page", + "managed_description": "{module} ships a purpose-built settings page; edit it there so validation and previews apply.", + "managed_open": "Open {module} settings" }, "modules_form": { "save": "Save", diff --git a/modules/settings/settings/module_registry.py b/modules/settings/settings/module_registry.py index bd28c7e8..4a80147b 100644 --- a/modules/settings/settings/module_registry.py +++ b/modules/settings/settings/module_registry.py @@ -17,11 +17,28 @@ class ModuleSettingsRegistry: """In-memory map of ``package`` → ``BaseSettings`` subclass.""" _classes: dict[str, type[BaseSettings]] = field(default_factory=dict) - - def register(self, package: str, cls: type[BaseSettings]) -> None: + _manage_urls: dict[str, str] = field(default_factory=dict) + + def register( + self, + package: str, + cls: type[BaseSettings], + manage_url: str | None = None, + ) -> None: if package in self._classes: raise ValueError(f"{package!r} already registered") self._classes[package] = cls + if manage_url: + self._manage_urls[package] = manage_url + + def manage_url(self, package: str) -> str | None: + """URL of the module's own management page, if it declared one. + + Modules with a purpose-built settings screen (e.g. Branding) declare it + so the generic module-settings editor links there instead of offering a + second, raw editor for the same fields. + """ + return self._manage_urls.get(package) def get(self, package: str) -> type[BaseSettings] | None: return self._classes.get(package) diff --git a/modules/settings/settings/pages/ModulesEdit.tsx b/modules/settings/settings/pages/ModulesEdit.tsx index f76cfeae..f5eec358 100644 --- a/modules/settings/settings/pages/ModulesEdit.tsx +++ b/modules/settings/settings/pages/ModulesEdit.tsx @@ -1,9 +1,10 @@ -import { Head } from '@inertiajs/react'; +import { Head, Link } from '@inertiajs/react'; import { keys, useT } from '@simple-module-py/i18n'; +import { Button } from '@simple-module-py/ui/components/ui/button'; import { Card } from '@simple-module-py/ui/components/ui/card'; import { Input } from '@simple-module-py/ui/components/ui/input'; import { AdminLayout } from '@simple-module-py/ui/layouts/AdminLayout'; -import { Box, Search } from 'lucide-react'; +import { ArrowRight, Box, Search } from 'lucide-react'; import type React from 'react'; import { useMemo, useState } from 'react'; import { ModuleForm, type ModuleView } from './components/ModuleForm'; @@ -96,7 +97,29 @@ function ModulesEdit({ modules, testable = [] }: Props) {
- {current ? ( + {current?.manage_url ? ( + + {/* No second editor for these fields — the module's own page is + the one place they're edited. */} +
+ + +

+ {t(keys.settings.modules.managed_title)} +

+

+ {t(keys.settings.modules.managed_description, { module: current.module_name })} +

+ +
+ + ) : current ? ( diff --git a/modules/settings/settings/pages/components/ModuleForm.tsx b/modules/settings/settings/pages/components/ModuleForm.tsx index 50122046..6b28ee1f 100644 --- a/modules/settings/settings/pages/components/ModuleForm.tsx +++ b/modules/settings/settings/pages/components/ModuleForm.tsx @@ -11,6 +11,8 @@ export type ModuleView = { env_prefix: string; class_name: string; fields: FieldMeta[]; + /** The module's own management page; when set, the generic editor links there. */ + manage_url?: string | null; }; type Props = { diff --git a/modules/settings/settings/registration.py b/modules/settings/settings/registration.py index 2ab3196f..3456ead1 100644 --- a/modules/settings/settings/registration.py +++ b/modules/settings/settings/registration.py @@ -26,9 +26,15 @@ def register_module_settings( package: str, settings_cls: type[BaseSettings], services_factory: Callable[[BaseSettings], Any], + manage_url: str | None = None, ) -> None: - """Register a module's BaseSettings class and mount its services on app.state.""" + """Register a module's BaseSettings class and mount its services on app.state. + + ``manage_url`` points at the module's own management page when it has one; + the generic module-settings editor then links there instead of rendering a + second editor for the same fields. + """ registry = getattr(app.state, MODULE_PACKAGE).module_registry - registry.register(package, settings_cls) + registry.register(package, settings_cls, manage_url=manage_url) defaults = settings_cls() setattr(app.state, package, services_factory(defaults)) diff --git a/modules/settings/tests/test_module_settings_render.py b/modules/settings/tests/test_module_settings_render.py index 0dcbf3ca..5b1edcf1 100644 --- a/modules/settings/tests/test_module_settings_render.py +++ b/modules/settings/tests/test_module_settings_render.py @@ -88,3 +88,55 @@ async def test_full_page_load_still_works( assert resp.status_code == _OK assert resp.headers["content-type"].startswith("text/html") + + async def test_dedicated_page_modules_link_instead_of_double_editing( + self, + app_with_path_setting: FastAPI, + authenticated_client: httpx.AsyncClient, + ) -> None: + """Branding declares its own page; generic modules don't. + + The editor uses ``manage_url`` to link there instead of rendering a + second editor for the same fields. + """ + resp = await authenticated_client.get("/admin/settings/", headers=_INERTIA) + modules = resp.json()["props"]["modules"] + + branding = next(m for m in modules if m["package"] == "branding") + assert branding["manage_url"] == "/admin/branding/" + + demo = next(m for m in modules if m["package"] == "pathdemo") + assert demo["manage_url"] is None + + +class TestManageUrlModulesRejectGenericWrites: + """The generic PUT/DELETE endpoints must not double-edit a module that + declares its own settings page (``manage_url``) — the UI already routes + around it (see ``test_dedicated_page_modules_link_instead_of_double_editing`` + above), and the JSON API must enforce the same invariant server-side.""" + + async def test_put_is_rejected_for_a_manage_url_module( + self, + app_with_path_setting: FastAPI, + authenticated_client: httpx.AsyncClient, + ) -> None: + resp = await authenticated_client.put( + "/api/settings/modules/branding", json={"app_name": "Hijacked"} + ) + assert resp.status_code == 409 + + async def test_delete_is_rejected_for_a_manage_url_module( + self, + app_with_path_setting: FastAPI, + authenticated_client: httpx.AsyncClient, + ) -> None: + resp = await authenticated_client.delete("/api/settings/modules/branding/app_name") + assert resp.status_code == 409 + + async def test_put_still_works_for_a_module_without_manage_url( + self, + app_with_path_setting: FastAPI, + authenticated_client: httpx.AsyncClient, + ) -> None: + resp = await authenticated_client.put("/api/settings/modules/pathdemo", json={"workers": 5}) + assert resp.status_code == 200 diff --git a/modules/settings/tests/test_module_settings_serialize.py b/modules/settings/tests/test_module_settings_serialize.py index cd3cf69e..9f50cedd 100644 --- a/modules/settings/tests/test_module_settings_serialize.py +++ b/modules/settings/tests/test_module_settings_serialize.py @@ -20,8 +20,8 @@ from settings._module_settings import ( ModuleSettingField, ModuleSettingsView, - serialize, ) +from settings._module_settings_props import serialize def _view_with(value: Any, default: Any = "") -> ModuleSettingsView: diff --git a/modules/settings/tests/test_settings_field_sources.py b/modules/settings/tests/test_settings_field_sources.py index e4888fcd..f3e49856 100644 --- a/modules/settings/tests/test_settings_field_sources.py +++ b/modules/settings/tests/test_settings_field_sources.py @@ -120,3 +120,54 @@ async def test_failing_check_still_returns_200_with_the_reason(self, authenticat for check in body["checks"]: assert check["status"] in ("healthy", "degraded", "unhealthy") assert "detail" in check + + +class TestSecretMaskingIsTypeAware: + """Only string fields can hold credential material. + + The name-based pattern deliberately avoids the bare words "token" and + "key", but it cannot avoid "password" — and + ``reset_password_token_lifetime_seconds`` is an int that contains it. + Masking it made a plain duration uneditable in the admin UI, so the + declared type gates the match. + """ + + def _field(self, cls, name: str): + from settings._module_settings import _field_view + + return _field_view(name, cls(), "SM_USERS_", frozenset()) + + def test_an_int_named_like_a_secret_is_not_masked(self): + from users.settings import UsersSettings + + field = self._field(UsersSettings, "reset_password_token_lifetime_seconds") + assert field.is_secret is False + assert field.type == "int" + assert isinstance(field.value, int) + + def test_a_real_string_secret_is_still_masked(self): + from settings._module_settings import SECRET_MASK + from users.settings import UsersSettings + + field = self._field(UsersSettings, "reset_password_token_secret") + assert field.is_secret is True + assert field.value == SECRET_MASK + + def test_an_optional_string_secret_stays_masked(self): + """The gate must fail safe on a type it doesn't recognise. + + ``value_type_for_field`` reports "json" for any union, so a secret + declared ``str | None`` is not the "string" case. Exempting only the + types that cannot hold a credential keeps it masked; masking only + "string" would have silently exposed it. + """ + from pydantic_settings import BaseSettings + from settings._module_settings import SECRET_MASK, _field_view + + class _OptionalSecret(BaseSettings): + smtp_password: str | None = "hunter2" + + field = _field_view("smtp_password", _OptionalSecret(), "SM_X_", frozenset()) + assert field.type == "json" + assert field.is_secret is True + assert field.value == SECRET_MASK diff --git a/modules/users/tests/test_settings.py b/modules/users/tests/test_settings.py index bc628197..31151710 100644 --- a/modules/users/tests/test_settings.py +++ b/modules/users/tests/test_settings.py @@ -165,3 +165,30 @@ def test_real_secrets_accepted_in_production(self, monkeypatch): ) assert s.reset_password_token_secret == "real-reset-secret" assert s.verification_token_secret == "real-verify-secret" + + +class TestLoginRedirectUrl: + """A blanked ``login_redirect_url`` must never reach a consumer. + + Nothing stops an admin clearing it in the generic module-settings editor, + and every consumer treats it as a destination — the login view hands it to + Inertia (``router.visit("")`` reloads the current page), while the Keycloak + and OAuth callbacks put it directly into a ``Location`` header. Normalising + on the settings class covers all three, since hydration and + ``apply_changes_and_reload`` both reconstruct through it. + """ + + def test_blank_falls_back_to_the_default(self): + from users.settings import UsersSettings + + assert UsersSettings(login_redirect_url="").login_redirect_url == "/dashboard/" + + def test_whitespace_only_falls_back_to_the_default(self): + from users.settings import UsersSettings + + assert UsersSettings(login_redirect_url=" ").login_redirect_url == "/dashboard/" + + def test_a_real_value_is_left_alone(self): + from users.settings import UsersSettings + + assert UsersSettings(login_redirect_url="/home/").login_redirect_url == "/home/" diff --git a/modules/users/tests/test_views.py b/modules/users/tests/test_views.py index 8cb4bdec..1e2cd78b 100644 --- a/modules/users/tests/test_views.py +++ b/modules/users/tests/test_views.py @@ -121,6 +121,26 @@ async def test_login_redirect_url_is_dashboard_when_installed(self, anon_client) ) assert resp.json()["props"]["login_redirect_url"] == "/dashboard/" + @pytest.mark.anyio + async def test_login_redirect_url_falls_back_when_setting_is_blanked(self, anon_client): + """An admin can blank the DB-backed setting via the generic module + editor — no consumer may then receive "". + + Normalisation lives on the settings class (unit-tested in + test_settings.py), so it applies wherever the value is constructed. + Assigning the attribute here would bypass pydantic and test nothing, + so this goes through the real path. + """ + from users.settings import UsersSettings + + app = anon_client._transport.app + app.state.users.settings = UsersSettings(login_redirect_url="") + resp = await anon_client.get( + "/users/login", + headers={"X-Inertia": "true", "X-Inertia-Version": "1.0"}, + ) + assert resp.json()["props"]["login_redirect_url"] == "/dashboard/" + class TestRegisterPage: @pytest.mark.anyio diff --git a/modules/users/users/auth_local/views.py b/modules/users/users/auth_local/views.py index 90ea19b4..ca3956df 100644 --- a/modules/users/users/auth_local/views.py +++ b/modules/users/users/auth_local/views.py @@ -62,6 +62,9 @@ async def login_page(request: Request, inertia: InertiaDep) -> InertiaResponse: # handler clears it once login actually succeeds. "login_redirect_url": ( safe_next_or_none(request.session.get(SESSION_NEXT_KEY)) + # Never "" — UsersSettings normalises a blanked value back to + # the default, so every consumer (here, Keycloak, OAuth) gets + # a usable target rather than each guarding for itself. or users_settings.login_redirect_url ), "oauth_providers": users_state.oauth_providers, diff --git a/modules/users/users/settings.py b/modules/users/users/settings.py index 83143b73..22ba66f9 100644 --- a/modules/users/users/settings.py +++ b/modules/users/users/settings.py @@ -12,14 +12,17 @@ import os -from pydantic import Field, model_validator +from pydantic import Field, field_validator, model_validator from pydantic_settings import BaseSettings, SettingsConfigDict from simple_module_core.dotenv import env_str from simple_module_core.environments import NON_PROD_ENVIRONMENTS +from simple_module_core.redirect_safety import non_empty_redirect _PLACEHOLDER_RESET_SECRET = "dev-reset-token-secret-change-me" _PLACEHOLDER_VERIFY_SECRET = "dev-verify-token-secret-change-me" +DEFAULT_LOGIN_REDIRECT_URL = "/dashboard/" + class UsersSettings(BaseSettings): """Local user management configuration.""" @@ -33,7 +36,20 @@ class UsersSettings(BaseSettings): # Where the login page sends a successful sign-in. Sites without the # bundled ``dashboard`` module (``smpy new --preset minimal``) override # this to wherever their post-login landing lives. - login_redirect_url: str = "/dashboard/" + login_redirect_url: str = DEFAULT_LOGIN_REDIRECT_URL + + @field_validator("login_redirect_url") + @classmethod + def _non_empty_redirect(cls, value: str) -> str: + """Blank is never a usable navigation target. + + Covers this module's consumers — the login view (which hands it to + Inertia, where ``router.visit("")`` silently reloads the current page) + and the generic OAuth callback (which puts it in a ``Location`` + header). Keycloak has its own settings class with its own copy of this + field and normalises it the same way. + """ + return non_empty_redirect(value, default=DEFAULT_LOGIN_REDIRECT_URL) # Token secrets — MUST be set in production. Dev default is a deterministic # placeholder that's obvious in logs so it can't be mistaken for a real key. diff --git a/packages/i18n/src/generated-resources.ts b/packages/i18n/src/generated-resources.ts index db614cfd..48586959 100644 --- a/packages/i18n/src/generated-resources.ts +++ b/packages/i18n/src/generated-resources.ts @@ -17,6 +17,8 @@ export default { 'audit_log.browse.showing': '', 'audit_log.browse.title': '', 'audit_log.changes.fields_set': '', + 'audit_log.changes.fields_set_one': '', + 'audit_log.changes.fields_set_other': '', 'audit_log.changes.no_changes': '', 'audit_log.changes.show_less': '', 'audit_log.changes.show_more': '', @@ -184,28 +186,31 @@ export default { 'branding.nav.branding': '', 'dashboard.doctor.active': '', 'dashboard.doctor.alert': '', + 'dashboard.doctor.all_clear_hint': '', + 'dashboard.doctor.all_clear_title': '', 'dashboard.doctor.applied': '', - 'dashboard.doctor.apply': '', - 'dashboard.doctor.clean': '', + 'dashboard.doctor.at_head': '', + 'dashboard.doctor.behind': '', 'dashboard.doctor.description': '', - 'dashboard.doctor.dev_server': '', + 'dashboard.doctor.diagnostics': '', + 'dashboard.doctor.env_database': '', + 'dashboard.doctor.env_locales': '', + 'dashboard.doctor.env_mode': '', + 'dashboard.doctor.env_python': '', + 'dashboard.doctor.env_revision': '', 'dashboard.doctor.environment': '', - 'dashboard.doctor.generate': '', 'dashboard.doctor.installed_modules': '', - 'dashboard.doctor.just_now': '', 'dashboard.doctor.loaded': '', 'dashboard.doctor.ok': '', - 'dashboard.doctor.pending': '', 'dashboard.doctor.recent_migrations': '', 'dashboard.doctor.rerun': '', 'dashboard.doctor.review': '', 'dashboard.doctor.run_command': '', - 'dashboard.doctor.running': '', - 'dashboard.doctor.stat_checks_passed': '', + 'dashboard.doctor.stat_errors': '', 'dashboard.doctor.stat_health': '', 'dashboard.doctor.stat_modules': '', - 'dashboard.doctor.stat_pending_migrations': '', - 'dashboard.doctor.static_checks': '', + 'dashboard.doctor.stat_warnings': '', + 'dashboard.doctor.suggestion': '', 'dashboard.doctor.title': '', 'dashboard.home.description': '', 'dashboard.home.description_body': '', @@ -485,6 +490,9 @@ export default { 'settings.modules.env_var_hint': '', 'settings.modules.field_count_suffix': '', 'settings.modules.head_title': '', + 'settings.modules.managed_description': '', + 'settings.modules.managed_open': '', + 'settings.modules.managed_title': '', 'settings.modules.no_fields': '', 'settings.modules.search_placeholder': '', 'settings.modules.secret_badge': '', @@ -539,8 +547,10 @@ export default { 'ui.errors.reload_button': '', 'ui.nav.admin': '', 'ui.nav_groups.access': '', + 'ui.nav_groups.account': '', 'ui.nav_groups.appearance': '', 'ui.nav_groups.content': '', + 'ui.nav_groups.navigation': '', 'ui.nav_groups.system': '', 'ui.public_nav.close_menu': '', 'ui.public_nav.docs': '', diff --git a/packages/i18n/src/keys.generated.ts b/packages/i18n/src/keys.generated.ts index 418e40f2..2bd88dba 100644 --- a/packages/i18n/src/keys.generated.ts +++ b/packages/i18n/src/keys.generated.ts @@ -23,6 +23,8 @@ export const keys = { }, changes: { fields_set: 'audit_log.changes.fields_set', + fields_set_one: 'audit_log.changes.fields_set_one', + fields_set_other: 'audit_log.changes.fields_set_other', no_changes: 'audit_log.changes.no_changes', show_less: 'audit_log.changes.show_less', show_more: 'audit_log.changes.show_more', @@ -241,28 +243,31 @@ export const keys = { doctor: { active: 'dashboard.doctor.active', alert: 'dashboard.doctor.alert', + all_clear_hint: 'dashboard.doctor.all_clear_hint', + all_clear_title: 'dashboard.doctor.all_clear_title', applied: 'dashboard.doctor.applied', - apply: 'dashboard.doctor.apply', - clean: 'dashboard.doctor.clean', + at_head: 'dashboard.doctor.at_head', + behind: 'dashboard.doctor.behind', description: 'dashboard.doctor.description', - dev_server: 'dashboard.doctor.dev_server', + diagnostics: 'dashboard.doctor.diagnostics', + env_database: 'dashboard.doctor.env_database', + env_locales: 'dashboard.doctor.env_locales', + env_mode: 'dashboard.doctor.env_mode', + env_python: 'dashboard.doctor.env_python', + env_revision: 'dashboard.doctor.env_revision', environment: 'dashboard.doctor.environment', - generate: 'dashboard.doctor.generate', installed_modules: 'dashboard.doctor.installed_modules', - just_now: 'dashboard.doctor.just_now', loaded: 'dashboard.doctor.loaded', ok: 'dashboard.doctor.ok', - pending: 'dashboard.doctor.pending', recent_migrations: 'dashboard.doctor.recent_migrations', rerun: 'dashboard.doctor.rerun', review: 'dashboard.doctor.review', run_command: 'dashboard.doctor.run_command', - running: 'dashboard.doctor.running', - stat_checks_passed: 'dashboard.doctor.stat_checks_passed', + stat_errors: 'dashboard.doctor.stat_errors', stat_health: 'dashboard.doctor.stat_health', stat_modules: 'dashboard.doctor.stat_modules', - stat_pending_migrations: 'dashboard.doctor.stat_pending_migrations', - static_checks: 'dashboard.doctor.static_checks', + stat_warnings: 'dashboard.doctor.stat_warnings', + suggestion: 'dashboard.doctor.suggestion', title: 'dashboard.doctor.title', }, home: { @@ -639,6 +644,9 @@ export const keys = { env_var_hint: 'settings.modules.env_var_hint', field_count_suffix: 'settings.modules.field_count_suffix', head_title: 'settings.modules.head_title', + managed_description: 'settings.modules.managed_description', + managed_open: 'settings.modules.managed_open', + managed_title: 'settings.modules.managed_title', no_fields: 'settings.modules.no_fields', search_placeholder: 'settings.modules.search_placeholder', secret_badge: 'settings.modules.secret_badge', @@ -717,8 +725,10 @@ export const keys = { }, nav_groups: { access: 'ui.nav_groups.access', + account: 'ui.nav_groups.account', appearance: 'ui.nav_groups.appearance', content: 'ui.nav_groups.content', + navigation: 'ui.nav_groups.navigation', system: 'ui.nav_groups.system', }, public_nav: { diff --git a/packages/ui/locales/en.json b/packages/ui/locales/en.json index f6bfb403..66c05472 100644 --- a/packages/ui/locales/en.json +++ b/packages/ui/locales/en.json @@ -23,7 +23,9 @@ "access": "Access", "appearance": "Appearance", "content": "Content", - "system": "System" + "system": "System", + "navigation": "Navigation", + "account": "Account" }, "command_palette": { "trigger": "Search", diff --git a/packages/ui/locales/es.json b/packages/ui/locales/es.json index 2c4331f0..75058250 100644 --- a/packages/ui/locales/es.json +++ b/packages/ui/locales/es.json @@ -23,7 +23,9 @@ "access": "Acceso", "appearance": "Apariencia", "content": "Contenido", - "system": "Sistema" + "system": "Sistema", + "navigation": "Navegación", + "account": "Cuenta" }, "command_palette": { "trigger": "Buscar", diff --git a/packages/ui/src/components/CommandPalette.tsx b/packages/ui/src/components/CommandPalette.tsx index 1ac346d2..6fc8c2ce 100644 --- a/packages/ui/src/components/CommandPalette.tsx +++ b/packages/ui/src/components/CommandPalette.tsx @@ -10,7 +10,7 @@ import { } from '@simple-module-py/ui/components/ui/command'; import { Search } from 'lucide-react'; import { useEffect, useMemo, useState } from 'react'; -import type { MenuItem } from '../types'; +import { isPostMenuItem, type MenuItem } from '../types'; import { NavIcon } from './NavIcon'; interface CommandPaletteProps { @@ -20,10 +20,6 @@ interface CommandPaletteProps { accountItems: MenuItem[]; } -function groupOf(item: MenuItem): string { - return item.group || 'Navigation'; -} - /** * ⌘K over everything the sidebar can reach. * @@ -50,29 +46,30 @@ export function CommandPalette({ navItems, accountItems }: CommandPaletteProps) const go = (item: MenuItem) => { setOpen(false); - // Menu entries carry their own method — logging out is a POST, and - // visiting it with a GET would silently do nothing. - if (item.method === 'post') router.post(item.url); + if (isPostMenuItem(item)) router.post(item.url); else router.visit(item.url); }; + const navigationGroupLabel = t(keys.ui.nav_groups.navigation); + const accountGroupLabel = t(keys.ui.nav_groups.account); + const groups = useMemo(() => { const bucketed: Record = {}; for (const item of navItems) { - const key = groupOf(item); + const key = item.group || navigationGroupLabel; if (!bucketed[key]) bucketed[key] = []; bucketed[key].push(item); } // Account actions render through the same loop as every other group — // folded in last so they keep sorting after Navigation, matching where // "log out" otherwise lives, behind the avatar dropdown. Merged rather - // than assigned: a nav item whose own `group` happens to be "Account" - // must not silently vanish from the palette. + // than assigned: a nav item whose own `group` happens to translate to + // the same label as "Account" must not silently vanish from the palette. if (accountItems.length > 0) { - bucketed.Account = [...(bucketed.Account ?? []), ...accountItems]; + bucketed[accountGroupLabel] = [...(bucketed[accountGroupLabel] ?? []), ...accountItems]; } return bucketed; - }, [navItems, accountItems]); + }, [navItems, accountItems, navigationGroupLabel, accountGroupLabel]); return ( <> diff --git a/packages/ui/src/layouts/AdminLayout.tsx b/packages/ui/src/layouts/AdminLayout.tsx index ba1ed4db..1334d7bd 100644 --- a/packages/ui/src/layouts/AdminLayout.tsx +++ b/packages/ui/src/layouts/AdminLayout.tsx @@ -1,17 +1,12 @@ import { Link } from '@inertiajs/react'; import { keys, useT } from '@simple-module-py/i18n'; import type React from 'react'; -import { SidebarLayout } from './SidebarLayout'; +import { DEFAULT_SIDEBAR_THEME, SidebarLayout } from './SidebarLayout'; +// Same visual language as the app sidebar — the admin area announces itself +// through the panel badge and its own menu, not through an alarm color. const THEME = { - sidebarBg: 'bg-admin-bg', - accentColor: 'bg-gradient-to-br from-red-500 to-red-700', - avatarBg: 'bg-red-700', - hoverBg: 'hover:bg-admin-hover', - activeClass: 'bg-red-600/15 text-red-300 border-l-2 border-red-400', - inactiveClass: - 'text-admin-text hover:bg-admin-hover hover:text-white border-l-2 border-transparent', - mutedTextClass: 'text-admin-text-muted', + ...DEFAULT_SIDEBAR_THEME, mobileTitleLabel: 'Admin', } as const; @@ -25,11 +20,11 @@ function AdminBadge() {
- + {t(keys.ui.admin.panel_badge)} @@ -55,7 +50,7 @@ function BackToApp() {