fix(jexl): don't consider the value of hidden fields in JEXL expr. (#198

) This fixes some subtle issue that occur when once field depends on multiple other fields: 1) The field's JEXL expression only doesn't have to be calculated, if _all_ fields it depends on are hidden (not just one!) 2) If some of the fields it depends on are hidden, those fields should behave as if they were unanswered, even if the have an answer in the database.
projectcaluma · May 6, 2019 · f934741 · f934741
1 parent e39ddc5
commit f934741
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 22 deletions.
diff --git a/addon/lib/document.js b/addon/lib/document.js
@@ -78,8 +78,21 @@ export default EmberObject.extend({
   }),
 
   findAnswer(slugWithPath) {
-    const result = this.findField(slugWithPath);
-    return result && result.answer.value;
+    const field = this.findField(slugWithPath);
+    if (!field || !field.answer) {
+      return null;
+    }
+
+    // Multiple choice questions should return an empty array if there is no answer
+    // otherwise `intersects` operator breaks
+    const emptyValue =
+      field.question.__typename == "MultipleChoiceQuestion" ? [] : null;
+
+    if (field.answer.value && !field.question.hidden) {
+      return field.answer.value;
+    }
+
+    return emptyValue;
   },
 
   findField(slugWithPath) {

diff --git a/addon/lib/question.js b/addon/lib/question.js
@@ -60,12 +60,14 @@ export default EmberObject.extend({
    * @return {Boolean}
    */
   hiddenTask: task(function*() {
-    let hidden = this.dependsOn.some(
-      field =>
-        field.question.hidden ||
-        field.answer.value === null ||
-        field.answer.value === undefined
-    );
+    let hidden =
+      this.dependsOn.length &&
+      this.dependsOn.every(
+        field =>
+          field.question.hidden ||
+          field.answer.value === null ||
+          field.answer.value === undefined
+      );
 
     hidden =
       hidden || (yield this.field.document.questionJexl.eval(this.isHidden));

diff --git a/tests/unit/lib/document-test.js b/tests/unit/lib/document-test.js
@@ -43,7 +43,7 @@ module("Unit | Library | document", function(hooks) {
                 slug: "question-2",
                 label: "Question 2",
                 isRequired: "false",
-                isHidden: "'question-1'|answer == 'magic'",
+                isHidden: "!('question-1'|answer == 'show-question-2')",
                 __typename: "TextQuestion"
               }
             },
@@ -52,7 +52,8 @@ module("Unit | Library | document", function(hooks) {
                 slug: "question-3",
                 label: "Question 3",
                 isRequired: "false",
-                isHidden: "'question-2'|answer == 'Harry Potter'",
+                isHidden:
+                  "!('question-1'|answer == 'show-question-3' || 'question-2'|answer == 'show-question-3')",
                 __typename: "TextQuestion"
               }
             }
@@ -84,8 +85,8 @@ module("Unit | Library | document", function(hooks) {
 
   test("it recomputes isHidden on value change of dependency", async function(assert) {
     assert.expect(1);
-    await this.setFieldValue("question-1", "foo");
-    await this.setFieldValue("question-2", "Harry Potter");
+    await this.setFieldValue("question-1", "show-question-2");
+    await this.setFieldValue("question-2", "foo");
     assert.deepEqual(this.getDocumentHiddenState(), [
       ["question-1", false],
       ["question-2", false],
@@ -94,26 +95,22 @@ module("Unit | Library | document", function(hooks) {
   });
 
   test("it recomputes isHidden on isHidden change of dependency", async function(assert) {
-    assert.expect(3);
-    await this.setFieldValue("question-1", "foo");
-    await this.setFieldValue("question-2", "bar");
+    assert.expect(2);
+    await this.setFieldValue("question-1", "show-question-2");
+    await this.setFieldValue("question-2", "show-question-3");
     assert.deepEqual(this.getDocumentHiddenState(), [
       ["question-1", false],
       ["question-2", false],
       ["question-3", false]
     ]);
-    await this.setFieldValue("question-1", "magic");
+    await this.setFieldValue("question-1", "foo");
+
+    // since question 2 is hidden, it's value is not considered in question 3's jexl.
     assert.deepEqual(this.getDocumentHiddenState(), [
       ["question-1", false],
       ["question-2", true],
       ["question-3", true]
     ]);
-    await this.setFieldValue("question-1", "foo");
-    assert.deepEqual(this.getDocumentHiddenState(), [
-      ["question-1", false],
-      ["question-2", false],
-      ["question-3", false]
-    ]);
   });
 
   test("can do cross-form path traversal", async function(assert) {