Skip to content

Commit

Permalink
refactor: drop django-mptt in favour of django-tree-queries
Browse files Browse the repository at this point in the history
Django-Tree-Queries works with recursive CTEs (common table expressions) to deal
with hierarchical relationships, generally achieving the same result, but without
the `right`, `left` and `tree_id` fields.

Django-MPTT has been unmaintained for several years, and we're suspecting some
subtle bugs in it that we can't really reproduce or verify right now. Anyway it's
a good time to get rid of it.

Note: Some old migrations needed to be changed, as we cannot refer to
MPTT fields anymore. They were essentially regular foreign keys however,
so this won't break the database.

BREAKING! This breaks if Emeis is used as a Django app directly, and depends on
any of the MPTT API / features.
  • Loading branch information
winged committed Jun 5, 2024
1 parent c7a95bf commit 61a30e0
Show file tree
Hide file tree
Showing 18 changed files with 634 additions and 97 deletions.
2 changes: 0 additions & 2 deletions emeis/core/management/commands/create_scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from django.core.management.base import BaseCommand

from emeis.core.models import ACL, Role, Scope, User
from emeis.core.utils import rebuild_mptt_model


class Command(BaseCommand):
Expand Down Expand Up @@ -78,7 +77,6 @@ def handle(self, *args, **options):
options["name"] = json.loads(options["name"])

new_scope = Scope.objects.create(name=options["name"], parent=parent)
rebuild_mptt_model(Scope)
out_info["scope_id"] = str(new_scope.pk)

if options["user"]:
Expand Down
3 changes: 1 addition & 2 deletions emeis/core/migrations/0001_initial.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import django.utils.timezone
import localized_fields.fields.char_field
import localized_fields.fields.text_field
import mptt.fields
from django.conf import settings
from django.db import migrations, models

Expand Down Expand Up @@ -224,7 +223,7 @@ class Migration(migrations.Migration):
),
(
"parent",
mptt.fields.TreeForeignKey(
models.ForeignKey(
blank=True,
null=True,
on_delete=django.db.models.deletion.SET_NULL,
Expand Down
5 changes: 2 additions & 3 deletions emeis/core/migrations/0009_alter_scope_parent.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
# Generated by Django 3.2.12 on 2022-04-21 07:24

from django.db import migrations
import django.db.models.deletion
import mptt.fields
from django.db import migrations, models


class Migration(migrations.Migration):
Expand All @@ -14,7 +13,7 @@ class Migration(migrations.Migration):
migrations.AlterField(
model_name="scope",
name="parent",
field=mptt.fields.TreeForeignKey(
field=models.ForeignKey(
blank=True,
null=True,
on_delete=django.db.models.deletion.CASCADE,
Expand Down
2 changes: 1 addition & 1 deletion emeis/core/migrations/0010_scope_full_name.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Generated by Django 3.2.15 on 2022-09-23 09:13

from django.db import migrations
import localized_fields.fields.char_field
from django.db import migrations

from emeis.core import models

Expand Down
45 changes: 45 additions & 0 deletions emeis/core/migrations/0011_mptt_to_tree_queries.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# Generated by Django 3.2.25 on 2024-06-03 14:17

import django.db.models.deletion
from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("emeis_core", "0010_scope_full_name"),
]

operations = [
migrations.AlterModelOptions(
name="scope",
options={"ordering": ["full_name"]},
),
migrations.RemoveField(
model_name="scope",
name="level",
),
migrations.RemoveField(
model_name="scope",
name="lft",
),
migrations.RemoveField(
model_name="scope",
name="rght",
),
migrations.RemoveField(
model_name="scope",
name="tree_id",
),
migrations.AlterField(
model_name="scope",
name="parent",
field=models.ForeignKey(
blank=True,
null=True,
on_delete=django.db.models.deletion.CASCADE,
related_name="children",
to="emeis_core.scope",
verbose_name="parent",
),
),
]
21 changes: 11 additions & 10 deletions emeis/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from django.utils import timezone, translation
from django.utils.translation import gettext_lazy as _
from localized_fields.fields import LocalizedCharField, LocalizedTextField
from mptt.models import MPTTModel, TreeForeignKey
from tree_queries.models import TreeNode, TreeQuerySet


def make_uuid():
Expand Down Expand Up @@ -158,7 +158,7 @@ def is_authenticated(self):
return True


class Scope(MPTTModel, UUIDModel):
class Scope(TreeNode, UUIDModel):
name = LocalizedCharField(_("scope name"), blank=False, null=False, required=False)

full_name = LocalizedCharField(
Expand All @@ -168,20 +168,21 @@ class Scope(MPTTModel, UUIDModel):
description = LocalizedTextField(
_("scope description"), null=True, blank=True, required=False
)
parent = TreeForeignKey(
"self",
on_delete=models.CASCADE,
null=True,
blank=True,
related_name="children",
)
is_active = models.BooleanField(default=True)

objects = TreeQuerySet.as_manager(with_tree_fields=True)

def save(self, *args, **kwargs):
# django-tree-queries does validation in TreeNode.clean(), which is not
# called by DRF (only by django forms), so we have to do this here
self.clean()
return super().save(*args, **kwargs)

def __str__(self):
return f"{type(self).__name__} ({self.full_name}, pk={self.pk})"

class Meta:
ordering = ["full_name"]
ordering = ["name"]


@receiver(pre_save, sender=Scope)
Expand Down
25 changes: 24 additions & 1 deletion emeis/core/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ class Meta:


class MyACLSerializer(BaseSerializer):
scope = serializers.ResourceRelatedField(
queryset=Scope.objects.all(), required=False, many=False
)

included_serializers = {
"scope": "emeis.core.serializers.ScopeSerializer",
"role": "emeis.core.serializers.RoleSerializer",
Expand Down Expand Up @@ -73,6 +77,22 @@ class Meta:


class ScopeSerializer(BaseSerializer):
level = serializers.SerializerMethodField()

def get_level(self, obj):
depth = getattr(obj, "tree_depth", None)
if depth is not None:
return depth

# Note: This should only happen on CREATE, never in GET (Either list,
# detail, or include!) In CREATE, it's a new object that doesn't come
# from a QS

# Sometimes, the model object may come out of a non-django-tree-queries
# QS, and thus would not have the `tree_*` attributes amended. Then we
# need to go the "slow path"
return obj.ancestors().count()

class Meta:
model = Scope
fields = BaseSerializer.Meta.fields + (
Expand All @@ -83,7 +103,7 @@ class Meta:
"full_name",
"is_active",
)
read_only_fields = ["full_name"]
read_only_fields = ["full_name", "level"]


class PermissionSerializer(BaseSerializer):
Expand Down Expand Up @@ -111,6 +131,9 @@ class Meta:


class ACLSerializer(BaseSerializer):
scope = serializers.ResourceRelatedField(
queryset=Scope.objects.all(), required=False, many=False
)
included_serializers = {
"user": UserSerializer,
"scope": ScopeSerializer,
Expand Down
Loading

0 comments on commit 61a30e0

Please sign in to comment.