fix(#55): address feedback

medic · Mar 29, 2024 · 9d4a9ba · 9d4a9ba
1 parent 848f06c
commit 9d4a9ba
Show file tree

Hide file tree

Showing 11 changed files with 37 additions and 39 deletions.
diff --git a/README.md b/README.md
@@ -67,11 +67,11 @@ The `ConfigPropertyType` defines a property's validation rules and auto-formatti
 | name      | Must be defined                                        | Same as string + title case + `parameter` behavior                  | One or more regexes which are removed from the value when matched (eg. `"parameter": ["\\sCHU"]` will format `this CHU` into `This`) |
 | regex     | Must match the `regex` captured by `parameter`         | Same as `string`                                                    | A regex which must be matched to pass validation (eg. `"parameter": "^\\d{6}$"` will accept only 6 digit numbers)     |
 | phone     | A valid phone number for the specified locality        | Auto formatting provided by [libphonenumber](https://github.com/google/libphonenumber)          | Two letter country code specifying the locality of phone number (eg. `"parameter": "KE"`)             |
-| none      | None                                                   | None                                                                | None                                                                                                   |
-| gender    | A binary gender (eg. `Male`, `Woman`, `M`)             | Formats to either `Male` or `Female`                                | None                                                                                                   |
+None                                                                                                   |
 | generated | None. No user inputs.                                  | Uses [LiquidJS](https://liquidjs.com) templates to generate data    | None                                                                                                   | [Details](#The-Generated-ConfigPropertyType)
 | select_one      | Single choice from a list of options            | Same as `string`                                                                                               | None                                                                                                   | Dictionary where the keys are the option values and the values are the corresponding labels |
 | select_multiple | Multiple choice from a list of options          | Same as `string`                                                                                               | None                                                                                                   | Same as `select_one`    
+| none      | None                                                   | None                                                                | None                                                                                                   |
 
 #### The Generated ConfigPropertyType
 ContactProperties with `type: "generated"` use the [LiquidJS](https://liquidjs.com) template engine to populate a property with data. Here is an example of some configuration properties which use `"type": "generated"`:

diff --git a/src/config/index.ts b/src/config/index.ts
@@ -115,7 +115,7 @@ export class Config {
       property_name: 'role',
       type: 'select_multiple',
       required: true,
-      parameter: parameter,
+      parameter,
     };
   }
 

diff --git a/src/lib/validation.ts b/src/lib/validation.ts
@@ -9,10 +9,10 @@ import ValidatorGenerated from './validator-generated';
 import ValidatorName from './validator-name';
 import ValidatorPhone from './validator-phone';
 import ValidatorRegex from './validator-regex';
-import ValidatorSkip from './validator-skip';
-import ValidatorString from './validator-string';
 import ValidatorSelectMultiple from './validator-select_multiple';
 import ValidatorSelectOne from './validator-select_one';
+import ValidatorSkip from './validator-skip';
+import ValidatorString from './validator-string';
 
 export type ValidationError = {
   property_name: string;

diff --git a/src/lib/validator-name.ts b/src/lib/validator-name.ts
@@ -3,7 +3,12 @@ import { IValidator } from './validation';
 import ValidatorString from './validator-string';
 
 export default class ValidatorName implements IValidator {
-  isValid(input: string) : boolean | string {
+  isValid(input: string, property : ContactProperty) : boolean | string {
+    // Verify property.parameter is always array
+    if (property.parameter && !Array.isArray(property.parameter)) {
+      throw Error(`property '${property.friendly_name}' of type 'regex' expects 'parameter' to be an array.`);
+    }
+
     return !!input;
   }
 

diff --git a/src/lib/validator-phone.ts b/src/lib/validator-phone.ts
@@ -9,6 +9,10 @@ export default class ValidatorPhone implements IValidator {
       throw Error(`property of type phone on ${property.friendly_name} missing parameter with locale`);
     }
 
+    if (typeof property.parameter !== 'string') {
+      throw Error(`property '${property.friendly_name}' of type 'phone' expects 'parameter' to be a string.`);
+    }
+
     try {
       const isValid = isValidNumberForRegion(input, property.parameter as CountryCode);
       if (isValid) {

diff --git a/src/lib/validator-regex.ts b/src/lib/validator-regex.ts
@@ -9,10 +9,10 @@ export default class ValidatorRegex implements IValidator {
       throw Error(`property of type regex - ${property.friendly_name} is missing parameter`);
     }
 
-    if (Array.isArray(property.parameter)) {
-      throw Error(`property of type regex - 'parameter' should not be an array`);
+    if (typeof property.parameter !== 'string') {
+      throw Error(`property '${property.friendly_name}' of type 'regex' expects 'parameter' to be a string.`);
     }
-
+    
     const regex = new RegExp(property.parameter.toString());
     const validatorStr = new ValidatorString();
     const altered = validatorStr.format(input);

diff --git a/src/lib/validator-select_multiple.ts b/src/lib/validator-select_multiple.ts
@@ -21,7 +21,7 @@ export default class ValidatorSelectMultiple implements IValidator {
     );
 
     if (invalidValues.length > 0) {
-      return `Invalid values: ${invalidValues.join(', ')}`;
+      return `Invalid values for property "${property.friendly_name}": ${invalidValues.join(', ')}`;
     }
 
     // Check if any values are missing and property is required
@@ -48,7 +48,7 @@ export default class ValidatorSelectMultiple implements IValidator {
     return input
       .split(this.DELIMITER)
       .map(value => stringValidator.format(value))
-      .filter(value => value);
+      .filter(Boolean);
   }
 }
 
diff --git a/src/liquid/components/contact_type_property.html b/src/liquid/components/contact_type_property.html
@@ -6,16 +6,12 @@
     </label>
     <div class="control">
       {% if include.prop.type == 'select_one' or include.prop.type == 'select_multiple' %}
-        {% assign is_multiple = include.prop.type == 'select_multiple'  %}
-        <div class="select is-fullwidth{% if is_multiple %} is-multiple{% endif %}">
-          <select name="{{ prop_name }}" {% if is_multiple %}multiple{% endif %}>
-            {% for params in include.prop.parameter %}
-            <option value="{{ params[0] }}" {% if data[prop_name] == params[0] or data[prop_name] contains params[0] %}selected{% endif %}>
-              {{ params[1] }}
-            </option>
-            {% endfor %}
-          </select>
-        </div>
+        {%
+          include "components/contact_type_select.html"
+          prop_name=prop_name
+          prop=prop
+          data=data
+        %}
       {% else %}
         <input
           name="{{ prop_name }}"

diff --git a/src/liquid/components/contact_type_select.html b/src/liquid/components/contact_type_select.html
@@ -0,0 +1,10 @@
+{% assign is_multiple = include.prop.type == 'select_multiple'  %}
+<div class="select is-fullwidth{% if is_multiple %} is-multiple{% endif %}">
+  <select name="{{ prop_name }}" {% if is_multiple %}multiple{% endif %}>
+    {% for params in include.prop.parameter %}
+    <option value="{{ params[0] }}" {% if data[prop_name] == params[0] or data[prop_name] contains params[0] %}selected{% endif %}>
+      {{ params[1] }}
+    </option>
+    {% endfor %}
+  </select>
+</div>
diff --git a/src/liquid/components/user_role_property.html b/src/liquid/components/user_role_property.html
diff --git a/test/lib/validation.spec.ts b/test/lib/validation.spec.ts
@@ -195,7 +195,7 @@ describe('lib/validation.ts', () => {
 
     expect(Validation.getValidationErrors(place)).to.deep.eq([{
       property_name: 'user_role',
-      description: 'Invalid values: stockmanager'
+      description: `Invalid values for property "Role(s)": stockmanager`
     }]);
   });
 });