From 17d1f5833689739003abf379950137d7e8eeb2f5 Mon Sep 17 00:00:00 2001 From: tin-berri Date: Fri, 30 Oct 2026 10:31:57 -0700 Subject: [PATCH] fix(ui): preserve team model grants on unrelated settings saves (#49258) --- .../ModelSelect/ModelSelect.test.tsx | 33 +++++++++++ .../components/ModelSelect/ModelSelect.tsx | 5 +- .../team/TeamInfo.integration.test.tsx | 59 ++++++++++++++++++- .../src/components/team/TeamInfo.test.tsx | 1 - .../src/components/team/TeamInfo.tsx | 5 +- 5 files changed, 97 insertions(+), 6 deletions(-) diff --git a/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.test.tsx b/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.test.tsx index 7ab757cfed9..d7394b1f949 100644 --- a/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.test.tsx +++ b/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.test.tsx @@ -775,6 +775,39 @@ describe("ModelSelect", () => { expect(mockOnChange).toHaveBeenCalledWith(["gpt-4"]); }); + it.each([ + { value: "all-proxy-models", label: "All Proxy Models", includeSpecialOptions: true }, + { value: "no-default-models", label: "No Default Models", includeSpecialOptions: false }, + ])( + "should label retained $value without offering it again after removal", + async ({ value, label, includeSpecialOptions }) => { + const user = userEvent.setup(); + mockUseOrganization.mockReturnValue({ + data: createMockOrganization(["gpt-4"]), + isLoading: false, + } as unknown as ReturnType); + const props = { + onChange: mockOnChange, + context: "team" as const, + organizationID: "org-1", + options: { includeSpecialOptions }, + }; + const { rerender } = renderWithProviders(); + + expect(screen.getByLabelText(label)).toBeInTheDocument(); + await openModelList(user); + const retainedGroup = within(screen.getByRole("listbox")).getAllByRole("group")[0]; + expect(within(retainedGroup).getByText("Unavailable")).toBeInTheDocument(); + await user.click(within(retainedGroup).getByRole("option", { name: label })); + expect(mockOnChange).toHaveBeenCalledWith([]); + + rerender(); + expectNotOffered(label); + expectNotOffered("claude-3"); + expectOffered("gpt-4"); + }, + ); + it("should keep the other selections when removing one unavailable model alongside a special option", async () => { const user = userEvent.setup(); renderWithProviders( diff --git a/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.tsx b/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.tsx index f169382b4ed..c7c428f2ccf 100644 --- a/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.tsx +++ b/ui/litellm-dashboard/src/components/ModelSelect/ModelSelect.tsx @@ -240,7 +240,10 @@ export const ModelSelect = (props: ModelSelectProps) => { ]; const offeredByValue = new Map(offeredGroups.flatMap((group) => group.items).map((option) => [option.value, option])); - const selectedOptions = value.map((v) => offeredByValue.get(v) ?? { label: v, value: v }); + const selectedOptions = value.map( + (v) => + offeredByValue.get(v) ?? MODEL_SENTINEL_OPTIONS.find((option) => option.value === v) ?? { label: v, value: v }, + ); const groups: ModelOptionGroup[] = [ ...unavailableGroups( selectedOptions, diff --git a/ui/litellm-dashboard/src/components/team/TeamInfo.integration.test.tsx b/ui/litellm-dashboard/src/components/team/TeamInfo.integration.test.tsx index fc054e51e5b..545c504281b 100644 --- a/ui/litellm-dashboard/src/components/team/TeamInfo.integration.test.tsx +++ b/ui/litellm-dashboard/src/components/team/TeamInfo.integration.test.tsx @@ -1,6 +1,6 @@ import { useTeamMetadataSchema } from "@/app/(dashboard)/hooks/teams/useTeamMetadataSchema"; import * as networking from "@/components/networking"; -import { fireEvent, screen, waitFor } from "@testing-library/react"; +import { fireEvent, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { renderWithProviders } from "../../../tests/test-utils"; @@ -817,7 +817,7 @@ describe("TeamInfoView - member budget reset prompt", () => { }); }); -describe("TeamInfoView - model rate limit picker", () => { +describe("TeamInfoView - model settings", () => { const props = { teamId: "123", onUpdate: vi.fn(), @@ -836,6 +836,61 @@ describe("TeamInfoView - model rate limit picker", () => { vi.clearAllMocks(); }); + it.each([ + { name: "unrestricted", models: [], organizationId: null }, + { name: "explicit all models", models: ["all-proxy-models"], organizationId: null }, + { name: "access groups only", models: ["no-default-models"], organizationId: null }, + { name: "retained explicit models", models: ["retired-model", "openai/*"], organizationId: null }, + { name: "unrestricted within an organization", models: [], organizationId: "org-1" }, + ])("preserves $name model grants when saving only a budget", async ({ models, organizationId }) => { + const user = userEvent.setup({ delay: null }); + vi.mocked(networking.teamInfoCall).mockResolvedValue( + createMockTeamData({ models, organization_id: organizationId }), + ); + mockUseTeam.mockReturnValue({ + data: { team_id: "123", organization_models: organizationId ? ["gpt-4"] : undefined }, + isLoading: false, + } as unknown as ReturnType); + + renderWithProviders(); + await user.click(await screen.findByRole("tab", { name: "Settings" })); + await user.click(await screen.findByRole("button", { name: /edit settings/i })); + if (models.length === 0) { + expect(await screen.findByLabelText("All Proxy Models")).toBeInTheDocument(); + } + fireEvent.change(await screen.findByLabelText("Max Budget (USD)"), { target: { value: "100" } }); + await user.click(screen.getByRole("button", { name: /save changes/i })); + + await waitFor(() => expect(networking.teamUpdateCall).toHaveBeenCalledTimes(1)); + const payload = vi.mocked(networking.teamUpdateCall).mock.calls[0][1]; + expect(payload).toMatchObject({ team_id: "123", max_budget: 100 }); + expect(payload).not.toHaveProperty("models"); + }); + + it("discards a cancelled model clear but saves an intentional clear of unrestricted access", async () => { + const user = userEvent.setup({ delay: null }); + vi.mocked(networking.teamInfoCall).mockResolvedValue(createMockTeamData({ models: [] })); + + renderWithProviders(); + await user.click(await screen.findByRole("tab", { name: "Settings" })); + await user.click(await screen.findByRole("button", { name: /edit settings/i })); + await user.click(within(await screen.findByLabelText("All Proxy Models")).getByRole("button")); + await user.click(screen.getByRole("button", { name: "Cancel" })); + await user.click(await screen.findByRole("button", { name: /edit settings/i })); + expect(await screen.findByLabelText("All Proxy Models")).toBeInTheDocument(); + fireEvent.change(screen.getByLabelText("Max Budget (USD)"), { target: { value: "100" } }); + await user.click(screen.getByRole("button", { name: /save changes/i })); + + await waitFor(() => expect(networking.teamUpdateCall).toHaveBeenCalledTimes(1)); + expect(vi.mocked(networking.teamUpdateCall).mock.calls[0][1]).not.toHaveProperty("models"); + await user.click(await screen.findByRole("button", { name: /edit settings/i })); + await user.click(within(await screen.findByLabelText("All Proxy Models")).getByRole("button")); + await user.click(screen.getByRole("button", { name: /save changes/i })); + + await waitFor(() => expect(networking.teamUpdateCall).toHaveBeenCalledTimes(2)); + expect(vi.mocked(networking.teamUpdateCall).mock.calls[1][1].models).toEqual(["no-default-models"]); + }); + it("offers access-group models, not no-default-models, in the model rate limit picker", async () => { const user = userEvent.setup({ delay: null }); vi.mocked(networking.teamInfoCall).mockResolvedValue( diff --git a/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx b/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx index e4bf67ab72f..46081ae96f5 100644 --- a/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx +++ b/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx @@ -2421,7 +2421,6 @@ describe("TeamInfoView - the exact bytes the update call sends", () => { const alwaysSent = { team_id: "123", team_alias: "Test Team", - models: ["gpt-4"], tpm_limit: 1000, rpm_limit: 1000, tpd_limit: null, diff --git a/ui/litellm-dashboard/src/components/team/TeamInfo.tsx b/ui/litellm-dashboard/src/components/team/TeamInfo.tsx index 0d20d160283..329552d735a 100644 --- a/ui/litellm-dashboard/src/components/team/TeamInfo.tsx +++ b/ui/litellm-dashboard/src/components/team/TeamInfo.tsx @@ -72,6 +72,7 @@ import { } from "../key_team_helpers/ModelMaxBudgetEditor"; import { modelMaxBudgetUpdate, StoredModelMaxBudget } from "../key_team_helpers/modelMaxBudgetPayload"; import { + ALL_PROXY_MODELS, computeTeamModelBadges, normalizeTeamModelSelection, TeamAccessGroupModelGrant, @@ -515,7 +516,7 @@ const computeEffectiveGuardrails = (info: TeamInfoRecord, globalGuardrailNames: const toTeamFormValues = (info: TeamInfoRecord, effectiveGuardrails: string[]): TeamUpdateFormValues => ({ team_alias: info.team_alias, - models: info.models, + models: info.models.length === 0 ? [ALL_PROXY_MODELS] : info.models, max_budget: info.max_budget, soft_budget: info.soft_budget, soft_budget_alerting_emails: Array.isArray(info.metadata?.soft_budget_alerting_emails) @@ -1064,7 +1065,7 @@ const TeamInfoView: React.FC = ({ const updateData: any = { team_id: teamId, team_alias: values.team_alias, - models: normalizeTeamModelSelection(values.models), + ...(form.getFieldState("models").isDirty ? { models: normalizeTeamModelSelection(values.models) } : {}), tpm_limit: numberOrNull(values.tpm_limit), rpm_limit: numberOrNull(values.rpm_limit), tpd_limit: numberOrNull(values.tpd_limit),