From f616af6beae4deea434e0a1c7812d5daed78f829 Mon Sep 17 00:00:00 2001 From: hedhoud <74668966+hedhoud@users.noreply.github.com> Date: Mon, 27 Jul 2026 10:43:33 +0200 Subject: [PATCH 01/10] Add user management search --- ui/src/components/shared/data-table.tsx | 4 +- ui/src/pages/admin/users/list.test.tsx | 94 +++++++++++++++++++++---- ui/src/pages/admin/users/list.tsx | 74 ++++++++++++++++++- 3 files changed, 156 insertions(+), 16 deletions(-) diff --git a/ui/src/components/shared/data-table.tsx b/ui/src/components/shared/data-table.tsx index 71fb6668f..abee87400 100644 --- a/ui/src/components/shared/data-table.tsx +++ b/ui/src/components/shared/data-table.tsx @@ -28,6 +28,7 @@ interface BaseDataTableProps { columns: ColumnDef[]; data: TData[]; pageSize?: number; + emptyMessage?: string; initialSorting?: SortingState; /** Render a leading checkbox column. */ enableSelection?: boolean; @@ -54,6 +55,7 @@ export function DataTable({ columns, data, pageSize = 10, + emptyMessage = "No results.", initialSorting = [], enableSelection = false, canSelectRow, @@ -177,7 +179,7 @@ export function DataTable({ colSpan={columnCount} className="h-24 text-center text-muted-foreground" > - No results. + {emptyMessage} )} diff --git a/ui/src/pages/admin/users/list.test.tsx b/ui/src/pages/admin/users/list.test.tsx index 757aa936a..957c58e0a 100644 --- a/ui/src/pages/admin/users/list.test.tsx +++ b/ui/src/pages/admin/users/list.test.tsx @@ -1,8 +1,10 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import { MemoryRouter } from "react-router-dom"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { listUsers } from "@/lib/api/users"; +import type { UserResponse } from "@/lib/api/users"; import UserListPage from "./list"; vi.mock("sonner", () => ({ @@ -28,6 +30,20 @@ vi.mock("@/lib/api/users", async () => { const listUsersMock = vi.mocked(listUsers); +function makeUser(overrides: Partial = {}): UserResponse { + return { + id: 2, + display_name: "Ada Lovelace", + external_user_id: "ada", + email: "ada@example.test", + is_admin: false, + file_quota: null, + file_count: 0, + created_at: null, + ...overrides, + }; +} + function renderUsers() { const queryClient = new QueryClient({ defaultOptions: { @@ -47,19 +63,9 @@ function renderUsers() { describe("UserListPage", () => { beforeEach(() => { + vi.clearAllMocks(); listUsersMock.mockResolvedValue({ - users: [ - { - id: 2, - display_name: "Ada Lovelace", - external_user_id: "ada", - email: "ada@example.test", - is_admin: false, - file_quota: null, - file_count: 0, - created_at: null, - }, - ], + users: [makeUser()], }); }); @@ -72,4 +78,68 @@ describe("UserListPage", () => { expect(view.getAttribute("data-size")).toBe("icon-xs"); expect(deleteAction.getAttribute("data-size")).toBe("icon-xs"); }); + + it("searches visible identifiers across paginated rows", async () => { + const users = Array.from({ length: 10 }, (_, index) => + makeUser({ + id: index + 2, + display_name: `User ${index + 2}`, + external_user_id: `subject-${index + 2}`, + email: `user-${index + 2}@example.test`, + }), + ); + users.push( + makeUser({ + id: 12, + display_name: "Zara Operator", + external_user_id: "oidc-zara", + email: "zara@example.test", + }), + ); + listUsersMock.mockResolvedValue({ users }); + + renderUsers(); + + const search = await screen.findByRole("searchbox", { name: "Search users" }); + expect(screen.queryByText("Zara Operator")).toBeNull(); + + await userEvent.type(search, "ZARA@EXAMPLE.TEST"); + + expect(await screen.findByText("Zara Operator")).toBeTruthy(); + expect(screen.getByRole("status").textContent).toBe("1 of 11 users"); + expect(listUsersMock).toHaveBeenCalledTimes(1); + + await userEvent.click(screen.getByRole("button", { name: "Clear user search" })); + await userEvent.type(search, "oidc-zara"); + + expect(await screen.findByText("Zara Operator")).toBeTruthy(); + + await userEvent.click(screen.getByRole("button", { name: "Clear user search" })); + await userEvent.type(search, "zArA operator"); + + expect(await screen.findByText("Zara Operator")).toBeTruthy(); + }); + + it("shows a clear no-result state and restores the list when search is cleared", async () => { + renderUsers(); + + const search = await screen.findByRole("searchbox", { name: "Search users" }); + await userEvent.type(search, "missing account"); + + expect(screen.getByText("No users match “missing account”.")).toBeTruthy(); + + await userEvent.click(screen.getByRole("button", { name: "Clear user search" })); + + expect(await screen.findByText("Ada Lovelace")).toBeTruthy(); + expect(screen.getByRole("status").textContent).toBe("1 user"); + }); + + it("distinguishes an empty directory from an unsuccessful search", async () => { + listUsersMock.mockResolvedValue({ users: [] }); + + renderUsers(); + + expect(await screen.findByText("No users have been created yet.")).toBeTruthy(); + expect(screen.getByRole("status").textContent).toBe("0 users"); + }); }); diff --git a/ui/src/pages/admin/users/list.tsx b/ui/src/pages/admin/users/list.tsx index 21651fb79..07908b7c3 100644 --- a/ui/src/pages/admin/users/list.tsx +++ b/ui/src/pages/admin/users/list.tsx @@ -1,9 +1,9 @@ -import { useState } from "react"; +import { useMemo, useState } from "react"; import { Link } from "react-router-dom"; import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; import { toast } from "sonner"; import type { ColumnDef } from "@tanstack/react-table"; -import { Trash2, Eye, Plus, Copy } from "lucide-react"; +import { Trash2, Eye, Plus, Copy, Search, X } from "lucide-react"; import { listUsers, deleteUser, createUser, effectiveQuota } from "@/lib/api/users"; import type { UserResponse, UserWithToken } from "@/lib/api/users"; import { getConfig } from "@/lib/api/system"; @@ -26,6 +26,8 @@ import { DialogFooter, } from "@/components/ui/dialog"; +const EMPTY_USERS: UserResponse[] = []; + // "indexed / effective-quota", e.g. "11 / 200". ∞ for unlimited; `over` flags // users at or past their cap (shown in red) so admins spot blocked uploaders. function formatUsage( @@ -44,6 +46,7 @@ function formatUsage( export default function UserListPage() { const queryClient = useQueryClient(); const [dialogOpen, setDialogOpen] = useState(false); + const [search, setSearch] = useState(""); const { data, isLoading } = useQuery({ queryKey: ["users"], @@ -55,6 +58,30 @@ export default function UserListPage() { const { data: config } = useQuery({ queryKey: ["system-config"], queryFn: getConfig }); const globalDefault = (config?.rdb as { default_file_quota?: number } | undefined)?.default_file_quota ?? null; + const users = data?.users ?? EMPTY_USERS; + const normalizedSearch = search.trim().toLowerCase(); + const filteredUsers = useMemo(() => { + if (!normalizedSearch) return users; + return users.filter((user) => + [ + user.display_name, + user.external_user_id, + user.email, + String(user.id), + `User #${user.id}`, + ] + .filter(Boolean) + .some((value) => String(value).toLowerCase().includes(normalizedSearch)), + ); + }, [normalizedSearch, users]); + const totalUsersLabel = `${users.length} ${users.length === 1 ? "user" : "users"}`; + const resultSummary = normalizedSearch + ? `${filteredUsers.length} of ${totalUsersLabel}` + : totalUsersLabel; + const emptyMessage = + users.length === 0 + ? "No users have been created yet." + : `No users match “${search.trim()}”.`; const deleteMut = useMutation({ mutationFn: deleteUser, @@ -167,7 +194,48 @@ export default function UserListPage() { {isLoading ? ( ) : ( - +
+
+
+ +
+

+ {resultSummary} +

+
+ +
)} From 653809f9212fe9ae8fa9b86d6ec67ddcc29f39c3 Mon Sep 17 00:00:00 2001 From: hedhoud <74668966+hedhoud@users.noreply.github.com> Date: Mon, 27 Jul 2026 11:26:07 +0200 Subject: [PATCH 02/10] Address user search review feedback --- openrag/services/persistence/user_repo.py | 1 + .../persistence/test_user_repo_external_id.py | 30 +++++++++++++++- ui/src/pages/admin/users/list.test.tsx | 20 +++++++++++ ui/src/pages/admin/users/list.tsx | 36 ++++++++++++++++--- 4 files changed, 81 insertions(+), 6 deletions(-) diff --git a/openrag/services/persistence/user_repo.py b/openrag/services/persistence/user_repo.py index 88206850b..b09c26998 100644 --- a/openrag/services/persistence/user_repo.py +++ b/openrag/services/persistence/user_repo.py @@ -332,6 +332,7 @@ async def list_users_dict(self) -> list[dict]: "id": r["id"], "display_name": r["display_name"], "external_user_id": r["external_user_id"], + "email": r["email"], "is_admin": r["is_admin"], "file_quota": r["file_quota"], "file_count": r["file_count"], diff --git a/tests/unit/services/persistence/test_user_repo_external_id.py b/tests/unit/services/persistence/test_user_repo_external_id.py index f582db6ef..f8cdae63c 100644 --- a/tests/unit/services/persistence/test_user_repo_external_id.py +++ b/tests/unit/services/persistence/test_user_repo_external_id.py @@ -30,10 +30,14 @@ def __init__(self): self.last_query: str | None = None self.last_params: tuple = () self._next_row: _FakeRow | None = None + self._rows: list[_FakeRow] = [] def set_next_row(self, **fields): self._next_row = _FakeRow(fields) + def set_rows(self, *rows: _FakeRow): + self._rows = list(rows) + async def fetchrow(self, query: str, *params): self.last_query = query self.last_params = params @@ -49,7 +53,7 @@ async def execute(self, query: str, *params): async def fetch(self, query: str, *params): self.last_query = query self.last_params = params - return [] + return self._rows def _make_user_with_ext(ext: str | None): @@ -141,3 +145,27 @@ async def test_create_legacy_user_coerces_empty_external_id_to_none(): ) # Same column position (display_name, external_user_id, ...) assert pool.last_params[1] is None + + +@pytest.mark.asyncio +async def test_list_users_dict_includes_email(): + from services.persistence.user_repo import PgUserRepository + + pool = _FakePool() + pool.set_rows( + _FakeRow( + id=42, + display_name="Alice", + external_user_id="kc-alice", + email="alice@example.com", + is_admin=False, + file_quota=None, + file_count=0, + created_at=__import__("datetime").datetime(2026, 1, 1), + ) + ) + repo = PgUserRepository(pool_getter=lambda: pool) + + users = await repo.list_users_dict() + + assert users[0]["email"] == "alice@example.com" diff --git a/ui/src/pages/admin/users/list.test.tsx b/ui/src/pages/admin/users/list.test.tsx index 957c58e0a..36a88e952 100644 --- a/ui/src/pages/admin/users/list.test.tsx +++ b/ui/src/pages/admin/users/list.test.tsx @@ -118,6 +118,12 @@ describe("UserListPage", () => { await userEvent.type(search, "zArA operator"); expect(await screen.findByText("Zara Operator")).toBeTruthy(); + + await userEvent.click(screen.getByRole("button", { name: "Clear user search" })); + await userEvent.type(search, "User #12"); + + expect(await screen.findByText("Zara Operator")).toBeTruthy(); + expect(screen.getByRole("status").textContent).toBe("1 of 11 users"); }); it("shows a clear no-result state and restores the list when search is cleared", async () => { @@ -142,4 +148,18 @@ describe("UserListPage", () => { expect(await screen.findByText("No users have been created yet.")).toBeTruthy(); expect(screen.getByRole("status").textContent).toBe("0 users"); }); + + it("shows a retryable error instead of reporting a failed request as an empty directory", async () => { + listUsersMock.mockRejectedValueOnce(new Error("User service unavailable")); + + renderUsers(); + + expect((await screen.findByRole("alert")).textContent).toContain("Users could not be loaded"); + expect(screen.getByRole("alert").textContent).toContain("User service unavailable"); + expect(screen.queryByText("No users have been created yet.")).toBeNull(); + + await userEvent.click(screen.getByRole("button", { name: "Try again" })); + + expect(await screen.findByText("Ada Lovelace")).toBeTruthy(); + }); }); diff --git a/ui/src/pages/admin/users/list.tsx b/ui/src/pages/admin/users/list.tsx index 07908b7c3..77aad4cee 100644 --- a/ui/src/pages/admin/users/list.tsx +++ b/ui/src/pages/admin/users/list.tsx @@ -3,7 +3,7 @@ import { Link } from "react-router-dom"; import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; import { toast } from "sonner"; import type { ColumnDef } from "@tanstack/react-table"; -import { Trash2, Eye, Plus, Copy, Search, X } from "lucide-react"; +import { Trash2, Eye, Plus, Copy, Search, X, AlertCircle, RefreshCw } from "lucide-react"; import { listUsers, deleteUser, createUser, effectiveQuota } from "@/lib/api/users"; import type { UserResponse, UserWithToken } from "@/lib/api/users"; import { getConfig } from "@/lib/api/system"; @@ -16,6 +16,7 @@ import { Skeleton } from "@/components/ui/skeleton"; import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { Switch } from "@/components/ui/switch"; +import { Alert, AlertDescription, AlertTitle } from "@/components/ui/alert"; import { copyToClipboard } from "@/lib/utils"; import { Dialog, @@ -48,7 +49,7 @@ export default function UserListPage() { const [dialogOpen, setDialogOpen] = useState(false); const [search, setSearch] = useState(""); - const { data, isLoading } = useQuery({ + const usersQuery = useQuery({ queryKey: ["users"], queryFn: listUsers, }); @@ -58,7 +59,7 @@ export default function UserListPage() { const { data: config } = useQuery({ queryKey: ["system-config"], queryFn: getConfig }); const globalDefault = (config?.rdb as { default_file_quota?: number } | undefined)?.default_file_quota ?? null; - const users = data?.users ?? EMPTY_USERS; + const users = usersQuery.data?.users ?? EMPTY_USERS; const normalizedSearch = search.trim().toLowerCase(); const filteredUsers = useMemo(() => { if (!normalizedSearch) return users; @@ -191,8 +192,33 @@ export default function UserListPage() { } /> - {isLoading ? ( + {usersQuery.isLoading ? ( + ) : usersQuery.isError ? ( + + ) : (
@@ -210,7 +236,7 @@ export default function UserListPage() { value={search} onChange={(event) => setSearch(event.target.value)} placeholder="Search by name, email or external ID" - className="pl-9 pr-10" + className="pl-9 pr-10 [&::-webkit-search-cancel-button]:hidden" /> {search && ( + + + ); +} + // "indexed / effective-quota", e.g. "11 / 200". ∞ for unlimited; `over` flags // users at or past their cap (shown in red) so admins spot blocked uploaders. function formatUsage( @@ -83,6 +115,10 @@ export default function UserListPage() { users.length === 0 ? "No users have been created yet." : `No users match “${search.trim()}”.`; + const usersErrorMessage = + usersQuery.error instanceof Error + ? usersQuery.error.message + : "The user directory request failed."; const deleteMut = useMutation({ mutationFn: deleteUser, @@ -194,33 +230,27 @@ export default function UserListPage() { {usersQuery.isLoading ? ( - ) : usersQuery.isError ? ( - - + ) : usersQuery.isLoadingError ? ( + { + usersQuery.refetch(); + }} + /> ) : (
+ {usersQuery.isRefetchError && ( + { + usersQuery.refetch(); + }} + /> + )}