diff --git a/ui/litellm-dashboard/src/components/add_pass_through.integration.test.tsx b/ui/litellm-dashboard/src/components/add_pass_through.integration.test.tsx index 3e2af8821e4..df4dbff4111 100644 --- a/ui/litellm-dashboard/src/components/add_pass_through.integration.test.tsx +++ b/ui/litellm-dashboard/src/components/add_pass_through.integration.test.tsx @@ -34,20 +34,59 @@ const openModal = async (user: User) => { await screen.findByText("Route Configuration"); }; -const fillRequiredFields = async (user: User) => { - fireEvent.change(screen.getByPlaceholderText("bria"), { target: { value: "bria" } }); +interface RequiredFieldValues { + path?: string; + target?: string; + headerName?: string; + headerValue?: string; +} + +const fillRequiredFields = async (user: User, values: RequiredFieldValues = {}) => { + const { + path = "bria", + target = "https://example.com", + headerName = "Authorization", + headerValue = "Bearer abc", + } = values; + + fireEvent.change(screen.getByPlaceholderText("bria"), { target: { value: path } }); fireEvent.change(screen.getByPlaceholderText("https://engine.prod.bria-api.com"), { - target: { value: "https://example.com" }, + target: { value: target }, }); await user.click(screen.getByRole("button", { name: /add header/i })); - fireEvent.change(screen.getByPlaceholderText("Header Name"), { target: { value: "Authorization" } }); - fireEvent.change(screen.getByPlaceholderText("Header Value"), { target: { value: "Bearer abc" } }); + fireEvent.change(screen.getByPlaceholderText("Header Name"), { target: { value: headerName } }); + fireEvent.change(screen.getByPlaceholderText("Header Value"), { target: { value: headerValue } }); +}; + +const addQueryParam = async (user: User, name: string, value: string) => { + await user.click(screen.getByRole("button", { name: /add query parameter/i })); + fireEvent.change(screen.getByPlaceholderText("Parameter Name (e.g., version)"), { target: { value: name } }); + fireEvent.change(screen.getByPlaceholderText("Parameter Value (e.g., v1)"), { target: { value } }); }; const submit = async (user: User) => user.click(screen.getByRole("button", { name: "Add Pass-Through Endpoint" })); const lastPayload = () => createPassThroughEndpoint.mock.calls.at(-1)?.[1] as Record; +const reopenedFields: RequiredFieldValues = { + path: "adobe", + target: "https://adobe.example.com", + headerName: "x-api-key", + headerValue: "second-secret", +}; + +const reopenedPayload = { + path: "/adobe", + target: "https://adobe.example.com", + headers: { "x-api-key": "second-secret" }, + include_subpath: true, + methods: undefined, + default_query_params: undefined, + auth: undefined, + timeout: undefined, + cost_per_request: undefined, +}; + describe("add_pass_through submit payload", () => { beforeEach(() => { vi.clearAllMocks(); @@ -218,4 +257,35 @@ describe("add_pass_through submit payload", () => { expect(createPassThroughEndpoint).not.toHaveBeenCalled(); }); + + it("leaves no header or query parameter rows behind when the modal is cancelled and reopened", async () => { + const user = setup(); + renderForm(); + await openModal(user); + await fillRequiredFields(user); + await addQueryParam(user, "version", "v1"); + + await user.click(screen.getByRole("button", { name: "Cancel" })); + await openModal(user); + + expect(screen.queryByPlaceholderText("Header Name")).not.toBeInTheDocument(); + expect(screen.queryByPlaceholderText("Parameter Name (e.g., version)")).not.toBeInTheDocument(); + }); + + it("submits the reopened endpoint instead of rejecting it as missing headers", async () => { + const user = setup(); + renderForm(); + await openModal(user); + await fillRequiredFields(user); + + await user.click(screen.getByRole("button", { name: "Cancel" })); + await openModal(user); + await fillRequiredFields(user, reopenedFields); + + await submit(user); + + await waitFor(() => expect(createPassThroughEndpoint).toHaveBeenCalled()); + expect(screen.queryByText("Please configure the headers")).not.toBeInTheDocument(); + expect(lastPayload()).toStrictEqual(reopenedPayload); + }); }); diff --git a/ui/litellm-dashboard/src/components/add_pass_through.tsx b/ui/litellm-dashboard/src/components/add_pass_through.tsx index 8539ba0d532..e38c3082b45 100644 --- a/ui/litellm-dashboard/src/components/add_pass_through.tsx +++ b/ui/litellm-dashboard/src/components/add_pass_through.tsx @@ -8,7 +8,7 @@ import { z } from "zod/v4"; import { createPassThroughEndpoint } from "./networking"; import NumericalInput from "./shared/numerical_input"; -import KeyValueInput from "./key_value_input"; +import KeyValueInput, { type KeyValuePair } from "./key_value_input"; import QueryParamInput from "./query_param_input"; import { passThroughItem } from "./PassThroughSettings/PassThroughSettings"; import RoutePreview from "./route_preview"; @@ -31,6 +31,8 @@ const HTTP_METHOD_OPTIONS = HTTP_METHODS.map((method) => ({ label: method, value type GuardrailSettings = Record; +const keyValuePairsSchema = z.array(z.tuple([z.string(), z.string()])); + const passThroughFormSchema = z.object({ path: z.string().min(1, "Path is required").regex(/^\//, "Path is required"), target: z @@ -39,8 +41,10 @@ const passThroughFormSchema = z.object({ .pipe(z.url({ error: "Please enter a valid URL" })), methods: z.array(z.string()).optional(), include_subpath: z.boolean(), - headers: z.record(z.string(), z.string(), { error: "Please configure the headers" }), - default_query_params: z.record(z.string(), z.string()).optional(), + headers: keyValuePairsSchema.refine((pairs) => pairs.some(([name]) => name !== ""), { + error: "Please configure the headers", + }), + default_query_params: keyValuePairsSchema.optional(), auth: z.boolean().optional(), timeout: z.string().optional(), cost_per_request: z.string().optional(), @@ -53,7 +57,7 @@ const emptyFormValues = { target: "", methods: undefined, include_subpath: true, - headers: undefined, + headers: [], default_query_params: undefined, auth: undefined, timeout: undefined, @@ -72,6 +76,14 @@ const labelWithHint = (label: React.ReactNode, hint: string): React.ReactNode => const optionalText = (raw: string): string | undefined => (raw === "" ? undefined : raw); +const toRecord = (pairs: readonly KeyValuePair[]): Record => + Object.fromEntries(pairs.filter(([name]) => name !== "")); + +const optionalRecord = (pairs: readonly KeyValuePair[] | undefined): Record | undefined => { + const record = toRecord(pairs ?? []); + return Object.keys(record).length > 0 ? record : undefined; +}; + interface AddFallbacksProps { accessToken: string; passThroughItems: passThroughItem[]; @@ -109,8 +121,8 @@ const AddPassThroughEndpoint: React.FC = ({ target: values.target, methods: values.methods, include_subpath: values.include_subpath, - headers: values.headers, - default_query_params: values.default_query_params, + headers: toRecord(values.headers), + default_query_params: optionalRecord(values.default_query_params), ...(premiumUser ? { auth: values.auth } : {}), timeout: values.timeout, cost_per_request: values.cost_per_request, diff --git a/ui/litellm-dashboard/src/components/key_value_input.test.tsx b/ui/litellm-dashboard/src/components/key_value_input.test.tsx index 0f9b8e682ae..649720676e9 100644 --- a/ui/litellm-dashboard/src/components/key_value_input.test.tsx +++ b/ui/litellm-dashboard/src/components/key_value_input.test.tsx @@ -1,12 +1,26 @@ +import { useState } from "react"; import { fireEvent, render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, expect, it, vi } from "vitest"; -import KeyValueInput from "./key_value_input"; +import KeyValueInput, { type KeyValuePair } from "./key_value_input"; + +const ControlledKeyValueInput = ({ onChange }: { onChange?: (value: readonly KeyValuePair[]) => void }) => { + const [value, setValue] = useState([]); + return ( + { + setValue(next); + onChange?.(next); + }} + /> + ); +}; describe("KeyValueInput", () => { it("renders existing header pairs", () => { - render(); + render(); expect(screen.getByPlaceholderText("Header Name")).toHaveValue("Authorization"); expect(screen.getByPlaceholderText("Header Value")).toHaveValue("Bearer token"); @@ -15,12 +29,54 @@ describe("KeyValueInput", () => { it("adds a header row and emits edits", async () => { const user = userEvent.setup(); const onChange = vi.fn(); - render(); + render(); await user.click(screen.getByRole("button", { name: /Add Header$/ })); fireEvent.change(screen.getByPlaceholderText("Header Name"), { target: { value: "X-Trace" } }); fireEvent.change(screen.getByPlaceholderText("Header Value"), { target: { value: "enabled" } }); - expect(onChange).toHaveBeenLastCalledWith({ "X-Trace": "enabled" }); + expect(onChange).toHaveBeenLastCalledWith([["X-Trace", "enabled"]]); + }); + + it("renders no rows once the value prop goes back to empty", () => { + const { rerender } = render(); + expect(screen.getByPlaceholderText("Header Name")).toHaveValue("Authorization"); + + rerender(); + + expect(screen.queryByPlaceholderText("Header Name")).not.toBeInTheDocument(); + expect(screen.queryByPlaceholderText("Header Value")).not.toBeInTheDocument(); + }); + + it("keeps a half-typed row editable while its name is still empty", async () => { + const user = userEvent.setup(); + render(); + + await user.click(screen.getByRole("button", { name: /Add Header$/ })); + await user.click(screen.getByRole("button", { name: /Add Header$/ })); + expect(screen.getAllByPlaceholderText("Header Name")).toHaveLength(2); + + fireEvent.change(screen.getAllByPlaceholderText("Header Value")[1], { target: { value: "pending" } }); + + expect(screen.getAllByPlaceholderText("Header Name")).toHaveLength(2); + expect(screen.getAllByPlaceholderText("Header Value")[1]).toHaveValue("pending"); + }); + + it("removes only the row whose remove button is clicked", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + render( + , + ); + + await user.click(screen.getByRole("button", { name: "Remove header 1" })); + + expect(onChange).toHaveBeenLastCalledWith([["b", "2"]]); }); }); diff --git a/ui/litellm-dashboard/src/components/key_value_input.tsx b/ui/litellm-dashboard/src/components/key_value_input.tsx index a35968ce0e7..363f24f4535 100644 --- a/ui/litellm-dashboard/src/components/key_value_input.tsx +++ b/ui/litellm-dashboard/src/components/key_value_input.tsx @@ -1,40 +1,30 @@ -import React, { useState } from "react"; +import React from "react"; import { Minus, Plus } from "lucide-react"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; +export type KeyValuePair = readonly [string, string]; + interface KeyValueInputProps { - value?: Record; - onChange?: (value: Record) => void; + value?: readonly KeyValuePair[]; + onChange?: (value: readonly KeyValuePair[]) => void; } -const KeyValueInput: React.FC = ({ value = {}, onChange }) => { - const [pairs, setPairs] = useState<[string, string][]>(Object.entries(value)); +const KeyValueInput: React.FC = ({ value = [], onChange }) => { + const handleAdd = () => onChange?.([...value, ["", ""]]); - const handleAdd = () => { - setPairs([...pairs, ["", ""]]); - }; + const handleRemove = (index: number) => onChange?.(value.filter((_, i) => i !== index)); - const handleRemove = (index: number) => { - const newPairs = pairs.filter((_, i) => i !== index); - setPairs(newPairs); - onChange?.(Object.fromEntries(newPairs)); - }; - - const handleChange = (index: number, key: string, val: string) => { - const newPairs = [...pairs]; - newPairs[index] = [key, val]; - setPairs(newPairs); - onChange?.(Object.fromEntries(newPairs)); - }; + const handleChange = (index: number, pair: KeyValuePair) => + onChange?.(value.map((existing, i) => (i === index ? pair : existing))); return (
- {pairs.map(([key, val], index) => ( + {value.map(([key, val], index) => (
- handleChange(index, e.target.value, val)} /> - handleChange(index, key, e.target.value)} /> + handleChange(index, [e.target.value, val])} /> + handleChange(index, [key, e.target.value])} />