diff --git a/src/components/common/TeamSelect.test.tsx b/src/components/common/TeamSelect.test.tsx new file mode 100644 index 0000000..9990c0c --- /dev/null +++ b/src/components/common/TeamSelect.test.tsx @@ -0,0 +1,54 @@ +import { describe, expect, it, vi } from "vitest"; +import { screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { renderWithProviders } from "@/test/test-utils"; +import type { Team } from "@/types/team"; +import { TeamSelect } from "./TeamSelect"; + +const personalTeam = { id: "team-personal", name: "Personal team", is_personal: true } as Team; +const sharedTeam = { id: "team-shared", name: "Shared team", is_personal: false } as Team; + +describe("TeamSelect", () => { + it("renders nothing for a single team", () => { + const { container } = renderWithProviders( + , + ); + + expect(container).toBeEmptyDOMElement(); + }); + + it("renders an error even without a selector", () => { + // A failed /teams load leaves no teams to choose from, so the error is the + // only thing explaining why the form will not submit. + renderWithProviders(); + + expect(screen.getByText("Team is required")).toBeInTheDocument(); + }); + + it("reports the chosen team", async () => { + const onChange = vi.fn(); + renderWithProviders( + , + ); + + await userEvent.setup().click(screen.getByRole("combobox", { name: /^team/i })); + await userEvent.setup().click(screen.getByRole("option", { name: "Shared team" })); + + expect(onChange).toHaveBeenCalledWith(sharedTeam.id); + }); + + it("marks the field invalid when in error", () => { + renderWithProviders( + , + ); + + const select = screen.getByRole("combobox", { name: /^team/i }); + expect(select).toHaveAttribute("aria-invalid", "true"); + expect(select).toHaveAccessibleDescription("Team is required"); + }); +}); diff --git a/src/components/common/TeamSelect.tsx b/src/components/common/TeamSelect.tsx new file mode 100644 index 0000000..6351402 --- /dev/null +++ b/src/components/common/TeamSelect.tsx @@ -0,0 +1,78 @@ +import { useIntl } from "react-intl"; + +import { Label } from "@/components/ui/label"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select"; +import type { Team } from "@/types/team"; + +interface TeamSelectProps { + /** Teams the caller belongs to, from `useTeams()`. */ + teams: Team[]; + value?: string; + onChange: (teamId: string) => void; + /** Validation message for the field, rendered below the select. */ + error?: string; + /** Element id for the select, so each form can scope it. */ + id?: string; +} + +/** + * Team picker for `team`-visibility records. + * + * Renders nothing when the caller has fewer than two teams: everyone belongs to + * at least their own personal team, so a single-team caller has no choice to + * make and the form scopes to that team silently (see `resolveTeamId`). The + * exception is an error — shown even without a selector, so a failed `/teams` + * load explains itself instead of leaving the submit button inert. + */ +export function TeamSelect({ teams, value, onChange, error, id = "team" }: TeamSelectProps) { + const intl = useIntl(); + const errorId = `${id}-error`; + + if (teams.length < 2) { + return error ? ( +

+ {error} +

+ ) : null; + } + + return ( +
+ + + {error && ( +

+ {error} +

+ )} +
+ ); +} diff --git a/src/components/mcp-servers/AdvancedSettings.test.tsx b/src/components/mcp-servers/AdvancedSettings.test.tsx index 6cdec5b..3e1fdb6 100644 --- a/src/components/mcp-servers/AdvancedSettings.test.tsx +++ b/src/components/mcp-servers/AdvancedSettings.test.tsx @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { renderWithProviders as render, screen } from "@/test/test-utils"; +import { renderWithProviders as render, screen, waitFor } from "@/test/test-utils"; import userEvent from "@testing-library/user-event"; +import { api } from "@/api/client"; import * as AuthContextModule from "@/auth/AuthContext"; import { AdvancedSettings } from "./AdvancedSettings"; @@ -8,7 +9,22 @@ vi.mock("@/auth/AuthContext", () => ({ useAuthContext: vi.fn(), })); +vi.mock("@/api/client", () => ({ + api: { get: vi.fn() }, +})); + const mockUseAuthContext = vi.mocked(AuthContextModule.useAuthContext); +const mockGet = vi.mocked(api.get); + +const personalTeam = { id: "team-personal", name: "Personal team", is_personal: true }; +const sharedTeam = { id: "team-shared", name: "Shared team", is_personal: false }; + +/** Answers `GET /teams` with the given teams; everything else stays empty. */ +function mockTeams(teams: Array>) { + mockGet.mockImplementation((path: string) => + path === "/teams" ? Promise.resolve({ teams }) : Promise.resolve([]), + ); +} type AdvancedSettingsProps = Parameters[0]; @@ -81,6 +97,7 @@ const makeProps = (overrides: Partial = {}): AdvancedSett describe("AdvancedSettings", () => { beforeEach(() => { vi.clearAllMocks(); + mockTeams([personalTeam]); mockUseAuthContext.mockReturnValue(makeAuthContext()); }); @@ -155,17 +172,28 @@ describe("AdvancedSettings", () => { expect(onTeamIdChange).toHaveBeenCalledWith(""); }); - it("clears teamId when selectedTeamId becomes null while visibility is team", () => { - mockUseAuthContext.mockReturnValue(makeAuthContext(null)); + it("falls back to the caller's own team on a switch to All teams", () => { + mockUseAuthContext.mockReturnValue(makeAuthContext("team-A")); const onTeamIdChange = vi.fn(); + const { rerender } = render( + , + ); + onTeamIdChange.mockClear(); - render( + mockUseAuthContext.mockReturnValue(makeAuthContext(null)); + rerender( , ); - expect(onTeamIdChange).toHaveBeenCalledWith(""); + // "All teams" is not a scope a server can be created in, so the form + // falls back rather than leaving it unscoped. + return waitFor(() => { + expect(onTeamIdChange).toHaveBeenCalledWith(personalTeam.id); + }); }); it("does not call onTeamIdChange when visibility is not team and teamId is already empty", () => { @@ -208,32 +236,32 @@ describe("AdvancedSettings", () => { }); }); - describe("team visibility — hint message", () => { - it("shows 'scoped to currently selected team' when visibility is team and a team is selected", () => { - mockUseAuthContext.mockReturnValue(makeAuthContext("team-A")); - - render(); + describe("team visibility — selector", () => { + it("stays hidden for a single team", async () => { + render(); - expect(screen.getByText(/scoped to your currently selected team/i)).toBeInTheDocument(); + await waitFor(() => { + expect(screen.queryByRole("combobox", { name: /^team/i })).not.toBeInTheDocument(); + }); }); - it("shows 'please select a team' when visibility is team but no team is selected", () => { - mockUseAuthContext.mockReturnValue(makeAuthContext(null)); + it("lists the caller's teams", async () => { + mockTeams([personalTeam, sharedTeam]); - render(); + render(); - expect(screen.getByText(/please select a team using the team switcher/i)).toBeInTheDocument(); + const teamSelect = await screen.findByRole("combobox", { name: /^team/i }); + expect(teamSelect).toHaveTextContent("Personal team"); }); - it("does not show either team hint when visibility is not team", () => { - mockUseAuthContext.mockReturnValue(makeAuthContext("team-A")); + it("stays hidden when visibility is not team", async () => { + mockTeams([personalTeam, sharedTeam]); render(); - expect(screen.queryByText(/scoped to your currently selected team/i)).not.toBeInTheDocument(); - expect( - screen.queryByText(/please select a team using the team switcher/i), - ).not.toBeInTheDocument(); + await waitFor(() => { + expect(screen.queryByRole("combobox", { name: /^team/i })).not.toBeInTheDocument(); + }); }); }); diff --git a/src/components/mcp-servers/AdvancedSettings.tsx b/src/components/mcp-servers/AdvancedSettings.tsx index bd53ec3..5c5f501 100644 --- a/src/components/mcp-servers/AdvancedSettings.tsx +++ b/src/components/mcp-servers/AdvancedSettings.tsx @@ -1,4 +1,4 @@ -import { useEffect } from "react"; +import { useEffect, useState } from "react"; import { useIntl } from "react-intl"; import { Info, TriangleAlert } from "lucide-react"; import { Textarea } from "@/components/ui/textarea"; @@ -18,8 +18,10 @@ import { CustomHeadersAuth, type CustomHeader } from "@/components/mcp-servers/C import { OAuth2Auth } from "@/components/mcp-servers/OAuth2Auth"; import { QueryParameterAuth } from "@/components/mcp-servers/QueryParameterAuth"; import { useAuthContext } from "@/auth/AuthContext"; +import { resolveTeamId, useTeams } from "@/hooks/useTeams"; import type { Visibility } from "@/types/server"; import { VisibilityInfoPopover } from "@/components/common/VisibilityInfoPopover"; +import { TeamSelect } from "@/components/common/TeamSelect"; export type { CustomHeader }; @@ -30,6 +32,8 @@ interface AdvancedSettingsProps { onVisibilityChange: (value: Visibility) => void; teamId: string; onTeamIdChange: (value: string) => void; + /** Validation message for the team field, shown on the selector. */ + teamError?: string; authType: AuthType; onAuthTypeChange: (value: AuthType) => void; basicAuthUsername: string; @@ -81,6 +85,7 @@ export function AdvancedSettings({ onVisibilityChange, teamId, onTeamIdChange, + teamError, authType, onAuthTypeChange, basicAuthUsername, @@ -127,17 +132,32 @@ export function AdvancedSettings({ oauthErrors, }: AdvancedSettingsProps) { const { selectedTeamId } = useAuthContext(); + const { teams } = useTeams(); const intl = useIntl(); + const [pickedInForm, setPickedInForm] = useState(false); + + // The sidebar switcher stays authoritative (#5077) until the caller picks a + // team in the selector below. "All teams" is not a scope a server can be + // created in, so it resolves to the caller's own team rather than leaving the + // server unscoped. useEffect(() => { - if (visibility === "team") { - if ((selectedTeamId ?? "") !== teamId) { - onTeamIdChange(selectedTeamId ?? ""); - } - } else if (teamId) { - onTeamIdChange(""); + if (visibility !== "team") { + if (teamId) onTeamIdChange(""); + return; + } + if (pickedInForm) return; + + const resolved = resolveTeamId(teams, selectedTeamId); + if (resolved && resolved !== teamId) { + onTeamIdChange(resolved); } - }, [visibility, selectedTeamId, teamId, onTeamIdChange]); + }, [visibility, selectedTeamId, teams, teamId, pickedInForm, onTeamIdChange]); + + const handleTeamChange = (nextTeamId: string) => { + setPickedInForm(true); + onTeamIdChange(nextTeamId); + }; const renderAuthContent = () => { switch (authType) { @@ -234,15 +254,18 @@ export function AdvancedSettings({ - {visibility === "team" && ( -

- {selectedTeamId - ? "This server will be scoped to your currently selected team" - : "Please select a team using the team switcher in the sidebar"} -

- )} + {visibility === "team" && ( + + )} + {/* Authentication type */}