Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions src/components/common/TeamSelect.test.tsx
Original file line number Diff line number Diff line change
@@ -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(
<TeamSelect teams={[personalTeam]} value={personalTeam.id} onChange={vi.fn()} />,
);

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(<TeamSelect teams={[]} onChange={vi.fn()} error="Team is required" />);

expect(screen.getByText("Team is required")).toBeInTheDocument();
});

it("reports the chosen team", async () => {
const onChange = vi.fn();
renderWithProviders(
<TeamSelect teams={[personalTeam, sharedTeam]} value={personalTeam.id} onChange={onChange} />,
);

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(
<TeamSelect
teams={[personalTeam, sharedTeam]}
onChange={vi.fn()}
error="Team is required"
id="prompt-team"
/>,
);

const select = screen.getByRole("combobox", { name: /^team/i });
expect(select).toHaveAttribute("aria-invalid", "true");
expect(select).toHaveAccessibleDescription("Team is required");
});
});
78 changes: 78 additions & 0 deletions src/components/common/TeamSelect.tsx
Original file line number Diff line number Diff line change
@@ -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 ? (
<p id={errorId} className="text-sm text-destructive">
{error}
</p>
) : null;
}

return (
<div className="space-y-2.5">
<Label htmlFor={id} className="block text-sm font-medium text-foreground">
{intl.formatMessage({ id: "common.team.label" })}{" "}
<span className="text-destructive" aria-hidden="true">
{intl.formatMessage({ id: "common.required" })}
</span>
</Label>
<Select value={value ?? ""} onValueChange={onChange}>
<SelectTrigger
id={id}
aria-required="true"
aria-invalid={!!error}
aria-describedby={error ? errorId : undefined}
className="h-10 w-full rounded-md border-neutral-300 text-sm shadow-none focus-visible:ring-1 focus-visible:ring-ring focus-visible:ring-offset-0 dark:border-neutral-700"
>
<SelectValue placeholder={intl.formatMessage({ id: "common.team.placeholder" })} />
</SelectTrigger>
<SelectContent>
{teams.map((team) => (
<SelectItem key={team.id} value={team.id}>
{team.name}
</SelectItem>
))}
</SelectContent>
</Select>
{error && (
<p id={errorId} className="text-sm text-destructive">
{error}
</p>
)}
</div>
);
}
70 changes: 49 additions & 21 deletions src/components/mcp-servers/AdvancedSettings.test.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,30 @@
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";

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<Record<string, unknown>>) {
mockGet.mockImplementation((path: string) =>
path === "/teams" ? Promise.resolve({ teams }) : Promise.resolve([]),
);
}

type AdvancedSettingsProps = Parameters<typeof AdvancedSettings>[0];

Expand Down Expand Up @@ -81,6 +97,7 @@ const makeProps = (overrides: Partial<AdvancedSettingsProps> = {}): AdvancedSett
describe("AdvancedSettings", () => {
beforeEach(() => {
vi.clearAllMocks();
mockTeams([personalTeam]);
mockUseAuthContext.mockReturnValue(makeAuthContext());
});

Expand Down Expand Up @@ -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(
<AdvancedSettings
{...makeProps({ visibility: "team", teamId: "team-A", onTeamIdChange })}
/>,
);
onTeamIdChange.mockClear();

render(
mockUseAuthContext.mockReturnValue(makeAuthContext(null));
rerender(
<AdvancedSettings
{...makeProps({ visibility: "team", teamId: "team-A", onTeamIdChange })}
/>,
);

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", () => {
Expand Down Expand Up @@ -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(<AdvancedSettings {...makeProps({ visibility: "team", teamId: "team-A" })} />);
describe("team visibility — selector", () => {
it("stays hidden for a single team", async () => {
render(<AdvancedSettings {...makeProps({ visibility: "team", teamId: personalTeam.id })} />);

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(<AdvancedSettings {...makeProps({ visibility: "team", teamId: "" })} />);
render(<AdvancedSettings {...makeProps({ visibility: "team", teamId: personalTeam.id })} />);

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(<AdvancedSettings {...makeProps({ visibility: "public" })} />);

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();
});
});
});

Expand Down
53 changes: 38 additions & 15 deletions src/components/mcp-servers/AdvancedSettings.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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 };

Expand All @@ -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;
Expand Down Expand Up @@ -81,6 +85,7 @@ export function AdvancedSettings({
onVisibilityChange,
teamId,
onTeamIdChange,
teamError,
authType,
onAuthTypeChange,
basicAuthUsername,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -234,15 +254,18 @@ export function AdvancedSettings({
</SelectItem>
</SelectContent>
</Select>
{visibility === "team" && (
<p className="text-sm text-neutral-600 dark:text-neutral-400">
{selectedTeamId
? "This server will be scoped to your currently selected team"
: "Please select a team using the team switcher in the sidebar"}
</p>
)}
</div>

{visibility === "team" && (
<TeamSelect
id="server-team"
teams={teams}
value={teamId || undefined}
onChange={handleTeamChange}
error={teamError}
/>
)}

{/* Authentication type */}
<div className="space-y-3">
<label className="text-sm font-medium text-neutral-950 dark:text-white">
Expand Down
Loading
Loading