fix(ui): withhold model row actions and team edit rights from view-only admins

This commit is contained in:
mateo-berri 2026-09-05 04:05:30 -07:00
parent bf6aa9c390
commit 344b992bed
6 changed files with 52 additions and 7 deletions

View file

@ -48,7 +48,7 @@ const AllModelsTab = ({
setSelectedTeamId,
}: AllModelsTabProps) => {
const { data: modelCostMapData, isLoading: isLoadingModelCostMap } = useModelCostMap();
const { accessToken, userId, userRole } = useAuthorized();
const { accessToken, userId, userRole, isViewOnly } = useAuthorized();
const { data: teams, isLoading: isLoadingTeams } = useTeams();
const queryClient = useQueryClient();
@ -295,6 +295,7 @@ const AllModelsTab = ({
availableModelAccessGroups={availableModelAccessGroups}
userRole={userRole}
userID={userId}
isViewOnly={isViewOnly}
onModelIdClick={setSelectedModelId}
onTeamIdClick={setSelectedTeamId}
onDeleteClick={handleDeleteClick}

View file

@ -59,6 +59,7 @@ const baseProps = {
availableModelAccessGroups: ["sales-team"],
userRole: "Admin",
userID: "alice",
isViewOnly: false,
onModelIdClick: vi.fn(),
onTeamIdClick: vi.fn(),
onDeleteClick: vi.fn(),
@ -254,6 +255,17 @@ describe("AllModelsTable", () => {
expect(onTogglePauseClick).not.toHaveBeenCalled();
});
it("does not let a view-only admin toggle a model", async () => {
const user = userEvent.setup();
const onTogglePauseClick = vi.fn();
render(<AllModelsTable {...baseProps} isViewOnly onTogglePauseClick={onTogglePauseClick} />);
const toggle = screen.getByTestId("model-pause-toggle-model-1");
expect(toggle).toHaveAttribute("data-disabled");
await user.click(toggle);
expect(onTogglePauseClick).not.toHaveBeenCalled();
});
it("does not let anyone toggle a config model", async () => {
const user = userEvent.setup();
const onTogglePauseClick = vi.fn();
@ -309,6 +321,17 @@ describe("AllModelsTable", () => {
expect(onDeleteClick).not.toHaveBeenCalled();
});
it("blocks a view-only admin from deleting a DB model they created", async () => {
const user = userEvent.setup();
const onDeleteClick = vi.fn();
render(<AllModelsTable {...baseProps} isViewOnly onDeleteClick={onDeleteClick} />);
const deleteButton = screen.getByTestId("model-delete-model-1");
expect(deleteButton).toBeDisabled();
await user.click(deleteButton);
expect(onDeleteClick).not.toHaveBeenCalled();
});
it("blocks deleting a config model", async () => {
const user = userEvent.setup();
const onDeleteClick = vi.fn();

View file

@ -73,6 +73,7 @@ interface AllModelsTableProps {
availableModelAccessGroups: string[];
userRole: string;
userID: string;
isViewOnly: boolean;
onModelIdClick: (modelId: string) => void;
onTeamIdClick: (teamId: string) => void;
onDeleteClick: (modelId: string) => void;
@ -120,6 +121,7 @@ export function AllModelsTable({
availableModelAccessGroups,
userRole,
userID,
isViewOnly,
onModelIdClick,
onTeamIdClick,
onDeleteClick,
@ -132,6 +134,7 @@ export function AllModelsTable({
const columnDeps = {
userRole,
userID,
isViewOnly,
onModelIdClick,
onTeamIdClick,
onDeleteClick,
@ -139,7 +142,7 @@ export function AllModelsTable({
pausingModelId,
};
return getModelsTableColumns(columnDeps);
}, [userRole, userID, onModelIdClick, onTeamIdClick, onDeleteClick, onTogglePauseClick, pausingModelId]);
}, [userRole, userID, isViewOnly, onModelIdClick, onTeamIdClick, onDeleteClick, onTogglePauseClick, pausingModelId]);
const modelGroupOptions = useMemo(
() => [

View file

@ -247,6 +247,7 @@ interface ModelRowActionsProps {
model: ModelData;
userRole: string;
userID: string;
isViewOnly: boolean;
isPausing: boolean;
onDeleteClick?: (modelId: string) => void;
onTogglePauseClick?: (modelId: string, blocked: boolean) => void | Promise<void>;
@ -256,14 +257,15 @@ function ModelRowActions({
model,
userRole,
userID,
isViewOnly,
isPausing,
onDeleteClick,
onTogglePauseClick,
}: ModelRowActionsProps) {
const modelId = model.model_info?.id;
const isConfigModel = !model.model_info?.db_model;
const isAdmin = userRole === "Admin";
const canEditModel = isAdmin || model.model_info?.created_by === userID;
const isAdmin = userRole === "Admin" && !isViewOnly;
const canEditModel = !isViewOnly && (isAdmin || model.model_info?.created_by === userID);
const isBlocked = model.model_info?.blocked === true;
const isPauseToggleable = !isConfigModel && isAdmin && Boolean(onTogglePauseClick);
@ -340,6 +342,7 @@ function ModelRowActions({
export interface ModelsTableColumnDeps {
userRole: string;
userID: string;
isViewOnly: boolean;
onModelIdClick: (modelId: string) => void;
onTeamIdClick: (teamId: string) => void;
onDeleteClick?: (modelId: string) => void;
@ -350,6 +353,7 @@ export interface ModelsTableColumnDeps {
export const getModelsTableColumns = ({
userRole,
userID,
isViewOnly,
onModelIdClick,
onTeamIdClick,
onDeleteClick,
@ -479,6 +483,7 @@ export const getModelsTableColumns = ({
model={row.original}
userRole={userRole}
userID={userID}
isViewOnly={isViewOnly}
isPausing={pausingModelId === row.original.model_info?.id}
onDeleteClick={onDeleteClick}
onTogglePauseClick={onTogglePauseClick}

View file

@ -26,7 +26,11 @@ vi.mock("@/components/model_info_view", () => ({
default: ({ modelId }: { modelId: string }) => <div data-testid="model-info">model:{modelId}</div>,
}));
vi.mock("@/components/team/TeamInfo", () => ({
default: ({ teamId }: { teamId: string }) => <div data-testid="team-info">team:{teamId}</div>,
default: ({ teamId, is_team_admin }: { teamId: string; is_team_admin: boolean }) => (
<div data-testid="team-info" data-team-admin={String(is_team_admin)}>
team:{teamId}
</div>
),
}));
const mockUseAuthorized = vi.fn();
@ -96,10 +100,19 @@ describe("ModelsAndEndpointsPage", () => {
expect(screen.queryByRole("tab", { name: "All Models" })).not.toBeInTheDocument();
});
it("renders the team detail overlay from the ?team drill-in", () => {
it("renders the team detail overlay from the ?team drill-in with admin edit rights", () => {
detailState.teamId = "team-9";
renderPage();
expect(screen.getByTestId("team-info")).toHaveTextContent("team:team-9");
expect(screen.getByTestId("team-info")).toHaveAttribute("data-team-admin", "true");
});
it("opens the ?team drill-in without edit rights for a view-only admin", () => {
mockUseAuthorized.mockReturnValue(VIEW_ONLY_ADMIN);
detailState.teamId = "team-9";
renderPage();
expect(screen.getByTestId("team-info")).toHaveTextContent("team:team-9");
expect(screen.getByTestId("team-info")).toHaveAttribute("data-team-admin", "false");
});
it("hides admin-only tabs for a non-admin user", () => {

View file

@ -145,7 +145,7 @@ export default function ModelsAndEndpointsPage() {
teamId={teamId}
onClose={close}
accessToken={accessToken}
is_team_admin={userRole === "Admin"}
is_team_admin={userRole === "Admin" && !isViewOnly}
is_proxy_admin={userRole === "Proxy Admin"}
userModels={allModelsOnProxy}
editTeam={false}