fix: Changed contact form async validations to onSubmit (#29250)

Co-authored-by: Kevin Aleman <11577696+KevLehman@users.noreply.github.com>
RocketChat · May 26, 2023 · 0c34904 · 0c34904
1 parent 6a474ff
commit 0c34904
Show file tree

Hide file tree

Showing 4 changed files with 92 additions and 58 deletions.
diff --git a/.changeset/great-dolphins-remember.md b/.changeset/great-dolphins-remember.md
@@ -0,0 +1,5 @@
+---
+'@rocket.chat/meteor': patch
+---
+
+Fixed omnichannel contact form asynchronous validations
diff --git a/apps/meteor/app/livechat/server/lib/Contacts.ts b/apps/meteor/app/livechat/server/lib/Contacts.ts
@@ -103,16 +103,24 @@ export const Contacts = {
 			livechatData[cf._id] = cfValue;
 		}
 
+		const fieldsToRemove = {
+			// if field is explicitely set to empty string, remove
+			...(phone === '' && { phone: 1 }),
+			...(visitorEmail === '' && { visitorEmails: 1 }),
+			...(!contactManager?.username && { contactManager: 1 }),
+		};
+
 		const updateUser: { $set: MatchKeysAndValues<ILivechatVisitor>; $unset?: OnlyFieldsOfType<ILivechatVisitor> } = {
 			$set: {
 				token,
 				name,
 				livechatData,
+				// if phone has some value, set
 				...(phone && { phone: [{ phoneNumber: phone }] }),
 				...(visitorEmail && { visitorEmails: [{ address: visitorEmail }] }),
 				...(contactManager?.username && { contactManager: { username: contactManager.username } }),
 			},
-			...(!contactManager?.username && { $unset: { contactManager: 1 } }),
+			...(Object.keys(fieldsToRemove).length && { $unset: fieldsToRemove }),
 		};
 
 		await LivechatVisitors.updateOne({ _id: contactId }, updateUser);

diff --git a/apps/meteor/client/views/omnichannel/directory/contacts/contextualBar/ContactNewEdit.tsx b/apps/meteor/client/views/omnichannel/directory/contacts/contextualBar/ContactNewEdit.tsx
@@ -7,7 +7,6 @@ import { useForm } from 'react-hook-form';
 
 import { hasAtLeastOnePermission } from '../../../../../../app/authorization/client';
 import { validateEmail } from '../../../../../../lib/emailValidator';
-import { withDebouncing } from '../../../../../../lib/utils/highOrderFunctions';
 import { ContextualbarScrollableContent, ContextualbarFooter } from '../../../../../components/Contextualbar';
 import { CustomFieldsForm } from '../../../../../components/CustomFieldsFormV2';
 import { createToken } from '../../../../../lib/utils/createToken';
@@ -82,19 +81,17 @@ const ContactNewEdit = ({ id, data, close }: ContactNewEditProps): ReactElement
 
 	const {
 		register,
-		formState: { errors, isValid: isFormValid, isDirty },
+		formState: { errors, isValid, isDirty },
 		control,
 		setValue,
 		handleSubmit,
-		trigger,
+		setError,
 	} = useForm<ContactFormData>({
-		mode: 'onSubmit',
-		reValidateMode: 'onSubmit',
+		mode: 'onChange',
+		reValidateMode: 'onChange',
 		defaultValues: initialValue,
 	});
 
-	const isValid = isDirty && isFormValid;
-
 	useEffect(() => {
 		if (!initialUsername) {
 			return;
@@ -105,29 +102,29 @@ const ContactNewEdit = ({ id, data, close }: ContactNewEditProps): ReactElement
 		});
 	}, [getUserData, initialUsername]);
 
-	const isEmailValid = async (email: string): Promise<boolean | string> => {
-		if (email === initialValue.email) {
+	const validateEmailFormat = (email: string): boolean | string => {
+		if (!email || email === initialValue.email) {
 			return true;
 		}
 
 		if (!validateEmail(email)) {
 			return t('error-invalid-email-address');
 		}
 
-		const { contact } = await getContactBy({ email });
-		return !contact || contact._id === id || t('Email_already_exists');
+		return true;
 	};
 
-	const isPhoneValid = async (phone: string): Promise<boolean | string> => {
-		if (!phone || initialValue.phone === phone) {
+	const validateContactField = async (name: 'phone' | 'email', value: string, optional = true) => {
+		if ((optional && !value) || value === initialValue[name]) {
 			return true;
 		}
 
-		const { contact } = await getContactBy({ phone });
-		return !contact || contact._id === id || t('Phone_already_exists');
+		const query = { [name]: value } as Record<'phone' | 'email', string>;
+		const { contact } = await getContactBy(query);
+		return !contact || contact._id === id;
 	};
 
-	const isNameValid = (v: string): string | boolean => (!v.trim() ? t('The_field_is_required', t('Name')) : true);
+	const validateName = (v: string): string | boolean => (!v.trim() ? t('The_field_is_required', t('Name')) : true);
 
 	const handleContactManagerChange = async (userId: string): Promise<void> => {
 		setUserId(userId);
@@ -141,9 +138,21 @@ const ContactNewEdit = ({ id, data, close }: ContactNewEditProps): ReactElement
 		setValue('username', user.username || '', { shouldDirty: true });
 	};
 
-	const validate = (fieldName: keyof ContactFormData): (() => void) => withDebouncing({ wait: 500 })(() => trigger(fieldName));
+	const validateAsync = async ({ phone = '', email = '' } = {}) => {
+		const isEmailValid = await validateContactField('email', email);
+		const isPhoneValid = await validateContactField('phone', phone);
+
+		!isEmailValid && setError('email', { message: t('Email_already_exists') });
+		!isPhoneValid && setError('phone', { message: t('Phone_already_exists') });
+
+		return isEmailValid && isPhoneValid;
+	};
 
 	const handleSave = async (data: ContactFormData): Promise<void> => {
+		if (!(await validateAsync(data))) {
+			return;
+		}
+
 		const { name, phone, email, customFields, username, token } = data;
 
 		const payload = {
@@ -175,29 +184,21 @@ const ContactNewEdit = ({ id, data, close }: ContactNewEditProps): ReactElement
 				<Field>
 					<Field.Label>{t('Name')}*</Field.Label>
 					<Field.Row>
-						<TextInput {...register('name', { validate: isNameValid })} error={errors.name?.message} flexGrow={1} />
+						<TextInput {...register('name', { validate: validateName })} error={errors.name?.message} flexGrow={1} />
 					</Field.Row>
 					<Field.Error>{errors.name?.message}</Field.Error>
 				</Field>
 				<Field>
 					<Field.Label>{t('Email')}</Field.Label>
 					<Field.Row>
-						<TextInput
-							{...register('email', { validate: isEmailValid, onChange: validate('email') })}
-							error={errors.email?.message}
-							flexGrow={1}
-						/>
+						<TextInput {...register('email', { validate: validateEmailFormat })} error={errors.email?.message} flexGrow={1} />
 					</Field.Row>
 					<Field.Error>{errors.email?.message}</Field.Error>
 				</Field>
 				<Field>
 					<Field.Label>{t('Phone')}</Field.Label>
 					<Field.Row>
-						<TextInput
-							{...register('phone', { validate: isPhoneValid, onChange: validate('phone') })}
-							error={errors.phone?.message}
-							flexGrow={1}
-						/>
+						<TextInput {...register('phone')} error={errors.phone?.message} flexGrow={1} />
 					</Field.Row>
 					<Field.Error>{errors.phone?.message}</Field.Error>
 				</Field>
@@ -209,7 +210,7 @@ const ContactNewEdit = ({ id, data, close }: ContactNewEditProps): ReactElement
 					<Button flexGrow={1} onClick={close}>
 						{t('Cancel')}
 					</Button>
-					<Button mie='none' type='submit' onClick={handleSubmit(handleSave)} flexGrow={1} disabled={!isValid} primary>
+					<Button mie='none' type='submit' onClick={handleSubmit(handleSave)} flexGrow={1} disabled={!isValid || !isDirty} primary>
 						{t('Save')}
 					</Button>
 				</ButtonGroup>

diff --git a/apps/meteor/tests/e2e/omnichannel-contact-center.spec.ts b/apps/meteor/tests/e2e/omnichannel-contact-center.spec.ts
@@ -102,22 +102,26 @@ test.describe('Omnichannel Contact Center', () => {
 			await expect(poContacts.newContact.errorMessage(ERROR.invalidEmail)).toBeVisible();
 		});
 
-		await test.step('validate existing email', async () => {
+		await test.step('input existing email', async () => {
 			await poContacts.newContact.inputEmail.selectText();
 			await poContacts.newContact.inputEmail.type(EXISTING_CONTACT.email);
-			await expect(poContacts.newContact.errorMessage(ERROR.existingEmail)).toBeVisible();
-			await expect(poContacts.newContact.btnSave).toBeDisabled();
-		});
-
-		await test.step('input email', async () => {
-			await poContacts.newContact.inputEmail.selectText();
-			await poContacts.newContact.inputEmail.type(NEW_CONTACT.email);
 			await expect(poContacts.newContact.errorMessage(ERROR.invalidEmail)).not.toBeVisible();
 			await expect(poContacts.newContact.errorMessage(ERROR.existingEmail)).not.toBeVisible();
 		});
 
-		await test.step('validate existing phone ', async () => {
+		await test.step('input existing phone ', async () => {
+			await poContacts.newContact.inputPhone.selectText();
 			await poContacts.newContact.inputPhone.type(EXISTING_CONTACT.phone);
+			await expect(poContacts.newContact.errorMessage(ERROR.existingPhone)).not.toBeVisible();
+		});
+
+		await test.step('run async validations ', async () => {
+			await expect(poContacts.newContact.btnSave).toBeEnabled();
+			await poContacts.newContact.btnSave.click();
+
+			await expect(poContacts.newContact.errorMessage(ERROR.existingEmail)).toBeVisible();
+			await expect(poContacts.newContact.btnSave).toBeDisabled();
+
 			await expect(poContacts.newContact.errorMessage(ERROR.existingPhone)).toBeVisible();
 			await expect(poContacts.newContact.btnSave).toBeDisabled();
 		});
@@ -128,6 +132,13 @@ test.describe('Omnichannel Contact Center', () => {
 			await expect(poContacts.newContact.errorMessage(ERROR.existingPhone)).not.toBeVisible();
 		});
 
+		await test.step('input email', async () => {
+			await poContacts.newContact.inputEmail.selectText();
+			await poContacts.newContact.inputEmail.type(NEW_CONTACT.email);
+			await expect(poContacts.newContact.errorMessage(ERROR.invalidEmail)).not.toBeVisible();
+			await expect(poContacts.newContact.errorMessage(ERROR.existingEmail)).not.toBeVisible();
+		});
+
 		await test.step('save new contact ', async () => {
 			await expect(poContacts.newContact.btnSave).toBeEnabled();
 			await poContacts.newContact.btnSave.click();
@@ -172,49 +183,58 @@ test.describe('Omnichannel Contact Center', () => {
 			await expect(poContacts.contactInfo.errorMessage(ERROR.invalidEmail)).toBeVisible();
 		});
 
-		await test.step('validate existing email', async () => {
+		await test.step('input existing email', async () => {
 			await poContacts.contactInfo.inputEmail.selectText();
 			await poContacts.contactInfo.inputEmail.type(EXISTING_CONTACT.email);
-			await expect(poContacts.contactInfo.errorMessage(ERROR.existingEmail)).toBeVisible();
-			await expect(poContacts.contactInfo.btnSave).toBeDisabled();
-		});
-
-		await test.step('input email', async () => {
-			await poContacts.contactInfo.inputEmail.selectText();
-			await poContacts.contactInfo.inputEmail.type(EDIT_CONTACT.email);
 			await expect(poContacts.contactInfo.errorMessage(ERROR.invalidEmail)).not.toBeVisible();
 			await expect(poContacts.contactInfo.errorMessage(ERROR.existingEmail)).not.toBeVisible();
 			await expect(poContacts.contactInfo.btnSave).toBeEnabled();
 		});
 
-		await test.step('validate existing phone ', async () => {
+		await test.step('input existing phone ', async () => {
 			await poContacts.contactInfo.inputPhone.selectText();
 			await poContacts.contactInfo.inputPhone.type(EXISTING_CONTACT.phone);
-			await expect(poContacts.contactInfo.errorMessage(ERROR.existingPhone)).toBeVisible();
-			await expect(poContacts.contactInfo.btnSave).toBeDisabled();
-		});
-
-		await test.step('input phone ', async () => {
-			await poContacts.contactInfo.inputPhone.selectText();
-			await poContacts.contactInfo.inputPhone.type(EDIT_CONTACT.phone);
 			await expect(poContacts.contactInfo.errorMessage(ERROR.existingPhone)).not.toBeVisible();
 			await expect(poContacts.contactInfo.btnSave).toBeEnabled();
 		});
 
 		await test.step('validate name is required', async () => {
 			await poContacts.contactInfo.inputName.selectText();
 			await poContacts.contactInfo.inputName.type(' ');
-
-			await expect(poContacts.contactInfo.btnSave).toBeEnabled();
-			await poContacts.contactInfo.btnSave.click();
 			await expect(poContacts.contactInfo.errorMessage(ERROR.nameRequired)).toBeVisible();
+
+			await expect(poContacts.contactInfo.btnSave).not.toBeEnabled();
 		});
 
 		await test.step('edit name', async () => {
 			await poContacts.contactInfo.inputName.selectText();
 			await poContacts.contactInfo.inputName.type(EDIT_CONTACT.name);
 		});
 
+		await test.step('run async validations ', async () => {
+			await expect(poContacts.newContact.btnSave).toBeEnabled();
+			await poContacts.newContact.btnSave.click();
+
+			await expect(poContacts.newContact.errorMessage(ERROR.existingEmail)).toBeVisible();
+			await expect(poContacts.newContact.btnSave).toBeDisabled();
+
+			await expect(poContacts.newContact.errorMessage(ERROR.existingPhone)).toBeVisible();
+			await expect(poContacts.newContact.btnSave).toBeDisabled();
+		});
+
+		await test.step('input phone ', async () => {
+			await poContacts.newContact.inputPhone.selectText();
+			await poContacts.newContact.inputPhone.type(EDIT_CONTACT.phone);
+			await expect(poContacts.newContact.errorMessage(ERROR.existingPhone)).not.toBeVisible();
+		});
+
+		await test.step('input email', async () => {
+			await poContacts.newContact.inputEmail.selectText();
+			await poContacts.newContact.inputEmail.type(EDIT_CONTACT.email);
+			await expect(poContacts.newContact.errorMessage(ERROR.invalidEmail)).not.toBeVisible();
+			await expect(poContacts.newContact.errorMessage(ERROR.existingEmail)).not.toBeVisible();
+		});
+
 		await test.step('save new contact ', async () => {
 			await poContacts.contactInfo.btnSave.click();
 			await expect(poContacts.toastSuccess).toBeVisible();