From 2aa690b694d44b01e56b05f78f7e1fbdb62c3049 Mon Sep 17 00:00:00 2001 From: Viktor Vorona Date: Fri, 15 Sep 2023 16:01:11 +0200 Subject: [PATCH] SONAR-20348 Improve axios + cover users endpoints with axios --- .../src/main/js/api/mocks/UsersServiceMock.ts | 28 +++++------ .../sonar-web/src/main/js/api/provisioning.ts | 8 ++- server/sonar-web/src/main/js/api/users.ts | 50 ++++++------------- .../js/apps/users/__tests__/UsersApp-it.tsx | 45 +++++++++++++++++ .../js/apps/users/components/UserForm.tsx | 35 +++++++------ server/sonar-web/src/main/js/queries/users.ts | 8 ++- server/sonar-web/src/main/js/types/axios.d.ts | 36 +++++++++++++ server/sonar-web/src/main/js/types/users.ts | 4 +- 8 files changed, 141 insertions(+), 73 deletions(-) create mode 100644 server/sonar-web/src/main/js/types/axios.d.ts diff --git a/server/sonar-web/src/main/js/api/mocks/UsersServiceMock.ts b/server/sonar-web/src/main/js/api/mocks/UsersServiceMock.ts index d8e61963ae1..3c76abf76b0 100644 --- a/server/sonar-web/src/main/js/api/mocks/UsersServiceMock.ts +++ b/server/sonar-web/src/main/js/api/mocks/UsersServiceMock.ts @@ -20,9 +20,10 @@ import { isAfter, isBefore } from 'date-fns'; import { cloneDeep, isEmpty, isUndefined, omitBy } from 'lodash'; +import { HttpStatus } from '../../helpers/request'; import { mockClusterSysInfo, mockIdentityProvider, mockRestUser } from '../../helpers/testMocks'; import { IdentityProvider, SysInfoCluster } from '../../types/types'; -import { ChangePasswordResults, RestUserDetailed, User } from '../../types/users'; +import { ChangePasswordResults, RestUserDetailed } from '../../types/users'; import { getSystemInfo } from '../system'; import { addUserToGroup, removeUserFromGroup } from '../user_groups'; import { @@ -130,7 +131,7 @@ export default class UsersServiceMock { constructor() { jest.mocked(getSystemInfo).mockImplementation(this.handleGetSystemInfo); jest.mocked(getIdentityProviders).mockImplementation(this.handleGetIdentityProviders); - jest.mocked(getUsers).mockImplementation((p) => this.handleGetUsers(p)); + jest.mocked(getUsers).mockImplementation(this.handleGetUsers); jest.mocked(postUser).mockImplementation(this.handlePostUser); jest.mocked(updateUser).mockImplementation(this.handleUpdateUser); jest.mocked(getUserGroups).mockImplementation(this.handleGetUserGroups); @@ -265,8 +266,10 @@ export default class UsersServiceMock { const { email, local, login, name, scmAccounts } = data; if (scmAccounts.some((a) => isEmpty(a.trim()))) { return Promise.reject({ - status: 400, - json: () => Promise.resolve({ message: 'Error: Empty SCM' }), + response: { + status: HttpStatus.BadRequest, + data: { message: 'Error: Empty SCM' }, + }, }); } const newUser = mockRestUser({ @@ -277,24 +280,19 @@ export default class UsersServiceMock { scmAccounts, }); this.users.push(newUser); - return this.reply(undefined); + return this.reply(newUser); }; - handleUpdateUser = (data: { - email?: string; - login: string; - name: string; - scmAccount: string[]; - }) => { - const { email, login, name, scmAccount } = data; - const user = this.users.find((u) => u.login === login) as User; + handleUpdateUser: typeof updateUser = (id, data) => { + const { email, name, scmAccounts } = data; + const user = this.users.find((u) => u.login === id); if (!user) { return Promise.reject('No such user'); } Object.assign(user, { - ...omitBy({ name, email, scmAccounts: scmAccount }, isUndefined), + ...omitBy({ name, email, scmAccounts }, isUndefined), }); - return this.reply({ user }); + return this.reply(user); }; handleGetIdentityProviders = (): Promise<{ identityProviders: IdentityProvider[] }> => { diff --git a/server/sonar-web/src/main/js/api/provisioning.ts b/server/sonar-web/src/main/js/api/provisioning.ts index 756bf57dfa4..b33172edd22 100644 --- a/server/sonar-web/src/main/js/api/provisioning.ts +++ b/server/sonar-web/src/main/js/api/provisioning.ts @@ -58,15 +58,13 @@ export function syncNowGithubProvisioning(): Promise { export function fetchGithubRolesMapping() { return axios - .get( - '/api/v2/github-permission-mappings', - ) + .get<{ githubPermissionsMappings: GitHubMapping[] }>('/api/v2/github-permission-mappings') .then((data) => data.githubPermissionsMappings); } export function updateGithubRolesMapping( role: string, data: Partial>, -): Promise { - return axios.patch(`/api/v2/github-permission-mappings/${role}`, data); +) { + return axios.patch(`/api/v2/github-permission-mappings/${role}`, data); } diff --git a/server/sonar-web/src/main/js/api/users.ts b/server/sonar-web/src/main/js/api/users.ts index 754da05a7d5..218806c32b0 100644 --- a/server/sonar-web/src/main/js/api/users.ts +++ b/server/sonar-web/src/main/js/api/users.ts @@ -17,16 +17,9 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +import axios from 'axios'; import { throwGlobalError } from '../helpers/error'; -import { - deleteJSON, - getJSON, - HttpStatus, - parseJSON, - post, - postJSON, - postJSONBody, -} from '../helpers/request'; +import { HttpStatus, axiosToCatch, getJSON, parseJSON, post } from '../helpers/request'; import { IdentityProvider, Paging } from '../types/types'; import { ChangePasswordResults, @@ -34,7 +27,7 @@ import { HomePage, NoticeType, RestUserBase, - User, + RestUserDetailed, } from '../types/users'; export function getCurrentUser(): Promise { @@ -92,8 +85,8 @@ export function getUsers(data: { sonarLintLastConnectionDateTo?: string; pageSize?: number; pageIndex?: number; -}): Promise<{ page: Paging; users: T[] }> { - return getJSON('/api/v2/users', data).catch(throwGlobalError); +}) { + return axios.get<{ page: Paging; users: T[] }>(`/api/v2/users`, { params: data }); } export function postUser(data: { @@ -102,34 +95,19 @@ export function postUser(data: { name: string; password?: string; scmAccounts: string[]; -}): Promise { - return postJSONBody('/api/v2/users', data); +}) { + return axiosToCatch.post('/api/v2/users', data); } -type UpdateUserArg = - | { - email?: string; - login: string; - name?: string; - scmAccount: string[]; - } - | { login: string; scmAccount: string[] }; - -export function updateUser(data: UpdateUserArg): Promise<{ user: User }> { - return postJSON('/api/users/update', { - ...data, - scmAccount: data.scmAccount.length > 0 ? data.scmAccount : '', - }); +export function updateUser( + id: string, + data: Partial>, +) { + return axiosToCatch.patch(`/api/v2/users/${id}`, data); } -export function deleteUser({ - login, - anonymize, -}: { - login: string; - anonymize?: boolean; -}): Promise { - return deleteJSON(`/api/v2/users/${login}`, { anonymize }).catch(throwGlobalError); +export function deleteUser({ login, anonymize }: { login: string; anonymize?: boolean }) { + return axios.delete(`/api/v2/users/${login}`, { params: { anonymize } }); } export function setHomePage(homepage: HomePage): Promise { diff --git a/server/sonar-web/src/main/js/apps/users/__tests__/UsersApp-it.tsx b/server/sonar-web/src/main/js/apps/users/__tests__/UsersApp-it.tsx index 19fc0093fb0..74ed1c6839f 100644 --- a/server/sonar-web/src/main/js/apps/users/__tests__/UsersApp-it.tsx +++ b/server/sonar-web/src/main/js/apps/users/__tests__/UsersApp-it.tsx @@ -51,6 +51,7 @@ const ui = { showMore: byRole('button', { name: 'show_more' }), aliceUpdateGroupButton: byRole('button', { name: 'users.update_users_groups.alice.merveille' }), aliceUpdateButton: byRole('button', { name: 'users.manage_user.alice.merveille' }), + denisUpdateButton: byRole('button', { name: 'users.manage_user.denis.villeneuve' }), alicedDeactivateButton: byRole('button', { name: 'users.deactivate' }), bobUpdateGroupButton: byRole('button', { name: 'users.update_users_groups.bob.marley' }), bobUpdateButton: byRole('button', { name: 'users.manage_user.bob.marley' }), @@ -421,6 +422,50 @@ describe('in non managed mode', () => { expect(ui.dialogPasswords.query()).not.toBeInTheDocument(); }); + + it('should not allow to update non-local user', async () => { + const user = userEvent.setup(); + const currentUser = mockLoggedInUser({ login: 'denis.villeneuve' }); + renderUsersApp([], currentUser); + + await act(async () => user.click(await ui.denisUpdateButton.find())); + await user.click( + await within(ui.denisRow.get()).findByRole('button', { name: 'update_details' }), + ); + expect(await ui.dialogUpdateUser.find()).toBeInTheDocument(); + + expect(ui.userNameInput.get()).toHaveValue('Denis Villeneuve'); + expect(ui.userNameInput.get()).toBeDisabled(); + expect(ui.emailInput.get()).toBeDisabled(); + await user.click(ui.scmAddButton.get()); + await user.type(ui.dialogSCMInput().get(), 'SCM'); + await act(() => user.click(ui.updateButton.get())); + expect(ui.dialogUpdateUser.query()).not.toBeInTheDocument(); + expect(await within(ui.denisRow.get()).findByText('SCM')).toBeInTheDocument(); + }); + + it('should be able to remove email', async () => { + const user = userEvent.setup(); + const currentUser = mockLoggedInUser({ login: 'alice.merveille' }); + renderUsersApp([], currentUser); + + expect( + within(await ui.aliceRow.find()).getByText('alice.merveille@wonderland.com'), + ).toBeInTheDocument(); + await act(async () => user.click(await ui.aliceUpdateButton.find())); + await user.click( + await within(ui.aliceRow.get()).findByRole('button', { name: 'update_details' }), + ); + expect(await ui.dialogUpdateUser.find()).toBeInTheDocument(); + + expect(ui.emailInput.get()).toHaveValue('alice.merveille@wonderland.com'); + await user.clear(ui.emailInput.get()); + await act(() => user.click(ui.updateButton.get())); + expect(ui.dialogUpdateUser.query()).not.toBeInTheDocument(); + expect( + within(ui.aliceRow.get()).queryByText('alice.merveille@wonderland.com'), + ).not.toBeInTheDocument(); + }); }); describe('in manage mode', () => { diff --git a/server/sonar-web/src/main/js/apps/users/components/UserForm.tsx b/server/sonar-web/src/main/js/apps/users/components/UserForm.tsx index 9a74b5ae745..c722d017cfc 100644 --- a/server/sonar-web/src/main/js/apps/users/components/UserForm.tsx +++ b/server/sonar-web/src/main/js/apps/users/components/UserForm.tsx @@ -17,15 +17,16 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +import { AxiosError, AxiosResponse } from 'axios'; import * as React from 'react'; import SimpleModal from '../../../components/controls/SimpleModal'; import { Button, ResetButtonLink, SubmitButton } from '../../../components/controls/buttons'; import { Alert } from '../../../components/ui/Alert'; import MandatoryFieldMarker from '../../../components/ui/MandatoryFieldMarker'; import MandatoryFieldsExplanation from '../../../components/ui/MandatoryFieldsExplanation'; -import { throwGlobalError } from '../../../helpers/error'; +import { addGlobalErrorMessage } from '../../../helpers/globalMessages'; import { translate, translateWithParameters } from '../../../helpers/l10n'; -import { parseError } from '../../../helpers/request'; +import { parseErrorResponse } from '../../../helpers/request'; import { usePostUserMutation, useUpdateUserMutation } from '../../../queries/users'; import { RestUserDetailed } from '../../../types/users'; import UserScmAccountInput from './UserScmAccountInput'; @@ -52,11 +53,14 @@ export default function UserForm(props: Props) { const [scmAccounts, setScmAccounts] = React.useState(user?.scmAccounts ?? []); const [error, setError] = React.useState(undefined); - const handleError = (response: Response) => { - if (![BAD_REQUEST, INTERNAL_SERVER_ERROR].includes(response.status)) { - throwGlobalError(response); + const handleError = (error: AxiosError) => { + const { response } = error; + const message = parseErrorResponse(response); + + if (!response || ![BAD_REQUEST, INTERNAL_SERVER_ERROR].includes(response.status)) { + addGlobalErrorMessage(message); } else { - parseError(response).then((errorMsg) => setError(errorMsg), throwGlobalError); + setError(message); } }; @@ -77,14 +81,17 @@ export default function UserForm(props: Props) { const { user } = props; updateUser( - isInstanceManaged - ? { scmAccount: scmAccounts, login } - : { - email: user?.local ? email : undefined, - login, - name: user?.local ? name : undefined, - scmAccount: scmAccounts, - }, + { + id: login, + data: + isInstanceManaged || !user?.local + ? { scmAccounts } + : { + email: email !== '' ? email : null, + name, + scmAccounts, + }, + }, { onSuccess: props.onClose, onError: handleError }, ); }; diff --git a/server/sonar-web/src/main/js/queries/users.ts b/server/sonar-web/src/main/js/queries/users.ts index 3826de75f26..45ddf02c39b 100644 --- a/server/sonar-web/src/main/js/queries/users.ts +++ b/server/sonar-web/src/main/js/queries/users.ts @@ -94,7 +94,13 @@ export function useUpdateUserMutation() { const queryClient = useQueryClient(); return useMutation({ - mutationFn: (data: Parameters[0]) => updateUser(data), + mutationFn: ({ + id, + data, + }: { + id: Parameters[0]; + data: Parameters[1]; + }) => updateUser(id, data), onSuccess() { queryClient.invalidateQueries({ queryKey: ['user', 'list'] }); }, diff --git a/server/sonar-web/src/main/js/types/axios.d.ts b/server/sonar-web/src/main/js/types/axios.d.ts new file mode 100644 index 00000000000..c9b67a26671 --- /dev/null +++ b/server/sonar-web/src/main/js/types/axios.d.ts @@ -0,0 +1,36 @@ +/* + * SonarQube + * Copyright (C) 2009-2023 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +import 'axios'; + +declare module 'axios' { + export interface AxiosInstance { + get(url: string, config?: AxiosRequestConfig): Promise; + delete(url: string, config?: AxiosRequestConfig): Promise; + post(url: string, data?: D, config?: AxiosRequestConfig): Promise; + patch(url: string, data?: D, config?: AxiosRequestConfig): Promise; + + defaults: Omit & { + headers: HeadersDefaults & { + [key: string]: AxiosHeaderValue; + }; + }; + } +} diff --git a/server/sonar-web/src/main/js/types/users.ts b/server/sonar-web/src/main/js/types/users.ts index a556d6c1a7c..9ee0ffdf389 100644 --- a/server/sonar-web/src/main/js/types/users.ts +++ b/server/sonar-web/src/main/js/types/users.ts @@ -87,7 +87,7 @@ export interface User extends UserBase { export interface UserBase { active?: boolean; avatar?: string; - email?: string; + email?: string | null; login: string; name?: string; } @@ -99,7 +99,7 @@ export interface RestUserBase { } export interface RestUser extends RestUserBase { - email: string; + email: string | null; active: boolean; local: boolean; externalProvider: string; -- 2.39.5