From f61cd38993880e2d8ba996c78bacbd633d54670d Mon Sep 17 00:00:00 2001 From: yustme Date: Mon, 29 Jun 2026 18:45:25 +0200 Subject: [PATCH 1/3] feat(web): multi-select + bulk remove projects with a confirm modal Add checkbox selection to the Projects table (per-row + select-all with an indeterminate state) and a "Remove from kbagent" action that removes several projects at once. Replaces the native window.confirm() with a styled ConfirmModal that lists the affected aliases and spells out that this only unregisters them locally (edits kbagent config) and does NOT delete the Keboola projects. Backend: ProjectService.bulk_remove_projects (per-alias error accumulation + dry_run, local config.json only) and POST /projects/bulk-delete, declared before the /{alias} routes so the literal path is matched first. The single trash button now flows through the same modal + endpoint. Tests: service (partial failure, dry-run, dedup) + router (kwargs, dry_run forwarding, route-not-shadowed). --- .../server/routers/projects.py | 18 +++ .../services/project_service.py | 49 ++++++++ tests/test_server_router_calls.py | 70 ++++++++++++ tests/test_services.py | 75 ++++++++++++ web/frontend/src/components/ConfirmModal.tsx | 104 +++++++++++++++++ web/frontend/src/components/Table.tsx | 78 ++++++++++--- web/frontend/src/pages/Projects.tsx | 107 ++++++++++++++++-- 7 files changed, 476 insertions(+), 25 deletions(-) create mode 100644 web/frontend/src/components/ConfirmModal.tsx diff --git a/src/keboola_agent_cli/server/routers/projects.py b/src/keboola_agent_cli/server/routers/projects.py index c3b02651..154b90ab 100644 --- a/src/keboola_agent_cli/server/routers/projects.py +++ b/src/keboola_agent_cli/server/routers/projects.py @@ -29,6 +29,11 @@ class ProjectDescription(BaseModel): description: str +class ProjectBulkDelete(BaseModel): + aliases: list[str] + dry_run: bool = False + + @router.get("", summary="List registered projects") def list_projects(registry: ServiceRegistry = Depends(get_registry)) -> dict[str, Any]: """All registered project aliases.""" @@ -43,6 +48,19 @@ def add_project( return registry.project.add_project(body.alias, body.stack_url, body.token) +@router.post("/bulk-delete", summary="Remove multiple projects") +def bulk_delete_projects( + body: ProjectBulkDelete, registry: ServiceRegistry = Depends(get_registry) +) -> dict[str, Any]: + """Remove several projects at once, accumulating per-alias errors. + + Returns ``{removed, failed, dry_run}``; one bad alias does not block the + rest. Mirrors `kbagent project remove` applied to a list. Declared before + the ``/{alias}`` routes so the literal path is matched first. + """ + return registry.project.bulk_remove_projects(aliases=body.aliases, dry_run=body.dry_run) + + @router.delete("/{alias}", summary="Remove a project") def remove_project(alias: str, registry: ServiceRegistry = Depends(get_registry)) -> dict[str, Any]: """Remove a project by alias. Mirrors `kbagent project remove`.""" diff --git a/src/keboola_agent_cli/services/project_service.py b/src/keboola_agent_cli/services/project_service.py index 48353079..2c783883 100644 --- a/src/keboola_agent_cli/services/project_service.py +++ b/src/keboola_agent_cli/services/project_service.py @@ -97,6 +97,55 @@ def remove_project(self, alias: str) -> dict[str, str]: self._config_store.remove_project(alias) return {"alias": alias, "message": f"Project '{alias}' removed."} + def bulk_remove_projects( + self, + aliases: list[str], + dry_run: bool = False, + ) -> dict[str, Any]: + """Remove several projects in one call, accumulating per-alias errors. + + One failing alias (e.g. it does not exist, or is an ephemeral + ``__env__`` project that cannot be removed) does not stop the others -- + the failure is recorded under ``failed`` and the rest proceed. Like the + single-remove path this only edits ``config.json`` locally; no remote + API call is made. + + Args: + aliases: Project aliases to remove. Duplicates are de-duplicated + while preserving order. + dry_run: When True, validate each alias and report what WOULD be + removed without mutating ``config.json``. + + Returns: + ``{"removed": [...], "failed": [{"alias", "error"}], "dry_run": bool}`` + where ``removed`` lists the aliases removed (or that would be + removed in dry-run mode). + """ + seen: set[str] = set() + ordered: list[str] = [] + for alias in aliases: + if alias not in seen: + seen.add(alias) + ordered.append(alias) + + removed: list[str] = [] + failed: list[dict[str, str]] = [] + for alias in ordered: + try: + if dry_run: + # Validate existence (and the ephemeral guard) without + # mutating: a missing alias has no persisted project. + if self._config_store.get_project(alias) is None: + raise ConfigError(f"Project '{alias}' not found.") + removed.append(alias) + else: + self._config_store.remove_project(alias) + removed.append(alias) + except ConfigError as exc: + failed.append({"alias": alias, "error": exc.message}) + + return {"removed": removed, "failed": failed, "dry_run": dry_run} + def edit_project( self, alias: str, diff --git a/tests/test_server_router_calls.py b/tests/test_server_router_calls.py index 31c44745..6876bbf3 100644 --- a/tests/test_server_router_calls.py +++ b/tests/test_server_router_calls.py @@ -852,3 +852,73 @@ def test_data_app_runs_endpoint_calls_service(tmp_path: Path) -> None: assert res.status_code == 200, res.text data_app_svc.list_app_runs.assert_called_once_with(PROJECT, APP_ID, limit=3) + + +# --------------------------------------------------------------------------- +# projects.py POST /projects/bulk-delete +# Service: project.bulk_remove_projects(aliases=..., dry_run=...) +# --------------------------------------------------------------------------- + + +def test_bulk_delete_projects_passes_aliases(tmp_path: Path) -> None: + """POST /projects/bulk-delete must call ProjectService.bulk_remove_projects.""" + project_svc = MagicMock() + project_svc.bulk_remove_projects.return_value = { + "removed": ["a", "b"], + "failed": [], + "dry_run": False, + } + registry = _mock_registry(project=project_svc) + app = _make_app_with_registry(tmp_path, registry) + + with TestClient(app) as client: + res = client.post( + "/projects/bulk-delete", + headers=AUTH, + json={"aliases": ["a", "b"]}, + ) + + assert res.status_code == 200, res.text + assert res.json()["removed"] == ["a", "b"] + project_svc.bulk_remove_projects.assert_called_once_with(aliases=["a", "b"], dry_run=False) + + +def test_bulk_delete_projects_forwards_dry_run(tmp_path: Path) -> None: + """The dry_run flag in the body must reach the service.""" + project_svc = MagicMock() + project_svc.bulk_remove_projects.return_value = { + "removed": ["a"], + "failed": [], + "dry_run": True, + } + registry = _mock_registry(project=project_svc) + app = _make_app_with_registry(tmp_path, registry) + + with TestClient(app) as client: + res = client.post( + "/projects/bulk-delete", + headers=AUTH, + json={"aliases": ["a"], "dry_run": True}, + ) + + assert res.status_code == 200, res.text + project_svc.bulk_remove_projects.assert_called_once_with(aliases=["a"], dry_run=True) + + +def test_bulk_delete_route_not_shadowed_by_alias_delete(tmp_path: Path) -> None: + """The literal /projects/bulk-delete POST must not be swallowed by /{alias}. + + A DELETE /{alias} exists; the bulk route is a POST to a literal path, so it + must resolve to bulk_remove_projects, never remove_project('bulk-delete'). + """ + project_svc = MagicMock() + project_svc.bulk_remove_projects.return_value = {"removed": [], "failed": [], "dry_run": False} + registry = _mock_registry(project=project_svc) + app = _make_app_with_registry(tmp_path, registry) + + with TestClient(app) as client: + res = client.post("/projects/bulk-delete", headers=AUTH, json={"aliases": []}) + + assert res.status_code == 200, res.text + project_svc.bulk_remove_projects.assert_called_once() + project_svc.remove_project.assert_not_called() diff --git a/tests/test_services.py b/tests/test_services.py index fb76c71e..0c6851f3 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -183,6 +183,81 @@ def test_remove_nonexistent_raises_error(self, tmp_config_dir: Path) -> None: service.remove_project("nonexistent") +class TestBulkRemoveProjects: + """Tests for ProjectService.bulk_remove_projects().""" + + @staticmethod + def _service_with_projects(tmp_config_dir: Path, *aliases: str) -> ProjectService: + store = ConfigStore(config_dir=tmp_config_dir) + mock_client = make_mock_client() + service = ProjectService( + config_store=store, + client_factory=lambda url, token: mock_client, + ) + for alias in aliases: + service.add_project( + alias=alias, + stack_url="https://connection.keboola.com", + token="901-55555-fakeTestTokenDoNotUseXXXXXXXX", + ) + return service + + def test_removes_all_requested(self, tmp_config_dir: Path) -> None: + service = self._service_with_projects(tmp_config_dir, "a", "b", "c") + + result = service.bulk_remove_projects(["a", "c"]) + + assert result["removed"] == ["a", "c"] + assert result["failed"] == [] + assert result["dry_run"] is False + assert service._config_store.get_project("a") is None + assert service._config_store.get_project("c") is None + # Untouched alias survives. + assert service._config_store.get_project("b") is not None + + def test_partial_failure_accumulates(self, tmp_config_dir: Path) -> None: + """A missing alias is recorded under failed; the rest still go.""" + service = self._service_with_projects(tmp_config_dir, "a", "b") + + result = service.bulk_remove_projects(["a", "ghost", "b"]) + + assert result["removed"] == ["a", "b"] + assert len(result["failed"]) == 1 + assert result["failed"][0]["alias"] == "ghost" + assert "not found" in result["failed"][0]["error"].lower() + assert service._config_store.get_project("a") is None + assert service._config_store.get_project("b") is None + + def test_dry_run_does_not_mutate(self, tmp_config_dir: Path) -> None: + service = self._service_with_projects(tmp_config_dir, "a", "b") + + result = service.bulk_remove_projects(["a", "b"], dry_run=True) + + assert result["dry_run"] is True + assert result["removed"] == ["a", "b"] + assert result["failed"] == [] + # Nothing actually removed. + assert service._config_store.get_project("a") is not None + assert service._config_store.get_project("b") is not None + + def test_dry_run_reports_missing_as_failed(self, tmp_config_dir: Path) -> None: + service = self._service_with_projects(tmp_config_dir, "a") + + result = service.bulk_remove_projects(["a", "ghost"], dry_run=True) + + assert result["removed"] == ["a"] + assert [f["alias"] for f in result["failed"]] == ["ghost"] + assert service._config_store.get_project("a") is not None + + def test_duplicate_aliases_deduplicated(self, tmp_config_dir: Path) -> None: + service = self._service_with_projects(tmp_config_dir, "a") + + result = service.bulk_remove_projects(["a", "a"]) + + assert result["removed"] == ["a"] + assert result["failed"] == [] + + class TestEditProject: """Tests for ProjectService.edit_project().""" diff --git a/web/frontend/src/components/ConfirmModal.tsx b/web/frontend/src/components/ConfirmModal.tsx new file mode 100644 index 00000000..47c4d874 --- /dev/null +++ b/web/frontend/src/components/ConfirmModal.tsx @@ -0,0 +1,104 @@ +import { AlertTriangle, X } from "lucide-react"; +import { useEffect, useRef } from "react"; +import type { ReactNode } from "react"; + +/** + * Lightweight confirmation dialog matching the app's modal style (see + * ManageTokenModal). Replaces the native ``window.confirm()`` for actions that + * deserve a clearer, on-brand prompt. Esc or a backdrop click cancels; the + * confirm button is focused on open so Enter confirms. + */ +export function ConfirmModal({ + title, + body, + items, + confirmLabel = "Confirm", + cancelLabel = "Cancel", + danger = false, + busy = false, + onConfirm, + onCancel, +}: { + title: string; + body?: ReactNode; + /** Optional list rendered in a scrollable box (e.g. affected aliases). */ + items?: string[]; + confirmLabel?: string; + cancelLabel?: string; + danger?: boolean; + busy?: boolean; + onConfirm: () => void; + onCancel: () => void; +}) { + const confirmRef = useRef(null); + + useEffect(() => { + confirmRef.current?.focus(); + const onKey = (e: KeyboardEvent) => { + if (e.key === "Escape" && !busy) onCancel(); + }; + window.addEventListener("keydown", onKey); + return () => window.removeEventListener("keydown", onKey); + }, [busy, onCancel]); + + return ( +
{ + if (!busy) onCancel(); + }} + > +
e.stopPropagation()} + > +
+

+ {title} +

+ +
+ + {body ?
{body}
: null} + + {items && items.length > 0 ? ( +
    + {items.map((it) => ( +
  • + {it} +
    • + ))} +
+ ) : null} + +
+ + +
+
+
+ ); +} diff --git a/web/frontend/src/components/Table.tsx b/web/frontend/src/components/Table.tsx index caeb165d..398eba6f 100644 --- a/web/frontend/src/components/Table.tsx +++ b/web/frontend/src/components/Table.tsx @@ -1,4 +1,4 @@ -import type { ReactNode } from "react"; +import type { ChangeEvent, ReactNode } from "react"; export interface Column { header: string; @@ -13,23 +13,53 @@ export function DataTable({ rowKey, onRowClick, emptyMessage = "No data", + selectedKeys, + onToggleRow, + onToggleAll, }: { rows: T[]; columns: Column[]; rowKey: (row: T) => string; onRowClick?: (row: T) => void; emptyMessage?: string; + /** + * When provided together with `onToggleRow`, a leading checkbox column is + * rendered and rows whose key is in this set show as checked. Toggling a + * checkbox does not trigger `onRowClick`. + */ + selectedKeys?: Set; + onToggleRow?: (key: string, checked: boolean) => void; + onToggleAll?: (checked: boolean) => void; }) { + const selectable = !!selectedKeys && !!onToggleRow; + if (rows.length === 0) { return (
{emptyMessage}
); } + + const allSelected = selectable && rows.every((r) => selectedKeys!.has(rowKey(r))); + const someSelected = selectable && !allSelected && rows.some((r) => selectedKeys!.has(rowKey(r))); + return (
+ {selectable && ( + + )} {columns.map((col, i) => ( - {rows.map((row) => ( - onRowClick(row) : undefined} - className={`border-b border-zinc-100 dark:border-zinc-900/50 ${ - onRowClick ? "cursor-pointer hover:bg-zinc-100 dark:hover:bg-zinc-900/40" : "" - }`} - > - {columns.map((col, i) => ( - - ))} - - ))} + {rows.map((row) => { + const key = rowKey(row); + return ( + onRowClick(row) : undefined} + className={`border-b border-zinc-100 dark:border-zinc-900/50 ${ + onRowClick ? "cursor-pointer hover:bg-zinc-100 dark:hover:bg-zinc-900/40" : "" + }`} + > + {selectable && ( + + )} + {columns.map((col, i) => ( + + ))} + + ); + })}
+ { + if (el) el.indeterminate = someSelected; + }} + onChange={(e: ChangeEvent) => onToggleAll?.(e.target.checked)} + /> + ({
- {col.cell(row)} -
+ e.stopPropagation()} + onChange={(e: ChangeEvent) => + onToggleRow!(key, e.target.checked) + } + /> + + {col.cell(row)} +
diff --git a/web/frontend/src/pages/Projects.tsx b/web/frontend/src/pages/Projects.tsx index ad6f16b1..b43512c0 100644 --- a/web/frontend/src/pages/Projects.tsx +++ b/web/frontend/src/pages/Projects.tsx @@ -2,15 +2,25 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { CheckCircle2, Plus, RefreshCw, Trash2, XCircle } from "lucide-react"; import { useEffect, useState } from "react"; import { ApiError, api } from "../api/client"; +import { ConfirmModal } from "../components/ConfirmModal"; import { Empty, ErrorBox, Loading, PageTitle } from "../components/Empty"; import { JsonView } from "../components/JsonView"; import { DataTable } from "../components/Table"; import type { Project, ProjectStatus } from "../types"; +interface BulkDeleteResult { + removed: string[]; + failed: { alias: string; error: string }[]; + dry_run: boolean; +} + export function ProjectsPage() { const qc = useQueryClient(); const [showAdd, setShowAdd] = useState(false); const [selected, setSelected] = useState(null); + const [selectedAliases, setSelectedAliases] = useState>(new Set()); + // Aliases pending a remove-confirmation (single trash button or bulk action). + const [confirmAliases, setConfirmAliases] = useState(null); const projectsQ = useQuery<{ projects: Project[] }>({ queryKey: ["projects"], @@ -29,16 +39,59 @@ export function ProjectsPage() { if (statusQ.data) qc.invalidateQueries({ queryKey: ["projects"] }); }, [statusQ.data, qc]); - const removeMu = useMutation({ - mutationFn: (alias: string) => api.delete(`/projects/${encodeURIComponent(alias)}`), - onSuccess: () => qc.invalidateQueries({ queryKey: ["projects"] }), - }); - const useMu = useMutation({ mutationFn: (alias: string) => api.post(`/projects/use/${encodeURIComponent(alias)}`), onSuccess: () => qc.invalidateQueries({ queryKey: ["projects"] }), }); + const bulkDeleteMu = useMutation({ + mutationFn: (aliases: string[]) => + api.post("/projects/bulk-delete", { aliases }), + onSuccess: (res) => { + setSelectedAliases(new Set()); + qc.invalidateQueries({ queryKey: ["projects"] }); + if (res.failed.length > 0) { + const lines = res.failed.map((f) => `• ${f.alias}: ${f.error}`).join("\n"); + alert(`Removed ${res.removed.length}; ${res.failed.length} failed:\n${lines}`); + } + }, + }); + + const projects = projectsQ.data?.projects ?? []; + + // Keep the selection in sync with the live project list: drop any alias that + // no longer exists (e.g. removed in another tab) so stale keys never linger. + useEffect(() => { + setSelectedAliases((prev) => { + const live = new Set(projects.map((p) => p.alias)); + const next = new Set([...prev].filter((a) => live.has(a))); + return next.size === prev.size ? prev : next; + }); + }, [projects]); + + const toggleRow = (alias: string, checked: boolean) => + setSelectedAliases((prev) => { + const next = new Set(prev); + if (checked) next.add(alias); + else next.delete(alias); + return next; + }); + + const toggleAll = (checked: boolean) => + setSelectedAliases(checked ? new Set(projects.map((p) => p.alias)) : new Set()); + + const requestBulkRemove = () => { + if (selectedAliases.size === 0) return; + setConfirmAliases([...selectedAliases]); + }; + + const confirmRemove = () => { + if (!confirmAliases) return; + bulkDeleteMu.mutate(confirmAliases, { + onSettled: () => setConfirmAliases(null), + }); + }; + const statusByAlias = new Map(statusQ.data?.status?.map((s) => [s.alias, s]) ?? []); return ( @@ -48,6 +101,17 @@ export function ProjectsPage() { description="Keboola projects registered in this kbagent config." actions={ <> + {selectedAliases.size > 0 ? ( + + ) : null} + + ) : null} + {showAdd ? ( {