From f9045f8c0b32afc64c4452c068021fd5d8e1842b Mon Sep 17 00:00:00 2001
From: "mathias.chouet" <mathias.chouet@irstea.fr>
Date: Wed, 21 Aug 2019 17:52:39 +0200
Subject: [PATCH] Fix #264 - better error messages on file loading error

---
 e2e/load-malformed-files.e2e-spec.ts          |  82 +++++++++++++
 e2e/session/session-bad-syntax.json           | 102 ++++++++++++++++
 e2e/session/session-empty-modules-list.json   |   9 ++
 e2e/session/session-format-too-old.json       | 112 ++++++++++++++++++
 e2e/session/session-missing-info.json         | 103 ++++++++++++++++
 e2e/sidenav.po.ts                             |   6 +-
 protractor.conf.js                            |   2 +-
 .../dialog-load-session.component.html        |   4 +-
 .../dialog-load-session.component.scss        |   4 +-
 .../dialog-load-session.component.ts          |  57 +++++++--
 src/locale/messages.en.json                   |   2 +
 src/locale/messages.fr.json                   |   2 +
 12 files changed, 470 insertions(+), 15 deletions(-)
 create mode 100644 e2e/load-malformed-files.e2e-spec.ts
 create mode 100644 e2e/session/session-bad-syntax.json
 create mode 100644 e2e/session/session-empty-modules-list.json
 create mode 100644 e2e/session/session-format-too-old.json
 create mode 100644 e2e/session/session-missing-info.json

diff --git a/e2e/load-malformed-files.e2e-spec.ts b/e2e/load-malformed-files.e2e-spec.ts
new file mode 100644
index 000000000..d561129dc
--- /dev/null
+++ b/e2e/load-malformed-files.e2e-spec.ts
@@ -0,0 +1,82 @@
+import { AppPage } from "./app.po";
+import { Navbar } from "./navbar.po";
+import { SideNav } from "./sidenav.po";
+import { browser, element, by } from "protractor";
+
+/**
+ * Load 4 malformed session files :
+ *  - one having an format version too old
+ *  - one having an empty list of modules
+ *  - one having missing mandatory data (ex: parameter symbol)
+ *  - one having malformed JSON syntax
+ * @WARNING error messages are tested in french
+ */
+describe("ngHyd − load malformed session files − ", () => {
+  let startPage: AppPage;
+  let navbar: Navbar;
+  let sidenav: SideNav;
+
+  function init() {
+      startPage = new AppPage();
+      navbar = new Navbar();
+      sidenav = new SideNav();
+  }
+  beforeEach(init);
+
+   it("when loading session-bad-syntax.json, displayed error should be relevant", async () => {
+    await startPage.navigateTo();
+
+    await navbar.clickMenuButton();
+    await browser.sleep(200);
+
+    await sidenav.clickLoadSessionButton();
+    await browser.sleep(200);
+
+    await sidenav.loadSessionFile("./session/session-bad-syntax.json");
+    const err = element(by.css(".file-problem .mat-list-item-content"));
+    expect(await err.getText()).toContain("La syntaxe du fichier semble incorrecte");
+  });
+
+  it("when loading session-missing-info.json, displayed error should be relevant", async () => {
+   await startPage.navigateTo();
+
+   await navbar.clickMenuButton();
+   await browser.sleep(200);
+
+   await sidenav.clickLoadSessionButton();
+   await browser.sleep(200);
+
+   await sidenav.loadSessionFile("./session/session-missing-info.json");
+   const err = element(by.css(".file-problem .mat-list-item-content"));
+   expect(await err.getText()).toContain("La syntaxe du fichier semble incorrecte");
+ });
+
+ it("when loading session-empty-modules-list.json, displayed error should be relevant", async () => {
+  await startPage.navigateTo();
+
+  await navbar.clickMenuButton();
+  await browser.sleep(200);
+
+  await sidenav.clickLoadSessionButton();
+  await browser.sleep(200);
+
+  await sidenav.loadSessionFile("./session/session-empty-modules-list.json");
+  const err = element(by.css(".file-problem .mat-list-item-content"));
+  expect(await err.getText()).toContain("Le fichier ne contient aucun module");
+});
+
+it("when loading session-format-too-old.json, displayed error should be relevant", async () => {
+ await startPage.navigateTo();
+
+ await navbar.clickMenuButton();
+ await browser.sleep(200);
+
+ await sidenav.clickLoadSessionButton();
+ await browser.sleep(200);
+
+ await sidenav.loadSessionFile("./session/session-format-too-old.json", false);
+ const err = element(by.css(".file-problem .mat-list-item-content"));
+ expect(await err.getText()).toContain("Mauvaise version du format de fichier");
+});
+
+});
diff --git a/e2e/session/session-bad-syntax.json b/e2e/session/session-bad-syntax.json
new file mode 100644
index 000000000..c2de57a8d
--- /dev/null
+++ b/e2e/session/session-bad-syntax.json
@@ -0,0 +1,102 @@
+{
+    "header": {
+        "source": "jalhyd",
+        "format_version": "1.1",
+        "created": "2019-08-21T13:20:54.284Z"
+    },
+    "session": [
+        {
+            "children": [
+                {
+                    "uid": "NnF4NT",
+                    "props": {
+                        "calcType": "Structure",
+                        "structureType": "SeuilRectangulaire",
+                        "loiDebit": "WeirSubmergedLarinier"
+                    },
+                    "children": [],
+                    "parameters": [
+                            "symbol": "h1",
+                            "mode": "SINGLE",
+                            "value": 1
+                        },
+                        {
+                            "symbol": "L",
+                            "mode": "MINMAX",
+                            "min": 0.1,
+                            "max": 0.4,
+                            "step": 0.015000000000000003,
+                            "extensionStrategy": 0
+                        },
+                        {
+                            "symbol": "CdWSL",
+                            "mode": "SINGLE",
+                            "value": 0.65
+                        }
+                    ]
+                },
+                {
+                    "uid": "c3ZreG",
+                    "props": {
+                        "calcType": "Structure",
+                        "structureType": "SeuilRectangulaire",
+                        "loiDebit": "WeirSubmergedLarinier"
+                    },
+                    "children": [],
+                    "parameters": [
+                        {
+                            "symbol": "h1",
+                            "mode": "SINGLE",
+                            "value": 1
+                        },
+                        {
+                            "symbol": "L",
+                            "mode": "MINMAX",
+                            "min": 1.1,
+                            "max": 1.4,
+                            "step": 0.015,
+                            "extensionStrategy": 0
+                        },
+                        {
+                            "symbol": "CdWSL",
+                            "mode": "SINGLE",
+                            "value": 0.65
+                        }
+                    ]
+                }
+            ],
+            "parameters": [
+                {
+                    "symbol": "Q",
+                    "mode": "CALCUL"
+                },
+                {
+                    "symbol": "Z1",
+                    "mode": "SINGLE",
+                    "value": 102
+                },
+                {
+                    "value": 10
+                },
+                {
+                    "symbol": "BB",
+                    "mode": "SINGLE",
+                    "value": 1
+                },
+                {
+                    "symbol": "PB",
+                    "mode": "SINGLE",
+                    "value": 1
+                },
+                {
+                    "symbol": "DH",
+                    "mode": "MINMAX",
+                    "min": 0.25,
+                    "max": 1,
+                    "step": 0.0375,
+                    "extensionStrategy": 0
+                }
+            ]
+        }
+    ]
+}
\ No newline at end of file
diff --git a/e2e/session/session-empty-modules-list.json b/e2e/session/session-empty-modules-list.json
new file mode 100644
index 000000000..3a5bbefdd
--- /dev/null
+++ b/e2e/session/session-empty-modules-list.json
@@ -0,0 +1,9 @@
+{
+    "header": {
+        "source": "jalhyd",
+        "format_version": "1.1",
+        "created": "2019-08-21T13:20:54.284Z"
+    },
+    "session": [
+    ]
+}
\ No newline at end of file
diff --git a/e2e/session/session-format-too-old.json b/e2e/session/session-format-too-old.json
new file mode 100644
index 000000000..31cf0b0a8
--- /dev/null
+++ b/e2e/session/session-format-too-old.json
@@ -0,0 +1,112 @@
+{
+    "header": {
+        "source": "jalhyd",
+        "format_version": "1.0.3b",
+        "created": "2019-08-21T13:20:54.284Z"
+    },
+    "session": [
+        {
+            "uid": "emU1Nm",
+            "props": {
+                "calcType": "Cloisons"
+            },
+            "meta": {
+                "title": "Cloisons"
+            },
+            "children": [
+                {
+                    "uid": "NnF4NT",
+                    "props": {
+                        "calcType": "Structure",
+                        "structureType": "SeuilRectangulaire",
+                        "loiDebit": "WeirSubmergedLarinier"
+                    },
+                    "children": [],
+                    "parameters": [
+                        {
+                            "symbol": "h1",
+                            "mode": "SINGLE",
+                            "value": 1
+                        },
+                        {
+                            "symbol": "L",
+                            "mode": "MINMAX",
+                            "min": 0.1,
+                            "max": 0.4,
+                            "step": 0.015000000000000003,
+                            "extensionStrategy": 0
+                        },
+                        {
+                            "symbol": "CdWSL",
+                            "mode": "SINGLE",
+                            "value": 0.65
+                        }
+                    ]
+                },
+                {
+                    "uid": "c3ZreG",
+                    "props": {
+                        "calcType": "Structure",
+                        "structureType": "SeuilRectangulaire",
+                        "loiDebit": "WeirSubmergedLarinier"
+                    },
+                    "children": [],
+                    "parameters": [
+                        {
+                            "symbol": "h1",
+                            "mode": "SINGLE",
+                            "value": 1
+                        },
+                        {
+                            "symbol": "L",
+                            "mode": "MINMAX",
+                            "min": 1.1,
+                            "max": 1.4,
+                            "step": 0.015,
+                            "extensionStrategy": 0
+                        },
+                        {
+                            "symbol": "CdWSL",
+                            "mode": "SINGLE",
+                            "value": 0.65
+                        }
+                    ]
+                }
+            ],
+            "parameters": [
+                {
+                    "symbol": "Q",
+                    "mode": "CALCUL"
+                },
+                {
+                    "symbol": "Z1",
+                    "mode": "SINGLE",
+                    "value": 102
+                },
+                {
+                    "symbol": "LB",
+                    "mode": "SINGLE",
+                    "value": 10
+                },
+                {
+                    "symbol": "BB",
+                    "mode": "SINGLE",
+                    "value": 1
+                },
+                {
+                    "symbol": "PB",
+                    "mode": "SINGLE",
+                    "value": 1
+                },
+                {
+                    "symbol": "DH",
+                    "mode": "MINMAX",
+                    "min": 0.25,
+                    "max": 1,
+                    "step": 0.0375,
+                    "extensionStrategy": 0
+                }
+            ]
+        }
+    ]
+}
\ No newline at end of file
diff --git a/e2e/session/session-missing-info.json b/e2e/session/session-missing-info.json
new file mode 100644
index 000000000..fa6003940
--- /dev/null
+++ b/e2e/session/session-missing-info.json
@@ -0,0 +1,103 @@
+{
+    "header": {
+        "source": "jalhyd",
+        "format_version": "1.1",
+        "created": "2019-08-21T13:20:54.284Z"
+    },
+    "session": [
+        {
+            "children": [
+                {
+                    "uid": "NnF4NT",
+                    "props": {
+                        "calcType": "Structure",
+                        "structureType": "SeuilRectangulaire",
+                        "loiDebit": "WeirSubmergedLarinier"
+                    },
+                    "children": [],
+                    "parameters": [
+                        {
+                            "symbol": "h1",
+                            "mode": "SINGLE",
+                            "value": 1
+                        },
+                        {
+                            "symbol": "L",
+                            "mode": "MINMAX",
+                            "min": 0.1,
+                            "max": 0.4,
+                            "step": 0.015000000000000003,
+                            "extensionStrategy": 0
+                        },
+                        {
+                            "symbol": "CdWSL",
+                            "mode": "SINGLE",
+                            "value": 0.65
+                        }
+                    ]
+                },
+                {
+                    "uid": "c3ZreG",
+                    "props": {
+                        "calcType": "Structure",
+                        "structureType": "SeuilRectangulaire",
+                        "loiDebit": "WeirSubmergedLarinier"
+                    },
+                    "children": [],
+                    "parameters": [
+                        {
+                            "symbol": "h1",
+                            "mode": "SINGLE",
+                            "value": 1
+                        },
+                        {
+                            "symbol": "L",
+                            "mode": "MINMAX",
+                            "min": 1.1,
+                            "max": 1.4,
+                            "step": 0.015,
+                            "extensionStrategy": 0
+                        },
+                        {
+                            "symbol": "CdWSL",
+                            "mode": "SINGLE",
+                            "value": 0.65
+                        }
+                    ]
+                }
+            ],
+            "parameters": [
+                {
+                    "symbol": "Q",
+                    "mode": "CALCUL"
+                },
+                {
+                    "symbol": "Z1",
+                    "mode": "SINGLE",
+                    "value": 102
+                },
+                {
+                    "value": 10
+                },
+                {
+                    "symbol": "BB",
+                    "mode": "SINGLE",
+                    "value": 1
+                },
+                {
+                    "symbol": "PB",
+                    "mode": "SINGLE",
+                    "value": 1
+                },
+                {
+                    "symbol": "DH",
+                    "mode": "MINMAX",
+                    "min": 0.25,
+                    "max": 1,
+                    "step": 0.0375,
+                    "extensionStrategy": 0
+                }
+            ]
+        }
+    ]
+}
\ No newline at end of file
diff --git a/e2e/sidenav.po.ts b/e2e/sidenav.po.ts
index 2b3f469d4..2625022bb 100644
--- a/e2e/sidenav.po.ts
+++ b/e2e/sidenav.po.ts
@@ -21,10 +21,12 @@ export class SideNav {
     await ncb.click();
   }
 
-  async loadSessionFile(file: string) {
+  async loadSessionFile(file: string, click: boolean = true) {
     const absolutePath = path.resolve(__dirname, file);
     const input = this.getFileInput();
     await input.sendKeys(absolutePath);
-    await this.getFileLoadButton().click();
+    if (click) {
+      await this.getFileLoadButton().click();
+    }
   }
 }
diff --git a/protractor.conf.js b/protractor.conf.js
index 6c546c716..4c1db58bb 100644
--- a/protractor.conf.js
+++ b/protractor.conf.js
@@ -16,7 +16,7 @@ exports.config = {
   capabilities: {
     browserName: 'chrome',
     chromeOptions: {
-      args: [ "--headless", "--window-size=1024x768" ],
+      // args: [ "--headless", "--window-size=1024x768" ],
       prefs: {
         download: {
             prompt_for_download: false, 
diff --git a/src/app/components/dialog-load-session/dialog-load-session.component.html b/src/app/components/dialog-load-session/dialog-load-session.component.html
index 84ea1c729..ba088e2bf 100644
--- a/src/app/components/dialog-load-session/dialog-load-session.component.html
+++ b/src/app/components/dialog-load-session/dialog-load-session.component.html
@@ -28,10 +28,10 @@
       </button>
     </div>
 
-    <div class="format-problem" *ngIf="fileFormatVersionProblem">
+    <div class="file-problem" *ngIf="fileProblem">
       <mat-list role="list">
         <mat-list-item role="listitem">
-          <mat-icon color="warn">error_outline</mat-icon> {{ uitextFileFormatVersionProblem }}
+          <mat-icon color="warn">error_outline</mat-icon> {{ uitextFileProblem }}
         </mat-list-item>
       </mat-list>
     </div>
diff --git a/src/app/components/dialog-load-session/dialog-load-session.component.scss b/src/app/components/dialog-load-session/dialog-load-session.component.scss
index 38df56667..9652d3fb1 100644
--- a/src/app/components/dialog-load-session/dialog-load-session.component.scss
+++ b/src/app/components/dialog-load-session/dialog-load-session.component.scss
@@ -18,11 +18,11 @@ mat-form-field {
     }
 }
 
-.format-problem {
+.file-problem {
     margin-bottom: 1em;
 }
 
-.dependencies-problems, .format-problem {
+.dependencies-problems, .file-problem {
 
     .mat-list-base {
         padding-top: 0;
diff --git a/src/app/components/dialog-load-session/dialog-load-session.component.ts b/src/app/components/dialog-load-session/dialog-load-session.component.ts
index 382e00cdb..2cedc126b 100644
--- a/src/app/components/dialog-load-session/dialog-load-session.component.ts
+++ b/src/app/components/dialog-load-session/dialog-load-session.component.ts
@@ -31,6 +31,12 @@ export class DialogLoadSessionComponent {
 
     public emptyCurrentSession = false;
 
+    /** flag showing that a file loading attempt has failed */
+    private loadingError = false;
+
+    /** flag showing that a file loading attempt has succeeded */
+    private loadingComplete = false;
+
     constructor(
       public dialogRef: MatDialogRef<DialogLoadSessionComponent>,
       private intlService: I18nService,
@@ -135,6 +141,12 @@ export class DialogLoadSessionComponent {
     public onFileSelected(event: any) {
       if (event.target.files && event.target.files.length) {
         this.file = event.target.files[0];
+        // reinit file infos
+        this.calculators = [];
+        this.fileFormatVersion = "";
+        // reinit flags
+        this.loadingError = false;
+        this.loadingComplete = false;
 
         const formService = ServiceFactory.instance.formulaireService;
         formService.calculatorInfosFromSessionFile(this.file).then(
@@ -148,8 +160,12 @@ export class DialogLoadSessionComponent {
                 n.title = decode(formService.getLocalisedShortTitleFromCalculatorType(n.type));
               }
             }
+            this.loadingComplete = true;
           }
-        );
+        ).catch((err) => {
+          console.error(err);
+          this.loadingError = true;
+        });
       }
     }
 
@@ -169,10 +185,27 @@ export class DialogLoadSessionComponent {
       return ok;
     }
 
-    public get fileFormatVersionProblem(): boolean {
+    /**
+     * @returns true if any problem occurred during file loading
+     */
+    public get fileProblem(): boolean {
+      return (
+        this.loadingError
+        ||
+        this.fileFormatVersionProblem()
+        ||
+        this.fileIsEmpty()
+      );
+    }
+
+    private fileFormatVersionProblem(): boolean {
       return this.fileFormatVersion && (this.fileFormatVersion !== this.libFormatVersion);
     }
 
+    private fileIsEmpty(): boolean {
+      return this.loadingComplete && this.calculators.length === 0;
+    }
+
     public get uitextLoad() {
       return this.intlService.localizeText("INFO_OPTION_LOAD");
     }
@@ -205,11 +238,19 @@ export class DialogLoadSessionComponent {
       return this.intlService.localizeText("INFO_DIALOG_EMPTY_CURRENT_SESSION");
     }
 
-    public get uitextFileFormatVersionProblem() {
-      return sprintf(
-        this.intlService.localizeText("INFO_DIALOG_FORMAT_VERSIONS_MISMATCH"),
-        this.fileFormatVersion,
-        this.libFormatVersion
-      );
+    public get uitextFileProblem() {
+      if (this.loadingError) {
+        return this.intlService.localizeText("INFO_DIALOG_ERROR_LOADING_FILE");
+      }
+      if (this.fileFormatVersionProblem()) {
+        return sprintf(
+          this.intlService.localizeText("INFO_DIALOG_FORMAT_VERSIONS_MISMATCH"),
+          this.fileFormatVersion,
+          this.libFormatVersion
+        );
+      }
+      if (this.fileIsEmpty()) {
+        return this.intlService.localizeText("INFO_DIALOG_FILE_IS_EMPTY");
+      }
     }
 }
diff --git a/src/locale/messages.en.json b/src/locale/messages.en.json
index 152d21b6c..5e8589a68 100644
--- a/src/locale/messages.en.json
+++ b/src/locale/messages.en.json
@@ -78,6 +78,8 @@
     "INFO_DIALOG_EDIT_PAB_OPTION_INTERPOLATE": "Interpolate",
     "INFO_DIALOG_EDIT_PAB_TITLE": "Edit values",
     "INFO_DIALOG_EMPTY_CURRENT_SESSION": "Empty current session",
+    "INFO_DIALOG_ERROR_LOADING_FILE": "File syntax seems incorrect",
+    "INFO_DIALOG_FILE_IS_EMPTY": "File contains no module",
     "INFO_DIALOG_FIX_MISSING_DEPENDENCIES": "Fix missing dependencies",
     "INFO_DIALOG_FORMAT_VERSIONS_MISMATCH": "File format versions mismatch (file: %s, jalhyd: %s)",
     "INFO_DIALOG_LOAD_SESSION_FILENAME": "Choose a file",
diff --git a/src/locale/messages.fr.json b/src/locale/messages.fr.json
index 60e15cb86..c28d769f0 100644
--- a/src/locale/messages.fr.json
+++ b/src/locale/messages.fr.json
@@ -78,6 +78,8 @@
     "INFO_DIALOG_EDIT_PAB_OPTION_INTERPOLATE": "Interpoler",
     "INFO_DIALOG_EDIT_PAB_TITLE": "Modifier les valeurs",
     "INFO_DIALOG_EMPTY_CURRENT_SESSION": "Vider la session courante",
+    "INFO_DIALOG_ERROR_LOADING_FILE": "La syntaxe du fichier semble incorrecte",
+    "INFO_DIALOG_FILE_IS_EMPTY": "Le fichier ne contient aucun module",
     "INFO_DIALOG_FIX_MISSING_DEPENDENCIES": "Résoudre les dépendances",
     "INFO_DIALOG_FORMAT_VERSIONS_MISMATCH": "Mauvaise version du format de fichier (fichier: %s, jalhyd: %s)",
     "INFO_DIALOG_LOAD_SESSION_FILENAME": "Choisir un fichier",
-- 
GitLab