Merge pull request #21 from Hipo/fix/nested-error-handling

fix: Make sure message format options apply to nested error details

.gitignore

@@ -16,3 +16,5 @@ npm-debug.log
 
 # We gitignore the `/lib` but whitelist it at files property at package.json
 /lib
+
+.history

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@hipo/hipo-exceptions-js",
-  "version": "1.0.8",
+  "version": "1.0.9",
   "description": "JavaScript client for parsing the hipo-drf-exceptions",
   "main": "lib/index.js",
   "files": [

src/ExceptionTransformer.ts

@@ -151,9 +151,11 @@ class ExceptionTransformer {
           finalMessage = getStringMessage(
             removeKnownKeysFromErrorDetail(errorDetail, knownErrorKeys),
             {
-              shouldHideErrorKey,
-              shouldCapitalizeErrorKey,
-              fieldLabelMap
+              keyOptions: {
+                shouldHideErrorKey,
+                shouldCapitalizeErrorKey,
+                fieldLabelMap
+              }
             }
           );
         } else {

src/utils/errorUtils.ts

@@ -27,7 +27,7 @@ function generateMessageFromStringArray(array: string[], key?: string): string {
 function generateFieldErrorFromErrorDetail(
   fieldName: string,
   errorDetail: ExceptionDetail
-) {
+): string[] | undefined {
   if (typeof fieldName !== "string") {
     throw new Error("fieldName can be string only");
   }
@@ -50,24 +50,24 @@ function generateFieldErrorFromErrorDetail(
 }
 
 interface StringMessageGeneratorKeyOptions {
-  customKey?: string;
   shouldCapitalizeErrorKey?: boolean;
   shouldHideErrorKey?: boolean;
   fieldLabelMap?: {[key: string]: string};
 }
 
 function getStringMessage(
   errorDetailValue: ExceptionDetailValue,
-  keyOptions?: StringMessageGeneratorKeyOptions
+  options?: {key?: string; keyOptions?: StringMessageGeneratorKeyOptions}
 ): string {
   let message = "";
 
   if (Array.isArray(errorDetailValue)) {
     if (isArrayOfStrings(errorDetailValue)) {
+      // This is the exit condition of this recursion, string message can be generated now
       // errorDetailValue = ["", ""]
       message = generateMessageFromStringArray(
         errorDetailValue,
-        keyOptions?.customKey
+        generateErrorKeyToDisplay(options?.key || "", options?.keyOptions)
       );
     } else if (isArrayOfObjects(errorDetailValue)) {
       // errorDetailValue = [ {}, {}, {..} ]
@@ -76,7 +76,9 @@ function getStringMessage(
       );
 
       if (firstNonEmptyErrorObject) {
-        message = getStringMessage(firstNonEmptyErrorObject, keyOptions);
+        message = getStringMessage(firstNonEmptyErrorObject, {
+          keyOptions: options?.keyOptions
+        });
       }
     }
   } else if (typeof errorDetailValue === "object") {
@@ -89,19 +91,18 @@ function getStringMessage(
         errorDetailKeys.includes("non_field_errors") &&
         errorDetailValue.non_field_errors
       ) {
-        message = getStringMessage(
-          errorDetailValue.non_field_errors,
-          keyOptions
-        );
+        message = getStringMessage(errorDetailValue.non_field_errors, {
+          keyOptions: options?.keyOptions
+        });
       } else {
         const defaultErrorKey = errorDetailKeys[0];
 
-        // Generate message from the immediately found field error
+        // Start recursion again with the first key's value
+        // `key` should be sent in case `errorDetailValue[defaultErrorKey]` is the recursion's exit value: string[]
+        // `key` then can be processed according to `keyOptions`
         message = getStringMessage(errorDetailValue[defaultErrorKey], {
-          customKey: getErrorKeyForStringMessageGenerator(
-            defaultErrorKey,
-            keyOptions
-          )
+          key: defaultErrorKey,
+          keyOptions: options?.keyOptions
         });
       }
     }
@@ -113,16 +114,14 @@ function getStringMessage(
   return message;
 }
 
-function getErrorKeyForStringMessageGenerator(
+function generateErrorKeyToDisplay(
   defaultErrorKey: string,
   keyOptions: StringMessageGeneratorKeyOptions | undefined
-) {
+): string | undefined {
   let errorKey: string | undefined = defaultErrorKey;
 
   if (keyOptions?.shouldHideErrorKey) {
     errorKey = undefined;
-  } else if (keyOptions?.customKey) {
-    errorKey = keyOptions?.customKey;
   } else if (keyOptions?.fieldLabelMap?.[defaultErrorKey]) {
     errorKey = keyOptions.fieldLabelMap[defaultErrorKey];
   }

tests/utils/errorUtils.test.ts

@@ -170,7 +170,8 @@ describe("generateMessageFromStringArray", () => {
 describe("generateFieldErrorFromErrorDetail", () => {
   const mockErrorDetail: ExceptionDetail = {
     title: ["Title is missing"],
-    questions: [{}, {}, {}, {answer: ["required"]}, {}]
+    questions: [{}, {}, {}, {answer: ["required"]}, {}],
+    additional_info: {name: ["This field is required"]}
   };
 
   it("should return correct field error", () => {
@@ -180,6 +181,22 @@ describe("generateFieldErrorFromErrorDetail", () => {
     );
     expect(fieldError).toBe(mockErrorDetail.title);
   });
+
+  it("should return correct field error when field's value is not string[]", () => {
+    const fieldError = generateFieldErrorFromErrorDetail(
+      "questions",
+      mockErrorDetail
+    );
+    expect(fieldError).toStrictEqual(["answer: required"]);
+  });
+
+  it("should return correct field error when a more spesific field error is wanted", () => {
+    const fieldError = generateFieldErrorFromErrorDetail(
+      "additional_info.name",
+      mockErrorDetail
+    );
+    expect(fieldError).toStrictEqual(["This field is required"]);
+  });
 });
 
 describe("getStringMessage", () => {
@@ -205,7 +222,9 @@ describe("getStringMessage", () => {
 
     it("should return first non empty object's string with beautified key when given option", () => {
       const message = getStringMessage(mockErrorDetail, {
-        shouldCapitalizeErrorKey: true
+        keyOptions: {
+          shouldCapitalizeErrorKey: true
+        }
       });
       expect(message).toBe(
         "Phone Number: The phone number entered is not valid."
@@ -214,31 +233,26 @@ describe("getStringMessage", () => {
 
     it("should both replace key and capitalize when options are given", () => {
       const message = getStringMessage(mockErrorDetail, {
-        fieldLabelMap: {
-          phone_number: "tel_no"
-        },
-        shouldCapitalizeErrorKey: true
+        keyOptions: {
+          fieldLabelMap: {
+            phone_number: "tel_no"
+          },
+          shouldCapitalizeErrorKey: true
+        }
       });
       expect(message).toBe("Tel No: The phone number entered is not valid.");
     });
 
-    it("should return first non empty object's string with given custom key", () => {
-      const message = getStringMessage(mockErrorDetail, {
-        customKey: "My Error"
-      });
-      expect(message).toBe("My Error: The phone number entered is not valid.");
-    });
-
     it("should not display error key in message when shouldHideErrorKey is true", () => {
       const message = getStringMessage(mockErrorDetail, {
-        shouldHideErrorKey: true
+        keyOptions: {shouldHideErrorKey: true}
       });
       expect(message).toBe("The phone number entered is not valid.");
     });
 
     it("should replace error key using fieldLabelMap", () => {
       const message = getStringMessage(mockErrorDetail, {
-        fieldLabelMap: {phone_number: "Custom Title"}
+        keyOptions: {fieldLabelMap: {phone_number: "Custom Title"}}
       });
       expect(message).toBe(
         "Custom Title: The phone number entered is not valid."
@@ -247,7 +261,7 @@ describe("getStringMessage", () => {
 
     it("should not replace error key if no matching value in fieldLabelMap", () => {
       const message = getStringMessage(mockErrorDetail, {
-        fieldLabelMap: {address: "Custom Title"}
+        keyOptions: {fieldLabelMap: {address: "Custom Title"}}
       });
       expect(message).toBe(
         "phone_number: The phone number entered is not valid."
@@ -265,6 +279,15 @@ describe("getStringMessage", () => {
       const message = getStringMessage(mockErrorDetail);
       expect(message).toBe("title: Title is missing");
     });
+
+    it("should not get affected by `fieldLabelMap`", () => {
+      const message = getStringMessage(mockErrorDetail, {
+        keyOptions: {
+          fieldLabelMap: {"": "Additional Documents"}
+        }
+      });
+      expect(message).toBe("title: Title is missing");
+    });
   });
 
   describe("when error detail is an object with `non_field_errors`", () => {
@@ -283,6 +306,15 @@ describe("getStringMessage", () => {
       const message = getStringMessage(mockErrorDetail);
       expect(message).toBe("Attachments or body must be provided.");
     });
+
+    it("should return non-field error message with key given in `fieldLabelMap`", () => {
+      const message = getStringMessage(mockErrorDetail, {
+        keyOptions: {fieldLabelMap: {"": "Additional Documents"}}
+      });
+      expect(message).toBe(
+        "Additional Documents: Attachments or body must be provided."
+      );
+    });
   });
 
   describe("when error detail is an array of strings", () => {
@@ -297,9 +329,48 @@ describe("getStringMessage", () => {
     });
 
     it("should add given key to the message and return", () => {
-      const key = "Title";
-      const message = getStringMessage(mockErrorDetail, {customKey: key});
-      expect(message).toBe(`${key}: ${mockErrorDetail[0]}`);
+      const message = getStringMessage(mockErrorDetail, {
+        keyOptions: {
+          fieldLabelMap: {"": "title"},
+          shouldCapitalizeErrorKey: true
+        }
+      });
+      expect(message).toBe("Title: This field can't be missing");
+    });
+  });
+
+  describe("when error detail is an object of an array of objects", () => {
+    const mockErrorDetail: ExceptionDetailValue = {
+      manual_part_process_quotes: [
+        {lead_time: ["Lead time is required."]},
+        {lead_time: ["Lead time is required."]}
+      ]
+    };
+
+    it("should return the first meaningful error", () => {
+      const message = getStringMessage(mockErrorDetail);
+
+      expect(message).toBe("lead_time: Lead time is required.");
+    });
+
+    it("should remove error key when `shouldHideErrorKey` is true", () => {
+      const message = getStringMessage(mockErrorDetail, {
+        keyOptions: {shouldHideErrorKey: true}
+      });
+
+      expect(message).toBe("Lead time is required.");
+    });
+
+    it("should replace error key using given `fieldLabelMap`", () => {
+      const message = getStringMessage(mockErrorDetail, {
+        keyOptions: {
+          fieldLabelMap: {
+            lead_time: "TIME"
+          }
+        }
+      });
+
+      expect(message).toBe("TIME: Lead time is required.");
     });
   });
 });