diff --git a/.env.example b/.env.example index cc5503de..224dcecc 100644 --- a/.env.example +++ b/.env.example @@ -30,9 +30,9 @@ ADMIN_EMAILS= # Logging (E22). LOG_LEVEL is the BOOTSTRAP level — the level a process starts at and the only one # the worker/download child read; the API's running level is the admin Configuration page's -# `log_level` knob, which takes effect without a restart. TRACE | DEBUG | INFO | WARNING | ERROR -# (an unknown value falls back to INFO). LOG_FORMAT is "json" (one object per line — what a log -# collector ingests) or "text" (a readable line for a local terminal). +# `log_level` knob, which takes effect without a restart. TRACE | DEBUG | INFO | WARNING | ERROR, in +# any letter case; any other value stops the boot with an error. LOG_FORMAT is "json" (one object per +# line — what a log collector ingests) or "text" (a readable line for a local terminal). LOG_LEVEL=INFO LOG_FORMAT=json diff --git a/backend/app/config.py b/backend/app/config.py index fd59dfbc..5ba056ef 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -1,6 +1,7 @@ from pathlib import Path +from typing import Literal, get_args, get_origin -from pydantic import model_validator +from pydantic import ValidationInfo, field_validator, model_validator from pydantic_settings import BaseSettings, SettingsConfigDict # Shipped placeholder; production must override it. Used by the startup guard below. @@ -20,6 +21,25 @@ def _version_from_file() -> str: return "dev" +# Closed value sets. A setting typed with one of these is a CHOICE: pydantic rejects anything else at +# boot, and sysconfig derives the admin picker's options from the same Literal — so the list lives in +# exactly one place and a consumer never has to guess what a typo should fall back to. +SearchSource = Literal["scrape", "api"] +DownloadLayout = Literal["plex", "flat"] +LogLevel = Literal["TRACE", "DEBUG", "INFO", "WARNING", "ERROR"] +LogFormat = Literal["json", "text"] +TranscodeHw = Literal["auto", "on", "off"] +# libx264's presets, fastest first (`placebo` left out on purpose: no measurable gain for its cost). +TranscodePreset = Literal[ + "ultrafast", "superfast", "veryfast", "faster", "fast", "medium", "slow", "slower", "veryslow" +] + + +def choices_of(annotation: object) -> tuple[str, ...]: + """The allowed values of a choice-typed setting, or () for any other type.""" + return tuple(get_args(annotation)) if get_origin(annotation) is Literal else () + + class Settings(BaseSettings): model_config = SettingsConfigDict(env_file=".env", extra="ignore") @@ -85,7 +105,7 @@ class Settings(BaseSettings): search_daily_limit_per_user: int = 70 # Source for live YouTube search: "scrape" (InnerTube, no API quota) or "api" (search.list, # 100 units/page). Scrape is the default; flip to "api" if scraping is blocked/breaks. - search_source: str = "scrape" + search_source: SearchSource = "scrape" # Recent-first backfill: how far back to fetch on the first pass per channel. backfill_recent_max_videos: int = 100 backfill_recent_max_days: int = 365 @@ -221,7 +241,7 @@ class Settings(BaseSettings): audit_retention_days: int = 180 audit_gc_minutes: int = 1440 # On-disk layout: "plex" (Channel/Season YYYY/… [id].ext + .nfo/poster) or "flat". - download_layout: str = "plex" + download_layout: DownloadLayout = "plex" # Default SponsorBlock (skip sponsor segments) for new custom profiles. sponsorblock_default: bool = False @@ -230,10 +250,10 @@ class Settings(BaseSettings): # startup and on change without a restart. This is what a process logs at before it can reach # the database — and the only knob the worker/runner children read, since they are short-lived. # TRACE (below DEBUG) is the "Teljes" diagnostics level; see app/logs.py. - log_level: str = "INFO" + log_level: LogLevel = "INFO" # "json" (one object per line — what Loki and the log_events sink ingest) or "text" (a readable # line for a local terminal). Redaction applies to both. - log_format: str = "json" + log_format: LogFormat = "json" # --- Plex integration (optional) --- # The Plex module plays the LOCAL physical media file directly (our own ffmpeg/direct-serve); @@ -276,7 +296,7 @@ class Settings(BaseSettings): # Hardware-encoder policy: "auto" = VAAPI (h264_vaapi) when the iGPU render node is present AND the # source codec is hardware-decodable, else libx264 software; "on" = force VAAPI wherever the render # node exists (skip the decodability gate); "off" = always software. See plex/stream.py `_select_encode`. - plex_transcode_hw: str = "auto" + plex_transcode_hw: TranscodeHw = "auto" # Software-fallback libx264 tuning (IGNORED on the VAAPI path). `preset` trades CPU for size; `crf` # trades quality for size (lower = better + bigger). `max_height` caps the SOFTWARE transcode's output # height (0 = keep source) so a CPU re-encode of a tall source stays affordable — the GPU path always @@ -286,7 +306,7 @@ class Settings(BaseSettings): # call, not a fix. Only affects files that go through remux/transcode — a direct-played file is # served untouched, but those are browser-native stereo anyway, not the 5.1 mixes this is for. plex_audio_loudnorm: bool = False - plex_transcode_preset: str = "veryfast" + plex_transcode_preset: TranscodePreset = "veryfast" plex_transcode_crf: int = 20 plex_transcode_max_height: int = 1080 # Where on-the-fly HLS remux segments are written (transient, reaped). Empty → a `.plex-hls` @@ -335,6 +355,18 @@ class Settings(BaseSettings): https OAUTH_REDIRECT_URL as the signal for 'real deployment' vs local dev.""" return self.oauth_redirect_url.lower().startswith("https://") + @field_validator("*", mode="before") + @classmethod + def _canonical_choice(cls, value: object, info: ValidationInfo) -> object: + """A choice setting accepts its value in any letter case (`LOG_LEVEL=debug`) and stores the + canonical spelling; an unknown value still fails the boot with pydantic's own message.""" + field = cls.model_fields.get(info.field_name or "") + options = choices_of(field.annotation) if field else () + if options and isinstance(value, str): + folded = value.strip().casefold() + return next((o for o in options if o.casefold() == folded), value) + return value + @model_validator(mode="after") def _enforce_production_secrets(self) -> "Settings": """Fail fast rather than boot a public instance with the shipped placeholder diff --git a/backend/app/logs.py b/backend/app/logs.py index 9ef7b27f..708a7fb5 100644 --- a/backend/app/logs.py +++ b/backend/app/logs.py @@ -295,20 +295,20 @@ _THIRD_PARTY_LEVELS = { } -def resolve_level(raw: str | None) -> int: - """Level name (case-insensitive) → number, falling back to INFO for an unknown value. A typo in - a config knob must not silence the app.""" - if not raw: - return logging.INFO - value = logging.getLevelName(raw.strip().upper()) - return value if isinstance(value, int) else logging.INFO +def resolve_level(name: str) -> int: + """Level name → number. Both ways in (the `LOG_LEVEL` env and the `log_level` admin knob) are + already held to the `LogLevel` choices, so an unknown name here is a programming error.""" + value = logging.getLevelName(name) + if not isinstance(value, int): + raise ValueError(f"unknown log level {name!r}") + return value -def configure_logging(source: str, *, level: str | None = None) -> None: +def configure_logging(source: str) -> None: """Configure logging for THIS process. Call once, first thing, in every entry point: `main.py` (api), `worker.main`, `downloads/runner.main`, `alembic/env.py`. - `source` names the process in every line. `level` overrides `LOG_LEVEL` (the bootstrap default); + `source` names the process in every line. The level starts at `LOG_LEVEL` (the bootstrap default); the runtime admin knob goes through `set_level()` instead, so changing it needs no restart. Our loggers deliberately keep `propagate=True` and own no handler: everything reaches the single @@ -316,7 +316,7 @@ def configure_logging(source: str, *, level: str | None = None) -> None: is configured the moment it is created.""" global _source _source = source - formatter = "json" if (settings.log_format or "json").lower() != "text" else "text" + formatter = settings.log_format logging.config.dictConfig( { "version": 1, @@ -336,16 +336,16 @@ def configure_logging(source: str, *, level: str | None = None) -> None: "filters": ["context"], } }, - "root": {"level": resolve_level(level or settings.log_level), "handlers": ["stdout"]}, + "root": {"level": resolve_level(settings.log_level), "handlers": ["stdout"]}, "loggers": {name: {"level": lvl} for name, lvl in _THIRD_PARTY_LEVELS.items()}, } ) -def set_level(raw: str | None) -> None: +def set_level(name: str) -> None: """Change the running process's level (the `log_level` admin knob). Third-party loggers keep their pinned levels, so turning on DEBUG shows OUR detail, not SQLAlchemy's.""" - logging.getLogger().setLevel(resolve_level(raw)) + logging.getLogger().setLevel(resolve_level(name)) def current_level() -> str: diff --git a/backend/app/plex/stream.py b/backend/app/plex/stream.py index e65d65e8..aec74c3e 100644 --- a/backend/app/plex/stream.py +++ b/backend/app/plex/stream.py @@ -404,7 +404,7 @@ def _select_encode(db: DbSession, item: PlexItem, transcode: bool, src: Path) -> source that has an OpenCL runtime, while a 10-bit SDR source drops to software.""" if not transcode: return (False, False) - mode = (sysconfig.get_str(db, "plex_transcode_hw") or "auto").lower() + mode = sysconfig.get_str(db, "plex_transcode_hw") if mode == "off" or not _render_node_present(): return (False, False) if mode == "on": @@ -664,7 +664,7 @@ def start_session( # remux ignore preset/crf/max_height, and remux must not touch the DB for them). preset, crf, max_height = "veryfast", 20, 0 if transcode and not hw: - preset = sysconfig.get_str(db, "plex_transcode_preset") or "veryfast" + preset = sysconfig.get_str(db, "plex_transcode_preset") crf = sysconfig.get_int(db, "plex_transcode_crf") max_height = sysconfig.get_int(db, "plex_transcode_max_height") # Nice the SOFTWARE transcode child (not the HW path, and not remux) so a CPU-bound re-encode diff --git a/backend/app/routes/search.py b/backend/app/routes/search.py index 21338e8b..91ecad5d 100644 --- a/backend/app/routes/search.py +++ b/backend/app/routes/search.py @@ -231,9 +231,7 @@ def search_youtube( limit = max(1, min(limit, 100)) # Source: "scrape" (InnerTube, no API quota) or "api" (search.list, 100 units/page). - source = (sysconfig.get_str(db, "search_source") or "scrape").lower() - if source != "scrape": - source = "api" + source = sysconfig.get_str(db, "search_source") # Per-user daily cap applies to both sources (anti-abuse); with scrape it's the only guard # since the search itself spends no quota. diff --git a/backend/app/sysconfig.py b/backend/app/sysconfig.py index 3ca1e0e0..b9cc7417 100644 --- a/backend/app/sysconfig.py +++ b/backend/app/sysconfig.py @@ -14,7 +14,7 @@ from dataclasses import dataclass from sqlalchemy.orm import Session from app import security -from app.config import settings +from app.config import Settings, choices_of, settings from app.models import SystemConfig log = logging.getLogger("siftlode.config") @@ -27,7 +27,7 @@ _decrypt_warned: set[str] = set() @dataclass(frozen=True) class ConfigSpec: key: str - type: str # "int" | "str" | "bool" + type: str # "int" | "str" | "bool" | "choice" group: str # admin-UI grouping (logically related keys stay together) default_attr: str # attribute on `settings` holding the env/config default secret: bool = False @@ -35,16 +35,33 @@ class ConfigSpec: max: int | None = None allow_empty: bool = False # str keys: a stored "" is an explicit empty value, not "unset" + @property + def choices(self) -> tuple[str, ...]: + """A choice key's allowed values, read from its setting's Literal type in config.py — the one + place the list is written. The admin page renders them as a picker.""" + return choices_of(Settings.model_fields[self.default_attr].annotation) + # Registry of DB-overridable keys. Add a key here + wire its read through get_*() to move it -# off env into the admin UI; the admin page renders it automatically from this list. +# off env into the admin UI; the admin page renders it automatically from this list. The ORDER is the +# admin page's tab order (first appearance of each group), so keep a group's keys together. SPECS: tuple[ConfigSpec, ...] = ( + # --- Access / registration --- + ConfigSpec("allow_registration", "bool", "access", "allow_registration"), # --- Email / SMTP (one logical group; the password is the only secret) --- ConfigSpec("smtp_host", "str", "email", "smtp_host"), ConfigSpec("smtp_port", "int", "email", "smtp_port", min=1, max=65535), ConfigSpec("smtp_user", "str", "email", "smtp_user", allow_empty=True), ConfigSpec("smtp_from", "str", "email", "smtp_from"), ConfigSpec("smtp_password", "str", "email", "smtp_password", secret=True), + # --- YouTube Data API key (secret; optional — public reads prefer it over OAuth) --- + ConfigSpec("youtube_api_key", "str", "youtube", "youtube_api_key", secret=True), + # Optional egress proxy for YouTube API calls (fixed-IP host for an IP-restricted key). + ConfigSpec("youtube_api_proxy", "str", "youtube", "youtube_api_proxy", allow_empty=True), + # --- Google OAuth client (Sign in with Google; both encrypted, env fallback). Set from the + # install wizard / Configuration page so no restart is needed when the creds change. --- + ConfigSpec("google_client_id", "str", "google", "google_client_id", secret=True), + ConfigSpec("google_client_secret", "str", "google", "google_client_secret", secret=True), # --- Quota (shared YouTube Data API daily budget) --- ConfigSpec("quota_daily_budget", "int", "quota", "quota_daily_budget", min=1, max=1_000_000), ConfigSpec( @@ -59,7 +76,7 @@ SPECS: tuple[ConfigSpec, ...] = ( max=100_000, ), # Live-search source: "scrape" (InnerTube, zero API quota) or "api" (search.list, 100u/page). - ConfigSpec("search_source", "str", "quota", "search_source"), + ConfigSpec("search_source", "choice", "quota", "search_source"), # --- Backfill (recent-first first pass per channel) --- ConfigSpec( "backfill_recent_max_videos", @@ -96,16 +113,6 @@ SPECS: tuple[ConfigSpec, ...] = ( ConfigSpec("explore_grace_days", "int", "explore", "explore_grace_days", min=0, max=3_650), # Days an un-kept live-search result video is kept before the cleanup job removes it. ConfigSpec("search_grace_days", "int", "explore", "search_grace_days", min=0, max=3_650), - # --- YouTube Data API key (secret; optional — public reads prefer it over OAuth) --- - ConfigSpec("youtube_api_key", "str", "youtube", "youtube_api_key", secret=True), - # Optional egress proxy for YouTube API calls (fixed-IP host for an IP-restricted key). - ConfigSpec("youtube_api_proxy", "str", "youtube", "youtube_api_proxy", allow_empty=True), - # --- Google OAuth client (Sign in with Google; both encrypted, env fallback). Set from the - # install wizard / Configuration page so no restart is needed when the creds change. --- - ConfigSpec("google_client_id", "str", "google", "google_client_id", secret=True), - ConfigSpec("google_client_secret", "str", "google", "google_client_secret", secret=True), - # --- Access / registration --- - ConfigSpec("allow_registration", "bool", "access", "allow_registration"), # --- Download center (yt-dlp) — per-user defaults + retention/GC + layout --- ConfigSpec("download_total_max_bytes", "int", "downloads", "download_total_max_bytes", min=0), ConfigSpec( @@ -137,7 +144,7 @@ SPECS: tuple[ConfigSpec, ...] = ( ConfigSpec( "download_gc_grace_days", "int", "downloads", "download_gc_grace_days", min=0, max=3_650 ), - ConfigSpec("download_layout", "str", "downloads", "download_layout"), + ConfigSpec("download_layout", "choice", "downloads", "download_layout"), ConfigSpec( "download_worker_concurrency", "int", @@ -151,10 +158,9 @@ SPECS: tuple[ConfigSpec, ...] = ( # Days to keep admin audit-log rows before the audit_gc job prunes them (0 = keep forever). ConfigSpec("audit_retention_days", "int", "audit", "audit_retention_days", min=0, max=3650), # --- Diagnostics (E22) --- - # Runtime log level for the API process: TRACE | DEBUG | INFO | WARNING | ERROR. Free-text like - # search_source (an unknown value falls back to INFO in logs.resolve_level, so a typo can't - # silence the app). Applied without a restart — see routes/config.py and main.py's lifespan. - ConfigSpec("log_level", "str", "diagnostics", "log_level"), + # Runtime log level for the API process. Applied without a restart — see routes/config.py and + # main.py's lifespan. + ConfigSpec("log_level", "choice", "diagnostics", "log_level"), # --- Plex integration (optional; local-file playback + Plex metadata) --- ConfigSpec("plex_enabled", "bool", "plex", "plex_enabled"), ConfigSpec("plex_server_url", "str", "plex", "plex_server_url"), @@ -164,10 +170,10 @@ SPECS: tuple[ConfigSpec, ...] = ( ConfigSpec("plex_overlay_map", "str", "plex", "plex_overlay_map"), # (plex_sync_interval_min is tuned from the Scheduler dashboard, not here — it's a scheduler job.) ConfigSpec("plex_max_transcodes", "int", "plex", "plex_max_transcodes", min=0, max=16), - # Transcode re-encode tuning (E6 S1). hw = "auto"|"on"|"off" (free-text, like search_source); the - # libx264 knobs apply only to the software fallback. crf 0–51 (x264 range); max_height 0 = source. - ConfigSpec("plex_transcode_hw", "str", "plex", "plex_transcode_hw"), - ConfigSpec("plex_transcode_preset", "str", "plex", "plex_transcode_preset"), + # Transcode re-encode tuning (E6 S1). The libx264 knobs apply only to the software fallback. + # crf 0–51 (x264 range); max_height 0 = source. + ConfigSpec("plex_transcode_hw", "choice", "plex", "plex_transcode_hw"), + ConfigSpec("plex_transcode_preset", "choice", "plex", "plex_transcode_preset"), ConfigSpec("plex_transcode_crf", "int", "plex", "plex_transcode_crf", min=0, max=51), ConfigSpec( "plex_transcode_max_height", "int", "plex", "plex_transcode_max_height", min=0, max=4320 @@ -199,11 +205,24 @@ def _default(s: ConfigSpec): return getattr(settings, s.default_attr) +def _choice(s: ConfigSpec, raw: object) -> str: + """The canonical spelling of `raw` among the key's choices (any letter case), or ValueError.""" + folded = str(raw).strip().casefold() + for option in s.choices: + if option.casefold() == folded: + return option + raise ValueError(f"{s.key} must be one of: {', '.join(s.choices)}") + + def _coerce(s: ConfigSpec, raw: str): if s.type == "int": return int(raw) if s.type == "bool": return raw == "true" + if s.type == "choice": + # A stored value that is no longer a choice (the list shrank) reads as the default, like a + # non-numeric int override does — see get(). + return _choice(s, raw) return raw @@ -279,6 +298,8 @@ def set_value(db: Session, key: str, value) -> None: raw = str(v) elif s.type == "bool": raw = "true" if value else "false" + elif s.type == "choice": + raw = _choice(s, value) else: raw = "" if value is None else str(value) @@ -321,6 +342,7 @@ def describe(db: Session) -> dict: "max": s.max, "is_set": is_set, # an override row exists "allow_empty": s.allow_empty, # if true, the UI stores "" rather than deleting the row + "choices": list(s.choices), # the picker's options; [] for any other type } if s.secret: item["default_is_set"] = bool(_default(s)) diff --git a/backend/tests/test_logs.py b/backend/tests/test_logs.py index 2c19e185..62c79e54 100644 --- a/backend/tests/test_logs.py +++ b/backend/tests/test_logs.py @@ -17,6 +17,7 @@ from fastapi import FastAPI from fastapi.testclient import TestClient from app import logs +from app.config import LogLevel, choices_of class _Sink(logging.Handler): @@ -161,11 +162,17 @@ def test_bound_correlation_appears_and_is_released(sink): assert "job_id" not in sink.objects()[-1] -def test_an_unknown_level_name_falls_back_to_info_instead_of_silencing(): - assert logs.resolve_level("nonsense") == logging.INFO - assert logs.resolve_level(None) == logging.INFO - assert logs.resolve_level("trace") == logs.TRACE - assert logs.resolve_level(" debug ") == logging.DEBUG +def test_every_selectable_level_resolves_to_a_real_logging_level(): + # The picker's options come from `LogLevel`; each must be a level `logging` knows, TRACE included. + assert [logs.resolve_level(n) for n in choices_of(LogLevel)] == [ + logs.TRACE, + logging.DEBUG, + logging.INFO, + logging.WARNING, + logging.ERROR, + ] + with pytest.raises(ValueError): + logs.resolve_level("nonsense") def test_configure_logging_silences_the_uvicorn_access_log_and_keeps_ours_propagating(): diff --git a/backend/tests/test_sysconfig_registry.py b/backend/tests/test_sysconfig_registry.py new file mode 100644 index 00000000..7a25e5d6 --- /dev/null +++ b/backend/tests/test_sysconfig_registry.py @@ -0,0 +1,93 @@ +"""The sysconfig registry: its order is the admin page's tab order, and a closed-set knob is a CHOICE. + +The admin Configuration page used to keep its own hand-written group order, which had already drifted +(`explore` was missing, so its tab sorted last). The page now follows SPECS, so SPECS has to read in +tab order: one run of keys per group. + +A closed-set admin knob is a CHOICE, not free text (E22 S2 follow-up). + +`log_level`, `search_source`, `download_layout` and the transcode `hw`/`preset` used to be free-text +fields: the admin could type anything and every consumer carried its own `or "auto").lower()` guess +at what a typo meant. Now the value set is a `Literal` in config.py, pydantic holds the env to it at +boot, sysconfig holds a stored override to it, and the admin page renders a picker from the same list. +The DB lane (`db` fixture) covers the store; the rest is pure logic. +""" + +import pytest +from pydantic import ValidationError + +from app import sysconfig +from app.config import Settings, choices_of + +CHOICE_KEYS = {s.key for s in sysconfig.SPECS if s.type == "choice"} + + +def test_each_group_is_one_run_of_keys_so_the_registry_reads_in_tab_order(): + runs = [ + s.group + for i, s in enumerate(sysconfig.SPECS) + if i == 0 or sysconfig.SPECS[i - 1].group != s.group + ] + assert len(runs) == len(set(runs)), runs + assert runs[:2] == ["access", "email"] + + +def test_every_closed_set_knob_is_a_choice_whose_options_come_from_config(): + assert CHOICE_KEYS == { + "search_source", + "download_layout", + "log_level", + "plex_transcode_hw", + "plex_transcode_preset", + } + for s in sysconfig.SPECS: + field = Settings.model_fields[s.default_attr] + if s.type == "choice": + assert s.choices, s.key + assert field.default in s.choices, s.key + else: + # A Literal-typed setting exposed as "str" would be a free-text field again. + assert s.choices == () and choices_of(field.annotation) == (), s.key + + +def test_the_env_accepts_any_letter_case_and_rejects_an_unknown_value(): + assert Settings(log_level="debug").log_level == "DEBUG" + assert Settings(search_source=" API ").search_source == "api" + with pytest.raises(ValidationError): + Settings(log_level="verbose") + + +class _Row: + def __init__(self, value): + self.value, self.encrypted = value, False + + +class _FakeDb: + """Just enough of a Session for sysconfig's reads: one stored override row per key.""" + + def __init__(self, **rows): + self.rows = {k: _Row(v) for k, v in rows.items()} + + def get(self, _model, key): + return self.rows.get(key) + + +def test_a_stored_value_reads_back_canonical_and_a_stale_one_falls_back_to_the_default(): + assert sysconfig.get_str(_FakeDb(plex_transcode_hw="OFF"), "plex_transcode_hw") == "off" + # A value that is no longer (or never was) a choice is not passed through to the consumer. + assert sysconfig.get_str(_FakeDb(plex_transcode_hw="maybe"), "plex_transcode_hw") == "auto" + + +def test_describe_hands_the_picker_its_options(): + items = {i["key"]: i for g in sysconfig.describe(_FakeDb())["groups"].values() for i in g} + assert items["log_level"]["choices"] == ["TRACE", "DEBUG", "INFO", "WARNING", "ERROR"] + assert items["log_level"]["value"] == "INFO" + assert items["quota_daily_budget"]["choices"] == [] + + +def test_set_value_stores_the_canonical_spelling_and_refuses_anything_else(db): + sysconfig.set_value(db, "search_source", "API") + assert sysconfig.get_str(db, "search_source") == "api" + with pytest.raises(ValueError, match="scrape, api"): + sysconfig.set_value(db, "search_source", "bing") + db.rollback() diff --git a/frontend/src/components/ConfigPanel.choice.dom.test.tsx b/frontend/src/components/ConfigPanel.choice.dom.test.tsx new file mode 100644 index 00000000..8ad885fe --- /dev/null +++ b/frontend/src/components/ConfigPanel.choice.dom.test.tsx @@ -0,0 +1,118 @@ +// @vitest-environment jsdom +// +// A closed-set config key is picked, not typed: a short list renders as a segmented bar, a long one as +// a dropdown, and the pick is what Save sends. A cleared draft (reset to the default) shows the default +// as the active option rather than no option at all. +// +// i18n is a key-passthrough; the admin config API is stubbed. +import { afterEach, describe, expect, it, vi } from "vitest"; +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { ConfigItem } from "../lib/api"; + +vi.mock("react-i18next", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }), + }; +}); + +const choice = (key: string, choices: string[], value: string, is_set = false): ConfigItem => ({ + key, + type: "choice", + group: "g", + secret: false, + min: null, + max: null, + is_set, + allow_empty: false, + choices, + value, + default: choices[0], +}); + +const PRESETS = ["ultrafast", "superfast", "veryfast", "faster", "fast", "medium", "slow"]; +const setConfig = vi.fn((_key: string, _value: unknown) => Promise.resolve({})); +const resetConfig = vi.fn((_key: string) => Promise.resolve({})); + +vi.mock("../lib/api", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + api: { + ...actual.api, + adminConfig: () => + Promise.resolve({ + groups: { + g: [ + choice("plex_transcode_hw", ["auto", "on", "off"], "on", true), + choice("plex_transcode_preset", PRESETS, "veryfast"), + ], + }, + secrets_manageable: true, + }), + setConfig: (key: string, value: unknown) => setConfig(key, value), + resetConfig: (key: string) => resetConfig(key), + }, + }; +}); +vi.mock("../lib/useMe", () => ({ useMe: () => ({ email_configured: true }) })); + +import ConfigPanel from "./ConfigPanel"; + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +const mount = () => + render( + + + , + ); + +describe("ConfigPanel choice keys", () => { + it("renders a short list as a segmented bar and saves the pick", async () => { + mount(); + const bar = await screen.findByRole("group", { name: "config.fields.plex_transcode_hw.label" }); + const pressed = () => + [...bar.querySelectorAll('[aria-pressed="true"]')].map((b) => b.textContent); + expect(pressed()).toEqual(["config.choices.plex_transcode_hw.on"]); + expect(screen.queryByRole("textbox")).toBeNull(); // no free-text field for a choice key + + fireEvent.click(screen.getByRole("button", { name: "config.choices.plex_transcode_hw.off" })); + fireEvent.click(await screen.findByRole("button", { name: "config.save" })); + await waitFor(() => expect(setConfig).toHaveBeenCalledWith("plex_transcode_hw", "off")); + }); + + it("shows the default as active once the override is reset, and Save resets it", async () => { + mount(); + await screen.findByRole("group", { name: "config.fields.plex_transcode_hw.label" }); + fireEvent.click(screen.getByRole("button", { name: /config\.reset$/ })); + expect( + screen + .getByRole("button", { name: "config.choices.plex_transcode_hw.auto" }) + .getAttribute("aria-pressed"), + ).toBe("true"); + fireEvent.click(await screen.findByRole("button", { name: "config.save" })); + await waitFor(() => expect(resetConfig).toHaveBeenCalledWith("plex_transcode_hw")); + }); + + it("renders a long list as a dropdown showing only the value", async () => { + mount(); + const trigger = await screen.findByRole("button", { + name: "config.fields.plex_transcode_preset.label: config.choices.plex_transcode_preset.veryfast", + }); + expect(trigger.textContent).toBe("config.choices.plex_transcode_preset.veryfast"); + fireEvent.click(trigger); + // The popover rows are queried as FilterSelect.dom.test does (`hidden: true`, picked by text). + const rows = screen.getAllByRole("menuitemradio", { hidden: true }); + fireEvent.click( + rows.find((r) => r.textContent === "config.choices.plex_transcode_preset.slow")!, + ); + fireEvent.click(await screen.findByRole("button", { name: "config.save" })); + await waitFor(() => expect(setConfig).toHaveBeenCalledWith("plex_transcode_preset", "slow")); + }); +}); diff --git a/frontend/src/components/ConfigPanel.tsx b/frontend/src/components/ConfigPanel.tsx index 92ff29a0..26b4cea7 100644 --- a/frontend/src/components/ConfigPanel.tsx +++ b/frontend/src/components/ConfigPanel.tsx @@ -12,30 +12,17 @@ import Tabs, { usePersistedTab } from "./ui/Tabs"; import { LoadingState, StateMessage } from "./ui/QueryState"; import { PageToolbar } from "./ui/PageShell"; import { Switch } from "./ui/form"; +import SegmentedToggle from "./ui/SegmentedToggle"; +import FilterSelect from "./ui/FilterSelect"; import { DraftSaveBar, type SaveState } from "./ui/DraftSaveBar"; import { LS } from "../lib/storage"; import { notifyActionError } from "../lib/actionErrors"; // Admin Configuration page: edit DB-overridable operational settings (registry-driven from the // backend — adding a key there makes it appear here automatically). Edits are drafted and -// applied together via one Save/Discard bar; nothing hits -// the server until Save. Group order is fixed where known so logically related settings (e.g. -// Email/SMTP) stay together. -const GROUP_ORDER = [ - "access", - "email", - "youtube", - "google", - "quota", - "backfill", - "shorts", - "batch", - "maintenance", - "downloads", - "audit", - "plex", - "tmdb", -]; +// applied together via one Save/Discard bar; nothing hits the server until Save. The tabs follow the +// registry's own order (`sysconfig.SPECS`), so a new group lands where the backend puts it instead of +// needing a second, hand-kept order list here. export default function ConfigPanel() { const { t } = useTranslation(); @@ -171,9 +158,7 @@ export default function ConfigPanel() { return ; } - const groupKeys = Object.keys(data.groups).sort( - (a, b) => (GROUP_ORDER.indexOf(a) + 1 || 99) - (GROUP_ORDER.indexOf(b) + 1 || 99), - ); + const groupKeys = Object.keys(data.groups); // Clamp the persisted tab to a real group (the registry can change between sessions). The // tab pill carries a dot when that group has unsaved edits, so a draft in a hidden tab is // never silently lost behind the global Save bar. @@ -330,6 +315,7 @@ function Field({ const { t } = useTranslation(); const isNum = item.type === "int"; const isBool = item.type === "bool"; + const label = t(`config.fields.${item.key}.label`, item.key); const secretDisabled = item.secret && !secretsManageable; // Status line: never reveal a secret's value. @@ -356,7 +342,7 @@ function Field({ return (
-
{t(`config.fields.${item.key}.label`, item.key)}
+
{label}

{t(`config.fields.${item.key}.hint`, "")}

@@ -366,10 +352,12 @@ function Field({
{isBool ? ( onChange(v ? "true" : "false")} /> + ) : item.type === "choice" ? ( + ) : ( )} @@ -400,3 +388,47 @@ function Field({
); } + +// A closed-set key picks from its options instead of taking free text: a segmented bar for a short +// list, a dropdown past six (the house rule for chip vs. select). A cleared draft ("" = reset to the +// default on Save) shows the default as the active option, as a text field shows it as placeholder. +const MAX_SEGMENTS = 6; + +function ChoiceField({ + item, + label, + value, + onChange, +}: { + item: ConfigItem; + label: string; + value: string; + onChange: (v: string) => void; +}) { + const { t } = useTranslation(); + const options = item.choices.map((c) => ({ + value: c, + label: t(`config.choices.${item.key}.${c}`, c), + })); + const current = value || String(item.default ?? ""); + if (options.length <= MAX_SEGMENTS) { + return ( + + ); + } + return ( + next && onChange(next)} + /> + ); +} diff --git a/frontend/src/components/ui/FilterSelect.tsx b/frontend/src/components/ui/FilterSelect.tsx index 329e2491..bb12deac 100644 --- a/frontend/src/components/ui/FilterSelect.tsx +++ b/frontend/src/components/ui/FilterSelect.tsx @@ -21,6 +21,9 @@ export interface FilterSelectOption { export interface FilterSelectProps { /** Visible group name shown as the trigger prefix (`Label: value`) and the accessible name. */ label: string; + /** Show only the value on the trigger — for a form row whose own caption already names the field. + * The label still names the control and its menu for assistive tech. */ + hideLabel?: boolean; options: readonly FilterSelectOption[]; selected: string[]; onChange: (next: string[]) => void; @@ -39,6 +42,7 @@ export default function FilterSelect({ onChange, multiple = false, requireOne = true, + hideLabel = false, testId, }: FilterSelectProps) { const [open, setOpen] = useState(false); @@ -47,6 +51,14 @@ export default function FilterSelect({ onOpenChange: setOpen, mode: "menu", placement: "bottom-start", + // Single choice: opening lands on the option in use, not the first row. A multi-select toggle + // list has no one "current" row, so it keeps the default. + selectedIndex: multiple + ? null + : Math.max( + 0, + options.findIndex((o) => o.value === selected[0]), + ), }); const labelOf = (v: string) => options.find((o) => o.value === v)?.label ?? v; @@ -88,7 +100,7 @@ export default function FilterSelect({ open ? "border-accent" : "border-border" }`} > - {label}: + {!hideLabel && {label}:} {summary}