fix(validation): the requiredness check needs local context

When validating row documents, a question may be "visible" in another
row and hidden in the current one. However the `visible_questions` list
is global and is used mainly to speed up database access.

When evaluating requiredness, we also need to check if the question is
visible *locally*.

caluma/caluma_form/jexl.py

@@ -91,7 +91,7 @@ def _question(self, slug):
         )
 
     @contextmanager
-    def _question_local_structure(self, question_slug):
+    def use_question_context(self, question_slug):
         """Context manger to temporarily overwrite self._structure.
 
         This is used so we can evaluate each JEXL expression in the context
@@ -171,12 +171,13 @@ def is_hidden(self, question):
         # if the question is visible-in-context and not hidden by invisible dependencies,
         # we can evaluate it's own is_hidden expression
 
-        with self._question_local_structure(question.pk):
+        with self.use_question_context(question.pk):
             self._cache["hidden"][cache_key] = self.evaluate(question.is_hidden)
         return self._cache["hidden"][cache_key]
 
-    def is_required(self, question):
-        cache_key = self._cache_key(question.pk)
+    def is_required(self, question_field):
+        cache_key = (question_field.document.pk, question_field.question.pk)
+        question = question_field.question
 
         if cache_key in self._cache["required"]:
             return self._cache["required"][cache_key]
@@ -188,7 +189,7 @@ def is_required(self, question):
             # so assume requiredness to be False
             ret = False
         else:
-            with self._question_local_structure(question.pk):
+            with self.use_question_context(question.pk):
                 ret = self.evaluate(question.is_required)
         self._cache["required"][cache_key] = ret
         return ret

caluma/caluma_form/tests/test_jexl.py

@@ -83,7 +83,7 @@ def test_all_deps_hidden(db, form, document_factory, form_question_factory):
         }
     )
     assert qj.is_hidden(q2)
-    assert not qj.is_required(q2)
+    assert not qj.is_required(structure.Field(document, document.form, q2))
 
 
 @pytest.mark.parametrize("fq_is_hidden", ["true", "false"])
@@ -349,27 +349,45 @@ def test_answer_transform_on_hidden_question_types(
     assert qj.is_hidden(questions["form_question"])
 
 
-def test_answer_transform_in_tables(info, form_and_document, form_question_factory):
-    # Bug test: When a JEXL expression references two answers, and one of them
-    # is hidden, the `answer` transform must return None for the hidden question
-    # to ensure correct evaluation.
+@pytest.mark.parametrize(
+    "jexl_field,expr",
+    [
+        ("is_required", "('column'|answer == 5)"),
+        ("is_hidden", "('column'|answer > 10)"),
+    ],
+)
+def test_answer_transform_in_tables(
+    info,
+    form_and_document,
+    form_question_factory,
+    jexl_field,
+    expr,
+    answer_document_factory,
+):
 
     form, document, questions, answers = form_and_document(
         use_table=True, use_subform=False
     )
 
     table_question = questions["table_question"]
-    top_ans = answers["top_question"].value
 
-    form_question_factory(
-        form=table_question.row_form,
-        question__slug="column2",
-        question__is_required=f"'column'|answer == 5 && 'top_question'|answer == '{top_ans}'",
+    col2_question = form_question_factory(
+        **{
+            "form": table_question.row_form,
+            "question__slug": "column2",
+            "question__is_hidden": "false",
+            "question__is_required": "true",
+            # this overrwrites above "default values"
+            f"question__{jexl_field}": expr,
+        }
     ).question
+    assert getattr(col2_question, jexl_field) == expr
 
     table_answer = answers["table_question"]
 
-    row2_doc = table_answer.documents.create(form=table_question.row_form)
+    row2_doc = answer_document_factory(
+        answer=table_answer, document__form=table_question.row_form, sort=10
+    ).document
 
     # Second row has an answer that triggers the transform and implies a validation error.
     # This will wrongfully succeed if the answer value comes from the wrong row

caluma/caluma_form/validators.py

@@ -265,7 +265,6 @@ def visible_questions(self, document, validation_context=None):
             except (jexl.QuestionMissing, exceptions.ValidationError):
                 raise
             except Exception as exc:
-
                 log.error(
                     f"Error while evaluating `is_hidden` expression on question {question.slug}: "
                     f"{question.is_hidden}: {str(exc)}"
@@ -292,11 +291,19 @@ def _validate_required(self, validation_context):  # noqa: C901
             question = field.question
             try:
                 is_hidden = question.slug not in validation_context["visible_questions"]
-
                 if is_hidden:
                     continue
 
-                is_required = q_jexl.is_required(question)
+                # The above `is_hidden` is globally cached per question, mainly to optimize DB access.
+                # This means a question could be marked "visible" as it would be visible
+                # in another row, but would still be hidden in the local row, if this is a
+                # table question context.  Thus, in this case we need to re-evaluate it's
+                # hiddenness. Luckily, the JEXL evaluator caches those values (locally).
+                with q_jexl.use_question_context(question.pk):
+                    if q_jexl.is_hidden(question):
+                        continue
+
+                is_required = q_jexl.is_required(field)
 
                 if question.type == Question.TYPE_FORM:
                     # form questions's answers are still in the top level document