From d898b2ab1a23df610156159b6c29fd51567e3f70 Mon Sep 17 00:00:00 2001 From: "kasia.oczkowska" Date: Fri, 13 Dec 2024 13:52:22 +0000 Subject: [PATCH 1/3] feat: introduce inline error messages in EditUser form --- packages/web/src/helpers/errors.js | 18 ++ packages/web/src/hooks/useAdminUpdateUser.js | 13 -- packages/web/src/locales/en.json | 2 + packages/web/src/pages/EditUser/index.jsx | 206 ++++++++++++++----- 4 files changed, 169 insertions(+), 70 deletions(-) create mode 100644 packages/web/src/helpers/errors.js diff --git a/packages/web/src/helpers/errors.js b/packages/web/src/helpers/errors.js new file mode 100644 index 00000000..dc73867d --- /dev/null +++ b/packages/web/src/helpers/errors.js @@ -0,0 +1,18 @@ +// Helpers to extract errors received from the API + +export const getGeneralErrorMessage = ({ error, fallbackMessage }) => { + if (!error) { + return; + } + + const errors = error?.response?.data?.errors; + const generalError = errors?.general; + + if (generalError && Array.isArray(generalError)) { + return generalError.join(' '); + } + + if (!errors) { + return error?.message || fallbackMessage; + } +}; diff --git a/packages/web/src/hooks/useAdminUpdateUser.js b/packages/web/src/hooks/useAdminUpdateUser.js index d971f252..21caa801 100644 --- a/packages/web/src/hooks/useAdminUpdateUser.js +++ b/packages/web/src/hooks/useAdminUpdateUser.js @@ -1,12 +1,8 @@ import { useMutation, useQueryClient } from '@tanstack/react-query'; import api from 'helpers/api'; -import useEnqueueSnackbar from 'hooks/useEnqueueSnackbar'; -import useFormatMessage from 'hooks/useFormatMessage'; export default function useAdminUpdateUser(userId) { const queryClient = useQueryClient(); - const enqueueSnackbar = useEnqueueSnackbar(); - const formatMessage = useFormatMessage(); const query = useMutation({ mutationFn: async (payload) => { @@ -19,15 +15,6 @@ export default function useAdminUpdateUser(userId) { queryKey: ['admin', 'users'], }); }, - onError: () => { - enqueueSnackbar(formatMessage('editUser.error'), { - variant: 'error', - persist: true, - SnackbarProps: { - 'data-test': 'snackbar-error', - }, - }); - }, }); return query; diff --git a/packages/web/src/locales/en.json b/packages/web/src/locales/en.json index c9422557..699bec75 100644 --- a/packages/web/src/locales/en.json +++ b/packages/web/src/locales/en.json @@ -226,6 +226,8 @@ "userForm.email": "Email", "userForm.role": "Role", "userForm.password": "Password", + "userForm.mandatoryInput": "{inputName} is required.", + "userForm.validateEmail": "Email must be valid.", "createUser.submit": "Create", "createUser.successfullyCreated": "The user has been created.", "createUser.invitationEmailInfo": "Invitation email will be sent if SMTP credentials are valid. Otherwise, you can share the invitation link manually: ", diff --git a/packages/web/src/pages/EditUser/index.jsx b/packages/web/src/pages/EditUser/index.jsx index becbb854..8bba455e 100644 --- a/packages/web/src/pages/EditUser/index.jsx +++ b/packages/web/src/pages/EditUser/index.jsx @@ -5,9 +5,12 @@ import Stack from '@mui/material/Stack'; import Chip from '@mui/material/Chip'; import Typography from '@mui/material/Typography'; import MuiTextField from '@mui/material/TextField'; +import Alert from '@mui/material/Alert'; import useEnqueueSnackbar from 'hooks/useEnqueueSnackbar'; import * as React from 'react'; import { useNavigate, useParams } from 'react-router-dom'; +import { yupResolver } from '@hookform/resolvers/yup'; +import * as yup from 'yup'; import Can from 'components/Can'; import Container from 'components/Container'; @@ -20,11 +23,45 @@ import useFormatMessage from 'hooks/useFormatMessage'; import useRoles from 'hooks/useRoles.ee'; import useAdminUpdateUser from 'hooks/useAdminUpdateUser'; import useAdminUser from 'hooks/useAdminUser'; +import useCurrentUserAbility from 'hooks/useCurrentUserAbility'; +import { getGeneralErrorMessage } from 'helpers/errors'; function generateRoleOptions(roles) { return roles?.map(({ name: label, id: value }) => ({ label, value })); } +const getValidationSchema = (formatMessage, canUpdateRole) => { + const getMandatoryFieldMessage = (fieldTranslationId) => + formatMessage('userForm.mandatoryInput', { + inputName: formatMessage(fieldTranslationId), + }); + + return yup.object().shape({ + fullName: yup + .string() + .trim() + .required(getMandatoryFieldMessage('userForm.fullName')), + email: yup + .string() + .trim() + .email(formatMessage('userForm.validateEmail')) + .required(getMandatoryFieldMessage('userForm.email')), + ...(canUpdateRole + ? { + roleId: yup + .string() + .required(getMandatoryFieldMessage('userForm.role')), + } + : {}), + }); +}; + +const defaultValues = { + fullName: '', + email: '', + roleId: '', +}; + export default function EditUser() { const formatMessage = useFormatMessage(); const { userId } = useParams(); @@ -36,13 +73,15 @@ export default function EditUser() { const roles = data?.data; const enqueueSnackbar = useEnqueueSnackbar(); const navigate = useNavigate(); + const currentUserAbility = useCurrentUserAbility(); + const canUpdateRole = currentUserAbility.can('update', 'Role'); - const handleUserUpdate = async (userDataToUpdate) => { + const handleUserUpdate = async (userDataToUpdate, e, setError) => { try { await updateUser({ fullName: userDataToUpdate.fullName, email: userDataToUpdate.email, - roleId: userDataToUpdate.role?.id, + roleId: userDataToUpdate.roleId, }); enqueueSnackbar(formatMessage('editUser.successfullyUpdated'), { @@ -55,7 +94,31 @@ export default function EditUser() { navigate(URLS.USERS); } catch (error) { - throw new Error('Failed while updating!'); + const errors = error?.response?.data?.errors; + + if (errors) { + const fieldNames = Object.keys(defaultValues); + Object.entries(errors).forEach(([fieldName, fieldErrors]) => { + if (fieldNames.includes(fieldName) && Array.isArray(fieldErrors)) { + setError(fieldName, { + type: 'fieldRequestError', + message: fieldErrors.join(', '), + }); + } + }); + } + + const generalError = getGeneralErrorMessage({ + error, + fallbackMessage: formatMessage('editUser.error'), + }); + + if (generalError) { + setError('root.general', { + type: 'requestError', + message: generalError, + }); + } } }; @@ -80,65 +143,94 @@ export default function EditUser() { )} {!isUserLoading && ( -
- - - - {formatMessage('editUser.status')} - + ( + + + + {formatMessage('editUser.status')} + - - + + - - - - - - ( - - )} - loading={isRolesLoading} + error={!!errors?.fullName} + helperText={errors?.fullName?.message} /> - - - {formatMessage('editUser.submit')} - - - + + + + ( + + )} + loading={isRolesLoading} + showHelperText={false} + /> + + + {errors?.root?.general && ( + + {errors?.root?.general?.message} + + )} + + + {formatMessage('editUser.submit')} + +
+ )} + /> )} From 71e8928812d94e24dc36f03617f1b4facdaf2a37 Mon Sep 17 00:00:00 2001 From: "Jakub P." Date: Wed, 18 Dec 2024 20:54:00 +0100 Subject: [PATCH 2/3] test: check for field error instead of snackbar when editing user fails --- .../fixtures/admin/create-user-page.js | 8 +- .../fixtures/admin/edit-user-page.js | 3 +- .../tests/admin/manage-users.spec.js | 460 ++++++++---------- 3 files changed, 219 insertions(+), 252 deletions(-) diff --git a/packages/e2e-tests/fixtures/admin/create-user-page.js b/packages/e2e-tests/fixtures/admin/create-user-page.js index 135b38fb..e59ba2c5 100644 --- a/packages/e2e-tests/fixtures/admin/create-user-page.js +++ b/packages/e2e-tests/fixtures/admin/create-user-page.js @@ -14,8 +14,12 @@ export class AdminCreateUserPage extends AuthenticatedPage { this.roleInput = page.getByTestId('role.id-autocomplete'); this.createButton = page.getByTestId('create-button'); this.pageTitle = page.getByTestId('create-user-title'); - this.invitationEmailInfoAlert = page.getByTestId('invitation-email-info-alert'); - this.acceptInvitationLink = page.getByTestId('invitation-email-info-alert').getByRole('link'); + this.invitationEmailInfoAlert = page.getByTestId( + 'invitation-email-info-alert' + ); + this.acceptInvitationLink = page + .getByTestId('invitation-email-info-alert') + .getByRole('link'); } seed(seed) { diff --git a/packages/e2e-tests/fixtures/admin/edit-user-page.js b/packages/e2e-tests/fixtures/admin/edit-user-page.js index 4bc3a1b6..d0ee043f 100644 --- a/packages/e2e-tests/fixtures/admin/edit-user-page.js +++ b/packages/e2e-tests/fixtures/admin/edit-user-page.js @@ -13,9 +13,10 @@ export class AdminEditUserPage extends AuthenticatedPage { super(page); this.fullNameInput = page.getByTestId('full-name-input'); this.emailInput = page.getByTestId('email-input'); - this.roleInput = page.getByTestId('role.id-autocomplete'); + this.roleInput = page.getByTestId('roleId-autocomplete'); this.updateButton = page.getByTestId('update-button'); this.pageTitle = page.getByTestId('edit-user-title'); + this.fieldError = page.locator('p[id$="-helper-text"]'); } /** diff --git a/packages/e2e-tests/tests/admin/manage-users.spec.js b/packages/e2e-tests/tests/admin/manage-users.spec.js index d6fc1507..35b7490b 100644 --- a/packages/e2e-tests/tests/admin/manage-users.spec.js +++ b/packages/e2e-tests/tests/admin/manage-users.spec.js @@ -5,281 +5,243 @@ const { test, expect } = require('../../fixtures/index'); * otherwise tests will fail since users are only *soft*-deleted */ test.describe('User management page', () => { - test.beforeEach(async ({ adminUsersPage }) => { await adminUsersPage.navigateTo(); await adminUsersPage.closeSnackbar(); }); - test( - 'User creation and deletion process', - async ({ adminCreateUserPage, adminEditUserPage, adminUsersPage }) => { - adminCreateUserPage.seed(9000); - const user = adminCreateUserPage.generateUser(); - await adminUsersPage.usersLoader.waitFor({ - state: 'detached' /* Note: state: 'visible' introduces flakiness + test('User creation and deletion process', async ({ + adminCreateUserPage, + adminEditUserPage, + adminUsersPage, + }) => { + adminCreateUserPage.seed(9000); + const user = adminCreateUserPage.generateUser(); + await adminUsersPage.usersLoader.waitFor({ + state: 'detached' /* Note: state: 'visible' introduces flakiness because visibility: hidden is used as part of the state transition in notistack, see https://github.com/iamhosseindhv/notistack/blob/122f47057eb7ce5a1abfe923316cf8475303e99a/src/transitions/Collapse/Collapse.tsx#L110 - */ + */, + }); + await test.step('Create a user', async () => { + await adminUsersPage.createUserButton.click(); + await adminCreateUserPage.fullNameInput.fill(user.fullName); + await adminCreateUserPage.emailInput.fill(user.email); + await adminCreateUserPage.roleInput.click(); + await adminCreateUserPage.page + .getByRole('option', { name: 'Admin' }) + .click(); + await adminCreateUserPage.createButton.click(); + await adminCreateUserPage.invitationEmailInfoAlert.waitFor({ + state: 'attached', }); - await test.step( - 'Create a user', - async () => { - await adminUsersPage.createUserButton.click(); - await adminCreateUserPage.fullNameInput.fill(user.fullName); - await adminCreateUserPage.emailInput.fill(user.email); - await adminCreateUserPage.roleInput.click(); - await adminCreateUserPage.page.getByRole( - 'option', { name: 'Admin' } - ).click(); - await adminCreateUserPage.createButton.click(); - await adminCreateUserPage.invitationEmailInfoAlert.waitFor({ - state: 'attached' - }); - const snackbar = await adminUsersPage.getSnackbarData( - 'snackbar-create-user-success' - ); - await expect(snackbar.variant).toBe('success'); - await adminUsersPage.navigateTo(); - await adminUsersPage.closeSnackbar(); - } + const snackbar = await adminUsersPage.getSnackbarData( + 'snackbar-create-user-success' ); - await test.step( - 'Check the user exists with the expected properties', - async () => { - await adminUsersPage.findUserPageWithEmail(user.email); - const userRow = await adminUsersPage.getUserRowByEmail(user.email); - const data = await adminUsersPage.getRowData(userRow); - await expect(data.email).toBe(user.email); - await expect(data.fullName).toBe(user.fullName); - await expect(data.role).toBe('Admin'); - } + await expect(snackbar.variant).toBe('success'); + await adminUsersPage.navigateTo(); + await adminUsersPage.closeSnackbar(); + }); + await test.step('Check the user exists with the expected properties', async () => { + await adminUsersPage.findUserPageWithEmail(user.email); + const userRow = await adminUsersPage.getUserRowByEmail(user.email); + const data = await adminUsersPage.getRowData(userRow); + await expect(data.email).toBe(user.email); + await expect(data.fullName).toBe(user.fullName); + await expect(data.role).toBe('Admin'); + }); + await test.step('Edit user info and make sure the edit works correctly', async () => { + await adminUsersPage.findUserPageWithEmail(user.email); + + let userRow = await adminUsersPage.getUserRowByEmail(user.email); + await adminUsersPage.clickEditUser(userRow); + await adminEditUserPage.waitForLoad(user.fullName); + const newUserInfo = adminEditUserPage.generateUser(); + await adminEditUserPage.fullNameInput.fill(newUserInfo.fullName); + await adminEditUserPage.updateButton.click(); + + const snackbar = await adminUsersPage.getSnackbarData( + 'snackbar-edit-user-success' ); - await test.step( - 'Edit user info and make sure the edit works correctly', - async () => { - await adminUsersPage.findUserPageWithEmail(user.email); + await expect(snackbar.variant).toBe('success'); + await adminUsersPage.closeSnackbar(); - let userRow = await adminUsersPage.getUserRowByEmail(user.email); - await adminUsersPage.clickEditUser(userRow); - await adminEditUserPage.waitForLoad(user.fullName); - const newUserInfo = adminEditUserPage.generateUser(); - await adminEditUserPage.fullNameInput.fill(newUserInfo.fullName); - await adminEditUserPage.updateButton.click(); + await adminUsersPage.findUserPageWithEmail(user.email); + userRow = await adminUsersPage.getUserRowByEmail(user.email); + const rowData = await adminUsersPage.getRowData(userRow); + await expect(rowData.fullName).toBe(newUserInfo.fullName); + }); + await test.step('Delete user and check the page confirms this deletion', async () => { + await adminUsersPage.findUserPageWithEmail(user.email); + const userRow = await adminUsersPage.getUserRowByEmail(user.email); + await adminUsersPage.clickDeleteUser(userRow); + const modal = adminUsersPage.deleteUserModal; + await modal.deleteButton.click(); - const snackbar = await adminUsersPage.getSnackbarData( - 'snackbar-edit-user-success' - ); - await expect(snackbar.variant).toBe('success'); - await adminUsersPage.closeSnackbar(); - - await adminUsersPage.findUserPageWithEmail(user.email); - userRow = await adminUsersPage.getUserRowByEmail(user.email); - const rowData = await adminUsersPage.getRowData(userRow); - await expect(rowData.fullName).toBe(newUserInfo.fullName); - } + const snackbar = await adminUsersPage.getSnackbarData( + 'snackbar-delete-user-success' ); - await test.step( - 'Delete user and check the page confirms this deletion', - async () => { - await adminUsersPage.findUserPageWithEmail(user.email); - const userRow = await adminUsersPage.getUserRowByEmail(user.email); - await adminUsersPage.clickDeleteUser(userRow); - const modal = adminUsersPage.deleteUserModal; - await modal.deleteButton.click(); + await expect(snackbar.variant).toBe('success'); + await adminUsersPage.closeSnackbar(); + await expect(userRow).not.toBeVisible(false); + }); + }); - const snackbar = await adminUsersPage.getSnackbarData( - 'snackbar-delete-user-success' - ); - await expect(snackbar.variant).toBe('success'); - await adminUsersPage.closeSnackbar(); - await expect(userRow).not.toBeVisible(false); - } + test('Creating a user which has been deleted', async ({ + adminCreateUserPage, + adminUsersPage, + }) => { + adminCreateUserPage.seed(9100); + const testUser = adminCreateUserPage.generateUser(); + + await test.step('Create the test user', async () => { + await adminUsersPage.navigateTo(); + await adminUsersPage.createUserButton.click(); + await adminCreateUserPage.fullNameInput.fill(testUser.fullName); + await adminCreateUserPage.emailInput.fill(testUser.email); + await adminCreateUserPage.roleInput.click(); + await adminCreateUserPage.page + .getByRole('option', { name: 'Admin' }) + .click(); + await adminCreateUserPage.createButton.click(); + const snackbar = await adminUsersPage.getSnackbarData( + 'snackbar-create-user-success' ); + await expect(snackbar.variant).toBe('success'); + await adminUsersPage.closeSnackbar(); }); - test( - 'Creating a user which has been deleted', - async ({ adminCreateUserPage, adminUsersPage }) => { - adminCreateUserPage.seed(9100); - const testUser = adminCreateUserPage.generateUser(); - - await test.step( - 'Create the test user', - async () => { - await adminUsersPage.navigateTo(); - await adminUsersPage.createUserButton.click(); - await adminCreateUserPage.fullNameInput.fill(testUser.fullName); - await adminCreateUserPage.emailInput.fill(testUser.email); - await adminCreateUserPage.roleInput.click(); - await adminCreateUserPage.page.getByRole( - 'option', { name: 'Admin' } - ).click(); - await adminCreateUserPage.createButton.click(); - const snackbar = await adminUsersPage.getSnackbarData( - 'snackbar-create-user-success' - ); - await expect(snackbar.variant).toBe('success'); - await adminUsersPage.closeSnackbar(); - } + await test.step('Delete the created user', async () => { + await adminUsersPage.navigateTo(); + await adminUsersPage.findUserPageWithEmail(testUser.email); + const userRow = await adminUsersPage.getUserRowByEmail(testUser.email); + await adminUsersPage.clickDeleteUser(userRow); + const modal = adminUsersPage.deleteUserModal; + await modal.deleteButton.click(); + const snackbar = await adminUsersPage.getSnackbarData( + 'snackbar-delete-user-success' ); + await expect(snackbar).not.toBeNull(); + await expect(snackbar.variant).toBe('success'); + await adminUsersPage.closeSnackbar(); + await expect(userRow).not.toBeVisible(false); + }); - await test.step( - 'Delete the created user', - async () => { - await adminUsersPage.navigateTo(); - await adminUsersPage.findUserPageWithEmail(testUser.email); - const userRow = await adminUsersPage.getUserRowByEmail(testUser.email); - await adminUsersPage.clickDeleteUser(userRow); - const modal = adminUsersPage.deleteUserModal; - await modal.deleteButton.click(); - const snackbar = await adminUsersPage.getSnackbarData( - 'snackbar-delete-user-success' - ); - await expect(snackbar).not.toBeNull(); - await expect(snackbar.variant).toBe('success'); - await adminUsersPage.closeSnackbar(); - await expect(userRow).not.toBeVisible(false); - } + await test.step('Create the user again', async () => { + await adminUsersPage.createUserButton.click(); + await adminCreateUserPage.fullNameInput.fill(testUser.fullName); + await adminCreateUserPage.emailInput.fill(testUser.email); + await adminCreateUserPage.roleInput.click(); + await adminCreateUserPage.page + .getByRole('option', { name: 'Admin' }) + .click(); + await adminCreateUserPage.createButton.click(); + const snackbar = await adminUsersPage.getSnackbarData('snackbar-error'); + await expect(snackbar.variant).toBe('error'); + await adminUsersPage.closeSnackbar(); + }); + }); + + test('Creating a user which already exists', async ({ + adminCreateUserPage, + adminUsersPage, + page, + }) => { + adminCreateUserPage.seed(9200); + const testUser = adminCreateUserPage.generateUser(); + + await test.step('Create the test user', async () => { + await adminUsersPage.createUserButton.click(); + await adminCreateUserPage.fullNameInput.fill(testUser.fullName); + await adminCreateUserPage.emailInput.fill(testUser.email); + await adminCreateUserPage.roleInput.click(); + await adminCreateUserPage.page + .getByRole('option', { name: 'Admin' }) + .click(); + await adminCreateUserPage.createButton.click(); + const snackbar = await adminUsersPage.getSnackbarData( + 'snackbar-create-user-success' ); + await expect(snackbar.variant).toBe('success'); + await adminUsersPage.closeSnackbar(); + }); - await test.step( - 'Create the user again', - async () => { - await adminUsersPage.createUserButton.click(); - await adminCreateUserPage.fullNameInput.fill(testUser.fullName); - await adminCreateUserPage.emailInput.fill(testUser.email); - await adminCreateUserPage.roleInput.click(); - await adminCreateUserPage.page.getByRole( - 'option', { name: 'Admin' } - ).click(); - await adminCreateUserPage.createButton.click(); - const snackbar = await adminUsersPage.getSnackbarData('snackbar-error'); - await expect(snackbar.variant).toBe('error'); - await adminUsersPage.closeSnackbar(); - } + await test.step('Create the user again', async () => { + await adminUsersPage.navigateTo(); + await adminUsersPage.createUserButton.click(); + await adminCreateUserPage.fullNameInput.fill(testUser.fullName); + await adminCreateUserPage.emailInput.fill(testUser.email); + const createUserPageUrl = page.url(); + await adminCreateUserPage.roleInput.click(); + await adminCreateUserPage.page + .getByRole('option', { name: 'Admin' }) + .click(); + await adminCreateUserPage.createButton.click(); + + await expect(page.url()).toBe(createUserPageUrl); + const snackbar = await adminUsersPage.getSnackbarData('snackbar-error'); + await expect(snackbar.variant).toBe('error'); + await adminUsersPage.closeSnackbar(); + }); + }); + + test('Editing a user to have the same email as another user should not be allowed', async ({ + adminCreateUserPage, + adminEditUserPage, + adminUsersPage, + page, + }) => { + adminCreateUserPage.seed(9300); + const user1 = adminCreateUserPage.generateUser(); + const user2 = adminCreateUserPage.generateUser(); + await test.step('Create the first user', async () => { + await adminUsersPage.navigateTo(); + await adminUsersPage.createUserButton.click(); + await adminCreateUserPage.fullNameInput.fill(user1.fullName); + await adminCreateUserPage.emailInput.fill(user1.email); + await adminCreateUserPage.roleInput.click(); + await adminCreateUserPage.page + .getByRole('option', { name: 'Admin' }) + .click(); + await adminCreateUserPage.createButton.click(); + const snackbar = await adminUsersPage.getSnackbarData( + 'snackbar-create-user-success' ); - } - ); + await expect(snackbar.variant).toBe('success'); + await adminUsersPage.closeSnackbar(); + }); - test( - 'Creating a user which already exists', - async ({ adminCreateUserPage, adminUsersPage, page }) => { - adminCreateUserPage.seed(9200); - const testUser = adminCreateUserPage.generateUser(); - - await test.step( - 'Create the test user', - async () => { - await adminUsersPage.createUserButton.click(); - await adminCreateUserPage.fullNameInput.fill(testUser.fullName); - await adminCreateUserPage.emailInput.fill(testUser.email); - await adminCreateUserPage.roleInput.click(); - await adminCreateUserPage.page.getByRole( - 'option', { name: 'Admin' } - ).click(); - await adminCreateUserPage.createButton.click(); - const snackbar = await adminUsersPage.getSnackbarData( - 'snackbar-create-user-success' - ); - await expect(snackbar.variant).toBe('success'); - await adminUsersPage.closeSnackbar(); - } + await test.step('Create the second user', async () => { + await adminUsersPage.navigateTo(); + await adminUsersPage.createUserButton.click(); + await adminCreateUserPage.fullNameInput.fill(user2.fullName); + await adminCreateUserPage.emailInput.fill(user2.email); + await adminCreateUserPage.roleInput.click(); + await adminCreateUserPage.page + .getByRole('option', { name: 'Admin' }) + .click(); + await adminCreateUserPage.createButton.click(); + const snackbar = await adminUsersPage.getSnackbarData( + 'snackbar-create-user-success' ); + await expect(snackbar.variant).toBe('success'); + await adminUsersPage.closeSnackbar(); + }); - await test.step( - 'Create the user again', - async () => { - await adminUsersPage.navigateTo(); - await adminUsersPage.createUserButton.click(); - await adminCreateUserPage.fullNameInput.fill(testUser.fullName); - await adminCreateUserPage.emailInput.fill(testUser.email); - const createUserPageUrl = page.url(); - await adminCreateUserPage.roleInput.click(); - await adminCreateUserPage.page.getByRole( - 'option', { name: 'Admin' } - ).click(); - await adminCreateUserPage.createButton.click(); + await test.step('Try editing the second user to have the email of the first user', async () => { + await adminUsersPage.navigateTo(); + await adminUsersPage.findUserPageWithEmail(user2.email); + let userRow = await adminUsersPage.getUserRowByEmail(user2.email); + await adminUsersPage.clickEditUser(userRow); + await adminEditUserPage.waitForLoad(user2.fullName); + await adminEditUserPage.emailInput.fill(user1.email); + const editPageUrl = page.url(); + await adminEditUserPage.updateButton.click(); - await expect(page.url()).toBe(createUserPageUrl); - const snackbar = await adminUsersPage.getSnackbarData('snackbar-error'); - await expect(snackbar.variant).toBe('error'); - await adminUsersPage.closeSnackbar(); - } - ); - } - ); - - test( - 'Editing a user to have the same email as another user should not be allowed', - async ({ - adminCreateUserPage, adminEditUserPage, adminUsersPage, page - }) => { - adminCreateUserPage.seed(9300); - const user1 = adminCreateUserPage.generateUser(); - const user2 = adminCreateUserPage.generateUser(); - await test.step( - 'Create the first user', - async () => { - await adminUsersPage.navigateTo(); - await adminUsersPage.createUserButton.click(); - await adminCreateUserPage.fullNameInput.fill(user1.fullName); - await adminCreateUserPage.emailInput.fill(user1.email); - await adminCreateUserPage.roleInput.click(); - await adminCreateUserPage.page.getByRole( - 'option', { name: 'Admin' } - ).click(); - await adminCreateUserPage.createButton.click(); - const snackbar = await adminUsersPage.getSnackbarData( - 'snackbar-create-user-success' - ); - await expect(snackbar.variant).toBe('success'); - await adminUsersPage.closeSnackbar(); - } - ); - - await test.step( - 'Create the second user', - async () => { - await adminUsersPage.navigateTo(); - await adminUsersPage.createUserButton.click(); - await adminCreateUserPage.fullNameInput.fill(user2.fullName); - await adminCreateUserPage.emailInput.fill(user2.email); - await adminCreateUserPage.roleInput.click(); - await adminCreateUserPage.page.getByRole( - 'option', { name: 'Admin' } - ).click(); - await adminCreateUserPage.createButton.click(); - const snackbar = await adminUsersPage.getSnackbarData( - 'snackbar-create-user-success' - ); - await expect(snackbar.variant).toBe('success'); - await adminUsersPage.closeSnackbar(); - } - ); - - await test.step( - 'Try editing the second user to have the email of the first user', - async () => { - await adminUsersPage.navigateTo(); - await adminUsersPage.findUserPageWithEmail(user2.email); - let userRow = await adminUsersPage.getUserRowByEmail(user2.email); - await adminUsersPage.clickEditUser(userRow); - await adminEditUserPage.waitForLoad(user2.fullName); - await adminEditUserPage.emailInput.fill(user1.email); - const editPageUrl = page.url(); - await adminEditUserPage.updateButton.click(); - - const snackbar = await adminUsersPage.getSnackbarData( - 'snackbar-error' - ); - await expect(snackbar.variant).toBe('error'); - await adminUsersPage.closeSnackbar(); - await expect(page.url()).toBe(editPageUrl); - } - ); - } - ); + await expect(adminEditUserPage.fieldError).toHaveCount(1); + await expect(page.url()).toBe(editPageUrl); + }); + }); }); From dae7c365ef5513d58bc193b1020d3373b6fd5b3e Mon Sep 17 00:00:00 2001 From: "kasia.oczkowska" Date: Fri, 20 Dec 2024 10:00:48 +0000 Subject: [PATCH 3/3] refactor: use form's centralized error management --- packages/web/src/helpers/errors.js | 18 --------------- packages/web/src/locales/en.json | 1 - packages/web/src/pages/EditUser/index.jsx | 27 ++--------------------- 3 files changed, 2 insertions(+), 44 deletions(-) delete mode 100644 packages/web/src/helpers/errors.js diff --git a/packages/web/src/helpers/errors.js b/packages/web/src/helpers/errors.js deleted file mode 100644 index dc73867d..00000000 --- a/packages/web/src/helpers/errors.js +++ /dev/null @@ -1,18 +0,0 @@ -// Helpers to extract errors received from the API - -export const getGeneralErrorMessage = ({ error, fallbackMessage }) => { - if (!error) { - return; - } - - const errors = error?.response?.data?.errors; - const generalError = errors?.general; - - if (generalError && Array.isArray(generalError)) { - return generalError.join(' '); - } - - if (!errors) { - return error?.message || fallbackMessage; - } -}; diff --git a/packages/web/src/locales/en.json b/packages/web/src/locales/en.json index 699bec75..cd6d1121 100644 --- a/packages/web/src/locales/en.json +++ b/packages/web/src/locales/en.json @@ -236,7 +236,6 @@ "editUser.status": "Status", "editUser.submit": "Update", "editUser.successfullyUpdated": "The user has been updated.", - "editUser.error": "Error while updating the user.", "userList.fullName": "Full name", "userList.email": "Email", "userList.role": "Role", diff --git a/packages/web/src/pages/EditUser/index.jsx b/packages/web/src/pages/EditUser/index.jsx index 8bba455e..28713ece 100644 --- a/packages/web/src/pages/EditUser/index.jsx +++ b/packages/web/src/pages/EditUser/index.jsx @@ -24,7 +24,6 @@ import useRoles from 'hooks/useRoles.ee'; import useAdminUpdateUser from 'hooks/useAdminUpdateUser'; import useAdminUser from 'hooks/useAdminUser'; import useCurrentUserAbility from 'hooks/useCurrentUserAbility'; -import { getGeneralErrorMessage } from 'helpers/errors'; function generateRoleOptions(roles) { return roles?.map(({ name: label, id: value }) => ({ label, value })); @@ -76,7 +75,7 @@ export default function EditUser() { const currentUserAbility = useCurrentUserAbility(); const canUpdateRole = currentUserAbility.can('update', 'Role'); - const handleUserUpdate = async (userDataToUpdate, e, setError) => { + const handleUserUpdate = async (userDataToUpdate) => { try { await updateUser({ fullName: userDataToUpdate.fullName, @@ -96,29 +95,7 @@ export default function EditUser() { } catch (error) { const errors = error?.response?.data?.errors; - if (errors) { - const fieldNames = Object.keys(defaultValues); - Object.entries(errors).forEach(([fieldName, fieldErrors]) => { - if (fieldNames.includes(fieldName) && Array.isArray(fieldErrors)) { - setError(fieldName, { - type: 'fieldRequestError', - message: fieldErrors.join(', '), - }); - } - }); - } - - const generalError = getGeneralErrorMessage({ - error, - fallbackMessage: formatMessage('editUser.error'), - }); - - if (generalError) { - setError('root.general', { - type: 'requestError', - message: generalError, - }); - } + throw errors || error; } };