UI - Improve validation of SMTP settings form (#25051)

## #25009 - Update validation to match pattern defined in `frontend/docs/patterns.md` - Validate email even when not enabling the feature, since we allow setting it - Remove "CONFIGURED" and "NOT CONFIGURED" copy <img width="838" alt="Screenshot 2024-12-30 at 11 27 08 AM" src="https://github.com/user-attachments/assets/42132ea2-3364-412a-bb35-2c35f9f6caea" /> <img width="838" alt="Screenshot 2024-12-30 at 11 27 16 AM" src="https://github.com/user-attachments/assets/f9f3c1c0-a166-4ea0-aaa6-b356e7cf9c69" /> <img width="838" alt="Screenshot 2024-12-30 at 11 27 24 AM" src="https://github.com/user-attachments/assets/8685d01d-b2ae-4bc5-addc-80b326f18863" /> <img width="706" alt="Screenshot 2024-12-30 at 11 44 10 AM" src="https://github.com/user-attachments/assets/af8f0f5f-588f-4226-b7e7-8cf753f4822b" /> - [x] Changes file added for user-visible changes in `changes/` - [x] Manual QA for all new/changed functionality --------- Co-authored-by: Jacob Shandling <jacob@fleetdm.com>
fleetdm · Jan 2, 2025 · 495fddc · 495fddc
1 parent 120f01a
commit 495fddc
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 77 deletions.
diff --git a/changes/25009-smtp-page-validation b/changes/25009-smtp-page-validation
@@ -0,0 +1 @@
+- Improve validation workflow on SMTP settings page
diff --git a/frontend/pages/admin/OrgSettingsPage/_styles.scss b/frontend/pages/admin/OrgSettingsPage/_styles.scss
@@ -58,18 +58,6 @@
           em {
             font-style: normal;
           }
-
-          &--configured {
-            em {
-              color: $ui-success;
-            }
-          }
-
-          &--notconfigured {
-            em {
-              color: $ui-error;
-            }
-          }
         }
       }
 

diff --git a/frontend/pages/admin/OrgSettingsPage/cards/Smtp/Smtp.tsx b/frontend/pages/admin/OrgSettingsPage/cards/Smtp/Smtp.tsx
@@ -1,4 +1,4 @@
-import React, { useState, useEffect, useContext } from "react";
+import React, { useState, useContext } from "react";
 
 import { AppContext } from "context/app";
 
@@ -43,6 +43,54 @@ interface ISmtpConfigFormErrors {
   password?: string | null;
 }
 
+const validateFormData = (newData: ISmtpConfigFormData) => {
+  const errors: ISmtpConfigFormErrors = {};
+
+  const {
+    enableSMTP,
+    smtpSenderAddress,
+    smtpServer,
+    smtpPort,
+    smtpAuthenticationType,
+    smtpUsername,
+    smtpPassword,
+  } = newData;
+
+  if (enableSMTP) {
+    if (!smtpSenderAddress) {
+      errors.sender_address = "SMTP sender address must be present";
+    } else if (!validEmail(smtpSenderAddress)) {
+      errors.sender_address = `${smtpSenderAddress} is not a valid email`;
+    }
+
+    if (!smtpServer) {
+      errors.server = "SMTP server must be present";
+    }
+    if (!smtpPort) {
+      errors.server = "SMTP server port must be present";
+      errors.server_port = "Port";
+    }
+    if (!smtpServer && !smtpPort) {
+      errors.server = "SMTP server and server port must be present";
+      errors.server_port = "Port";
+    }
+    if (smtpAuthenticationType === "authtype_username_password") {
+      if (smtpUsername === "") {
+        errors.user_name = "SMTP username must be present";
+      }
+      if (smtpPassword === "") {
+        errors.password = "SMTP password must be present";
+      }
+    }
+  } else if (smtpSenderAddress && !validEmail(smtpSenderAddress)) {
+    // validations for valid submissions even when smtp not enabled, i.e., updating what will be
+    // used once it IS enabled
+    errors.sender_address = `${smtpSenderAddress} is not a valid email`;
+  }
+
+  return errors;
+};
+
 const baseClass = "app-config-form";
 
 const Smtp = ({
@@ -82,50 +130,35 @@ const Smtp = ({
   const sesConfigured = appConfig.email?.backend === "ses" || false;
 
   const onInputChange = ({ name, value }: IFormField) => {
-    setFormData({ ...formData, [name]: value });
-  };
-
-  const validateForm = () => {
-    const errors: ISmtpConfigFormErrors = {};
-
-    if (enableSMTP) {
-      if (!smtpSenderAddress) {
-        errors.sender_address = "SMTP sender address must be present";
-      } else if (!validEmail(smtpSenderAddress)) {
-        errors.sender_address = `${smtpSenderAddress} is not a valid email`;
-      }
-
-      if (!smtpServer) {
-        errors.server = "SMTP server must be present";
-      }
-      if (!smtpPort) {
-        errors.server = "SMTP server port must be present";
-        errors.server_port = "Port";
+    const newFormData = { ...formData, [name]: value };
+    setFormData(newFormData);
+    const newErrs = validateFormData(newFormData);
+    // only set errors that are updates of existing errors
+    // new errors are only set onBlur or submit
+    const errsToSet: Record<string, string> = {};
+    Object.keys(formErrors).forEach((k) => {
+      // @ts-ignore
+      if (newErrs[k]) {
+        // @ts-ignore
+        errsToSet[k] = newErrs[k];
       }
-      if (!smtpServer && !smtpPort) {
-        errors.server = "SMTP server and server port must be present";
-        errors.server_port = "Port";
-      }
-      if (smtpAuthenticationType === "authtype_username_password") {
-        if (smtpUsername === "") {
-          errors.user_name = "SMTP username must be present";
-        }
-        if (smtpPassword === "") {
-          errors.password = "SMTP password must be present";
-        }
-      }
-    }
-
-    setFormErrors(errors);
+    });
+    setFormErrors(errsToSet);
   };
 
-  useEffect(() => {
-    validateForm();
-  }, [smtpAuthenticationType]);
+  const onInputBlur = () => {
+    setFormErrors(validateFormData(formData));
+  };
 
   const onFormSubmit = (evt: React.MouseEvent<HTMLFormElement>) => {
     evt.preventDefault();
 
+    const errs = validateFormData(formData);
+    if (Object.keys(errs).length > 0) {
+      setFormErrors(errs);
+      return;
+    }
+
     // Formatting of API not UI
     const formDataToSubmit = {
       smtp_settings: {
@@ -157,7 +190,7 @@ const Smtp = ({
           name="smtpUsername"
           value={smtpUsername}
           parseTarget
-          onBlur={validateForm}
+          onBlur={onInputBlur}
           error={formErrors.user_name}
           blockAutoComplete
         />
@@ -168,7 +201,7 @@ const Smtp = ({
           name="smtpPassword"
           value={smtpPassword}
           parseTarget
-          onBlur={validateForm}
+          onBlur={onInputBlur}
           error={formErrors.password}
           blockAutoComplete
         />
@@ -177,6 +210,7 @@ const Smtp = ({
           options={authMethodOptions}
           placeholder=""
           onChange={onInputChange}
+          onBlur={onInputBlur}
           name="smtpAuthenticationMethod"
           value={smtpAuthenticationMethod}
           parseTarget
@@ -205,6 +239,7 @@ const Smtp = ({
       <form onSubmit={onFormSubmit} autoComplete="off">
         <Checkbox
           onChange={onInputChange}
+          onBlur={onInputBlur}
           name="enableSMTP"
           value={enableSMTP}
           parseTarget
@@ -217,7 +252,7 @@ const Smtp = ({
           name="smtpSenderAddress"
           value={smtpSenderAddress}
           parseTarget
-          onBlur={validateForm}
+          onBlur={onInputBlur}
           error={formErrors.sender_address}
           tooltip="The sender address for emails from Fleet."
         />
@@ -228,7 +263,7 @@ const Smtp = ({
             name="smtpServer"
             value={smtpServer}
             parseTarget
-            onBlur={validateForm}
+            onBlur={onInputBlur}
             error={formErrors.server}
             tooltip="The hostname / private IP address and corresponding port of your organization's SMTP server."
           />
@@ -239,12 +274,13 @@ const Smtp = ({
             name="smtpPort"
             value={smtpPort}
             parseTarget
-            onBlur={validateForm}
+            onBlur={onInputBlur}
             error={formErrors.server_port}
           />
         </div>
         <Checkbox
           onChange={onInputChange}
+          onBlur={onInputBlur}
           name="smtpEnableSSLTLS"
           value={smtpEnableSSLTLS}
           parseTarget
@@ -255,6 +291,7 @@ const Smtp = ({
           label="Authentication type"
           options={authTypeOptions}
           onChange={onInputChange}
+          onBlur={onInputBlur}
           name="smtpAuthenticationType"
           value={smtpAuthenticationType}
           parseTarget
@@ -289,28 +326,7 @@ const Smtp = ({
   return (
     <div className={baseClass}>
       <div className={`${baseClass}__section`}>
-        <SectionHeader
-          title="SMTP options"
-          details={
-            !sesConfigured ? (
-              <small
-                className={`smtp-options smtp-options--${
-                  appConfig.smtp_settings?.configured
-                    ? "configured"
-                    : "notconfigured"
-                }`}
-              >
-                <em>
-                  {appConfig.smtp_settings?.configured
-                    ? "CONFIGURED"
-                    : "NOT CONFIGURED"}
-                </em>
-              </small>
-            ) : (
-              <></>
-            )
-          }
-        />
+        <SectionHeader title="SMTP options" />
         {sesConfigured ? renderSesEnabled() : renderSmtpForm()}
       </div>
     </div>
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		- Improve validation workflow on SMTP settings page