From 2ee15a6efb2a7b42134180bc9d57de28621fc2f5 Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Tue, 18 Aug 2026 11:42:16 -0700 Subject: [PATCH] refactor(ui): migrate user, policy, and margin forms to shadcn (#37305) * refactor(ui): migrate user, policy, and margin forms to shadcn Move four dashboard forms off antd Form and Tremor onto the shadcn field kit, with react-hook-form where the form owns its own submit. Submit payloads are unchanged: edit_user still emits exactly six keys with spend as a number and max_budget as a string, policy_test_panel still omits empty context keys, and add_attachment_form still builds the same attachment body. add_margin_form had no Form of its own and no bound field names, so its Form.Item rules were inert; it keeps its parent-owned state props and only swaps the presentation. Adds characterization tests for edit_user and policy_test_panel, both proven green against the antd originals before the migration, plus a case pinning the provider value add_margin_form reports upward. The existing add_attachment_form and add_margin_form suites pass unedited. Extracts TokenSelect for the tag and alias inputs shared across the policy forms, keeping antd's token separators and blur-commit behavior. * refactor(ui): seed the user edit form by remount instead of an effect Key the form on the edited user so react-hook-form seeds from its defaults on each user, replacing the effect that reset the form and the exhaustive-deps suppression that came with it. Cancel and submit still reset, so reopening the same user shows stored values. * test(ui): pin that the user edit form forwards null fields unchanged The proxy returns null rather than omitting unset fields, and antd forwarded whatever it received. The only fixture seeded every field, so nothing proved the migrated form still emits null instead of an empty string. Proven against the antd original first, and it fails if toFormValues coerces. --- ui/litellm-dashboard/eslint-suppressions.json | 9 +- .../_components/add_margin_form.test.tsx | 14 +- .../_components/add_margin_form.tsx | 309 ++++++------ .../policies/_components/TokenSelect.tsx | 132 ++++++ .../_components/add_attachment_form.tsx | 446 +++++++++++------- .../_components/policy_test_panel.test.tsx | 149 ++++++ .../_components/policy_test_panel.tsx | 230 ++++++--- .../users/_components/edit_user.test.tsx | 216 +++++++++ .../users/_components/edit_user.tsx | 237 +++++++--- 9 files changed, 1251 insertions(+), 491 deletions(-) create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/policies/_components/TokenSelect.tsx create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/policies/_components/policy_test_panel.test.tsx create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/users/_components/edit_user.test.tsx diff --git a/ui/litellm-dashboard/eslint-suppressions.json b/ui/litellm-dashboard/eslint-suppressions.json index abc83120e16..4aace020dda 100644 --- a/ui/litellm-dashboard/eslint-suppressions.json +++ b/ui/litellm-dashboard/eslint-suppressions.json @@ -199,9 +199,6 @@ "src/app/(dashboard)/cost-tracking/_components/add_margin_form.tsx": { "local/filename-pascal-case": { "count": 1 - }, - "no-restricted-imports": { - "count": 2 } }, "src/app/(dashboard)/cost-tracking/_components/add_provider_form.tsx": { @@ -1096,7 +1093,7 @@ "count": 1 }, "no-restricted-imports": { - "count": 2 + "count": 1 }, "react-hooks/immutability": { "count": 1 @@ -1210,7 +1207,7 @@ "count": 1 }, "no-restricted-imports": { - "count": 2 + "count": 1 }, "react-hooks/immutability": { "count": 1 @@ -1493,7 +1490,7 @@ "count": 1 }, "no-restricted-imports": { - "count": 2 + "count": 1 } }, "src/app/(dashboard)/users/_components/index.tsx": { diff --git a/ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/add_margin_form.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/add_margin_form.test.tsx index 63a0b3c355f..dc2e79d3199 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/add_margin_form.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/add_margin_form.test.tsx @@ -1,7 +1,7 @@ import React from "react"; import { describe, it, expect, vi, beforeEach } from "vitest"; import { screen } from "@testing-library/react"; -import userEvent from "@testing-library/user-event"; +import userEvent, { PointerEventsCheckLevel } from "@testing-library/user-event"; import { renderWithProviders } from "../../../../../tests/test-utils"; import AddMarginForm from "./add_margin_form"; import { MarginConfig } from "./types"; @@ -103,4 +103,16 @@ describe("AddMarginForm", () => { await user.click(screen.getByText("Fixed Amount")); expect(onMarginTypeChange).toHaveBeenCalledWith("fixed"); }); + + it("should call onProviderChange with the provider key when a provider is picked", async () => { + const onProviderChange = vi.fn(); + const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never }); + renderWithProviders(); + + await user.click(screen.getByRole("combobox")); + await user.click(await screen.findByText("Anthropic")); + + expect(onProviderChange.mock.calls).toHaveLength(1); + expect(onProviderChange.mock.calls[0]?.[0]).toBe("Anthropic"); + }); }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/add_margin_form.tsx b/ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/add_margin_form.tsx index a17b7fc4ac3..7fc1559d683 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/add_margin_form.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/add_margin_form.tsx @@ -1,9 +1,20 @@ import React from "react"; -import { TextInput, Button } from "@tremor/react"; -import { Select as AntdSelect, Form, Tooltip, Radio } from "antd"; -import { InfoCircleOutlined } from "@ant-design/icons"; +import { CircleHelp } from "lucide-react"; import { Providers, provider_map } from "@/components/provider_info_helpers"; import { Logo } from "@/components/molecules/logo/Logo"; +import { Field, FieldLabel, FieldTitle } from "@/components/shared/form/field"; +import { Button } from "@/components/ui/button"; +import { + Combobox, + ComboboxContent, + ComboboxEmpty, + ComboboxInput, + ComboboxItem, + ComboboxList, +} from "@/components/ui/combobox"; +import { Input } from "@/components/ui/input"; +import { RadioGroup, RadioGroupItem } from "@/components/ui/radio-group"; +import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; import { MarginConfig } from "./types"; interface AddMarginFormProps { @@ -19,6 +30,39 @@ interface AddMarginFormProps { onAddProvider: () => void; } +interface ProviderOption { + value: string; + label: string; + providerEnum: string | null; +} + +const GLOBAL_OPTION: ProviderOption = { + value: "global", + label: "Global (All Providers)", + providerEnum: null, +}; + +const buildProviderOptions = (marginConfig: MarginConfig): ProviderOption[] => [ + GLOBAL_OPTION, + ...Object.entries(Providers).flatMap(([providerEnum, providerDisplayName]) => { + const providerValue = provider_map[providerEnum as keyof typeof provider_map]; + if (providerValue && marginConfig[providerValue]) { + return []; + } + return [{ value: providerEnum, label: providerDisplayName, providerEnum }]; + }), +]; + +const labelWithHint = (label: string, hint: string): React.ReactNode => ( + <> + {label} + + } /> + {hint} + + +); + const AddMarginForm: React.FC = ({ marginConfig, selectedProvider, @@ -31,163 +75,116 @@ const AddMarginForm: React.FC = ({ onFixedAmountChange, onAddProvider, }) => { + const providerOptions = buildProviderOptions(marginConfig); + const selectedOption = providerOptions.find((option) => option.value === selectedProvider) ?? null; + return ( -
- - Provider - - - - - } - rules={[{ required: true, message: "Please select a provider" }]} - > - - String(option?.label ?? "") - .toLowerCase() - .includes(input.toLowerCase()) - } - > - -
- Global (All Providers) + +
+ + + {labelWithHint( + "Provider", + "Select 'Global' to apply margin to all providers, or select a specific provider", + )} + + onProviderChange(option?.value)} + itemToStringLabel={(option: ProviderOption) => option.label} + isItemEqualToValue={(option: ProviderOption, selected: ProviderOption) => option.value === selected.value} + > + + + No matching providers + + {(option: ProviderOption) => ( + + + {option.providerEnum !== null && ( + + )} + {option.label} + + + )} + + + + + + + + {labelWithHint("Margin Type", "Choose how to apply the margin: percentage-based or fixed amount")} + + onMarginTypeChange(value as "percentage" | "fixed")} + className="w-full" + > + + + Percentage-based + + + + Fixed Amount + + + + + {marginType === "percentage" && ( + + + {labelWithHint("Margin Percentage", "Enter a percentage value (e.g., 10 for 10% margin)")} + +
+ onPercentageChange(event.target.value)} + className="rounded-lg flex-1" + /> + %
- - {Object.entries(Providers).map(([providerEnum, providerDisplayName]) => { - const providerValue = provider_map[providerEnum as keyof typeof provider_map]; - // Only show providers that don't already have a margin configured - if (providerValue && marginConfig[providerValue]) { - return null; + + )} + + {marginType === "fixed" && ( + + + {labelWithHint("Fixed Margin Amount", "Enter a fixed amount in USD (e.g., 0.001 for $0.001 per request)")} + +
+ $ + onFixedAmountChange(event.target.value)} + className="rounded-lg flex-1" + /> +
+ + )} + +
+ + > + Add Provider Margin + +
-
+ ); }; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/policies/_components/TokenSelect.tsx b/ui/litellm-dashboard/src/app/(dashboard)/policies/_components/TokenSelect.tsx new file mode 100644 index 00000000000..fb1418d02b9 --- /dev/null +++ b/ui/litellm-dashboard/src/app/(dashboard)/policies/_components/TokenSelect.tsx @@ -0,0 +1,132 @@ +"use client"; + +import * as React from "react"; + +import { + Combobox, + ComboboxChip, + ComboboxChips, + ComboboxChipsInput, + ComboboxContent, + ComboboxEmpty, + ComboboxItem, + ComboboxList, + ComboboxValue, + useComboboxAnchor, +} from "@/components/ui/combobox"; + +interface TokenSelectProps { + id: string; + value: readonly string[] | undefined; + onValueChange: (value: string[]) => void; + onBlur?: () => void; + placeholder: string; + options?: readonly string[]; + allowCustomValues?: boolean; + tokenSeparators?: readonly string[]; + emptyText?: string; + ariaInvalid?: true; + ariaDescribedBy?: string; +} + +const splitOnSeparators = (text: string, separators: readonly string[]): string[] => + separators.reduce((parts, separator) => parts.flatMap((part) => part.split(separator)), [text]); + +const withAdditions = (current: readonly string[], additions: readonly string[]): string[] => [ + ...current, + ...additions.filter((addition) => addition !== "" && !current.includes(addition)), +]; + +export const includesQuery = (item: string, query: string): boolean => item.toLowerCase().includes(query.toLowerCase()); + +export const TokenSelect: React.FC = ({ + id, + value, + onValueChange, + onBlur, + placeholder, + options, + allowCustomValues = false, + tokenSeparators = [], + emptyText = "No options found", + ariaInvalid, + ariaDescribedBy, +}) => { + const anchor = useComboboxAnchor(); + const [query, setQuery] = React.useState(""); + const selected = value ?? []; + const showDropdown = options !== undefined; + + const pendingCustomValue = allowCustomValues && query.trim() !== "" && !options?.includes(query.trim()); + const items = pendingCustomValue ? [...(options ?? []), query.trim()] : options ?? []; + + const handleInputValueChange = (next: string) => { + if (!allowCustomValues || !tokenSeparators.some((separator) => next.includes(separator))) { + setQuery(next); + return; + } + const parts = splitOnSeparators(next, tokenSeparators); + const committed = parts.slice(0, -1).map((part) => part.trim()); + onValueChange(withAdditions(selected, committed)); + setQuery(parts[parts.length - 1]); + }; + + const handleBlur = () => { + const pending = query.trim(); + if (allowCustomValues && pending !== "") { + onValueChange(withAdditions(selected, [pending])); + } + setQuery(""); + onBlur?.(); + }; + + return ( + { + onValueChange(next); + setQuery(""); + }} + inputValue={query} + onInputValueChange={handleInputValueChange} + filter={includesQuery} + > + }> + + {(chips: string[]) => ( + <> + {chips.map((chip) => ( + + {chip} + + ))} + + + )} + + + {showDropdown && ( + + {emptyText} + + {(item: string) => ( + + {item} + + )} + + + )} + + ); +}; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/policies/_components/add_attachment_form.tsx b/ui/litellm-dashboard/src/app/(dashboard)/policies/_components/add_attachment_form.tsx index 73b58be1179..cdb8c7ffab9 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/policies/_components/add_attachment_form.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/policies/_components/add_attachment_form.tsx @@ -1,15 +1,23 @@ import React, { useState, useEffect } from "react"; -import { Modal, Form, Select, Radio, Divider, Typography } from "antd"; -import { Button } from "@tremor/react"; +import { Modal } from "antd"; +import { CircleHelp } from "lucide-react"; +import { z } from "zod/v4"; import { Policy } from "@/components/policies/types"; import { teamListCall, keyListCall, modelAvailableCall, estimateAttachmentImpactCall } from "@/components/networking"; import { toast } from "@/lib/toast"; import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; +import { FieldGroup, FieldLabel, FieldTitle } from "@/components/shared/form/field"; +import { FormField } from "@/components/shared/form/FormField"; +import { Button } from "@/components/ui/button"; +import { RadioGroup, RadioGroupItem } from "@/components/ui/radio-group"; +import { Separator } from "@/components/ui/separator"; +import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; +import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner"; +import { useZodForm } from "@/lib/forms/useZodForm"; import { buildAttachmentData } from "./build_attachment_data"; import { getInvalidTeamEntries } from "./scope_validation"; import ImpactPreviewAlert from "./impact_preview_alert"; - -const { Text } = Typography; +import { TokenSelect } from "./TokenSelect"; interface AddAttachmentFormProps { visible: boolean; @@ -20,6 +28,60 @@ interface AddAttachmentFormProps { createAttachment: (accessToken: string, attachmentData: any) => Promise; } +type ScopeType = "global" | "specific"; + +interface AttachmentFormValues { + policy_names: string[]; + teams: string[]; + keys: string[]; + models: string[]; + tags: string[]; +} + +const EMPTY_VALUES: AttachmentFormValues = { + policy_names: [], + teams: [], + keys: [], + models: [], + tags: [], +}; + +const attachmentShape = { + policy_names: z.array(z.string()).min(1, "Please select at least one policy"), + teams: z.array(z.string()), + keys: z.array(z.string()), + models: z.array(z.string()), + tags: z.array(z.string()), +}; + +const buildAttachmentSchema = (scopeType: ScopeType, teamsLoaded: boolean, availableTeams: string[]) => + z.object(attachmentShape).superRefine((values, ctx) => { + if (scopeType !== "specific" || !teamsLoaded) { + return; + } + const invalid = getInvalidTeamEntries(values.teams, availableTeams); + if (invalid.length === 0) { + return; + } + ctx.addIssue({ + code: "custom", + path: ["teams"], + message: + `These teams don't exist: ${invalid.join(", ")}. ` + + `Choose an existing team, or use a wildcard like "team-*" to match by prefix.`, + }); + }); + +const labelWithHint = (label: string, hint: string): React.ReactNode => ( + <> + {label} + + } /> + {hint} + + +); + const AddAttachmentForm: React.FC = ({ visible, onClose, @@ -28,9 +90,8 @@ const AddAttachmentForm: React.FC = ({ policies, createAttachment, }) => { - const [form] = Form.useForm(); const [isSubmitting, setIsSubmitting] = useState(false); - const [scopeType, setScopeType] = useState<"global" | "specific">("global"); + const [scopeType, setScopeType] = useState("global"); const [availableTeams, setAvailableTeams] = useState([]); const [teamsLoaded, setTeamsLoaded] = useState(false); const [availableKeys, setAvailableKeys] = useState([]); @@ -41,6 +102,9 @@ const AddAttachmentForm: React.FC = ({ const [isEstimating, setIsEstimating] = useState(false); const [impactResult, setImpactResult] = useState(null); const { userId, userRole } = useAuthorized(); + const form = useZodForm(buildAttachmentSchema(scopeType, teamsLoaded, availableTeams), { + defaultValues: EMPTY_VALUES, + }); useEffect(() => { if (visible && accessToken) { @@ -95,30 +159,22 @@ const AddAttachmentForm: React.FC = ({ }; const resetForm = () => { - form.resetFields(); + form.reset(EMPTY_VALUES); setScopeType("global"); setImpactResult(null); }; const handlePreviewImpact = async () => { if (!accessToken) return; - try { - await form.validateFields(["policy_names"]); - } catch { + if (!(await form.trigger("policy_names"))) { return; } setIsEstimating(true); try { - const { policy_names = [] } = form.getFieldsValue(true); - const firstPolicy = policy_names?.[0]; + const values = form.getValues(); + const firstPolicy = values.policy_names[0]; if (!firstPolicy) return; - const data = buildAttachmentData( - { - ...form.getFieldsValue(true), - policy_name: firstPolicy, - }, - scopeType, - ); + const data = buildAttachmentData({ ...values, policy_name: firstPolicy }, scopeType); const result = await estimateAttachmentImpactCall(accessToken, data); setImpactResult(result); } catch (error) { @@ -133,27 +189,17 @@ const AddAttachmentForm: React.FC = ({ onClose(); }; - const handleSubmit = async () => { + const handleSubmit = async (values: AttachmentFormValues) => { try { setIsSubmitting(true); - await form.validateFields(); if (!accessToken) { throw new Error("No access token available"); } - const values = form.getFieldsValue(true); - const selectedPolicyNames: string[] = values.policy_names || []; - const results = await Promise.allSettled( - selectedPolicyNames.map((policyName) => { - const data = buildAttachmentData( - { - ...values, - policy_name: policyName, - }, - scopeType, - ); + values.policy_names.map((policyName) => { + const data = buildAttachmentData({ ...values, policy_name: policyName }, scopeType); return createAttachment(accessToken, data); }), ); @@ -182,165 +228,207 @@ const AddAttachmentForm: React.FC = ({ } }; - const policyOptions = policies.map((p) => ({ - label: p.policy_name, - value: p.policy_name, - })); + const policyOptions = policies.map((p) => p.policy_name); return ( -
- - ({ - label: team, - value: team, - }))} - tokenSeparators={[","]} - showSearch - filterOption={(input, option) => (option?.label ?? "").toLowerCase().includes(input.toLowerCase())} - style={{ width: "100%" }} - /> - + {scopeType === "specific" && ( + <> + + {({ + id, + value, + onChange, + onBlur, + "aria-invalid": ariaInvalid, + "aria-describedby": ariaDescribedBy, + }) => ( + + )} + - - ({ - label: model, - value: model, - }))} - tokenSeparators={[","]} - showSearch - filterOption={(input, option) => (option?.label ?? "").toLowerCase().includes(input.toLowerCase())} - style={{ width: "100%" }} - /> - + + {({ + id, + value, + onChange, + onBlur, + "aria-invalid": ariaInvalid, + "aria-describedby": ariaDescribedBy, + }) => ( + + )} + - - Matches tags from key/team metadata.tags or tags passed dynamically in the request body. - Use * as a suffix wildcard (e.g., prod-* matches prod-us,{" "} - prod-eu). - - } - > - ({ label: t, value: t }))} - filterOption={(input, option) => (option?.label ?? "").toLowerCase().includes(input.toLowerCase())} - /> - - - ({ label: m, value: m }))} - filterOption={(input, option) => (option?.label ?? "").toLowerCase().includes(input.toLowerCase())} - /> - - - } + - + + {({ id, value, onChange, "aria-invalid": ariaInvalid, "aria-describedby": ariaDescribedBy }) => ( + + )} + - - - {possibleUIRoles && - Object.entries(possibleUIRoles).map(([role, { ui_label, description }]) => ( - -
- {ui_label}{" "} -

- {description} -

-
- - ))} - - + + {({ ref, value, onChange, onBlur, ...field }) => ( + onChange(event.target.value === "" ? null : event.target.valueAsNumber)} + onBlur={() => { + onBlur(); + clampSpendToMinimum(); + }} + /> + )} + - - - + + {({ ref: _ref, value, ...field }) => ( + + )} + - - - + + {({ id, value, onChange }) => } + + - - - - -
- Save +
+
-
- Save +
+
- - + + ); }; +const EditUserModal: React.FC = ({ user, ...props }) => { + if (!user) { + return null; + } + + return ; +}; + export default EditUserModal;