From 6f4f4f69df2e2369e95235eaa8a6c0e1aea5a6aa Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Sat, 18 Jul 2026 11:24:03 -0700 Subject: [PATCH] refactor(ui): consolidate Add/Edit credential modals into one CredentialModal (#32572) * refactor(ui): consolidate Add/Edit credential modals into one CredentialModal AddCredentialModal and EditCredentialModal were ~90% identical: the same provider select, ProviderSpecificFields, and submit/filter logic, differing only in title, button text, edit-mode prefill, and the disabled credential name. Replace both with a single CredentialModal driven by a mode: 'add' | 'edit' prop, and point the two call sites in credentials.tsx at it. Removes ~120 lines of duplication and drops the no-explicit-any and no-restricted-imports baselines. The two per-file tests merge into one CredentialModal.test.tsx covering both modes (add: editable empty name; edit: prefilled, disabled name; provider fields render). * refactor(ui): derive credential name disabled state from mode, not data The disabled flag on the credential name field was tied to whether existingCredential?.credential_name is truthy, an artifact of the old EditCredentialModal. Drive it from the isEdit flag like the rest of the component so mode='add' with a stray existingCredential can't disable the field and mode='edit' with an empty name can't leave it editable. Behavior is unchanged for real call sites; adds a regression test for the edit-with- empty-name case. * refactor(ui): prefill credential form declaratively instead of via useEffect The edit-mode form was seeded with an imperative form.setFieldsValue inside a useEffect that also set React state (setSelectedProvider), an antd anti- pattern carried over from the old EditCredentialModal. Both call sites mount the modal fresh with existingCredential already present (conditional && plus destroyOnHidden), so there is no 'prop arrives after mount' case to handle. Replace it with antd's declarative initialValues on the Form and a lazy useState initializer for the provider. Removes the effect, its react-hooks/set-state-in-effect suppression and exhaustive-deps warning, and one any cast; behavior is unchanged (edit now shows the real provider on first paint instead of flashing the default). Existing tests cover prefill and the disabled name field. --- ui/litellm-dashboard/eslint-suppressions.json | 10 +- .../model_add/AddCredentialModal.test.tsx | 108 ------------- .../model_add/CredentialModal.test.tsx | 140 ++++++++++++++++ ...redentialModal.tsx => CredentialModal.tsx} | 70 ++++---- .../model_add/EditCredentialModal.test.tsx | 123 -------------- .../model_add/EditCredentialModal.tsx | 150 ------------------ .../src/components/model_add/credentials.tsx | 13 +- 7 files changed, 191 insertions(+), 423 deletions(-) delete mode 100644 ui/litellm-dashboard/src/components/model_add/AddCredentialModal.test.tsx create mode 100644 ui/litellm-dashboard/src/components/model_add/CredentialModal.test.tsx rename ui/litellm-dashboard/src/components/model_add/{AddCredentialModal.tsx => CredentialModal.tsx} (71%) delete mode 100644 ui/litellm-dashboard/src/components/model_add/EditCredentialModal.test.tsx delete mode 100644 ui/litellm-dashboard/src/components/model_add/EditCredentialModal.tsx diff --git a/ui/litellm-dashboard/eslint-suppressions.json b/ui/litellm-dashboard/eslint-suppressions.json index 90b0c84244e..dcf482450e9 100644 --- a/ui/litellm-dashboard/eslint-suppressions.json +++ b/ui/litellm-dashboard/eslint-suppressions.json @@ -1885,19 +1885,11 @@ "count": 1 } }, - "src/components/model_add/AddCredentialModal.tsx": { + "src/components/model_add/CredentialModal.tsx": { "no-restricted-imports": { "count": 1 } }, - "src/components/model_add/EditCredentialModal.tsx": { - "no-restricted-imports": { - "count": 1 - }, - "react-hooks/set-state-in-effect": { - "count": 1 - } - }, "src/components/model_add/credentials.tsx": { "no-restricted-imports": { "count": 1 diff --git a/ui/litellm-dashboard/src/components/model_add/AddCredentialModal.test.tsx b/ui/litellm-dashboard/src/components/model_add/AddCredentialModal.test.tsx deleted file mode 100644 index aee7a0cdd1d..00000000000 --- a/ui/litellm-dashboard/src/components/model_add/AddCredentialModal.test.tsx +++ /dev/null @@ -1,108 +0,0 @@ -import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { render, screen, waitFor } from "@testing-library/react"; -import { describe, expect, it, vi } from "vitest"; -import { Providers } from "../provider_info_helpers"; -import AddCredentialModal from "./AddCredentialModal"; - -vi.mock("../networking", async () => { - const actual = await vi.importActual("../networking"); - return { - ...actual, - getProviderCreateMetadata: vi.fn().mockResolvedValue([ - { - provider: "OpenAI", - provider_display_name: Providers.OpenAI, - litellm_provider: "openai", - default_model_placeholder: "gpt-3.5-turbo", - credential_fields: [ - { - key: "api_key", - label: "OpenAI API Key", - field_type: "password", - required: true, - }, - { - key: "api_base", - label: "API Base", - field_type: "text", - placeholder: "https://api.openai.com/v1", - }, - ], - }, - { - provider: "Anthropic", - provider_display_name: Providers.Anthropic, - litellm_provider: "anthropic", - default_model_placeholder: "claude-3-opus-20240229", - credential_fields: [ - { - key: "api_key", - label: "Anthropic API Key", - field_type: "password", - required: true, - }, - ], - }, - ]), - }; -}); - -const createQueryClient = () => - new QueryClient({ - defaultOptions: { - queries: { - retry: false, - gcTime: 0, - }, - }, - }); - -const mockUploadProps = { - beforeUpload: vi.fn(), - onChange: vi.fn(), -}; - -describe("AddCredentialModal", () => { - it("should render", () => { - const queryClient = createQueryClient(); - const onCancel = vi.fn(); - const onAddCredential = vi.fn(); - - render( - - - , - ); - - expect(screen.getByText("Add New Credential")).toBeInTheDocument(); - expect(screen.getByLabelText("Credential Name:")).toBeInTheDocument(); - expect(screen.getByLabelText("Provider:")).toBeInTheDocument(); - }); - - it("should show the correct provider fields", async () => { - const queryClient = createQueryClient(); - const onCancel = vi.fn(); - const onAddCredential = vi.fn(); - - render( - - - , - ); - - await waitFor(() => { - expect(screen.getByLabelText("OpenAI API Key")).toBeInTheDocument(); - expect(screen.getByPlaceholderText("https://api.openai.com/v1")).toBeInTheDocument(); - }); - }); -}); diff --git a/ui/litellm-dashboard/src/components/model_add/CredentialModal.test.tsx b/ui/litellm-dashboard/src/components/model_add/CredentialModal.test.tsx new file mode 100644 index 00000000000..6804d0cba92 --- /dev/null +++ b/ui/litellm-dashboard/src/components/model_add/CredentialModal.test.tsx @@ -0,0 +1,140 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render, screen, waitFor } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import { Providers } from "../provider_info_helpers"; +import { CredentialItem } from "../networking"; +import CredentialModal from "./CredentialModal"; + +vi.mock("../networking", async () => { + const actual = await vi.importActual("../networking"); + return { + ...actual, + getProviderCreateMetadata: vi.fn().mockResolvedValue([ + { + provider: "OpenAI", + provider_display_name: Providers.OpenAI, + litellm_provider: "openai", + default_model_placeholder: "gpt-3.5-turbo", + credential_fields: [ + { + key: "api_key", + label: "OpenAI API Key", + field_type: "password", + required: true, + }, + { + key: "api_base", + label: "API Base", + field_type: "text", + placeholder: "https://api.openai.com/v1", + }, + ], + }, + { + provider: "Anthropic", + provider_display_name: Providers.Anthropic, + litellm_provider: "anthropic", + default_model_placeholder: "claude-3-opus-20240229", + credential_fields: [ + { + key: "api_key", + label: "Anthropic API Key", + field_type: "password", + required: true, + }, + ], + }, + ]), + }; +}); + +const createQueryClient = () => + new QueryClient({ + defaultOptions: { + queries: { + retry: false, + gcTime: 0, + }, + }, + }); + +const mockUploadProps = { + beforeUpload: vi.fn(), + onChange: vi.fn(), +}; + +const mockCredential: CredentialItem = { + credential_name: "test-credential", + credential_values: { + api_key: "test-api-key", + api_base: "https://api.test.com", + }, + credential_info: { + custom_llm_provider: Providers.OpenAI, + }, +}; + +const renderModal = (props: Partial> = {}) => + render( + + + , + ); + +describe("CredentialModal", () => { + describe("add mode", () => { + it("renders the add title and an editable credential name", () => { + renderModal({ mode: "add" }); + + expect(screen.getByText("Add New Credential")).toBeInTheDocument(); + expect(screen.getByText("Add Credential")).toBeInTheDocument(); + const nameInput = screen.getByLabelText("Credential Name:") as HTMLInputElement; + expect(nameInput.value).toBe(""); + expect(nameInput.disabled).toBe(false); + }); + + it("shows provider-specific fields for the selected provider", async () => { + renderModal({ mode: "add" }); + + await waitFor(() => { + expect(screen.getByLabelText("OpenAI API Key")).toBeInTheDocument(); + expect(screen.getByPlaceholderText("https://api.openai.com/v1")).toBeInTheDocument(); + }); + }); + }); + + describe("edit mode", () => { + it("renders the edit title and update button", () => { + renderModal({ mode: "edit", existingCredential: mockCredential }); + + expect(screen.getByText("Edit Credential")).toBeInTheDocument(); + expect(screen.getByText("Update Credential")).toBeInTheDocument(); + }); + + it("prefills the credential name and disables it", async () => { + renderModal({ mode: "edit", existingCredential: mockCredential }); + + await waitFor(() => { + const nameInput = screen.getByLabelText("Credential Name:") as HTMLInputElement; + expect(nameInput.value).toBe("test-credential"); + expect(nameInput.disabled).toBe(true); + }); + }); + + it("disables the name from the mode, not the credential's name value", () => { + renderModal({ + mode: "edit", + existingCredential: { ...mockCredential, credential_name: "" }, + }); + + expect((screen.getByLabelText("Credential Name:") as HTMLInputElement).disabled).toBe(true); + }); + }); +}); diff --git a/ui/litellm-dashboard/src/components/model_add/AddCredentialModal.tsx b/ui/litellm-dashboard/src/components/model_add/CredentialModal.tsx similarity index 71% rename from ui/litellm-dashboard/src/components/model_add/AddCredentialModal.tsx rename to ui/litellm-dashboard/src/components/model_add/CredentialModal.tsx index b86a379d3d1..c92a4a90578 100644 --- a/ui/litellm-dashboard/src/components/model_add/AddCredentialModal.tsx +++ b/ui/litellm-dashboard/src/components/model_add/CredentialModal.tsx @@ -1,23 +1,47 @@ import { TextInput } from "@tremor/react"; import { Select as AntdSelect, Button, Form, Modal, Tooltip, Typography } from "antd"; import type { UploadProps } from "antd/es/upload"; -import React, { useState } from "react"; +import { useState } from "react"; import ProviderSpecificFields from "../add_model/provider_specific_fields"; +import { CredentialItem } from "../networking"; import { Providers, providerLogoMap } from "../provider_info_helpers"; import { resolveLogoSrc } from "@/lib/assetPaths"; import { resetCredentialFormOnProviderChange } from "./credential_form_helpers"; + const { Link } = Typography; -interface AddCredentialsModalProps { +interface CredentialModalProps { open: boolean; onCancel: () => void; - onAddCredential: (values: any) => void; + onSubmit: (values: any) => void; uploadProps: UploadProps; + mode: "add" | "edit"; + existingCredential?: CredentialItem | null; } -const AddCredentialsModal: React.FC = ({ open, onCancel, onAddCredential, uploadProps }) => { +export default function CredentialModal({ + open, + onCancel, + onSubmit, + uploadProps, + mode, + existingCredential = null, +}: CredentialModalProps) { + const isEdit = mode === "edit"; const [form] = Form.useForm(); - const [selectedProvider, setSelectedProvider] = useState(Providers.OpenAI); + const [selectedProvider, setSelectedProvider] = useState( + (existingCredential?.credential_info.custom_llm_provider as Providers) ?? Providers.OpenAI, + ); + + const initialValues = existingCredential + ? { + credential_name: existingCredential.credential_name, + custom_llm_provider: existingCredential.credential_info.custom_llm_provider, + ...Object.fromEntries( + Object.entries(existingCredential.credential_values || {}).map(([key, value]) => [key, value ?? null]), + ), + } + : undefined; const handleSubmit = (values: any) => { const filteredValues = Object.entries(values).reduce((acc, [key, value]) => { @@ -26,32 +50,33 @@ const AddCredentialsModal: React.FC = ({ open, onCance } return acc; }, {} as any); - onAddCredential(filteredValues); + onSubmit(filteredValues); + form.resetFields(); + }; + + const closeAndReset = () => { + onCancel(); form.resetFields(); }; return ( { - onCancel(); - form.resetFields(); - }} + onCancel={closeAndReset} footer={null} width={600} + destroyOnHidden={isEdit} > -
- {/* Credential Name */} + - + - {/* Provider Selection */} = ({ open, onCance - {/* Modal Footer */}
Need Help?
- - +
); -}; - -export default AddCredentialsModal; +} diff --git a/ui/litellm-dashboard/src/components/model_add/EditCredentialModal.test.tsx b/ui/litellm-dashboard/src/components/model_add/EditCredentialModal.test.tsx deleted file mode 100644 index def3b4f6cd7..00000000000 --- a/ui/litellm-dashboard/src/components/model_add/EditCredentialModal.test.tsx +++ /dev/null @@ -1,123 +0,0 @@ -import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { render, screen, waitFor } from "@testing-library/react"; -import { describe, expect, it, vi } from "vitest"; -import { Providers } from "../provider_info_helpers"; -import { CredentialItem } from "../networking"; -import EditCredentialModal from "./EditCredentialModal"; - -vi.mock("../networking", async () => { - const actual = await vi.importActual("../networking"); - return { - ...actual, - getProviderCreateMetadata: vi.fn().mockResolvedValue([ - { - provider: "OpenAI", - provider_display_name: Providers.OpenAI, - litellm_provider: "openai", - default_model_placeholder: "gpt-3.5-turbo", - credential_fields: [ - { - key: "api_key", - label: "OpenAI API Key", - field_type: "password", - required: true, - }, - { - key: "api_base", - label: "API Base", - field_type: "text", - placeholder: "https://api.openai.com/v1", - }, - ], - }, - { - provider: "Anthropic", - provider_display_name: Providers.Anthropic, - litellm_provider: "anthropic", - default_model_placeholder: "claude-3-opus-20240229", - credential_fields: [ - { - key: "api_key", - label: "Anthropic API Key", - field_type: "password", - required: true, - }, - ], - }, - ]), - }; -}); - -const createQueryClient = () => - new QueryClient({ - defaultOptions: { - queries: { - retry: false, - gcTime: 0, - }, - }, - }); - -const mockUploadProps = { - beforeUpload: vi.fn(), - onChange: vi.fn(), -}; - -const mockCredential: CredentialItem = { - credential_name: "test-credential", - credential_values: { - api_key: "test-api-key", - api_base: "https://api.test.com", - }, - credential_info: { - custom_llm_provider: Providers.OpenAI, - }, -}; - -describe("EditCredentialModal", () => { - it("should render", () => { - const queryClient = createQueryClient(); - const onCancel = vi.fn(); - const onUpdateCredential = vi.fn(); - - render( - - - , - ); - - expect(screen.getByText("Edit Credential")).toBeInTheDocument(); - expect(screen.getByLabelText("Credential Name:")).toBeInTheDocument(); - expect(screen.getByLabelText("Provider:")).toBeInTheDocument(); - }); - - it("should render initial values", async () => { - const queryClient = createQueryClient(); - const onCancel = vi.fn(); - const onUpdateCredential = vi.fn(); - - render( - - - , - ); - - await waitFor(() => { - const credentialNameInput = screen.getByLabelText("Credential Name:") as HTMLInputElement; - expect(credentialNameInput.value).toBe("test-credential"); - expect(credentialNameInput.disabled).toBe(true); - }); - }); -}); diff --git a/ui/litellm-dashboard/src/components/model_add/EditCredentialModal.tsx b/ui/litellm-dashboard/src/components/model_add/EditCredentialModal.tsx deleted file mode 100644 index d087edc1069..00000000000 --- a/ui/litellm-dashboard/src/components/model_add/EditCredentialModal.tsx +++ /dev/null @@ -1,150 +0,0 @@ -import { TextInput } from "@tremor/react"; -import { Select as AntdSelect, Button, Form, Modal, Tooltip, Typography } from "antd"; -import type { UploadProps } from "antd/es/upload"; -import { useEffect, useState } from "react"; -import ProviderSpecificFields from "../add_model/provider_specific_fields"; -import { CredentialItem } from "../networking"; -import { Providers, providerLogoMap } from "../provider_info_helpers"; -import { resolveLogoSrc } from "@/lib/assetPaths"; -import { resetCredentialFormOnProviderChange } from "./credential_form_helpers"; -const { Link } = Typography; - -interface EditCredentialsModalProps { - open: boolean; - onCancel: () => void; - onUpdateCredential: (values: any) => void; - uploadProps: UploadProps; - existingCredential: CredentialItem | null; -} - -export default function EditCredentialsModal({ - open, - onCancel, - onUpdateCredential, - uploadProps, - existingCredential, -}: EditCredentialsModalProps) { - const [form] = Form.useForm(); - const [selectedProvider, setSelectedProvider] = useState(Providers.Anthropic); - - const handleSubmit = (values: any) => { - const filteredValues = Object.entries(values).reduce((acc, [key, value]) => { - if (value !== "" && value !== undefined && value !== null) { - acc[key] = value; - } - return acc; - }, {} as any); - onUpdateCredential(filteredValues); - form.resetFields(); - }; - - useEffect(() => { - if (existingCredential) { - // Spread all credential_values dynamically, converting undefined/null to null for form compatibility - const credentialValues = Object.entries(existingCredential.credential_values || {}).reduce( - (acc, [key, value]) => { - acc[key] = value ?? null; - return acc; - }, - {} as Record, - ); - - form.setFieldsValue({ - credential_name: existingCredential.credential_name, - custom_llm_provider: existingCredential.credential_info.custom_llm_provider, - ...credentialValues, - }); - setSelectedProvider(existingCredential.credential_info.custom_llm_provider as Providers); - } - }, [existingCredential]); - - return ( - { - onCancel(); - form.resetFields(); - }} - footer={null} - width={600} - destroyOnHidden={true} - > -
- {/* Credential Name */} - - - - - {/* Provider Selection */} - - { - resetCredentialFormOnProviderChange(form, value as Providers, setSelectedProvider); - }} - > - {Object.entries(Providers).map(([providerEnum, providerDisplayName]) => ( - -
- {`${providerEnum} { - const target = e.target as HTMLImageElement; - const parent = target.parentElement; - if (parent) { - const fallbackDiv = document.createElement("div"); - fallbackDiv.className = - "w-5 h-5 rounded-full bg-gray-200 flex items-center justify-center text-xs"; - fallbackDiv.textContent = providerDisplayName.charAt(0); - parent.replaceChild(fallbackDiv, target); - } - }} - /> - {providerDisplayName} -
- - ))} - - - - - - {/* Modal Footer */} -
- - Need Help? - - -
- - -
-
- -
- ); -} diff --git a/ui/litellm-dashboard/src/components/model_add/credentials.tsx b/ui/litellm-dashboard/src/components/model_add/credentials.tsx index 82320b7ff8d..9289888c1ed 100644 --- a/ui/litellm-dashboard/src/components/model_add/credentials.tsx +++ b/ui/litellm-dashboard/src/components/model_add/credentials.tsx @@ -22,8 +22,7 @@ import { UploadProps } from "antd/es/upload"; import { useState } from "react"; import DeleteResourceModal from "../common_components/DeleteResourceModal"; import NotificationsManager from "../molecules/notifications_manager"; -import AddCredentialsTab from "./AddCredentialModal"; -import EditCredentialsModal from "./EditCredentialModal"; +import CredentialModal from "./CredentialModal"; import { useCredentials } from "@/app/(dashboard)/hooks/credentials/useCredentials"; import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; import { isProxyAdminRole } from "@/utils/roles"; @@ -201,18 +200,20 @@ const CredentialsPanel: React.FC = ({ uploadProps }) => { {isAddModalOpen && ( - setIsAddModalOpen(false)} uploadProps={uploadProps} /> )} {isUpdateModalOpen && ( - setIsUpdateModalOpen(false)} />