From e20e31e985b8133cf618c9e4abb7704105d1f7bb Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Tue, 18 Aug 2026 11:42:58 -0700 Subject: [PATCH] refactor(ui): migrate CloudZero and cost tracking forms to react-hook-form and shadcn (#37312) * refactor(ui): migrate CloudZero and cost tracking forms to react-hook-form and shadcn Moves four forms off antd Form onto react-hook-form with the shadcn field primitives, keeping the request payloads byte identical. The two CloudZero modals were near duplicates, so the payload builder and the API key input now live in shared modules next to them. The Update modal keeps its redaction behaviour: the key field arrives empty with a "leave empty to keep existing" hint, and an untouched save omits api_key from the request so the stored secret survives. There is a test that fails if that regresses. add_provider_form had no Form instance of its own, and its Form.Item wrappers carried no name, so nothing was registered in the parent store. The parent in cost_tracking_settings still owns an antd Form element, which stays for now, and the migrated button keeps type="submit" so the parent's onFinish path behaves exactly as before. Each unit got characterization tests written against the antd version first, then re-run unedited against the migration. Payload parity was also checked side by side with toStrictEqual across seven scenarios. * test(ui): guard the CloudZero null connection id against a zod type error The proxy returns connection_id as null rather than omitting it, and a plain z.string() rejects null. The seeding coalesces it to "" so an untouched save reports the friendly required message instead of "expected string, received null". Dropping that coalesce fails this test. --- ui/litellm-dashboard/eslint-suppressions.json | 8 - .../PromptCompressionTab.integration.test.tsx | 127 ++++++++++++ .../_components/PromptCompressionTab.tsx | 142 +++++++++----- .../add_provider_form.integration.test.tsx | 68 +++++++ .../_components/add_provider_form.tsx | 184 +++++++++++------- .../CloudZeroCreateModal.integration.test.tsx | 95 +++++++++ .../CloudZeroCreateModal.tsx | 114 ++++++----- .../CloudZeroFormControls.tsx | 41 ++++ .../CloudZeroUpdateModal.integration.test.tsx | 139 +++++++++++++ .../CloudZeroUpdateModal.tsx | 121 ++++++------ .../cloudZeroPayload.test.ts | 43 ++++ .../CloudZeroCostTracking/cloudZeroPayload.ts | 23 +++ 12 files changed, 851 insertions(+), 254 deletions(-) create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/cost-optimization/_components/PromptCompressionTab.integration.test.tsx create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/add_provider_form.integration.test.tsx create mode 100644 ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroCreateModal.integration.test.tsx create mode 100644 ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroFormControls.tsx create mode 100644 ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroUpdateModal.integration.test.tsx create mode 100644 ui/litellm-dashboard/src/components/CloudZeroCostTracking/cloudZeroPayload.test.ts create mode 100644 ui/litellm-dashboard/src/components/CloudZeroCostTracking/cloudZeroPayload.ts diff --git a/ui/litellm-dashboard/eslint-suppressions.json b/ui/litellm-dashboard/eslint-suppressions.json index 64615c50cc1..0555884026c 100644 --- a/ui/litellm-dashboard/eslint-suppressions.json +++ b/ui/litellm-dashboard/eslint-suppressions.json @@ -191,11 +191,6 @@ "count": 1 } }, - "src/app/(dashboard)/cost-optimization/_components/PromptCompressionTab.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "src/app/(dashboard)/cost-tracking/_components/add_margin_form.tsx": { "local/filename-pascal-case": { "count": 1 @@ -204,9 +199,6 @@ "src/app/(dashboard)/cost-tracking/_components/add_provider_form.tsx": { "local/filename-pascal-case": { "count": 1 - }, - "no-restricted-imports": { - "count": 2 } }, "src/app/(dashboard)/cost-tracking/_components/cost_tracking_settings.tsx": { diff --git a/ui/litellm-dashboard/src/app/(dashboard)/cost-optimization/_components/PromptCompressionTab.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/cost-optimization/_components/PromptCompressionTab.integration.test.tsx new file mode 100644 index 00000000000..785976b8df4 --- /dev/null +++ b/ui/litellm-dashboard/src/app/(dashboard)/cost-optimization/_components/PromptCompressionTab.integration.test.tsx @@ -0,0 +1,127 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import PromptCompressionTab from "./PromptCompressionTab"; + +const createGuardrailCall = vi.fn(); +const getGuardrailsList = vi.fn(); + +vi.mock("@/components/networking", () => ({ + createGuardrailCall: (...args: unknown[]) => createGuardrailCall(...args), + getGuardrailsList: (...args: unknown[]) => getGuardrailsList(...args), +})); + +const submittedPayload = (): Record => { + expect(createGuardrailCall).toHaveBeenCalledTimes(1); + return createGuardrailCall.mock.calls[0][1] as Record; +}; + +describe("PromptCompressionTab submit payload", () => { + beforeEach(() => { + createGuardrailCall.mockClear().mockResolvedValue({}); + getGuardrailsList.mockClear().mockResolvedValue({ guardrails: [] }); + }); + + it("sends the trimmed name and api base with default_on true", async () => { + const user = userEvent.setup(); + render(); + + await user.type(screen.getByLabelText("Name"), " headroom-compression "); + await user.type(screen.getByLabelText("Headroom API base"), " https://headroom.example.com "); + await user.click(screen.getByRole("button", { name: "Add guardrail" })); + + await vi.waitFor(() => + expect(submittedPayload()).toEqual({ + guardrail_name: "headroom-compression", + litellm_params: { + guardrail: "headroom", + mode: "pre_call", + api_base: "https://headroom.example.com", + default_on: true, + }, + }), + ); + expect(createGuardrailCall.mock.calls[0][0]).toBe("test-token"); + }); + + it("sends default_on false once the apply-to-all switch is turned off", async () => { + const user = userEvent.setup(); + render(); + + await user.type(screen.getByLabelText("Name"), "headroom-optin"); + await user.type(screen.getByLabelText("Headroom API base"), "https://headroom.example.com"); + await user.click(screen.getByLabelText("Apply to all requests")); + await user.click(screen.getByRole("button", { name: "Add guardrail" })); + + await vi.waitFor(() => + expect(submittedPayload()).toEqual({ + guardrail_name: "headroom-optin", + litellm_params: { + guardrail: "headroom", + mode: "pre_call", + api_base: "https://headroom.example.com", + default_on: false, + }, + }), + ); + }); + + it("blocks submission and shows both required messages when the form is empty", async () => { + const user = userEvent.setup(); + render(); + + await user.click(screen.getByRole("button", { name: "Add guardrail" })); + + expect(await screen.findByText("Name is required")).toBeInTheDocument(); + expect(screen.getByText("API base is required")).toBeInTheDocument(); + expect(createGuardrailCall).not.toHaveBeenCalled(); + }); + + it("submits when Enter is pressed inside a text field", async () => { + const user = userEvent.setup(); + render(); + + await user.type(screen.getByLabelText("Name"), "headroom-compression"); + await user.type(screen.getByLabelText("Headroom API base"), "https://headroom.example.com{Enter}"); + + await vi.waitFor(() => expect(createGuardrailCall).toHaveBeenCalledTimes(1)); + }); + + it("clears the name and restores the default switch state after a successful create", async () => { + const user = userEvent.setup(); + render(); + + await user.type(screen.getByLabelText("Name"), "headroom-compression"); + await user.type(screen.getByLabelText("Headroom API base"), "https://headroom.example.com"); + await user.click(screen.getByLabelText("Apply to all requests")); + await user.click(screen.getByRole("button", { name: "Add guardrail" })); + + await vi.waitFor(() => expect(screen.getByLabelText("Name")).toHaveValue("")); + expect(screen.getByLabelText("Headroom API base")).toHaveValue(""); + expect(screen.getByLabelText("Apply to all requests")).toBeChecked(); + expect(getGuardrailsList).toHaveBeenCalledTimes(2); + }); + + it("keeps the always-on and opt-in badges for the guardrails it lists", async () => { + getGuardrailsList.mockResolvedValue({ + guardrails: [ + { + guardrail_id: "g-1", + guardrail_name: "always-on-one", + litellm_params: { guardrail: "headroom", api_base: "https://a.example.com", default_on: true }, + }, + { + guardrail_id: "g-2", + guardrail_name: "opt-in-one", + litellm_params: { guardrail: "headroom", api_base: "https://b.example.com", default_on: false }, + }, + ], + }); + render(); + + expect(await screen.findByText("Always on")).toBeInTheDocument(); + expect(screen.getByText("Opt-in")).toBeInTheDocument(); + expect(screen.getByText("https://a.example.com")).toBeInTheDocument(); + }); +}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/cost-optimization/_components/PromptCompressionTab.tsx b/ui/litellm-dashboard/src/app/(dashboard)/cost-optimization/_components/PromptCompressionTab.tsx index 3aab8c59259..c9f662ca985 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/cost-optimization/_components/PromptCompressionTab.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/cost-optimization/_components/PromptCompressionTab.tsx @@ -1,10 +1,19 @@ "use client"; import React, { useCallback, useEffect, useState } from "react"; -import { Button, Form, Input, Switch } from "antd"; +import { CircleHelp } from "lucide-react"; +import { z } from "zod/v4"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { createGuardrailCall, getGuardrailsList } from "@/components/networking"; +import { FieldGroup } from "@/components/shared/form/field"; +import { FormField } from "@/components/shared/form/FormField"; +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { Switch } from "@/components/ui/switch"; +import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; +import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner"; +import { useZodForm } from "@/lib/forms/useZodForm"; import { toast } from "@/lib/toast"; import { buildCompressionGuardrailPayload, @@ -17,14 +26,32 @@ interface PromptCompressionTabProps { accessToken: string | null; } -interface CompressionFormValues { - name: string; - apiBase: string; - defaultOn: boolean; -} +const compressionSchema = z.object({ + name: z.string().min(1, "Name is required"), + apiBase: z.string().min(1, "API base is required"), + defaultOn: z.boolean(), +}); + +type CompressionFormValues = z.infer; + +const EMPTY_VALUES: CompressionFormValues = { + name: "", + apiBase: "", + defaultOn: true, +}; + +const labelWithHint = (label: string, hint: string): React.ReactNode => ( + <> + {label} + + } /> + {hint} + + +); const PromptCompressionTab: React.FC = ({ accessToken }) => { - const [form] = Form.useForm(); + const form = useZodForm(compressionSchema, { defaultValues: EMPTY_VALUES }); const [guardrails, setGuardrails] = useState([]); const [isLoading, setIsLoading] = useState(true); const [isSaving, setIsSaving] = useState(false); @@ -61,7 +88,7 @@ const PromptCompressionTab: React.FC = ({ accessToken }), ); toast.success("Compression guardrail created"); - form.resetFields(); + form.reset(EMPTY_VALUES); await loadGuardrails(); } catch (error) { console.error("Failed to create compression guardrail:", error); @@ -85,7 +112,7 @@ const PromptCompressionTab: React.FC = ({ accessToken href="https://docs.litellm.ai/docs/proxy/headroom" target="_blank" rel="noopener noreferrer" - className="text-blue-600 underline" + className="text-blue-600 underline dark:text-blue-400" > Headroom setup docs @@ -97,7 +124,7 @@ const PromptCompressionTab: React.FC = ({ accessToken

)} {!isLoading && guardrails.length > 0 && ( -
    +
      {guardrails.map((guardrail) => (
    • @@ -107,8 +134,8 @@ const PromptCompressionTab: React.FC = ({ accessToken {guardrail.litellm_params?.default_on ? "Always on" : "Opt-in"} @@ -125,48 +152,57 @@ const PromptCompressionTab: React.FC = ({ accessToken Add Headroom compression guardrail -
      - - - - - - - - - -
      -

      - Applying compression to all requests is available to all users. Enabling it selectively per key or team - is a LiteLLM Enterprise feature. Get a trial key{" "} - + + + + {({ ref, ...field }) => } + + - here - -

      -
      -
      - -
      -
      + {({ ref, ...field }) => } + + + {({ value, onChange, ref: _ref, ...field }) => ( + } + checked={value} + onCheckedChange={onChange} + /> + )} + + +
      +

      + Applying compression to all requests is available to all users. Enabling it selectively per key or + team is a LiteLLM Enterprise feature. Get a trial key{" "} + + here + +

      +
      +
      + +
      + +
      diff --git a/ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/add_provider_form.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/add_provider_form.integration.test.tsx new file mode 100644 index 00000000000..83875cfc676 --- /dev/null +++ b/ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/add_provider_form.integration.test.tsx @@ -0,0 +1,68 @@ +// eslint-disable-next-line no-restricted-imports -- the parent cost_tracking_settings still owns this antd Form, and the point of this test is that AddProviderForm registers nothing in it +import { Form } from "antd"; +import { screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { renderWithProviders } from "../../../../../tests/test-utils"; +import AddProviderForm from "./add_provider_form"; +import { DiscountConfig } from "./types"; + +const onAddProvider = vi.fn(); +const onParentFinish = vi.fn(); +const readParentStore = vi.fn(); + +const ParentOwnedForm = () => { + const [form] = Form.useForm(); + return ( +
      + { + readParentStore(form.getFieldsValue()); + onAddProvider(); + }} + /> + + ); +}; + +describe("AddProviderForm inside the antd form its parent owns", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("registers no field in the parent FormInstance, so the parent's resetFields is a no-op", async () => { + const user = userEvent.setup(); + renderWithProviders(); + + await user.click(screen.getByRole("button", { name: /add provider discount/i })); + + expect(readParentStore).toHaveBeenCalledTimes(1); + expect(readParentStore.mock.calls[0][0]).toEqual({}); + }); + + it("drives both the onAddProvider prop and the parent form submit from one click", async () => { + const user = userEvent.setup(); + renderWithProviders(); + + await user.click(screen.getByRole("button", { name: /add provider discount/i })); + + expect(onAddProvider).toHaveBeenCalledTimes(1); + expect(onParentFinish).toHaveBeenCalledTimes(1); + }); + + it("treats Enter in the discount field exactly like a click on the add button", async () => { + const user = userEvent.setup(); + renderWithProviders(); + + await user.type(screen.getByPlaceholderText("5"), "{Enter}"); + + await vi.waitFor(() => expect(onParentFinish).toHaveBeenCalledTimes(1)); + expect(onAddProvider).toHaveBeenCalledTimes(1); + }); +}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/add_provider_form.tsx b/ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/add_provider_form.tsx index 0fdaed8814b..4f4edf6ff30 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/add_provider_form.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/cost-tracking/_components/add_provider_form.tsx @@ -1,11 +1,28 @@ import React from "react"; -import { TextInput, Button } from "@tremor/react"; -import { Select as AntdSelect, Form, Tooltip } from "antd"; -import { InfoCircleOutlined } from "@ant-design/icons"; -import { Providers, provider_map } from "@/components/provider_info_helpers"; +import { CircleHelp } from "lucide-react"; + import { Logo } from "@/components/molecules/logo/Logo"; +import { Providers, provider_map } from "@/components/provider_info_helpers"; +import { Field, FieldGroup, FieldLabel } 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 { InputGroupAddon } from "@/components/ui/input-group"; +import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; import { DiscountConfig } from "./types"; +interface ProviderOption { + value: string; + label: string; +} + interface AddProviderFormProps { discountConfig: DiscountConfig; selectedProvider: string | undefined; @@ -15,6 +32,35 @@ interface AddProviderFormProps { onAddProvider: () => void; } +const PROVIDER_FIELD_ID = "add-provider-discount-provider"; +const DISCOUNT_FIELD_ID = "add-provider-discount-percentage"; + +const providerOptionsWithoutDiscount = (discountConfig: DiscountConfig): ProviderOption[] => + Object.entries(Providers) + .filter(([providerEnum]) => { + const providerValue = provider_map[providerEnum as keyof typeof provider_map]; + return !(providerValue && discountConfig[providerValue]); + }) + .map(([value, label]) => ({ value, label })); + +const selectedProviderOption = (selectedProvider: string | undefined): ProviderOption | null => { + if (!selectedProvider) { + return null; + } + const label = Providers[selectedProvider as keyof typeof Providers]; + return label ? { value: selectedProvider, label } : null; +}; + +const labelWithHint = (label: string, hint: string): React.ReactNode => ( + <> + {label} + + } /> + {hint} + + +); + const AddProviderForm: React.FC = ({ discountConfig, selectedProvider, @@ -23,79 +69,71 @@ const AddProviderForm: React.FC = ({ onDiscountChange, onAddProvider, }) => { + const options = providerOptionsWithoutDiscount(discountConfig); + const selectedOption = selectedProviderOption(selectedProvider); + return ( -
      - - Provider - - - - - } - rules={[{ required: true, message: "Please select a provider" }]} - > - - String(option?.label ?? "") - .toLowerCase() - .includes(input.toLowerCase()) - } - > - {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 discount configured - if (providerValue && discountConfig[providerValue]) { - return null; - } - return ( - -
      - - {providerDisplayName} -
      -       - ); - })} -       -       + +
      + + + + {labelWithHint("Provider", "Select the LLM provider you want to configure a discount for")} + + onProviderChange(option?.value)} + itemToStringLabel={(option: ProviderOption) => option.label} + isItemEqualToValue={(option: ProviderOption, value: ProviderOption) => option.value === value.value} + > + + {selectedOption && ( + + + + )} + + + No providers found + + {(option: ProviderOption) => ( + + + + {option.label} + + + )} + + + + - - Discount Percentage - - - - - } - rules={[{ required: true, message: "Please enter a discount percentage" }]} - > -
      - - % + + + {labelWithHint("Discount Percentage", "Enter a percentage value (e.g., 5 for 5% discount)")} + +
      + onDiscountChange(event.target.value)} + className="flex-1 rounded-lg" + /> + % +
      +       + + +
      +
      - - -
      -
      -
      + ); }; diff --git a/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroCreateModal.integration.test.tsx b/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroCreateModal.integration.test.tsx new file mode 100644 index 00000000000..4df208bece2 --- /dev/null +++ b/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroCreateModal.integration.test.tsx @@ -0,0 +1,95 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import CloudZeroCreateModal from "./CloudZeroCreateModal"; + +const mutate = vi.fn(); + +vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({ + __esModule: true, + default: () => ({ accessToken: "test-token" }), +})); + +vi.mock("@/app/(dashboard)/hooks/cloudzero/useCloudZeroCreate", () => ({ + useCloudZeroCreate: () => ({ mutate, isPending: false }), +})); + +const renderModal = () => { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + return render( + + + , + ); +}; + +const submittedPayload = (): Record => { + expect(mutate).toHaveBeenCalledTimes(1); + return mutate.mock.calls[0][0] as Record; +}; + +describe("CloudZeroCreateModal submit payload", () => { + beforeEach(() => { + mutate.mockClear(); + }); + + it("sends every filled field verbatim", async () => { + const user = userEvent.setup(); + renderModal(); + + await user.type(screen.getByLabelText("CloudZero API Key"), "cz-secret-key"); + await user.type(screen.getByLabelText("Connection ID"), "conn-42"); + await user.type(screen.getByLabelText("Timezone"), "America/New_York"); + await user.click(screen.getByRole("button", { name: "Create" })); + + await vi.waitFor(() => + expect(submittedPayload()).toEqual({ + connection_id: "conn-42", + timezone: "America/New_York", + api_key: "cz-secret-key", + }), + ); + }); + + it("defaults an untouched timezone to UTC", async () => { + const user = userEvent.setup(); + renderModal(); + + await user.type(screen.getByLabelText("CloudZero API Key"), "cz-secret-key"); + await user.type(screen.getByLabelText("Connection ID"), "conn-42"); + await user.click(screen.getByRole("button", { name: "Create" })); + + await vi.waitFor(() => + expect(submittedPayload()).toEqual({ + connection_id: "conn-42", + timezone: "UTC", + api_key: "cz-secret-key", + }), + ); + }); + + it("blocks submission and shows both required messages when the form is empty", async () => { + const user = userEvent.setup(); + renderModal(); + + await user.click(screen.getByRole("button", { name: "Create" })); + + expect(await screen.findByText("Please enter your CloudZero API key")).toBeInTheDocument(); + expect(screen.getByText("Please enter your CloudZero connection ID")).toBeInTheDocument(); + expect(mutate).not.toHaveBeenCalled(); + }); + + it("does not submit when Enter is pressed inside a text field", async () => { + const user = userEvent.setup(); + renderModal(); + + await user.type(screen.getByLabelText("CloudZero API Key"), "cz-secret-key"); + await user.type(screen.getByLabelText("Connection ID"), "conn-42{Enter}"); + + expect(mutate).not.toHaveBeenCalled(); + }); +}); diff --git a/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroCreateModal.tsx b/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroCreateModal.tsx index bd08bf028ad..4b1bffb5f52 100644 --- a/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroCreateModal.tsx +++ b/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroCreateModal.tsx @@ -1,8 +1,18 @@ -import { Form, Modal, Input } from "antd"; -import { toast } from "@/lib/toast"; +import { Modal } from "antd"; import { useEffect } from "react"; -import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; +import { z } from "zod/v4"; + import { useCloudZeroCreate } from "@/app/(dashboard)/hooks/cloudzero/useCloudZeroCreate"; +import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; +import { FieldGroup } from "@/components/shared/form/field"; +import { FormField } from "@/components/shared/form/FormField"; +import { Input } from "@/components/ui/input"; +import { TooltipProvider } from "@/components/ui/tooltip"; +import { useZodForm } from "@/lib/forms/useZodForm"; +import { toast } from "@/lib/toast"; + +import { CloudZeroApiKeyInput, labelWithHint } from "./CloudZeroFormControls"; +import { buildCloudZeroPayload, EMPTY_CLOUDZERO_FORM_VALUES, type CloudZeroFormValues } from "./cloudZeroPayload"; interface CloudZeroCreationModalProps { open: boolean; @@ -10,50 +20,38 @@ interface CloudZeroCreationModalProps { onCancel: () => void; } +const createSchema = z.object({ + api_key: z.string().min(1, "Please enter your CloudZero API key"), + connection_id: z.string().min(1, "Please enter your CloudZero connection ID"), + timezone: z.string(), +}); + export default function CloudZeroCreationModal({ open, onOk, onCancel }: CloudZeroCreationModalProps) { const { accessToken } = useAuthorized(); - const [form] = Form.useForm(); + const form = useZodForm(createSchema, { defaultValues: EMPTY_CLOUDZERO_FORM_VALUES }); const createMutation = useCloudZeroCreate(accessToken || ""); useEffect(() => { if (open) { - form.resetFields(); + form.reset(EMPTY_CLOUDZERO_FORM_VALUES); } }, [open, form]); - const handleSubmit = async () => { - try { - const values = await form.validateFields(); - createMutation.mutate( - { - connection_id: values.connection_id, - timezone: values.timezone || "UTC", - ...(values.api_key && { api_key: values.api_key }), - }, - { - onSuccess: () => { - toast.success("CloudZero integration created successfully"); - form.resetFields(); - onOk(); - }, - onError: (error: any) => { - if (error?.errorFields) { - return; - } - toast.error(error?.message || "Failed to create CloudZero integration"); - }, - }, - ); - } catch (error: any) { - if (error?.errorFields) { - return; - } - toast.error(error?.message || "Failed to create CloudZero integration"); - } + const handleSubmit = (values: CloudZeroFormValues) => { + createMutation.mutate(buildCloudZeroPayload(values), { + onSuccess: () => { + toast.success("CloudZero integration created successfully"); + form.reset(EMPTY_CLOUDZERO_FORM_VALUES); + onOk(); + }, + onError: (error: Error) => { + toast.error(error.message || "Failed to create CloudZero integration"); + }, + }); }; const handleCancel = () => { - form.resetFields(); + form.reset(EMPTY_CLOUDZERO_FORM_VALUES); onCancel(); }; @@ -61,7 +59,7 @@ export default function CloudZeroCreationModal({ open, onOk, onCancel }: CloudZe void form.handleSubmit(handleSubmit)()} onCancel={handleCancel} confirmLoading={createMutation.isPending} okText={createMutation.isPending ? "Creating..." : "Create"} @@ -73,29 +71,27 @@ export default function CloudZeroCreationModal({ open, onOk, onCancel }: CloudZe disabled: createMutation.isPending, }} > -
      - - - - - - - - - -
      + +
      event.preventDefault()} noValidate> + + + {({ ref, ...field }) => ( + + )} + + + {({ ref, ...field }) => } + + + {({ ref, ...field }) => } + + +
      +       ); } diff --git a/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroFormControls.tsx b/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroFormControls.tsx new file mode 100644 index 00000000000..7f11f76cfe0 --- /dev/null +++ b/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroFormControls.tsx @@ -0,0 +1,41 @@ +"use client"; + +import { CircleHelp, Eye, EyeOff } from "lucide-react"; +import * as React from "react"; + +import { InputGroup, InputGroupAddon, InputGroupButton, InputGroupInput } from "@/components/ui/input-group"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; + +export const labelWithHint = (label: string, hint: string): React.ReactNode => ( + <> + {label} + + } /> + {hint} + + +); + +export const CloudZeroApiKeyInput = React.forwardRef< + HTMLInputElement, + Omit, "type"> +>(({ className, ...props }, ref) => { + const [revealed, setRevealed] = React.useState(false); + + return ( + + + + setRevealed((current) => !current)} + > + {revealed ? : } + + + + ); +}); +CloudZeroApiKeyInput.displayName = "CloudZeroApiKeyInput"; diff --git a/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroUpdateModal.integration.test.tsx b/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroUpdateModal.integration.test.tsx new file mode 100644 index 00000000000..a0582657281 --- /dev/null +++ b/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroUpdateModal.integration.test.tsx @@ -0,0 +1,139 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import CloudZeroUpdateModal from "./CloudZeroUpdateModal"; +import { CloudZeroSettings } from "./types"; + +const mutate = vi.fn(); + +vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({ + __esModule: true, + default: () => ({ accessToken: "test-token" }), +})); + +vi.mock("@/app/(dashboard)/hooks/cloudzero/useCloudZeroSettings", () => ({ + useCloudZeroUpdateSettings: () => ({ mutate, isPending: false }), +})); + +const STORED_SETTINGS: CloudZeroSettings = { + connection_id: "stored-connection-id", + api_key_masked: "sk-cz-****last4", + timezone: "Europe/Berlin", + status: "Active", +}; + +const renderModal = (settings: CloudZeroSettings = STORED_SETTINGS) => { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + return render( + + + , + ); +}; + +const submittedPayload = (): Record => { + expect(mutate).toHaveBeenCalledTimes(1); + return mutate.mock.calls[0][0] as Record; +}; + +describe("CloudZeroUpdateModal submit payload", () => { + beforeEach(() => { + mutate.mockClear(); + }); + + it("seeds the stored connection id and timezone but never the stored key", () => { + renderModal(); + + expect(screen.getByLabelText("Connection ID")).toHaveValue("stored-connection-id"); + expect(screen.getByLabelText("Timezone")).toHaveValue("Europe/Berlin"); + expect(screen.getByLabelText("CloudZero API Key")).toHaveValue(""); + }); + + it("omits api_key entirely when the key field is left untouched, preserving the stored secret", async () => { + const user = userEvent.setup(); + renderModal(); + + await user.click(screen.getByRole("button", { name: "Update" })); + + await vi.waitFor(() => + expect(submittedPayload()).toEqual({ + connection_id: "stored-connection-id", + timezone: "Europe/Berlin", + }), + ); + expect("api_key" in submittedPayload()).toBe(false); + }); + + it("sends api_key only once the user types a replacement key", async () => { + const user = userEvent.setup(); + renderModal(); + + await user.type(screen.getByLabelText("CloudZero API Key"), "cz-rotated-key"); + await user.click(screen.getByRole("button", { name: "Update" })); + + await vi.waitFor(() => + expect(submittedPayload()).toEqual({ + connection_id: "stored-connection-id", + timezone: "Europe/Berlin", + api_key: "cz-rotated-key", + }), + ); + }); + + it("falls back to UTC when the stored timezone is cleared", async () => { + const user = userEvent.setup(); + renderModal(); + + await user.clear(screen.getByLabelText("Timezone")); + await user.click(screen.getByRole("button", { name: "Update" })); + + await vi.waitFor(() => + expect(submittedPayload()).toEqual({ + connection_id: "stored-connection-id", + timezone: "UTC", + }), + ); + }); + + it("falls back to UTC when the stored settings carry no timezone", () => { + renderModal({ ...STORED_SETTINGS, timezone: null }); + + expect(screen.getByLabelText("Timezone")).toHaveValue("UTC"); + }); + + it("reports a null connection id from the server as the required field, not as a type error", async () => { + const user = userEvent.setup(); + renderModal({ ...STORED_SETTINGS, connection_id: null, timezone: null }); + + await user.click(screen.getByRole("button", { name: "Update" })); + + expect(await screen.findByText("Please enter your CloudZero connection ID")).toBeInTheDocument(); + expect(screen.queryByText(/expected string, received null/i)).not.toBeInTheDocument(); + expect(mutate).not.toHaveBeenCalled(); + }); + + it("blocks submission when the connection id is cleared, and leaves the key optional", async () => { + const user = userEvent.setup(); + renderModal(); + + await user.clear(screen.getByLabelText("Connection ID")); + await user.click(screen.getByRole("button", { name: "Update" })); + + expect(await screen.findByText("Please enter your CloudZero connection ID")).toBeInTheDocument(); + expect(screen.queryByText("Please enter your CloudZero API key")).not.toBeInTheDocument(); + expect(mutate).not.toHaveBeenCalled(); + }); + + it("does not submit when Enter is pressed inside a text field", async () => { + const user = userEvent.setup(); + renderModal(); + + await user.type(screen.getByLabelText("CloudZero API Key"), "cz-rotated-key{Enter}"); + + expect(mutate).not.toHaveBeenCalled(); + }); +}); diff --git a/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroUpdateModal.tsx b/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroUpdateModal.tsx index 44049714c6c..74c16624887 100644 --- a/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroUpdateModal.tsx +++ b/ui/litellm-dashboard/src/components/CloudZeroCostTracking/CloudZeroUpdateModal.tsx @@ -1,8 +1,18 @@ +import { Modal } from "antd"; +import { useEffect } from "react"; +import { z } from "zod/v4"; + import { useCloudZeroUpdateSettings } from "@/app/(dashboard)/hooks/cloudzero/useCloudZeroSettings"; import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; -import { Form, Input, Modal } from "antd"; +import { FieldGroup } from "@/components/shared/form/field"; +import { FormField } from "@/components/shared/form/FormField"; +import { Input } from "@/components/ui/input"; +import { TooltipProvider } from "@/components/ui/tooltip"; +import { useZodForm } from "@/lib/forms/useZodForm"; import { toast } from "@/lib/toast"; -import { useEffect } from "react"; + +import { CloudZeroApiKeyInput, labelWithHint } from "./CloudZeroFormControls"; +import { buildCloudZeroPayload, EMPTY_CLOUDZERO_FORM_VALUES, type CloudZeroFormValues } from "./cloudZeroPayload"; import { CloudZeroSettings } from "./types"; interface CloudZeroUpdateModalProps { @@ -12,56 +22,44 @@ interface CloudZeroUpdateModalProps { settings: CloudZeroSettings; } +const updateSchema = z.object({ + api_key: z.string(), + connection_id: z.string().min(1, "Please enter your CloudZero connection ID"), + timezone: z.string(), +}); + export default function CloudZeroUpdateModal({ open, onOk, onCancel, settings }: CloudZeroUpdateModalProps) { const { accessToken } = useAuthorized(); - const [form] = Form.useForm(); + const form = useZodForm(updateSchema, { defaultValues: EMPTY_CLOUDZERO_FORM_VALUES }); const updateMutation = useCloudZeroUpdateSettings(accessToken || ""); useEffect(() => { if (open && settings) { - form.setFieldsValue({ - connection_id: settings.connection_id, + form.reset({ + connection_id: settings.connection_id ?? "", timezone: settings.timezone || "UTC", api_key: "", }); } else if (open) { - form.resetFields(); + form.reset(EMPTY_CLOUDZERO_FORM_VALUES); } }, [open, settings, form]); - const handleSubmit = async () => { - try { - const values = await form.validateFields(); - updateMutation.mutate( - { - connection_id: values.connection_id, - timezone: values.timezone || "UTC", - ...(values.api_key && { api_key: values.api_key }), - }, - { - onSuccess: () => { - toast.success("CloudZero integration updated successfully"); - form.resetFields(); - onOk(); - }, - onError: (error: any) => { - if (error?.errorFields) { - return; - } - toast.error(error?.message || "Failed to update CloudZero integration"); - }, - }, - ); - } catch (error: any) { - if (error?.errorFields) { - return; - } - toast.error(error?.message || "Failed to update CloudZero integration"); - } + const handleSubmit = (values: CloudZeroFormValues) => { + updateMutation.mutate(buildCloudZeroPayload(values), { + onSuccess: () => { + toast.success("CloudZero integration updated successfully"); + form.reset(EMPTY_CLOUDZERO_FORM_VALUES); + onOk(); + }, + onError: (error: Error) => { + toast.error(error.message || "Failed to update CloudZero integration"); + }, + }); }; const handleCancel = () => { - form.resetFields(); + form.reset(EMPTY_CLOUDZERO_FORM_VALUES); onCancel(); }; @@ -69,7 +67,7 @@ export default function CloudZeroUpdateModal({ open, onOk, onCancel, settings }: void form.handleSubmit(handleSubmit)()} onCancel={handleCancel} confirmLoading={updateMutation.isPending} okText={updateMutation.isPending ? "Updating..." : "Update"} @@ -81,30 +79,31 @@ export default function CloudZeroUpdateModal({ open, onOk, onCancel, settings }: disabled: updateMutation.isPending, }} > -
      - - - - - - - - - -
      + +
      event.preventDefault()} noValidate> + + + {({ ref, ...field }) => ( + + )} + + + {({ ref, ...field }) => } + + + {({ ref, ...field }) => } + + +
      +       ); } diff --git a/ui/litellm-dashboard/src/components/CloudZeroCostTracking/cloudZeroPayload.test.ts b/ui/litellm-dashboard/src/components/CloudZeroCostTracking/cloudZeroPayload.test.ts new file mode 100644 index 00000000000..8517295b4fe --- /dev/null +++ b/ui/litellm-dashboard/src/components/CloudZeroCostTracking/cloudZeroPayload.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from "vitest"; + +import { buildCloudZeroPayload } from "./cloudZeroPayload"; + +describe("buildCloudZeroPayload", () => { + it("passes a filled form through verbatim", () => { + expect( + buildCloudZeroPayload({ api_key: "cz-key", connection_id: "conn-1", timezone: "America/New_York" }), + ).toStrictEqual({ + connection_id: "conn-1", + timezone: "America/New_York", + api_key: "cz-key", + }); + }); + + it("omits the api_key key entirely when the field is blank, so a stored secret survives an untouched save", () => { + const payload = buildCloudZeroPayload({ api_key: "", connection_id: "conn-1", timezone: "UTC" }); + + expect("api_key" in payload).toBe(false); + expect(payload).toStrictEqual({ connection_id: "conn-1", timezone: "UTC" }); + }); + + it.each([ + ["blank", ""], + ["absent", undefined], + ])("falls back to UTC when the timezone is %s", (_label, timezone) => { + const payload = buildCloudZeroPayload({ + api_key: "cz-key", + connection_id: "conn-1", + timezone: timezone as string, + }); + + expect(payload.timezone).toBe("UTC"); + }); + + it("never invents a timezone default over a real value", () => { + expect(buildCloudZeroPayload({ api_key: "", connection_id: "conn-1", timezone: "UTC+2" }).timezone).toBe("UTC+2"); + }); + + it("keeps a whitespace-only api_key, matching the truthiness check the antd modals used", () => { + expect(buildCloudZeroPayload({ api_key: " ", connection_id: "conn-1", timezone: "UTC" }).api_key).toBe(" "); + }); +}); diff --git a/ui/litellm-dashboard/src/components/CloudZeroCostTracking/cloudZeroPayload.ts b/ui/litellm-dashboard/src/components/CloudZeroCostTracking/cloudZeroPayload.ts new file mode 100644 index 00000000000..3873df97058 --- /dev/null +++ b/ui/litellm-dashboard/src/components/CloudZeroCostTracking/cloudZeroPayload.ts @@ -0,0 +1,23 @@ +export interface CloudZeroFormValues { + api_key: string; + connection_id: string; + timezone: string; +} + +export interface CloudZeroPayload { + connection_id: string; + timezone: string; + api_key?: string; +} + +export const EMPTY_CLOUDZERO_FORM_VALUES: CloudZeroFormValues = { + api_key: "", + connection_id: "", + timezone: "", +}; + +export const buildCloudZeroPayload = (values: CloudZeroFormValues): CloudZeroPayload => ({ + connection_id: values.connection_id, + timezone: values.timezone || "UTC", + ...(values.api_key && { api_key: values.api_key }), +});