mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-17 04:34:25 +00:00
fix(ui): validate the Add Model form when Test Connect is clicked (#46834)
* fix(ui): gate Add Model's Test Connect on the form's own validation Test Connect stays disabled until the Add Model form would pass validation, with a tooltip saying why. Provider password, text and textarea inputs now render empty after a successful add resets the form, and an empty prepared request shows a plain message instead of a destructuring crash * fix(ui): validate the Add Model form when Test Connect is clicked Test Connect stays enabled and, on click, runs the same validation Add Model runs. An incomplete form now shows the inline required errors instead of opening a crashing connection test, and a valid form tests as before. This replaces the disabled-button gate, which read a validity flag that went stale after programmatic form updates * test(ui): cover a blank required Azure API Base on Test Connect
This commit is contained in:
parent
cb7e6a7f4c
commit
0620f1a515
6 changed files with 310 additions and 17 deletions
|
|
@ -8,9 +8,9 @@ import {
|
|||
renderWithProviders,
|
||||
} from "../../../tests/test-utils";
|
||||
import userEvent, { PointerEventsCheckLevel } from "@testing-library/user-event";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { Team } from "../key_team_helpers/key_list";
|
||||
import { credentialCreateCall, modelCreateCall, type CredentialItem } from "../networking";
|
||||
import { credentialCreateCall, modelCreateCall, testConnectionRequest, type CredentialItem } from "../networking";
|
||||
import { Providers } from "../provider_info_helpers";
|
||||
import { projectMountedValues, useMountRegistry, type MountedFormValues } from "../common_components/MountedFormField";
|
||||
import { useForm } from "react-hook-form";
|
||||
|
|
@ -53,7 +53,17 @@ vi.mock("../networking", async () => {
|
|||
provider_display_name: "OpenAI",
|
||||
litellm_provider: "openai",
|
||||
default_model_placeholder: "gpt-3.5-turbo",
|
||||
credential_fields: [],
|
||||
credential_fields: [{ key: "api_key", label: "OpenAI API Key", field_type: "password", required: true }],
|
||||
},
|
||||
{
|
||||
provider: "Azure",
|
||||
provider_display_name: "Azure",
|
||||
litellm_provider: "azure",
|
||||
default_model_placeholder: "my-deployment",
|
||||
credential_fields: [
|
||||
{ key: "api_base", label: "API Base", field_type: "text", required: true },
|
||||
{ key: "api_key", label: "Azure API Key", field_type: "password" },
|
||||
],
|
||||
},
|
||||
{
|
||||
provider: "MICROSOFT_365_COPILOT",
|
||||
|
|
@ -98,7 +108,17 @@ vi.mock("@/app/(dashboard)/hooks/providers/useProviderFields", () => ({
|
|||
provider_display_name: "OpenAI",
|
||||
litellm_provider: "openai",
|
||||
default_model_placeholder: "gpt-3.5-turbo",
|
||||
credential_fields: [],
|
||||
credential_fields: [{ key: "api_key", label: "OpenAI API Key", field_type: "password", required: true }],
|
||||
},
|
||||
{
|
||||
provider: "Azure",
|
||||
provider_display_name: "Azure",
|
||||
litellm_provider: "azure",
|
||||
default_model_placeholder: "my-deployment",
|
||||
credential_fields: [
|
||||
{ key: "api_base", label: "API Base", field_type: "text", required: true },
|
||||
{ key: "api_key", label: "Azure API Key", field_type: "password" },
|
||||
],
|
||||
},
|
||||
{
|
||||
provider: "MICROSOFT_365_COPILOT",
|
||||
|
|
@ -252,6 +272,22 @@ const createTestProps = (userRole = "proxy_admin", userId = "user-1", isTeamAdmi
|
|||
};
|
||||
};
|
||||
|
||||
type User = ReturnType<typeof userEvent.setup>;
|
||||
|
||||
const pickProvider = async (user: User, name: RegExp) => {
|
||||
await user.click(screen.getByRole("combobox", { name: /provider/i }));
|
||||
await user.click(await screen.findByRole("option", { name }));
|
||||
};
|
||||
|
||||
const pickModel = async (user: User, name: string) => {
|
||||
await user.click(await screen.findByPlaceholderText("Select models"));
|
||||
await user.click(await screen.findByRole("option", { name }));
|
||||
await user.keyboard("{Escape}");
|
||||
};
|
||||
|
||||
const fillApiKey = (label: string) =>
|
||||
fireEvent.change(screen.getByLabelText(new RegExp(label)), { target: { value: "sk-test" } });
|
||||
|
||||
describe("AddModelForm", () => {
|
||||
it("should render", async () => {
|
||||
const mockUseAuthorized = vi.mocked(await import("@/app/(dashboard)/hooks/useAuthorized"));
|
||||
|
|
@ -444,10 +480,15 @@ describe("AddModelForm", () => {
|
|||
it("shows only the Close button in the connection test dialog footer", async () => {
|
||||
const mockUseAuthorized = vi.mocked(await import("@/app/(dashboard)/hooks/useAuthorized"));
|
||||
mockUseAuthorized.default.mockReturnValue(mockAuthorizedUser("proxy_admin", "user-1", true));
|
||||
const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never });
|
||||
|
||||
renderWithProviders(<AddModelForm {...createTestProps()} />);
|
||||
await screen.findByText("Provider");
|
||||
await pickProvider(user, /OpenAI/);
|
||||
fillApiKey("OpenAI API Key");
|
||||
await pickModel(user, "gpt-4");
|
||||
|
||||
await userEvent.click(await screen.findByTestId("test-connect-btn"));
|
||||
await user.click(screen.getByTestId("test-connect-btn"));
|
||||
|
||||
const dialog = await screen.findByRole("dialog");
|
||||
const footer = dialog.querySelector('[data-slot="dialog-footer"]');
|
||||
|
|
@ -786,6 +827,180 @@ describe("AddModelForm", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("Test Connect validates the form the way Add Model does", () => {
|
||||
beforeEach(() => {
|
||||
vi.mocked(testConnectionRequest).mockClear();
|
||||
});
|
||||
|
||||
const renderForm = async (selectedProvider: Providers = Providers.OpenAI) => {
|
||||
const mockUseAuthorized = vi.mocked(await import("@/app/(dashboard)/hooks/useAuthorized"));
|
||||
mockUseAuthorized.default.mockReturnValue(mockAuthorizedUser("proxy_admin", "user-1", true));
|
||||
const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never });
|
||||
const props = { ...createTestProps(), selectedProvider };
|
||||
renderWithProviders(<AddModelForm {...props} />);
|
||||
await screen.findByText("Provider");
|
||||
return { user, props, clickTestConnect: () => user.click(screen.getByTestId("test-connect-btn")) };
|
||||
};
|
||||
|
||||
const expectTestRun = async (litellmParams: Record<string, unknown>) => {
|
||||
expect(await screen.findByTestId("connection-success-msg")).toBeInTheDocument();
|
||||
expect(testConnectionRequest).toHaveBeenCalledWith(
|
||||
"test-access-token",
|
||||
expect.objectContaining(litellmParams),
|
||||
expect.anything(),
|
||||
undefined,
|
||||
);
|
||||
};
|
||||
|
||||
const expectBlockedWith = async (inlineError: string) => {
|
||||
expect(await screen.findByText(inlineError)).toBeInTheDocument();
|
||||
expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
|
||||
expect(testConnectionRequest).not.toHaveBeenCalled();
|
||||
};
|
||||
|
||||
it("shows the inline required errors instead of testing a fresh form", async () => {
|
||||
const { clickTestConnect } = await renderForm();
|
||||
|
||||
await clickTestConnect();
|
||||
|
||||
await expectBlockedWith("Please enter at least one model.");
|
||||
expect(screen.getAllByText("Required")).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("runs the connection test once provider, auth and model are filled in", async () => {
|
||||
const { user, clickTestConnect } = await renderForm();
|
||||
await pickProvider(user, /OpenAI/);
|
||||
fillApiKey("OpenAI API Key");
|
||||
await pickModel(user, "gpt-4");
|
||||
|
||||
await clickTestConnect();
|
||||
|
||||
await expectTestRun({ api_key: "sk-test", model: "gpt-4" });
|
||||
});
|
||||
|
||||
it("shows emptied inputs and inline errors instead of testing after a successful Add Model", async () => {
|
||||
const { user, props, clickTestConnect } = await renderForm();
|
||||
props.handleOk.mockImplementation(async () => {
|
||||
props.form.reset({ litellm_credential_name: null });
|
||||
return true;
|
||||
});
|
||||
await pickProvider(user, /OpenAI/);
|
||||
fillApiKey("OpenAI API Key");
|
||||
await pickModel(user, "gpt-4");
|
||||
await user.click(screen.getByTestId("add-model-btn"));
|
||||
await waitFor(() => expect(screen.getByLabelText(/OpenAI API Key/)).toHaveValue(""));
|
||||
expect(props.handleOk).toHaveBeenCalledOnce();
|
||||
|
||||
await clickTestConnect();
|
||||
|
||||
await expectBlockedWith("Please enter at least one model.");
|
||||
});
|
||||
|
||||
it("runs the connection test for the provider wildcard", async () => {
|
||||
const { user, clickTestConnect } = await renderForm();
|
||||
await pickProvider(user, /OpenAI/);
|
||||
await pickModel(user, "All OpenAI Models (Wildcard)");
|
||||
fillApiKey("OpenAI API Key");
|
||||
|
||||
await clickTestConnect();
|
||||
|
||||
await expectTestRun({ model: "openai/*" });
|
||||
});
|
||||
|
||||
it("asks for the custom model name before testing a custom model", async () => {
|
||||
const { user, clickTestConnect } = await renderForm();
|
||||
await pickProvider(user, /OpenAI/);
|
||||
fillApiKey("OpenAI API Key");
|
||||
await pickModel(user, "Custom Model Name (Enter below)");
|
||||
|
||||
await clickTestConnect();
|
||||
await expectBlockedWith("Please enter a custom model name.");
|
||||
|
||||
fireEvent.change(screen.getByPlaceholderText("Enter custom model name"), { target: { value: "my-fine-tune" } });
|
||||
await clickTestConnect();
|
||||
|
||||
await expectTestRun({ model: "my-fine-tune" });
|
||||
});
|
||||
|
||||
it("runs the connection test for an Azure deployment name", async () => {
|
||||
const { user, clickTestConnect } = await renderForm(Providers.Azure);
|
||||
await pickProvider(user, /Azure/);
|
||||
fireEvent.change(screen.getByLabelText(/LiteLLM Model Name/), { target: { value: "my-deployment" } });
|
||||
|
||||
await clickTestConnect();
|
||||
await expectBlockedWith("Required");
|
||||
|
||||
fireEvent.change(screen.getByLabelText(/API Base/), { target: { value: "https://example.openai.azure.com" } });
|
||||
await clickTestConnect();
|
||||
|
||||
await expectTestRun({ model: "azure/my-deployment", api_base: "https://example.openai.azure.com" });
|
||||
});
|
||||
|
||||
it("asks for a credential before testing a credential-only auth type", async () => {
|
||||
const errorToast = vi.spyOn(toast, "error");
|
||||
const { user, clickTestConnect } = await renderForm(Providers.MICROSOFT_365_COPILOT);
|
||||
await pickProvider(user, /Microsoft 365 Copilot/);
|
||||
await pickModel(user, "gpt-4");
|
||||
expect(await screen.findByRole("button", { name: "Create credential" })).toBeInTheDocument();
|
||||
|
||||
await clickTestConnect();
|
||||
expect(errorToast).toHaveBeenCalledWith("Create or select a credential for this auth type");
|
||||
expect(screen.queryByRole("dialog")).not.toBeInTheDocument();
|
||||
expect(testConnectionRequest).not.toHaveBeenCalled();
|
||||
errorToast.mockRestore();
|
||||
|
||||
await user.click(screen.getByPlaceholderText("Select or search for existing credentials"));
|
||||
await user.click(await screen.findByRole("option", { name: /test-credential/ }));
|
||||
await clickTestConnect();
|
||||
|
||||
await expectTestRun({ litellm_credential_name: "test-credential" });
|
||||
});
|
||||
|
||||
it("runs the connection test with a credential just created from the credential dialog", async () => {
|
||||
const { user, clickTestConnect } = await renderForm();
|
||||
await pickProvider(user, /OpenAI/);
|
||||
await pickModel(user, "gpt-4");
|
||||
await user.click(screen.getByRole("button", { name: "Use workload identity federation" }));
|
||||
fireEvent.change(screen.getByLabelText("Credential Name:"), { target: { value: "openai-federated" } });
|
||||
fireEvent.change(screen.getByLabelText(/Service Account ID/), { target: { value: "svc_new" } });
|
||||
await user.click(screen.getByRole("button", { name: "Add Credential" }));
|
||||
await waitFor(() => expect(screen.queryByLabelText(/OpenAI API Key/)).not.toBeInTheDocument());
|
||||
|
||||
await clickTestConnect();
|
||||
|
||||
await expectTestRun({ litellm_credential_name: "openai-federated", model: "gpt-4" });
|
||||
});
|
||||
|
||||
it("runs the connection test after the public model name is edited in the mapping table", async () => {
|
||||
const { user, clickTestConnect } = await renderForm();
|
||||
await pickProvider(user, /OpenAI/);
|
||||
fillApiKey("OpenAI API Key");
|
||||
await pickModel(user, "gpt-4");
|
||||
const publicName = screen.getByDisplayValue("gpt-4");
|
||||
await user.clear(publicName);
|
||||
const apiKey = screen.getByLabelText(/OpenAI API Key/);
|
||||
await user.clear(apiKey);
|
||||
await user.type(apiKey, "sk-rotated");
|
||||
await user.type(publicName, "my-gpt");
|
||||
|
||||
await clickTestConnect();
|
||||
|
||||
await expectTestRun({ api_key: "sk-rotated", model: "gpt-4" });
|
||||
});
|
||||
|
||||
it("shows the mapping error instead of testing when the public model name is cleared", async () => {
|
||||
const { user, clickTestConnect } = await renderForm();
|
||||
await pickProvider(user, /OpenAI/);
|
||||
fillApiKey("OpenAI API Key");
|
||||
await pickModel(user, "gpt-4");
|
||||
fireEvent.change(screen.getByDisplayValue("gpt-4"), { target: { value: "" } });
|
||||
|
||||
await clickTestConnect();
|
||||
|
||||
await expectBlockedWith("All model mappings must have valid public names");
|
||||
});
|
||||
});
|
||||
|
||||
describe("cache control bindings reach the parent form store", () => {
|
||||
const renderWithForm = async () => {
|
||||
const mockUseAuthorized = vi.mocked(await import("@/app/(dashboard)/hooks/useAuthorized"));
|
||||
|
|
|
|||
|
|
@ -134,15 +134,20 @@ const AddModelForm: React.FC<AddModelFormProps> = ({
|
|||
setIsCredentialModalOpen(true);
|
||||
};
|
||||
|
||||
const handleSubmit = async () => {
|
||||
if (selectedAuthType?.credentialOnly && !selectedCredentialName) {
|
||||
const rejectMissingCredential = (): boolean => {
|
||||
const missing = selectedAuthType?.credentialOnly === true && !selectedCredentialName;
|
||||
if (missing) {
|
||||
toast.error("Create or select a credential for this auth type");
|
||||
return false;
|
||||
}
|
||||
return handleOk();
|
||||
return missing;
|
||||
};
|
||||
|
||||
const handleSubmit = async () => (rejectMissingCredential() ? false : handleOk());
|
||||
|
||||
const handleTestConnection = async () => {
|
||||
if (rejectMissingCredential() || !(await form.trigger(registry.mountedNames() as string[]))) {
|
||||
return;
|
||||
}
|
||||
setIsTestingConnection(true);
|
||||
setConnectionTestId(`test-${Date.now()}`);
|
||||
setIsResultModalVisible(true);
|
||||
|
|
|
|||
|
|
@ -93,6 +93,19 @@ describe("ModelConnectionTest", () => {
|
|||
expect(toast.success).toHaveBeenCalledWith("Copied to clipboard");
|
||||
});
|
||||
|
||||
it("asks for the required fields instead of crashing when nothing could be prepared", async () => {
|
||||
vi.mocked(prepareModelAddRequest).mockResolvedValue([] as never);
|
||||
|
||||
render(<ModelConnectionTest formValues={{}} accessToken="sk-test" testMode="chat" />);
|
||||
await finishConnectionTest();
|
||||
|
||||
expect(screen.getByTestId("connection-failure-msg")).toHaveTextContent("Connection to this model failed");
|
||||
expect(
|
||||
screen.getByText("Required fields are not filled in. Fill them in and test the connection again."),
|
||||
).toBeInTheDocument();
|
||||
expect(testConnectionRequest).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("shows a preparation failure without sending a connection request", async () => {
|
||||
vi.mocked(prepareModelAddRequest).mockResolvedValue(null as never);
|
||||
|
||||
|
|
|
|||
|
|
@ -43,8 +43,12 @@ const ModelConnectionTest: React.FC<ModelConnectionTestProps> = ({
|
|||
try {
|
||||
const result = await prepareModelAddRequest(formValues, accessToken, null);
|
||||
|
||||
if (!result) {
|
||||
setError("Failed to prepare model data. Please check your form inputs.");
|
||||
if (!result?.length) {
|
||||
setError(
|
||||
result
|
||||
? "Required fields are not filled in. Fill them in and test the connection again."
|
||||
: "Failed to prepare model data. Please check your form inputs.",
|
||||
);
|
||||
setIsSuccess(false);
|
||||
setIsLoading(false);
|
||||
return;
|
||||
|
|
|
|||
|
|
@ -112,6 +112,13 @@ vi.mock("../networking", async () => {
|
|||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
provider: "Oracle",
|
||||
provider_display_name: Providers.Oracle,
|
||||
litellm_provider: "oci",
|
||||
default_model_placeholder: "oci/xai.grok-4",
|
||||
credential_fields: [{ key: "oci_key", label: "OCI Private Key (PEM)", field_type: "textarea", required: true }],
|
||||
},
|
||||
{
|
||||
provider: "MICROSOFT_365_COPILOT",
|
||||
provider_display_name: Providers.MICROSOFT_365_COPILOT,
|
||||
|
|
@ -182,6 +189,15 @@ const VertexCredentialsProbe = () => {
|
|||
return <output data-testid="vertex-credentials">{String(watch("vertex_credentials") ?? "")}</output>;
|
||||
};
|
||||
|
||||
const ResetFormButton = () => {
|
||||
const form = useFormContext<MountedFormValues>();
|
||||
return (
|
||||
<button type="button" onClick={() => form.reset({ litellm_credential_name: null })}>
|
||||
Reset form
|
||||
</button>
|
||||
);
|
||||
};
|
||||
|
||||
const ValidationForm = ({ children }: { readonly children: ReactNode }) => {
|
||||
const form = useFormContext<MountedFormValues>();
|
||||
return (
|
||||
|
|
@ -642,6 +658,49 @@ describe("ProviderSpecificFields", () => {
|
|||
expect(await screen.findAllByText("Required")).toHaveLength(3);
|
||||
});
|
||||
|
||||
it("renders typed password and text inputs empty after the form is reset", async () => {
|
||||
render(
|
||||
<QueryClientProvider client={createQueryClient()}>
|
||||
<MountedFormHost>
|
||||
<ProviderSpecificFields selectedProvider={Providers.OpenAI} />
|
||||
<ResetFormButton />
|
||||
</MountedFormHost>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
const apiKey = await screen.findByLabelText("OpenAI API Key");
|
||||
const organization = screen.getByLabelText("OpenAI Organization ID");
|
||||
const apiBase = screen.getByLabelText(/API Base/);
|
||||
expect(apiBase).toHaveValue("https://api.openai.com/v1");
|
||||
fireEvent.change(apiKey, { target: { value: "sk-typed" } });
|
||||
fireEvent.change(organization, { target: { value: "org-typed" } });
|
||||
fireEvent.change(apiBase, { target: { value: "https://proxy.example.test/v1" } });
|
||||
expect(apiKey).toHaveValue("sk-typed");
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "Reset form" }));
|
||||
|
||||
await waitFor(() => expect(apiKey).toHaveValue(""));
|
||||
expect(organization).toHaveValue("");
|
||||
expect(apiBase).toHaveValue("https://api.openai.com/v1");
|
||||
});
|
||||
|
||||
it("renders a typed textarea input empty after the form is reset", async () => {
|
||||
render(
|
||||
<QueryClientProvider client={createQueryClient()}>
|
||||
<MountedFormHost>
|
||||
<ProviderSpecificFields selectedProvider={"Oracle" as Providers} />
|
||||
<ResetFormButton />
|
||||
</MountedFormHost>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
const privateKey = await screen.findByLabelText(/OCI Private Key/);
|
||||
fireEvent.change(privateKey, { target: { value: "-----BEGIN PRIVATE KEY-----" } });
|
||||
expect(privateKey).toHaveValue("-----BEGIN PRIVATE KEY-----");
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "Reset form" }));
|
||||
|
||||
await waitFor(() => expect(privateKey).toHaveValue(""));
|
||||
});
|
||||
|
||||
it("does not render an Auth Type selector for OpenAI", async () => {
|
||||
const queryClient = createQueryClient();
|
||||
render(
|
||||
|
|
|
|||
|
|
@ -301,11 +301,10 @@ const ProviderSpecificFieldsContent: React.FC<ProviderSpecificFieldsProps> = ({
|
|||
return (
|
||||
<Textarea
|
||||
id={control.id}
|
||||
value={control.value as string | undefined}
|
||||
value={(control.value as string | undefined) ?? field.defaultValue ?? ""}
|
||||
onChange={control.onChange}
|
||||
onBlur={control.onBlur}
|
||||
placeholder={field.placeholder}
|
||||
defaultValue={field.defaultValue}
|
||||
rows={6}
|
||||
className="font-mono text-xs"
|
||||
/>
|
||||
|
|
@ -316,11 +315,10 @@ const ProviderSpecificFieldsContent: React.FC<ProviderSpecificFieldsProps> = ({
|
|||
return (
|
||||
<PasswordInput
|
||||
id={control.id}
|
||||
value={control.value as string | undefined}
|
||||
value={(control.value as string | undefined) ?? field.defaultValue ?? ""}
|
||||
onChange={control.onChange}
|
||||
onBlur={control.onBlur}
|
||||
placeholder={field.placeholder}
|
||||
defaultValue={field.defaultValue}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
|
@ -328,11 +326,10 @@ const ProviderSpecificFieldsContent: React.FC<ProviderSpecificFieldsProps> = ({
|
|||
return (
|
||||
<Input
|
||||
id={control.id}
|
||||
value={(control.value as string | undefined) ?? undefined}
|
||||
value={(control.value as string | undefined) ?? field.defaultValue ?? ""}
|
||||
onBlur={control.onBlur}
|
||||
placeholder={field.placeholder}
|
||||
type="text"
|
||||
defaultValue={field.defaultValue}
|
||||
onChange={(event) => {
|
||||
control.onChange(event);
|
||||
if (field.key === "api_base") {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue