mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-05 08:07:05 +00:00
fix(ui): stop the create team form resetting organization and models (#39476)
* fix(ui): stop the create team form resetting organization and models The organization preselect ran in an effect keyed on the organizations query, so any refetch of that list while the Create Team modal was open overwrote the user's organization pick, which in turn cleared their models pick. The models field was also cleared whenever the available models fetch resolved. Preselect the organization when the modal opens instead, and clear the models only when the user picks a different organization. An org admin whose admin orgs narrow to one while the form is open can still pick, rather than facing a locked empty field. * fix(ui): block team create when the picked organization is no longer available An organization picked in the Create Team form now survives a refetch of the organization list, so it can outlive the admin's access to it. Refuse the create with a message on the field rather than letting the request fail authorization at the proxy. * fix(ui): keep the team create organization field usable when the pick goes stale Locking the field on a single admin organization also locked it while it held a rejected organization, so an admin who lost access could not pick the one organization left. Lock it only while it holds that organization. * test(ui): hoist the created team fixture out of the mock call The inline object pushed the repo past its no-large-inline-object-arg lint budget, which has no headroom.
This commit is contained in:
yuneng-jiang
GitHub
parent
534003da03
commit
c841a56e9a
3 changed files with 243 additions and 36 deletions
|
|
@ -1399,9 +1399,6 @@
|
|||
},
|
||||
"prefer-const": {
|
||||
"count": 2
|
||||
},
|
||||
"react-hooks/set-state-in-effect": {
|
||||
"count": 1
|
||||
}
|
||||
},
|
||||
"src/components/TeamsPage/teamTableColumns.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(<Teams accessToken="test-token" userID="user-123" userRole="Admin" />);
|
||||
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<string[]>((resolve) => {
|
||||
resolveModels = resolve;
|
||||
}),
|
||||
);
|
||||
renderWithQueryClient(<Teams accessToken="test-token" userID="user-123" userRole="Admin" />);
|
||||
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(<Teams accessToken="test-token" userID="user-123" userRole="Admin" />);
|
||||
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(<Teams accessToken="test-token" userID="user-123" userRole="Admin" />);
|
||||
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(<Teams accessToken="test-token" userID="user-123" userRole="Internal User" />);
|
||||
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(<Teams accessToken="test-token" userID="user-123" userRole="Internal User" />);
|
||||
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(<Teams accessToken="test-token" userID="user-123" userRole="Internal User" />);
|
||||
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(<Teams accessToken="test-token" userID="user-123" userRole="Internal User" />);
|
||||
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(<Teams accessToken="test-token" userID="user-123" userRole="Admin" />);
|
||||
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("");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -208,7 +208,6 @@ const Teams: React.FC<TeamProps> = ({ accessToken, userID, userRole, premiumUser
|
|||
const queryClient = useQueryClient();
|
||||
const refreshTeams = () => queryClient.invalidateQueries({ queryKey: teamsTableKeys.all });
|
||||
const [currentOrg] = useState<Organization | null>(null);
|
||||
const [currentOrgForCreateTeam, setCurrentOrgForCreateTeam] = useState<Organization | null>(null);
|
||||
|
||||
const isOrgAdmin = userRole !== "Admin";
|
||||
const [additionalSettingsOpen, setAdditionalSettingsOpen] = useState(false);
|
||||
|
|
@ -216,17 +215,33 @@ const Teams: React.FC<TeamProps> = ({ 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<TeamProps> = ({ 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<TeamProps> = ({ 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<TeamProps> = ({ accessToken, userID, userRole, premiumUser
|
|||
subtitle="Manage teams, members, and their access to models and budgets"
|
||||
primaryAction={
|
||||
canCreateOrManageTeams(userRole, userID, organizations) ? (
|
||||
<UIButton onClick={() => setIsTeamModalVisible(true)} data-testid="create-team-button">
|
||||
<UIButton onClick={openCreateTeamModal} data-testid="create-team-button">
|
||||
<Plus className="size-4" />
|
||||
Create Team
|
||||
</UIButton>
|
||||
|
|
@ -683,9 +696,9 @@ const Teams: React.FC<TeamProps> = ({ accessToken, userID, userRole, premiumUser
|
|||
)}
|
||||
</FormField>
|
||||
{(() => {
|
||||
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<TeamProps> = ({ 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)}
|
||||
/>
|
||||
)}
|
||||
</FormField>
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue