From b3501a3a820613c562e379efa8c9bd8fd3818d91 Mon Sep 17 00:00:00 2001 From: David Vogt Date: Wed, 29 Sep 2021 19:01:30 +0200 Subject: [PATCH] feat(analysis): add path attribute / mixin to structural models Part of a series of PRs that will lead to the analysis module. Add a denormalisation step to the structural data models (case, workitem, and document) that will allow faster queries across case structures down the road. --- caluma/caluma_core/apps.py | 1 + caluma/caluma_core/models.py | 58 +++++++++++++++++++ caluma/caluma_core/signals.py | 19 ++++++ caluma/caluma_core/tests/test_set_path.py | 21 +++++++ caluma/caluma_form/factories.py | 8 +-- .../migrations/0041_add_path_attribute.py | 34 +++++++++++ .../migrations/0042_fill_path_attributes.py | 18 ++++++ caluma/caluma_form/models.py | 4 +- .../migrations/0028_add_path_attribute.py | 54 +++++++++++++++++ .../migrations/0029_fill_path_attributes.py | 22 +++++++ caluma/caluma_workflow/models.py | 10 +++- caluma/tests/__snapshots__/test_schema.ambr | 5 ++ 12 files changed, 246 insertions(+), 8 deletions(-) create mode 100644 caluma/caluma_core/signals.py create mode 100644 caluma/caluma_core/tests/test_set_path.py create mode 100644 caluma/caluma_form/migrations/0041_add_path_attribute.py create mode 100644 caluma/caluma_form/migrations/0042_fill_path_attributes.py create mode 100644 caluma/caluma_workflow/migrations/0028_add_path_attribute.py create mode 100644 caluma/caluma_workflow/migrations/0029_fill_path_attributes.py diff --git a/caluma/caluma_core/apps.py b/caluma/caluma_core/apps.py index de43e8eda..2923cd99d 100644 --- a/caluma/caluma_core/apps.py +++ b/caluma/caluma_core/apps.py @@ -28,3 +28,4 @@ def ready(self): import_module(module) import_module("caluma.caluma_form.signals") + import_module("caluma.caluma_core.signals") diff --git a/caluma/caluma_core/models.py b/caluma/caluma_core/models.py index 0dceac8fc..99dbb8b9e 100644 --- a/caluma/caluma_core/models.py +++ b/caluma/caluma_core/models.py @@ -1,6 +1,8 @@ import uuid +from django.contrib.postgres.fields import ArrayField from django.db import models +from django.db.utils import ProgrammingError from graphene.utils.str_converters import to_camel_case from simple_history.models import HistoricalRecords @@ -89,6 +91,62 @@ class Meta: abstract = True +class PathModelMixin(models.Model): + """ + Mixin that stores a path to the object. + + The path attribute is used for analytics and allows direct access + and faster SELECTs. + + To you use this mixin, you must define a property named `path_parent_attrs` + on the model class. It's supposed to be a list of strings that contain the + attributes to check. The first attribute that exists will be used. + This way, you can define multiple possible parents (in a document, for example + you can first check if it's attached to a case, or a work item, then a document family) + """ + + path = ArrayField( + models.CharField(max_length=150), + default=list, + help_text="Stores a path to the given object", + ) + + def calculate_path(self, _seen_keys=None): + if not _seen_keys: + _seen_keys = set() + self_pk_list = [str(self.pk)] + if _seen_keys.intersection(set(self_pk_list)): + # Not recursing any more. Root elements *may* point + # to themselves in certain circumstances + return [] + + path_parent_attrs = getattr(self, "path_parent_attrs", None) + + if not isinstance(path_parent_attrs, list): + raise ProgrammingError( # pragma: no cover + "If you use the PathModelMixin, you must define " + "`path_parent_attrs` on the model (a list of " + "strings that contains the attributes to check)" + ) + + for attr in path_parent_attrs: + parent = getattr(self, attr, None) + if parent: + parent_path = parent.calculate_path(set([*self_pk_list, *_seen_keys])) + if parent_path: + return parent_path + self_pk_list + + # Else case: If parent returns an empty list (loop case), we may + # be in the wrong parent attribute. We continue checking the other + # attributes (if any). If we don't find any other parents that work, + # we'll just return as if we're the root object. + + return self_pk_list + + class Meta: + abstract = True + + class NaturalKeyModel(BaseModel, HistoricalModel): """Models which use a natural key as primary key.""" diff --git a/caluma/caluma_core/signals.py b/caluma/caluma_core/signals.py new file mode 100644 index 000000000..30fbd71db --- /dev/null +++ b/caluma/caluma_core/signals.py @@ -0,0 +1,19 @@ +from django.db.models.signals import pre_save +from django.dispatch import receiver + +from caluma.caluma_core.events import filter_events +from caluma.caluma_core.models import PathModelMixin + + +@receiver(pre_save) +@filter_events(lambda sender: PathModelMixin in sender.mro()) +def store_path(sender, instance, **kwargs): + """Store/update the path of the object. + + Note: Due to the fact that this structure is relatively rigid, + we don't update our children. For one, they may be difficult to + collect, but also, the parent-child relationship is not expected + to change, and structures are built top-down, so any object + is expected to exist before it's children come into play. + """ + instance.path = instance.calculate_path() diff --git a/caluma/caluma_core/tests/test_set_path.py b/caluma/caluma_core/tests/test_set_path.py new file mode 100644 index 000000000..17fba9e55 --- /dev/null +++ b/caluma/caluma_core/tests/test_set_path.py @@ -0,0 +1,21 @@ +def test_set_paths(db, case, work_item_factory): + + workitem = work_item_factory(case=case) + # workitem.save() # trigger the signal + assert workitem.path == [str(case.pk), str(workitem.pk)] + + +def test_row_document_path(db, case, form_and_document): + form, document, questions, answers = form_and_document( + use_table=True, use_subform=True + ) + + case.document = document + case.save() + document.save() + assert document.path == [str(case.pk), str(document.pk)] + + table_ans = answers["table"] + row_doc = table_ans.documents.first() + row_doc.save() + assert row_doc.path == [str(case.pk), str(document.pk), str(row_doc.pk)] diff --git a/caluma/caluma_form/factories.py b/caluma/caluma_form/factories.py index 59260c01c..17807a03a 100644 --- a/caluma/caluma_form/factories.py +++ b/caluma/caluma_form/factories.py @@ -131,10 +131,10 @@ class AnswerFactory(DjangoModelFactory): @lazy_attribute def value(self): - if ( - self.question.type == models.Question.TYPE_MULTIPLE_CHOICE - or self.question.type == models.Question.TYPE_DYNAMIC_MULTIPLE_CHOICE - ): + if self.question.type in [ + models.Question.TYPE_MULTIPLE_CHOICE, + models.Question.TYPE_DYNAMIC_MULTIPLE_CHOICE, + ]: return [faker.Faker().name(), faker.Faker().name()] elif self.question.type == models.Question.TYPE_FLOAT: return faker.Faker().pyfloat() diff --git a/caluma/caluma_form/migrations/0041_add_path_attribute.py b/caluma/caluma_form/migrations/0041_add_path_attribute.py new file mode 100644 index 000000000..4df132e70 --- /dev/null +++ b/caluma/caluma_form/migrations/0041_add_path_attribute.py @@ -0,0 +1,34 @@ +# Generated by Django 2.2.22 on 2021-09-29 15:13 + +import django.contrib.postgres.fields +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("caluma_form", "0040_add_modified_by_user_group"), + ] + + operations = [ + migrations.AddField( + model_name="document", + name="path", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField(max_length=150), + default=list, + help_text="Stores a path to the given object", + size=None, + ), + ), + migrations.AddField( + model_name="historicaldocument", + name="path", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField(max_length=150), + default=list, + help_text="Stores a path to the given object", + size=None, + ), + ), + ] diff --git a/caluma/caluma_form/migrations/0042_fill_path_attributes.py b/caluma/caluma_form/migrations/0042_fill_path_attributes.py new file mode 100644 index 000000000..2f8f03e19 --- /dev/null +++ b/caluma/caluma_form/migrations/0042_fill_path_attributes.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.22 on 2021-09-29 16:48 + +from django.db import migrations + + +def add_path_attribute(apps, schema_editor): + for doc in apps.get_model("caluma_form.document").objects.all(): + doc.path = doc.calculate_path() + doc.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ("caluma_form", "0041_add_path_attribute"), + ] + + operations = [migrations.RunPython(add_path_attribute, migrations.RunPython.noop)] diff --git a/caluma/caluma_form/models.py b/caluma/caluma_form/models.py index eccd4edf8..828612e9e 100644 --- a/caluma/caluma_form/models.py +++ b/caluma/caluma_form/models.py @@ -226,9 +226,11 @@ def create_document_for_task(self, task, user): return SaveDocumentLogic.create({"form": task.form}, user=user) -class Document(core_models.UUIDModel): +class Document(core_models.UUIDModel, core_models.PathModelMixin): objects = DocumentManager() + path_parent_attrs = ["work_item", "case", "family"] + family = models.ForeignKey( "self", help_text="Family id which document belongs too.", diff --git a/caluma/caluma_workflow/migrations/0028_add_path_attribute.py b/caluma/caluma_workflow/migrations/0028_add_path_attribute.py new file mode 100644 index 000000000..5e6b78ce6 --- /dev/null +++ b/caluma/caluma_workflow/migrations/0028_add_path_attribute.py @@ -0,0 +1,54 @@ +# Generated by Django 2.2.22 on 2021-09-29 15:13 + +import django.contrib.postgres.fields +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("caluma_workflow", "0027_add_modified_by_user_group"), + ] + + operations = [ + migrations.AddField( + model_name="case", + name="path", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField(max_length=150), + default=list, + help_text="Stores a path to the given object", + size=None, + ), + ), + migrations.AddField( + model_name="historicalcase", + name="path", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField(max_length=150), + default=list, + help_text="Stores a path to the given object", + size=None, + ), + ), + migrations.AddField( + model_name="historicalworkitem", + name="path", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField(max_length=150), + default=list, + help_text="Stores a path to the given object", + size=None, + ), + ), + migrations.AddField( + model_name="workitem", + name="path", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField(max_length=150), + default=list, + help_text="Stores a path to the given object", + size=None, + ), + ), + ] diff --git a/caluma/caluma_workflow/migrations/0029_fill_path_attributes.py b/caluma/caluma_workflow/migrations/0029_fill_path_attributes.py new file mode 100644 index 000000000..33f19f144 --- /dev/null +++ b/caluma/caluma_workflow/migrations/0029_fill_path_attributes.py @@ -0,0 +1,22 @@ +# Generated by Django 2.2.22 on 2021-09-29 15:20 + +from django.db import migrations + + +def add_path_attribute(apps, schema_editor): + for model in [ + "caluma_workflow.workitem", + "caluma_workflow.case", + ]: + for obj in apps.get_model(model).objects.all(): + obj.path = obj.calculate_path() + obj.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ("caluma_workflow", "0028_add_path_attribute"), + ] + + operations = [migrations.RunPython(add_path_attribute, migrations.RunPython.noop)] diff --git a/caluma/caluma_workflow/models.py b/caluma/caluma_workflow/models.py index a1d033158..29175edc3 100644 --- a/caluma/caluma_workflow/models.py +++ b/caluma/caluma_workflow/models.py @@ -8,7 +8,7 @@ from django.utils import timezone from localized_fields.fields import LocalizedField -from ..caluma_core.models import ChoicesCharField, SlugModel, UUIDModel +from ..caluma_core.models import ChoicesCharField, PathModelMixin, SlugModel, UUIDModel class Task(SlugModel): @@ -106,7 +106,7 @@ class Meta: unique_together = ("workflow", "task") -class Case(UUIDModel): +class Case(UUIDModel, PathModelMixin): STATUS_RUNNING = "running" STATUS_COMPLETED = "completed" STATUS_CANCELED = "canceled" @@ -119,6 +119,8 @@ class Case(UUIDModel): (STATUS_SUSPENDED, "Case is suspended."), ) + path_parent_attrs = ["parent_work_item"] + family = models.ForeignKey( "self", help_text="Family id which case belongs to.", @@ -163,7 +165,7 @@ def set_case_family(sender, instance, **kwargs): instance.family = instance -class WorkItem(UUIDModel): +class WorkItem(UUIDModel, PathModelMixin): STATUS_READY = "ready" STATUS_COMPLETED = "completed" STATUS_CANCELED = "canceled" @@ -178,6 +180,8 @@ class WorkItem(UUIDModel): (STATUS_SUSPENDED, "Work item is suspended."), ) + path_parent_attrs = ["case"] + name = LocalizedField( blank=False, null=False, diff --git a/caluma/tests/__snapshots__/test_schema.ambr b/caluma/tests/__snapshots__/test_schema.ambr index 51553d516..ee2075a29 100644 --- a/caluma/tests/__snapshots__/test_schema.ambr +++ b/caluma/tests/__snapshots__/test_schema.ambr @@ -166,6 +166,7 @@ modifiedByUser: String modifiedByGroup: String id: ID! + path: [String]! closedAt: DateTime closedByUser: String closedByGroup: String @@ -493,6 +494,7 @@ modifiedByUser: String modifiedByGroup: String id: ID! + path: [String]! form: Form! source: Document meta: GenericScalar @@ -947,6 +949,7 @@ modifiedByUser: String modifiedByGroup: String id: ID! + path: [String]! meta: GenericScalar historyUserId: String form: Form @@ -2126,6 +2129,7 @@ CREATED_BY_GROUP MODIFIED_BY_USER MODIFIED_BY_GROUP + PATH FORM SOURCE } @@ -2480,6 +2484,7 @@ modifiedByUser: String modifiedByGroup: String id: ID! + path: [String]! name: String! description: String closedAt: DateTime