diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index de6cc6370..a8c1d09c7 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -2,19 +2,19 @@ repos: - repo: local hooks: - id: ruff-format - stages: [commit] + stages: [pre-commit] name: format code language: system entry: ruff format . types: [python] - id: ruff-check - stages: [commit] + stages: [pre-commit] name: check format,import language: system - entry: ruff check . + entry: ruff check --fix . types: [python] - id: reuse - stages: [commit] + stages: [pre-commit] name: reuse description: Check licenses entry: reuse lint diff --git a/.reuse/dep5 b/.reuse/dep5 index adcb533dc..20e89b1a4 100644 --- a/.reuse/dep5 +++ b/.reuse/dep5 @@ -33,6 +33,7 @@ License: CC0-1.0 Files: caluma/*.py caluma/*.ambr + caluma/*.json Copyright: 2019 Adfinis AG <info@adfinis.com> License: GPL-3.0-or-later diff --git a/caluma/caluma_core/exceptions.py b/caluma/caluma_core/exceptions.py new file mode 100644 index 000000000..c40e2e9e7 --- /dev/null +++ b/caluma/caluma_core/exceptions.py @@ -0,0 +1,16 @@ +# Some common exceptions + + +class ConfigurationError(Exception): + """Invalid configuration detected. + + Use this exception type if a configuration does not make + sense or is generally un-processable. + + For example: circular dependencies in JEXL expressions, + invalid form hierarchies etc + """ + + +class QuestionMissing(Exception): + pass diff --git a/caluma/caluma_core/jexl.py b/caluma/caluma_core/jexl.py index 27c797bc9..a38d444ab 100644 --- a/caluma/caluma_core/jexl.py +++ b/caluma/caluma_core/jexl.py @@ -167,6 +167,11 @@ def _length_transform(self, value, *options): return None def evaluate(self, expression, context=None): + # log.info( + # "JEXL: evaluating expression <<< %s >>> in context: %s", + # str(expression), + # str(dict(context)), + # ) self._expr_stack.append(expression) try: return super().evaluate(expression, context) @@ -243,18 +248,3 @@ def visit_Transform(self, transform): yield arg.value yield from self.generic_visit(transform) - - -class ExtractTransformSubjectAndArgumentsAnalyzer(CalumaAnalyzer): - """ - Extract all referenced subjects and arguments of a given transforms. - - If no transforms are given all references of all transforms will be extracted. - """ - - def visit_Transform(self, transform): - if not self.transforms or transform.name in self.transforms: - if not isinstance(transform.subject, type(transform)): - yield (transform.subject.value, transform.args) - - yield from self.generic_visit(transform) diff --git a/caluma/caluma_form/domain_logic.py b/caluma/caluma_form/domain_logic.py index 83c255618..7eb766b55 100644 --- a/caluma/caluma_form/domain_logic.py +++ b/caluma/caluma_form/domain_logic.py @@ -1,4 +1,5 @@ from graphlib import TopologicalSorter +from logging import getLogger from typing import Optional from django.db import transaction @@ -8,10 +9,14 @@ from caluma.caluma_core.models import BaseModel from caluma.caluma_core.relay import extract_global_id from caluma.caluma_form import models, structure, utils, validators -from caluma.caluma_form.utils import update_or_create_calc_answer +from caluma.caluma_form.utils import ( + recalculate_field, +) from caluma.caluma_user.models import BaseUser from caluma.utils import update_model +log = getLogger(__name__) + class BaseLogic: @staticmethod @@ -153,17 +158,48 @@ def post_save(answer: models.Answer) -> models.Answer: return answer @staticmethod - def update_calc_dependents(answer): - if not answer.question.calc_dependents: + def update_calc_dependents(answer, update_info): + is_table = update_info and answer.question.type == models.Question.TYPE_TABLE + if not is_table and not answer.question.calc_dependents: + return + if not answer.document: + # Default answer return + log.debug("update_calc_dependents(%s)", answer) root_doc = utils.prefetch_document(answer.document.family_id) - struc = structure.FieldSet(root_doc, root_doc.form) - - for question in models.Question.objects.filter( - pk__in=answer.question.calc_dependents - ): - update_or_create_calc_answer(question, root_doc, struc) + struc = structure.FieldSet(root_doc) + + field = struc.find_field_by_answer(answer) + if is_table: + # We treat all children of the table as "changed" + # Find all children, and if any are of type calc (and are in the "created" + # update info bit), recalculate them. + for child in field.get_all_fields(): + # TODO: if we're in a table row, how do we know that a dependant is + # *only* inside the table and therefore only *our* row needs + # *recalculating? + if ( + child.question.type == models.Question.TYPE_CALCULATED_FLOAT + and child.parent._document.pk in update_info["created"] + ): + recalculate_field(child) + + for dep_slug in field.question.calc_dependents or []: + # ... maybe this is enough? Cause this will find the closest "match", + # going only to the outer context from a table if the field is not found + # inside of it + for dep_field in struc.find_all_fields_by_slug(dep_slug): + # Need to iterate, because a calc question could reside *inside* + # a table row, so there could be multiple of them, all needing to + # be updated because of "our" change + + log.debug( + "update_calc_dependents(%s): updating question %s", + answer, + dep_field.question.pk, + ) + recalculate_field(dep_field) @classmethod @transaction.atomic @@ -179,10 +215,11 @@ def create( if validated_data["question"].type == models.Question.TYPE_FILES: cls.update_answer_files(answer, files) + update_info = None if answer.question.type == models.Question.TYPE_TABLE: - answer.create_answer_documents(documents) + update_info = answer.create_answer_documents(documents) - cls.update_calc_dependents(answer) + cls.update_calc_dependents(answer, update_info) return answer @@ -198,10 +235,11 @@ def update(cls, answer, validated_data, user: Optional[BaseUser] = None): BaseLogic.update(answer, validated_data, user) + update_info = None if answer.question.type == models.Question.TYPE_TABLE: - answer.create_answer_documents(documents) + update_info = answer.create_answer_documents(documents) - cls.update_calc_dependents(answer) + cls.update_calc_dependents(answer, update_info) answer.refresh_from_db() return answer @@ -303,13 +341,15 @@ def _initialize_calculated_answers(document): """ Initialize all calculated questions in the document. - In order to do this efficiently, we get all calculated questions with their dependents, - sort them topoligically, and then update their answer. + In order to do this efficiently, we get all calculated questions with + their dependents, sort them topoligically, and then update their answer. """ root_doc = utils.prefetch_document(document.family_id) - struc = structure.FieldSet(root_doc, root_doc.form) + struc = structure.FieldSet(root_doc) calculated_questions = ( + # TODO we could fetch those as fields from the structure. Minus a DB query, + # and we'd already have the fields as well models.Form.get_all_questions([(document.family or document).form_id]) .filter(type=models.Question.TYPE_CALCULATED_FLOAT) .values("slug", "calc_dependents") @@ -323,14 +363,10 @@ def _initialize_calculated_answers(document): # just reverse the resulting order. sorted_question_slugs = list(reversed(list(ts.static_order()))) - # fetch all related questions in one query, but iterate according - # to pre-established sorting - _questions = models.Question.objects.in_bulk(sorted_question_slugs) for slug in sorted_question_slugs: print("question", slug) - update_or_create_calc_answer( - _questions[slug], document, struc, update_dependents=False - ) + for field in struc.find_all_fields_by_slug(slug): + recalculate_field(field, update_recursively=False) return document diff --git a/caluma/caluma_form/jexl.py b/caluma/caluma_form/jexl.py index 79c3a718b..cb1a01ec3 100644 --- a/caluma/caluma_form/jexl.py +++ b/caluma/caluma_form/jexl.py @@ -1,22 +1,30 @@ -from collections import defaultdict -from contextlib import contextmanager +import weakref +from collections import ChainMap from functools import partial from pyjexl.analysis import ValidatingAnalyzer -from pyjexl.evaluator import Context + +from caluma.caluma_core.exceptions import QuestionMissing from ..caluma_core.jexl import ( JEXL, ExtractTransformArgumentAnalyzer, ExtractTransformSubjectAnalyzer, - ExtractTransformSubjectAndArgumentsAnalyzer, ) -from .models import Question -from .structure import Field +""" +Rewrite of the JEXL handling code. + +Design principles: -class QuestionMissing(Exception): - pass +* The JEXL classes do not deal with context switching between questions anymore +* The QuestionJexl class only sets up the "runtime", any context is used from the + structure code +* We only deal with the *evaluation*, no transform/extraction is happening here - that code + is mostly fine and doesn't need a rewrite +* Caching is done by the structure2 code, not here +* JEXL evaluation happens lazily, but the results are cached. +""" class QuestionValidatingAnalyzer(ValidatingAnalyzer): @@ -28,58 +36,44 @@ def visit_Transform(self, transform): class QuestionJexl(JEXL): - def __init__(self, validation_context=None, **kwargs): - if validation_context: - if "jexl_cache" not in validation_context: - validation_context["jexl_cache"] = defaultdict(dict) - self._cache = validation_context["jexl_cache"] - else: - self._cache = defaultdict(dict) + def __init__(self, field, **kwargs): + """Initialize QuestionJexl2. + Note: The field *may* be set to `None` if you're intending + to use the object for expression analysis only (exctract_* methods + for example) + """ super().__init__(**kwargs) - - self._structure = None - self._form = None - - context_data = None - - if validation_context: - # cleaned up variant - self._form = validation_context.get("form") - self._structure = validation_context.get("structure") - context_data = { - "form": self._form.slug if self._form else None, - "info": self._structure, - } - - self.context = Context(context_data) + self.field = weakref.proxy(field) if field else None self.add_transform("answer", self.answer_transform) def answer_transform(self, question_slug, *args): - field = self._structure.get_field(question_slug) + field = self.field.get_field(question_slug) - # The first and only argument is the default value. If passed the field - # is not required and we return that argument. - if not field and len(args): - return args[0] - - if self.is_hidden(field): + def _default_or_empty(): + if len(args): + return args[0] return field.question.empty_value() - # This overrides the logic in field.value() to consider visibility for - # table cells - elif field.question.type == Question.TYPE_TABLE and field.answer is not None: - return [ - { - cell.question.slug: cell.value() - for cell in row.children() - if not self.is_hidden(cell) - } - for row in field.children() - ] + if not field: + if args: + return args[0] + else: + # No default arg, so we must raise an exception + raise QuestionMissing( + f"Question `{question_slug}` could not be found in form {self.field.get_form()}" + ) + + if field.is_hidden(): + # Hidden fields *always* return the empty value, even if we have + # a default + return field.question.empty_value() + elif field.is_empty(): + # not hidden, but empty + return _default_or_empty() - return field.value() + return field.get_value() def validate(self, expression, **kwargs): return super().validate(expression, QuestionValidatingAnalyzer) @@ -90,118 +84,21 @@ def extract_referenced_questions(self, expr): expr, partial(ExtractTransformSubjectAnalyzer, transforms=transforms) ) - def extract_referenced_questions_with_arguments(self, expr): - transforms = ["answer"] - yield from self.analyze( - expr, - partial(ExtractTransformSubjectAndArgumentsAnalyzer, transforms=transforms), - ) - def extract_referenced_mapby_questions(self, expr): transforms = ["mapby"] yield from self.analyze( expr, partial(ExtractTransformArgumentAnalyzer, transforms=transforms) ) - @contextmanager - def use_field_context(self, field: Field): - """Context manger to temporarily overwrite self._structure. - - This is used so we can evaluate each JEXL expression in the context - of the corresponding question, not from where the question was - referenced. - This is relevant in table questions and form questions, so we always - lookup the correct answer value (no "crosstalk" between rows, for example) - """ - - # field's parent is the fieldset - which is a valid structure object - old_structure = self._structure - self._structure = field.parent() or self._structure - yield - self._structure = old_structure - - def _get_referenced_fields(self, field: Field, expr: str): - deps = list(self.extract_referenced_questions_with_arguments(expr)) - referenced_fields = [self._structure.get_field(slug) for slug, _ in deps] - - referenced_slugs = [ref.question.slug for ref in referenced_fields if ref] - - for slug, args in deps: - required = len(args) == 0 - if slug not in referenced_slugs and required: - raise QuestionMissing( - f"Question `{slug}` could not be found in form {field.form}" - ) - - return [field for field in referenced_fields if field] - - def is_hidden(self, field: Field): - """Return True if the given field is hidden. - - This checks whether the dependency questions are hidden, then - evaluates the field's is_hidden expression itself. - """ - cache_key = (field.document.pk, field.question.pk) - - if cache_key in self._cache["hidden"]: - return self._cache["hidden"][cache_key] - - # Check visibility of dependencies before actually evaluating the `is_hidden` - # expression. If all dependencies are hidden, - # there is no way to evaluate our own visibility, so we default to - # hidden state as well. - referenced_fields = self._get_referenced_fields(field, field.question.is_hidden) - - # all() returns True for the empty set, thus we need to - # check that we have some deps at all first - all_deps_hidden = bool(referenced_fields) and all( - self.is_hidden(ref_field) for ref_field in referenced_fields - ) - if all_deps_hidden: - self._cache["hidden"][cache_key] = True - return True - - # Also check if the question is hidden indirectly, - # for example via parent formquestion. - parent = field.parent() - if parent and parent.question and self.is_hidden(parent): - # no way this is shown somewhere - self._cache["hidden"][cache_key] = True - return True - - # if the question is visible-in-context and not hidden by invisible dependencies, - # we can evaluate it's own is_hidden expression - with self.use_field_context(field): - self._cache["hidden"][cache_key] = self.evaluate(field.question.is_hidden) - - return self._cache["hidden"][cache_key] - - def is_required(self, field: Field): - cache_key = (field.document.pk, field.question.pk) - question = field.question - - if cache_key in self._cache["required"]: - return self._cache["required"][cache_key] - - referenced_fields = self._get_referenced_fields(field, question.is_required) - - # all() returns True for the empty set, thus we need to - # check that we have some deps at all first - all_deps_hidden = bool(referenced_fields) and all( - self.is_hidden(ref_field) for ref_field in referenced_fields - ) - if all_deps_hidden: - ret = False - else: - with self.use_field_context(field): - ret = self.evaluate(question.is_required) - self._cache["required"][cache_key] = ret - return ret - def evaluate(self, expr, raise_on_error=True): try: - return super().evaluate(expr) - except (TypeError, ValueError, ZeroDivisionError): + return super().evaluate(expr, ChainMap(self.context)) + except ( + TypeError, + ValueError, + ZeroDivisionError, + AttributeError, + ): if raise_on_error: raise return None diff --git a/caluma/caluma_form/models.py b/caluma/caluma_form/models.py index 29bceb202..300d4d1b9 100644 --- a/caluma/caluma_form/models.py +++ b/caluma/caluma_form/models.py @@ -518,8 +518,14 @@ def delete(self, *args, **kwargs): super().delete(args, kwargs) def create_answer_documents(self, documents): + """Create AnswerDocuments for this table question, and attach them. + + Return a dict with two keys: "created", and "updated", each + containing a list of document IDs that were either created or kept. + """ family = getattr(self.document, "family", None) document_ids = [document.pk for document in documents] + res = {"updated": [], "created": []} for sort, document_id in enumerate(reversed(document_ids), start=1): ans_doc, created = AnswerDocument.objects.get_or_create( @@ -532,6 +538,10 @@ def create_answer_documents(self, documents): # Already-existing documents are already in the family, # so we're updating only the newly attached rows ans_doc.document.set_family(family) + res["created"].append(document_id) + else: + res["updated"].append(document_id) + return res def unlink_unused_rows(self, docs_to_keep): existing = AnswerDocument.objects.filter(answer=self).exclude( diff --git a/caluma/caluma_form/serializers.py b/caluma/caluma_form/serializers.py index cd5861472..3427e5798 100644 --- a/caluma/caluma_form/serializers.py +++ b/caluma/caluma_form/serializers.py @@ -18,7 +18,7 @@ class QuestionJexlField(serializers.JexlField): def __init__(self, **kwargs): - super().__init__(QuestionJexl(), **kwargs) + super().__init__(QuestionJexl(field=None), **kwargs) class ButtonActionField(serializers.CalumaChoiceField): diff --git a/caluma/caluma_form/signals.py b/caluma/caluma_form/signals.py index df79414b8..ded245ccb 100644 --- a/caluma/caluma_form/signals.py +++ b/caluma/caluma_form/signals.py @@ -103,7 +103,10 @@ def remove_calc_dependents(sender, instance, **kwargs): @filter_events(lambda instance: instance.type == models.Question.TYPE_CALCULATED_FLOAT) @filter_events(lambda instance: getattr(instance, "calc_expression_changed", False)) def update_calc_from_question(sender, instance, created, update_fields, **kwargs): - for document in models.Document.objects.filter(form__questions=instance): + # TODO: we need to find documents that contain this form as a subform + # as well. Tis would only find documents where the question is attached + # top-level. + for document in models.Document.objects.filter(form__questions=instance).iterator(): update_or_create_calc_answer(instance, document) @@ -113,5 +116,8 @@ def update_calc_from_question(sender, instance, created, update_fields, **kwargs lambda instance: instance.question.type == models.Question.TYPE_CALCULATED_FLOAT ) def update_calc_from_form_question(sender, instance, created, **kwargs): - for document in instance.form.documents.all(): + # TODO: we need to find documents that contain this form as a subform + # as well. Tis would only find documents where the question is attached + # top-level. + for document in instance.form.documents.all().iterator(): update_or_create_calc_answer(instance.question, document) diff --git a/caluma/caluma_form/structure.py b/caluma/caluma_form/structure.py index 687b1b090..15ce8f1fa 100644 --- a/caluma/caluma_form/structure.py +++ b/caluma/caluma_form/structure.py @@ -1,273 +1,668 @@ -"""Hierarchical representation of a document / form.""" +""" +Structure - Fast and correct form structure representation. +Design requirements: + +* Fast initialisation from a document. Use preload if needed to reduce + number of queries +* Eager loading of a full document structure. No lazy-loading with questionable + performance expectations +* Correct navigation via question slugs (JEXL references) from any context +* Fast lookups once initialized +* Extensible for caching JEXL expression results + +* No properties. Code is always in methods +""" + +from __future__ import annotations + +import collections +import copy +import typing import weakref -from functools import singledispatch -from typing import List, Optional +from abc import ABC +from collections.abc import Iterable +from dataclasses import dataclass, field +from functools import singledispatch, wraps +from logging import getLogger +from typing import Optional + +from django.db.models import QuerySet -from .models import Question +from caluma.caluma_core import exceptions + +if typing.TYPE_CHECKING: # pragma: no cover + from caluma.caluma_form.jexl import QuestionJexl + +from caluma.caluma_form.models import ( + Answer, + AnswerDocument, + FormQuestion, + Question, +) + +log = getLogger(__name__) def object_local_memoise(method): + """Decorate a method to become object-local memoised. + + In other words - The method will cache it's results. If the method is called + twice with the same arguments, it will return the cached result instead. + + For debugging purposes, you can also set `object_local_memoise.enabled` + to `False`, which will then behave just as if the memoising didn't happen. + """ + + @wraps(method) def new_method(self, *args, **kwargs): + if not object_local_memoise.enabled: # pragma: no cover + # for debugging purposes + return method(self, *args, **kwargs) if not hasattr(self, "_memoise"): self._memoise = {} + self._memoise_hit_count = 0 + self._memoise_miss_count = 0 key = str([args, kwargs, method]) if key in self._memoise: + object_local_memoise.hit_count += 1 + self._memoise_hit_count += 1 return self._memoise[key] ret = method(self, *args, **kwargs) + self._memoise_miss_count += 1 + object_local_memoise.miss_count += 1 self._memoise[key] = ret return ret return new_method -class Element: - aliases = {} +# This should only be set to `False` for debugging +setattr(object_local_memoise, "enabled", True) - def __init__(self, parent=None): - self._parent = weakref.ref(parent) if parent else None +# Statistics - for analysis / debugging +setattr(object_local_memoise, "hit_count", 0) +setattr(object_local_memoise, "miss_count", 0) - def parent(self): - return self._parent() if self._parent else None - def children(self): # pragma: no cover - return [] +def clear_memoise(obj): + """Clear memoise cache for given object. - def root(self): - parent = self.parent() - if parent: - return parent.root() - return self + If an object uses the `@object_local_memoise` decorator, you can then + call `clear_memoise()` on that object to clear all it's cached data. + """ + obj._memoise = {} - def get(self, name, default=None): - name = self.aliases.get(name, name) - out = getattr(self, name) +@dataclass +class BaseField(ABC): + """Base class for the field types. This is the interface we aim to provide.""" - # if a method is requested, execute it before continuing - if callable(out): - out = out() - if isinstance(out, Element) or isinstance(out, dict): - return out - if out is None: - return None - return str(out) + parent: Optional["FieldSet"] = field(default=None) + question: Optional[Question] = field(default=None) + answer: Optional[Answer] = field(default=None) -class Field(Element): - def __init__(self, document, form, question, answer=None, parent=None): - super().__init__(parent) - self.document = document - self.form = form - self.question = question - self.answer = answer + @object_local_memoise + def get_evaluator(self) -> QuestionJexl: + """Return the JEXL evaluator for this field.""" + + # JEXL is implemented such that it has one context in the engine, and + # trying to swap it out to switch context is just problematic. So we use + # one JEXL instance per field. + + # deferred import to avoid circular dependency + from caluma.caluma_form.jexl import QuestionJexl + + # The "info" block is provided by the global context, but we need + # to patch it to represent some local information: The "form" and + # "document" bits should point to the current context, not to the global + # structure. This is a bit unfortunate, but we *have* to fill this in + # two separate places to avoid breaking compatibility + context = collections.ChainMap( + # We need a deep copy of the global context, so we can + # extend the info block without leaking + copy.deepcopy(self.get_global_context()), + self.get_context(), + ) - @classmethod - def factory(cls, document, form, question, answer=None, parent=None): - if question.type == Question.TYPE_FORM: - return FieldSet( - document, form=question.sub_form, question=question, parent=parent - ) - elif question.type == Question.TYPE_TABLE: - return RowField( - document, - form=question.row_form, - question=question, - answer=answer, - parent=parent, - ) + context["info"].update(self.get_local_info_context()) - return Field(document, form, question, answer, parent=parent) + # Legacy form ref - pointing to the root form + context["form"] = self._get_root().get_form().slug - def value(self): - if self.answer is None: - # no answer object at all - return empty in every case - return self.question.empty_value() + return QuestionJexl(field=self, context=context) - elif self.answer.value is not None: - return self.answer.value + def _get_root(self): + return self.parent._get_root() if self.parent else self - elif self.question.type == Question.TYPE_TABLE: # pragma: no cover - return [ - {cell.question.slug: cell.value() for cell in row.children()} - for row in self.children() - ] + @object_local_memoise + def get_local_info_context(self): + """Return the dictionary to be used in the local `info` context block. + + Properties (See Ember-Caluma's field source for reference): + - `form`: Legacy property pointing to the root form. + -> defined in get_evaluator() as we're only returning `info` here + * Form information + - `info.form`: The form this question is attached to. + - `info.formMeta`: The meta of the form this question is attached to. + - `info.parent.form`: The parent form if applicable. + - `info.parent.formMeta`: The parent form meta if applicable. + - `info.root.form`: The new property for the root form. + - `info.root.formMeta`: The new property for the root form meta. + * Case information is taken from the global context + - `info.case.form`: The cases' form (works for task forms and case forms). + - `info.case.workflow`: The cases' workflow (works for task forms and case forms). + - `info.case.root.form`: The _root_ cases' form (works for task forms and case forms). + - `info.case.root.workflow`: The _root_ cases' workflow (works for task forms and case forms). - elif self.question.type == Question.TYPE_FILES: - return [f.name for f in self.answer.files.all()] + """ + form = self.get_form() - elif self.question.type == Question.TYPE_DATE: - return self.answer.date + if parent_info := self.get_parent_fieldset(): + parent_data = { + "form": parent_info.get_form().slug, + "formMeta": parent_info.get_form().meta, + } + else: + parent_data = None + return { + # "_type": type(self).__qualname__, + "question": self.question.slug if self.question else None, + "form": form and form.slug or None, + "formMeta": form and form.meta or None, + "parent": parent_data, + # TODO how is "root" expected to behave if we're *already* on root? + "root": self._get_root().get_local_info_context() if self.parent else None, + } - elif self.question.type in ( - Question.TYPE_MULTIPLE_CHOICE, - Question.TYPE_DYNAMIC_MULTIPLE_CHOICE, - ): - return [] + def get_parent_fieldset(self): + """Return the parent fieldset, according to JEXL semantics. - # no value, no special handling - return None + In JEXL, the parent refers to th next field up that represents another + form. In Rows for example, this is two levels up, but in regular nested + forms, it's only one level up. + """ + my_form = self.get_form() + p = self.parent + while p and p.get_form() == my_form: + p = p.parent + return p - def __repr__(self): - return f"<Field question={self.question.slug}, value={self.value()} hidden=({self.question.is_hidden}) req=({self.question.is_required})>" + @object_local_memoise + def is_required(self) -> bool: + """Return True if the field is required. + + Evaluate the `is_required` expression of the field. But if the field + is hidden, it is considered not-required regardless of what the JEXL + expression says. + """ + if self.is_hidden(): + # hidden can never be required + return False + if self.all_dependencies_hidden_or_empty(self.question.is_required): + # All dependencies are hidden, which means we can't calculate + # the expression at all - assume the field is not required regardless + # of the expression + return False + + return self.evaluate_jexl(self.question.is_required) -class RowField(Field): @object_local_memoise - def children(self): - if not self.answer: - return [] # pragma: no cover + def is_visible(self) -> bool: + """Just return the opposite of is_hidden() for convenience.""" + return not self.is_hidden() - # We're sorting in python-space here to reuse the already-executed query - # for the answerdocument_set. Sorting in DB would re-issue the query - rows = sorted( - self.answer.answerdocument_set.all(), - key=lambda answer_document: answer_document.sort, + @object_local_memoise + def all_dependencies_hidden_or_empty(self, expr): + """Return True if all dependencies of the given expression are hidden. + + Note: If there are no dependencies, we return False. Question slugs + referring to missing fields are ignored. + """ + dependencies = list(self.get_evaluator().extract_referenced_questions(expr)) + dep_fields = { + dep: self.get_field(dep) for dep in dependencies if self.get_field(dep) + } + + return dep_fields and all( + # Field missing may be acceptable if it's used in an `answer` + # transform with a default. Therefore if a field is missing here, + # we consider it "hidden" as well + (field.is_hidden() or field.is_empty()) if field else True + for field in dep_fields.values() ) - return [ - FieldSet( - ans_doc.document, - self.question.row_form, - question=self.question, - parent=self.parent(), + + @object_local_memoise + def is_hidden(self, raise_on_error=True) -> bool: + """Return True if the field is hidden. + + A field is hidden if either it's parent is hidden, or it's `is_hidden` + JEXL expression evaluates to `True`. + """ + if self.parent and self.parent.is_hidden(): + # If the parent is hidden, then *we* are implicitly also hidden, + # without even evaluating our own is_hidden expression + return True + + if not self.question and not self.parent: + # Root field is always visible + return False + + # do_raise = self.all_dependencies_hidden(self.question.is_hidden) + + if self.all_dependencies_hidden_or_empty(self.question.is_hidden): + # All dependencies are hidden, which means we can't calculate + # the expression at all, and therefore we don't show this field + # in the form. + return True + + try: + return self.evaluate_jexl(self.question.is_hidden, raise_on_error) + except exceptions.QuestionMissing: + if raise_on_error: + # if raise_on_error is False, we ignore this one as well. + # should be used internally only, for example in get_value() + raise + # If the expression fails, we assume an error + # and consider ourselves as hidden + return True + + @object_local_memoise + def slug(self): + return self.question and self.question.slug or None + + def get_context(self) -> collections.ChainMap: ... + + @object_local_memoise + def get_field(self, slug) -> BaseField: + return self.get_context().get(slug) + + @object_local_memoise + def find_field_by_answer(self, answer) -> BaseField: + q_field = self.get_field(answer.question_id) + if q_field and q_field.answer and q_field.answer.pk == answer.pk: + return q_field + + # answer is not in "our" document, probably we're in a row doc. + # Therefore, search "everywhere" + for fld in self._get_root().find_all_fields_by_slug(answer.question_id): + if fld.answer.pk == answer.pk: + return fld + return None + + def refresh(self, answer=None): + """Refresh this field's answer. + + If an answer is given, use it and update the structure in-place. + Otherwise, look in the DB. + + Also clear out all the caches on our own field as well as all + the calc dependents. + """ + if answer: + self.answer = answer + + # TODO: update / save answer + # TODO: reset caches in all dependents (calc dependents are easy, but what + # about the rest? Like visibility dependents etc?) + clear_memoise(self) + for dep in self.question.calc_dependents: + for dep_field in self._get_root().find_all_fields_by_slug(dep): + dep_field.refresh() + + def calculate(self): + try: + return self.evaluate_jexl(self.question.calc_expression) + except Exception: + # In calc, if an expression evaluation fails, we just return None. + # We could be in an unattached table row, for example, and we'd + # do the recalculation when we're actually being attached. + return None + + def evaluate_jexl(self, expression: str, raise_on_error=True): + # Some stupid shortcuts to avoid building up an evaluation context for + # ~90% of cases where the expression is a simple "true" or "false" + fast_results = {"true": True, "false": False} + if (fast_result := fast_results.get(expression)) is not None: + return fast_result + + eval = self.get_evaluator() + + try: + result = eval.evaluate(expression, raise_on_error) + return result + except exceptions.QuestionMissing: + raise + except Exception as exc: + log.error( + f"Error while evaluating expression on question {self.slug()}: " + f"{expression!r}: {str(exc)}" + ) + if raise_on_error: + raise RuntimeError( + f"Error while evaluating expression on question {self.slug()}: " + f"{expression!r}. The system log contains more information" + ) + # return None is implied + + def get_global_context(self) -> dict: + return self._global_context + + @object_local_memoise + def get_form(self): + parent_form = self.parent.form if self.parent else None + return parent_form or self.form or None + + +@dataclass +class ValueField(BaseField): + """Represent a field in the form. + + This is roughly 1:1 with a question, but repeated in case of a table + row for example. + """ + + # empty string is acceptable + EMPTY_VALUES = ([], (), {}, None) + + @object_local_memoise + def get_value(self): + if self.answer and not self.is_hidden(raise_on_error=False): + if self.answer.value not in self.EMPTY_VALUES: + return self.answer.value + if self.answer.date: + return self.answer.date + if self.question.type == Question.TYPE_FILES: + # TODO return files - how exactly? Returning list-of-filenames + # for now + return [f.name for f in self.answer.files.all()] + return self.question.empty_value() + + @object_local_memoise + def get_context(self) -> collections.ChainMap: + return self.parent.get_context() + + @object_local_memoise + def is_empty(self): + if not self.answer: + return True + return ( + # Yeah it's weird - get_value should return empty string if that's + # what the answer has stored, but is_empty() should treat it as + # empty, still ¯\_(ツ)_/¯ + not bool( + (self.answer.value not in (*self.EMPTY_VALUES, "")) + or (self.answer.date is not None) + or self.answer.files.exists() ) - for ans_doc in rows - ] + # Being hidden makes you empty even if an answer exists + or self.is_hidden() + ) + + def get_global_context(self) -> dict: + return self.parent.get_global_context() + + def __str__(self): + return f"Field({self.question.slug}, {self.get_value()})" + def __repr__(self): + return f"ValueField(q={self.question.slug}, v={self.get_value()})" -class FieldSet(Element): - aliases = {"formMeta": "form_meta"} - def __init__(self, document, form, question=None, parent=None): - super().__init__(parent) - self.document = document - self.form = form - self.form_meta = form.meta +class FieldSet(BaseField): + def __init__( + self, document, form=None, parent=None, question=None, global_context=None + ): + # TODO: prefetch document once we have the structure built up + # self.question = question - self._fields = None - self._sub_forms = None - self._case = "NOTSET" - - @property - def case(self): - if self._case != "NOTSET": - return self._case # pragma: no cover - - if hasattr(self.document.family, "work_item"): - case = self.document.family.work_item.case - elif hasattr(self.document.family, "case"): - # if we're not in a task form, we might be the root document - case = self.document.family.case + self._document = document + self.form = form or document.form + + self._global_context = global_context or {"info": {}} + + # uplinks always weak + self.parent = weakref.proxy(parent) if parent else None + + self._own_fields = {} + + if parent: + # TODO This likely causes a circular dependency - Verify + + # Our context is an extension of the parent's context. That way, we can + # see the fields from the parent context. + self._context = collections.ChainMap( + self._own_fields, self.parent.get_context() + ) + + # Extending parent's context with our own: This makes our own fields + # visible in the parent context as well + self._extend_root_context(self._own_fields) else: - self._case = None - return self._case - - root = case.family - self._case = { - "form": case.document.form.slug, - "workflow": case.workflow.slug, - "root": { - "form": root.document.form.slug, - "workflow": root.workflow.slug, - }, + # Root fieldset + self._context = collections.ChainMap(self._own_fields) + + self._build_context() + + def _extend_root_context(self, new_map): + if self.parent: + self.parent._extend_root_context(new_map) + else: + self._context.maps.append(new_map) + + @object_local_memoise + def is_empty(self): + # Fieldset: If *any* field is non-empty, we consider ourselves also + # non-empty. + # We reverse the lookup here to speed up (don't need to see if *all* are + # empty, just if *one* has a value) + if self.is_hidden(): + # Hidden makes us empty, even if there's theoretically a value. + # We do the "cheap" check first, then iterate over the children. + return True + has_at_least_one_value = any(not child.is_empty() for child in self.children()) + return not has_at_least_one_value + + def get_context(self): + return self._context + + @object_local_memoise + def get_value(self): + if self.is_hidden() or self.is_empty(): + return {} + return { + formfield.question.slug: formfield.get_value() + for formfield in self._own_fields.values() } - return self._case - - @property - def fields(self): - if self._fields is None: - self._fields = {field.question.slug: field for field in self.children()} - return self._fields - - @property - def sub_forms(self) -> List[Field]: - if self._sub_forms is None: - self._sub_forms = [ - field - for field in self.children() - if field.question.type == Question.TYPE_FORM - ] + [ - child - for field in self.children() - for child in field.children() - if field.question.type == Question.TYPE_TABLE - ] - return self._sub_forms - - def get_field( - self, question_slug: str, check_parent: bool = True - ) -> Optional[Field]: - """Collect fields where the question occurs throughout this structure. - - Cases: - 0. question not in structure - 1. question is in the same form (-> greedily returns the question) - 2. question in a neighbor form, ie. answer would be in same document (excluding tables) - 3. question in multiple neighbor forms - 4. question in a table form (same fieldset) - 5. (question in a table form (different row)) - 6. question in a table form, lower than current fieldset - 7. question in upper structure (from table row) - - Expected: - 0: return [] - 1-3: answer exists once, but might be in multiple forms -> multiple fields - 4: return only row-local fields (not looking up / other rows) - 5: incomplete row missing answer -> return fields with empty value (like case 4) - 6: return all fields for all rows - 7: same as 1-3 - """ - field = self.fields.get(question_slug) + def is_required(self) -> bool: + # Fieldsets (in other words - subforms) should never be required. + # TODO: Verify this assumption - if field: - return field + return False - elif check_parent: - field = self.parent().get_field(question_slug) if self.parent() else None - if field: - return field + def get_all_fields(self) -> Iterable[BaseField]: + """Return all fields in the structure, as an iterator. - # OK start looking in subforms / row forms below our level. - # Since we're looking down, we're disallowing recursing to outer context - # to avoid recursing back to where we are - for subform in self.sub_forms: - sub_field = subform.get_field(question_slug, check_parent=False) - if sub_field: - return sub_field + Yields (slug,field) tuples. But note that a slug may be repeated + as we iterate over table rows. - # if we reach this line, we didn't find the question - return None + NOTE: For tables, the same *question* may be repeated in each row. This + is intended and normal behaviour. + """ + for formfield in self._own_fields.values(): + yield formfield + if isinstance(formfield, FieldSet): + yield from formfield.get_all_fields() + if isinstance(formfield, RowSet): + # row sets *must* have fieldsets as children. Let's loop + # over them here + for child in formfield.children(): + yield child + yield from child.get_all_fields() + + def find_all_fields_by_slug(self, slug: str) -> list[BaseField]: + """Return all fields with the given question slug. + + This may return multiple fields, as tables are traversed as well. + + Should be used for debugging or analytics only. If you need the one + field that the `answer` transform would return in this context, use + `.get_field()` instead. + """ + result = [] + for formfield in self.get_all_fields(): + if formfield.question and formfield.slug() == slug: + result.append(formfield) + return result @object_local_memoise + def find_field_by_document_and_question(self, document_id, question_id): + """Find the given field, identified by document and question. + + In some contexts, we have the (full document) structure, but can't + directly know where in the structure a (document,question) element + is located, even though it's deterministic. + """ + # TODO: Possibly optimize search algorithm + for fld in self.find_all_fields_by_slug(question_id): + if fld.parent._document.pk == document_id: + return fld + def children(self): - answers = {ans.question_id: ans for ans in self.document.answers.all()} - return [ - Field.factory( - document=self.document, - form=self.form, - question=question, - answer=answers.get(question.slug), - parent=self, + # This should already be sorted, as the context buildup + # is doing that for us. TODO: Verify this claim + return list(self._own_fields.values()) + + def _build_context(self): + # context inheritance: The ChainMap allows lookups in "parent" + # contexts, so a row context will be able to look "out". We implement + # form questions the same way, even though not strictly neccessary + + root_answers = self._document.answers.all().select_related("question") + answers_by_q_slug = {ans.question_id: ans for ans in root_answers} + + formquestions: QuerySet[FormQuestion] = FormQuestion.objects.filter( + form=self.form + ).order_by("-sort") + + for fq in formquestions: + question = fq.question + if question.type == Question.TYPE_FORM: + self._context[question.slug] = FieldSet( + document=self._document, + # question=question, + form=question.sub_form, + parent=self, + question=question, + global_context=self.get_global_context(), + ) + elif question.type == Question.TYPE_TABLE: + self._context[question.slug] = RowSet( + question=question, + answer=answers_by_q_slug.get(question.slug), + parent=self, + ) + else: + # "leaf" question + self._context[question.slug] = ValueField( + question=question, + answer=answers_by_q_slug.get(question.slug), + parent=self, + ) + + def __str__(self): + return f"FieldSet({self.form.slug})" + + def __repr__(self): + q_slug = self.question.slug if self.question else "(root)" + + return f"FieldSet(q={q_slug}, f={self.form.slug})" + + +class RowSet(BaseField): + rows: list[FieldSet] + + def __init__(self, question, parent, answer: Optional[Answer] = None): + self.form = question.row_form + self.question = question + self.answer = answer + + if not parent: # pragma: no cover + raise exceptions.ConfigurationError( + f"Table question {self.slug()} has no parent" ) - for question in self.form.questions.all() - ] - def set_answer(self, question_slug, answer): - field = self.get_field(question_slug) - if field: - field.answer = answer + self.parent = weakref.proxy(parent) + + if answer: + self.rows = [ + FieldSet( + document=row_doc.document, + question=question, + form=question.row_form, + parent=self, + global_context=self.get_global_context(), + ) + for row_doc in AnswerDocument.objects.all() + .filter(answer=answer) + .order_by("-sort") + ] + else: + self.rows = [] - def __repr__(self): - q_slug = self.question.slug if self.question else None - if q_slug: - return f"<FieldSet fq={q_slug}, doc={self.document.pk} hidden=({self.question.is_hidden}) req=({self.question.is_required})>" + def get_value(self): + if self.is_hidden(): # pragma: no cover + return [] + + return [row.get_value() for row in self.children()] + + def get_all_fields(self) -> Iterable[BaseField]: + for row in self.children(): + yield row + # Row field children are always FieldSets + yield from row.get_all_fields() + + def get_context(self): + # Rowset does not have it's own context: Any field within is + # basically in it's own world (but has a view onto the "outside") + return self.parent.get_context() + + def children(self): + return self.rows + + def _extend_root_context(self, new_map): + # Rowset: We do not recurse further up when extending context, + # and we're also not updating our own context from the rows + pass + + @object_local_memoise + def get_global_context(self) -> dict: + if not self.parent: # pragma: no cover + raise exceptions.ConfigurationError( + f"Table question {self.slug()} has no parent" + ) + + return self.parent.get_global_context() + + @object_local_memoise + def is_empty(self): + # Table is considered empty if it has no rows. + # Hidden implies empty, even if there *theoretically* is an answer + # present + return self.is_hidden() or not bool(self.rows) + + def __str__(self): + return f"RowSet({self.form.slug})" - return f"<FieldSet form={self.form.slug}, doc={self.document.pk}>" + def __repr__(self): + return f"RowSet(q={self.question.slug}, f={self.form.slug})" -def print_document_structure(document): # pragma: no cover +def print_structure(fieldset: FieldSet, print_fn=None, method=str): """Print a document's structure. Intended halfway as an example on how to use the structure @@ -275,17 +670,56 @@ def print_document_structure(document): # pragma: no cover """ ind = {"i": 0} + print_fn = print_fn or print + @singledispatch - def visit(vis): + def visit(vis): # pragma: no cover + # Should never happen - for completeness only raise Exception(f"generic visit(): {vis}") - @visit.register(Element) - def _(vis): - print(" " * ind["i"], vis) + @visit.register(FieldSet) + def _(vis: FieldSet): + print_fn(" " * ind["i"], method(vis)) ind["i"] += 1 - for c in vis.children(): - visit(c) + for sub in vis.children(): + visit(sub) ind["i"] -= 1 - struc = FieldSet(document, document.form) - visit(struc) + @visit.register(RowSet) + def _(vis: RowSet): + print_fn(" " * ind["i"], method(vis)) + ind["i"] += 1 + for sub in vis.children(): + visit(sub) + ind["i"] -= 1 + + @visit.register(ValueField) + def _(vis): + print_fn(" " * ind["i"], method(vis)) + + visit(fieldset) + + +def list_structure(fieldset, method=str): + """List the given fieldset's structure. + + Use the given method (Default: str()) for stringification. + Return a list of strings, each representing a field in the structure, + properly indented, useful for visualisation + """ + out_lines = [] + + def fake_print(*args): + out_lines.append(" ".join([x for x in args])) + + print_structure(fieldset, print_fn=fake_print, method=method) + return out_lines + + +def list_document_structure(document, method=str): + """List the given document's structure. + + Use the given method (Default: str()) for stringification. + """ + fs = FieldSet(document) + return list_structure(fs, method) diff --git a/caluma/caluma_form/tests/test_complex_jexl.py b/caluma/caluma_form/tests/test_complex_jexl.py new file mode 100644 index 000000000..4f252f96f --- /dev/null +++ b/caluma/caluma_form/tests/test_complex_jexl.py @@ -0,0 +1,307 @@ +# Test cases for the (NEW!) structure utility class +from pathlib import Path + +import pytest +from django.core.management import call_command + +from caluma.caluma_form import api, structure +from caluma.caluma_form.models import AnswerDocument, Document, Form, Question + + +@pytest.fixture +def complex_jexl_form(): + """Return a form with a bit of structure. + + The structure is as follows: + + demo-formular-1 (Root form) + demo-outer-table-question-1 (Table) + demo-table-form-1 (Row form) + demo-table-question-1 (Integer) + demo-table-question-2 (Calculated) + demo-outer-question-1 + demo-outer-table-question-2 + """ + # Complex JEXL evaluation tests: + # * Recalculation witin table rows + # * Visibility checks in "outer" form with indirect calc question evaluation + data_file = Path(__file__).parent / "test_data/complex_jexl_context.json" + assert data_file.exists() + call_command("loaddata", str(data_file)) + return Form.objects.get(slug="demo-formular-1") + + +@pytest.fixture +def complex_jexl_doc(complex_jexl_form): + """Return a document with a few questions answered. + + The structure is as follows: + + FieldSet(demo-formular-1) + Field(demo-outer-table-question-1, None) + FieldSet(demo-table-form-1) + Field(demo-table-question-1, 3) + Field(demo-table-question-2, 1) + FieldSet(demo-table-form-1) + Field(demo-table-question-1, 20) + Field(demo-table-question-2, 100) + Field(demo-outer-question-1, demo-outer-question-1-outer-option-a) + Field(demo-outer-table-question-2, None) + """ + # -> root_doc, row1, row2 as tuple + doc = Document.objects.create(form=complex_jexl_form) + + api.save_answer( + Question.objects.get(pk="demo-outer-question-1"), + doc, + value="demo-outer-question-1-outer-option-a", + ) + table_ans = api.save_answer( + Question.objects.get(pk="demo-outer-table-question-1"), + doc, + ) + + row1 = AnswerDocument.objects.create( + answer=table_ans, + document=Document.objects.create(form=table_ans.question.row_form, family=doc), + sort=2, + ).document + row2 = AnswerDocument.objects.create( + answer=table_ans, + document=Document.objects.create(form=table_ans.question.row_form, family=doc), + sort=1, + ).document + + api.save_answer( + Question.objects.get(pk="demo-table-question-1"), document=row1, value=3 + ) + + api.save_answer( + Question.objects.get(pk="demo-table-question-1"), document=row2, value=20 + ) + return doc, row1, row2 + + +def test_evaluating_calc_inside_table( + transactional_db, complex_jexl_form, complex_jexl_doc +): + doc, *_ = complex_jexl_doc + + assert structure.list_document_structure(doc, method=repr) == [ + " FieldSet(q=(root), f=demo-formular-1)", + " RowSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " FieldSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " ValueField(q=demo-table-question-1, v=3)", + " ValueField(q=demo-table-question-2, v=1)", + " FieldSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " ValueField(q=demo-table-question-1, v=20)", + " ValueField(q=demo-table-question-2, v=100)", + " ValueField(q=demo-outer-question-1, v=demo-outer-question-1-outer-option-a)", + " RowSet(q=demo-outer-table-question-2, f=demo-table-form-2)", + ] + + +def test_update_calc_dependency_inside_table( + transactional_db, complex_jexl_form, complex_jexl_doc +): + doc, row1, row2 = complex_jexl_doc + + assert structure.list_document_structure(doc, method=repr) == [ + " FieldSet(q=(root), f=demo-formular-1)", + " RowSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " FieldSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " ValueField(q=demo-table-question-1, v=3)", + " ValueField(q=demo-table-question-2, v=1)", + " FieldSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " ValueField(q=demo-table-question-1, v=20)", + " ValueField(q=demo-table-question-2, v=100)", + " ValueField(q=demo-outer-question-1, v=demo-outer-question-1-outer-option-a)", + " RowSet(q=demo-outer-table-question-2, f=demo-table-form-2)", + ] + api.save_answer( + Question.objects.get(pk="demo-table-question-1"), document=row1, value=30 + ) + assert ( + structure.list_document_structure(doc, method=repr) + == [ + " FieldSet(q=(root), f=demo-formular-1)", + " RowSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " FieldSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " ValueField(q=demo-table-question-1, v=30)", # this was set + " ValueField(q=demo-table-question-2, v=100)", # should have been recalc'd + " FieldSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " ValueField(q=demo-table-question-1, v=20)", + " ValueField(q=demo-table-question-2, v=100)", + " ValueField(q=demo-outer-question-1, v=demo-outer-question-1-outer-option-a)", + " RowSet(q=demo-outer-table-question-2, f=demo-table-form-2)", + ] + ) + + api.save_answer( + Question.objects.get(pk="demo-table-question-1"), document=row1, value=3 + ) + assert structure.list_document_structure(doc, method=repr) == [ + " FieldSet(q=(root), f=demo-formular-1)", + " RowSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " FieldSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " ValueField(q=demo-table-question-1, v=3)", # updated again + " ValueField(q=demo-table-question-2, v=1)", # recalculated again + " FieldSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " ValueField(q=demo-table-question-1, v=20)", + " ValueField(q=demo-table-question-2, v=100)", + " ValueField(q=demo-outer-question-1, v=demo-outer-question-1-outer-option-a)", + " RowSet(q=demo-outer-table-question-2, f=demo-table-form-2)", + ] + + api.save_answer( + Question.objects.get(pk="demo-table-question-1"), document=row2, value=40 + ) + assert ( + structure.list_document_structure(doc, method=repr) + == [ + " FieldSet(q=(root), f=demo-formular-1)", + " RowSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " FieldSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " ValueField(q=demo-table-question-1, v=3)", + " ValueField(q=demo-table-question-2, v=1)", + " FieldSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " ValueField(q=demo-table-question-1, v=40)", # updated here + " ValueField(q=demo-table-question-2, v=100)", # recalculated , same value + " ValueField(q=demo-outer-question-1, v=demo-outer-question-1-outer-option-a)", + " RowSet(q=demo-outer-table-question-2, f=demo-table-form-2)", + ] + ) + + api.save_answer( + Question.objects.get(pk="demo-table-question-1"), document=row2, value=2 + ) + assert structure.list_document_structure(doc, method=repr) == [ + " FieldSet(q=(root), f=demo-formular-1)", + " RowSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " FieldSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " ValueField(q=demo-table-question-1, v=3)", + " ValueField(q=demo-table-question-2, v=1)", + " FieldSet(q=demo-outer-table-question-1, f=demo-table-form-1)", + " ValueField(q=demo-table-question-1, v=2)", # updated + " ValueField(q=demo-table-question-2, v=1)", # recalculated + " ValueField(q=demo-outer-question-1, v=demo-outer-question-1-outer-option-a)", + " RowSet(q=demo-outer-table-question-2, f=demo-table-form-2)", + ] + + +def test_update_calc_dependency_inside_table_with_outer_reference( + transactional_db, complex_jexl_form, complex_jexl_doc +): + doc, _, _ = complex_jexl_doc + + assert structure.list_document_structure(doc) == [ + " FieldSet(demo-formular-1)", + " RowSet(demo-table-form-1)", + " FieldSet(demo-table-form-1)", + " Field(demo-table-question-1, 3)", + " Field(demo-table-question-2, 1)", + " FieldSet(demo-table-form-1)", + " Field(demo-table-question-1, 20)", + " Field(demo-table-question-2, 100)", + " Field(demo-outer-question-1, demo-outer-question-1-outer-option-a)", + " RowSet(demo-table-form-2)", + ] + + # TODO: This implementation corresponds to the current frontend logic, this + # might change so that table row documents are attached on creation + new_table_doc = api.save_document(form=Form.objects.get(pk="demo-table-form-2")) + + assert structure.list_document_structure(new_table_doc) == [ + " FieldSet(demo-table-form-2)", + " Field(demo-table-question-outer-ref-hidden, None)", + " Field(demo-table-question-outer-ref-calc, None)", + ] + + api.save_answer( + Question.objects.get(pk="demo-table-question-outer-ref-hidden"), + document=new_table_doc, + value=30, + ) + + # We did save a value of 30, but the `outer-ref-hidden` refers to + # an out-of-reach question in it's `is_hidden` expression, thus is + # considered hidden itself. In turn, the `outer-ref-calc` can't calculate + # either. + assert structure.list_document_structure(new_table_doc) == [ + " FieldSet(demo-table-form-2)", + " Field(demo-table-question-outer-ref-hidden, None)", + " Field(demo-table-question-outer-ref-calc, None)", + ] + + # We now attach the table row to the main document, which should make + # it fully calculated, as the outer questions referenced from within + # the table are now reachable. + api.save_answer( + question=Question.objects.get(pk="demo-outer-table-question-2"), + document=doc, + value=[new_table_doc.pk], + ) + + assert structure.list_document_structure(doc) == [ + " FieldSet(demo-formular-1)", + " RowSet(demo-table-form-1)", + " FieldSet(demo-table-form-1)", + " Field(demo-table-question-1, 3)", + " Field(demo-table-question-2, 1)", + " FieldSet(demo-table-form-1)", + " Field(demo-table-question-1, 20)", + " Field(demo-table-question-2, 100)", + " Field(demo-outer-question-1, demo-outer-question-1-outer-option-a)", + " RowSet(demo-table-form-2)", + " FieldSet(demo-table-form-2)", + " Field(demo-table-question-outer-ref-hidden, 30)", + " Field(demo-table-question-outer-ref-calc, 30)", + ] + + api.save_answer( + Question.objects.get(pk="demo-table-question-outer-ref-hidden"), + document=new_table_doc, + value=20, + ) + + assert structure.list_document_structure(doc) == [ + " FieldSet(demo-formular-1)", + " RowSet(demo-table-form-1)", + " FieldSet(demo-table-form-1)", + " Field(demo-table-question-1, 3)", + " Field(demo-table-question-2, 1)", + " FieldSet(demo-table-form-1)", + " Field(demo-table-question-1, 20)", + " Field(demo-table-question-2, 100)", + " Field(demo-outer-question-1, demo-outer-question-1-outer-option-a)", + " RowSet(demo-table-form-2)", + " FieldSet(demo-table-form-2)", + " Field(demo-table-question-outer-ref-hidden, 20)", + " Field(demo-table-question-outer-ref-calc, 20)", + ] + + +def test_structure_caching(transactional_db, complex_jexl_form, complex_jexl_doc): + doc, _, _ = complex_jexl_doc + + hit_count_before = structure.object_local_memoise.hit_count + miss_count_before = structure.object_local_memoise.miss_count + + assert structure.list_document_structure(doc) == [ + " FieldSet(demo-formular-1)", + " RowSet(demo-table-form-1)", + " FieldSet(demo-table-form-1)", + " Field(demo-table-question-1, 3)", + " Field(demo-table-question-2, 1)", + " FieldSet(demo-table-form-1)", + " Field(demo-table-question-1, 20)", + " Field(demo-table-question-2, 100)", + " Field(demo-outer-question-1, demo-outer-question-1-outer-option-a)", + " RowSet(demo-table-form-2)", + ] + + # Note: If those fail, just update the counts. I'm more interested in a + # rather rough overview of cache hits, not the exact numbers. Changing the + # caching will affect hese numbers. + assert structure.object_local_memoise.hit_count - hit_count_before == 54 + assert structure.object_local_memoise.miss_count - miss_count_before == 54 diff --git a/caluma/caluma_form/tests/test_data/complex_jexl_context.json b/caluma/caluma_form/tests/test_data/complex_jexl_context.json new file mode 100644 index 000000000..34dcb4135 --- /dev/null +++ b/caluma/caluma_form/tests/test_data/complex_jexl_context.json @@ -0,0 +1,446 @@ +[ +{ + "model": "caluma_form.form", + "pk": "demo-formular-1", + "fields": { + "created_at": "2024-02-16T09:46:57.788Z", + "modified_at": "2024-02-16T09:46:57.788Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "name": "{\"en\": \"Formular-1\"}", + "description": "{\"en\": \"\"}", + "meta": {}, + "is_published": true, + "is_archived": false, + "source": null + } +}, +{ + "model": "caluma_form.option", + "pk": "demo-outer-question-1-outer-option-b", + "fields": { + "created_at": "2025-01-13T16:44:16.611Z", + "modified_at": "2025-01-13T16:44:16.611Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "label": "{\"en\": \"Outer Option B\"}", + "is_archived": false, + "meta": {}, + "source": null + } +}, +{ + "model": "caluma_form.option", + "pk": "demo-outer-question-1-outer-option-a", + "fields": { + "created_at": "2025-01-13T16:44:16.611Z", + "modified_at": "2025-01-13T16:44:16.611Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "label": "{\"en\": \"Outer Option A\"}", + "is_archived": false, + "meta": {}, + "source": null + } +}, +{ + "model": "caluma_form.question", + "pk": "demo-outer-question-1", + "fields": { + "created_at": "2025-01-13T16:44:16.671Z", + "modified_at": "2025-01-14T07:58:30.499Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "label": "{\"en\": \"Outer Question 1\"}", + "type": "choice", + "is_required": "true", + "is_hidden": "false", + "is_archived": false, + "placeholder": "{\"en\": null}", + "info_text": "{\"en\": null}", + "hint_text": "{\"en\": null}", + "static_content": "{\"en\": \"\"}", + "configuration": {}, + "meta": {}, + "data_source": null, + "row_form": null, + "sub_form": null, + "source": null, + "format_validators": "[]", + "default_answer": null, + "calc_expression": null, + "calc_dependents": "[]" + } +}, +{ + "model": "caluma_form.question", + "pk": "demo-outer-table-question-1", + "fields": { + "created_at": "2025-01-14T07:39:16.987Z", + "modified_at": "2025-01-14T07:39:16.987Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "label": "{\"en\": \"Outer Table Question 1\"}", + "type": "table", + "is_required": "true", + "is_hidden": "false", + "is_archived": false, + "placeholder": "{\"en\": null}", + "info_text": "{\"en\": null}", + "hint_text": "{\"en\": null}", + "static_content": "{\"en\": \"\"}", + "configuration": {}, + "meta": {}, + "data_source": null, + "row_form": "demo-table-form-1", + "sub_form": null, + "source": null, + "format_validators": "[]", + "default_answer": null, + "calc_expression": null, + "calc_dependents": "[]" + } +}, +{ + "model": "caluma_form.question", + "pk": "demo-outer-table-question-2", + "fields": { + "created_at": "2025-01-14T07:48:17.961Z", + "modified_at": "2025-01-14T07:48:17.961Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "label": "{\"en\": \"Outer Table Question 2\"}", + "type": "table", + "is_required": "true", + "is_hidden": "false", + "is_archived": false, + "placeholder": "{\"en\": null}", + "info_text": "{\"en\": null}", + "hint_text": "{\"en\": null}", + "static_content": "{\"en\": \"\"}", + "configuration": {}, + "meta": {}, + "data_source": null, + "row_form": "demo-table-form-2", + "sub_form": null, + "source": null, + "format_validators": "[]", + "default_answer": null, + "calc_expression": null, + "calc_dependents": "[]" + } +}, +{ + "model": "caluma_form.questionoption", + "pk": "demo-outer-question-1.demo-outer-question-1-outer-option-a", + "fields": { + "created_at": "2025-01-13T16:44:16.686Z", + "modified_at": "2025-01-13T16:44:16.686Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "question": "demo-outer-question-1", + "option": "demo-outer-question-1-outer-option-a", + "sort": 2 + } +}, +{ + "model": "caluma_form.questionoption", + "pk": "demo-outer-question-1.demo-outer-question-1-outer-option-b", + "fields": { + "created_at": "2025-01-13T16:44:16.680Z", + "modified_at": "2025-01-13T16:44:16.680Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "question": "demo-outer-question-1", + "option": "demo-outer-question-1-outer-option-b", + "sort": 1 + } +}, +{ + "model": "caluma_form.formquestion", + "pk": "demo-formular-1.demo-outer-table-question-1", + "fields": { + "created_at": "2025-01-14T07:39:17.045Z", + "modified_at": "2025-01-14T07:39:17.045Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "form": "demo-formular-1", + "question": "demo-outer-table-question-1", + "sort": 3 + } +}, +{ + "model": "caluma_form.formquestion", + "pk": "demo-formular-1.demo-outer-question-1", + "fields": { + "created_at": "2025-01-13T16:44:16.725Z", + "modified_at": "2025-01-13T16:44:16.725Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "form": "demo-formular-1", + "question": "demo-outer-question-1", + "sort": 2 + } +}, +{ + "model": "caluma_form.formquestion", + "pk": "demo-formular-1.demo-outer-table-question-2", + "fields": { + "created_at": "2025-01-14T07:48:18.049Z", + "modified_at": "2025-01-14T07:48:18.049Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "form": "demo-formular-1", + "question": "demo-outer-table-question-2", + "sort": 1 + } +}, +{ + "model": "caluma_form.form", + "pk": "demo-table-form-1", + "fields": { + "created_at": "2025-01-14T07:36:10.888Z", + "modified_at": "2025-01-14T07:36:18.005Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "name": "{\"en\": \"Table Form 1\"}", + "description": "{\"en\": \"\"}", + "meta": {}, + "is_published": false, + "is_archived": false, + "source": null + } +}, +{ + "model": "caluma_form.form", + "pk": "demo-table-form-2", + "fields": { + "created_at": "2025-01-14T07:45:59.594Z", + "modified_at": "2025-01-14T07:45:59.594Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "name": "{\"en\": \"Table Form 2\"}", + "description": "{\"en\": \"\"}", + "meta": {}, + "is_published": false, + "is_archived": false, + "source": null + } +}, +{ + "model": "caluma_form.question", + "pk": "demo-table-question-1", + "fields": { + "created_at": "2025-01-14T07:37:00.748Z", + "modified_at": "2025-01-14T07:38:25.075Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "label": "{\"en\": \"Table Question 1\"}", + "type": "integer", + "is_required": "true", + "is_hidden": "false", + "is_archived": false, + "placeholder": "{\"en\": null}", + "info_text": "{\"en\": null}", + "hint_text": "{\"en\": null}", + "static_content": "{\"en\": \"\"}", + "configuration": { + "max_value": null, + "min_value": null + }, + "meta": {}, + "data_source": null, + "row_form": null, + "sub_form": null, + "source": null, + "format_validators": "[]", + "default_answer": null, + "calc_expression": null, + "calc_dependents": "[\"demo-table-question-2\"]" + } +}, +{ + "model": "caluma_form.question", + "pk": "demo-table-question-2", + "fields": { + "created_at": "2025-01-14T07:38:25.084Z", + "modified_at": "2025-01-14T07:41:42.007Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "label": "{\"en\": \"Table Question 2\"}", + "type": "calculated_float", + "is_required": "false", + "is_hidden": "false", + "is_archived": false, + "placeholder": "{\"en\": null}", + "info_text": "{\"en\": \"\"}", + "hint_text": "{\"en\": \"\"}", + "static_content": "{\"en\": \"\"}", + "configuration": {}, + "meta": {}, + "data_source": null, + "row_form": null, + "sub_form": null, + "source": null, + "format_validators": "[]", + "default_answer": null, + "calc_expression": "'demo-table-question-1'|answer > 10 ? 100 : 1", + "calc_dependents": "[]" + } +}, +{ + "model": "caluma_form.question", + "pk": "demo-table-question-outer-ref-hidden", + "fields": { + "created_at": "2025-01-14T07:52:50.053Z", + "modified_at": "2025-01-14T10:01:38.803Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "label": "{\"en\": \"Table Question Outer Ref Hidden\"}", + "type": "integer", + "is_required": "true", + "is_hidden": "!'demo-outer-question-1'|answer", + "is_archived": false, + "placeholder": "{\"en\": \"\"}", + "info_text": "{\"en\": \"\"}", + "hint_text": "{\"en\": \"\"}", + "static_content": "{\"en\": \"\"}", + "configuration": { + "max_value": null, + "min_value": null + }, + "meta": {}, + "data_source": null, + "row_form": null, + "sub_form": null, + "source": null, + "format_validators": "[]", + "default_answer": null, + "calc_expression": null, + "calc_dependents": "[\"demo-table-question-outer-ref-calc\"]" + } +}, +{ + "model": "caluma_form.question", + "pk": "demo-table-question-outer-ref-calc", + "fields": { + "created_at": "2025-01-14T07:47:42.242Z", + "modified_at": "2025-01-14T10:01:38.827Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "label": "{\"en\": \"Table Question Outer Ref Calc\"}", + "type": "calculated_float", + "is_required": "false", + "is_hidden": "false", + "is_archived": false, + "placeholder": "{\"en\": null}", + "info_text": "{\"en\": \"\"}", + "hint_text": "{\"en\": \"\"}", + "static_content": "{\"en\": \"\"}", + "configuration": {}, + "meta": {}, + "data_source": null, + "row_form": null, + "sub_form": null, + "source": null, + "format_validators": "[]", + "default_answer": null, + "calc_expression": "'demo-table-question-outer-ref-hidden'|answer", + "calc_dependents": "[]" + } +}, +{ + "model": "caluma_form.formquestion", + "pk": "demo-table-form-1.demo-table-question-1", + "fields": { + "created_at": "2025-01-14T07:37:00.792Z", + "modified_at": "2025-01-14T07:37:00.792Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "form": "demo-table-form-1", + "question": "demo-table-question-1", + "sort": 2 + } +}, +{ + "model": "caluma_form.formquestion", + "pk": "demo-table-form-2.demo-table-question-outer-ref-hidden", + "fields": { + "created_at": "2025-01-14T07:52:50.098Z", + "modified_at": "2025-01-14T07:52:50.098Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "form": "demo-table-form-2", + "question": "demo-table-question-outer-ref-hidden", + "sort": 2 + } +}, +{ + "model": "caluma_form.formquestion", + "pk": "demo-table-form-1.demo-table-question-2", + "fields": { + "created_at": "2025-01-14T07:38:25.131Z", + "modified_at": "2025-01-14T07:38:25.131Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "form": "demo-table-form-1", + "question": "demo-table-question-2", + "sort": 1 + } +}, +{ + "model": "caluma_form.formquestion", + "pk": "demo-table-form-2.demo-table-question-outer-ref-calc", + "fields": { + "created_at": "2025-01-14T07:47:42.314Z", + "modified_at": "2025-01-14T07:47:42.314Z", + "created_by_user": null, + "created_by_group": null, + "modified_by_user": null, + "modified_by_group": null, + "form": "demo-table-form-2", + "question": "demo-table-question-outer-ref-calc", + "sort": 1 + } +} +] diff --git a/caluma/caluma_form/tests/test_document.py b/caluma/caluma_form/tests/test_document.py index 109a2b96e..ae24de819 100644 --- a/caluma/caluma_form/tests/test_document.py +++ b/caluma/caluma_form/tests/test_document.py @@ -363,50 +363,53 @@ def test_query_all_documents_filter_answers_by_questions( assert set(expect_data) == set(result_lengths) -@pytest.mark.parametrize("use_python_api", [True, False]) -@pytest.mark.parametrize("update", [True, False]) -def test_save_document( - db, - document, - schema_executor, - form_factory, - form_question_factory, - answer_factory, - update, - use_python_api, -): - query = """ - mutation SaveDocument($input: SaveDocumentInput!) { - saveDocument(input: $input) { - document { - form { - slug - } - id - answers { - edges { - node { - ... on StringAnswer { - strValue: value - } - ... on IntegerAnswer { - intValue: value - } - ... on FloatAnswer { - floatValue: value - } - } +SAVE_DOCUMENT_QUERY = """ + mutation SaveDocument($input: SaveDocumentInput!) { + saveDocument(input: $input) { + document { + form { + slug + } + id + answers { + edges { + node { + ... on StringAnswer { + strValue: value + } + ... on IntegerAnswer { + intValue: value + } + ... on FloatAnswer { + floatValue: value } } } - clientMutationId } } - """ + clientMutationId + } + } +""" + + +def _setup_for_save_document(form_question_factory, answer_factory, form_factory, form): + """Set up form structure for the save_document test cases. + Just to avoid copy/pasta or a complicated multiplex test. + + Use the given `form` and add some structure to it: + + * `some_int` - Integer + * (random slug) - calculated float on basis of above question + * subform (random slug) + - sub form question (random slug) - Text with default answer + """ # create an integer question that has a default answer form_question_int = form_question_factory( - question__type=Question.TYPE_INTEGER, form=document.form + question__type=Question.TYPE_INTEGER, + form=form, + question__slug="some_int", ) default_answer = answer_factory( question=form_question_int.question, value=23, document=None @@ -417,7 +420,7 @@ def test_save_document( # create a calculated question referencing the integer question we created above form_question_factory( question__type=Question.TYPE_CALCULATED_FLOAT, - form=document.form, + form=form, question__calc_expression=f"'{form_question_int.question.slug}'|answer * 2", ) @@ -425,7 +428,7 @@ def test_save_document( sub_form = form_factory() form_question_factory( question__type=Question.TYPE_FORM, - form=document.form, + form=form, question__sub_form=sub_form, ) sub_form_question = form_question_factory( @@ -437,63 +440,118 @@ def test_save_document( sub_form_question.question.default_answer = sub_default_answer sub_form_question.question.save() + +@pytest.mark.parametrize("update", [True, False]) +def test_save_document_client( + db, + document, + schema_executor, + form_factory, + form_question_factory, + answer_factory, + update, +): + """Test saving of a document via GQL client. + + We test two variants - either an Update, or a Create operation. + The test form contains two questions with a default answer, as well as + a calculated question. + + In the Create operation, we assume that the default answers are copied into + the new document, and then the calculated question is, well, calculated and + is added as an answer to the document as well, leading to three answers on the + document. + + In the Update operation, the form is actually created after the document, + and so during the form construction (see `_setup_for_save_document()`), when + the calculated question is attached to the form, it's being run, creating + an (empty) answer. But changing (or adding) default answers shall not + update existing documents, and thus the resulting number of answers in this + operation is just the one (calc) answer. + """ + _setup_for_save_document( + form_question_factory, answer_factory, form_factory, document.form + ) + default_answer = document.form.questions.get(slug="some_int").default_answer inp = { "input": extract_serializer_input_fields( serializers.DocumentSerializer, document ) } + if not update: + # not update = create = we don't pass the ID + del inp["input"]["id"] - if not use_python_api: - if not update: - # not update = create = we don't pass the ID - del inp["input"]["id"] - - result = schema_executor(query, variable_values=inp) + result = schema_executor(SAVE_DOCUMENT_QUERY, variable_values=inp) - assert not result.errors - assert ( - result.data["saveDocument"]["document"]["form"]["slug"] - == document.form.slug - ) + assert not result.errors + assert result.data["saveDocument"]["document"]["form"]["slug"] == document.form.slug - same_id = extract_global_id( - result.data["saveDocument"]["document"]["id"] - ) == str(document.id) + same_id = extract_global_id(result.data["saveDocument"]["document"]["id"]) == str( + document.id + ) - # if updating, the resulting document must be the same - assert same_id == update + # if updating, the resulting document must be the same + assert same_id == update - assert len(result.data["saveDocument"]["document"]["answers"]["edges"]) == ( - 1 if update else 3 - ) - if not update: - assert sorted( - [ - str(a["node"]["intValue"]) - if "intValue" in a["node"] - else ( - a["node"]["strValue"] - if "strValue" in a["node"] - else str(a["node"]["floatValue"]) - ) - for a in result.data["saveDocument"]["document"]["answers"]["edges"] - ] - ) == ["23", "46.0", "foo"] - else: - doc = ( - api.save_document(document.form, document=document) - if update - else api.save_document(document.form) - ) - assert (doc.pk == document.pk) == update - - assert doc.answers.count() == (1 if update else 3) - if not update: - assert sorted([str(a.value) for a in doc.answers.iterator()]) == [ - "23", - "46", - "foo", + # TODO: That 1 should probably be 0 - there are no answers around + # in the "bare" document fixture, right? + assert len(result.data["saveDocument"]["document"]["answers"]["edges"]) == ( + 1 if update else 3 + ) + if not update: + assert sorted( + [ + str(a["node"]["intValue"]) + if "intValue" in a["node"] + else ( + a["node"]["strValue"] + if "strValue" in a["node"] + else str(a["node"]["floatValue"]) + ) + for a in result.data["saveDocument"]["document"]["answers"]["edges"] ] + ) == ["23", "46.0", "foo"] + + # Make sure the default answers document is still None + default_answer.refresh_from_db() + assert default_answer.document_id is None + + +@pytest.mark.parametrize("update", [True, False]) +def test_save_document_python( + db, + document, + schema_executor, + form_factory, + form_question_factory, + answer_factory, + update, +): + """Test saving a document via the Python API. + + For detailled explanation about the expected behaviour, see the docs for + `test_save_document_client()`. + """ + _setup_for_save_document( + form_question_factory, answer_factory, form_factory, document.form + ) + default_answer = document.form.questions.get(slug="some_int").default_answer + + doc = ( + api.save_document(document.form, document=document) + if update + else api.save_document(document.form) + ) + assert (doc.pk == document.pk) == update + + assert doc.answers.count() == (1 if update else 3) + if not update: + assert sorted([str(a.value) for a in doc.answers.iterator()]) == [ + "23", + "46", + "foo", + ] # Make sure the default answers document is still None default_answer.refresh_from_db() @@ -555,6 +613,9 @@ def test_save_document_answer( # noqa:C901 use_python_api, admin_user, ): + # question needs to be part of our form + answer.document.form.questions.add(question, through_defaults={"sort": 3}) + mutation_func = mutation[0].lower() + mutation[1:] query = f""" mutation {mutation}($input: {mutation}Input!) {{ @@ -709,6 +770,7 @@ def test_save_document_table_answer_invalid_row_document( schema_executor, answer, document_factory, + form_question_factory, question, form_factory, question_factory, @@ -716,8 +778,11 @@ def test_save_document_table_answer_invalid_row_document( """Ensure that we can save incomplete row documents.""" question.row_form = form_factory() question.save() - - question.row_form.questions.add(question_factory(is_required="true")) + # question needs to be part of our form + form_question_factory( + form=question.row_form, question=question_factory(is_required="true") + ) + form_question_factory(form=answer.document.form, question=question) query = """ mutation SaveDocumentTableAnswer($input: SaveDocumentTableAnswerInput!) { @@ -769,6 +834,7 @@ def test_save_document_table_answer_setting_family( answer, answer_factory, document_factory, + form_question_factory, answer_document_factory, ): query = """ @@ -785,6 +851,8 @@ def test_save_document_table_answer_setting_family( ) } + form_question_factory(form=answer.document.form, question=answer.question) + main_pk = answer.document.pk main_family = answer.document.family remaining_document = document_factory(form=answer.question.row_form) @@ -1539,7 +1607,7 @@ def test_efficient_init_of_calc_questions( spy = mocker.spy(QuestionJexl, "evaluate") document = api.save_document(form) # twice for calc value, once for hidden state of calc-1 - assert spy.call_count == 3 + assert spy.call_count == 2 calc_ans = document.answers.get(question_id="calc-2") assert calc_ans.value == 2 diff --git a/caluma/caluma_form/tests/test_jexl.py b/caluma/caluma_form/tests/test_jexl.py index f86edf57c..f15b933e5 100644 --- a/caluma/caluma_form/tests/test_jexl.py +++ b/caluma/caluma_form/tests/test_jexl.py @@ -1,3 +1,8 @@ +import gc +import itertools +from collections import Counter +from contextlib import nullcontext as does_not_raise + import pytest from .. import models, structure, validators @@ -19,7 +24,7 @@ ], ) def test_question_jexl_validate(expression, num_errors): - jexl = QuestionJexl() + jexl = QuestionJexl(field=None) assert len(list(jexl.validate(expression))) == num_errors @@ -37,18 +42,22 @@ def test_question_jexl_validate(expression, num_errors): ], ) def test_intersects_operator(expression, result): - assert QuestionJexl().evaluate(expression) == result + assert QuestionJexl(field=None).evaluate(expression) == result @pytest.mark.parametrize("form__slug", ["f-main-slug"]) def test_jexl_form(db, form): + # TODO: this test is not really meaningful anymore with the new + # form jexl classes answer_by_question = { "a1": {"value": "A1", "form": form}, "b1": {"value": "B1", "form": form}, } assert ( - QuestionJexl({"answers": answer_by_question, "form": form}).evaluate("form") + QuestionJexl( + field=None, context={"answers": answer_by_question, "form": form.slug} + ).evaluate("form") == "f-main-slug" ) @@ -62,17 +71,14 @@ def test_all_deps_hidden(db, form, document_factory, form_question_factory): ).question document = document_factory(form=form) - qj = QuestionJexl( - { - "document": document, - "answers": {}, - "form": form, - "structure": structure.FieldSet(document, document.form), - } - ) - field = structure.Field(document, document.form, q2) - assert qj.is_hidden(field) - assert not qj.is_required(field) + struc = structure.FieldSet(document) + field = struc.get_field(q2.slug) + # Q2 is dependent on Q1 for it's hidden and required properties. + # Since Q1 is hidden, Q2 can't really evaluate both expressions. + # Therefore, is_hidden should evaluate to True, but + # is_required should evaluate to False + assert field.is_hidden() + assert not field.is_required() @pytest.mark.parametrize("fq_is_hidden", ["true", "false"]) @@ -272,9 +278,13 @@ def test_new_jexl_expressions( elif document_owner == "root_case": root_case = case_factory(workflow__slug="main-case-workflow", document=document) - # expression test method: we delete an answer and set it's is_hidden + document.refresh_from_db() + + # Expression test method: we delete an answer and set it's is_hidden # to an expression to be tested. If the expression evaluates to True, - # we won't have a ValidationError. + # the question is hidden and the missing answer is not a problem, thus + # we don't get a validation error. If it evaluates to False, the missing answer + # causes an exception, and we will know. answers[question].delete() questions[question].is_hidden = expr questions[question].save() @@ -287,7 +297,25 @@ def do_check(): except validators.CustomValidationError: # pragma: no cover return False - assert do_check() == expectation + # The following few lines are just to generate a more useful assertion + # message in case the expectations are not met. They're not influencing + # the actual result + context = [] + from caluma.caluma_form import structure + + fieldset = structure.FieldSet(document) + structure.print_structure( + fieldset, + print_fn=lambda *x: context.append(" ".join([str(f) for f in x])), + ) + ctx_str = "\n".join(context) + result = do_check() + relevant_field = fieldset.find_all_fields_by_slug(question)[0] + assert result == expectation, ( + f"Expected JEXL({expr}) on question '{question}' to evaluate " + f"to {expectation} but got {result}; context was: \n{ctx_str};\n" + f"field-local `info` context: {relevant_field.get_local_info_context()}" + ) def test_answer_transform_on_hidden_question(info, form_and_document): @@ -326,6 +354,20 @@ def test_answer_transform_on_hidden_question(info, form_and_document): questions["column"].is_hidden = "false" questions["column"].save() + # We're just validating the assumptions here for better understanding of + # the test situation + assert structure.list_document_structure(document, method=repr) == [ + " FieldSet(q=(root), f=top_form)", + " ValueField(q=top_question, v=xyz)", + " RowSet(q=table, f=row_form)", + " FieldSet(q=table, f=row_form)", + " ValueField(q=column, v=None)", # this is our test field + " FieldSet(q=form, f=sub_form)", + " ValueField(q=sub_question, v=None)", + ] + # TODO: This test fails because our new _extend_context() likely doesn't properly + # update the chainmaps as expected + validator = validators.DocumentValidator() with pytest.raises(validators.CustomValidationError): validator.validate(document, info) @@ -385,17 +427,10 @@ def test_answer_transform_on_hidden_question_types( questions["top_question"].type = question_type questions["top_question"].save() - qj = QuestionJexl( - { - "document": document, - "answers": answers, - "form": form, - "structure": structure.FieldSet(document, document.form), - } - ) + struc = structure.FieldSet(document) + field = struc.get_field("form") - field = structure.Field(document, form, questions["form"]) - assert qj.is_hidden(field) + assert field.is_hidden() @pytest.mark.parametrize( @@ -544,18 +579,11 @@ def test_is_hidden_neighboring_table( form=form, question__type=Question.TYPE_FORM, question__sub_form=neighbor_form ) - qj = QuestionJexl( - { - "document": document, - "answers": answers, - "form": form, - "structure": structure.FieldSet(document, document.form), - } - ) + struc = structure.FieldSet(document) - field = structure.Field(document, form, neighbor_sub_question) + field = struc.get_field(neighbor_sub_question.slug) - assert qj.is_hidden(field) + assert field.is_hidden() validator = validators.DocumentValidator() assert neighbor_sub_question not in validator.visible_questions(document) @@ -585,3 +613,90 @@ def test_optional_answer_transform(info, form_and_document): with pytest.raises(QuestionMissing): validator.validate(document, info) + + +def _gc_object_counts_by_type(): + # We sort, so the insertion order into the counter is something + # sorta-kinda useful. Otherwise, we'd get a random-ish insertion order + # that makes looking at the counter difficult (for humans) + o_types_sorted = sorted( + ((type(o).__module__, type(o).__qualname__) for o in gc.get_objects()) + ) + return Counter(o_types_sorted) + + +def test_jexl2_memory_leaks(info, form_and_document): + """Ensure our JEXL and form structures do not leak memory. + + We use quite a lot of forward and back references between objects, and + those need to be cleaned up when the work is done, as otherwise we may + use up a lot of memory over time. + + Measure the GC stats before and after a simple document validation call, + and if anything is left over that wasn't there before, we consider it a + bug. + """ + form, document, questions, answers = form_and_document( + use_table=True, use_subform=True + ) + + gc.collect() + + stats_before = _gc_object_counts_by_type() + stats_after = Counter() # so it's already there + + validator = validators.DocumentValidator() + validator.validate(document, info) + + # Delete the validator, and with this, everything should be gone + # To validate the test, comment-out this line, and the test should fail + del validator + + gc.collect() + stats_after = _gc_object_counts_by_type() + + # We are not concerned about django's or any other module's leaks, + # just our own + relevant_diffs = {} + for mod, cls in sorted( + set(itertools.chain(stats_before.keys(), stats_after.keys())) + ): + before = stats_before[mod, cls] + after = stats_after[mod, cls] + if mod.startswith("caluma"): + # Only look at caluma data. Graphene and Python interna are + # hard to keep track of (and not our responsibility to fix) + relevant_diffs[mod, cls] = (before, after) + + # Should be empty + for (mod, cls), (before, after) in relevant_diffs.items(): + leaked = after - before + assert leaked == 0, ( + f"Leak detected: {mod}.{cls} was {before} before test run, " + f"but {after} afterwards (leaked {leaked} objects)" + ) + + +@pytest.mark.parametrize( + "do_raise, expectation", + [ + (True, pytest.raises(RuntimeError)), + (False, does_not_raise()), + ], +) +def test_evaluate_error_no_raise(info, form_and_document, do_raise, expectation): + """When passing raise_on_error=False, ensure it's honored.""" + form, document, questions, answers = form_and_document( + use_table=False, use_subform=False + ) + + # Syntactically valid, but should cause a type error + questions["top_question"].is_hidden = "1234 / 0 == 1" + questions["top_question"].save() + + fieldset = structure.FieldSet(document) + + top_q_field = fieldset.get_field("top_question") + + with expectation: + assert top_q_field.is_hidden(raise_on_error=do_raise) is None diff --git a/caluma/caluma_form/tests/test_question.py b/caluma/caluma_form/tests/test_question.py index 66d3e781c..0e45a96c5 100644 --- a/caluma/caluma_form/tests/test_question.py +++ b/caluma/caluma_form/tests/test_question.py @@ -1005,7 +1005,7 @@ def test_calculated_question( result = schema_executor(query, variable_values=inp) assert not result.errors - calc_answer = calc_question.answers.filter().first() + calc_answer.refresh_from_db() assert calc_answer assert calc_answer.value == expected @@ -1151,6 +1151,33 @@ def test_calculated_question_update_calc_expr( assert spy.call_count == call_count +def test_recalc_missing_dependency( + db, schema_executor, form_and_document, form_question_factory, mocker +): + """ + Test recalculation behaviour for missing dependency. + + Verify the update mechanism works correctly even if a calc dependency + does not exist in a given form. + """ + form, document, questions_dict, answers_dict = form_and_document(True, True) + + sub_question = questions_dict["sub_question"] + sub_question.type = models.Question.TYPE_INTEGER + sub_question.save() + + # Calculated question in another form + form_question_factory( + # in another form entirely + question__slug="some_calc_question", + question__type=models.Question.TYPE_CALCULATED_FLOAT, + question__calc_expression="'sub_question'|answer + 1", + ).question + + # update answer - should trigger recalc + api.save_answer(sub_question, document, value=100) + + def test_calculated_question_answer_document( db, schema_executor, @@ -1263,5 +1290,7 @@ def test_init_of_calc_questions_queries( question__calc_expression="'table'|answer|mapby('column')|sum + 'top_question'|answer + 'sub_question'|answer", ) - with django_assert_num_queries(35): + with django_assert_num_queries(86): + # TODO: This used to be 35 queries - I think we need to redo the + # whole preload for the new structure to get this down again api.save_answer(questions_dict["top_question"], document, value="1") diff --git a/caluma/caluma_form/tests/test_structure.py b/caluma/caluma_form/tests/test_structure.py new file mode 100644 index 000000000..69fdca9c5 --- /dev/null +++ b/caluma/caluma_form/tests/test_structure.py @@ -0,0 +1,245 @@ +# Test cases for the (NEW!) structure utility class + +from datetime import date, datetime + +import pytest + +from caluma.caluma_form import structure +from caluma.caluma_form.models import Answer, Question + + +@pytest.fixture() +def simple_form_structure( + db, form_factory, form_question_factory, answer_factory, document_factory +): + form = form_factory(slug="root") + root_leaf1 = form_question_factory( + form=form, question__type=Question.TYPE_TEXT, question__slug="leaf1", sort=90 + ).question + root_leaf2 = form_question_factory( + form=form, + question__type=Question.TYPE_INTEGER, + question__slug="leaf2", + sort=80, + ).question + + root_formquestion = form_question_factory( + form=form, question__type=Question.TYPE_FORM, question__slug="subform", sort=70 + ).question + assert root_formquestion.sub_form + + form_question_factory( + form=root_formquestion.sub_form, + question__type=Question.TYPE_TEXT, + question__slug="sub_leaf1", + sort=60, + ) + form_question_factory( + form=root_formquestion.sub_form, + question__type=Question.TYPE_INTEGER, + question__slug="sub_leaf2", + sort=50, + ) + + sub_table = form_question_factory( + form=root_formquestion.sub_form, + question__type=Question.TYPE_TABLE, + question__slug="sub_table", + sort=40, + ).question + + row_field_1 = form_question_factory( + form=sub_table.row_form, + question__type=Question.TYPE_DATE, + question__slug="row_field_1", + sort=30, + ).question + row_field_2 = form_question_factory( + form=sub_table.row_form, + question__type=Question.TYPE_FLOAT, + question__slug="row_field_2", + sort=20, + ).question + + # row field has a dependency *outside* the row, and one *inside* + form_question_factory( + form=sub_table.row_form, + question__type=Question.TYPE_CALCULATED_FLOAT, + question__calc_expression=f"'{root_leaf2.slug}'|answer + '{row_field_2.slug}'|answer", + question__slug="row_calc", + sort=10, + ) + + root_doc = document_factory(form=form) + + answer_factory(document=root_doc, question=root_leaf1, value="Some Value") + answer_factory(document=root_doc, question=root_leaf2, value=33) + table_ans = answer_factory(document=root_doc, question=sub_table) + + row_doc1 = document_factory(form=sub_table.row_form) + answer_factory(document=row_doc1, question=row_field_1, date=datetime(2025, 1, 13)) + answer_factory(document=row_doc1, question=row_field_2, value=99.5) + + row_doc2 = document_factory(form=sub_table.row_form) + answer_factory(document=row_doc2, question=row_field_1, date=datetime(2025, 1, 10)) + answer_factory(document=row_doc2, question=row_field_2, value=23.0) + + table_ans.documents.add(row_doc1) + table_ans.documents.add(row_doc2) + + return root_doc + + +def test_printing_structure(simple_form_structure): + out_lines = [] + + def fake_print(*args): + out_lines.append(" ".join([str(x) for x in args])) + + fieldset = structure.FieldSet(simple_form_structure) + structure.print_structure(fieldset, print_fn=fake_print) + + assert out_lines == [ + " FieldSet(root)", + " Field(leaf1, Some Value)", + " Field(leaf2, 33)", + " FieldSet(measure-evening)", + " Field(sub_leaf1, None)", + " Field(sub_leaf2, None)", + " RowSet(too-wonder-option)", + " FieldSet(too-wonder-option)", + " Field(row_field_1, 2025-01-13)", + " Field(row_field_2, 99.5)", + " Field(row_calc, None)", + " FieldSet(too-wonder-option)", + " Field(row_field_1, 2025-01-10)", + " Field(row_field_2, 23.0)", + " Field(row_calc, None)", + ] + + +@pytest.mark.parametrize("empty", [True, False]) +def test_root_getvalue(simple_form_structure, empty): + if empty: + Answer.objects.all().delete() + + fieldset = structure.FieldSet(simple_form_structure) + + expected_value = ( + {} + if empty + else { + "leaf1": "Some Value", + "leaf2": 33, + "subform": { + "sub_leaf1": None, + "sub_leaf2": None, + "sub_table": [ + { + "row_calc": None, + "row_field_1": date(2025, 1, 13), + "row_field_2": 99.5, + }, + { + "row_calc": None, + "row_field_1": date(2025, 1, 10), + "row_field_2": 23.0, + }, + ], + }, + } + ) + assert fieldset.get_value() == expected_value + + +@pytest.mark.parametrize("hidden", ["true", "false"]) +def test_hidden_fieldset(simple_form_structure, hidden): + # We hide the subform to test it's get_value() result + subform_q = Question.objects.get(slug="subform") + subform_q.is_hidden = hidden + subform_q.save() + + fieldset = structure.FieldSet(simple_form_structure) + + expected_value = ( + { + "leaf1": "Some Value", + "leaf2": 33, + "subform": {}, + } + if (hidden == "true") + else { + "leaf1": "Some Value", + "leaf2": 33, + "subform": { + "sub_leaf1": None, + "sub_leaf2": None, + "sub_table": [ + { + "row_calc": None, + "row_field_1": date(2025, 1, 13), + "row_field_2": 99.5, + }, + { + "row_calc": None, + "row_field_1": date(2025, 1, 10), + "row_field_2": 23.0, + }, + ], + }, + } + ) + assert fieldset.get_value() == expected_value + + +def test_find_missing_answer(simple_form_structure, answer_factory): + fieldset = structure.FieldSet(simple_form_structure) + + # Find unrelated answer - should not be found + assert fieldset.find_field_by_answer(answer_factory()) is None + + +def test_hidden_root_field(simple_form_structure): + fieldset = structure.FieldSet(simple_form_structure) + + # Can never fail: Root field must always be visible (as it doesn't have + # a question associated... ¯\_(ツ)_/¯) + assert not fieldset.is_hidden() + + +def test_empty_formfield(simple_form_structure): + """Verify form fields to be empty if none of their sub fields have answers. + + If a form question (subform) has no answer within, we consider it empty + and `is_empty()` should return `True`. + """ + fieldset = structure.FieldSet(simple_form_structure) + + subform = fieldset.get_field("subform") + for field in subform.get_all_fields(): + if field.answer: + field.answer.delete() + + # new fieldset to avoid caching leftovers + fieldset = structure.FieldSet(simple_form_structure) + subform = fieldset.get_field("subform") + + assert subform.is_empty() + + +def test_hidden_formfield(simple_form_structure): + """A hidden fieldset (form field) should be considered empty when hidden. + + As hidden questions are not shown, their value should also not be considered + during evaluation. Therefore, `is_empty()` should return True if the + corresponding question is hidden. + """ + fieldset = structure.FieldSet(simple_form_structure) + + subform = fieldset.get_field("subform") + subform.question.is_hidden = "true" + subform.question.save() + + # The form is not really empty, but it's hidden, so is_empty() should + # return True nonetheless + assert subform.is_empty() diff --git a/caluma/caluma_form/tests/test_validators.py b/caluma/caluma_form/tests/test_validators.py index c353f14cc..747a392c7 100644 --- a/caluma/caluma_form/tests/test_validators.py +++ b/caluma/caluma_form/tests/test_validators.py @@ -2,11 +2,11 @@ import pytest from rest_framework.exceptions import ValidationError +from ...caluma_core.exceptions import QuestionMissing from ...caluma_core.tests import extract_serializer_input_fields from ...caluma_form import api from ...caluma_form.models import DynamicOption, Question from .. import serializers, structure -from ..jexl import QuestionMissing from ..validators import DocumentValidator, QuestionValidator @@ -34,7 +34,7 @@ def test_validate_hidden_required_field( form_question.question.save() document = document_factory(form=form_question.form) - error_msg = f"Questions {form_question.question.slug} are required but not provided" + error_msg = f"Question {form_question.question.slug} is required but not provided" if should_throw: with pytest.raises(ValidationError, match=error_msg): DocumentValidator().validate(document, admin_user) @@ -252,7 +252,7 @@ def test_validate_table( q_slug = sub_question_b.question.slug if required_jexl_sub == "false": q_slug = other_q_2.question.slug - error_msg = f"Questions {q_slug} are required but not provided" + error_msg = f"Question {q_slug} is required but not provided" with pytest.raises(ValidationError, match=error_msg): DocumentValidator().validate(main_document, admin_user) else: @@ -315,7 +315,7 @@ def test_validate_empty_answers( ): struct = structure.FieldSet(document, document.form) field = struct.get_field(question.slug) - answer_value = field.value() if field else field + answer_value = field.get_value() if field else field assert answer_value == expected_value @@ -328,7 +328,7 @@ def test_validate_empty_answers( "'foo' in blah", "false", "", - "Error while evaluating `is_required` expression on question q-slug: 'foo' in blah. The system log contains more information", + "Error while evaluating expression on question q-slug: \"'foo' in blah\". The system log contains more information", ), ( "q-slug", @@ -336,7 +336,7 @@ def test_validate_empty_answers( "true", "'foo' in blah", "", - "Error while evaluating `is_hidden` expression on question q-slug: 'foo' in blah. The system log contains more information", + "Error while evaluating expression on question q-slug: \"'foo' in blah\". The system log contains more information", ), ( "q-slug", @@ -489,7 +489,7 @@ def test_validate_hidden_subform( DocumentValidator().validate(document, admin_user) # Verify that the sub_sub_question is not the cause of the exception: # it should not be checked at all because it's parent is always hidden - assert excinfo.match(r"Questions \bsub_question\s.* required but not provided.") + assert excinfo.match(r"Question sub_question is required but not provided.") # can't do `not excinfo.match()` as it throws an AssertionError itself # if it can't match :() @@ -620,7 +620,7 @@ def test_validate_missing_in_subform( # Verify that the sub_sub_question is not the cause of the exception: # it should not be checked at all because it's parent is always hidden - assert excinfo.match(r"Questions \bsub_question\s.* required but not provided.") + assert excinfo.match(r"Question sub_question is required but not provided.") @pytest.mark.parametrize("table_required", [True, False]) @@ -687,9 +687,7 @@ def test_validate_missing_in_table( DocumentValidator().validate(document, admin_user) # Verify that the sub_sub_question is not the cause of the exception: # it should not be checked at all because it's parent is always hidden - assert excinfo.match( - r"Questions \bsub_question2\s.* required but not provided." - ) + assert excinfo.match(r"Question sub_question2 is required but not provided.") else: # should not raise @@ -698,7 +696,14 @@ def test_validate_missing_in_table( @pytest.mark.parametrize( "column_is_hidden,expect_error", - [("form == 'topform'", False), ("form == 'rowform'", True)], + [ + # `form == 'topform'` should evaluate to True, the question is hidden + # and the answer therefore not required. + ("form == 'topform'", False), + # `form == 'rowform'` should evaluate to False, the question is required + # and the missing answer therefore triggers an error. + ("form == 'rowform'", True), + ], ) def test_validate_form_in_table( db, @@ -712,6 +717,11 @@ def test_validate_form_in_table( expect_error, admin_user, ): + """Ensure that the `form` expression is correct within a table row. + + The `form` expression should refer to the root form (legacy, you should + actually use `info.root_form` instead) + """ # First, build our nested form: # top_form # \__ table_question # requiredness parametrized @@ -765,9 +775,7 @@ def test_validate_form_in_table( DocumentValidator().validate(document, admin_user) # Verify that the sub_sub_question is not the cause of the exception: # it should not be checked at all because it's parent is always hidden - assert excinfo.match( - r"Questions \bsub_question2\s.* required but not provided." - ) + assert excinfo.match(r"Question sub_question2 is required but not provided.") else: # Should not raise, as the "form" referenced by the diff --git a/caluma/caluma_form/utils.py b/caluma/caluma_form/utils.py index f7ac560bd..cee93723c 100644 --- a/caluma/caluma_form/utils.py +++ b/caluma/caluma_form/utils.py @@ -1,8 +1,12 @@ +from logging import getLogger + from django.db.models import Prefetch from caluma.caluma_form import models, structure from caluma.caluma_form.jexl import QuestionJexl +log = getLogger(__name__) + def prefetch_document(document_id): """Fetch a document while prefetching the entire structure. @@ -83,7 +87,18 @@ def _build_document_prefetch_statements(prefix="", prefetch_options=False): def update_calc_dependents(slug, old_expr, new_expr): - jexl = QuestionJexl() + """Update the calc_dependents lists of our calc *dependencies*. + + The given (old and new) expressions are analyzed to see which + questions are referenced in "our" calc question. Then, those + questions' calc_dependents list is updated, such that it correctly + represents the new situation. + + Example: If our expression newly contains the question `foo`, then the + `foo` question needs to know about it (we add "our" slug to the `foo`s + calc dependents) + """ + jexl = QuestionJexl(field=None) old_q = set( list(jexl.extract_referenced_questions(old_expr)) + list(jexl.extract_referenced_mapby_questions(old_expr)) @@ -108,37 +123,50 @@ def update_calc_dependents(slug, old_expr, new_expr): question.save() -def update_or_create_calc_answer( - question, document, struc=None, update_dependents=True +def recalculate_field( + calc_field: structure.ValueField, update_recursively: bool = True ): - root_doc = document.family + """Recalculate the given value field and store the new answer. - if not struc: - struc = structure.FieldSet(root_doc, root_doc.form) - - field = struc.get_field(question.slug) - - # skip if question doesn't exist in this document structure - if field is None: - return - - jexl = QuestionJexl( - {"form": field.form, "document": field.document, "structure": field.parent()} - ) + If it's not a calculated field, nothing happens. + """ + if calc_field.question.type != models.Question.TYPE_CALCULATED_FLOAT: + # Not a calc field - skip + return # pragma: no cover - # Ignore errors because we evaluate greedily as soon as possible. At - # this moment we might be missing some answers or the expression might - # be invalid, in which case we return None - value = jexl.evaluate(field.question.calc_expression, raise_on_error=False) + value = calc_field.calculate() answer, _ = models.Answer.objects.update_or_create( - question=question, document=field.document, defaults={"value": value} + question=calc_field.question, + document=calc_field.parent._document, + defaults={"value": value}, ) # also save new answer to structure for reuse - struc.set_answer(question.slug, answer) + calc_field.refresh(answer) + if update_recursively: + recalculate_dependent_fields(calc_field, update_recursively) + + +def recalculate_dependent_fields( + changed_field: structure.ValueField, update_recursively: bool = True +): + """Update any calculated dependencies of the given field. + + If `update_recursively=False` is passed, no subsequent calc dependencies + are updated (left to the caller in that case). + """ + for dep_slug in changed_field.question.calc_dependents: + dep_field = changed_field.get_field(dep_slug) + if not dep_field: # pragma: no cover + # Calculated field is not in our form structure, which is + # absolutely valid and OK + continue + recalculate_field(dep_field, update_recursively) + + +def update_or_create_calc_answer(question, document, update_dependents=True): + """Recalculate all answers in the document after calc dependency change.""" - if update_dependents: - for _question in models.Question.objects.filter( - pk__in=field.question.calc_dependents - ): - update_or_create_calc_answer(_question, document, struc) + root = structure.FieldSet(document.family) + for field in root.find_all_fields_by_slug(question.slug): + recalculate_field(field, update_recursively=update_calc_dependents) diff --git a/caluma/caluma_form/validators.py b/caluma/caluma_form/validators.py index a57a14431..cc0b0b56c 100644 --- a/caluma/caluma_form/validators.py +++ b/caluma/caluma_form/validators.py @@ -1,5 +1,4 @@ import sys -from collections import defaultdict from datetime import date from logging import getLogger @@ -8,10 +7,11 @@ from rest_framework import exceptions from caluma.caluma_data_source.data_source_handlers import get_data_sources +from caluma.caluma_form import structure +from caluma.caluma_workflow.models import Case -from . import jexl, models, structure +from . import jexl, models from .format_validators import get_format_validators -from .jexl import QuestionJexl from .models import DynamicOption, Question log = getLogger() @@ -92,35 +92,23 @@ def _validate_question_date(self, question, value, **kwargs): def _evaluate_options_jexl( self, document, question, validation_context=None, qs=None ): - def _validation_context(document): - # we need to build the context in two steps (for now), as - # `self.visible_questions()` already needs a context to evaluate - # `is_hidden` expressions - intermediate_context = { - "form": document.family.form, - "document": document.family, - "visible_questions": None, - "jexl_cache": defaultdict(dict), - "structure": structure.FieldSet(document.family, document.family.form), - } - - return intermediate_context + """Return a list of slugs that are, according to their is_hidden JEXL, visible.""" options = qs if qs else question.options.all() - # short circuit if none of the options has an `is_hidden`-jexl + # short circuit if none of the options has an `is_hidden`-jexl. if not options.exclude( Q(is_hidden="false") | Q(is_hidden__isnull=True) ).exists(): return [o.slug for o in options] - validation_context = validation_context or _validation_context(document) + validation_context = validation_context or structure.FieldSet(document.family) - jexl = QuestionJexl(validation_context) - with jexl.use_field_context( - validation_context["structure"].get_field(question.slug) - ): - return [o.slug for o in options if not jexl.evaluate(o.is_hidden)] + my_field = validation_context.find_field_by_document_and_question( + document.pk, question.pk + ) + + return [o.slug for o in options if not my_field.evaluate_jexl(o.is_hidden)] def visible_options(self, document, question, qs): return self._evaluate_options_jexl(document, question, qs=qs) @@ -218,11 +206,22 @@ def _remove_unused_dynamic_options(self, question, document, used_values): ).delete() def _validate_question_table( - self, question, value, document, user, instance=None, origin=False, **kwargs + self, + question, + value, + document, + user, + validation_context: structure.RowSet, + instance=None, + origin=False, + **kwargs, ): if not origin: - for row_doc in value: - DocumentValidator().validate(row_doc, user=user, **kwargs) + # Rowsets should always be validated in context, so we can use that + for row in validation_context.children(): + DocumentValidator().validate( + row._document, user=user, **kwargs, validation_context=row + ) return # this answer was the entry-point of the validation @@ -312,165 +311,109 @@ def validate( if not validation_context: validation_context = self._validation_context(document) - self._validate_required(validation_context) + required_but_empty = [] - for answer in document.answers.filter( - question_id__in=validation_context["visible_questions"] - ): - validator = AnswerValidator() - validator.validate( - document=document, - question=answer.question, - value=answer.value, - documents=answer.documents.all(), - user=user, - validation_context=validation_context, - data_source_context=data_source_context, + all_fields = list(validation_context.get_all_fields()) + for field in all_fields: + if field.is_required() and field.is_empty(): + required_but_empty.append(field) + + if required_but_empty: + # When dealing with tables, the same question slug + # might be missing multiple times. So we're normalizing a bit here + affected_slugs = sorted( + set([f.slug() for f in required_but_empty if f.slug()]) + ) + + question_s, is_are = ( + ("Question", "is") if len(affected_slugs) == 1 else ("Questions", "are") + ) + raise CustomValidationError( + f"{question_s} {', '.join(affected_slugs)} " + f"{is_are} required but not provided.", + slugs=affected_slugs, ) + for field in all_fields: + if field.is_visible() and field.answer: + # if answer is not given, the required_but_empty check above would + # already have raised an exception + validator = AnswerValidator() + validator.validate( + document=field.answer.document, + question=field.question, + value=field.answer.value, + documents=field.answer.documents.all(), + user=user, + # TODO those contexts should probably be dropped, + # or passed along as the correct field + # form the structure2 object + validation_context=field, + data_source_context=data_source_context, + ) + def _validation_context(self, document): - # we need to build the context in two steps (for now), as - # `self.visible_questions()` already needs a context to evaluate - # `is_hidden` expressions - intermediate_context = { - "form": document.form, - "document": document, - "visible_questions": None, - "jexl_cache": defaultdict(dict), - "structure": structure.FieldSet(document, document.form), - } + relevant_case: Case = ( + getattr(document, "case", None) + or getattr(getattr(document, "work_item", None), "case", None) + or None + ) - intermediate_context["visible_questions"] = self.visible_questions( - document, intermediate_context + case_form = ( + relevant_case.document.form_id + if relevant_case and relevant_case.document + else None + ) + case_family_form = ( + relevant_case.family.document.form_id + if relevant_case and relevant_case.family.document + else None + ) + case_info = ( + { + # Why are we even represent this in context of the case? + # The form does not belong there, it's not even there in + # the model + "form": case_form, + "workflow": relevant_case.workflow_id, + "root": { + "form": case_family_form, + "workflow": relevant_case.family.workflow_id, + }, + } + if relevant_case + else None ) - return intermediate_context + workitem_info = ( + document.work_item.__dict__ if hasattr(document, "work_item") else None + ) + context = { + # TODO: Build the global context so it's the same as it was before + # the refactoring + "info": { + "document": document.family, + "form": document.form, + "case": case_info, + "work_item": workitem_info, + } + } + + return structure.FieldSet(document, global_context=context) - def visible_questions(self, document, validation_context=None): + def visible_questions(self, document) -> list[Question]: """Evaluate the visibility of the questions for the given context. This evaluates the `is_hidden` expression for each question to decide if the question is visible. Return a list of question slugs that are visible. """ - if not validation_context: - validation_context = self._validation_context(document) - visible_questions = [] - - q_jexl = jexl.QuestionJexl(validation_context) - for field in validation_context["structure"].children(): - question = field.question - try: - is_hidden = q_jexl.is_hidden(field) - - if is_hidden: - # no need to descend further - continue - - visible_questions.append(question.slug) - if question.type == Question.TYPE_FORM: - # answers to questions in subforms are still in - # the top level document - sub_context = {**validation_context, "structure": field} - visible_questions.extend( - self.visible_questions(document, sub_context) - ) - - elif question.type == Question.TYPE_TABLE: - row_visibles = set() - # make a copy of the validation context, so we - # can reuse it for each row - row_context = {**validation_context} - for row in field.children(): - sub_context = {**row_context, "structure": row} - row_visibles.update( - self.visible_questions(document, sub_context) - ) - visible_questions.extend(row_visibles) - - 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)}" - ) - raise RuntimeError( - f"Error while evaluating `is_hidden` expression on question {question.slug}: " - f"{question.is_hidden}. The system log contains more information" - ) - return visible_questions + validation_context = self._validation_context(document) - def _validate_required(self, validation_context): # noqa: C901 - """Validate the 'requiredness' of the given answers. - - Raise exceptions if a required question is not answered. - - Since we're iterating and evaluating `is_hidden` as well for this - purpose, we help our call site by returning a list of *non-hidden* - question slugs. - """ - required_but_empty = [] - - q_jexl = jexl.QuestionJexl(validation_context) - for field in validation_context["structure"].children(): - question = field.question - try: - is_hidden = question.slug not in validation_context["visible_questions"] - if is_hidden: - continue - - # 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_field_context(field): - if q_jexl.is_hidden(field): - 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 - sub_context = {**validation_context, "structure": field} - self._validate_required(sub_context) - - elif question.type == Question.TYPE_TABLE: - # We need to validate presence in at least one row, but only - # if the table question is required. - if is_required and not field.children(): - raise CustomValidationError( - f"no rows in {question.slug}", slugs=[question.slug] - ) - for row in field.children(): - sub_context = {**validation_context, "structure": row} - self._validate_required(sub_context) - else: - value = field.value() - if value in EMPTY_VALUES and is_required: - required_but_empty.append(question.slug) - - except CustomValidationError as exc: - required_but_empty.extend(exc.slugs) - - except (jexl.QuestionMissing, exceptions.ValidationError): - raise - except Exception as exc: - log.error( - f"Error while evaluating `is_required` expression on question {question.slug}: " - f"{question.is_required}: {str(exc)}" - ) - - raise RuntimeError( - f"Error while evaluating `is_required` expression on question {question.slug}: " - f"{question.is_required}. The system log contains more information" - ) - - if required_but_empty: - raise CustomValidationError( - f"Questions {','.join(required_but_empty)} are required but not provided.", - slugs=required_but_empty, - ) + return [ + field.question + for field in validation_context.get_all_fields() + if field.is_visible() + ] class QuestionValidator: @@ -498,7 +441,7 @@ def _validate_calc_expression(data): if not expr: return - question_jexl = jexl.QuestionJexl() + question_jexl = jexl.QuestionJexl(field=None) deps = set(question_jexl.extract_referenced_questions(expr)) inexistent_slugs = deps - set( diff --git a/caluma/conftest.py b/caluma/conftest.py index 6f6c0c51f..b5ab73d8e 100644 --- a/caluma/conftest.py +++ b/caluma/conftest.py @@ -2,6 +2,7 @@ import datetime import functools import inspect +import itertools import sys from collections import defaultdict @@ -265,11 +266,15 @@ def form_and_document( * sub_form: sub_form * question: sub_question """ + sorter = itertools.count(9999, -1) def fallback_factory(factory, **kwargs): existing = factory._meta.model.objects.filter(**kwargs).first() if existing: return existing + if factory is form_question_factory: + # Make form structure / sorting deterministic + kwargs["sort"] = next(sorter) return factory(**kwargs) def factory(use_table=False, use_subform=False, table_row_count=1):