mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-03 02:22:24 +00:00
fix(ui): render access group MCP and agent selections as wrapping chips (#41228)
* fix(ui): render access group MCP and agent selections as wrapping chips Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(ui): cover 20 selected MCP servers rendering as separate chips Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(ui): cover MCP and agent chip selection in access group create dialog Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: ryan-crabbe-berri <ryan@berri.ai>
This commit is contained in:
parent
3930c5bab6
commit
253627f484
4 changed files with 83 additions and 114 deletions
|
|
@ -9,8 +9,8 @@ import { useMCPServers } from "@/app/(dashboard)/hooks/mcpServers/useMCPServers"
|
|||
import { ModelSelect } from "@/components/ModelSelect/ModelSelect";
|
||||
import { FieldGroup } from "@/components/ui/field";
|
||||
import { FormField } from "@/components/shared/form/FormField";
|
||||
import { MultiSelect } from "@/components/shared/MultiSelect";
|
||||
import { Input } from "@/components/ui/input";
|
||||
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select";
|
||||
import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs";
|
||||
import { Textarea } from "@/components/ui/textarea";
|
||||
|
||||
|
|
@ -29,53 +29,6 @@ export const MODELS_TAB = "models";
|
|||
export const MCP_SERVERS_TAB = "mcp-servers";
|
||||
export const AGENTS_TAB = "agents";
|
||||
|
||||
interface MultiSelectOption {
|
||||
value: string;
|
||||
label: string;
|
||||
}
|
||||
|
||||
interface MultiSelectProps {
|
||||
id: string;
|
||||
value: string[];
|
||||
onChange: (value: string[]) => void;
|
||||
options: MultiSelectOption[];
|
||||
placeholder: string;
|
||||
"aria-invalid": true | undefined;
|
||||
"aria-describedby": string | undefined;
|
||||
}
|
||||
|
||||
const MultiSelect = ({
|
||||
id,
|
||||
value,
|
||||
onChange,
|
||||
options,
|
||||
placeholder,
|
||||
"aria-invalid": ariaInvalid,
|
||||
"aria-describedby": ariaDescribedBy,
|
||||
}: MultiSelectProps) => (
|
||||
<Select multiple items={options} value={value} onValueChange={onChange}>
|
||||
<SelectTrigger id={id} aria-invalid={ariaInvalid} aria-describedby={ariaDescribedBy} className="w-full">
|
||||
<SelectValue placeholder={placeholder}>
|
||||
{(selected: string[]) =>
|
||||
selected.length === 0
|
||||
? placeholder
|
||||
: options
|
||||
.filter((option) => selected.includes(option.value))
|
||||
.map((option) => option.label)
|
||||
.join(", ")
|
||||
}
|
||||
</SelectValue>
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
{options.map((option) => (
|
||||
<SelectItem key={option.value} value={option.value} title={option.label}>
|
||||
{option.label}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
);
|
||||
|
||||
interface AccessGroupBaseFormProps {
|
||||
form: UseFormReturn<AccessGroupFormValues>;
|
||||
isNameDisabled?: boolean;
|
||||
|
|
@ -145,15 +98,13 @@ export function AccessGroupBaseForm({
|
|||
|
||||
<TabsContent value={MCP_SERVERS_TAB} className="pt-4">
|
||||
<FormField control={form.control} name="mcpServerIds" label="Allowed MCP Servers">
|
||||
{({ id, value, onChange, "aria-invalid": ariaInvalid, "aria-describedby": ariaDescribedBy }) => (
|
||||
{({ id, value, onChange }) => (
|
||||
<MultiSelect
|
||||
id={id}
|
||||
value={value}
|
||||
onChange={onChange}
|
||||
onValueChange={onChange}
|
||||
options={mcpServerOptions}
|
||||
placeholder="Select MCP servers"
|
||||
aria-invalid={ariaInvalid}
|
||||
aria-describedby={ariaDescribedBy}
|
||||
/>
|
||||
)}
|
||||
</FormField>
|
||||
|
|
@ -161,15 +112,13 @@ export function AccessGroupBaseForm({
|
|||
|
||||
<TabsContent value={AGENTS_TAB} className="pt-4">
|
||||
<FormField control={form.control} name="agentIds" label="Allowed Agents">
|
||||
{({ id, value, onChange, "aria-invalid": ariaInvalid, "aria-describedby": ariaDescribedBy }) => (
|
||||
{({ id, value, onChange }) => (
|
||||
<MultiSelect
|
||||
id={id}
|
||||
value={value}
|
||||
onChange={onChange}
|
||||
onValueChange={onChange}
|
||||
options={agentOptions}
|
||||
placeholder="Select agents"
|
||||
aria-invalid={ariaInvalid}
|
||||
aria-describedby={ariaDescribedBy}
|
||||
/>
|
||||
)}
|
||||
</FormField>
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import userEvent, { PointerEventsCheckLevel } from "@testing-library/user-event";
|
||||
import { fireEvent, renderWithProviders, screen, waitFor } from "../../../../../../tests/test-utils";
|
||||
import { fireEvent, renderWithProviders, screen, waitFor, within } from "../../../../../../tests/test-utils";
|
||||
import { AccessGroupEditModal } from "./AccessGroupEditModal";
|
||||
import { AccessGroupResponse } from "@/app/(dashboard)/hooks/accessGroups/useAccessGroups";
|
||||
|
||||
|
|
@ -14,8 +14,13 @@ vi.mock("@/app/(dashboard)/hooks/agents/useAgents", () => ({
|
|||
useAgents: () => ({ data: { agents: [{ agent_id: "agent-1", agent_name: "Support Bot" }] } }),
|
||||
}));
|
||||
|
||||
const manyServers = Array.from({ length: 20 }, (_, i) => ({
|
||||
server_id: `srv-${i + 1}`,
|
||||
server_name: `Server ${i + 1}`,
|
||||
}));
|
||||
|
||||
vi.mock("@/app/(dashboard)/hooks/mcpServers/useMCPServers", () => ({
|
||||
useMCPServers: () => ({ data: [{ server_id: "srv-1", server_name: "Files" }] }),
|
||||
useMCPServers: () => ({ data: [{ server_id: "srv-1", server_name: "Files" }, ...manyServers.slice(1)] }),
|
||||
}));
|
||||
|
||||
vi.mock("@/components/ModelSelect/ModelSelect", () => ({
|
||||
|
|
@ -164,6 +169,47 @@ describe("AccessGroupEditModal submit payload", () => {
|
|||
expect(mutate).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("renders each selected MCP server as its own removable chip and drops one on remove", async () => {
|
||||
const user = setup();
|
||||
renderModal();
|
||||
await screen.findByDisplayValue("Engineering");
|
||||
|
||||
await user.click(screen.getByRole("tab", { name: /MCP Servers/ }));
|
||||
const chip = await screen.findByLabelText("Files");
|
||||
expect(chip).toHaveAttribute("data-slot", "combobox-chip");
|
||||
expect(screen.queryByText("srv-1")).not.toBeInTheDocument();
|
||||
|
||||
await user.click(within(chip).getByRole("button"));
|
||||
await save(user);
|
||||
|
||||
await waitFor(() => expect(mutate).toHaveBeenCalled());
|
||||
expect(variables().params.access_mcp_server_ids).toStrictEqual([]);
|
||||
});
|
||||
|
||||
it("keeps 20 selected MCP servers as separate chips instead of one joined string", async () => {
|
||||
const user = setup();
|
||||
renderModal({ ...accessGroup, access_mcp_server_ids: manyServers.map((s) => s.server_id) });
|
||||
await screen.findByDisplayValue("Engineering");
|
||||
|
||||
await user.click(screen.getByRole("tab", { name: /MCP Servers/ }));
|
||||
await screen.findByLabelText("Server 20");
|
||||
const chips = screen.getAllByLabelText(/^(Files|Server \d+)$/);
|
||||
expect(chips).toHaveLength(20);
|
||||
expect(chips.map((chip) => chip.textContent)).toStrictEqual([
|
||||
"Files",
|
||||
...manyServers.slice(1).map((s) => s.server_name),
|
||||
]);
|
||||
expect(screen.queryByText(/Server 2, Server 3/)).not.toBeInTheDocument();
|
||||
|
||||
await user.click(within(screen.getByLabelText("Server 7")).getByRole("button"));
|
||||
await save(user);
|
||||
|
||||
await waitFor(() => expect(mutate).toHaveBeenCalled());
|
||||
expect(variables().params.access_mcp_server_ids).toStrictEqual(
|
||||
manyServers.map((s) => s.server_id).filter((id) => id !== "srv-7"),
|
||||
);
|
||||
});
|
||||
|
||||
it("sends models chosen on the Models tab", async () => {
|
||||
const user = setup();
|
||||
renderModal({ ...accessGroup, access_model_names: [] });
|
||||
|
|
|
|||
|
|
@ -98,6 +98,31 @@ describe("AccessGroupCreateDialog", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("sends MCP servers and agents picked from the chip selectors as ids", async () => {
|
||||
const user = userEvent.setup();
|
||||
const { createAccessGroup } = renderDialog();
|
||||
|
||||
await user.type(screen.getByLabelText("Group Name"), "mcp-group");
|
||||
await user.click(screen.getByRole("tab", { name: "MCP Servers" }));
|
||||
await user.click(screen.getByLabelText("Allowed MCP Servers"));
|
||||
await user.click(await screen.findByRole("option", { name: "GitHub MCP" }));
|
||||
expect(screen.getByLabelText("GitHub MCP")).toHaveAttribute("data-slot", "combobox-chip");
|
||||
await user.keyboard("{Escape}");
|
||||
|
||||
await user.click(screen.getByRole("tab", { name: "Agents" }));
|
||||
await user.click(screen.getByLabelText("Allowed Agents"));
|
||||
await user.click(await screen.findByRole("option", { name: "Support Agent" }));
|
||||
await user.keyboard("{Escape}");
|
||||
await user.click(screen.getByRole("button", { name: "Create Group" }));
|
||||
|
||||
await waitFor(() => expect(createAccessGroup).toHaveBeenCalledTimes(1));
|
||||
expect(createAccessGroup.mock.calls[0][0]).toStrictEqual({
|
||||
access_group_name: "mcp-group",
|
||||
access_mcp_server_ids: ["srv-1"],
|
||||
access_agent_ids: ["agent-1"],
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps the dialog open with the entered values when the create fails", async () => {
|
||||
const user = userEvent.setup();
|
||||
const { createAccessGroup } = renderDialog({
|
||||
|
|
|
|||
|
|
@ -11,10 +11,10 @@ import { ModelSelect } from "@/components/ModelSelect/ModelSelect";
|
|||
import { toast } from "@/lib/toast";
|
||||
import { FieldGroup } from "@/components/ui/field";
|
||||
import { FormField } from "@/components/shared/form/FormField";
|
||||
import { MultiSelect } from "@/components/shared/MultiSelect";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Dialog, DialogContent, DialogFooter, DialogHeader, DialogTitle } from "@/components/ui/dialog";
|
||||
import { Input } from "@/components/ui/input";
|
||||
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select";
|
||||
import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs";
|
||||
import { Textarea } from "@/components/ui/textarea";
|
||||
import { useZodForm } from "@/lib/forms/useZodForm";
|
||||
|
|
@ -25,53 +25,6 @@ import { accessGroupCreateSchema } from "./schema";
|
|||
|
||||
const GENERAL_TAB = "general";
|
||||
|
||||
interface MultiSelectOption {
|
||||
value: string;
|
||||
label: string;
|
||||
}
|
||||
|
||||
interface MultiSelectProps {
|
||||
id: string;
|
||||
value: string[];
|
||||
onChange: (value: string[]) => void;
|
||||
options: MultiSelectOption[];
|
||||
placeholder: string;
|
||||
"aria-invalid": true | undefined;
|
||||
"aria-describedby": string | undefined;
|
||||
}
|
||||
|
||||
const MultiSelect = ({
|
||||
id,
|
||||
value,
|
||||
onChange,
|
||||
options,
|
||||
placeholder,
|
||||
"aria-invalid": ariaInvalid,
|
||||
"aria-describedby": ariaDescribedBy,
|
||||
}: MultiSelectProps) => (
|
||||
<Select multiple items={options} value={value} onValueChange={onChange}>
|
||||
<SelectTrigger id={id} aria-invalid={ariaInvalid} aria-describedby={ariaDescribedBy} className="w-full">
|
||||
<SelectValue placeholder={placeholder}>
|
||||
{(selected: string[]) =>
|
||||
selected.length === 0
|
||||
? placeholder
|
||||
: options
|
||||
.filter((option) => selected.includes(option.value))
|
||||
.map((option) => option.label)
|
||||
.join(", ")
|
||||
}
|
||||
</SelectValue>
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
{options.map((option) => (
|
||||
<SelectItem key={option.value} value={option.value}>
|
||||
{option.label}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
);
|
||||
|
||||
const defaultCreateAccessGroup = async (body: AccessGroupCreateBody): Promise<unknown> => {
|
||||
const { data } = await fetchClient.POST("/v1/access_group", { body });
|
||||
return data;
|
||||
|
|
@ -193,15 +146,13 @@ export const AccessGroupCreateDialog = ({
|
|||
|
||||
<TabsContent value="mcp-servers" className="pt-4">
|
||||
<FormField control={form.control} name="mcpServerIds" label="Allowed MCP Servers">
|
||||
{({ id, value, onChange, "aria-invalid": ariaInvalid, "aria-describedby": ariaDescribedBy }) => (
|
||||
{({ id, value, onChange }) => (
|
||||
<MultiSelect
|
||||
id={id}
|
||||
value={value}
|
||||
onChange={onChange}
|
||||
onValueChange={onChange}
|
||||
options={mcpServerOptions}
|
||||
placeholder="Select MCP servers"
|
||||
aria-invalid={ariaInvalid}
|
||||
aria-describedby={ariaDescribedBy}
|
||||
/>
|
||||
)}
|
||||
</FormField>
|
||||
|
|
@ -209,15 +160,13 @@ export const AccessGroupCreateDialog = ({
|
|||
|
||||
<TabsContent value="agents" className="pt-4">
|
||||
<FormField control={form.control} name="agentIds" label="Allowed Agents">
|
||||
{({ id, value, onChange, "aria-invalid": ariaInvalid, "aria-describedby": ariaDescribedBy }) => (
|
||||
{({ id, value, onChange }) => (
|
||||
<MultiSelect
|
||||
id={id}
|
||||
value={value}
|
||||
onChange={onChange}
|
||||
onValueChange={onChange}
|
||||
options={agentOptions}
|
||||
placeholder="Select agents"
|
||||
aria-invalid={ariaInvalid}
|
||||
aria-describedby={ariaDescribedBy}
|
||||
/>
|
||||
)}
|
||||
</FormField>
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue