Skip to content

Commit

Permalink
fix: remove unnecessary signals, don't recompute on copy, tests
Browse files Browse the repository at this point in the history
  • Loading branch information
czosel committed Dec 30, 2024
1 parent 64d4df6 commit a47c6e1
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 425 deletions.
6 changes: 5 additions & 1 deletion caluma/caluma_form/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ def save_answer(
data.update(kwargs)

answer = models.Answer.objects.filter(question=question, document=document).first()
answer = domain_logic.SaveAnswerLogic.get_new_answer(data, user, answer)
if answer:
answer = domain_logic.SaveAnswerLogic.update(answer, data, user)
else:
answer = domain_logic.SaveAnswerLogic.create(data, user)

return answer

Expand All @@ -45,6 +48,7 @@ def save_default_answer(
data.update(kwargs)

answer = question.default_answer
# TODO refactor to use create / udpate as well?
answer = domain_logic.SaveDefaultAnswerLogic.get_new_answer(data, user, answer)

return answer
Expand Down
8 changes: 0 additions & 8 deletions caluma/caluma_form/domain_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,7 @@ def create(
if answer.question.type == models.Question.TYPE_TABLE:
answer.create_answer_documents(documents)

print("creating answer", flush=True)
if answer.question.calc_dependents:
print(
"creating answer for question",
answer.question,
answer.question.calc_dependents,
)
root_doc = answer.document.family
root_doc = (
models.Document.objects.filter(pk=answer.document.family_id)
Expand All @@ -188,13 +182,11 @@ def create(
)
.first()
)
print("init structure top level")
struc = structure.FieldSet(root_doc, root_doc.form)

for question in models.Question.objects.filter(
pk__in=answer.question.calc_dependents
):
print(f"recalculating {question} from domain logic _create_")
update_or_create_calc_answer(question, root_doc, struc)

return answer
Expand Down
5 changes: 2 additions & 3 deletions caluma/caluma_form/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,9 +396,9 @@ def set_family(self, root_doc):

def copy(self, family=None, user=None):
"""Create a copy including all its answers."""
from caluma.caluma_form.utils import recalculate_answers_from_document

# defer calculated questions, as many unecessary recomputations will happen otherwise
# no need to update calculated questions, since calculated answers
# are copied as well
meta = dict(self.meta)
meta["_defer_calculation"] = True

Expand All @@ -423,7 +423,6 @@ def copy(self, family=None, user=None):

new_document.meta.pop("_defer_calculation", None)
new_document.save()
recalculate_answers_from_document(new_document)

return new_document

Expand Down
91 changes: 1 addition & 90 deletions caluma/caluma_form/signals.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
import itertools
from django.db.models import Prefetch

from django.db.models.signals import (
m2m_changed,
post_delete,
post_init,
post_save,
pre_delete,
pre_save,
)
Expand All @@ -16,11 +11,7 @@
from caluma.utils import disable_raw

from . import models
from .utils import (
recalculate_answers_from_document,
update_calc_dependents,
update_or_create_calc_answer,
)
from .utils import update_calc_dependents


@receiver(pre_create_historical_record, sender=models.HistoricalAnswer)
Expand Down Expand Up @@ -94,83 +85,3 @@ 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):
# TODO optimize: only update answers if calc_expression is updated
# needs to happen during save() or __init__()

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)


# @receiver(post_save, sender=models.Answer)
# @disable_raw
# @filter_events(lambda instance: instance.document and instance.question.calc_dependents)
# def update_calc_from_answer(sender, instance, **kwargs):
# # If there is no document on the answer it means that it's a default
# # answer. They shouldn't trigger a recalculation of a calculated field
# # even when they are technically listed as a dependency.
# # Also skip non-referenced answers.
# if instance.document.family.meta.get("_defer_calculation"):
# return
#
# if instance.question.type == models.Question.TYPE_TABLE:
# print("skipping update calc of table questions in event layer, because we don't have access to the question slug here")
# return
#
# print(f"saved answer to {instance.question.pk}, recalculate dependents:")
# document = models.Document.objects.filter(pk=instance.document_id).prefetch_related(
# *build_document_prefetch_statements(
# "family", prefetch_options=True
# ),
# ).first()
#
# for question in models.Question.objects.filter(
# pk__in=instance.question.calc_dependents
# ):
# print(f"- {question.pk}")
# update_or_create_calc_answer(question, document)


@receiver(post_save, sender=models.Document)
@disable_raw
# 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):
# we do this in a more focused way (only updating the calc dependents in the domain logic
# recalculate_answers_from_document(instance)
pass


@receiver(m2m_changed, sender=models.AnswerDocument)
def update_calc_from_answerdocument(sender, instance, **kwargs):
dependents = instance.document.form.questions.exclude(
calc_dependents=[]
).values_list("calc_dependents", flat=True)

dependent_questions = list(itertools.chain(*dependents))
# TODO: when is this even called?
print(f"answerdocument {instance.pk} changed, update {dependent_questions}")

for question in models.Question.objects.filter(pk__in=dependent_questions):
update_or_create_calc_answer(question, instance.document)
5 changes: 5 additions & 0 deletions caluma/caluma_form/structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,11 @@ def children(self):
for question in self.form.questions.all()
]

def set_answer(self, question_slug, answer):
field = self.get_field(question_slug)
if field:
field.answer = answer

def __repr__(self):
q_slug = self.question.slug if self.question else None
if q_slug:
Expand Down
Loading

0 comments on commit a47c6e1

Please sign in to comment.