From 9094569b83e0fdafd09e82c8dc1172a5222eef61 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9rgio=20Salgado?= <sergiodiassalgado@gmail.com>
Date: Mon, 29 Mar 2021 15:29:06 +0100
Subject: [PATCH] fix: triggers wizard fields are more responsive to errors,
 improved UX

---
 .../src/pages/Triggers/Wizard.js              |  65 +++++++--
 .../src/pages/Triggers/helper.js              | 137 ++++++++++++------
 2 files changed, 150 insertions(+), 52 deletions(-)

diff --git a/new-lamassu-admin/src/pages/Triggers/Wizard.js b/new-lamassu-admin/src/pages/Triggers/Wizard.js
index 66a16fdc..f83bb3e6 100644
--- a/new-lamassu-admin/src/pages/Triggers/Wizard.js
+++ b/new-lamassu-admin/src/pages/Triggers/Wizard.js
@@ -222,6 +222,42 @@ const Wizard = ({ onClose, save, error, currency }) => {
     })
   }
 
+  const createErrorMessage = (errors, touched, values) => {
+    const triggerType = values?.triggerType
+    const containsType = R.contains(triggerType)
+    const isSuspend = values?.requirement?.requirement === 'suspend'
+
+    const hasRequirementError =
+      !!errors.requirement?.suspensionDays &&
+      !!touched.requirement?.suspensionDays &&
+      (!values.requirement?.suspensionDays ||
+        values.requirement?.suspensionDays < 0)
+
+    const hasAmountError =
+      !!errors.threshold &&
+      !!touched.threshold?.threshold &&
+      !containsType(['consecutiveDays']) &&
+      (!values.threshold?.threshold || values.threshold?.threshold < 0)
+
+    const hasDaysError =
+      !!errors.threshold &&
+      !!touched.threshold?.thresholdDays &&
+      !containsType(['txAmount']) &&
+      (!values.threshold?.thresholdDays || values.threshold?.thresholdDays < 0)
+
+    if (containsType(['txAmount', 'txVolume', 'txVelocity']) && hasAmountError)
+      return errors.threshold
+
+    if (
+      containsType(['txVolume', 'txVelocity', 'consecutiveDays']) &&
+      hasDaysError
+    )
+      return errors.threshold
+
+    if (isSuspend && hasRequirementError)
+      return errors.requirement?.suspensionDays
+  }
+
   return (
     <>
       <Modal
@@ -246,21 +282,28 @@ const Wizard = ({ onClose, save, error, currency }) => {
         />
         <Formik
           validateOnBlur={false}
-          validateOnChange={false}
+          validateOnChange={true}
           enableReinitialize
           onSubmit={onContinue}
           initialValues={stepOptions.initialValues}
           validationSchema={stepOptions.schema}>
-          <Form className={classes.form}>
-            <GetValues setValues={setLiveValues} />
-            <stepOptions.Component {...stepOptions.props} />
-            <div className={classes.submit}>
-              {error && <ErrorMessage>Failed to save</ErrorMessage>}
-              <Button className={classes.button} type="submit">
-                {isLastStep ? 'Finish' : 'Next'}
-              </Button>
-            </div>
-          </Form>
+          {({ errors, touched, values }) => (
+            <Form className={classes.form}>
+              <GetValues setValues={setLiveValues} />
+              <stepOptions.Component {...stepOptions.props} />
+              <div className={classes.submit}>
+                {error && <ErrorMessage>Failed to save</ErrorMessage>}
+                {createErrorMessage(errors, touched, values) && (
+                  <ErrorMessage>
+                    {createErrorMessage(errors, touched, values)}
+                  </ErrorMessage>
+                )}
+                <Button className={classes.button} type="submit">
+                  {isLastStep ? 'Finish' : 'Next'}
+                </Button>
+              </div>
+            </Form>
+          )}
         </Formik>
       </Modal>
     </>
diff --git a/new-lamassu-admin/src/pages/Triggers/helper.js b/new-lamassu-admin/src/pages/Triggers/helper.js
index 165b3505..e3b075a1 100644
--- a/new-lamassu-admin/src/pages/Triggers/helper.js
+++ b/new-lamassu-admin/src/pages/Triggers/helper.js
@@ -258,25 +258,48 @@ const typeSchema = Yup.object()
         .nullable()
     })
   })
-  .test(
-    'are-fields-set',
-    'All fields must be set.',
-    ({ triggerType, threshold }, context) => {
-      const validator = {
-        txAmount: threshold => threshold.threshold >= 0,
-        txVolume: threshold =>
-          threshold.threshold >= 0 && threshold.thresholdDays > 0,
-        txVelocity: threshold =>
-          threshold.threshold > 0 && threshold.thresholdDays > 0,
-        consecutiveDays: threshold => threshold.thresholdDays > 0
-      }
-
-      return (
-        (triggerType && validator?.[triggerType](threshold)) ||
-        context.createError({ path: 'threshold' })
-      )
+  .test(({ threshold, triggerType }, context) => {
+    console.log(context)
+    const errorMessages = {
+      txAmount: threshold => 'Amount must be greater than or equal to 0',
+      txVolume: threshold => {
+        const thresholdMessage = 'Volume must be greater than or equal to 0'
+        const thresholdDaysMessage = 'Days must be greater than 0'
+        const message = []
+        if (!threshold.threshold || threshold.threshold < 0)
+          message.push(thresholdMessage)
+        if (!threshold.thresholdDays || threshold.thresholdDays <= 0)
+          message.push(thresholdDaysMessage)
+        return message.join(', ')
+      },
+      txVelocity: threshold => {
+        const thresholdMessage = 'Transactions must be greater than 0'
+        const thresholdDaysMessage = 'Days must be greater than 0'
+        const message = []
+        if (!threshold.threshold || threshold.threshold <= 0)
+          message.push(thresholdMessage)
+        if (!threshold.thresholdDays || threshold.thresholdDays <= 0)
+          message.push(thresholdDaysMessage)
+        return message.join(', ')
+      },
+      consecutiveDays: threshold => 'Days must be greater than 0'
     }
-  )
+    const thresholdValidator = {
+      txAmount: threshold => threshold.threshold >= 0,
+      txVolume: threshold =>
+        threshold.threshold >= 0 && threshold.thresholdDays > 0,
+      txVelocity: threshold =>
+        threshold.threshold > 0 && threshold.thresholdDays > 0,
+      consecutiveDays: threshold => threshold.thresholdDays > 0
+    }
+
+    if (triggerType && thresholdValidator[triggerType](threshold)) return
+
+    return context.createError({
+      path: 'threshold',
+      message: errorMessages[triggerType](threshold)
+    })
+  })
 
 const typeOptions = [
   { display: 'Transaction amount', code: 'txAmount' },
@@ -299,11 +322,21 @@ const Type = ({ ...props }) => {
   const isThresholdDaysEnabled = containsType(['txVolume', 'txVelocity'])
   const isConsecutiveDaysEnabled = containsType(['consecutiveDays'])
 
+  const hasAmountError =
+    !!errors.threshold &&
+    !!touched.threshold?.threshold &&
+    !isConsecutiveDaysEnabled &&
+    (!values.threshold?.threshold || values.threshold?.threshold < 0)
+  const hasDaysError =
+    !!errors.threshold &&
+    !!touched.threshold?.thresholdDays &&
+    !containsType(['txAmount']) &&
+    (!values.threshold?.thresholdDays || values.threshold?.thresholdDays < 0)
+
+  const triggerTypeError = !!(hasDaysError || hasAmountError)
+
   const thresholdClass = {
-    [classes.error]:
-      errors.threshold &&
-      ((!containsType(['consecutiveDays']) && touched.threshold?.threshold) ||
-        (!containsType(['txAmount']) && touched.threshold?.thresholdDays))
+    [classes.error]: triggerTypeError
   }
 
   const isRadioGroupActive = () => {
@@ -343,6 +376,7 @@ const Type = ({ ...props }) => {
                 component={NumberInput}
                 size="lg"
                 name="threshold.threshold"
+                error={hasAmountError}
               />
               <Info1 className={classnames(classes.description)}>
                 {props.currency}
@@ -356,6 +390,7 @@ const Type = ({ ...props }) => {
                 component={NumberInput}
                 size="lg"
                 name="threshold.threshold"
+                error={hasAmountError}
               />
               <Info1 className={classnames(classes.description)}>
                 transactions
@@ -377,6 +412,7 @@ const Type = ({ ...props }) => {
                 component={NumberInput}
                 size="lg"
                 name="threshold.thresholdDays"
+                error={hasDaysError}
               />
               <Info1 className={classnames(classes.description)}>days</Info1>
             </>
@@ -388,6 +424,7 @@ const Type = ({ ...props }) => {
                 component={NumberInput}
                 size="lg"
                 name="threshold.thresholdDays"
+                error={hasDaysError}
               />
               <Info1 className={classnames(classes.description)}>
                 consecutive days
@@ -411,20 +448,34 @@ const type = currency => ({
   }
 })
 
-const requirementSchema = Yup.object().shape({
-  requirement: Yup.object({
-    requirement: Yup.string().required(),
-    suspensionDays: Yup.number().when('requirement', {
-      is: value => value === 'suspend',
-      then: Yup.number()
-        .required()
-        .min(1),
-      otherwise: Yup.number()
-        .nullable()
-        .transform(() => null)
+const requirementSchema = Yup.object()
+  .shape({
+    requirement: Yup.object({
+      requirement: Yup.string().required(),
+      suspensionDays: Yup.number().when('requirement', {
+        is: value => value === 'suspend',
+        then: Yup.number()
+          .required()
+          .min(1),
+        otherwise: Yup.number()
+          .nullable()
+          .transform(() => null)
+      })
+    }).required()
+  })
+  .test(({ requirement }, context) => {
+    const requirementValidator = requirement =>
+      requirement.requirement === 'suspend'
+        ? requirement.suspensionDays > 0
+        : true
+
+    if (requirement && requirementValidator(requirement)) return
+
+    return context.createError({
+      path: 'requirement',
+      message: 'Suspension days must be greater than 0'
     })
-  }).required()
-})
+  })
 
 const requirementOptions = [
   { display: 'SMS verification', code: 'sms' },
@@ -442,17 +493,20 @@ const Requirement = () => {
   const classes = useStyles()
   const { touched, errors, values } = useFormikContext()
 
+  const hasRequirementError =
+    !!errors.requirement?.suspensionDays &&
+    !!touched.requirement?.suspensionDays &&
+    (!values.requirement?.suspensionDays ||
+      values.requirement?.suspensionDays < 0)
+
+  const isSuspend = values?.requirement?.requirement === 'suspend'
+
   const titleClass = {
     [classes.error]:
       !R.isEmpty(R.omit(['suspensionDays'], errors.requirement)) ||
-      (errors.requirement &&
-        touched.requirement &&
-        errors.requirement.suspensionDays &&
-        touched.requirement.suspensionDays)
+      (isSuspend && hasRequirementError)
   }
 
-  const isSuspend = values?.requirement?.requirement === 'suspend'
-
   return (
     <>
       <Box display="flex" alignItems="center">
@@ -474,6 +528,7 @@ const Requirement = () => {
           label="Days"
           size="lg"
           name="requirement.suspensionDays"
+          error={hasRequirementError}
         />
       )}
     </>