From 21d4ec4ef37f08502f083f77af1fb80ce60581d7 Mon Sep 17 00:00:00 2001
From: Douglas Fabris <deefabris@gmail.com>
Date: Mon, 1 Feb 2021 22:51:36 -0300
Subject: [PATCH] [IMPROVE] Add visual validation on users admin forms (#20308)

Co-authored-by: Gabriel Henriques <gabriel.henriques@rocket.chat>
---
 client/views/admin/users/AddUser.js  | 34 ++++++++++++++++++++++-----
 client/views/admin/users/EditUser.js | 35 ++++++++++++++++++++++------
 client/views/admin/users/UserForm.js | 30 +++++++++++++++++-------
 client/views/admin/users/UserInfo.js |  3 +--
 4 files changed, 78 insertions(+), 24 deletions(-)

diff --git a/client/views/admin/users/AddUser.js b/client/views/admin/users/AddUser.js
index d0299b11f41..4ac0662b3dd 100644
--- a/client/views/admin/users/AddUser.js
+++ b/client/views/admin/users/AddUser.js
@@ -1,5 +1,6 @@
-import React, { useMemo, useCallback } from 'react';
+import React, { useMemo, useCallback, useState } from 'react';
 import { Field, Box, Button } from '@rocket.chat/fuselage';
+import { useMutableCallback } from '@rocket.chat/fuselage-hooks';
 
 import { useTranslation } from '../../../contexts/TranslationContext';
 import { useEndpointData } from '../../../hooks/useEndpointData';
@@ -15,6 +16,18 @@ export function AddUser({ roles, ...props }) {
 	const router = useRoute('admin-users');
 
 	const { value: roleData } = useEndpointData('roles.list', '');
+	const [errors, setErrors] = useState({});
+
+	const validationKeys = {
+		name: (name) => setErrors((errors) => ({ ...errors, name: !name.trim().length ? t('The_field_is_required', t('name')) : undefined })),
+		username: (username) => setErrors((errors) => ({ ...errors, username: !username.trim().length ? t('The_field_is_required', t('username')) : undefined })),
+		email: (email) => setErrors((errors) => ({ ...errors, email: !email.trim().length ? t('The_field_is_required', t('email')) : undefined })),
+		password: (password) => setErrors((errors) => ({ ...errors, password: !password.trim().length ? t('The_field_is_required', t('password')) : undefined })),
+	};
+
+	const validateForm = ({ key, value }) => {
+		validationKeys[key] && validationKeys[key](value);
+	};
 
 	const {
 		values,
@@ -36,21 +49,30 @@ export function AddUser({ roles, ...props }) {
 		sendWelcomeEmail: true,
 		joinDefaultChannels: true,
 		customFields: {},
-	});
+	}, validateForm);
 
 	const goToUser = useCallback((id) => router.push({
 		context: 'info',
 		id,
 	}), [router]);
 
-	const saveAction = useEndpointAction('POST', 'users.create', values, t('User_created_successfully'));
+	const saveAction = useEndpointAction('POST', 'users.create', values, t('User_created_successfully!'));
+
+	const handleSave = useMutableCallback(async () => {
+		Object.entries(values).forEach(([key, value]) => {
+			validateForm({ key, value });
+		});
+
+		const { name, username, password, email } = values;
+		if (name === '' || username === '' || password === '' || email === '') {
+			return false;
+		}
 
-	const handleSave = useCallback(async () => {
 		const result = await saveAction();
 		if (result.success) {
 			goToUser(result.user._id);
 		}
-	}, [goToUser, saveAction]);
+	});
 
 	const availableRoles = useMemo(() => roleData?.roles?.map(({ _id, description }) => [_id, description || _id]) ?? [], [roleData]);
 
@@ -63,5 +85,5 @@ export function AddUser({ roles, ...props }) {
 		</Field.Row>
 	</Field>, [hasUnsavedChanges, reset, t, handleSave]);
 
-	return <UserForm formValues={values} formHandlers={handlers} availableRoles={availableRoles} append={append} {...props}/>;
+	return <UserForm errors={errors} formValues={values} formHandlers={handlers} availableRoles={availableRoles} append={append} {...props}/>;
 }
diff --git a/client/views/admin/users/EditUser.js b/client/views/admin/users/EditUser.js
index 5d97ec5ad51..c4da7d9001c 100644
--- a/client/views/admin/users/EditUser.js
+++ b/client/views/admin/users/EditUser.js
@@ -1,5 +1,6 @@
 import React, { useMemo, useState, useCallback } from 'react';
 import { Box, Field, Margins, Button, Callout } from '@rocket.chat/fuselage';
+import { useMutableCallback } from '@rocket.chat/fuselage-hooks';
 
 import { useTranslation } from '../../../contexts/TranslationContext';
 import { useEndpointAction } from '../../../hooks/useEndpointAction';
@@ -18,7 +19,7 @@ export function EditUserWithData({ uid, ...props }) {
 	const { value: data, phase: state, error } = useEndpointData('users.info', useMemo(() => ({ userId: uid }), [uid]));
 
 	if ([state, roleState].includes(AsyncStatePhase.LOADING)) {
-		return <FormSkeleton/>;
+		return <Box p='x24'><FormSkeleton/></Box>;
 	}
 
 	if (error || roleError) {
@@ -48,8 +49,19 @@ export function EditUser({ data, roles, ...props }) {
 	const t = useTranslation();
 
 	const [avatarObj, setAvatarObj] = useState();
+	const [errors, setErrors] = useState({});
 
-	const { values, handlers, reset, hasUnsavedChanges } = useForm(getInitialValue(data));
+	const validationKeys = {
+		name: (name) => setErrors((errors) => ({ ...errors, name: !name.trim().length ? t('The_field_is_required', t('name')) : undefined })),
+		username: (username) => setErrors((errors) => ({ ...errors, username: !username.trim().length ? t('The_field_is_required', t('username')) : undefined })),
+		email: (email) => setErrors((errors) => ({ ...errors, email: !email.trim().length ? t('The_field_is_required', t('email')) : undefined })),
+	};
+
+	const validateForm = ({ key, value }) => {
+		validationKeys[key] && validationKeys[key](value);
+	};
+
+	const { values, handlers, reset, hasUnsavedChanges } = useForm(getInitialValue(data), validateForm);
 
 	const router = useRoute('admin-users');
 
@@ -88,7 +100,16 @@ export function EditUser({ data, roles, ...props }) {
 		return saveAvatarAction(avatarObj);
 	}, [avatarObj, resetAvatarAction, saveAvatarAction, saveAvatarUrlAction, data._id]);
 
-	const handleSave = useCallback(async () => {
+	const handleSave = useMutableCallback(async () => {
+		Object.entries(values).forEach(([key, value]) => {
+			validationKeys[key] && validationKeys[key](value);
+		});
+
+		const { name, username, email } = values;
+		if (name === '' || username === '' || email === '') {
+			return false;
+		}
+
 		const result = await saveAction();
 		if (result.success) {
 			if (avatarObj) {
@@ -96,7 +117,7 @@ export function EditUser({ data, roles, ...props }) {
 			}
 			goToUser(data._id);
 		}
-	}, [avatarObj, data._id, goToUser, saveAction, updateAvatar]);
+	}, [avatarObj, data._id, goToUser, saveAction, updateAvatar, values, errors, validationKeys]);
 
 	const availableRoles = roles.map(({ _id, description }) => [_id, description || _id]);
 
@@ -109,11 +130,11 @@ export function EditUser({ data, roles, ...props }) {
 			<Box display='flex' flexDirection='row' justifyContent='space-between' w='full'>
 				<Margins inlineEnd='x4'>
 					<Button flexGrow={1} type='reset' disabled={!canSaveOrReset} onClick={reset}>{t('Reset')}</Button>
-					<Button mie='none' flexGrow={1} disabled={!canSaveOrReset || values.email.length === 0} onClick={handleSave}>{t('Save')}</Button>
+					<Button mie='none' flexGrow={1} disabled={!canSaveOrReset} onClick={handleSave}>{t('Save')}</Button>
 				</Margins>
 			</Box>
 		</Field.Row>
-	</Field>, [handleSave, canSaveOrReset, reset, t, values]);
+	</Field>, [handleSave, canSaveOrReset, reset, t]);
 
-	return <UserForm formValues={values} formHandlers={handlers} availableRoles={availableRoles} prepend={prepend} append={append} {...props}/>;
+	return <UserForm errors={errors} formValues={values} formHandlers={handlers} availableRoles={availableRoles} prepend={prepend} append={append} {...props}/>;
 }
diff --git a/client/views/admin/users/UserForm.js b/client/views/admin/users/UserForm.js
index 2bbb32a3e7e..a065c6fc508 100644
--- a/client/views/admin/users/UserForm.js
+++ b/client/views/admin/users/UserForm.js
@@ -6,7 +6,7 @@ import { isEmail } from '../../../../app/utils/lib/isEmail.js';
 import VerticalBar from '../../../components/VerticalBar';
 import CustomFieldsForm from '../../../components/CustomFieldsForm';
 
-export default function UserForm({ formValues, formHandlers, availableRoles, append, prepend, ...props }) {
+export default function UserForm({ formValues, formHandlers, availableRoles, append, prepend, errors, ...props }) {
 	const t = useTranslation();
 	const [hasCustomFields, setHasCustomFields] = useState(false);
 
@@ -52,26 +52,35 @@ export default function UserForm({ formValues, formHandlers, availableRoles, app
 			{useMemo(() => <Field>
 				<Field.Label>{t('Name')}</Field.Label>
 				<Field.Row>
-					<TextInput flexGrow={1} value={name} onChange={handleName}/>
+					<TextInput error={errors && errors.name} flexGrow={1} value={name} onChange={handleName}/>
 				</Field.Row>
-			</Field>, [t, name, handleName])}
+				{errors && errors.name && <Field.Error>
+					{errors.name}
+				</Field.Error>}
+			</Field>, [t, name, handleName, errors])}
 			{useMemo(() => <Field>
 				<Field.Label>{t('Username')}</Field.Label>
 				<Field.Row>
-					<TextInput flexGrow={1} value={username} onChange={handleUsername} addon={<Icon name='at' size='x20'/>}/>
+					<TextInput error={errors && errors.username} flexGrow={1} value={username} onChange={handleUsername} addon={<Icon name='at' size='x20'/>}/>
 				</Field.Row>
-			</Field>, [t, username, handleUsername])}
+				{errors && errors.username && <Field.Error>
+					{errors.username}
+				</Field.Error>}
+			</Field>, [t, username, handleUsername, errors])}
 			{useMemo(() => <Field>
 				<Field.Label>{t('Email')}</Field.Label>
 				<Field.Row>
-					<TextInput flexGrow={1} value={email} error={!isEmail(email) && email.length > 0 ? 'error' : undefined} onChange={handleEmail} addon={<Icon name='mail' size='x20'/>}/>
+					<TextInput error={errors && errors.email} flexGrow={1} value={email} error={!isEmail(email) && email.length > 0 ? 'error' : undefined} onChange={handleEmail} addon={<Icon name='mail' size='x20'/>}/>
 				</Field.Row>
+				{errors && errors.email && <Field.Error>
+					{errors.email}
+				</Field.Error>}
 				<Field.Row>
 					<Box flexGrow={1} display='flex' flexDirection='row' alignItems='center' justifyContent='space-between' mbs='x4'>
 						<Box>{t('Verified')}</Box><ToggleSwitch checked={verified} onChange={handleVerified} />
 					</Box>
 				</Field.Row>
-			</Field>, [t, email, handleEmail, verified, handleVerified])}
+			</Field>, [t, email, handleEmail, verified, handleVerified, errors])}
 			{useMemo(() => <Field>
 				<Field.Label>{t('StatusMessage')}</Field.Label>
 				<Field.Row>
@@ -93,9 +102,12 @@ export default function UserForm({ formValues, formHandlers, availableRoles, app
 			{useMemo(() => <Field>
 				<Field.Label>{t('Password')}</Field.Label>
 				<Field.Row>
-					<PasswordInput autoComplete='off' flexGrow={1} value={password} onChange={handlePassword} addon={<Icon name='key' size='x20'/>}/>
+					<PasswordInput errors={errors && errors.password} autoComplete='off' flexGrow={1} value={password} onChange={handlePassword} addon={<Icon name='key' size='x20'/>}/>
 				</Field.Row>
-			</Field>, [t, password, handlePassword])}
+				{errors && errors.password && <Field.Error>
+					{errors.password}
+				</Field.Error>}
+			</Field>, [t, password, handlePassword, errors])}
 			{useMemo(() => <Field>
 				<Field.Row>
 					<Box flexGrow={1} display='flex' flexDirection='row' alignItems='center' justifyContent='space-between'>
diff --git a/client/views/admin/users/UserInfo.js b/client/views/admin/users/UserInfo.js
index 137c45e3006..5cda5b92be3 100644
--- a/client/views/admin/users/UserInfo.js
+++ b/client/views/admin/users/UserInfo.js
@@ -1,4 +1,3 @@
-
 import React, { useMemo } from 'react';
 import { Box } from '@rocket.chat/fuselage';
 import { useMutableCallback } from '@rocket.chat/fuselage-hooks';
@@ -59,7 +58,7 @@ export function UserInfoWithData({ uid, username, ...props }) {
 	}, [approveManuallyUsers, data, showRealNames]);
 
 	if (state === AsyncStatePhase.LOADING) {
-		return <FormSkeleton/>;
+		return <Box p='x24'><FormSkeleton/></Box>;
 	}
 
 	if (error) {