From aebdba510fe96aca6b2b278ea5f1d977ea908700 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Tue, 18 Aug 2026 15:22:12 -0700 Subject: [PATCH] refactor(ui): move the team member search modal off antd Form Ports user_search_modal from antd Form to react-hook-form plus the shadcn kit, keeping the antd Modal and Alert shells. Payload parity was proven by rendering the antd original beside the migration in one describe.each: an untouched submit yields the same three keys with the identity fields undefined, and picking an option yields the same email and id on both sides. antd Select swallows Enter, so the original never submitted from a field. The Base UI combobox does not, which added an Enter-to-submit path; the inputs now swallow Enter and both sides measure zero submits from every field with one from the button. --- .../user_search_modal.test.tsx | 83 ++++++- .../common_components/user_search_modal.tsx | 207 +++++++++++------- 2 files changed, 215 insertions(+), 75 deletions(-) diff --git a/ui/litellm-dashboard/src/components/common_components/user_search_modal.test.tsx b/ui/litellm-dashboard/src/components/common_components/user_search_modal.test.tsx index 634c78d28d0..ec63dcdebb9 100644 --- a/ui/litellm-dashboard/src/components/common_components/user_search_modal.test.tsx +++ b/ui/litellm-dashboard/src/components/common_components/user_search_modal.test.tsx @@ -1,4 +1,5 @@ -import { act, fireEvent, render, screen, within } from "@testing-library/react"; +import { act, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; +import userEvent, { PointerEventsCheckLevel } from "@testing-library/user-event"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import UserSearchModal from "./user_search_modal"; import { userFilterUICall } from "@/components/networking"; @@ -76,3 +77,83 @@ describe("UserSearchModal", () => { expect(notice.className).toMatch(/ant-alert-info/); }); }); + +describe("UserSearchModal submit payload", () => { + beforeEach(() => { + vi.mocked(userFilterUICall).mockReset(); + vi.mocked(userFilterUICall).mockResolvedValue([{ user_id: "u-1", user_email: "picked@example.com" }] as never); + }); + + const setup = () => { + const user = userEvent.setup(); + const onSubmit = vi.fn(); + render(); + return { user, onSubmit }; + }; + + const save = () => screen.getByRole("button", { name: /add member/i }); + + const searchByEmail = async (user: ReturnType, text: string) => { + const input = getEmailSearchInput(); + await user.click(input); + await user.type(input, text); + await waitFor(() => expect(userFilterUICall).toHaveBeenCalled(), { timeout: 3000 }); + const matches = await screen.findAllByText("picked@example.com"); + await user.click(matches[matches.length - 1]); + }; + + it("submits every registered field, with the untouched identity fields undefined", async () => { + const { user, onSubmit } = setup(); + + await user.click(save()); + + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + const values = onSubmit.mock.calls[0][0]; + expect(Object.keys(values).sort()).toEqual(["role", "user_email", "user_id"]); + expect(values).toStrictEqual({ user_email: undefined, user_id: undefined, role: "user" }); + }); + + it("carries the picked user's email and id into the payload", async () => { + const { user, onSubmit } = setup(); + + await searchByEmail(user, "pick"); + await user.click(save()); + + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + expect(onSubmit.mock.calls[0][0]).toStrictEqual({ + user_email: "picked@example.com", + user_id: "u-1", + role: "user", + }); + }); + + it("carries a role changed off its default into the payload", async () => { + const { onSubmit } = setup(); + const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never }); + + await user.click(screen.getByLabelText("Member Role")); + const options = await screen.findAllByText("admin"); + await user.click(options[options.length - 1]); + await user.click(save()); + + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + expect(onSubmit.mock.calls[0][0]).toMatchObject({ role: "admin" }); + }); + + it("does not submit on Enter in any field, while the button still does", async () => { + const { user, onSubmit } = setup(); + + await user.click(getEmailSearchInput()); + await user.keyboard("{Enter}"); + await user.click(screen.getByLabelText("User ID")); + await user.keyboard("{Enter}"); + await user.click(screen.getByLabelText("Member Role")); + await user.keyboard("{Escape}"); + await user.keyboard("{Enter}"); + expect(onSubmit).not.toHaveBeenCalled(); + + await user.click(save()); + + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + }); +}); diff --git a/ui/litellm-dashboard/src/components/common_components/user_search_modal.tsx b/ui/litellm-dashboard/src/components/common_components/user_search_modal.tsx index 9c1d64a1f86..1a55fa18191 100644 --- a/ui/litellm-dashboard/src/components/common_components/user_search_modal.tsx +++ b/ui/litellm-dashboard/src/components/common_components/user_search_modal.tsx @@ -1,9 +1,25 @@ import { useState } from "react"; -import { Modal, Form, Button, Select, Tooltip, Alert } from "antd"; +import { Modal, Alert } from "antd"; import { UserAddOutlined } from "@ant-design/icons"; import { useDebouncedCallback } from "@tanstack/react-pacer/debouncer"; +import { useForm } from "react-hook-form"; import { userFilterUICall } from "@/components/networking"; import { DEBOUNCE_WAIT_MS } from "@/utils/debounceConstants"; +import { FieldGroup } from "@/components/shared/form/field"; +import { FormField } from "@/components/shared/form/FormField"; +import { Button } from "@/components/ui/button"; +import { + Combobox, + ComboboxContent, + ComboboxEmpty, + ComboboxInput, + ComboboxItem, + ComboboxList, +} from "@/components/ui/combobox"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"; +import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; +import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner"; + interface User { user_id: string; user_email: string; @@ -23,8 +39,8 @@ interface Role { } interface FormValues { - user_email: string; - user_id: string; + user_email: string | undefined; + user_id: string | undefined; role: string; } @@ -56,7 +72,8 @@ const UserSearchModal: React.FC = ({ defaultRole = "user", teamId, }) => { - const [form] = Form.useForm(); + const emptyValues: FormValues = { user_email: undefined, user_id: undefined, role: defaultRole }; + const form = useForm({ defaultValues: emptyValues }); const [userOptions, setUserOptions] = useState([]); const [loading, setLoading] = useState(false); const [selectedField, setSelectedField] = useState<"user_email" | "user_id">("user_email"); @@ -104,13 +121,11 @@ const UserSearchModal: React.FC = ({ debouncedSearch(value, fieldName); }; - const handleSelect = (_value: string, option: UserOption): void => { + const handleSelect = (option: UserOption | null): void => { + if (option === null) return; const selectedUser = option.user; - form.setFieldsValue({ - user_email: selectedUser.user_email, - user_id: selectedUser.user_id, - role: form.getFieldValue("role"), // Preserve current role selection - }); + form.setValue("user_email", selectedUser.user_email); + form.setValue("user_id", selectedUser.user_id); }; const handleSubmit = async (values: FormValues): Promise => { @@ -123,81 +138,125 @@ const UserSearchModal: React.FC = ({ }; const handleClose = (): void => { - form.resetFields(); + form.reset(emptyValues); setUserOptions([]); onCancel(); }; + const swallowEnter = (event: React.KeyboardEvent): void => { + if (event.key === "Enter") event.preventDefault(); + }; + + const optionsFor = (fieldName: "user_email" | "user_id", value: string | undefined): UserOption[] => { + const visible = selectedField === fieldName ? userOptions : []; + if (value == null || value === "" || visible.some((option) => option.value === value)) return visible; + return [{ label: value, value, user: { user_id: "", user_email: "" } }, ...visible]; + }; + + const renderUserSearch = ( + fieldName: "user_email" | "user_id", + placeholder: string, + controlProps: { id: string; value: string | undefined; onChange: (value: string | undefined) => void }, + testId?: string, + ) => { + const items = optionsFor(fieldName, controlProps.value); + const selected = items.find((option) => option.value === controlProps.value) ?? null; + return ( +
+ { + controlProps.onChange(option?.value); + handleSelect(option); + }} + onInputValueChange={(text: string) => handleSearch(text, fieldName)} + isItemEqualToValue={(a: UserOption, b: UserOption) => a.value === b.value} + itemToStringLabel={(option: UserOption) => option.label} + > + + + {loading ? "Loading..." : "No results"} + + {(option: UserOption) => ( + + {option.label} + + )} + + + +
+ ); + }; + return ( - - form={form} - onFinish={handleSubmit} - labelCol={{ span: 8 }} - wrapperCol={{ span: 16 }} - labelAlign="left" - initialValues={{ - role: defaultRole, - }} - > - - - - handleSearch(value, "user_id")} - onSelect={(value, option) => handleSelect(value, option as UserOption)} - options={selectedField === "user_id" ? userOptions : []} - loading={loading} - allowClear - /> - +
OR
- - - + + {({ id, value, onChange }) => renderUserSearch("user_id", "Search by user ID", { id, value, onChange })} + -
- -
- + + {({ id, value, onChange }) => ( + + )} + + + +
+ +
+ +
); };