Small issues (#16406)

This commit is contained in:
yuneng-jiang 2025-11-08 16:05:42 -08:00 • committed by GitHub
parent 86d73c918c
commit 11af05f5f4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 284 additions and 187 deletions

View file

@ -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(
<OldTeams
teams={[
{
team_id: "1",
team_alias: "Test Team",
organization_id: "org-123",
models: ["gpt-4"],
max_budget: 100,
budget_duration: "1d",
tpm_limit: 1000,
rpm_limit: 1000,
created_at: new Date().toISOString(),
keys: [],
members_with_roles: [],
},
]}
searchParams={{}}
accessToken="test-token"
setTeams={vi.fn()}
userID="user-123"
userRole="Admin"
organizations={[]}
/>,
);
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);
});
});
});

View file

@ -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<TeamProps> = ({
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<TeamProps> = ({
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<TeamProps> = ({
// 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<TeamProps> = ({
onClick={() => handleDelete(team.team_id)}
icon={TrashIcon}
size="sm"
data-testid="delete-team-button"
/>
</>
) : null}
@ -1155,15 +1144,11 @@ const Teams: React.FC<TeamProps> = ({
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) => {

View file

@ -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<SchemaFormFieldsProps> = ({
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 = [];

View file

@ -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: () => <div>Mock Alerting Settings</div>,
}));
vi.mock("./email_settings", () => ({
__esModule: true,
default: () => <div>Mock Email Settings</div>,
}));
// 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(<Settings {...defaultProps} />);
await waitFor(() => {
expect(getByText("Active Logging Callbacks")).toBeInTheDocument();
});
});
it("should display additional settings tabs", async () => {
const { getByText } = render(<Settings {...defaultProps} />);
await waitFor(() => {
expect(getByText("Alerting Types")).toBeInTheDocument();
expect(getByText("Alerting Settings")).toBeInTheDocument();
expect(getByText("Email Alerts")).toBeInTheDocument();
});
});
});

View file

@ -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<SettingsPageProps> = ({ 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<SettingsPageProps> = ({ accessToken, userRole, userID,
setSelectedCallbackParams([]);
}
};
const handleSaveAlerts = async () => {
if (!accessToken) {
return;
@ -594,124 +591,109 @@ const Settings: React.FC<SettingsPageProps> = ({ accessToken, userRole, userID,
wrapperCol={{ span: 16 }}
labelAlign="left"
>
<FormItem
label="Callback"
name="callback"
rules={[{ required: true, message: "Please select a callback" }]}
<FormItem label="Callback" name="callback" rules={[{ required: true, message: "Please select a callback" }]}>
<Select
placeholder="Choose a logging callback..."
size="large"
className="w-full"
showSearch
filterOption={(input, option) => {
return (option?.value?.toString() ?? "").toLowerCase().includes(input.toLowerCase());
}}
onChange={(value) => {
handleSelectedCallbackChange(value);
}}
>
<Select
placeholder="Choose a logging callback..."
size="large"
className="w-full"
showSearch
filterOption={(input, option) =>
(option?.children?.toString() ?? "")
.toLowerCase()
.includes(input.toLowerCase())
}
onChange={(value) => {
handleSelectedCallbackChange(value);
}}
>
{CALLBACK_CONFIGS.map((callbackConfig) => (
<SelectItem
key={callbackConfig.id}
value={callbackConfig.id}
>
<div className="flex items-center space-x-3 py-1">
<div className="w-6 h-6 flex items-center justify-center">
{/* eslint-disable-next-line @next/next/no-img-element */}
<img
src={callbackConfig.logo}
alt={`${callbackConfig.displayName} logo`}
className="w-6 h-6 rounded object-contain"
onError={(e) => {
e.currentTarget.style.display = 'none';
}}
/>
</div>
<span className="font-medium text-gray-900">
{callbackConfig.displayName}
</span>
{CALLBACK_CONFIGS.map((callbackConfig) => (
<SelectItem key={callbackConfig.id} value={callbackConfig.id}>
<div className="flex items-center space-x-3 py-1">
<div className="w-6 h-6 flex items-center justify-center">
{/* eslint-disable-next-line @next/next/no-img-element */}
<img
src={callbackConfig.logo}
alt={`${callbackConfig.displayName} logo`}
className="w-6 h-6 rounded object-contain"
onError={(e) => {
e.currentTarget.style.display = "none";
}}
/>
</div>
</SelectItem>
))}
</Select>
</FormItem>
<span className="font-medium text-gray-900">{callbackConfig.displayName}</span>
</div>
</SelectItem>
))}
</Select>
</FormItem>
{selectedCallbackParams && selectedCallbackParams.length > 0 && (
<div className="space-y-4 mt-6 p-4 bg-gray-50 rounded-lg border">
{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 (
<FormItem
label={
<span className="text-sm font-medium text-gray-700">
{fieldLabel}
<span className="text-red-500 ml-1">*</span>
</span>
}
name={param}
key={param}
className="mb-4"
rules={[
{
required: true,
message: `Please enter the ${fieldLabel.toLowerCase()}`,
},
]}
>
{paramType === "password" ? (
<Input.Password
size="large"
placeholder={`Enter your ${fieldLabel.toLowerCase()}`}
className="w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500"
/>
) : paramType === "number" ? (
<Input
type="number"
size="large"
placeholder={`Enter ${fieldLabel.toLowerCase()}`}
className="w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500"
min={0}
max={1}
step={0.1}
/>
) : (
<Input
size="large"
placeholder={`Enter your ${fieldLabel.toLowerCase()}`}
className="w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500"
/>
)}
</FormItem>
);
})}
</div>
)}
{selectedCallbackParams && selectedCallbackParams.length > 0 && (
<div className="space-y-4 mt-6 p-4 bg-gray-50 rounded-lg border">
{selectedCallbackParams.map((param) => {
// Get the callback configuration to look up parameter types
const callbackConfig = getCallbackById(selectedCallback || "");
const paramType = callbackConfig?.dynamic_params[param] || "text";
<div className="flex justify-end space-x-3 pt-6 mt-6 border-t border-gray-200">
<Button
onClick={() => {
setShowAddCallbacksModal(false);
setSelectedCallback(null);
setSelectedCallbackParams([]);
addForm.resetFields();
}}
>
Cancel
</Button>
<Button2
htmlType="submit"
>
Add Callback
</Button2>
const fieldLabel = param.replace(/_/g, " ").replace(/\b\w/g, (l) => l.toUpperCase());
return (
<FormItem
label={
<span className="text-sm font-medium text-gray-700">
{fieldLabel}
<span className="text-red-500 ml-1">*</span>
</span>
}
name={param}
key={param}
className="mb-4"
rules={[
{
required: true,
message: `Please enter the ${fieldLabel.toLowerCase()}`,
},
]}
>
{paramType === "password" ? (
<Input.Password
size="large"
placeholder={`Enter your ${fieldLabel.toLowerCase()}`}
className="w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500"
/>
) : paramType === "number" ? (
<Input
type="number"
size="large"
placeholder={`Enter ${fieldLabel.toLowerCase()}`}
className="w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500"
min={0}
max={1}
step={0.1}
/>
) : (
<Input
size="large"
placeholder={`Enter your ${fieldLabel.toLowerCase()}`}
className="w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500"
/>
)}
</FormItem>
);
})}
</div>
)}
<div className="flex justify-end space-x-3 pt-6 mt-6 border-t border-gray-200">
<Button
onClick={() => {
setShowAddCallbacksModal(false);
setSelectedCallback(null);
setSelectedCallbackParams([]);
addForm.resetFields();
}}
>
Cancel
</Button>
<Button2 htmlType="submit">Add Callback</Button2>
</div>
</Form>
</Modal>

View file

@ -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");
});
});

View file

@ -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());
};