From d0c5d71b8307c2d9097ba8aab98897a4a77d01f0 Mon Sep 17 00:00:00 2001 From: Quentin BEY Date: Tue, 26 Nov 2024 17:06:46 +0100 Subject: [PATCH 1/5] =?UTF-8?q?=F0=9F=97=83=EF=B8=8F(teams)=20add=20Team?= =?UTF-8?q?=20dependencies=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 | 53 ++++++++++-- .../demo/tests/test_commands_create_demo.py | 23 ++---- src/backend/people/settings.py | 1 + src/backend/pyproject.toml | 1 + 10 files changed, 237 insertions(+), 28 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 915d29735..ef6627af7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to ### Added +- ✨(teams) add Team dependencies #560 - ✨(anct) fetch and display organization names of communes #583 - ✨(frontend) display email if no username #562 - 🧑‍💻(oidc) add ability to pull registration ID (e.g. SIRET) from OIDC #577 diff --git a/src/backend/core/admin.py b/src/backend/core/admin.py index 7df08d0eb..3eaf28a71 100644 --- a/src/backend/core/admin.py +++ b/src/backend/core/admin.py @@ -4,6 +4,9 @@ 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 @@ -118,9 +121,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 = ( @@ -129,6 +133,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 baa43a09d..d92f37abd 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 000000000..08e8ab08f --- /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 9aa6a87f7..86813569e 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -26,6 +26,7 @@ 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 @@ def __str__(self): 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 c2f19953e..bb6298c81 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 038d92b43..d59d1bea1 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.utils.text import slugify from faker import Faker +from treebeard.mp_tree import MP_Node from core import models @@ -45,7 +46,33 @@ def _bulk_create(self, objects): 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 @@ -106,6 +133,20 @@ def __exit__(self, exc_type, exc_value, exc_tb): return elapsed_time +def _get_random_domain_name(excluded): + """Get a fake domain using an exclusion list.""" + name = fake.domain_name() + for _max_retry in range(10): # Limit iterations to avoid infinite loop + if name not in excluded: + return name + name = fake.domain_name() + + if name in excluded: + raise RuntimeError("Could not find a domain name that is not already created") + + return name + + def create_demo(stdout): # pylint: disable=too-many-locals """ Create a database with demo data for developers to work in a realistic environment. @@ -194,19 +235,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 + name = _get_random_domain_name(created) created.add(name) - slug = slugify(name) - 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 f19412f95..436c8edbd 100644 --- a/src/backend/demo/tests/test_commands_create_demo.py +++ b/src/backend/demo/tests/test_commands_create_demo.py @@ -1,7 +1,5 @@ """Test the `create_demo` management command""" -from unittest import mock - from django.core.management import call_command from django.test import override_settings @@ -9,21 +7,13 @@ from core import models -from demo import defaults +from demo.defaults import NB_OBJECTS 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 @override_settings(DEBUG=True) -@mock.patch.dict(defaults.NB_OBJECTS, TEST_NB_OBJECTS) def test_commands_create_demo(): """The create_demo management command should create objects as expected.""" call_command("create_demo") @@ -32,17 +22,16 @@ def test_commands_create_demo(): # 3 user with team rights # 3 user with domain rights # 3 x 3 user with both rights - assert models.User.objects.count() == TEST_NB_OBJECTS["users"] + 3 + 3 + 3 + 9 + assert models.User.objects.count() == NB_OBJECTS["users"] + 3 + 3 + 3 + 9 - assert models.Team.objects.count() == TEST_NB_OBJECTS["teams"] - assert models.TeamAccess.objects.count() >= TEST_NB_OBJECTS["teams"] - assert mailbox_models.MailDomain.objects.count() == TEST_NB_OBJECTS["domains"] + assert models.Team.objects.count() == NB_OBJECTS["teams"] + assert models.TeamAccess.objects.count() >= NB_OBJECTS["teams"] + assert mailbox_models.MailDomain.objects.count() == NB_OBJECTS["domains"] # 3 domain access for each user with domain rights # 3 x 3 domain access for each user with both rights assert ( - mailbox_models.MailDomainAccess.objects.count() - == TEST_NB_OBJECTS["domains"] + 3 + 9 + mailbox_models.MailDomainAccess.objects.count() == NB_OBJECTS["domains"] + 3 + 9 ) diff --git a/src/backend/people/settings.py b/src/backend/people/settings.py index a9cbe7870..f363aad44 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 fbf23b68b..7695ca38e 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", From 61643ee5c06ee13aff08e2966f9bb59cdc63e2c1 Mon Sep 17 00:00:00 2001 From: Quentin BEY Date: Thu, 12 Dec 2024 18:08:15 +0100 Subject: [PATCH 2/5] =?UTF-8?q?=E2=9C=A8(teams)=20return=20parent=20teams?= =?UTF-8?q?=20in=20API?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also return the parent teams in the user's team endpoints. This is a first implementation which returns a flat list of teams (not a tree). This is not really helpful, but it allows to create hierarchical teams manually (via admin) if an organization needs it. --- src/backend/core/api/client/viewsets.py | 43 +++- .../tests/teams/test_core_api_teams_delete.py | 56 ++++++ .../tests/teams/test_core_api_teams_list.py | 186 ++++++++++++++++++ .../teams/test_core_api_teams_retrieve.py | 64 ++++++ .../tests/teams/test_core_api_teams_update.py | 72 +++++++ src/backend/core/tests/test_models_teams.py | 1 + 6 files changed, 418 insertions(+), 4 deletions(-) diff --git a/src/backend/core/api/client/viewsets.py b/src/backend/core/api/client/viewsets.py index 50167bf0e..95a1ef106 100644 --- a/src/backend/core/api/client/viewsets.py +++ b/src/backend/core/api/client/viewsets.py @@ -1,7 +1,11 @@ """API endpoints""" +import operator +from functools import reduce + from django.conf import settings -from django.db.models import OuterRef, Q, Subquery +from django.db.models import OuterRef, Q, Subquery, Value +from django.db.models.functions import Coalesce from rest_framework import ( decorators, @@ -299,13 +303,21 @@ class TeamViewSet( permission_classes = [permissions.AccessPermission] serializer_class = serializers.TeamSerializer filter_backends = [filters.OrderingFilter] - ordering_fields = ["created_at"] + ordering_fields = ["created_at", "name", "path"] ordering = ["-created_at"] queryset = models.Team.objects.all() pagination_class = None def get_queryset(self): """Custom queryset to get user related teams.""" + teams_queryset = models.Team.objects.filter( + accesses__user=self.request.user, + ) + depth_path = teams_queryset.values("depth", "path") + + if not depth_path: + return models.Team.objects.none() + user_role_query = models.TeamAccess.objects.filter( user=self.request.user, team=OuterRef("pk") ).values("role")[:1] @@ -313,9 +325,32 @@ def get_queryset(self): return ( models.Team.objects.prefetch_related("accesses", "service_providers") .filter( - accesses__user=self.request.user, + reduce( + operator.or_, + ( + Q( + # The team the user has access to + depth=d["depth"], + path=d["path"], + ) + | Q( + # The parent team the user has access to + depth__lt=d["depth"], + path__startswith=d["path"][: models.Team.steplen], + organization_id=self.request.user.organization_id, + ) + for d in depth_path + ), + ), + ) + # Abilities are computed based on logged-in user's role for the team + # and if the user does not have access, it's ok to consider them as a member + # because it's a parent team. + .annotate( + user_role=Coalesce( + Subquery(user_role_query), Value(models.RoleChoices.MEMBER.value) + ) ) - .annotate(user_role=Subquery(user_role_query)) ) def perform_create(self, serializer): diff --git a/src/backend/core/tests/teams/test_core_api_teams_delete.py b/src/backend/core/tests/teams/test_core_api_teams_delete.py index 4e3de4fa3..3426fb37d 100644 --- a/src/backend/core/tests/teams/test_core_api_teams_delete.py +++ b/src/backend/core/tests/teams/test_core_api_teams_delete.py @@ -113,3 +113,59 @@ def test_api_teams_delete_authenticated_owner(): assert response.status_code == HTTP_204_NO_CONTENT assert models.Team.objects.exists() is False + + +@pytest.mark.parametrize( + "role", + ["owner", "administrator", "member"], +) +def test_api_teams_delete_authenticated_owner_parent_team(client, role): + """ + Authenticated users should not be able to delete a parent team they + don't own. + """ + user = factories.UserFactory() + + client.force_login(user) + + root_team = factories.TeamFactory(name="Root") + first_team = factories.TeamFactory(name="First", parent_id=root_team.pk) + second_team = factories.TeamFactory(name="Second", parent_id=first_team.pk) + + # user is a member of the second team + factories.TeamAccessFactory(team=second_team, user=user, role=role) + + response = client.delete(f"/api/v1.0/teams/{first_team.pk}/") + + assert response.status_code == HTTP_403_FORBIDDEN + assert response.json() == { + "detail": "You do not have permission to perform this action." + } + assert models.Team.objects.count() == 3 + + +@pytest.mark.parametrize( + "role", + ["owner", "administrator", "member"], +) +def test_api_teams_delete_authenticated_owner_child_team(client, role): + """ + Authenticated users should not be able to delete a children team they + don't own. + """ + user = factories.UserFactory() + + client.force_login(user) + + root_team = factories.TeamFactory(name="Root") + first_team = factories.TeamFactory(name="First", parent_id=root_team.pk) + second_team = factories.TeamFactory(name="Second", parent_id=first_team.pk) + + # user is a member of the first team + factories.TeamAccessFactory(team=first_team, user=user, role=role) + + response = client.delete(f"/api/v1.0/teams/{second_team.pk}/") + + assert response.status_code == HTTP_404_NOT_FOUND + assert response.json() == {"detail": "No Team matches the given query."} + assert models.Team.objects.count() == 3 diff --git a/src/backend/core/tests/teams/test_core_api_teams_list.py b/src/backend/core/tests/teams/test_core_api_teams_list.py index 24ef55a56..8fa02acdb 100644 --- a/src/backend/core/tests/teams/test_core_api_teams_list.py +++ b/src/backend/core/tests/teams/test_core_api_teams_list.py @@ -125,3 +125,189 @@ def test_api_teams_order_param(): assert ( response_team_ids == team_ids ), "created_at values are not sorted from oldest to newest" + + +@pytest.mark.parametrize( + "role,local_team_abilities", + [ + ( + "owner", + { + "delete": True, + "get": True, + "manage_accesses": True, + "patch": True, + "put": True, + }, + ), + ( + "administrator", + { + "delete": False, + "get": True, + "manage_accesses": True, + "patch": True, + "put": True, + }, + ), + ( + "member", + { + "delete": False, + "get": True, + "manage_accesses": False, + "patch": False, + "put": False, + }, + ), + ], +) +def test_api_teams_list_authenticated_team_tree(client, role, local_team_abilities): + """ + Authenticated users should be able to list teams + they are an owner/administrator/member of, or any parent teams. + """ + user = factories.UserFactory() + + client.force_login(user) + + root_team = factories.TeamFactory(name="Root") + first_team = factories.TeamFactory(name="First", parent_id=root_team.pk) + second_team = factories.TeamFactory(name="Second", parent_id=first_team.pk) + third_team = factories.TeamFactory(name="Third", parent_id=second_team.pk) + _fourth_team = factories.TeamFactory(name="Fourth", parent_id=third_team.pk) + + # user is a member of the second team + user_access = factories.TeamAccessFactory(team=second_team, user=user, role=role) + + response = client.get("/api/v1.0/teams/") + + assert response.status_code == HTTP_200_OK + # By default, the teams are sorted by 'created_at' descending + assert response.json() == [ + { + # I have the abilities only on the team I have a specific role + "abilities": local_team_abilities, + "accesses": [str(user_access.pk)], + "created_at": second_team.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + "id": str(second_team.pk), + "name": "Second", + "service_providers": [], + "updated_at": second_team.updated_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + }, + { + # For parent teams, I only have the ability to list/retrieve + "abilities": { + "delete": False, + "get": True, + "manage_accesses": False, + "patch": False, + "put": False, + }, + "accesses": [], + "created_at": first_team.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + "id": str(first_team.pk), + "name": "First", + "service_providers": [], + "updated_at": first_team.updated_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + }, + { + # For parent teams, I only have the ability to list/retrieve + "abilities": { + "delete": False, + "get": True, + "manage_accesses": False, + "patch": False, + "put": False, + }, + "accesses": [], + "created_at": root_team.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + "id": str(root_team.pk), + "name": "Root", + "service_providers": [], + "updated_at": root_team.updated_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + }, + ] + + +@pytest.mark.parametrize( + "role,local_team_abilities", + [ + ( + "owner", + { + "delete": True, + "get": True, + "manage_accesses": True, + "patch": True, + "put": True, + }, + ), + ( + "administrator", + { + "delete": False, + "get": True, + "manage_accesses": True, + "patch": True, + "put": True, + }, + ), + ( + "member", + { + "delete": False, + "get": True, + "manage_accesses": False, + "patch": False, + "put": False, + }, + ), + ], +) +def test_api_teams_list_authenticated_team_different_organization( + client, role, local_team_abilities +): + """ + Authenticated users should be able to list teams they + are an owner/administrator/member of and any parent teams + only if from the same organization. + """ + organization = factories.OrganizationFactory(with_registration_id=True) + user = factories.UserFactory(organization=organization) + + other_organization = factories.OrganizationFactory(with_registration_id=True) + root_team = factories.TeamFactory(name="Root", organization=other_organization) + first_team = factories.TeamFactory( + name="First", parent_id=root_team.pk, organization=other_organization + ) + second_team = factories.TeamFactory( + name="Second", parent_id=first_team.pk, organization=other_organization + ) + third_team = factories.TeamFactory( + name="Third", parent_id=second_team.pk, organization=other_organization + ) + _fourth_team = factories.TeamFactory( + name="Fourth", parent_id=third_team.pk, organization=other_organization + ) + + client.force_login(user) + + # user is a member of the second team + user_access = factories.TeamAccessFactory(team=second_team, user=user, role=role) + + response = client.get("/api/v1.0/teams/") + + assert response.status_code == HTTP_200_OK + assert response.json() == [ + { + # I have the abilities only on the team I have a specific role + "abilities": local_team_abilities, + "accesses": [str(user_access.pk)], + "created_at": second_team.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + "id": str(second_team.pk), + "name": "Second", + "service_providers": [], + "updated_at": second_team.updated_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + }, + ] diff --git a/src/backend/core/tests/teams/test_core_api_teams_retrieve.py b/src/backend/core/tests/teams/test_core_api_teams_retrieve.py index 607dec778..7782e4b0e 100644 --- a/src/backend/core/tests/teams/test_core_api_teams_retrieve.py +++ b/src/backend/core/tests/teams/test_core_api_teams_retrieve.py @@ -74,3 +74,67 @@ def test_api_teams_retrieve_authenticated_related(): "updated_at": team.updated_at.isoformat().replace("+00:00", "Z"), "service_providers": [], } + + +@pytest.mark.parametrize( + "role", + ["owner", "administrator", "member"], +) +def test_api_teams_retrieve_authenticated_related_parent(client, role): + """ + Authenticated users should be allowed to retrieve a parent team + to which they are related through the child team whatever the role. + """ + user = factories.UserFactory() + + client.force_login(user) + + root_team = factories.TeamFactory(name="Root") + first_team = factories.TeamFactory(name="First", parent_id=root_team.pk) + second_team = factories.TeamFactory(name="Second", parent_id=first_team.pk) + + # user is a member of the second team + factories.TeamAccessFactory(team=second_team, user=user, role=role) + + response = client.get(f"/api/v1.0/teams/{first_team.pk!s}/") + + # the abilities enforces the "get" via the queryset + abilities = first_team.get_abilities(user) + abilities["get"] = True + + assert response.status_code == status.HTTP_200_OK + assert response.json() == { + "id": str(first_team.pk), + "name": first_team.name, + "abilities": abilities, + "accesses": [], + "created_at": first_team.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": first_team.updated_at.isoformat().replace("+00:00", "Z"), + "service_providers": [], + } + + +@pytest.mark.parametrize( + "role", + ["owner", "administrator", "member"], +) +def test_api_teams_retrieve_authenticated_related_children(client, role): + """ + Authenticated users should NOT be allowed to retrieve a child team + to which they are related through the parent team whatever the role. + """ + user = factories.UserFactory() + + client.force_login(user) + + root_team = factories.TeamFactory(name="Root") + first_team = factories.TeamFactory(name="First", parent_id=root_team.pk) + second_team = factories.TeamFactory(name="Second", parent_id=first_team.pk) + + # user is a member of the first team + factories.TeamAccessFactory(team=first_team, user=user, role=role) + + response = client.get(f"/api/v1.0/teams/{second_team.pk!s}/") + + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.json() == {"detail": "No Team matches the given query."} diff --git a/src/backend/core/tests/teams/test_core_api_teams_update.py b/src/backend/core/tests/teams/test_core_api_teams_update.py index 81379e31c..c5d6cd526 100644 --- a/src/backend/core/tests/teams/test_core_api_teams_update.py +++ b/src/backend/core/tests/teams/test_core_api_teams_update.py @@ -219,3 +219,75 @@ def test_api_teams_update_authenticated_owners_add_service_providers(): team.refresh_from_db() assert team.service_providers.count() == 2 assert set(team.service_providers.all()) == {service_provider_1, service_provider_2} + + +@pytest.mark.parametrize( + "role", + ["owner", "administrator", "member"], +) +def test_api_teams_update_whatever_access_of_child_team(client, role): + """ + Being member, administrator or owner of a team should not grant + authorization to update a parent team. + """ + user = factories.UserFactory() + + client.force_login(user) + + root_team = factories.TeamFactory(name="Root") + first_team = factories.TeamFactory(name="First", parent_id=root_team.pk) + second_team = factories.TeamFactory(name="Second", parent_id=first_team.pk) + + # user is a member of the second team + factories.TeamAccessFactory(team=second_team, user=user, role=role) + + response = client.patch( + f"/api/v1.0/teams/{first_team.pk}/", + { + "name": "New name", + }, + format="json", + ) + + assert response.status_code == HTTP_403_FORBIDDEN + assert response.json() == { + "detail": "You do not have permission to perform this action." + } + + first_team.refresh_from_db() + assert first_team.name == "First" + + +@pytest.mark.parametrize( + "role", + ["owner", "administrator", "member"], +) +def test_api_teams_update_whatever_access_of_parent_team(client, role): + """ + Being member, administrator or owner of a team should not grant + authorization to update a child team. + """ + user = factories.UserFactory() + + client.force_login(user) + + root_team = factories.TeamFactory(name="Root") + first_team = factories.TeamFactory(name="First", parent_id=root_team.pk) + second_team = factories.TeamFactory(name="Second", parent_id=first_team.pk) + + # user is a member of the first team + factories.TeamAccessFactory(team=first_team, user=user, role=role) + + response = client.patch( + f"/api/v1.0/teams/{second_team.pk}/", + { + "name": "New name", + }, + format="json", + ) + + assert response.status_code == HTTP_404_NOT_FOUND + assert response.json() == {"detail": "No Team matches the given query."} + + second_team.refresh_from_db() + assert second_team.name == "Second" diff --git a/src/backend/core/tests/test_models_teams.py b/src/backend/core/tests/test_models_teams.py index bb6298c81..18ff2223d 100644 --- a/src/backend/core/tests/test_models_teams.py +++ b/src/backend/core/tests/test_models_teams.py @@ -138,6 +138,7 @@ def test_models_teams_get_abilities_preset_role(django_assert_num_queries): # test trees + def test_models_teams_create_root_team(): """Create a root team.""" team = models.Team.add_root(name="Root Team") From d68645e8b24e7b7861f4a640b64deae8a98b80d8 Mon Sep 17 00:00:00 2001 From: Quentin BEY Date: Tue, 17 Dec 2024 18:21:24 +0100 Subject: [PATCH 3/5] =?UTF-8?q?=E2=9C=A8(teams)=20return=20parent=20teams?= =?UTF-8?q?=20in=20resource=20server?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also return the parent teams in the user's team endpoints. --- .../core/api/resource_server/viewsets.py | 41 +++++- .../resource_server_api/teams/test_list.py | 81 ++++++++++- .../teams/test_retrieve.py | 128 +++++++++++++++++- .../resource_server_api/teams/test_update.py | 92 +++++++++++++ 4 files changed, 337 insertions(+), 5 deletions(-) diff --git a/src/backend/core/api/resource_server/viewsets.py b/src/backend/core/api/resource_server/viewsets.py index 075a16c76..c14e4fb11 100644 --- a/src/backend/core/api/resource_server/viewsets.py +++ b/src/backend/core/api/resource_server/viewsets.py @@ -1,6 +1,10 @@ """Resource server API endpoints""" -from django.db.models import OuterRef, Prefetch, Subquery +import operator +from functools import reduce + +from django.db.models import OuterRef, Prefetch, Q, Subquery, Value +from django.db.models.functions import Coalesce from rest_framework import ( filters, @@ -56,6 +60,14 @@ class TeamViewSet( # pylint: disable=too-many-ancestors def get_queryset(self): """Custom queryset to get user related teams.""" + teams_queryset = models.Team.objects.filter( + accesses__user=self.request.user, + ) + depth_path = teams_queryset.values("depth", "path") + + if not depth_path: + return models.Team.objects.none() + user_role_query = models.TeamAccess.objects.filter( user=self.request.user, team=OuterRef("pk") ).values("role")[:1] @@ -74,10 +86,33 @@ def get_queryset(self): service_provider_prefetch, ) .filter( - accesses__user=self.request.user, + reduce( + operator.or_, + ( + Q( + # The team the user has access to + depth=d["depth"], + path=d["path"], + ) + | Q( + # The parent team the user has access to + depth__lt=d["depth"], + path__startswith=d["path"][: models.Team.steplen], + organization_id=self.request.user.organization_id, + ) + for d in depth_path + ), + ), service_providers__audience_id=service_provider_audience, ) - .annotate(user_role=Subquery(user_role_query)) + # Abilities are computed based on logged-in user's role for the team + # and if the user does not have access, it's ok to consider them as a member + # because it's a parent team. + .annotate( + user_role=Coalesce( + Subquery(user_role_query), Value(models.RoleChoices.MEMBER.value) + ) + ) ) def perform_create(self, serializer): diff --git a/src/backend/core/tests/resource_server_api/teams/test_list.py b/src/backend/core/tests/resource_server_api/teams/test_list.py index 121b334df..e72904130 100644 --- a/src/backend/core/tests/resource_server_api/teams/test_list.py +++ b/src/backend/core/tests/resource_server_api/teams/test_list.py @@ -64,7 +64,8 @@ def test_api_teams_list_authenticated( # pylint: disable=too-many-locals # Authenticate using the resource server, ie via the Authorization header with force_login_via_resource_server(client, user, service_provider.audience_id): - with django_assert_num_queries(4): # Count, Team, ServiceProvider, TeamAccess + with django_assert_num_queries(5): + # queries: Team path, Count, Team, ServiceProvider, TeamAccess response = client.get( "/resource-server/v1.0/teams/?ordering=created_at", format="json", @@ -182,3 +183,81 @@ def test_api_teams_order_param(client, force_login_via_resource_server): assert ( response_team_ids == team_ids ), "created_at values are not sorted from oldest to newest" + + +def test_api_teams_list_with_parent_teams(client, force_login_via_resource_server): + """ + Authenticated users should be able to list teams including parent teams. + Parent teams should not be listed if they don't have the service provider. + """ + user = factories.UserFactory() + service_provider = factories.ServiceProviderFactory() + + root_team = factories.TeamFactory(name="Root", service_providers=[service_provider]) + first_team = factories.TeamFactory(name="First", parent_id=root_team.pk) + second_team = factories.TeamFactory( + name="Second", parent_id=first_team.pk, service_providers=[service_provider] + ) + + factories.TeamAccessFactory(user=user, team=second_team, role="member") + + with force_login_via_resource_server(client, user, service_provider.audience_id): + response = client.get( + "/resource-server/v1.0/teams/", + format="json", + HTTP_AUTHORIZATION="Bearer b64untestedbearertoken", + ) + + assert response.status_code == HTTP_200_OK + response_data = response.json() + assert response_data["count"] == 2 + + team_ids = [team["id"] for team in response_data["results"]] + assert len(team_ids) == 2 + assert set(team_ids) == {str(root_team.id), str(second_team.id)} + + +def test_api_teams_list_with_parent_teams_other_organization( + client, force_login_via_resource_server +): + """ + Authenticated users should be able to list teams including parent teams. + Parent teams should not be listed if they don't have the service provider + or if the user does not belong to the organization. + """ + organization = factories.OrganizationFactory(with_registration_id=True) + user = factories.UserFactory(organization=organization) + service_provider = factories.ServiceProviderFactory() + + other_organization = factories.OrganizationFactory(with_registration_id=True) + root_team = factories.TeamFactory( + name="Root", + service_providers=[service_provider], + organization=other_organization, + ) + first_team = factories.TeamFactory( + name="First", parent_id=root_team.pk, organization=other_organization + ) + second_team = factories.TeamFactory( + name="Second", + parent_id=first_team.pk, + service_providers=[service_provider], + organization=other_organization, + ) + + factories.TeamAccessFactory(user=user, team=second_team, role="member") + + with force_login_via_resource_server(client, user, service_provider.audience_id): + response = client.get( + "/resource-server/v1.0/teams/", + format="json", + HTTP_AUTHORIZATION="Bearer b64untestedbearertoken", + ) + + assert response.status_code == HTTP_200_OK + response_data = response.json() + assert response_data["count"] == 1 + + team_ids = [team["id"] for team in response_data["results"]] + assert len(team_ids) == 1 + assert set(team_ids) == {str(second_team.id)} diff --git a/src/backend/core/tests/resource_server_api/teams/test_retrieve.py b/src/backend/core/tests/resource_server_api/teams/test_retrieve.py index 058936097..cdf0e1ba7 100644 --- a/src/backend/core/tests/resource_server_api/teams/test_retrieve.py +++ b/src/backend/core/tests/resource_server_api/teams/test_retrieve.py @@ -8,7 +8,7 @@ from rest_framework.test import APIClient from core import factories -from core.factories import UserFactory +from core.factories import TeamAccessFactory, UserFactory pytestmark = pytest.mark.django_db @@ -97,3 +97,129 @@ def test_api_teams_retrieve_authenticated_other_service_provider( ) assert response.status_code == HTTP_404_NOT_FOUND + + +def test_api_teams_retrieve_authenticated_related_parent_same_organization( + client, force_login_via_resource_server +): + """ + Authenticated users should be allowed to retrieve a parent team + of a team to which they are related, only if they belong to the + same organization. + """ + organization = factories.OrganizationFactory(with_registration_id=True) + user = factories.UserFactory(organization=organization) + service_provider = factories.ServiceProviderFactory() + + root_team = factories.TeamFactory( + name="Root", + organization=organization, + ) + first_team = factories.TeamFactory( + name="First", + parent_id=root_team.pk, + organization=organization, + service_providers=[service_provider], + ) + second_team = factories.TeamFactory( + name="Second", + parent_id=first_team.pk, + service_providers=[service_provider], + organization=organization, + ) + TeamAccessFactory(user=user, team=second_team, role="member") + + with force_login_via_resource_server(client, user, service_provider.audience_id): + response = client.get( + f"/resource-server/v1.0/teams/{first_team.pk}/", + HTTP_AUTHORIZATION="Bearer b64untestedbearertoken", + ) + + assert response.status_code == status.HTTP_200_OK + assert response.json() == { + "id": str(first_team.pk), + "name": first_team.name, + "created_at": first_team.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": first_team.updated_at.isoformat().replace("+00:00", "Z"), + } + + +def test_api_teams_retrieve_authenticated_related_parent_other_organization( + client, force_login_via_resource_server +): + """ + Authenticated users should be allowed to retrieve a parent team + of a team to which they are related, only if they belong to the + same organization. + """ + organization = factories.OrganizationFactory(with_registration_id=True) + user = factories.UserFactory(organization=organization) + service_provider = factories.ServiceProviderFactory() + + other_organization = factories.OrganizationFactory(with_registration_id=True) + root_team = factories.TeamFactory( + name="Root", + organization=other_organization, + ) + first_team = factories.TeamFactory( + name="First", + parent_id=root_team.pk, + organization=other_organization, + service_providers=[service_provider], + ) + second_team = factories.TeamFactory( + name="Second", + parent_id=first_team.pk, + service_providers=[service_provider], + organization=other_organization, + ) + TeamAccessFactory(user=user, team=second_team, role="member") + + with force_login_via_resource_server(client, user, service_provider.audience_id): + response = client.get( + f"/resource-server/v1.0/teams/{first_team.pk}/", + HTTP_AUTHORIZATION="Bearer b64untestedbearertoken", + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.json() == {"detail": "No Team matches the given query."} + + +def test_api_teams_retrieve_authenticated_related_child_same_organization( + client, force_login_via_resource_server +): + """ + Authenticated users should NOT be allowed to retrieve a child team + of a team to which they are related, even if they belong to the + same organization. + """ + organization = factories.OrganizationFactory(with_registration_id=True) + user = factories.UserFactory(organization=organization) + service_provider = factories.ServiceProviderFactory() + + root_team = factories.TeamFactory( + name="Root", + organization=organization, + ) + first_team = factories.TeamFactory( + name="First", + parent_id=root_team.pk, + organization=organization, + service_providers=[service_provider], + ) + second_team = factories.TeamFactory( + name="Second", + parent_id=first_team.pk, + service_providers=[service_provider], + organization=organization, + ) + TeamAccessFactory(user=user, team=first_team, role="member") + + with force_login_via_resource_server(client, user, service_provider.audience_id): + response = client.get( + f"/resource-server/v1.0/teams/{second_team.pk}/", + HTTP_AUTHORIZATION="Bearer b64untestedbearertoken", + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.json() == {"detail": "No Team matches the given query."} diff --git a/src/backend/core/tests/resource_server_api/teams/test_update.py b/src/backend/core/tests/resource_server_api/teams/test_update.py index 852f7d689..467c95a49 100644 --- a/src/backend/core/tests/resource_server_api/teams/test_update.py +++ b/src/backend/core/tests/resource_server_api/teams/test_update.py @@ -244,3 +244,95 @@ def test_api_teams_update_administrator_or_owner_of_another( team.refresh_from_db() team_values = serializers.TeamSerializer(instance=team).data assert team_values == old_team_values + + +@pytest.mark.parametrize("role", ["administrator", "member", "owner"]) +def test_api_teams_update_parent_team(client, force_login_via_resource_server, role): + """ + Belonging to a team should NOT grant authorization to update + another parent team. + """ + organization = factories.OrganizationFactory(with_registration_id=True) + user = factories.UserFactory(organization=organization) + service_provider = factories.ServiceProviderFactory() + + root_team = factories.TeamFactory( + name="Root", + organization=organization, + ) + first_team = factories.TeamFactory( + name="First", + parent_id=root_team.pk, + organization=organization, + service_providers=[service_provider], + ) + second_team = factories.TeamFactory( + name="Second", + parent_id=first_team.pk, + service_providers=[service_provider], + organization=organization, + ) + factories.TeamAccessFactory(user=user, team=second_team, role=role) + + with force_login_via_resource_server(client, user, service_provider.audience_id): + response = client.patch( + f"/resource-server/v1.0/teams/{first_team.pk}/", + { + "name": "New name", + }, + content_type="application/json", + HTTP_AUTHORIZATION="Bearer b64untestedbearertoken", + ) + + assert response.status_code == HTTP_403_FORBIDDEN + assert response.json() == { + "detail": "You do not have permission to perform this action." + } + + first_team.refresh_from_db() + assert first_team.name == "First" + + +@pytest.mark.parametrize("role", ["administrator", "member", "owner"]) +def test_api_teams_update_child_team(client, force_login_via_resource_server, role): + """ + Belonging to a team should NOT grant authorization to update + another child team. + """ + organization = factories.OrganizationFactory(with_registration_id=True) + user = factories.UserFactory(organization=organization) + service_provider = factories.ServiceProviderFactory() + + root_team = factories.TeamFactory( + name="Root", + organization=organization, + ) + first_team = factories.TeamFactory( + name="First", + parent_id=root_team.pk, + organization=organization, + service_providers=[service_provider], + ) + second_team = factories.TeamFactory( + name="Second", + parent_id=first_team.pk, + service_providers=[service_provider], + organization=organization, + ) + factories.TeamAccessFactory(user=user, team=first_team, role=role) + + with force_login_via_resource_server(client, user, service_provider.audience_id): + response = client.patch( + f"/resource-server/v1.0/teams/{second_team.pk}/", + { + "name": "New name", + }, + content_type="application/json", + HTTP_AUTHORIZATION="Bearer b64untestedbearertoken", + ) + + assert response.status_code == HTTP_404_NOT_FOUND + assert response.json() == {"detail": "No Team matches the given query."} + + second_team.refresh_from_db() + assert second_team.name == "Second" From 38df22d90e55e773c26946e6634398ee49bb5a76 Mon Sep 17 00:00:00 2001 From: Quentin BEY Date: Tue, 7 Jan 2025 15:45:37 +0100 Subject: [PATCH 4/5] =?UTF-8?q?=E2=9C=A8(teams)=20add=20treebeard=20data?= =?UTF-8?q?=20to=20serializers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will allow the frontend to represent teams as a tree if needed. --- src/backend/core/api/client/serializers.py | 14 ++++++++++---- .../core/api/resource_server/serializers.py | 8 +++++++- .../resource_server_api/teams/test_create.py | 3 +++ .../resource_server_api/teams/test_list.py | 12 ++++++++++++ .../resource_server_api/teams/test_retrieve.py | 10 ++++++++-- .../tests/teams/test_core_api_teams_list.py | 12 ++++++++++++ .../teams/test_core_api_teams_retrieve.py | 18 ++++++++++++------ .../tests/teams/test_core_api_teams_update.py | 4 ++-- 8 files changed, 66 insertions(+), 15 deletions(-) diff --git a/src/backend/core/api/client/serializers.py b/src/backend/core/api/client/serializers.py index df0410193..2d3864f13 100644 --- a/src/backend/core/api/client/serializers.py +++ b/src/backend/core/api/client/serializers.py @@ -259,18 +259,24 @@ class Meta: model = models.Team fields = [ "id", - "name", - "accesses", "abilities", + "accesses", "created_at", - "updated_at", + "depth", + "name", + "numchild", + "path", "service_providers", + "updated_at", ] read_only_fields = [ "id", - "accesses", "abilities", + "accesses", "created_at", + "depth", + "numchild", + "path", "updated_at", ] diff --git a/src/backend/core/api/resource_server/serializers.py b/src/backend/core/api/resource_server/serializers.py index 01029bf39..07973cf9a 100644 --- a/src/backend/core/api/resource_server/serializers.py +++ b/src/backend/core/api/resource_server/serializers.py @@ -12,13 +12,19 @@ class Meta: model = models.Team fields = [ "id", - "name", "created_at", + "depth", + "name", + "numchild", + "path", "updated_at", ] read_only_fields = [ "id", "created_at", + "depth", + "numchild", + "path", "updated_at", ] diff --git a/src/backend/core/tests/resource_server_api/teams/test_create.py b/src/backend/core/tests/resource_server_api/teams/test_create.py index 68ab4cadf..21a3b0bf3 100644 --- a/src/backend/core/tests/resource_server_api/teams/test_create.py +++ b/src/backend/core/tests/resource_server_api/teams/test_create.py @@ -59,7 +59,10 @@ def test_api_teams_create_authenticated_new_service_provider( assert response.json() == { "created_at": team.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), "id": str(team.pk), + "depth": team.depth, "name": "my team", + "numchild": team.numchild, + "path": team.path, "updated_at": team.updated_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), } diff --git a/src/backend/core/tests/resource_server_api/teams/test_list.py b/src/backend/core/tests/resource_server_api/teams/test_list.py index e72904130..20938345d 100644 --- a/src/backend/core/tests/resource_server_api/teams/test_list.py +++ b/src/backend/core/tests/resource_server_api/teams/test_list.py @@ -80,26 +80,38 @@ def test_api_teams_list_authenticated( # pylint: disable=too-many-locals "results": [ { "created_at": team_1.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + "depth": team_1.depth, "id": str(team_1.pk), "name": team_1.name, + "numchild": team_1.numchild, + "path": team_1.path, "updated_at": team_1.updated_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), }, { "created_at": team_2.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + "depth": team_2.depth, "id": str(team_2.pk), "name": team_2.name, + "numchild": team_2.numchild, + "path": team_2.path, "updated_at": team_2.updated_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), }, { "created_at": team_3.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + "depth": team_3.depth, "id": str(team_3.pk), "name": team_3.name, + "numchild": team_3.numchild, + "path": team_3.path, "updated_at": team_3.updated_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), }, { "created_at": team_4.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + "depth": team_4.depth, "id": str(team_4.pk), "name": team_4.name, + "numchild": team_4.numchild, + "path": team_4.path, "updated_at": team_4.updated_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), }, ], diff --git a/src/backend/core/tests/resource_server_api/teams/test_retrieve.py b/src/backend/core/tests/resource_server_api/teams/test_retrieve.py index cdf0e1ba7..c162945ce 100644 --- a/src/backend/core/tests/resource_server_api/teams/test_retrieve.py +++ b/src/backend/core/tests/resource_server_api/teams/test_retrieve.py @@ -66,9 +66,12 @@ def test_api_teams_retrieve_authenticated_related( assert response.status_code == status.HTTP_200_OK assert response.json() == { + "created_at": team.created_at.isoformat().replace("+00:00", "Z"), + "depth": 1, "id": str(team.id), "name": team.name, - "created_at": team.created_at.isoformat().replace("+00:00", "Z"), + "numchild": 0, + "path": team.path, "updated_at": team.updated_at.isoformat().replace("+00:00", "Z"), } @@ -137,9 +140,12 @@ def test_api_teams_retrieve_authenticated_related_parent_same_organization( assert response.status_code == status.HTTP_200_OK assert response.json() == { + "created_at": first_team.created_at.isoformat().replace("+00:00", "Z"), + "depth": 2, "id": str(first_team.pk), "name": first_team.name, - "created_at": first_team.created_at.isoformat().replace("+00:00", "Z"), + "numchild": 1, + "path": first_team.path, "updated_at": first_team.updated_at.isoformat().replace("+00:00", "Z"), } diff --git a/src/backend/core/tests/teams/test_core_api_teams_list.py b/src/backend/core/tests/teams/test_core_api_teams_list.py index 8fa02acdb..020ad0f9f 100644 --- a/src/backend/core/tests/teams/test_core_api_teams_list.py +++ b/src/backend/core/tests/teams/test_core_api_teams_list.py @@ -190,8 +190,11 @@ def test_api_teams_list_authenticated_team_tree(client, role, local_team_abiliti "abilities": local_team_abilities, "accesses": [str(user_access.pk)], "created_at": second_team.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + "depth": 3, "id": str(second_team.pk), "name": "Second", + "numchild": 1, + "path": second_team.path, "service_providers": [], "updated_at": second_team.updated_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), }, @@ -206,8 +209,11 @@ def test_api_teams_list_authenticated_team_tree(client, role, local_team_abiliti }, "accesses": [], "created_at": first_team.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + "depth": 2, "id": str(first_team.pk), "name": "First", + "numchild": 1, + "path": first_team.path, "service_providers": [], "updated_at": first_team.updated_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), }, @@ -222,8 +228,11 @@ def test_api_teams_list_authenticated_team_tree(client, role, local_team_abiliti }, "accesses": [], "created_at": root_team.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + "depth": 1, "id": str(root_team.pk), "name": "Root", + "numchild": 1, + "path": root_team.path, "service_providers": [], "updated_at": root_team.updated_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), }, @@ -305,8 +314,11 @@ def test_api_teams_list_authenticated_team_different_organization( "abilities": local_team_abilities, "accesses": [str(user_access.pk)], "created_at": second_team.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + "depth": 3, "id": str(second_team.pk), "name": "Second", + "numchild": 1, + "path": second_team.path, "service_providers": [], "updated_at": second_team.updated_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), }, diff --git a/src/backend/core/tests/teams/test_core_api_teams_retrieve.py b/src/backend/core/tests/teams/test_core_api_teams_retrieve.py index 7782e4b0e..08128f73f 100644 --- a/src/backend/core/tests/teams/test_core_api_teams_retrieve.py +++ b/src/backend/core/tests/teams/test_core_api_teams_retrieve.py @@ -67,12 +67,15 @@ def test_api_teams_retrieve_authenticated_related(): ] ) assert response.json() == { - "id": str(team.id), - "name": team.name, "abilities": team.get_abilities(user), "created_at": team.created_at.isoformat().replace("+00:00", "Z"), - "updated_at": team.updated_at.isoformat().replace("+00:00", "Z"), + "depth": 1, + "id": str(team.id), + "name": team.name, + "numchild": 0, + "path": team.path, "service_providers": [], + "updated_at": team.updated_at.isoformat().replace("+00:00", "Z"), } @@ -104,13 +107,16 @@ def test_api_teams_retrieve_authenticated_related_parent(client, role): assert response.status_code == status.HTTP_200_OK assert response.json() == { - "id": str(first_team.pk), - "name": first_team.name, "abilities": abilities, "accesses": [], "created_at": first_team.created_at.isoformat().replace("+00:00", "Z"), - "updated_at": first_team.updated_at.isoformat().replace("+00:00", "Z"), + "depth": 2, + "id": str(first_team.pk), + "name": first_team.name, + "numchild": 1, + "path": first_team.path, "service_providers": [], + "updated_at": first_team.updated_at.isoformat().replace("+00:00", "Z"), } diff --git a/src/backend/core/tests/teams/test_core_api_teams_update.py b/src/backend/core/tests/teams/test_core_api_teams_update.py index c5d6cd526..8f23be480 100644 --- a/src/backend/core/tests/teams/test_core_api_teams_update.py +++ b/src/backend/core/tests/teams/test_core_api_teams_update.py @@ -119,7 +119,7 @@ def test_api_teams_update_authenticated_administrators(): team.refresh_from_db() final_values = serializers.TeamSerializer(instance=team).data for key, value in final_values.items(): - if key in ["id", "accesses", "created_at"]: + if key in ["id", "accesses", "created_at", "depth", "path", "numchild"]: assert value == initial_values[key] elif key == "updated_at": assert value > initial_values[key] @@ -152,7 +152,7 @@ def test_api_teams_update_authenticated_owners(): team.refresh_from_db() team_values = serializers.TeamSerializer(instance=team).data for key, value in team_values.items(): - if key in ["id", "accesses", "created_at"]: + if key in ["id", "accesses", "created_at", "depth", "path", "numchild"]: assert value == old_team_values[key] elif key == "updated_at": assert value > old_team_values[key] From 26b1880bf34b74b8d3f910c7e5a51c7660946a8e Mon Sep 17 00:00:00 2001 From: Quentin BEY Date: Tue, 14 Jan 2025 16:30:42 +0100 Subject: [PATCH 5/5] =?UTF-8?q?fixup!=20=F0=9F=97=83=EF=B8=8F(teams)=20add?= =?UTF-8?q?=20Team=20dependencies=20as=20a=20tree?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../demo/management/commands/create_demo.py | 20 ++----------------- 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/src/backend/demo/management/commands/create_demo.py b/src/backend/demo/management/commands/create_demo.py index d59d1bea1..a88ee157b 100755 --- a/src/backend/demo/management/commands/create_demo.py +++ b/src/backend/demo/management/commands/create_demo.py @@ -133,20 +133,6 @@ def __exit__(self, exc_type, exc_value, exc_tb): return elapsed_time -def _get_random_domain_name(excluded): - """Get a fake domain using an exclusion list.""" - name = fake.domain_name() - for _max_retry in range(10): # Limit iterations to avoid infinite loop - if name not in excluded: - return name - name = fake.domain_name() - - if name in excluded: - raise RuntimeError("Could not find a domain name that is not already created") - - return name - - def create_demo(stdout): # pylint: disable=too-many-locals """ Create a database with demo data for developers to work in a realistic environment. @@ -233,10 +219,8 @@ 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 = _get_random_domain_name(created) - created.add(name) + for i in range(defaults.NB_OBJECTS["domains"]): + name = f"{fake.domain_name()}-i{i:d}" queue.push( mailbox_models.MailDomain(