From 0788067caa465bb590cd919348f72a495b8e33e9 Mon Sep 17 00:00:00 2001
From: Christian Zosel <christian@zosel.ch>
Date: Thu, 9 Jan 2025 11:10:59 +0100
Subject: [PATCH] fix(signals): only recompute calc answers when calc expr
 changed

---
 caluma/caluma_form/signals.py             | 34 ++++++++++++++++++++-
 caluma/caluma_form/tests/test_question.py | 37 +++++++++++++++++++++++
 caluma/caluma_form/utils.py               |  3 +-
 3 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/caluma/caluma_form/signals.py b/caluma/caluma_form/signals.py
index 7cb4fc3b0..d5f1db592 100644
--- a/caluma/caluma_form/signals.py
+++ b/caluma/caluma_form/signals.py
@@ -1,6 +1,7 @@
 from django.db.models.signals import (
     post_delete,
     post_init,
+    post_save,
     pre_delete,
     pre_save,
 )
@@ -11,7 +12,7 @@
 from caluma.utils import disable_raw
 
 from . import models
-from .utils import update_calc_dependents
+from .utils import update_calc_dependents, update_or_create_calc_answer
 
 
 @receiver(pre_create_historical_record, sender=models.HistoricalAnswer)
@@ -70,6 +71,7 @@ def save_calc_dependents(sender, instance, **kwargs):
         update_calc_dependents(
             instance.slug, old_expr="false", new_expr=instance.calc_expression
         )
+        instance.calc_expression_changed = True
 
     elif original.calc_expression != instance.calc_expression:
         update_calc_dependents(
@@ -77,6 +79,9 @@ def save_calc_dependents(sender, instance, **kwargs):
             old_expr=original.calc_expression,
             new_expr=instance.calc_expression,
         )
+        instance.calc_expression_changed = True
+    else:
+        instance.calc_expression_changed = False
 
 
 @receiver(pre_delete, sender=models.Question)
@@ -85,3 +90,30 @@ def remove_calc_dependents(sender, instance, **kwargs):
     update_calc_dependents(
         instance.slug, old_expr=instance.calc_expression, new_expr="false"
     )
+
+
+# Update calculated answer on post_save
+#
+# Try to update the calculated answer value whenever a mutation on a possibly
+# related model is performed.
+
+
+@receiver(post_save, sender=models.Question)
+@disable_raw
+@filter_events(lambda instance: instance.type == models.Question.TYPE_CALCULATED_FLOAT)
+def update_calc_from_question(sender, instance, created, update_fields, **kwargs):
+    if not instance.calc_expression_changed:
+        return
+
+    for document in models.Document.objects.filter(form__questions=instance):
+        update_or_create_calc_answer(instance, document)
+
+
+@receiver(post_save, sender=models.FormQuestion)
+@disable_raw
+@filter_events(
+    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():
+        update_or_create_calc_answer(instance.question, document)
diff --git a/caluma/caluma_form/tests/test_question.py b/caluma/caluma_form/tests/test_question.py
index f4da818ca..ccc148387 100644
--- a/caluma/caluma_form/tests/test_question.py
+++ b/caluma/caluma_form/tests/test_question.py
@@ -8,6 +8,7 @@
 )
 
 from .. import api, models, serializers
+from ..jexl import QuestionJexl
 
 
 @pytest.mark.parametrize(
@@ -1106,6 +1107,42 @@ def test_recursive_calculated_question(
     assert calc_ans.value == 8
 
 
+def test_calculated_question_update_calc_expr(
+    db, schema_executor, form_and_document, form_question_factory, mocker
+):
+    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()
+    sub_question_a = answers_dict["sub_question"]
+    sub_question_a.value = 100
+    sub_question_a.save()
+
+    calc_question = form_question_factory(
+        form=form,
+        question__slug="calc_question",
+        question__type=models.Question.TYPE_CALCULATED_FLOAT,
+        question__calc_expression="'sub_question'|answer + 1",
+    ).question
+
+    calc_ans = document.answers.get(question_id="calc_question")
+    assert calc_ans.value == 101
+    # spying on update_or_create_calc_answer doesn't seem to work, so we spy on QuestionJexl.evaluate instead
+    spy = mocker.spy(QuestionJexl, "evaluate")
+    calc_question.calc_expression = "'sub_question'|answer -1"
+    calc_question.save()
+    assert spy.call_count > 0
+    call_count = spy.call_count
+    calc_ans.refresh_from_db()
+    assert calc_ans.value == 99
+
+    calc_question.label = "New Label"
+    calc_question.save()
+    # if the calc expression is not changed, no jexl evaluation should be done
+    assert spy.call_count == call_count
+
+
 def test_calculated_question_answer_document(
     db,
     schema_executor,
diff --git a/caluma/caluma_form/utils.py b/caluma/caluma_form/utils.py
index d8d95e4a6..761c7c0a8 100644
--- a/caluma/caluma_form/utils.py
+++ b/caluma/caluma_form/utils.py
@@ -17,7 +17,8 @@ def build_document_prefetch_statements(prefix="", prefetch_options=False):
 
     This is needed to reduce the query count when almost all the form data
     is needed for a given document, e.g. when recalculating calculated
-    answers.
+    answers: in order to evaluate calc expressions the complete document
+    structure is needed, which would otherwise result in a lot of queries.
     """
     question_queryset = models.Question.objects.select_related(
         "sub_form", "row_form"