From 05781b68bdd408db79bad1f42f2b1ea56fcd1f28 Mon Sep 17 00:00:00 2001 From: Thomas Christory Date: Fri, 26 Jun 2026 16:16:27 +0200 Subject: [PATCH 1/2] feat(search): add local saved filter sets (saved searches) Persist named, per-resource filter sets locally under `saved_searches` in config, mirroring the existing `columns` column-prefs pattern end to end. - config: new `Config.saved_searches` field + framework-free `get_saved_search`/`list_saved_searches` helpers. - writer: `_save_search`/`_delete_search` round-trip persisters in tui_commands. - TUI: FilterScreen gains Ctrl+W (save current state under a name) and Ctrl+O (load a saved set); NscTuiApp gains `saved_searches_for`/`save_search`/ `delete_search` with write-through callbacks; new name-prompt + picker modals. - CLI: `--saved ` on list ops resolves saved params as the base layer; explicit `--filter` and typed query options override; unknown name exits 2. - keymap/help/README updated; distinct from server-side `extras saved-filters`. Closes #119 Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 16 ++++++ docs/reference/config.md | 1 + nsc/cli/registration.py | 57 +++++++++++++++++--- nsc/cli/tui_commands.py | 46 ++++++++++++++++ nsc/config/models.py | 4 ++ nsc/config/saved_searches.py | 22 ++++++++ nsc/tui/__init__.py | 9 ++++ nsc/tui/app.py | 30 +++++++++++ nsc/tui/keymap.py | 6 ++- nsc/tui/screens/filter.py | 52 +++++++++++++++++- nsc/tui/screens/list.py | 10 +++- nsc/tui/screens/saved_search_picker.py | 75 ++++++++++++++++++++++++++ nsc/tui/styles.tcss | 24 +++++++++ nsc/tui/widgets/help.py | 1 + tests/cli/test_registration.py | 74 +++++++++++++++++++++++++ tests/cli/test_tui_command.py | 44 +++++++++++++++ tests/config/test_models.py | 18 +++++++ tests/config/test_saved_searches.py | 54 +++++++++++++++++++ tests/tui/test_app.py | 47 ++++++++++++++++ tests/tui/test_filter_screen.py | 75 +++++++++++++++++++++++++- tests/tui/test_keymap.py | 10 ++-- 21 files changed, 660 insertions(+), 15 deletions(-) create mode 100644 nsc/config/saved_searches.py create mode 100644 nsc/tui/screens/saved_search_picker.py create mode 100644 tests/config/test_saved_searches.py diff --git a/README.md b/README.md index 98ca6cd..152fbe9 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,22 @@ uv run nsc circuits providers list --output csv uv run nsc ipam prefixes list --filter created__gte=2026-01-01 --output yaml ``` +### Saved searches + +Filter sets you save from the TUI (Ctrl+W in the filter builder) are stored +locally under `saved_searches` in `~/.nsc/config.yaml`, keyed by tag/resource. +Re-apply one on the CLI with `--saved`: + +```sh +uv run nsc dcim devices list --saved active-switches +# explicit --filter / typed options override the saved values: +uv run nsc dcim devices list --saved active-switches --status offline +``` + +This is a local, per-config convenience and is unrelated to NetBox's +server-side saved filters (`extras saved-filters`), which store web-UI query +strings. + ## Writing ```sh diff --git a/docs/reference/config.md b/docs/reference/config.md index 1d3c88b..0dafb6a 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -13,6 +13,7 @@ All fields below describe `~/.nsc/config.yaml`. | `profiles` | `dict[str, Profile]` | `{}` | | `defaults` | `` | `Defaults(output=, page_size=50, timeout=30.0, schema_refresh=, color_mode=, audit_redaction=)` | | `columns` | `dict[str, dict[str, list[str]]]` | `{}` | +| `saved_searches` | `dict[str, dict[str, dict[str, dict[str, str]]]]` | `{}` | ## `Profile` diff --git a/nsc/cli/registration.py b/nsc/cli/registration.py index 69612ef..59fb835 100644 --- a/nsc/cli/registration.py +++ b/nsc/cli/registration.py @@ -120,10 +120,39 @@ def _register_write( _GLOBAL_FLAG_NAMES: frozenset[str] = frozenset( - {"output", "compact", "columns", "limit", "all_", "filter_"} + {"output", "compact", "columns", "limit", "all_", "filter_", "saved"} ) +def _resolve_saved_filters( + ctx: RuntimeContext, + tag_name: str, + resource_name: str, + saved_name: str | None, + explicit_filters: list[tuple[str, str]], + typed_kwargs: dict[str, Any], +) -> list[tuple[str, str]]: + """Layer a `--saved` filter set under explicit `--filter` and typed options. + + Saved params are the base; explicit `--filter` pairs and any non-None typed + query option (already in ``typed_kwargs``) win. Order matters: ``handle_list`` + overlays ``ctx.filters`` onto the typed-option params, so saved keys that the + user also set via a typed flag must be dropped here to let the flag win. + """ + if saved_name is None: + return explicit_filters + from nsc.config.saved_searches import get_saved_search # noqa: PLC0415 + + saved = get_saved_search(ctx.config, tag_name, resource_name, saved_name) + if saved is None: + raise typer.BadParameter( + f"no saved search named {saved_name!r} for {tag_name}/{resource_name}" + ) + typed_keys = {k for k, v in typed_kwargs.items() if v is not None} + base = [(k, v) for k, v in saved.items() if k not in typed_keys] + return base + explicit_filters + + def _build_read_closure( operation: Operation, tag_name: str, @@ -151,17 +180,19 @@ def impl(**kwargs: Any) -> None: limit = kwargs.pop("limit", None) fetch_all = kwargs.pop("all_", False) filters_raw: list[str] = kwargs.pop("filter_", None) or [] + saved_name: str | None = kwargs.pop("saved", None) ctx = get_ctx() + explicit_filters = [ + (item.split("=", 1)[0], item.split("=", 1)[1]) for item in filters_raw if "=" in item + ] update: dict[str, Any] = { "compact": compact, "columns_override": columns_csv.split(",") if columns_csv else None, "limit": limit, "fetch_all": fetch_all, - "filters": [ - (item.split("=", 1)[0], item.split("=", 1)[1]) - for item in filters_raw - if "=" in item - ], + "filters": _resolve_saved_filters( + ctx, tag_name, resource_name, saved_name, explicit_filters, kwargs + ), } if output: update["output_format"] = OutputFormat(output) @@ -311,6 +342,20 @@ def _build_global_flag_params() -> tuple[inspect.Parameter, ...]: annotation=list[str] | None, default=typer.Option(None, "--filter"), ), + inspect.Parameter( + name="saved", + kind=inspect.Parameter.KEYWORD_ONLY, + annotation=str | None, + default=typer.Option( + None, + "--saved", + help=( + "Apply a LOCAL named filter set saved from the TUI (config " + "`saved_searches`). Explicit --filter/typed options override it. " + "Unrelated to NetBox server-side saved filters." + ), + ), + ), ) diff --git a/nsc/cli/tui_commands.py b/nsc/cli/tui_commands.py index dbe1faf..beda360 100644 --- a/nsc/cli/tui_commands.py +++ b/nsc/cli/tui_commands.py @@ -41,6 +41,42 @@ def _save_columns(tag: str, resource: str, columns: list[str]) -> None: atomic_write(path, dump_round_trip(doc)) +def _save_search(tag: str, resource: str, name: str, params: dict[str, str]) -> None: + """Persist a filter set to ``saved_searches...``.""" + from nsc.config.settings import default_paths # noqa: PLC0415 + from nsc.config.writer import ( # noqa: PLC0415 + acquire_lock, + atomic_write, + dump_round_trip, + load_round_trip, + set_path, + ) + + path = default_paths().config_file + with acquire_lock(path): + doc = load_round_trip(path) + set_path(doc, f"saved_searches.{tag}.{resource}.{name}", dict(params)) + atomic_write(path, dump_round_trip(doc)) + + +def _delete_search(tag: str, resource: str, name: str) -> None: + """Remove ``saved_searches...`` and prune empty parents.""" + from nsc.config.settings import default_paths # noqa: PLC0415 + from nsc.config.writer import ( # noqa: PLC0415 + acquire_lock, + atomic_write, + dump_round_trip, + load_round_trip, + unset_path, + ) + + path = default_paths().config_file + with acquire_lock(path): + doc = load_round_trip(path) + unset_path(doc, f"saved_searches.{tag}.{resource}.{name}") + atomic_write(path, dump_round_trip(doc)) + + def register(app: typer.Typer) -> None: def tui( ctx: typer.Context, @@ -54,12 +90,22 @@ def tui( from nsc.tui import run_tui # noqa: PLC0415 # deferred: keeps Textual lazy. column_prefs = {tag: dict(resources) for tag, resources in runtime.config.columns.items()} + saved_searches = { + tag: { + res: {name: dict(params) for name, params in sets.items()} + for res, sets in resources.items() + } + for tag, resources in runtime.config.saved_searches.items() + } run_tui( runtime.command_model, runtime.client, initial_resource=resource, save_columns=_save_columns, column_prefs=column_prefs, + saved_searches=saved_searches, + save_search=_save_search, + delete_search=_delete_search, ) # `tui` is canonical; `interactive` and `i` are hidden aliases for the same diff --git a/nsc/config/models.py b/nsc/config/models.py index e823efb..ee09156 100644 --- a/nsc/config/models.py +++ b/nsc/config/models.py @@ -68,3 +68,7 @@ class Config(_Frozen): profiles: dict[str, Profile] = Field(default_factory=dict) defaults: Defaults = Field(default_factory=Defaults) columns: dict[str, dict[str, list[str]]] = Field(default_factory=dict) + # Local, named filter sets keyed `.. = {filter_key: value}`. + # The leaf dict is exactly a `FilterState.as_params()` payload so a saved search + # round-trips through the TUI filter screen and the CLI `--saved` flag alike. + saved_searches: dict[str, dict[str, dict[str, dict[str, str]]]] = Field(default_factory=dict) diff --git a/nsc/config/saved_searches.py b/nsc/config/saved_searches.py new file mode 100644 index 0000000..cc94cca --- /dev/null +++ b/nsc/config/saved_searches.py @@ -0,0 +1,22 @@ +"""Framework-free lookups over the `saved_searches` config mapping. + +Keeps saved-search resolution out of `cli/` and `tui/` so both surfaces share +one implementation and the logic stays unit-testable without any I/O. +""" + +from __future__ import annotations + +from nsc.config.models import Config + + +def get_saved_search(config: Config, tag: str, resource: str, name: str) -> dict[str, str] | None: + """The stored filter params for `..`, or None if absent.""" + params = config.saved_searches.get(tag, {}).get(resource, {}).get(name) + if params is None: + return None + return dict(params) + + +def list_saved_searches(config: Config, tag: str, resource: str) -> list[str]: + """Sorted names of saved searches for `.` (empty if none).""" + return sorted(config.saved_searches.get(tag, {}).get(resource, {})) diff --git a/nsc/tui/__init__.py b/nsc/tui/__init__.py index 9b11c91..89588b6 100644 --- a/nsc/tui/__init__.py +++ b/nsc/tui/__init__.py @@ -10,6 +10,9 @@ __all__ = ["run_tui"] +SavedSearchMap = dict[str, dict[str, dict[str, dict[str, str]]]] + + def run_tui( model: CommandModel, client: Any, @@ -17,6 +20,9 @@ def run_tui( initial_resource: str | None = None, save_columns: Callable[[str, str, list[str]], None] | None = None, column_prefs: dict[str, dict[str, list[str]]] | None = None, + saved_searches: SavedSearchMap | None = None, + save_search: Callable[[str, str, str, dict[str, str]], None] | None = None, + delete_search: Callable[[str, str, str], None] | None = None, ) -> None: """Lazy entrypoint so importing `nsc.tui` never imports Textual eagerly.""" from nsc.tui.app import run_tui as _run # noqa: PLC0415 # deferred: keeps Textual lazy. @@ -27,4 +33,7 @@ def run_tui( initial_resource=initial_resource, save_columns=save_columns, column_prefs=column_prefs, + saved_searches=saved_searches, + save_search=save_search, + delete_search=delete_search, ) diff --git a/nsc/tui/app.py b/nsc/tui/app.py index 462ffb2..223bfc3 100644 --- a/nsc/tui/app.py +++ b/nsc/tui/app.py @@ -16,6 +16,8 @@ from nsc.tui.screens.picker import ResourcePicker from nsc.tui.widgets.help import HelpOverlay +SavedSearchMap = dict[str, dict[str, dict[str, dict[str, str]]]] + class NscTuiApp(App[None]): BINDINGS: ClassVar[list[BindingType]] = textual_bindings("global") @@ -32,6 +34,9 @@ def __init__( initial_resource: str | None = None, save_columns: Callable[[str, str, list[str]], None] | None = None, column_prefs: dict[str, dict[str, list[str]]] | None = None, + saved_searches: SavedSearchMap | None = None, + save_search: Callable[[str, str, str, dict[str, str]], None] | None = None, + delete_search: Callable[[str, str, str], None] | None = None, ) -> None: super().__init__() self._model = model @@ -39,6 +44,9 @@ def __init__( self._initial_resource = initial_resource self._save_columns = save_columns self._column_prefs = column_prefs or {} + self._saved_searches: SavedSearchMap = saved_searches or {} + self._save_search = save_search + self._delete_search = delete_search def columns_for(self, tag: str, resource: str) -> list[str] | None: """Saved visible columns for a resource, if any (read by ListScreen).""" @@ -51,6 +59,22 @@ def save_columns(self, tag: str, resource: str, columns: list[str]) -> None: if self._save_columns is not None: self._save_columns(tag, resource, columns) + def saved_searches_for(self, tag: str, resource: str) -> dict[str, dict[str, str]]: + """Named saved filter sets for a resource (read by FilterScreen).""" + return self._saved_searches.get(tag, {}).get(resource, {}) + + def save_search(self, tag: str, resource: str, name: str, params: dict[str, str]) -> None: + # Update the in-memory map too so the picker reflects the new entry + # immediately, without a relaunch. + self._saved_searches.setdefault(tag, {}).setdefault(resource, {})[name] = dict(params) + if self._save_search is not None: + self._save_search(tag, resource, name, params) + + def delete_search(self, tag: str, resource: str, name: str) -> None: + self._saved_searches.get(tag, {}).get(resource, {}).pop(name, None) + if self._delete_search is not None: + self._delete_search(tag, resource, name) + def on_mount(self) -> None: ref = self._resolve_initial() if ref is None: @@ -99,6 +123,9 @@ def run_tui( initial_resource: str | None = None, save_columns: Callable[[str, str, list[str]], None] | None = None, column_prefs: dict[str, dict[str, list[str]]] | None = None, + saved_searches: SavedSearchMap | None = None, + save_search: Callable[[str, str, str, dict[str, str]], None] | None = None, + delete_search: Callable[[str, str, str], None] | None = None, ) -> None: NscTuiApp( model, @@ -106,4 +133,7 @@ def run_tui( initial_resource=initial_resource, save_columns=save_columns, column_prefs=column_prefs, + saved_searches=saved_searches, + save_search=save_search, + delete_search=delete_search, ).run() diff --git a/nsc/tui/keymap.py b/nsc/tui/keymap.py index 85ec6c7..73d0245 100644 --- a/nsc/tui/keymap.py +++ b/nsc/tui/keymap.py @@ -8,7 +8,7 @@ from dataclasses import dataclass -Context = str # "global" | "list" | "detail" | "edit" | "bulk" +Context = str # "global" | "list" | "detail" | "edit" | "bulk" | "filter" # Textual key identifiers have no printable form; map them to the glyph the # user actually presses so footer and help do not show raw tokens. @@ -65,6 +65,9 @@ def _b(keys: str, action: str, description: str, context: Context, show: bool = _b("b", "go_back", "Back", "edit"), _b("p", "preview", "Preview changes", "bulk"), _b("b", "go_back", "Back", "bulk"), + _b("ctrl+s", "apply", "Apply", "filter"), + _b("ctrl+w", "save_search", "Save search", "filter"), + _b("ctrl+o", "load_search", "Load saved", "filter"), ) @@ -81,6 +84,7 @@ def help_groups() -> dict[Context, list[KeyBinding]]: "detail": [], "edit": [], "bulk": [], + "filter": [], } for b in KEYMAP: groups[b.context].append(b) diff --git a/nsc/tui/screens/filter.py b/nsc/tui/screens/filter.py index dd58134..9a08af2 100644 --- a/nsc/tui/screens/filter.py +++ b/nsc/tui/screens/filter.py @@ -34,6 +34,10 @@ class FilterScreen(ModalScreen[dict[str, str]]): Binding("down", "app.focus_next", "Next field", show=False), # ctrl+s (not ctrl+a, which inputs capture as cursor-home) applies. Binding("ctrl+s", "apply", "Apply"), + # ctrl+w saves the current state under a name; ctrl+o loads a saved one. + # Both avoid the printable keys the input-heavy form consumes. + Binding("ctrl+w", "save_search", "Save search"), + Binding("ctrl+o", "load_search", "Load saved"), ] def __init__( @@ -42,10 +46,15 @@ def __init__( client: Any, operation: Operation, current: dict[str, str], + *, + tag: str | None = None, + resource: str | None = None, ) -> None: super().__init__() self._model = model self._client = client + self._tag = tag + self._resource = resource self._common = common_filters(operation) self._searchable = searchable_filters(operation) self._fk_names = {p.name for p in self._common if self._is_fk(p.name)} @@ -80,7 +89,7 @@ def compose(self) -> ComposeResult: yield Button("Apply ⌃s", id="apply", variant="primary") yield Button("Clear", id="clear") yield Label( - "↓/Tab next field · Shift+Tab back · Enter pick · ⌃s apply · Esc cancel", + "↓/Tab next · Enter pick · ⌃s apply · ⌃w save · ⌃o load · Esc cancel", id="filter-hint", ) @@ -250,3 +259,44 @@ def action_clear(self) -> None: def action_cancel(self) -> None: self.dismiss(None) + + def action_save_search(self) -> None: + if self._tag is None or self._resource is None: + return + if not callable(getattr(self.app, "save_search", None)): + return + from nsc.tui.screens.saved_search_picker import SavedSearchNamePrompt # noqa: PLC0415 + + def _save(name: str | None) -> None: + if not name or not name.strip(): + return + self.app.save_search( # type: ignore[attr-defined] + self._tag, self._resource, name.strip(), self.state.as_params() + ) + + self.app.push_screen(SavedSearchNamePrompt(), _save) + + def action_load_search(self) -> None: + if self._tag is None or self._resource is None: + return + reader = getattr(self.app, "saved_searches_for", None) + if not callable(reader): + return + saved: dict[str, dict[str, str]] = reader(self._tag, self._resource) + if not saved: + self.notify("No saved searches for this resource yet.") + return + from nsc.tui.screens.saved_search_picker import SavedSearchPicker # noqa: PLC0415 + + def _load(name: str | None) -> None: + if name is None: + return + params = saved.get(name) + if params is None: + return + self.state = FilterState.from_params(params) + self._fk_display.clear() + self._sync_common_fields() + self._refresh_chips() + + self.app.push_screen(SavedSearchPicker(sorted(saved)), _load) diff --git a/nsc/tui/screens/list.py b/nsc/tui/screens/list.py index e01f8f5..c06ef2e 100644 --- a/nsc/tui/screens/list.py +++ b/nsc/tui/screens/list.py @@ -161,7 +161,15 @@ def _apply(result: dict[str, str] | None) -> None: self.apply_filters(result) self.app.push_screen( - FilterScreen(self._model, self._client, self._op, dict(self._extra_filters)), _apply + FilterScreen( + self._model, + self._client, + self._op, + dict(self._extra_filters), + tag=self._tag, + resource=self._resource_name, + ), + _apply, ) def apply_filters(self, params: dict[str, str]) -> None: diff --git a/nsc/tui/screens/saved_search_picker.py b/nsc/tui/screens/saved_search_picker.py new file mode 100644 index 0000000..ea7916b --- /dev/null +++ b/nsc/tui/screens/saved_search_picker.py @@ -0,0 +1,75 @@ +"""Modal screens for saving and loading local filter sets ("saved searches"). + +`SavedSearchNamePrompt` collects a name for the current filter state; it +dismisses with the typed string (blank means "no-op", handled by the caller). +`SavedSearchPicker` lists existing saved-search names and dismisses with the +chosen one (or None on cancel). +""" + +from __future__ import annotations + +from typing import ClassVar + +from textual.app import ComposeResult +from textual.binding import BindingType +from textual.containers import Vertical +from textual.screen import ModalScreen +from textual.widgets import Input, Label, ListItem, ListView + +_PROMPT_HINT = "Enter save · Esc cancel" +_PICKER_HINT = "Enter load · Esc cancel" + + +class SavedSearchNamePrompt(ModalScreen[str]): + BINDINGS: ClassVar[list[BindingType]] = [ + ("escape", "cancel", "Cancel"), + ] + + def compose(self) -> ComposeResult: + with Vertical(id="saved-name-box"): + yield Label("Save search as", classes="filter-heading") + yield Input(placeholder="name…", id="saved-name") + yield Label(_PROMPT_HINT, id="saved-name-hint") + + def on_mount(self) -> None: + self.query_one("#saved-name", Input).focus() + + def on_input_submitted(self, event: Input.Submitted) -> None: + self.dismiss(event.value) + + def action_cancel(self) -> None: + self.dismiss("") + + +class SavedSearchPicker(ModalScreen["str | None"]): + BINDINGS: ClassVar[list[BindingType]] = [ + ("escape", "cancel", "Cancel"), + ] + + def __init__(self, names: list[str]) -> None: + super().__init__() + self._names = names + + def compose(self) -> ComposeResult: + with Vertical(id="saved-picker-box"): + yield Label("Load saved search", classes="filter-heading") + yield ListView(id="saved-picker-list") + yield Label(_PICKER_HINT, id="saved-picker-hint") + + def on_mount(self) -> None: + listing = self.query_one("#saved-picker-list", ListView) + for name in self._names: + item = ListItem(Label(name)) + item.data = name # type: ignore[attr-defined] + listing.append(item) + if self._names: + listing.index = 0 + listing.focus() + + def on_list_view_selected(self, event: ListView.Selected) -> None: + name = getattr(event.item, "data", None) + if isinstance(name, str): + self.dismiss(name) + + def action_cancel(self) -> None: + self.dismiss(None) diff --git a/nsc/tui/styles.tcss b/nsc/tui/styles.tcss index 971623f..62012a1 100644 --- a/nsc/tui/styles.tcss +++ b/nsc/tui/styles.tcss @@ -147,6 +147,30 @@ ModalScreen > Vertical { color: $text-muted; } +/* ---- saved searches ---- */ +#saved-name-box { + height: auto; + border: round $panel; + padding: 1 2; +} + +#saved-picker-box { + height: 60%; + border: round $panel; + padding: 1 2; +} + +#saved-picker-list { + height: 1fr; + border: round $panel; +} + +#saved-name-hint, #saved-picker-hint { + width: 1fr; + text-align: center; + color: $text-muted; +} + /* ---- global search ---- */ #search-box { height: 85%; diff --git a/nsc/tui/widgets/help.py b/nsc/tui/widgets/help.py index 1a4253c..fbcb1fb 100644 --- a/nsc/tui/widgets/help.py +++ b/nsc/tui/widgets/help.py @@ -23,6 +23,7 @@ "detail": "Detail view", "edit": "Edit form", "bulk": "Bulk edit form", + "filter": "Filter builder", } _DISMISS_KEYS = {"escape", "q", "enter", "question_mark"} diff --git a/tests/cli/test_registration.py b/tests/cli/test_registration.py index 9e18f34..6c599e1 100644 --- a/tests/cli/test_registration.py +++ b/tests/cli/test_registration.py @@ -347,3 +347,77 @@ def test_runtime_context_carries_bulk_and_on_error_defaults() -> None: assert ctx.bulk is None assert ctx.no_bulk is None assert ctx.on_error == "stop" + + +def _ctx_with_saved(client: Any, saved: dict[str, Any]) -> RuntimeContext: + return RuntimeContext( + resolved_profile=ResolvedProfile( + name="prod", + url="https://nb.example/", + token="t", + verify_ssl=True, + timeout=5.0, + schema_url=None, + ), + config=Config(saved_searches={"dcim": {"devices": saved}}), + command_model=MagicMock(), + client=client, + output_format=OutputFormat.JSON, + fetch_all=True, + ) + + +def test_list_saved_resolves_into_filters() -> None: + app = typer.Typer() + client = MagicMock() + client.paginate.return_value = iter([]) + model = _model_with_list_op([]) + ctx = _ctx_with_saved(client, {"active-sw": {"status": "active", "role": "switch"}}) + register_dynamic_commands(app, model, lambda: ctx) + result = CliRunner().invoke(app, ["dcim", "devices", "list", "--saved", "active-sw"]) + assert result.exit_code == 0, result.output + client.paginate.assert_called_once_with( + "/api/dcim/devices/", {"status": "active", "role": "switch"} + ) + + +def test_explicit_filter_overrides_saved() -> None: + app = typer.Typer() + client = MagicMock() + client.paginate.return_value = iter([]) + model = _model_with_list_op([]) + ctx = _ctx_with_saved(client, {"active-sw": {"status": "active"}}) + register_dynamic_commands(app, model, lambda: ctx) + result = CliRunner().invoke( + app, ["dcim", "devices", "list", "--saved", "active-sw", "--filter", "status=offline"] + ) + assert result.exit_code == 0, result.output + client.paginate.assert_called_once_with("/api/dcim/devices/", {"status": "offline"}) + + +def test_explicit_typed_option_overrides_saved() -> None: + app = typer.Typer() + client = MagicMock() + client.paginate.return_value = iter([]) + model = _model_with_list_op( + [Parameter(name="status", location=ParameterLocation.QUERY, primitive=PrimitiveType.STRING)] + ) + ctx = _ctx_with_saved(client, {"active-sw": {"status": "active"}}) + register_dynamic_commands(app, model, lambda: ctx) + result = CliRunner().invoke( + app, ["dcim", "devices", "list", "--saved", "active-sw", "--status", "offline"] + ) + assert result.exit_code == 0, result.output + client.paginate.assert_called_once_with("/api/dcim/devices/", {"status": "offline"}) + + +def test_unknown_saved_name_exits_2() -> None: + app = typer.Typer() + client = MagicMock() + client.paginate.return_value = iter([]) + model = _model_with_list_op([]) + ctx = _ctx_with_saved(client, {"active-sw": {"status": "active"}}) + register_dynamic_commands(app, model, lambda: ctx) + result = CliRunner().invoke(app, ["dcim", "devices", "list", "--saved", "nope"]) + assert result.exit_code == 2 + client.paginate.assert_not_called() diff --git a/tests/cli/test_tui_command.py b/tests/cli/test_tui_command.py index d911e15..b4b5771 100644 --- a/tests/cli/test_tui_command.py +++ b/tests/cli/test_tui_command.py @@ -28,6 +28,7 @@ def _help_app() -> typer.Typer: class _FakeConfig: def __init__(self) -> None: self.columns: dict[str, dict[str, list[str]]] = {} + self.saved_searches: dict[str, dict[str, dict[str, dict[str, str]]]] = {} class _FakeRuntime: @@ -48,6 +49,9 @@ def _fake_run_tui( initial_resource: str | None = None, save_columns: Any = None, column_prefs: Any = None, + saved_searches: Any = None, + save_search: Any = None, + delete_search: Any = None, ) -> None: calls.append((model, client, initial_resource)) @@ -95,6 +99,46 @@ def config_file(self) -> Any: assert "name" in text +def test_save_search_persists_to_config(tmp_path: Any, monkeypatch: pytest.MonkeyPatch) -> None: + config_file = tmp_path / "config.yaml" + config_file.write_text("") + + class _Paths: + @property + def config_file(self) -> Any: + return config_file + + monkeypatch.setattr(settings, "default_paths", _Paths) + + tui_commands._save_search("dcim", "devices", "active-sw", {"status": "active"}) + + text = config_file.read_text() + assert "saved_searches:" in text + assert "active-sw" in text + assert "status" in text + assert "active" in text + + +def test_delete_search_removes_from_config(tmp_path: Any, monkeypatch: pytest.MonkeyPatch) -> None: + config_file = tmp_path / "config.yaml" + config_file.write_text("") + + class _Paths: + @property + def config_file(self) -> Any: + return config_file + + monkeypatch.setattr(settings, "default_paths", _Paths) + + tui_commands._save_search("dcim", "devices", "active-sw", {"status": "active"}) + tui_commands._delete_search("dcim", "devices", "active-sw") + + text = config_file.read_text() + assert "active-sw" not in text + # Pruning empty parents leaves no orphaned saved_searches tree. + assert "saved_searches" not in text + + def test_malformed_ctx_obj_exits_2() -> None: ctx = typer.Context(typer.main.get_command(_help_app())) ctx.obj = "not-a-tuple" diff --git a/tests/config/test_models.py b/tests/config/test_models.py index c61cc84..4b3af33 100644 --- a/tests/config/test_models.py +++ b/tests/config/test_models.py @@ -41,6 +41,24 @@ def test_config_holds_columns_per_tag_and_resource() -> None: assert c.columns["dcim"]["devices"] == ["id", "name", "site"] +def test_config_saved_searches_defaults_to_empty() -> None: + assert Config().saved_searches == {} + + +def test_config_holds_saved_searches_per_tag_resource_and_name() -> None: + c = Config.model_validate( + { + "saved_searches": { + "dcim": {"devices": {"active-sw": {"status": "active", "role": "switch"}}} + } + } + ) + assert c.saved_searches["dcim"]["devices"]["active-sw"] == { + "status": "active", + "role": "switch", + } + + def test_output_format_values() -> None: assert {f.value for f in OutputFormat} == {"table", "json", "jsonl", "yaml", "csv"} diff --git a/tests/config/test_saved_searches.py b/tests/config/test_saved_searches.py new file mode 100644 index 0000000..e417937 --- /dev/null +++ b/tests/config/test_saved_searches.py @@ -0,0 +1,54 @@ +from __future__ import annotations + +from nsc.config.models import Config +from nsc.config.saved_searches import get_saved_search, list_saved_searches + + +def _config() -> Config: + return Config.model_validate( + { + "saved_searches": { + "dcim": { + "devices": { + "active-sw": {"status": "active", "role": "switch"}, + "offline": {"status": "offline"}, + } + } + } + } + ) + + +def test_get_saved_search_hit() -> None: + assert get_saved_search(_config(), "dcim", "devices", "active-sw") == { + "status": "active", + "role": "switch", + } + + +def test_get_saved_search_unknown_name_returns_none() -> None: + assert get_saved_search(_config(), "dcim", "devices", "nope") is None + + +def test_get_saved_search_unknown_resource_returns_none() -> None: + assert get_saved_search(_config(), "dcim", "racks", "active-sw") is None + + +def test_get_saved_search_unknown_tag_returns_none() -> None: + assert get_saved_search(_config(), "ipam", "devices", "active-sw") is None + + +def test_get_saved_search_empty_config_returns_none() -> None: + assert get_saved_search(Config(), "dcim", "devices", "active-sw") is None + + +def test_list_saved_searches_returns_names_sorted() -> None: + assert list_saved_searches(_config(), "dcim", "devices") == ["active-sw", "offline"] + + +def test_list_saved_searches_unknown_resource_returns_empty() -> None: + assert list_saved_searches(_config(), "dcim", "racks") == [] + + +def test_list_saved_searches_empty_config_returns_empty() -> None: + assert list_saved_searches(Config(), "dcim", "devices") == [] diff --git a/tests/tui/test_app.py b/tests/tui/test_app.py index 154924a..8726ea5 100644 --- a/tests/tui/test_app.py +++ b/tests/tui/test_app.py @@ -66,6 +66,53 @@ async def test_save_columns_updates_in_memory_prefs() -> None: assert app.columns_for("dcim", "devices") == ["name", "id"] +@pytest.mark.asyncio +async def test_saved_searches_for_reads_memory() -> None: + app = NscTuiApp( + _model(), + _FakeClient(), + initial_resource="devices", + saved_searches={"dcim": {"devices": {"active-sw": {"status": "active"}}}}, + ) + async with app.run_test() as pilot: + await pilot.pause() + assert app.saved_searches_for("dcim", "devices") == {"active-sw": {"status": "active"}} + assert app.saved_searches_for("dcim", "racks") == {} + + +@pytest.mark.asyncio +async def test_save_search_updates_memory_and_callback() -> None: + calls: list[tuple[str, str, str, dict[str, str]]] = [] + app = NscTuiApp( + _model(), + _FakeClient(), + initial_resource="devices", + save_search=lambda t, r, n, p: calls.append((t, r, n, p)), + ) + async with app.run_test() as pilot: + await pilot.pause() + app.save_search("dcim", "devices", "active-sw", {"status": "active"}) + assert app.saved_searches_for("dcim", "devices") == {"active-sw": {"status": "active"}} + assert calls == [("dcim", "devices", "active-sw", {"status": "active"})] + + +@pytest.mark.asyncio +async def test_delete_search_updates_memory_and_callback() -> None: + calls: list[tuple[str, str, str]] = [] + app = NscTuiApp( + _model(), + _FakeClient(), + initial_resource="devices", + saved_searches={"dcim": {"devices": {"active-sw": {"status": "active"}}}}, + delete_search=lambda t, r, n: calls.append((t, r, n)), + ) + async with app.run_test() as pilot: + await pilot.pause() + app.delete_search("dcim", "devices", "active-sw") + assert app.saved_searches_for("dcim", "devices") == {} + assert calls == [("dcim", "devices", "active-sw")] + + @pytest.mark.asyncio async def test_ctrl_f_opens_global_search_from_any_screen() -> None: app = NscTuiApp(_model(), _FakeClient(), initial_resource="devices") diff --git a/tests/tui/test_filter_screen.py b/tests/tui/test_filter_screen.py index 4627283..e8d8d8b 100644 --- a/tests/tui/test_filter_screen.py +++ b/tests/tui/test_filter_screen.py @@ -55,10 +55,23 @@ def paginate( class _FilterApp(App[None]): - def __init__(self, current: dict[str, str] | None = None) -> None: + def __init__( + self, + current: dict[str, str] | None = None, + *, + saved: dict[str, dict[str, str]] | None = None, + ) -> None: super().__init__() self.result: dict[str, str] | None = None self._current = current or {} + self._saved = saved or {} + self.save_calls: list[tuple[str, str, str, dict[str, str]]] = [] + + def saved_searches_for(self, tag: str, resource: str) -> dict[str, dict[str, str]]: + return self._saved + + def save_search(self, tag: str, resource: str, name: str, params: dict[str, str]) -> None: + self.save_calls.append((tag, resource, name, params)) def compose(self) -> ComposeResult: yield Static("") @@ -67,7 +80,17 @@ async def on_mount(self) -> None: def _cb(result: dict[str, str] | None) -> None: self.result = result - await self.push_screen(FilterScreen(_model(), _FakeClient(), _op(), self._current), _cb) + await self.push_screen( + FilterScreen( + _model(), + _FakeClient(), + _op(), + self._current, + tag="dcim", + resource="devices", + ), + _cb, + ) @pytest.mark.asyncio @@ -371,3 +394,51 @@ async def test_fk_picker_stages_id_and_labels_chip_with_display() -> None: assert screen.state.as_params() == {"manufacturer_id": "8"} labels = {str(label.render()) for label in screen.query_one("#chips").query(Label)} assert "manufacturer_id = Cisco" in labels + + +@pytest.mark.asyncio +async def test_save_search_invokes_app_callback() -> None: + app = _FilterApp({"status": "active"}) + async with app.run_test() as pilot: + await pilot.pause() + screen = app.screen + assert isinstance(screen, FilterScreen) + screen.action_save_search() + await pilot.pause() + prompt = app.screen + prompt.dismiss("active-sw") + await pilot.pause() + assert app.save_calls == [("dcim", "devices", "active-sw", {"status": "active"})] + + +@pytest.mark.asyncio +async def test_save_search_blank_name_is_noop() -> None: + app = _FilterApp({"status": "active"}) + async with app.run_test() as pilot: + await pilot.pause() + screen = app.screen + assert isinstance(screen, FilterScreen) + screen.action_save_search() + await pilot.pause() + prompt = app.screen + prompt.dismiss(" ") + await pilot.pause() + assert app.save_calls == [] + + +@pytest.mark.asyncio +async def test_load_saved_search_repopulates_state_and_chips() -> None: + app = _FilterApp(saved={"active-sw": {"status": "active", "name": "sw1"}}) + async with app.run_test() as pilot: + await pilot.pause() + screen = app.screen + assert isinstance(screen, FilterScreen) + screen.action_load_search() + await pilot.pause() + picker = app.screen + picker.dismiss("active-sw") + await pilot.pause() + assert screen.state.as_params() == {"status": "active", "name": "sw1"} + labels = {str(label.render()) for label in screen.query_one("#chips").query(Label)} + assert "status = active" in labels + assert "name = sw1" in labels diff --git a/tests/tui/test_keymap.py b/tests/tui/test_keymap.py index 3d7cd34..74f685f 100644 --- a/tests/tui/test_keymap.py +++ b/tests/tui/test_keymap.py @@ -7,6 +7,7 @@ from nsc.tui.screens.bulk_edit_form import BulkEditForm from nsc.tui.screens.detail import DetailScreen from nsc.tui.screens.edit_form import EditForm +from nsc.tui.screens.filter import FilterScreen from nsc.tui.screens.list import ListScreen @@ -15,7 +16,7 @@ def test_every_binding_has_required_fields() -> None: assert b.keys, "binding must declare at least one key" assert b.action assert b.description - assert b.context in {"global", "list", "detail", "edit", "bulk"} + assert b.context in {"global", "list", "detail", "edit", "bulk", "filter"} def test_bindings_for_filters_by_context_and_includes_global() -> None: @@ -29,7 +30,7 @@ def test_bindings_for_filters_by_context_and_includes_global() -> None: def test_help_groups_are_keyed_by_context_and_nonempty() -> None: groups = help_groups() - assert set(groups) == {"global", "list", "detail", "edit", "bulk"} + assert set(groups) == {"global", "list", "detail", "edit", "bulk", "filter"} assert all(len(v) >= 1 for v in groups.values()) @@ -48,10 +49,11 @@ def test_keybinding_is_frozen_dataclass() -> None: "detail": (NscTuiApp, DetailScreen), "edit": (NscTuiApp, EditForm), "bulk": (NscTuiApp, BulkEditForm), + "filter": (NscTuiApp, FilterScreen), } -@pytest.mark.parametrize("context", ["global", "list", "detail", "edit", "bulk"]) +@pytest.mark.parametrize("context", ["global", "list", "detail", "edit", "bulk", "filter"]) def test_every_action_resolves_to_a_method(context: str) -> None: owners = _OWNER_FOR_CONTEXT[context] for b in bindings_for(context): @@ -61,7 +63,7 @@ def test_every_action_resolves_to_a_method(context: str) -> None: ) -@pytest.mark.parametrize("context", ["global", "list", "detail", "edit", "bulk"]) +@pytest.mark.parametrize("context", ["global", "list", "detail", "edit", "bulk", "filter"]) def test_no_key_maps_to_two_actions_in_a_context(context: str) -> None: seen: dict[str, str] = {} for b in bindings_for(context): From 1eed396e558b35a45f2ffbbb81835af5a2b51169 Mon Sep 17 00:00:00 2001 From: Thomas Christory Date: Fri, 26 Jun 2026 16:35:45 +0200 Subject: [PATCH 2/2] fix(tui): reject unsafe saved-search names and add delete affordance Saved searches persist at the dotted config path `saved_searches...`, and the writer splits on `.`. A name containing `.` (e.g. `prod.v2`) was written as a nested map instead of a leaf, so config.yaml then failed `Config.model_validate` on the next run, bricking every subsequent nsc invocation. Add a framework-free `validate_saved_search_name` that rejects names containing `.`, control characters, or surrounding whitespace. Enforce it at the point of entry (FilterScreen, with a user-facing error notification) and defensively in the `_save_search` persister before any disk write. Also wire a delete affordance into SavedSearchPicker: a `d` key binding dismisses with a typed `SavedSearchChoice("delete", name)` that FilterScreen routes to the existing `delete_search` plumbing, which previously had no UI path. Co-Authored-By: Claude Opus 4.8 (1M context) --- nsc/cli/tui_commands.py | 4 +++ nsc/config/saved_searches.py | 32 ++++++++++++++++++ nsc/tui/screens/filter.py | 35 +++++++++++++++---- nsc/tui/screens/saved_search_picker.py | 25 +++++++++++--- tests/cli/test_tui_command.py | 25 ++++++++++++++ tests/config/test_saved_searches.py | 34 ++++++++++++++++++- tests/tui/test_filter_screen.py | 47 +++++++++++++++++++++++++- 7 files changed, 190 insertions(+), 12 deletions(-) diff --git a/nsc/cli/tui_commands.py b/nsc/cli/tui_commands.py index beda360..2008656 100644 --- a/nsc/cli/tui_commands.py +++ b/nsc/cli/tui_commands.py @@ -43,6 +43,7 @@ def _save_columns(tag: str, resource: str, columns: list[str]) -> None: def _save_search(tag: str, resource: str, name: str, params: dict[str, str]) -> None: """Persist a filter set to ``saved_searches...``.""" + from nsc.config.saved_searches import validate_saved_search_name # noqa: PLC0415 from nsc.config.settings import default_paths # noqa: PLC0415 from nsc.config.writer import ( # noqa: PLC0415 acquire_lock, @@ -52,6 +53,9 @@ def _save_search(tag: str, resource: str, name: str, params: dict[str, str]) -> set_path, ) + # Defensive: a dotted name would split into a nested map under set_path and + # corrupt the file. Validate before touching disk so the file is untouched. + validate_saved_search_name(name) path = default_paths().config_file with acquire_lock(path): doc = load_round_trip(path) diff --git a/nsc/config/saved_searches.py b/nsc/config/saved_searches.py index cc94cca..c2080b5 100644 --- a/nsc/config/saved_searches.py +++ b/nsc/config/saved_searches.py @@ -8,6 +8,38 @@ from nsc.config.models import Config +_MIN_PRINTABLE = 0x20 + + +class InvalidSavedSearchName(ValueError): + """Raised when a saved-search name is unsafe to persist.""" + + +def validate_saved_search_name(name: str) -> None: + """Reject names that would corrupt config.yaml or surprise the user. + + Saved searches are persisted at the dotted config path + ``saved_searches...`` and the writer splits on ``.``, + so a name containing ``.`` would be written as a *nested map* rather than a + leaf — making the whole file fail ``Config.model_validate`` on the next run. + Control characters and surrounding whitespace are rejected for the same + "don't persist a name that won't round-trip cleanly" reason. + """ + if not name or name != name.strip(): + raise InvalidSavedSearchName( + "saved-search name must be non-empty and not start or end with whitespace" + ) + if "." in name: + raise InvalidSavedSearchName( + f"saved-search name {name!r} may not contain '.' " + "(it is used as a config-path separator)" + ) + if any((ch.isspace() and ch != " ") or ord(ch) < _MIN_PRINTABLE for ch in name): + raise InvalidSavedSearchName( + f"saved-search name {name!r} may not contain tabs, newlines, " + "or other control characters" + ) + def get_saved_search(config: Config, tag: str, resource: str, name: str) -> dict[str, str] | None: """The stored filter params for `..`, or None if absent.""" diff --git a/nsc/tui/screens/filter.py b/nsc/tui/screens/filter.py index 9a08af2..a7addde 100644 --- a/nsc/tui/screens/filter.py +++ b/nsc/tui/screens/filter.py @@ -265,13 +265,23 @@ def action_save_search(self) -> None: return if not callable(getattr(self.app, "save_search", None)): return + from nsc.config.saved_searches import ( # noqa: PLC0415 + InvalidSavedSearchName, + validate_saved_search_name, + ) from nsc.tui.screens.saved_search_picker import SavedSearchNamePrompt # noqa: PLC0415 def _save(name: str | None) -> None: if not name or not name.strip(): return + cleaned = name.strip() + try: + validate_saved_search_name(cleaned) + except InvalidSavedSearchName as exc: + self.notify(str(exc), severity="error") + return self.app.save_search( # type: ignore[attr-defined] - self._tag, self._resource, name.strip(), self.state.as_params() + self._tag, self._resource, cleaned, self.state.as_params() ) self.app.push_screen(SavedSearchNamePrompt(), _save) @@ -286,12 +296,18 @@ def action_load_search(self) -> None: if not saved: self.notify("No saved searches for this resource yet.") return - from nsc.tui.screens.saved_search_picker import SavedSearchPicker # noqa: PLC0415 + from nsc.tui.screens.saved_search_picker import ( # noqa: PLC0415 + SavedSearchChoice, + SavedSearchPicker, + ) - def _load(name: str | None) -> None: - if name is None: + def _chosen(choice: SavedSearchChoice | None) -> None: + if choice is None: + return + if choice.action == "delete": + self._delete_saved(choice.name) return - params = saved.get(name) + params = saved.get(choice.name) if params is None: return self.state = FilterState.from_params(params) @@ -299,4 +315,11 @@ def _load(name: str | None) -> None: self._sync_common_fields() self._refresh_chips() - self.app.push_screen(SavedSearchPicker(sorted(saved)), _load) + self.app.push_screen(SavedSearchPicker(sorted(saved)), _chosen) + + def _delete_saved(self, name: str) -> None: + deleter = getattr(self.app, "delete_search", None) + if not callable(deleter): + return + deleter(self._tag, self._resource, name) + self.notify(f"Deleted saved search {name!r}.") diff --git a/nsc/tui/screens/saved_search_picker.py b/nsc/tui/screens/saved_search_picker.py index ea7916b..b71d256 100644 --- a/nsc/tui/screens/saved_search_picker.py +++ b/nsc/tui/screens/saved_search_picker.py @@ -8,7 +8,8 @@ from __future__ import annotations -from typing import ClassVar +from dataclasses import dataclass +from typing import ClassVar, Literal from textual.app import ComposeResult from textual.binding import BindingType @@ -17,7 +18,15 @@ from textual.widgets import Input, Label, ListItem, ListView _PROMPT_HINT = "Enter save · Esc cancel" -_PICKER_HINT = "Enter load · Esc cancel" +_PICKER_HINT = "Enter load · d delete · Esc cancel" + + +@dataclass(frozen=True) +class SavedSearchChoice: + """A picker outcome: load or delete the named saved search.""" + + action: Literal["load", "delete"] + name: str class SavedSearchNamePrompt(ModalScreen[str]): @@ -41,9 +50,10 @@ def action_cancel(self) -> None: self.dismiss("") -class SavedSearchPicker(ModalScreen["str | None"]): +class SavedSearchPicker(ModalScreen["SavedSearchChoice | None"]): BINDINGS: ClassVar[list[BindingType]] = [ ("escape", "cancel", "Cancel"), + ("d", "delete", "Delete"), ] def __init__(self, names: list[str]) -> None: @@ -69,7 +79,14 @@ def on_mount(self) -> None: def on_list_view_selected(self, event: ListView.Selected) -> None: name = getattr(event.item, "data", None) if isinstance(name, str): - self.dismiss(name) + self.dismiss(SavedSearchChoice("load", name)) + + def action_delete(self) -> None: + listing = self.query_one("#saved-picker-list", ListView) + item = listing.highlighted_child + name = getattr(item, "data", None) + if isinstance(name, str): + self.dismiss(SavedSearchChoice("delete", name)) def action_cancel(self) -> None: self.dismiss(None) diff --git a/tests/cli/test_tui_command.py b/tests/cli/test_tui_command.py index b4b5771..783eced 100644 --- a/tests/cli/test_tui_command.py +++ b/tests/cli/test_tui_command.py @@ -11,6 +11,8 @@ from nsc.cli.app import app as real_app from nsc.cli.tui_commands import _runtime_from_ctx, register from nsc.config import settings +from nsc.config.loader import load_config +from nsc.config.saved_searches import InvalidSavedSearchName def test_tui_help_lists_resource_argument() -> None: @@ -139,6 +141,29 @@ def config_file(self) -> Any: assert "saved_searches" not in text +def test_save_search_dotted_name_does_not_corrupt_config( + tmp_path: Any, monkeypatch: pytest.MonkeyPatch +) -> None: + config_file = tmp_path / "config.yaml" + config_file.write_text("") + + class _Paths: + @property + def config_file(self) -> Any: + return config_file + + monkeypatch.setattr(settings, "default_paths", _Paths) + + # A dotted name must be rejected at persist time, not split into a nested + # map that later fails Config.model_validate and bricks every nsc run. + with pytest.raises(InvalidSavedSearchName): + tui_commands._save_search("dcim", "devices", "prod.v2", {"status": "active"}) + + # The config file is untouched and still loads cleanly. + assert config_file.read_text() == "" + load_config(config_file) + + def test_malformed_ctx_obj_exits_2() -> None: ctx = typer.Context(typer.main.get_command(_help_app())) ctx.obj = "not-a-tuple" diff --git a/tests/config/test_saved_searches.py b/tests/config/test_saved_searches.py index e417937..a9c8586 100644 --- a/tests/config/test_saved_searches.py +++ b/tests/config/test_saved_searches.py @@ -1,7 +1,14 @@ from __future__ import annotations +import pytest + from nsc.config.models import Config -from nsc.config.saved_searches import get_saved_search, list_saved_searches +from nsc.config.saved_searches import ( + InvalidSavedSearchName, + get_saved_search, + list_saved_searches, + validate_saved_search_name, +) def _config() -> Config: @@ -52,3 +59,28 @@ def test_list_saved_searches_unknown_resource_returns_empty() -> None: def test_list_saved_searches_empty_config_returns_empty() -> None: assert list_saved_searches(Config(), "dcim", "devices") == [] + + +@pytest.mark.parametrize( + "name", + [ + "prod.v2", # dotted: would split into a nested map under the writer. + ".", + "a.b.c", + "", + " ", # whitespace-only. + " leading", + "trailing ", + "with\ttab", + "with\nnewline", + "ctrl\x00null", + ], +) +def test_validate_saved_search_name_rejects_dangerous(name: str) -> None: + with pytest.raises(InvalidSavedSearchName): + validate_saved_search_name(name) + + +@pytest.mark.parametrize("name", ["active-sw", "prod_v2", "My Switches", "offline 2"]) +def test_validate_saved_search_name_accepts_safe(name: str) -> None: + validate_saved_search_name(name) diff --git a/tests/tui/test_filter_screen.py b/tests/tui/test_filter_screen.py index e8d8d8b..6e410ba 100644 --- a/tests/tui/test_filter_screen.py +++ b/tests/tui/test_filter_screen.py @@ -17,6 +17,7 @@ ) from nsc.tui.screens.filter import FilterScreen from nsc.tui.screens.record_picker import RecordPicker +from nsc.tui.screens.saved_search_picker import SavedSearchPicker def _op() -> Operation: @@ -66,6 +67,8 @@ def __init__( self._current = current or {} self._saved = saved or {} self.save_calls: list[tuple[str, str, str, dict[str, str]]] = [] + self.delete_calls: list[tuple[str, str, str]] = [] + self.notifications: list[str] = [] def saved_searches_for(self, tag: str, resource: str) -> dict[str, dict[str, str]]: return self._saved @@ -73,6 +76,13 @@ def saved_searches_for(self, tag: str, resource: str) -> dict[str, dict[str, str def save_search(self, tag: str, resource: str, name: str, params: dict[str, str]) -> None: self.save_calls.append((tag, resource, name, params)) + def delete_search(self, tag: str, resource: str, name: str) -> None: + self.delete_calls.append((tag, resource, name)) + self._saved.pop(name, None) + + def notify(self, message: str, **kwargs: Any) -> None: # type: ignore[override] + self.notifications.append(message) + def compose(self) -> ComposeResult: yield Static("") @@ -426,6 +436,23 @@ async def test_save_search_blank_name_is_noop() -> None: assert app.save_calls == [] +@pytest.mark.asyncio +async def test_save_search_dotted_name_rejected_and_notifies() -> None: + app = _FilterApp({"status": "active"}) + async with app.run_test() as pilot: + await pilot.pause() + screen = app.screen + assert isinstance(screen, FilterScreen) + screen.action_save_search() + await pilot.pause() + prompt = app.screen + # A dotted name would corrupt config.yaml via the dotted-path writer. + prompt.dismiss("prod.v2") + await pilot.pause() + assert app.save_calls == [] + assert app.notifications, "expected a user-facing rejection notice" + + @pytest.mark.asyncio async def test_load_saved_search_repopulates_state_and_chips() -> None: app = _FilterApp(saved={"active-sw": {"status": "active", "name": "sw1"}}) @@ -436,9 +463,27 @@ async def test_load_saved_search_repopulates_state_and_chips() -> None: screen.action_load_search() await pilot.pause() picker = app.screen - picker.dismiss("active-sw") + assert isinstance(picker, SavedSearchPicker) + await pilot.press("enter") await pilot.pause() assert screen.state.as_params() == {"status": "active", "name": "sw1"} labels = {str(label.render()) for label in screen.query_one("#chips").query(Label)} assert "status = active" in labels assert "name = sw1" in labels + + +@pytest.mark.asyncio +async def test_delete_saved_search_from_picker_invokes_app_callback() -> None: + app = _FilterApp(saved={"active-sw": {"status": "active"}, "offline": {"status": "offline"}}) + async with app.run_test() as pilot: + await pilot.pause() + screen = app.screen + assert isinstance(screen, FilterScreen) + screen.action_load_search() + await pilot.pause() + picker = app.screen + assert isinstance(picker, SavedSearchPicker) + # The delete key binding is the only way to remove a saved search. + await pilot.press("d") + await pilot.pause() + assert app.delete_calls == [("dcim", "devices", "active-sw")]