Skip to content

Commit 8cccc7e

Browse files
wingedczosel
authored andcommitted
fix(structure): correctly update structure in-place for recalculations
When calculating a field, we must take better care to update the dependents, invalidate all the cached values, and ensure the intermediate values are correctly used.
1 parent 3e0f0a0 commit 8cccc7e

File tree

3 files changed

+73
-5
lines changed

3 files changed

+73
-5
lines changed

caluma/caluma_form/structure.py

+14-4
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import collections
2020
import copy
21+
import itertools
2122
import typing
2223
import weakref
2324
from abc import ABC
@@ -175,9 +176,13 @@ def _store_question(self, question):
175176
continue
176177
# Evaluator is only used for extracting answer transforms & friends,
177178
# so is field-independent here
178-
for dependency_slug in self._evaluator.extract_referenced_questions(
179-
jexl_expr
180-
):
179+
referenced_questions = set(
180+
itertools.chain(
181+
self._evaluator.extract_referenced_questions(jexl_expr),
182+
self._evaluator.extract_referenced_mapby_questions(jexl_expr),
183+
)
184+
)
185+
for dependency_slug in referenced_questions:
181186
self._jexl_dependencies[dependency_slug][question.pk].append(
182187
expr_property
183188
)
@@ -553,6 +558,12 @@ def refresh(self, answer=None):
553558
"""
554559
if answer:
555560
self.answer = answer
561+
elif self.answer:
562+
self.answer.refresh_from_db()
563+
elif not isinstance(self, FieldSet):
564+
self.answer = Answer.objects.filter(
565+
question=self.question, document=self.parent._document
566+
).first()
556567

557568
clear_memoise(self)
558569
for dep in self._fastloader.dependents_of_question(self.slug()):
@@ -754,7 +765,6 @@ def is_empty(self):
754765
def get_context(self):
755766
return self._context
756767

757-
@object_local_memoise
758768
def get_value(self):
759769
if self.is_empty():
760770
# is_empty() will return True if we're hidden, so

caluma/caluma_form/tests/test_question.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -1167,7 +1167,7 @@ def test_recalc_missing_dependency(
11671167
sub_question.type = models.Question.TYPE_INTEGER
11681168
sub_question.save()
11691169

1170-
spy = mocker.spy(domain_logic.SaveAnswerLogic, "update_calc_dependents")
1170+
spy = mocker.spy(domain_logic.SaveAnswerLogic, "recalculate_dependents")
11711171

11721172
# Calculated question in another form
11731173
form_question_factory(

caluma/caluma_form/tests/test_structure.py

+58
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import pytest
66

77
from caluma.caluma_form import structure
8+
from caluma.caluma_form.api import save_answer
89
from caluma.caluma_form.models import Answer, Document, FormQuestion, Question
910

1011

@@ -422,3 +423,60 @@ def test_fastloader_no_duplicate_options(
422423
assert len(the_options) == len(
423424
struc._fastloader.options_for_question(choice_q.slug)
424425
)
426+
427+
428+
def test_multistage_calculation_updates(
429+
simple_form_structure,
430+
form_factory,
431+
django_assert_num_queries,
432+
form_question_factory,
433+
):
434+
"""Verify recalculation behaviour across multiple calc stages.
435+
436+
A chain of calculations across tables is done to ensure we get the right
437+
values at each step and don't end up in a cache trap
438+
"""
439+
form_question_factory(
440+
form_id="root",
441+
question__type=Question.TYPE_CALCULATED_FLOAT,
442+
question__slug="outer-calc",
443+
question__calc_expression="'sub_table'|answer([])|mapby('row_calc')|sum",
444+
)
445+
446+
struc0 = structure.FieldSet(simple_form_structure)
447+
448+
assert struc0._fastloader._questions["leaf2"].calc_dependents == ["row_calc"]
449+
assert struc0._fastloader._questions["row_field_2"].calc_dependents == ["row_calc"]
450+
assert struc0._fastloader._questions["row_calc"].calc_dependents == ["outer-calc"]
451+
assert struc0._fastloader._questions["outer-calc"].calc_dependents == []
452+
453+
for field in struc0.find_all_fields_by_slug("row_field_2"):
454+
answer = save_answer(
455+
field.question, field.answer.document, value=field.answer.value + 2
456+
)
457+
field.refresh(answer)
458+
459+
struc1 = structure.FieldSet(simple_form_structure)
460+
461+
# The struc0 should be internally-refreshed, so must match another
462+
# structure constructed freshly from DB
463+
assert struc0.list_structure() == struc1.list_structure()
464+
465+
assert struc1.list_structure() == [
466+
" FieldSet(root)",
467+
" Field(leaf1, Some Value)",
468+
" Field(leaf2, 33)",
469+
" FieldSet(measure-evening)",
470+
" Field(sub_leaf1, None)",
471+
" Field(sub_leaf2, None)",
472+
" RowSet(too-wonder-option)",
473+
" FieldSet(too-wonder-option)",
474+
" Field(row_field_1, 2025-01-13)",
475+
" Field(row_field_2, 101.5)",
476+
" Field(row_calc, 134.5)",
477+
" FieldSet(too-wonder-option)",
478+
" Field(row_field_1, 2025-01-10)",
479+
" Field(row_field_2, 25.0)",
480+
" Field(row_calc, 58.0)",
481+
" Field(outer-calc, 192.5)",
482+
]

0 commit comments

Comments
 (0)