mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
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.
This commit is contained in:
Yuneng Jiang
parent
b13fabe9c2
commit
aebdba510f
2 changed files with 215 additions and 75 deletions
|
|
@ -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(<UserSearchModal isVisible onCancel={vi.fn()} onSubmit={onSubmit} accessToken="sk-test" />);
|
||||
return { user, onSubmit };
|
||||
};
|
||||
|
||||
const save = () => screen.getByRole("button", { name: /add member/i });
|
||||
|
||||
const searchByEmail = async (user: ReturnType<typeof userEvent.setup>, 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));
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<UserSearchModalProps> = ({
|
|||
defaultRole = "user",
|
||||
teamId,
|
||||
}) => {
|
||||
const [form] = Form.useForm<FormValues>();
|
||||
const emptyValues: FormValues = { user_email: undefined, user_id: undefined, role: defaultRole };
|
||||
const form = useForm<FormValues>({ defaultValues: emptyValues });
|
||||
const [userOptions, setUserOptions] = useState<UserOption[]>([]);
|
||||
const [loading, setLoading] = useState<boolean>(false);
|
||||
const [selectedField, setSelectedField] = useState<"user_email" | "user_id">("user_email");
|
||||
|
|
@ -104,13 +121,11 @@ const UserSearchModal: React.FC<UserSearchModalProps> = ({
|
|||
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<void> => {
|
||||
|
|
@ -123,81 +138,125 @@ const UserSearchModal: React.FC<UserSearchModalProps> = ({
|
|||
};
|
||||
|
||||
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 (
|
||||
<div data-testid={testId}>
|
||||
<Combobox
|
||||
items={items}
|
||||
value={selected}
|
||||
filter={null}
|
||||
onValueChange={(option: UserOption | null) => {
|
||||
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}
|
||||
>
|
||||
<ComboboxInput
|
||||
id={controlProps.id}
|
||||
placeholder={placeholder}
|
||||
showClear={selected !== null}
|
||||
onKeyDown={swallowEnter}
|
||||
/>
|
||||
<ComboboxContent>
|
||||
<ComboboxEmpty>{loading ? "Loading..." : "No results"}</ComboboxEmpty>
|
||||
<ComboboxList>
|
||||
{(option: UserOption) => (
|
||||
<ComboboxItem key={option.value} value={option}>
|
||||
{option.label}
|
||||
</ComboboxItem>
|
||||
)}
|
||||
</ComboboxList>
|
||||
</ComboboxContent>
|
||||
</Combobox>
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
return (
|
||||
<Modal title={title} open={isVisible} onCancel={handleClose} footer={null} width={800} maskClosable={!isSubmitting}>
|
||||
<Form<FormValues>
|
||||
form={form}
|
||||
onFinish={handleSubmit}
|
||||
labelCol={{ span: 8 }}
|
||||
wrapperCol={{ span: 16 }}
|
||||
labelAlign="left"
|
||||
initialValues={{
|
||||
role: defaultRole,
|
||||
}}
|
||||
>
|
||||
<Alert
|
||||
type="info"
|
||||
showIcon
|
||||
className="mb-4"
|
||||
message="Search selects from users that already exist. To add someone new, ask a proxy admin to create their account first."
|
||||
data-testid="member-existing-users-notice"
|
||||
/>
|
||||
|
||||
<Form.Item label="Email" name="user_email" className="mb-4">
|
||||
<Select
|
||||
showSearch
|
||||
className="w-full"
|
||||
placeholder="Search by email"
|
||||
filterOption={false}
|
||||
onSearch={(value) => handleSearch(value, "user_email")}
|
||||
onSelect={(value, option) => handleSelect(value, option as UserOption)}
|
||||
options={selectedField === "user_email" ? userOptions : []}
|
||||
loading={loading}
|
||||
allowClear
|
||||
data-testid="member-email-search"
|
||||
<TooltipProvider>
|
||||
<form onSubmit={form.handleSubmit(handleSubmit)} noValidate>
|
||||
<Alert
|
||||
type="info"
|
||||
showIcon
|
||||
className="mb-4"
|
||||
message="Search selects from users that already exist. To add someone new, ask a proxy admin to create their account first."
|
||||
data-testid="member-existing-users-notice"
|
||||
/>
|
||||
</Form.Item>
|
||||
|
||||
<div className="text-center mb-4">OR</div>
|
||||
<FieldGroup>
|
||||
<FormField control={form.control} name="user_email" label="Email">
|
||||
{({ id, value, onChange }) =>
|
||||
renderUserSearch("user_email", "Search by email", { id, value, onChange }, "member-email-search")
|
||||
}
|
||||
</FormField>
|
||||
|
||||
<Form.Item label="User ID" name="user_id" className="mb-4">
|
||||
<Select
|
||||
showSearch
|
||||
className="w-full"
|
||||
placeholder="Search by user ID"
|
||||
filterOption={false}
|
||||
onSearch={(value) => handleSearch(value, "user_id")}
|
||||
onSelect={(value, option) => handleSelect(value, option as UserOption)}
|
||||
options={selectedField === "user_id" ? userOptions : []}
|
||||
loading={loading}
|
||||
allowClear
|
||||
/>
|
||||
</Form.Item>
|
||||
<div className="text-center">OR</div>
|
||||
|
||||
<Form.Item label="Member Role" name="role" className="mb-4">
|
||||
<Select defaultValue={defaultRole}>
|
||||
{roles.map((role) => (
|
||||
<Select.Option key={role.value} value={role.value}>
|
||||
<Tooltip title={role.description}>
|
||||
<span className="font-medium">{role.label}</span>
|
||||
<span className="ml-2 text-gray-500 text-sm">- {role.description}</span>
|
||||
</Tooltip>
|
||||
</Select.Option>
|
||||
))}
|
||||
</Select>
|
||||
</Form.Item>
|
||||
<FormField control={form.control} name="user_id" label="User ID">
|
||||
{({ id, value, onChange }) => renderUserSearch("user_id", "Search by user ID", { id, value, onChange })}
|
||||
</FormField>
|
||||
|
||||
<div className="text-right mt-4">
|
||||
<Button type="primary" htmlType="submit" icon={<UserAddOutlined />} loading={isSubmitting}>
|
||||
{isSubmitting ? "Adding..." : "Add Member"}
|
||||
</Button>
|
||||
</div>
|
||||
</Form>
|
||||
<FormField control={form.control} name="role" label="Member Role">
|
||||
{({ id, value, onChange }) => (
|
||||
<Select items={roles} value={value} onValueChange={(next) => onChange(next as string)}>
|
||||
<SelectTrigger id={id}>
|
||||
<SelectValue />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
{roles.map((role) => (
|
||||
<SelectItem key={role.value} value={role.value}>
|
||||
<Tooltip>
|
||||
<TooltipTrigger
|
||||
render={
|
||||
<span>
|
||||
<span className="font-medium">{role.label}</span>
|
||||
<span className="ml-2 text-sm text-muted-foreground">- {role.description}</span>
|
||||
</span>
|
||||
}
|
||||
/>
|
||||
<TooltipContent>{role.description}</TooltipContent>
|
||||
</Tooltip>
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
)}
|
||||
</FormField>
|
||||
</FieldGroup>
|
||||
|
||||
<div className="mt-4 text-right">
|
||||
<Button type="submit" disabled={isSubmitting}>
|
||||
{isSubmitting ? <UiLoadingSpinner className="size-4" /> : <UserAddOutlined />}
|
||||
{isSubmitting ? "Adding..." : "Add Member"}
|
||||
</Button>
|
||||
</div>
|
||||
</form>
|
||||
</TooltipProvider>
|
||||
</Modal>
|
||||
);
|
||||
};
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue