Merge pull request #1965 from open-dynaMIX/fix_recalculate_calc_answers

fix: replace faulty migration for calc answers with a command

caluma/caluma_form/management/commands/recalculate_calc_answers.py

@@ -0,0 +1,35 @@
+from django.core.management.base import BaseCommand
+
+from caluma.caluma_form.models import Answer, Question
+from caluma.caluma_form.signals import _update_or_create_calc_answer
+
+
+class Command(BaseCommand):
+    """
+    Recalculate calculated answers containing values from TableAnswers.
+
+    Due to a bug, answers to calculated questions were wrong, if they contained values
+    from table rows. This command recalculates all of them.
+    """
+
+    help = "Recalculate calculated answers containing values from TableAnswers."
+
+    def handle(self, *args, **options):
+        affected_questions = (
+            Question.objects.filter(type="table")
+            .exclude(calc_dependents=[])
+            .values_list("calc_dependents", flat=True)
+        )
+
+        affected_questions_clean = set(
+            [slug for entry in affected_questions for slug in entry]
+        )
+
+        calc_answers = Answer.objects.filter(
+            question__type="calculated_float",
+            document__isnull=False,
+            question__slug__in=affected_questions_clean,
+        )
+
+        for answer in calc_answers.iterator():
+            _update_or_create_calc_answer(answer.question, answer.document)

caluma/caluma_form/tests/test_migration.py

@@ -8,8 +8,6 @@
 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
 
 
@@ -431,82 +429,3 @@ 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

caluma/caluma_form/tests/test_recalculate_calc_answers_command.py

@@ -0,0 +1,74 @@
+import os
+
+from django.core.management import call_command
+
+from caluma.caluma_form.api import save_answer, save_document
+from caluma.caluma_form.models import Question
+
+
+def test_recalculate_calc_answers(
+    db,
+    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()
+
+    call_command("recalculate_calc_answers", stderr=open(os.devnull, "w"))
+
+    # assert correct calc_value after migration
+    calc_answer.refresh_from_db()
+    assert calc_answer.value == 23