diff --git a/ui/litellm-dashboard/eslint-suppressions.json b/ui/litellm-dashboard/eslint-suppressions.json index 7de7373b20b..7d7066addcf 100644 --- a/ui/litellm-dashboard/eslint-suppressions.json +++ b/ui/litellm-dashboard/eslint-suppressions.json @@ -1399,9 +1399,6 @@ }, "prefer-const": { "count": 2 - }, - "react-hooks/set-state-in-effect": { - "count": 1 } }, "src/components/TeamsPage/teamTableColumns.tsx": { diff --git a/ui/litellm-dashboard/src/components/Teams.test.tsx b/ui/litellm-dashboard/src/components/Teams.test.tsx index bee35ac5c5d..2bceb00aae1 100644 --- a/ui/litellm-dashboard/src/components/Teams.test.tsx +++ b/ui/litellm-dashboard/src/components/Teams.test.tsx @@ -15,6 +15,7 @@ import { teamCreateCall, } from "./networking"; import Teams from "./Teams"; +import { chooseSelectOption } from "../../tests/test-utils"; const can = vi.fn(); vi.mock("@/app/(dashboard)/hooks/useCan", () => ({ @@ -1488,3 +1489,204 @@ describe("Teams - the exact bytes the create call sends", () => { expect(teamCreateCall).not.toHaveBeenCalled(); }); }); + +describe("Teams - the create form keeps the organization and models picks while it is open", () => { + const ORGS = [ + { organization_id: "org-1", organization_alias: "Org 1", models: [], members: [] }, + { organization_id: "org-2", organization_alias: "Org 2", models: [], members: [] }, + ]; + + const orgField = () => screen.getByRole("combobox", { name: /organization/i }); + const modelsField = () => screen.getByTestId("create-team-models-select"); + + const openCreateModal = async () => { + act(() => { + fireEvent.click(screen.getAllByRole("button", { name: /create team/i })[0]); + }); + await screen.findByLabelText(/team name/i); + }; + + beforeEach(() => { + vi.clearAllMocks(); + mockTeamInfoView.mockClear(); + vi.mocked(fetchAvailableModelsForTeamOrKey).mockResolvedValue(["gpt-4", "gpt-3.5-turbo"]); + vi.mocked(fetchMCPAccessGroups).mockResolvedValue([]); + vi.mocked(getGuardrailsList).mockResolvedValue({ guardrails: [] }); + vi.mocked(getDefaultTeamSettings).mockResolvedValue({ values: {} }); + mockUseOrganizations.mockReturnValue({ data: ORGS }); + }); + + it("keeps both picks when the organizations list comes back changed from a refetch", async () => { + const user = userEvent.setup(); + renderWithQueryClient(); + await openCreateModal(); + + await chooseSelectOption(user, orgField(), /Org 1/); + fireEvent.change(modelsField(), { target: { value: "gpt-4" } }); + + mockUseOrganizations.mockReturnValue({ data: ORGS.map((org) => ({ ...org, spend: 1 })) }); + fireEvent.click(screen.getByText("Additional Settings")); + + expect(orgField()).toHaveValue("Org 1"); + expect(modelsField()).toHaveValue("gpt-4"); + }); + + it("keeps models picked before the available models finish loading", async () => { + let resolveModels: (models: string[]) => void = () => {}; + vi.mocked(fetchAvailableModelsForTeamOrKey).mockReturnValue( + new Promise((resolve) => { + resolveModels = resolve; + }), + ); + renderWithQueryClient(); + await openCreateModal(); + + fireEvent.change(modelsField(), { target: { value: "gpt-4" } }); + await act(async () => { + resolveModels(["gpt-4", "gpt-3.5-turbo"]); + }); + + expect(modelsField()).toHaveValue("gpt-4"); + }); + + it("clears the models pick when the organization is changed, since models are org scoped", async () => { + const user = userEvent.setup(); + renderWithQueryClient(); + await openCreateModal(); + + await chooseSelectOption(user, orgField(), /Org 1/); + fireEvent.change(modelsField(), { target: { value: "gpt-4" } }); + await chooseSelectOption(user, orgField(), /Org 2/); + + await waitFor(() => expect(orgField()).toHaveValue("Org 2")); + expect(modelsField()).toHaveValue(""); + }); + + it("keeps the models pick when the same organization is chosen again", async () => { + const user = userEvent.setup(); + renderWithQueryClient(); + await openCreateModal(); + + await chooseSelectOption(user, orgField(), /Org 1/); + fireEvent.change(modelsField(), { target: { value: "gpt-4" } }); + await chooseSelectOption(user, orgField(), /Org 1/); + + expect(orgField()).toHaveValue("Org 1"); + expect(modelsField()).toHaveValue("gpt-4"); + }); + + it("still preselects the only organization an org admin can create teams in", async () => { + mockUseOrganizations.mockReturnValue({ + data: [ + { + organization_id: "org-1", + organization_alias: "Org 1", + models: [], + members: [{ user_id: "user-123", user_role: "org_admin" }], + }, + ], + }); + renderWithQueryClient(); + await openCreateModal(); + + expect(orgField()).toHaveValue("Org 1"); + expect(orgField()).toBeDisabled(); + }); + + it("leaves an org admin able to pick when their admin orgs narrow to one while the form is open", async () => { + const orgAdminOrgs = [ + { + organization_id: "org-1", + organization_alias: "Org 1", + models: [], + members: [{ user_id: "user-123", user_role: "org_admin" }], + }, + { + organization_id: "org-2", + organization_alias: "Org 2", + models: [], + members: [{ user_id: "user-123", user_role: "org_admin" }], + }, + ]; + mockUseOrganizations.mockReturnValue({ data: orgAdminOrgs }); + renderWithQueryClient(); + await openCreateModal(); + expect(orgField()).toHaveValue(""); + + mockUseOrganizations.mockReturnValue({ data: [orgAdminOrgs[0]] }); + fireEvent.click(screen.getByText("Additional Settings")); + + expect(orgField()).toBeEnabled(); + }); + + it("refuses to create the team in an organization the admin has lost access to", async () => { + const user = userEvent.setup(); + const orgAdminOrgs = ORGS.map((org) => ({ ...org, members: [{ user_id: "user-123", user_role: "org_admin" }] })); + mockUseOrganizations.mockReturnValue({ data: orgAdminOrgs }); + renderWithQueryClient(); + await openCreateModal(); + + fireEvent.change(screen.getByTestId("team-name-input"), { target: { value: "Revoked Team" } }); + await chooseSelectOption(user, orgField(), /Org 1/); + + mockUseOrganizations.mockReturnValue({ data: [orgAdminOrgs[1]] }); + fireEvent.click(screen.getByText("Additional Settings")); + + const submitButtons = screen.getAllByRole("button", { name: /create team/i }); + fireEvent.click(submitButtons[submitButtons.length - 1]); + + await screen.findByText(/no longer create teams in this organization/i); + expect(teamCreateCall).not.toHaveBeenCalled(); + }); + + it("lets the admin switch to the one organization left after losing access to their pick", async () => { + const user = userEvent.setup(); + const orgAdminOrgs = ORGS.map((org) => ({ ...org, members: [{ user_id: "user-123", user_role: "org_admin" }] })); + mockUseOrganizations.mockReturnValue({ data: orgAdminOrgs }); + const createdTeam = { + team_id: "new-team-1", + team_alias: "Recovered Team", + models: [], + organization_id: "org-2", + keys: [], + members_with_roles: [], + spend: 0, + }; + vi.mocked(teamCreateCall).mockResolvedValue(createdTeam); + renderWithQueryClient(); + await openCreateModal(); + + fireEvent.change(screen.getByTestId("team-name-input"), { target: { value: "Recovered Team" } }); + await chooseSelectOption(user, orgField(), /Org 1/); + + mockUseOrganizations.mockReturnValue({ data: [orgAdminOrgs[1]] }); + fireEvent.click(screen.getByText("Additional Settings")); + + expect(orgField()).toBeEnabled(); + await chooseSelectOption(user, orgField(), /Org 2/); + const submitButtons = screen.getAllByRole("button", { name: /create team/i }); + fireEvent.click(submitButtons[submitButtons.length - 1]); + + await waitFor(() => + expect(teamCreateCall).toHaveBeenCalledWith( + "test-token", + expect.objectContaining({ team_alias: "Recovered Team", organization_id: "org-2" }), + ), + ); + }); + + it("starts the form clean again when the modal is closed and reopened", async () => { + const user = userEvent.setup(); + renderWithQueryClient(); + await openCreateModal(); + + await chooseSelectOption(user, orgField(), /Org 1/); + fireEvent.change(modelsField(), { target: { value: "gpt-4" } }); + fireEvent.click(screen.getByRole("button", { name: /^close$/i })); + await waitFor(() => expect(screen.queryByLabelText(/team name/i)).not.toBeInTheDocument()); + + await openCreateModal(); + expect(orgField()).toHaveValue(""); + expect(modelsField()).toHaveValue(""); + }); +}); diff --git a/ui/litellm-dashboard/src/components/Teams.tsx b/ui/litellm-dashboard/src/components/Teams.tsx index e2eda4adb23..ef58237a6aa 100644 --- a/ui/litellm-dashboard/src/components/Teams.tsx +++ b/ui/litellm-dashboard/src/components/Teams.tsx @@ -208,7 +208,6 @@ const Teams: React.FC = ({ accessToken, userID, userRole, premiumUser const queryClient = useQueryClient(); const refreshTeams = () => queryClient.invalidateQueries({ queryKey: teamsTableKeys.all }); const [currentOrg] = useState(null); - const [currentOrgForCreateTeam, setCurrentOrgForCreateTeam] = useState(null); const isOrgAdmin = userRole !== "Admin"; const [additionalSettingsOpen, setAdditionalSettingsOpen] = useState(false); @@ -216,17 +215,33 @@ const Teams: React.FC = ({ accessToken, userID, userRole, premiumUser const [agentSettingsOpen, setAgentSettingsOpen] = useState(false); const [searchToolSettingsOpen, setSearchToolSettingsOpen] = useState(false); + const adminOrgs = useMemo( + () => getAdminOrganizations(userRole, userID, organizations), + [userRole, userID, organizations], + ); + const teamCreateSchema = useMemo( () => teamCreateFieldsSchema.superRefine((values, ctx) => { if (isOrgAdmin && !values.organization_id) { ctx.addIssue({ code: "custom", message: SUPPRESSED_BY_DESCRIPTION, path: ["organization_id"] }); } + const organizationIsStillPickable = + values.organization_id == null || + organizations == null || + adminOrgs.some((org) => org.organization_id === values.organization_id); + if (!organizationIsStillPickable) { + ctx.addIssue({ + code: "custom", + message: "You can no longer create teams in this organization", + path: ["organization_id"], + }); + } if (additionalSettingsOpen && !isParsableJson(values.secret_manager_settings)) { ctx.addIssue({ code: "custom", message: SUPPRESSED_BY_DESCRIPTION, path: ["secret_manager_settings"] }); } }), - [isOrgAdmin, additionalSettingsOpen], + [isOrgAdmin, additionalSettingsOpen, adminOrgs, organizations], ); const form = useZodForm(teamCreateSchema, { defaultValues: EMPTY_TEAM_CREATE_VALUES }); @@ -264,28 +279,6 @@ const Teams: React.FC = ({ accessToken, userID, userRole, premiumUser ? `Default: ${getBudgetDurationLabel(defaultBudgetDuration)} (${defaultBudgetDuration})` : "n/a"; - useEffect(() => { - form.setValue("models", []); - }, [currentOrgForCreateTeam, userModels]); - - // Handle organization preselection when modal opens - useEffect(() => { - if (isTeamModalVisible) { - const adminOrgs = getAdminOrganizations(userRole, userID, organizations); - - // Org admins must scope a team to an org, so with exactly one we preselect it. - // Proxy admins can create org-less teams, so the field stays optional regardless of org count. - if (isOrgAdmin && adminOrgs.length === 1) { - const org = adminOrgs[0]; - form.setValue("organization_id", org.organization_id); - setCurrentOrgForCreateTeam(org); - } else { - form.setValue("organization_id", currentOrg?.organization_id || null); - setCurrentOrgForCreateTeam(currentOrg); - } - } - }, [isTeamModalVisible, isOrgAdmin, userRole, userID, organizations, currentOrg]); - // Add this useEffect to fetch guardrails useEffect(() => { const fetchGuardrails = async () => { @@ -320,6 +313,26 @@ const Teams: React.FC = ({ accessToken, userID, userRole, premiumUser if (canViewPolicies) fetchPolicies(); }, [accessToken, canViewPolicies]); + const openCreateTeamModal = () => { + // Org admins must scope a team to an org, so with exactly one we preselect it. + // Proxy admins can create org-less teams, so the field stays optional regardless of org count. + if (isOrgAdmin && adminOrgs.length === 1) { + form.setValue("organization_id", adminOrgs[0].organization_id); + } + setIsTeamModalVisible(true); + }; + + const selectCreateTeamOrganization = ( + next: string, + currentOrganizationId: string | null, + onChange: (organizationId: string | null) => void, + ) => { + const nextOrganizationId = next === "" ? null : next; + if (nextOrganizationId === currentOrganizationId) return; + onChange(nextOrganizationId); + form.setValue("models", []); + }; + const resetCreateForm = () => { form.reset(EMPTY_TEAM_CREATE_VALUES); setAdditionalSettingsOpen(false); @@ -636,7 +649,7 @@ const Teams: React.FC = ({ accessToken, userID, userRole, premiumUser subtitle="Manage teams, members, and their access to models and budgets" primaryAction={ canCreateOrManageTeams(userRole, userID, organizations) ? ( - setIsTeamModalVisible(true)} data-testid="create-team-button"> + Create Team @@ -683,9 +696,9 @@ const Teams: React.FC = ({ accessToken, userID, userRole, premiumUser )} {(() => { - const adminOrgs = getAdminOrganizations(userRole, userID, organizations); const isSingleOrg = adminOrgs.length === 1; const hasNoOrgs = adminOrgs.length === 0; + const soleOrganizationId = isSingleOrg ? adminOrgs[0].organization_id ?? null : null; return ( <> @@ -715,18 +728,13 @@ const Teams: React.FC = ({ accessToken, userID, userRole, premiumUser label: org.organization_alias ?? "", sublabel: org.organization_id ?? "", }))} - disabled={isOrgAdmin && isSingleOrg} + disabled={isOrgAdmin && soleOrganizationId !== null && value === soleOrganizationId} allowClear={!isOrgAdmin} placeholder={ hasNoOrgs ? "No organizations available" : "Search or select an Organization" } emptyText="No organizations available" - onValueChange={(next) => { - onChange(next === "" ? null : next); - setCurrentOrgForCreateTeam( - adminOrgs.find((org) => org.organization_id === next) ?? null, - ); - }} + onValueChange={(next) => selectCreateTeamOrganization(next, value ?? null, onChange)} /> )}