From cb668ba88371fe5d48ee4402348ec9182068ee18 Mon Sep 17 00:00:00 2001 From: Fabio Ambauen Date: Fri, 17 Feb 2023 11:50:39 +0100 Subject: [PATCH] fix: fix calc answer evaluation when adding new table rows --- .../0047_recalculate_calc_answers.py | 54 +++++++++++++ caluma/caluma_form/signals.py | 10 +-- caluma/caluma_form/tests/test_migration.py | 81 +++++++++++++++++++ 3 files changed, 140 insertions(+), 5 deletions(-) create mode 100644 caluma/caluma_form/migrations/0047_recalculate_calc_answers.py diff --git a/caluma/caluma_form/migrations/0047_recalculate_calc_answers.py b/caluma/caluma_form/migrations/0047_recalculate_calc_answers.py new file mode 100644 index 000000000..bdd24c9e9 --- /dev/null +++ b/caluma/caluma_form/migrations/0047_recalculate_calc_answers.py @@ -0,0 +1,54 @@ +from django.db import migrations + +from caluma.caluma_form.jexl import QuestionJexl +from caluma.caluma_form.structure import FieldSet + + +def recalculate_answer(answer): + """ + Recalculate a single calc-answer. + + This is more or less a copy/paste from `_update_or_create_calc_answer` in order to + use the old models. + """ + root_doc = answer.document.family + + struc = FieldSet(root_doc, root_doc.form) + field = struc.get_field(answer.question.slug) + + # skip if question doesn't exist in this document structure + if field is None: # pragma: no cover + return + + jexl = QuestionJexl( + {"form": field.form, "document": field.document, "structure": field.parent()} + ) + + # Ignore errors because we might evaluate partially built forms. So 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) + + answer.value = value + answer.save() + + +def recalculate_all_answers(apps, _): + AnswerModel = apps.get_model("caluma_form.Answer") + + calc_answers = AnswerModel.objects.filter( + question__type="calculated_float", document__isnull=False + ) + for answer in calc_answers.iterator(): + recalculate_answer(answer) + + +class Migration(migrations.Migration): + + dependencies = [ + ("caluma_form", "0046_file_answer_reverse_keys"), + ] + + operations = [ + migrations.RunPython(recalculate_all_answers, migrations.RunPython.noop), + ] diff --git a/caluma/caluma_form/signals.py b/caluma/caluma_form/signals.py index 10597b16f..b0cb05a6f 100644 --- a/caluma/caluma_form/signals.py +++ b/caluma/caluma_form/signals.py @@ -187,12 +187,12 @@ def update_calc_from_answer(sender, instance, **kwargs): @receiver(post_save, sender=models.Document) @disable_raw -@filter_events(lambda created: created) +# We're only interested in table row forms +@filter_events(lambda instance, created: instance.pk != instance.family_id or created) def update_calc_from_document(sender, instance, created, **kwargs): - - for question in instance.form.questions.filter( - type=models.Question.TYPE_CALCULATED_FLOAT - ): + for question in models.Form.get_all_questions( + [(instance.family or instance).form_id] + ).filter(type=models.Question.TYPE_CALCULATED_FLOAT): _update_or_create_calc_answer(question, instance) diff --git a/caluma/caluma_form/tests/test_migration.py b/caluma/caluma_form/tests/test_migration.py index 1fdbc6670..b48eff5d9 100644 --- a/caluma/caluma_form/tests/test_migration.py +++ b/caluma/caluma_form/tests/test_migration.py @@ -8,6 +8,8 @@ from django.db.utils import DataError from django.utils import timezone +from caluma.caluma_form.api import save_answer, save_document +from caluma.caluma_form.models import Question from caluma.utils import col_type_from_db @@ -429,3 +431,82 @@ def test_migrate_answer_history_question_type(post_migrate_to_current_state): new_hist_ans = MigratedHistAns.objects.get(history_id=new_hist_ans.history_id) assert new_hist_ans.history_question_type == new_hist_quest.type + + +def test_migrate_recalculate_calc_answers( + post_migrate_to_current_state, + form_factory, + question_factory, + form_question_factory, +): + # set up form structure + main_form_question = form_question_factory( + question__type=Question.TYPE_INTEGER, question__slug="dep1_main" + ) + main_form = main_form_question.form + dep1_main = main_form_question.question + + row_form = form_factory(slug="row_form") + table_question = question_factory( + type="table", + slug="table_question", + row_form=row_form, + is_required="true", + is_hidden="false", + ) + form_question_factory(form=main_form, question=table_question) + + row_form_question = form_question_factory( + form=row_form, + question__type=Question.TYPE_INTEGER, + question__slug="dep2_row", + question__is_required=True, + ) + dep2_row = row_form_question.question + + form_question_factory( + form=main_form, + question__slug="calc_question", + question__type=Question.TYPE_CALCULATED_FLOAT, + question__calc_expression=( + f'"dep1_main"|answer(0) + "{table_question.slug}"|answer([])|mapby("dep2_row")|sum' + ), + ) + + # assert calc_dependents + dep1_main.refresh_from_db() + dep2_row.refresh_from_db() + assert dep1_main.calc_dependents == ["calc_question"] + assert dep2_row.calc_dependents == ["calc_question"] + + # set up document and answers + main_doc = save_document(form=main_form) + save_answer(question=dep1_main, value=10, document=main_doc) + row_doc = save_document(form=row_form) + save_answer(question=dep2_row, value=13, document=row_doc) + save_answer(table_question, document=main_doc, value=[str(row_doc.pk)]) + calc_answer = main_doc.answers.get(question_id="calc_question") + + # assert calc bug is fixed + assert calc_answer.value == 23 + + # set calc_answer.value to 10, like it would have been without the fix + calc_answer.value = 10 + calc_answer.save() + + # migrate back and forth in order to run the data-migration + executor = MigrationExecutor(connection) + app = "caluma_form" + migrate_from = [(app, "0046_file_answer_reverse_keys")] + migrate_to = [(app, "0047_recalculate_calc_answers")] + executor.migrate(migrate_from) + executor.loader.build_graph() + executor.migrate(migrate_to) + + new_apps = executor.loader.project_state(migrate_to).apps + + MigratedAnswer = new_apps.get_model(app, "Answer") + + # assert correct calc_value after migration + calc_answer = MigratedAnswer.objects.get(question_id="calc_question") + assert calc_answer.value == 23