From 11af05f5f4a95061a89cc5f44ffd5b004dc1d8e6 Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Sat, 8 Nov 2025 16:05:42 -0800 Subject: [PATCH] Small issues (#16406) --- .../src/components/OldTeams.test.tsx | 90 ++++--- .../src/components/OldTeams.tsx | 43 ++-- .../check_openapi_schema.tsx | 3 +- .../src/components/settings.test.tsx | 97 ++++++++ .../src/components/settings.tsx | 222 ++++++++---------- .../src/utils/textUtils.test.ts | 8 + ui/litellm-dashboard/src/utils/textUtils.ts | 8 + 7 files changed, 284 insertions(+), 187 deletions(-) create mode 100644 ui/litellm-dashboard/src/components/settings.test.tsx create mode 100644 ui/litellm-dashboard/src/utils/textUtils.test.ts create mode 100644 ui/litellm-dashboard/src/utils/textUtils.ts diff --git a/ui/litellm-dashboard/src/components/OldTeams.test.tsx b/ui/litellm-dashboard/src/components/OldTeams.test.tsx index 85fa5472ad8..c055a2f85f7 100644 --- a/ui/litellm-dashboard/src/components/OldTeams.test.tsx +++ b/ui/litellm-dashboard/src/components/OldTeams.test.tsx @@ -1,5 +1,7 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { act, fireEvent, render } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { teamCreateCall } from "./networking"; +import OldTeams from "./OldTeams"; vi.mock("./networking", () => ({ teamCreateCall: vi.fn(), @@ -126,7 +128,7 @@ describe("OldTeams - handleCreate organization handling", () => { // Verify we're not sending an empty string expect(formValues.organization_id).not.toBe(""); - + // Verify it's explicitly null, not undefined expect(formValues.organization_id).toBeNull(); }); @@ -189,9 +191,10 @@ describe("OldTeams - handleCreate organization handling", () => { }; // For org admins, organization_id should be required - const hasOrganization = formValues.organization_id !== undefined && - formValues.organization_id !== null && - formValues.organization_id !== ""; + const hasOrganization = + formValues.organization_id !== undefined && + formValues.organization_id !== null && + formValues.organization_id !== ""; if (isOrgAdmin && !hasOrganization) { // This should trigger validation error @@ -222,7 +225,7 @@ describe("OldTeams - handleCreate organization handling", () => { // Type check: organization_id should never be an array expect(Array.isArray(invalidFormValues.organization_id)).toBe(true); - + // Correct it to null if (Array.isArray(invalidFormValues.organization_id)) { invalidFormValues.organization_id = null; @@ -231,6 +234,40 @@ describe("OldTeams - handleCreate organization handling", () => { expect(invalidFormValues.organization_id).toBeNull(); expect(Array.isArray(invalidFormValues.organization_id)).toBe(false); }); + + it("should clear the delete modal when the cancel button is clicked", async () => { + const { getByRole, getByTestId } = render( + , + ); + const deleteTeamButton = getByTestId("delete-team-button"); + act(() => { + fireEvent.click(deleteTeamButton); + }); + expect(getByRole("heading", { name: "Delete Team" })).toBeInTheDocument(); + expect(getByRole("button", { name: "Cancel" })).toBeInTheDocument(); + }); }); describe("OldTeams - helper functions", () => { @@ -267,33 +304,25 @@ describe("OldTeams - helper functions", () => { organization_id: "org-1", organization_alias: "Org 1", models: [], - members: [ - { user_id: "user-123", user_role: "org_admin" }, - ], + members: [{ user_id: "user-123", user_role: "org_admin" }], }, { organization_id: "org-2", organization_alias: "Org 2", models: [], - members: [ - { user_id: "user-456", user_role: "org_admin" }, - ], + members: [{ user_id: "user-456", user_role: "org_admin" }], }, { organization_id: "org-3", organization_alias: "Org 3", models: [], - members: [ - { user_id: "user-123", user_role: "member" }, - ], + members: [{ user_id: "user-123", user_role: "member" }], }, ]; // Simulate getAdminOrganizations logic const result = organizations.filter((org) => - org.members?.some( - (member) => member.user_id === userID && member.user_role === "org_admin" - ) + org.members?.some((member) => member.user_id === userID && member.user_role === "org_admin"), ); expect(result.length).toBe(1); @@ -307,17 +336,13 @@ describe("OldTeams - helper functions", () => { organization_id: "org-1", organization_alias: "Org 1", models: [], - members: [ - { user_id: "user-123", user_role: "org_admin" }, - ], + members: [{ user_id: "user-123", user_role: "org_admin" }], }, ]; // Simulate getAdminOrganizations logic const result = organizations.filter((org) => - org.members?.some( - (member) => member.user_id === userID && member.user_role === "org_admin" - ) + org.members?.some((member) => member.user_id === userID && member.user_role === "org_admin"), ); expect(result.length).toBe(0); @@ -338,16 +363,12 @@ describe("OldTeams - helper functions", () => { organization_id: "org-1", organization_alias: "Org 1", models: [], - members: [ - { user_id: "user-123", user_role: "org_admin" }, - ], + members: [{ user_id: "user-123", user_role: "org_admin" }], }, ]; const result = organizations.some((org) => - org.members?.some( - (member) => member.user_id === userID && member.user_role === "org_admin" - ) + org.members?.some((member) => member.user_id === userID && member.user_role === "org_admin"), ); expect(result).toBe(true); @@ -361,21 +382,16 @@ describe("OldTeams - helper functions", () => { organization_id: "org-1", organization_alias: "Org 1", models: [], - members: [ - { user_id: "user-123", user_role: "member" }, - ], + members: [{ user_id: "user-123", user_role: "member" }], }, ]; const isAdmin = userRole === "Admin"; const isOrgAdmin = organizations.some((org) => - org.members?.some( - (member) => member.user_id === userID && member.user_role === "org_admin" - ) + org.members?.some((member) => member.user_id === userID && member.user_role === "org_admin"), ); expect(isAdmin || isOrgAdmin).toBe(false); }); }); }); - diff --git a/ui/litellm-dashboard/src/components/OldTeams.tsx b/ui/litellm-dashboard/src/components/OldTeams.tsx index 5e40f9e257d..56c73e86914 100644 --- a/ui/litellm-dashboard/src/components/OldTeams.tsx +++ b/ui/litellm-dashboard/src/components/OldTeams.tsx @@ -1,18 +1,8 @@ import React, { useState, useEffect } from "react"; import { Typography } from "antd"; -import { - teamDeleteCall, - Organization, - fetchMCPAccessGroups, -} from "./networking"; +import { teamDeleteCall, Organization, fetchMCPAccessGroups } from "./networking"; import { fetchTeams } from "./common_components/fetch_teams"; -import { - PencilAltIcon, - RefreshIcon, - TrashIcon, - ChevronDownIcon, - ChevronRightIcon, -} from "@heroicons/react/outline"; +import { PencilAltIcon, RefreshIcon, TrashIcon, ChevronDownIcon, ChevronRightIcon } from "@heroicons/react/outline"; import { Button as Button2, Modal, Form, Input, Select as Select2, Tooltip } from "antd"; import NumericalInput from "./shared/numerical_input"; import { @@ -87,11 +77,7 @@ interface EditTeamModalProps { onSubmit: (data: FormData) => void; // Assuming FormData is the type of data to be submitted } -import { - teamCreateCall, - Member, - v2TeamListCall, -} from "./networking"; +import { teamCreateCall, Member, v2TeamListCall } from "./networking"; import { updateExistingKeys } from "@/utils/dataUtils"; interface TeamInfo { @@ -125,7 +111,7 @@ const getOrganizationModels = (organization: Organization | null, userModels: st const canCreateOrManageTeams = ( userRole: string | null, userID: string | null, - organizations: Organization[] | null + organizations: Organization[] | null, ): boolean => { // Admin role always has permission if (userRole === "Admin") { @@ -135,7 +121,7 @@ const canCreateOrManageTeams = ( // Check if user is an org_admin in any organization if (organizations && userID) { return organizations.some((org) => - org.members?.some((member) => member.user_id === userID && member.user_role === "org_admin") + org.members?.some((member) => member.user_id === userID && member.user_role === "org_admin"), ); } @@ -145,7 +131,7 @@ const canCreateOrManageTeams = ( const getAdminOrganizations = ( userRole: string | null, userID: string | null, - organizations: Organization[] | null + organizations: Organization[] | null, ): Organization[] => { // Global Admin can see all organizations if (userRole === "Admin") { @@ -155,7 +141,7 @@ const getAdminOrganizations = ( // Org Admin can only see organizations they're an admin for if (organizations && userID) { return organizations.filter((org) => - org.members?.some((member) => member.user_id === userID && member.user_role === "org_admin") + org.members?.some((member) => member.user_id === userID && member.user_role === "org_admin"), ); } @@ -235,7 +221,7 @@ const Teams: React.FC = ({ useEffect(() => { if (isTeamModalVisible) { const adminOrgs = getAdminOrganizations(userRole, userID, organizations); - + // If there's exactly one organization the user is admin for, preselect it if (adminOrgs.length === 1) { const org = adminOrgs[0]; @@ -347,7 +333,7 @@ const Teams: React.FC = ({ try { await teamDeleteCall(accessToken, teamToDelete); // Successfully completed the deletion. Update the state to trigger a rerender. - fetchTeams(accessToken, userID, userRole, currentOrg, setTeams); + await fetchTeams(accessToken, userID, userRole, currentOrg, setTeams); } catch (error) { console.error("Error deleting the team:", error); // Handle any error situations, such as displaying an error message to the user. @@ -356,12 +342,14 @@ const Teams: React.FC = ({ // Close the confirmation modal and reset the teamToDelete setIsDeleteModalOpen(false); setTeamToDelete(null); + setDeleteConfirmInput(""); }; const cancelDelete = () => { // Close the confirmation modal and reset the teamToDelete setIsDeleteModalOpen(false); setTeamToDelete(null); + setDeleteConfirmInput(""); }; useEffect(() => { @@ -961,6 +949,7 @@ const Teams: React.FC = ({ onClick={() => handleDelete(team.team_id)} icon={TrashIcon} size="sm" + data-testid="delete-team-button" /> ) : null} @@ -1155,15 +1144,11 @@ const Teams: React.FC = ({ showSearch allowClear={!isOrgAdmin} disabled={isSingleOrg} - placeholder={ - hasNoOrgs - ? "No organizations available" - : "Search or select an Organization" - } + placeholder={hasNoOrgs ? "No organizations available" : "Search or select an Organization"} onChange={(value) => { form.setFieldValue("organization_id", value); setCurrentOrgForCreateTeam( - adminOrgs?.find((org) => org.organization_id === value) || null + adminOrgs?.find((org) => org.organization_id === value) || null, ); }} filterOption={(input, option) => { diff --git a/ui/litellm-dashboard/src/components/common_components/check_openapi_schema.tsx b/ui/litellm-dashboard/src/components/common_components/check_openapi_schema.tsx index 85eeac4644e..9697ebad788 100644 --- a/ui/litellm-dashboard/src/components/common_components/check_openapi_schema.tsx +++ b/ui/litellm-dashboard/src/components/common_components/check_openapi_schema.tsx @@ -4,6 +4,7 @@ import { TextInput } from "@tremor/react"; import { InfoCircleOutlined } from "@ant-design/icons"; import { Tooltip } from "antd"; import { getOpenAPISchema } from "../networking"; +import { formatLabel } from "@/utils/textUtils"; interface SchemaProperty { type?: string; @@ -152,7 +153,7 @@ const SchemaFormFields: React.FC = ({ const type = getPropertyType(property); const isRequired = schemaProperties?.required?.includes(key); - const label = overrideLabels[key] || property.title || key; + const label = overrideLabels[key] || property.title || formatLabel(key); const tooltip = overrideTooltips[key] || property.description; const rules = []; diff --git a/ui/litellm-dashboard/src/components/settings.test.tsx b/ui/litellm-dashboard/src/components/settings.test.tsx new file mode 100644 index 00000000000..20f4988c168 --- /dev/null +++ b/ui/litellm-dashboard/src/components/settings.test.tsx @@ -0,0 +1,97 @@ +import { render, waitFor } from "@testing-library/react"; +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { alertingSettingsCall, getCallbacksCall } from "./networking"; +import Settings from "./settings"; + +vi.mock("./networking", () => ({ + getCallbacksCall: vi.fn(), + setCallbacksCall: vi.fn(), + serviceHealthCheck: vi.fn(), + deleteCallback: vi.fn(), + alertingSettingsCall: vi.fn().mockResolvedValue([]), +})); + +vi.mock("./molecules/notifications_manager", () => ({ + __esModule: true, + default: { + success: vi.fn(), + fromBackend: vi.fn(), + info: vi.fn(), + warning: vi.fn(), + clear: vi.fn(), + }, +})); + +vi.mock("./alerting/alerting_settings", () => ({ + __esModule: true, + default: () =>
Mock Alerting Settings
, +})); + +vi.mock("./email_settings", () => ({ + __esModule: true, + default: () =>
Mock Email Settings
, +})); + +// Polyfill ResizeObserver for components relying on it in tests +if (typeof window !== "undefined" && !window.ResizeObserver) { + window.ResizeObserver = class ResizeObserver { + observe() {} + unobserve() {} + disconnect() {} + }; +} + +beforeAll(() => { + Object.defineProperty(window, "matchMedia", { + writable: true, + value: vi.fn().mockImplementation((query: string) => ({ + matches: false, + media: query, + onchange: null, + addListener: vi.fn(), + removeListener: vi.fn(), + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + dispatchEvent: vi.fn(), + })), + }); +}); + +describe("Settings", () => { + const defaultProps = { + accessToken: "token", + userRole: "admin", + userID: "user-123", + premiumUser: false, + }; + const mockGetCallbacksCall = vi.mocked(getCallbacksCall); + const mockAlertingSettingsCall = vi.mocked(alertingSettingsCall); + + beforeEach(() => { + vi.clearAllMocks(); + mockGetCallbacksCall.mockResolvedValue({ + callbacks: [], + available_callbacks: [], + alerts: [], + }); + mockAlertingSettingsCall.mockResolvedValue([]); + }); + + it("should render the logging callbacks tab when access token is provided", async () => { + const { getByText } = render(); + + await waitFor(() => { + expect(getByText("Active Logging Callbacks")).toBeInTheDocument(); + }); + }); + + it("should display additional settings tabs", async () => { + const { getByText } = render(); + + await waitFor(() => { + expect(getByText("Alerting Types")).toBeInTheDocument(); + expect(getByText("Alerting Settings")).toBeInTheDocument(); + expect(getByText("Email Alerts")).toBeInTheDocument(); + }); + }); +}); diff --git a/ui/litellm-dashboard/src/components/settings.tsx b/ui/litellm-dashboard/src/components/settings.tsx index 727c8b9b511..25836a59753 100644 --- a/ui/litellm-dashboard/src/components/settings.tsx +++ b/ui/litellm-dashboard/src/components/settings.tsx @@ -32,10 +32,7 @@ const { Title, Paragraph } = Typography; import { getCallbacksCall, setCallbacksCall, serviceHealthCheck, deleteCallback } from "./networking"; import AlertingSettings from "./alerting/alerting_settings"; import FormItem from "antd/es/form/FormItem"; -import { - CALLBACK_CONFIGS, - getCallbackById, -} from "./callback_info_helpers"; +import { CALLBACK_CONFIGS, getCallbackById } from "./callback_info_helpers"; import { parseErrorMessage } from "./shared/errorUtils"; interface SettingsPageProps { accessToken: string | null; @@ -216,10 +213,10 @@ const Settings: React.FC = ({ accessToken, userRole, userID, const handleSelectedCallbackChange = (callbackName: string) => { setSelectedCallback(callbackName); - + // Get the callback configuration using the new clean structure const callbackConfig = getCallbackById(callbackName); - + // Get the parameters from the callback configuration if (callbackConfig?.dynamic_params) { const params = Object.keys(callbackConfig.dynamic_params); @@ -228,7 +225,7 @@ const Settings: React.FC = ({ accessToken, userRole, userID, setSelectedCallbackParams([]); } }; - + const handleSaveAlerts = async () => { if (!accessToken) { return; @@ -594,124 +591,109 @@ const Settings: React.FC = ({ accessToken, userRole, userID, wrapperCol={{ span: 16 }} labelAlign="left" > - + - (option?.children?.toString() ?? "") - .toLowerCase() - .includes(input.toLowerCase()) - } - onChange={(value) => { - handleSelectedCallbackChange(value); - }} - > - {CALLBACK_CONFIGS.map((callbackConfig) => ( - -
-
- {/* eslint-disable-next-line @next/next/no-img-element */} - {`${callbackConfig.displayName} { - e.currentTarget.style.display = 'none'; - }} - /> -
- - {callbackConfig.displayName} - + {CALLBACK_CONFIGS.map((callbackConfig) => ( + +
+
+ {/* eslint-disable-next-line @next/next/no-img-element */} + {`${callbackConfig.displayName} { + e.currentTarget.style.display = "none"; + }} + />
- - ))} - - + {callbackConfig.displayName} +
+ + ))} + + - {selectedCallbackParams && selectedCallbackParams.length > 0 && ( -
- {selectedCallbackParams.map((param) => { - // Get the callback configuration to look up parameter types - const callbackConfig = getCallbackById(selectedCallback || ''); - const paramType = callbackConfig?.dynamic_params[param] || "text"; - - const fieldLabel = param.replace(/_/g, " ").replace(/\b\w/g, l => l.toUpperCase()); - - return ( - - {fieldLabel} - * - - } - name={param} - key={param} - className="mb-4" - rules={[ - { - required: true, - message: `Please enter the ${fieldLabel.toLowerCase()}`, - }, - ]} - > - {paramType === "password" ? ( - - ) : paramType === "number" ? ( - - ) : ( - - )} - - ); - })} -
- )} + {selectedCallbackParams && selectedCallbackParams.length > 0 && ( +
+ {selectedCallbackParams.map((param) => { + // Get the callback configuration to look up parameter types + const callbackConfig = getCallbackById(selectedCallback || ""); + const paramType = callbackConfig?.dynamic_params[param] || "text"; -
- - - Add Callback - + const fieldLabel = param.replace(/_/g, " ").replace(/\b\w/g, (l) => l.toUpperCase()); + + return ( + + {fieldLabel} + * + + } + name={param} + key={param} + className="mb-4" + rules={[ + { + required: true, + message: `Please enter the ${fieldLabel.toLowerCase()}`, + }, + ]} + > + {paramType === "password" ? ( + + ) : paramType === "number" ? ( + + ) : ( + + )} + + ); + })}
+ )} + +
+ + Add Callback +
diff --git a/ui/litellm-dashboard/src/utils/textUtils.test.ts b/ui/litellm-dashboard/src/utils/textUtils.test.ts new file mode 100644 index 00000000000..2a036104b58 --- /dev/null +++ b/ui/litellm-dashboard/src/utils/textUtils.test.ts @@ -0,0 +1,8 @@ +import { describe, it, expect } from "vitest"; +import { formatLabel } from "./textUtils"; + +describe("textUtils", () => { + it("should format label", () => { + expect(formatLabel("test_label")).toBe("Test Label"); + }); +}); diff --git a/ui/litellm-dashboard/src/utils/textUtils.ts b/ui/litellm-dashboard/src/utils/textUtils.ts new file mode 100644 index 00000000000..07c4583d3ff --- /dev/null +++ b/ui/litellm-dashboard/src/utils/textUtils.ts @@ -0,0 +1,8 @@ +export const formatLabel = (text: string): string => { + if (!text) { + return text; + } + + const withSpaces = text.replace(/_/g, " "); + return withSpaces.replace(/\b\w/g, (char) => char.toUpperCase()); +};