Skip to content

Commit

Permalink
fix(signals): only recompute calc answers when calc expr changed
Browse files Browse the repository at this point in the history
  • Loading branch information
czosel committed Jan 9, 2025
1 parent 5c07195 commit 0788067
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 2 deletions.
34 changes: 33 additions & 1 deletion caluma/caluma_form/signals.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from django.db.models.signals import (
post_delete,
post_init,
post_save,
pre_delete,
pre_save,
)
Expand All @@ -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)
Expand Down Expand Up @@ -70,13 +71,17 @@ 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(
instance.slug,
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)
Expand All @@ -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)
37 changes: 37 additions & 0 deletions caluma/caluma_form/tests/test_question.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
)

from .. import api, models, serializers
from ..jexl import QuestionJexl


@pytest.mark.parametrize(
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion caluma/caluma_form/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 0788067

Please sign in to comment.