From 182f9c1d17368592097d78a61a0702dd31fafc4f Mon Sep 17 00:00:00 2001 From: Quentin BEY Date: Tue, 26 Nov 2024 17:06:46 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=97=83=EF=B8=8F(teams)=20add=20Team=20dep?= =?UTF-8?q?endencies=20as=20a=20tree?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This provides the technical way to create Team trees. The implementation is quite naive. --- CHANGELOG.md | 1 + src/backend/core/admin.py | 7 +- src/backend/core/factories.py | 1 - ..._depth_team_numchild_team_path_and_more.py | 58 +++++++++++++ src/backend/core/models.py | 38 ++++++++- src/backend/core/tests/test_models_teams.py | 82 +++++++++++++++++++ .../demo/management/commands/create_demo.py | 43 +++++++--- .../demo/tests/test_commands_create_demo.py | 15 ++-- src/backend/people/settings.py | 1 + src/backend/pyproject.toml | 1 + 10 files changed, 226 insertions(+), 21 deletions(-) create mode 100644 src/backend/core/migrations/0010_team_depth_team_numchild_team_path_and_more.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ed8ca2..037666c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to ### Added +- ✨(teams) add Team dependencies #560 - ✨(organization) add admin action for plugin #640 - ✨(anct) fetch and display organization names of communes #583 - ✨(frontend) display email if no username #562 diff --git a/src/backend/core/admin.py b/src/backend/core/admin.py index 854d541..1204fd7 100644 --- a/src/backend/core/admin.py +++ b/src/backend/core/admin.py @@ -4,6 +4,9 @@ from django.contrib import admin, messages from django.contrib.auth import admin as auth_admin from django.utils.translation import gettext_lazy as _ +from treebeard.admin import TreeAdmin +from treebeard.forms import movenodeform_factory + from mailbox_manager.admin import MailDomainAccessInline from . import models @@ -122,9 +125,10 @@ class TeamServiceProviderInline(admin.TabularInline): @admin.register(models.Team) -class TeamAdmin(admin.ModelAdmin): +class TeamAdmin(TreeAdmin): """Team admin interface declaration.""" + form = movenodeform_factory(models.Team) inlines = (TeamAccessInline, TeamWebhookInline, TeamServiceProviderInline) exclude = ("service_providers",) # Handled by the inline list_display = ( @@ -133,6 +137,7 @@ class TeamAdmin(admin.ModelAdmin): "updated_at", ) search_fields = ("name",) + readonly_fields = ("path", "depth", "numchild") @admin.register(models.TeamAccess) diff --git a/src/backend/core/factories.py b/src/backend/core/factories.py index baa43a0..d92f37a 100644 --- a/src/backend/core/factories.py +++ b/src/backend/core/factories.py @@ -199,7 +199,6 @@ class TeamFactory(factory.django.DjangoModelFactory): class Meta: model = models.Team - django_get_or_create = ("name",) skip_postgeneration_save = True name = factory.Sequence(lambda n: f"team{n}") diff --git a/src/backend/core/migrations/0010_team_depth_team_numchild_team_path_and_more.py b/src/backend/core/migrations/0010_team_depth_team_numchild_team_path_and_more.py new file mode 100644 index 0000000..08e8ab0 --- /dev/null +++ b/src/backend/core/migrations/0010_team_depth_team_numchild_team_path_and_more.py @@ -0,0 +1,58 @@ +# Generated by Django 5.1.3 on 2024-11-26 13:55 + +from django.db import migrations, models + +from treebeard.numconv import NumConv + + +def update_team_paths(apps, schema_editor): + Team = apps.get_model("core", "Team") + alphabet = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" + steplen = 5 + + # Initialize NumConv with the specified custom alphabet + converter = NumConv(len(alphabet), alphabet) + + nodes = Team.objects.all().order_by("created_at") + for i, node in enumerate(nodes, 1): + node.depth = 1 # root nodes are at depth 1 + + # Use NumConv to encode the index `i` to a base representation and + # pad it to the specified step length using the custom alphabet + node.path = converter.int2str(i).rjust(steplen, alphabet[0]) + + if nodes: + Team.objects.bulk_update(nodes, ["depth", "path"]) + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0009_contacts_add_index_on_data_emails'), + ] + + operations = [ + migrations.AddField( + model_name='team', + name='depth', + field=models.PositiveIntegerField(default=0), + preserve_default=False, + ), + migrations.AddField( + model_name='team', + name='numchild', + field=models.PositiveIntegerField(default=0), + ), + migrations.AddField( + model_name='team', + name='path', + field=models.CharField(default='', max_length=400, db_collation="C"), + preserve_default=False, + ), + migrations.RunPython(update_team_paths, migrations.RunPython.noop), + migrations.AlterField( + model_name="team", + name="path", + field=models.CharField(unique=True, max_length=400, db_collation="C"), + ), + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 9aa6a87..8681356 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -26,6 +26,7 @@ from django.utils.translation import override import jsonschema from timezone_field import TimeZoneField +from treebeard.mp_tree import MP_Node, MP_NodeManager from core.enums import WebhookStatusChoices from core.plugins.loader import organization_plugins_run_after_create @@ -633,7 +634,34 @@ class OrganizationAccess(BaseModel): return f"{self.user!s} is {self.role:s} in organization {self.organization!s}" -class Team(BaseModel): +class TeamManager(MP_NodeManager): + """ + Custom manager for the Team model, to manage complexity/automation. + """ + + def create(self, parent_id=None, **kwargs): + """ + Replace the default create method to ease the Team creation process. + + Notes: + - the `add_*` methods from django-treebeard does not support the "using db". + Which means it will always use the default db. + - the `add_*` methods from django-treebeard does not support the "force_insert". + + """ + if parent_id is None: + return self.model.add_root(**kwargs) + + # Retrieve parent object, because django-treebeard uses raw queries for most + # write operations, and raw queries don’t update the django objects of the db + # entries they modify. See caveats in the django-treebeard documentation. + # This might be changed later if we never do any operation on the parent object + # before creating the child. + # Beware the N+1 here. + return self.get(pk=parent_id).add_child(**kwargs) + + +class Team(MP_Node, BaseModel): """ Represents the link between teams and users, specifying the role a user has in a team. @@ -643,6 +671,12 @@ class Team(BaseModel): Team `service_providers`. """ + # Allow up to 80 nested teams with 62^5 (916_132_832) root nodes + alphabet = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" + # Django treebeard does not allow max_length = None... + steplen = 5 + path = models.CharField(max_length=5 * 80, unique=True, db_collation="C") + name = models.CharField(max_length=100) users = models.ManyToManyField( @@ -664,6 +698,8 @@ class Team(BaseModel): blank=True, ) + objects = TeamManager() + class Meta: db_table = "people_team" ordering = ("name",) diff --git a/src/backend/core/tests/test_models_teams.py b/src/backend/core/tests/test_models_teams.py index c2f1995..bb6298c 100644 --- a/src/backend/core/tests/test_models_teams.py +++ b/src/backend/core/tests/test_models_teams.py @@ -134,3 +134,85 @@ def test_models_teams_get_abilities_preset_role(django_assert_num_queries): "put": False, "manage_accesses": False, } + + +# test trees + +def test_models_teams_create_root_team(): + """Create a root team.""" + team = models.Team.add_root(name="Root Team") + assert team.is_root() + assert team.name == "Root Team" + + +def test_models_teams_create_child_team(): + """Create a child team.""" + root_team = models.Team.add_root(name="Root Team") + child_team = root_team.add_child(name="Child Team") + assert child_team.is_child_of(root_team) + assert child_team.name == "Child Team" + assert child_team.get_parent() == root_team + + +def test_models_teams_create_grandchild_team(): + """Create a grandchild team.""" + root_team = models.Team.add_root(name="Root Team") + child_team = root_team.add_child(name="Child Team") + grandchild_team = child_team.add_child(name="Grandchild Team") + assert grandchild_team.is_child_of(child_team) + assert grandchild_team.name == "Grandchild Team" + assert grandchild_team.get_parent() == child_team + + +def test_models_teams_move_team(): + """Move a team to another parent.""" + root_team = models.Team.add_root(name="Root Team") + child_team = root_team.add_child(name="Child Team") + new_root_team = models.Team.add_root(name="New Root Team") + child_team.move(new_root_team, pos="first-child") + child_team.refresh_from_db() + assert child_team.get_parent(update=True) == new_root_team + + +def test_models_teams_delete_team(): + """ + Delete a parent team also deletes children. + + This might not be what we want, but it's the default behavior of treebeard. + """ + root_team = models.Team.add_root(name="Root Team") + root_team.add_child(name="Child Team") + + assert models.Team.objects.all().count() == 2 + root_team.delete() + + assert models.Team.objects.all().count() == 0 + + +def test_models_teams_manager_create(): + """Create a team using the manager.""" + team = models.Team.objects.create(name="Team") + assert team.is_root() + assert team.name == "Team" + + child_team = models.Team.objects.create(name="Child Team", parent_id=team.pk) + assert child_team.is_child_of(team) + assert child_team.name == "Child Team" + + +def test_models_teams_tree_alphabet(): + """Test the creation of teams with treebeard methods.""" + organization = factories.OrganizationFactory(with_registration_id=True) + models.Team.load_bulk( + [ + { + "data": { + "name": f"team-{i}", + "organization_id": organization.pk, + } + } + for i in range(len(models.Team.alphabet) * 2) + ] + ) + + assert models.Team.objects.count() == len(models.Team.alphabet) * 2 diff --git a/src/backend/demo/management/commands/create_demo.py b/src/backend/demo/management/commands/create_demo.py index 038d92b..a88ee15 100755 --- a/src/backend/demo/management/commands/create_demo.py +++ b/src/backend/demo/management/commands/create_demo.py @@ -13,6 +13,7 @@ from django.core.management.base import BaseCommand, CommandError from django.utils.text import slugify from faker import Faker +from treebeard.mp_tree import MP_Node from core import models @@ -45,7 +46,33 @@ class BulkQueue: if not objects: return - objects[0]._meta.model.objects.bulk_create(objects, ignore_conflicts=False) # noqa: SLF001 + objects_model = objects[0]._meta.model # noqa: SLF001 + if issubclass(objects_model, MP_Node): + # For treebeard models, we need to create the tree structure + # in a specific way. This is not perfect but it works for the + # current use case. + model_fields = { + field + for field in objects_model._meta.concrete_fields # noqa: SLF001 + if field.name not in {"depth", "numchild", "path"} + } + bulk_data = [ + { + "data": { + field.name: field.value_from_object(obj) + for field in model_fields + if field.value_from_object(obj) + } + } + for obj in objects + ] + objects_model.load_bulk(bulk_data) + else: + objects_model.objects.bulk_create( + objects, + ignore_conflicts=False, + ) + # In debug mode, Django keeps query cache which creates a memory leak in this case db.reset_queries() self.queue[objects[0]._meta.model.__name__] = [] # noqa: SLF001 @@ -192,21 +219,15 @@ def create_demo(stdout): # pylint: disable=too-many-locals ) with Timeit(stdout, "Creating domains"): - created = set() - for _i in range(defaults.NB_OBJECTS["domains"]): - name = fake.domain_name() - if name in created: - continue - created.add(name) - - slug = slugify(name) + for i in range(defaults.NB_OBJECTS["domains"]): + name = f"{fake.domain_name()}-i{i:d}" queue.push( mailbox_models.MailDomain( name=name, # slug should be automatic but bulk_create doesn't use save - slug=slug, - status=random.choice(MailDomainStatusChoices.choices)[0], + slug=slugify(name), + status=random.choice(MailDomainStatusChoices.values), ) ) queue.flush() diff --git a/src/backend/demo/tests/test_commands_create_demo.py b/src/backend/demo/tests/test_commands_create_demo.py index f19412f..8b55dfe 100644 --- a/src/backend/demo/tests/test_commands_create_demo.py +++ b/src/backend/demo/tests/test_commands_create_demo.py @@ -12,16 +12,17 @@ from core import models from demo import defaults from mailbox_manager import models as mailbox_models -TEST_NB_OBJECTS = { - "users": 5, - "teams": 3, - "max_users_per_team": 5, - "domains": 2, -} - pytestmark = pytest.mark.django_db +TEST_NB_OBJECTS = { + "users": 100, + "teams": 100, + "max_users_per_team": 5, + "domains": 100, +} + + @override_settings(DEBUG=True) @mock.patch.dict(defaults.NB_OBJECTS, TEST_NB_OBJECTS) def test_commands_create_demo(): diff --git a/src/backend/people/settings.py b/src/backend/people/settings.py index a9cbe78..f363aad 100755 --- a/src/backend/people/settings.py +++ b/src/backend/people/settings.py @@ -215,6 +215,7 @@ class Base(Configuration): "dockerflow.django", "rest_framework", "parler", + "treebeard", "easy_thumbnails", # Django "django.contrib.auth", diff --git a/src/backend/pyproject.toml b/src/backend/pyproject.toml index 52c4149..13e2cfd 100644 --- a/src/backend/pyproject.toml +++ b/src/backend/pyproject.toml @@ -32,6 +32,7 @@ dependencies = [ "django-cors-headers==4.6.0", "django-countries==7.6.1", "django-parler==2.3", + "django-treebeard==4.7.1", "redis==5.2.1", "django-redis==5.4.0", "django-storages==1.14.4",