From 9c9350db5e7da00f4d29a57ee77fd4f6bb6e7f84 Mon Sep 17 00:00:00 2001
From: Christian Zosel <christian@zosel.ch>
Date: Sat, 4 May 2019 15:42:27 +0200
Subject: [PATCH] fix(jexl): don't consider the value of hidden fields in JEXL
 expr.

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.
---
 addon/lib/document.js           | 17 +++++++++++++++--
 addon/lib/question.js           | 14 ++++++++------
 tests/unit/lib/document-test.js | 25 +++++++++++--------------
 3 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/addon/lib/document.js b/addon/lib/document.js
index 85a784936..427b88b43 100644
--- 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
index 7103723f9..1b291823a 100644
--- 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
index 557a10366..2251e8cfc 100644
--- 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) {