From 4a0758bae52f7506ce2e602e2a7a0cbd9394edd2 Mon Sep 17 00:00:00 2001 From: Tom Ratcliffe Date: Thu, 31 Jul 2025 16:01:43 +0100 Subject: [PATCH] Teams: Tidy up CreateTeam tests and add fixtures/handlers (#108978) * Add fixtures and mock handlers for legacy teams API * Fix teams betterer issues and error handling * Refactor CreateTeams tests * Update .betterer.results --- .betterer.results | 5 - .../grafana-test-utils/src/fixtures/teams.ts | 25 +++++ .../src/handlers/all-handlers.ts | 4 +- .../src/handlers/api/teams/handlers.ts | 53 +++++++++ packages/grafana-test-utils/src/unstable.ts | 2 +- public/app/features/teams/CreateTeam.test.tsx | 103 +++++++++--------- public/app/features/teams/CreateTeam.tsx | 77 ++++++------- 7 files changed, 176 insertions(+), 93 deletions(-) create mode 100644 packages/grafana-test-utils/src/fixtures/teams.ts create mode 100644 packages/grafana-test-utils/src/handlers/api/teams/handlers.ts diff --git a/.betterer.results b/.betterer.results index 4482c94acc5..239af992de5 100644 --- a/.betterer.results +++ b/.betterer.results @@ -2752,11 +2752,6 @@ exports[`better eslint`] = { [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "0"], [0, 0, 0, "Using localeCompare() can cause performance issues when sorting large datasets. Consider using Intl.Collator for better performance when sorting arrays, or add an eslint-disable comment if sorting a small, known dataset.", "1"] ], - "public/app/features/teams/CreateTeam.tsx:5381": [ - [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "0"], - [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "1"], - [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "2"] - ], "public/app/features/teams/TeamSettings.tsx:5381": [ [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "0"], [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "1"], diff --git a/packages/grafana-test-utils/src/fixtures/teams.ts b/packages/grafana-test-utils/src/fixtures/teams.ts new file mode 100644 index 00000000000..86c737fd7a7 --- /dev/null +++ b/packages/grafana-test-utils/src/fixtures/teams.ts @@ -0,0 +1,25 @@ +import { Chance } from 'chance'; + +const chance = new Chance('mock-teams'); + +export const MOCK_TEAMS = [ + { + metadata: { + name: chance.string({ length: 14, pool: 'abcdefghijklmnopqrstuvwxyz1234567890' }), + namespace: 'default', + resourceVersion: '1737038862000', + creationTimestamp: '2025-01-16T14:47:42Z', + labels: { + 'grafana.app/deprecatedInternalID': chance.integer({ min: 1, max: 1000 }).toString(), + }, + annotations: { + 'grafana.app/updatedTimestamp': '2025-01-16T14:47:42Z', + }, + }, + spec: { + title: 'Test Team', + email: 'foo@example.com', + }, + status: {}, + }, +]; diff --git a/packages/grafana-test-utils/src/handlers/all-handlers.ts b/packages/grafana-test-utils/src/handlers/all-handlers.ts index be4d730a128..edf4c592ef2 100644 --- a/packages/grafana-test-utils/src/handlers/all-handlers.ts +++ b/packages/grafana-test-utils/src/handlers/all-handlers.ts @@ -1,5 +1,7 @@ import { HttpHandler } from 'msw'; -const allHandlers: HttpHandler[] = []; +import teamsHandlers from './api/teams/handlers'; + +const allHandlers: HttpHandler[] = [...teamsHandlers]; export default allHandlers; diff --git a/packages/grafana-test-utils/src/handlers/api/teams/handlers.ts b/packages/grafana-test-utils/src/handlers/api/teams/handlers.ts new file mode 100644 index 00000000000..31d513a2263 --- /dev/null +++ b/packages/grafana-test-utils/src/handlers/api/teams/handlers.ts @@ -0,0 +1,53 @@ +import { HttpResponse, http } from 'msw'; + +import { MOCK_TEAMS } from '../../../fixtures/teams'; + +const k8sTeamToLegacyTeam = (k8sTeam: (typeof MOCK_TEAMS)[number]) => { + return { + name: k8sTeam.spec.title, + email: k8sTeam.spec.email, + id: Number(k8sTeam.metadata.labels['grafana.app/deprecatedInternalID']), + uid: k8sTeam.metadata.name, + orgId: 1, + externalUID: '', + isProvisioned: false, + avatarUrl: '', + memberCount: 0, + permission: 0, + }; +}; + +const searchTeamsHandler = () => + http.get('/api/teams/search', async ({ request }) => { + const url = new URL(request.url); + // TODO in future: pagination and mock querying + const page = url.searchParams.get('page') ?? 1; + const perPage = url.searchParams.get('perPage') ?? 1000; + + return HttpResponse.json({ + totalCount: MOCK_TEAMS.length, + teams: MOCK_TEAMS.map(k8sTeamToLegacyTeam), + page, + perPage, + }); + }); + +const createTeamHandler = () => + http.post('/api/teams', async ({ request }) => { + const body = await request.json(); + + if (!body.name) { + return HttpResponse.json({ message: 'bad request data' }, { status: 400 }); + } + + const existingTeam = MOCK_TEAMS.find((t) => t.spec.title === body.name); + + if (existingTeam) { + return HttpResponse.json({ message: 'Team name taken' }, { status: 409 }); + } + return HttpResponse.json({ message: 'Team created', teamId: 10, uid: 'aethyfifmhwcgd' }, { status: 200 }); + }); + +const handlers = [searchTeamsHandler(), createTeamHandler()]; + +export default handlers; diff --git a/packages/grafana-test-utils/src/unstable.ts b/packages/grafana-test-utils/src/unstable.ts index cb0ff5c3b54..5b60f83479c 100644 --- a/packages/grafana-test-utils/src/unstable.ts +++ b/packages/grafana-test-utils/src/unstable.ts @@ -1 +1 @@ -export {}; +export { MOCK_TEAMS } from './fixtures/teams'; diff --git a/public/app/features/teams/CreateTeam.test.tsx b/public/app/features/teams/CreateTeam.test.tsx index bb219be7a71..519b835cd52 100644 --- a/public/app/features/teams/CreateTeam.test.tsx +++ b/public/app/features/teams/CreateTeam.test.tsx @@ -1,70 +1,75 @@ -import { render, screen, waitFor } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import { TestProvider } from 'test/helpers/TestProvider'; +import { UserEvent } from '@testing-library/user-event'; +import { Route, Routes } from 'react-router-dom-v5-compat'; +import { render, screen, waitFor } from 'test/test-utils'; -import { BackendSrv, setBackendSrv } from '@grafana/runtime'; +import { setBackendSrv } from '@grafana/runtime'; +import { setupMockServer } from '@grafana/test-utils/server'; +import { MOCK_TEAMS } from '@grafana/test-utils/unstable'; +import { backendSrv } from 'app/core/services/backend_srv'; +import { contextSrv } from 'app/core/services/context_srv'; import { CreateTeam } from './CreateTeam'; -beforeEach(() => { - jest.clearAllMocks(); -}); +setBackendSrv(backendSrv); +setupMockServer(); -jest.mock('app/core/core', () => ({ - contextSrv: { - licensedAccessControlEnabled: () => false, - hasPermission: () => true, - hasPermissionInMetadata: () => true, - user: { orgId: 1 }, - }, -})); - -jest.mock('app/core/components/RolePicker/hooks', () => ({ - useRoleOptions: jest.fn().mockReturnValue([{ roleOptions: [] }, jest.fn()]), -})); - -const mockPost = jest.fn(() => { - return Promise.resolve({}); -}); - -setBackendSrv({ - post: mockPost, -} as unknown as BackendSrv); - -const setup = () => { - return render( - - - +const setup = async () => { + const view = render( + + } /> + Edit team page} /> + , + { + historyOptions: { initialEntries: ['/org/teams/create'] }, + } ); + await waitFor(async () => expect(screen.queryAllByTestId('Spinner')).toHaveLength(0)); + return view; +}; + +const attemptCreateTeam = async (user: UserEvent, teamName?: string, teamEmail?: string) => { + teamName && (await user.type(screen.getByRole('textbox', { name: /name/i }), teamName)); + teamEmail && (await user.type(screen.getByLabelText(/email/i), teamEmail)); + await user.click(screen.getByRole('button', { name: /create/i })); }; describe('Create team', () => { - it('should render component', () => { - setup(); + beforeEach(() => { + contextSrv.licensedAccessControlEnabled = () => false; + contextSrv.hasPermission = () => true; + contextSrv.hasPermissionInMetadata = () => true; + contextSrv.fetchUserPermissions = () => Promise.resolve(); + }); + + it('should render component', async () => { + await setup(); + await waitFor(async () => expect(screen.queryAllByTestId('Spinner')).toHaveLength(0)); expect(screen.getByRole('textbox', { name: /name/i })).toBeInTheDocument(); expect(screen.getByLabelText(/email/i)).toBeInTheDocument(); expect(screen.getByRole('button')).toBeInTheDocument(); }); it('should send correct data to the server', async () => { - setup(); - await userEvent.type(screen.getByRole('textbox', { name: /name/i }), 'Test team'); - await userEvent.type(screen.getByLabelText(/email/i), 'team@test.com'); - await userEvent.click(screen.getByRole('button', { name: /create/i })); - await waitFor(() => { - expect(mockPost).toHaveBeenCalledWith(expect.anything(), { name: 'Test team', email: 'team@test.com' }); - }); + const { user } = await setup(); + await attemptCreateTeam(user, 'Test team', 'team@test.com'); + + expect(await screen.findByText(/edit team page/i)).toBeInTheDocument(); }); it('should validate required fields', async () => { - setup(); - await userEvent.type(screen.getByLabelText(/email/i), 'team@test.com'); - await userEvent.click(screen.getByRole('button', { name: /create/i })); - await waitFor(() => { - expect(mockPost).not.toBeCalled(); - }); - expect(screen.getAllByRole('alert')).toHaveLength(1); + const { user } = await setup(); + await attemptCreateTeam(user, undefined, 'team@test.com'); + + expect(await screen.findAllByRole('alert')).toHaveLength(1); expect(screen.getByText(/team name is required/i)).toBeInTheDocument(); + expect(screen.queryByText(/edit team page/i)).not.toBeInTheDocument(); + }); + + it('prevents creation of duplicate team name', async () => { + jest.spyOn(console, 'error').mockImplementation(); + const { user } = await setup(); + await attemptCreateTeam(user, MOCK_TEAMS[0].spec.title); + + expect(screen.queryByText(/edit team page/i)).not.toBeInTheDocument(); }); }); diff --git a/public/app/features/teams/CreateTeam.tsx b/public/app/features/teams/CreateTeam.tsx index 0744271c048..256c557756d 100644 --- a/public/app/features/teams/CreateTeam.tsx +++ b/public/app/features/teams/CreateTeam.tsx @@ -4,7 +4,7 @@ import { useForm } from 'react-hook-form'; import { NavModelItem } from '@grafana/data'; import { Trans, t } from '@grafana/i18n'; import { getBackendSrv, locationService } from '@grafana/runtime'; -import { Button, Field, Input, FieldSet } from '@grafana/ui'; +import { Button, Field, Input, FieldSet, Stack } from '@grafana/ui'; import { Page } from 'app/core/components/Page/Page'; import { TeamRolePicker } from 'app/core/components/RolePicker/TeamRolePicker'; import { updateTeamRoles } from 'app/core/components/RolePicker/api'; @@ -35,17 +35,17 @@ export const CreateTeam = (): JSX.Element => { contextSrv.hasPermission(AccessControlAction.ActionUserRolesRemove); const createTeam = async (formModel: TeamDTO) => { - const newTeam = await getBackendSrv().post('/api/teams', formModel); - if (newTeam.teamId) { - try { + try { + const newTeam = await getBackendSrv().post('/api/teams', formModel); + if (newTeam.teamId) { await contextSrv.fetchUserPermissions(); if (contextSrv.licensedAccessControlEnabled() && canUpdateRoles) { await updateTeamRoles(pendingRoles, newTeam.teamId, newTeam.orgId); } - } catch (e) { - console.error(e); + locationService.push(`/org/teams/edit/${newTeam.uid}`); } - locationService.push(`/org/teams/edit/${newTeam.uid}`); + } catch (e) { + console.error(e); } }; @@ -54,39 +54,42 @@ export const CreateTeam = (): JSX.Element => {
- - - - {contextSrv.licensedAccessControlEnabled() && ( - - + + + - )} - + + )} - > - {/* eslint-disable-next-line @grafana/i18n/no-untranslated-strings */} - - + + {/* eslint-disable-next-line @grafana/i18n/no-untranslated-strings */} + + +
-