refactor: apply suggestions from code review

1. For selectedValidation, create new enum in the frontend and simplify shared enum values.
2. Rename rangeMinimum and rangeMaximum to customMin and customMax respectively.
3. Rewrite typeof checks to truthiness checks in backend validators.
4. Remove unnecessary '' from selectedLengthValidation

frontend/src/features/admin-form/create/builder-and-design/BuilderAndDesignDrawer/EditFieldDrawer/edit-fieldtype/EditNumber/EditNumber.tsx

@@ -29,47 +29,58 @@ type EditNumberProps = EditFieldProps<NumberFieldBase>
 
 const EDIT_NUMBER_FIELD_KEYS = ['title', 'description', 'required'] as const
 
+// As we want to keep the values in the shared type simple,
+// we create a separate enum for frontend options and transform them as needed
+enum NumberSelectedValidationInputs {
+  Length = 'Character Length',
+  Range = 'Number Range',
+}
+
 type EditNumberInputs = Pick<
   NumberFieldBase,
   typeof EDIT_NUMBER_FIELD_KEYS[number]
 > & {
   ValidationOptions: {
-    selectedValidation: NumberSelectedValidation | ''
+    selectedValidation: NumberSelectedValidationInputs | ''
     LengthValidationOptions: {
       customVal: number | ''
       selectedLengthValidation: NumberSelectedLengthValidation | ''
     }
     RangeValidationOptions: {
-      rangeMinimum: number | ''
-      rangeMaximum: number | ''
+      customMin: number | ''
+      customMax: number | ''
     }
   }
 }
 
 const transformNumberFieldToEditForm = (
   field: NumberFieldBase,
 ): EditNumberInputs => {
+  const {
+    selectedValidation,
+    LengthValidationOptions,
+    RangeValidationOptions,
+  } = field.ValidationOptions
+
   const nextSelectedValidation =
-    field.ValidationOptions.selectedValidation || ('' as const)
+    selectedValidation === NumberSelectedValidation.Length
+      ? NumberSelectedValidationInputs.Length
+      : selectedValidation === NumberSelectedValidation.Range
+      ? NumberSelectedValidationInputs.Range
+      : ('' as const)
 
   const nextLengthValidationOptions = {
     selectedLengthValidation:
-      field.ValidationOptions.LengthValidationOptions
-        .selectedLengthValidation || ('' as const),
+      LengthValidationOptions.selectedLengthValidation || ('' as const),
     customVal:
-      (!!field.ValidationOptions.LengthValidationOptions
-        .selectedLengthValidation &&
-        field.ValidationOptions.LengthValidationOptions.customVal) ||
+      (!!LengthValidationOptions.selectedLengthValidation &&
+        LengthValidationOptions.customVal) ||
       ('' as const),
   }
 
   const nextRangeValidationOptions = {
-    rangeMinimum:
-      field.ValidationOptions.RangeValidationOptions.rangeMinimum ||
-      ('' as const),
-    rangeMaximum:
-      field.ValidationOptions.RangeValidationOptions.rangeMaximum ||
-      ('' as const),
+    customMin: RangeValidationOptions.customMin || ('' as const),
+    customMax: RangeValidationOptions.customMax || ('' as const),
   }
 
   return {
@@ -86,30 +97,40 @@ const transformNumberEditFormToField = (
   inputs: EditNumberInputs,
   originalField: NumberFieldBase,
 ): NumberFieldBase => {
+  const {
+    selectedValidation,
+    LengthValidationOptions,
+    RangeValidationOptions,
+  } = inputs.ValidationOptions
+
   const hasSelectedLengthValidationOption =
-    inputs.ValidationOptions.selectedValidation ===
-      NumberSelectedValidation.Length &&
-    inputs.ValidationOptions.LengthValidationOptions
-      .selectedLengthValidation !== ''
+    selectedValidation === NumberSelectedValidationInputs.Length &&
+    LengthValidationOptions.selectedLengthValidation !== ''
+
+  const nextSelectedValidation =
+    selectedValidation === NumberSelectedValidationInputs.Length
+      ? NumberSelectedValidation.Length
+      : selectedValidation === NumberSelectedValidationInputs.Range
+      ? NumberSelectedValidation.Range
+      : null
 
   const nextLengthValidationOptions = hasSelectedLengthValidationOption
-    ? inputs.ValidationOptions.LengthValidationOptions
+    ? LengthValidationOptions
     : {
         selectedLengthValidation: null,
         customVal: null,
       }
 
   const nextRangeValidationOptions =
-    inputs.ValidationOptions.selectedValidation ===
-    NumberSelectedValidation.Range
-      ? inputs.ValidationOptions.RangeValidationOptions
+    selectedValidation === NumberSelectedValidationInputs.Range
+      ? RangeValidationOptions
       : {
-          rangeMinimum: null,
-          rangeMaximum: null,
+          customMin: null,
+          customMax: null,
         }
 
   const nextValidationOptions = {
-    selectedValidation: inputs.ValidationOptions.selectedValidation || null,
+    selectedValidation: nextSelectedValidation,
     LengthValidationOptions: nextLengthValidationOptions,
     RangeValidationOptions: nextRangeValidationOptions,
   }
@@ -182,23 +203,23 @@ export const EditNumber = ({ field }: EditNumberProps): JSX.Element => {
     [getValues],
   )
 
-  // We use the rangeMinimum field to perform cross-field validation for
+  // We use the customMin field to perform cross-field validation for
   // the number range
   const RangeMinimumValidationOptions: RegisterOptions<
     EditNumberInputs,
-    'ValidationOptions.RangeValidationOptions.rangeMinimum'
+    'ValidationOptions.RangeValidationOptions.customMin'
   > = useMemo(
     () => ({
       validate: {
         hasValidRange: (val) => {
           const numVal = Number(val)
-          const rangeMaximum = getValues(
-            'ValidationOptions.RangeValidationOptions.rangeMaximum',
+          const customMax = getValues(
+            'ValidationOptions.RangeValidationOptions.customMax',
           )
-          const numRangeMaximum = Number(rangeMaximum)
+
           return (
-            isNaN(numRangeMaximum) ||
-            numVal <= numRangeMaximum ||
+            customMax === '' ||
+            numVal <= Number(customMax) ||
             `Please enter a valid range!`
           )
         },
@@ -212,8 +233,8 @@ export const EditNumber = ({ field }: EditNumberProps): JSX.Element => {
     if (!watchedSelectedValidation) {
       clearErrors('ValidationOptions')
       setValue('ValidationOptions.LengthValidationOptions.customVal', '')
-      setValue('ValidationOptions.RangeValidationOptions.rangeMinimum', '')
-      setValue('ValidationOptions.RangeValidationOptions.rangeMaximum', '')
+      setValue('ValidationOptions.RangeValidationOptions.customMin', '')
+      setValue('ValidationOptions.RangeValidationOptions.customMax', '')
     }
   }, [clearErrors, setValue, watchedSelectedValidation])
 
@@ -248,12 +269,12 @@ export const EditNumber = ({ field }: EditNumberProps): JSX.Element => {
           control={control}
           render={({ field }) => (
             <SingleSelect
-              items={Object.values(NumberSelectedValidation)}
+              items={Object.values(NumberSelectedValidationInputs)}
               {...field}
             />
           )}
         />
-        {watchedSelectedValidation === NumberSelectedValidation.Range && (
+        {watchedSelectedValidation === NumberSelectedValidationInputs.Range && (
           <>
             <FormLabel isRequired mt="0.5rem">
               Minimum and/or maximum value
@@ -264,7 +285,7 @@ export const EditNumber = ({ field }: EditNumberProps): JSX.Element => {
               spacing="0.5rem"
             >
               <Controller
-                name="ValidationOptions.RangeValidationOptions.rangeMinimum"
+                name="ValidationOptions.RangeValidationOptions.customMin"
                 control={control}
                 rules={RangeMinimumValidationOptions}
                 render={({ field: { onChange, ...rest } }) => (
@@ -278,7 +299,7 @@ export const EditNumber = ({ field }: EditNumberProps): JSX.Element => {
                 )}
               />
               <Controller
-                name="ValidationOptions.RangeValidationOptions.rangeMaximum"
+                name="ValidationOptions.RangeValidationOptions.customMax"
                 control={control}
                 render={({ field: { onChange, ...rest } }) => (
                   <NumberInput
@@ -293,13 +314,14 @@ export const EditNumber = ({ field }: EditNumberProps): JSX.Element => {
             </SimpleGrid>
             <FormErrorMessage>
               {
-                errors?.ValidationOptions?.RangeValidationOptions?.rangeMinimum
+                errors?.ValidationOptions?.RangeValidationOptions?.customMin
                   ?.message
               }
             </FormErrorMessage>
           </>
         )}
-        {watchedSelectedValidation === NumberSelectedValidation.Length && (
+        {watchedSelectedValidation ===
+          NumberSelectedValidationInputs.Length && (
           <>
             <FormLabel isRequired mt="0.5rem">
               Number of characters allowed

frontend/src/features/admin-form/create/builder-and-design/utils/fieldCreation.ts

@@ -112,8 +112,8 @@ export const getFieldCreationMeta = (fieldType: BasicField): FieldCreateDto => {
             customVal: null,
           },
           RangeValidationOptions: {
-            rangeMinimum: null,
-            rangeMaximum: null,
+            customMin: null,
+            customMax: null,
           },
         },
       }

frontend/src/utils/fieldValidation.ts

@@ -210,7 +210,7 @@ export const createNumberValidationRules: ValidationRuleFn<NumberFieldBase> = (
   const { selectedValidation } = schema.ValidationOptions
   const { selectedLengthValidation, customVal } =
     schema.ValidationOptions.LengthValidationOptions
-  const { rangeMinimum, rangeMaximum } =
+  const { customMin, customMax } =
     schema.ValidationOptions.RangeValidationOptions
 
   return {
@@ -249,19 +249,19 @@ export const createNumberValidationRules: ValidationRuleFn<NumberFieldBase> = (
           return true
 
         const numVal = Number(val)
-        const hasMinimum = typeof rangeMinimum === 'number'
-        const hasMaximum = typeof rangeMaximum === 'number'
+        const hasMinimum = !!customMin
+        const hasMaximum = !!customMax
         const isOutsideRange =
-          (hasMinimum && rangeMinimum > numVal) ||
-          (hasMaximum && rangeMaximum < numVal)
+          (hasMinimum && customMin > numVal) ||
+          (hasMaximum && customMax < numVal)
 
         if (!isOutsideRange) return true
         else if (hasMinimum && hasMaximum) {
-          return `Please enter a number within the range ${rangeMinimum} to ${rangeMaximum}`
+          return `Please enter a number within the range ${customMin} to ${customMax}`
         } else if (hasMinimum) {
-          return `Please enter a number that has a minimum value of ${rangeMinimum}`
+          return `Please enter a number that has a minimum value of ${customMin}`
         } else if (hasMaximum) {
-          return `Please enter a number that has a maximum value of ${rangeMaximum}`
+          return `Please enter a number that has a maximum value of ${customMax}`
         }
       },
     },

shared/types/field/numberField.ts

@@ -1,8 +1,8 @@
 import { BasicField, MyInfoableFieldBase } from './base'
 
 export enum NumberSelectedValidation {
-  Length = 'Character Length',
-  Range = 'Number Range',
+  Length = 'Length',
+  Range = 'Range',
 }
 
 export enum NumberSelectedLengthValidation {
@@ -13,12 +13,12 @@ export enum NumberSelectedLengthValidation {
 
 export type NumberLengthValidationOptions = {
   customVal: number | null
-  selectedLengthValidation: NumberSelectedLengthValidation | '' | null
+  selectedLengthValidation: NumberSelectedLengthValidation | null
 }
 
 export type NumberRangeValidationOptions = {
-  rangeMinimum: number | null
-  rangeMaximum: number | null
+  customMin: number | null
+  customMax: number | null
 }
 
 export type NumberValidationOptions = {

src/app/models/field/numberField.ts

@@ -11,28 +11,28 @@ import { MyInfoSchema } from './baseField'
 
 const createNumberFieldSchema = () => {
   const ValidationOptionsSchema = new Schema<NumberValidationOptions>({
-      selectedValidation: {
+    selectedValidation: {
+      type: String,
+      enum: [...Object.values(NumberSelectedValidation), null],
+    },
+    LengthValidationOptions: {
+      customVal: {
+        type: Number,
+      },
+      selectedLengthValidation: {
         type: String,
-        enum: [...Object.values(NumberSelectedValidation), null],
+        enum: [...Object.values(NumberSelectedLengthValidation), null],
       },
-      LengthValidationOptions: {
-        customVal: {
-          type: Number,
-        },
-        selectedLengthValidation: {
-          type: String,
-          enum: [...Object.values(NumberSelectedLengthValidation), null],
-        },
+    },
+    RangeValidationOptions: {
+      customMin: {
+        type: Number,
       },
-      RangeValidationOptions: {
-        rangeMinimum: {
-          type: Number,
-        },
-        rangeMaximum: {
-          type: Number,
-        },
+      customMax: {
+        type: Number,
       },
-    })
+    },
+  })
 
   const NumberFieldSchema = new Schema<INumberFieldSchema>({
     myInfo: MyInfoSchema,
@@ -46,8 +46,8 @@ const createNumberFieldSchema = () => {
           selectedLengthValidation: null,
         },
         RangeValidationOptions: {
-          rangeMinimum: null,
-          rangeMaximum: null,
+          customMin: null,
+          customMax: null,
         },
       },
     },

src/app/utils/field-validation/validators/numberValidator.ts

@@ -92,15 +92,13 @@ const getNumberLengthValidator: NumberValidatorConstructor = (numberField) => {
  * Returns a validation function to check if number is
  * within the number range specified.
  */
-const numberRangeValidator: NumberValidatorConstructor =
+const getNumberRangeValidator: NumberValidatorConstructor =
   (numberField) => (response) => {
     const val = Number(response.answer)
-    const { rangeMinimum, rangeMaximum } =
+    const { customMin, customMax } =
       numberField.ValidationOptions.RangeValidationOptions
-    const isWithinMinimum =
-      typeof rangeMinimum === 'number' && rangeMinimum <= val
-    const isWithinMaximum =
-      typeof rangeMaximum === 'number' && val <= rangeMaximum
+    const isWithinMinimum = !!customMin && customMin <= val
+    const isWithinMaximum = !!customMax && val <= customMax
 
     return isWithinMinimum && isWithinMaximum
       ? right(response)
@@ -116,7 +114,7 @@ const getNumberValidator: NumberValidatorConstructor = (numberField) => {
     case NumberSelectedValidation.Length:
       return getNumberLengthValidator(numberField)
     case NumberSelectedValidation.Range:
-      return numberRangeValidator(numberField)
+      return getNumberRangeValidator(numberField)
     default:
       return right
   }