Merge pull request #87 from Ragepanda12/error-message-formatting

Join error messages with punctuation more gramatically correctly

Author: draperd

packages/core/src/comparator.test.js

@@ -154,7 +154,7 @@ describe("compare date fields", () => {
     secondDate.prop("onBlur")();
     form.update();
     expect(form.state().fields[0].errorMessages).toBe(
-      "A value must be provided. Must be before second date"
+      "A value must be provided. Must be before second date."
     );
     expect(form.state().fields[1].errorMessages).toBe("");
   });

packages/core/src/utilities/validation.js

@@ -342,7 +342,31 @@ export const validateField: ValidateField = (
   const { required, visible, validWhen = {}, touched = false } = field;
   let isValid = true;
   let errorMessages = [];
-  const formattedErrorMessage = () => errorMessages.join(". ");
+  const formattedErrorMessage = () => {
+    if(errorMessages.length === 0){
+      return "";
+    }
+    else if(errorMessages.length === 1){
+      // Don't add punctuation on singular error messages.
+      return errorMessages[0];
+    }
+    else{
+      // Reverse through list of error messages to preserve order
+      let formattedErrorMessages = "";
+      errorMessages.slice().reverse().forEach((message)=>{
+        const lastChar = message.slice(-1);
+        if(lastChar === "." || lastChar ==="!" || lastChar === "?"){
+          formattedErrorMessages = `${message} ${formattedErrorMessages}`;
+        }
+        else {
+          formattedErrorMessages = `${message}. ${formattedErrorMessages}`;
+        }
+      });
+      // Remove trailing space
+      return formattedErrorMessages.slice(0, -1);
+    }
+
+  };
 
   if (visible) {
     const value = getValueFromField(field);

packages/core/src/utilities/validation.test.js

@@ -49,7 +49,7 @@ describe("validateField", () => {
     expect(validateField(testField, [testField], true).isValid).toBe(false);
   });
 
-  test("visible, required field with numberical value 0 is valid", () => {
+  test("visible, required field with numerical value 0 is valid", () => {
     const testField = {
       ...field1,
       visible: true,
@@ -99,6 +99,116 @@ describe("validateField", () => {
     expect(validateField(testField, [testField], true).isValid).toBe(true);
   });
 
+  test("correctly concats multiple error messages with ? punctuation", () => {
+    const errorMessage = "Where's my data?";
+    const errorMessage2 = "Where is my data!";
+    const testField = {
+      ...field1,
+      visible: true,
+      required: true,
+      value: "",
+      missingValueMessage: errorMessage,
+      validWhen:{
+        isNot:{
+          values:[""],
+          message: errorMessage2,
+        }
+      }
+    };
+    const validationResult = validateField(testField, [testField], true);
+    expect(validationResult.isValid).toBe(false);
+    expect(validationResult.errorMessages).toBe("Where's my data? Where is my data!");
+    expect(validateField(testField, [testField], true).isValid).toBe(false);
+  });
+
+  test("correctly concats multiple error messages with ! punctuation", () => {
+    const errorMessage = "Where's my data!";
+    const errorMessage2 = "Where is my data?";
+    const testField = {
+      ...field1,
+      visible: true,
+      required: true,
+      value: "",
+      missingValueMessage: errorMessage,
+      validWhen:{
+        isNot:{
+          values:[""],
+          message: errorMessage2,
+        }
+      }
+    };
+    const validationResult = validateField(testField, [testField], true);
+    expect(validationResult.isValid).toBe(false);
+    expect(validationResult.errorMessages).toBe("Where's my data! Where is my data?");
+    expect(validateField(testField, [testField], true).isValid).toBe(false);
+  });
+
+  test("correctly concats multiple error messages with no full stop", () => {
+    const errorMessage = "Where's my data";
+    const errorMessage2 = "Where is my data?";
+    const testField = {
+      ...field1,
+      visible: true,
+      required: true,
+      value: "",
+      missingValueMessage: errorMessage,
+      validWhen:{
+        isNot:{
+          values:[""],
+          message: errorMessage2,
+        }
+      }
+    };
+    const validationResult = validateField(testField, [testField], true);
+    expect(validationResult.isValid).toBe(false);
+    expect(validationResult.errorMessages).toBe("Where's my data. Where is my data?");
+    expect(validateField(testField, [testField], true).isValid).toBe(false);
+  });
+
+  test("correctly concats multiple error messages with a full stop", () => {
+    const errorMessage = "Where's my data.";
+    const errorMessage2 = "Where is my data.";
+    const testField = {
+      ...field1,
+      visible: true,
+      required: true,
+      value: "",
+      missingValueMessage: errorMessage,
+      validWhen:{
+        isNot:{
+          values:[""],
+          message: errorMessage2,
+        }
+      }
+    };
+    const validationResult = validateField(testField, [testField], true);
+    expect(validationResult.isValid).toBe(false);
+    expect(validationResult.errorMessages).toBe("Where's my data. Where is my data.");
+    expect(validateField(testField, [testField], true).isValid).toBe(false);
+  });
+
+  test("concats multiple error messages in order", () => {
+    const errorMessage = "This should be first";
+    const errorMessage2 = "This should be second";
+    const testField = {
+      ...field1,
+      visible: true,
+      required: true,
+      value: "",
+      missingValueMessage: errorMessage,
+      validWhen:{
+        isNot:{
+          values:[""],
+          message: errorMessage2,
+        }
+      }
+    };
+    const validationResult = validateField(testField, [testField], true);
+    expect(validationResult.isValid).toBe(false);
+    expect(validationResult.errorMessages).toBe("This should be first. This should be second.");
+    expect(validateField(testField, [testField], true).isValid).toBe(false);
+  });
+
   test("visible, required field with missing data shows custom message", () => {
     const errorMessage = "Where's my data?";
     const testField = {
@@ -239,7 +349,7 @@ describe("lengthIsLessThan validator", () => {
     ).toBeUndefined();
   });
 
-  test("with invvalid value", () => {
+  test("with invalid value", () => {
     expect(
       lengthIsLessThan({
         value: "test",
@@ -688,4 +798,4 @@ describe("hasValue", () => {
   test("null", () => {
     expect(hasValue(null)).toEqual(false);
   });
-});
+});