From 99cee241f99a4bd667d3ad2f76dbce7a69c9a09a Mon Sep 17 00:00:00 2001
From: Lebaud Antoine <lebaud.antoine131@gmail.com>
Date: Sat, 9 Mar 2024 18:08:03 +0100
Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(api)=20support=20TeamAccess=20orderin?=
 =?UTF-8?q?g=20on=20user-based=20fields?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Important ordering fields for TeamAccess depend on user's
identities data. User and identities has a one-to-many relationship,
which forced us to prefetch the user-related data when listing
team's accesses.

Prefetch get data from the database using two SQL queries, and
join data in Python. User's data were not available in the first
SQL query.

Without annotating the query set with user main identities data,
we could not use default OrderingFilter backend code, which order_by()
the queryset.
---
 src/backend/core/api/viewsets.py              | 14 +++--
 .../test_api_team_accesses_list.py            | 52 ++++++++++++++++---
 2 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py
index b99fa80..2595375 100644
--- a/src/backend/core/api/viewsets.py
+++ b/src/backend/core/api/viewsets.py
@@ -327,8 +327,8 @@ class TeamAccessViewSet(
     detail_serializer_class = serializers.TeamAccessSerializer
 
     filter_backends = [filters.OrderingFilter]
-    ordering = ['role']
-    ordering_fields = ['role']
+    ordering = ["role"]
+    ordering_fields = ["role", "email", "name"]
 
     def get_permissions(self):
         """User only needs to be authenticated to list team accesses"""
@@ -361,6 +361,10 @@ class TeamAccessViewSet(
                 user=self.request.user, team=self.kwargs["team_id"]
             ).values("role")[:1]
 
+            user_main_identity_query = models.Identity.objects.filter(
+                user=OuterRef("user_id"), is_main=True
+            )
+
             queryset = (
                 # The logged-in user should be part of a team to see its accesses
                 queryset.filter(
@@ -375,7 +379,11 @@ class TeamAccessViewSet(
                 )
                 # Abilities are computed based on logged-in user's role and
                 # the user role on each team access
-                .annotate(user_role=Subquery(user_role_query))
+                .annotate(
+                    user_role=Subquery(user_role_query),
+                    email=Subquery(user_main_identity_query.values("email")[:1]),
+                    name=Subquery(user_main_identity_query.values("name")[:1]),
+                )
                 .distinct()
             )
         return queryset
diff --git a/src/backend/core/tests/team_accesses/test_api_team_accesses_list.py b/src/backend/core/tests/team_accesses/test_api_team_accesses_list.py
index ec237f2..a76213d 100644
--- a/src/backend/core/tests/team_accesses/test_api_team_accesses_list.py
+++ b/src/backend/core/tests/team_accesses/test_api_team_accesses_list.py
@@ -3,6 +3,7 @@ Test for team accesses API endpoints in People's core app : list
 """
 
 import pytest
+from rest_framework.status import HTTP_200_OK
 from rest_framework.test import APIClient
 
 from core import factories, models
@@ -225,23 +226,60 @@ def test_api_team_accesses_list_authenticated_ordering():
     response = client.get(
         f"/api/v1.0/teams/{team.id!s}/accesses/?ordering=role",
     )
-    assert response.status_code == 200
+    assert response.status_code == HTTP_200_OK
     assert response.json()["count"] == 21
 
-    results = [
-        team_access["role"]
-        for team_access in response.json()["results"]
-    ]
+    results = [team_access["role"] for team_access in response.json()["results"]]
     assert sorted(results) == results
 
     response = client.get(
         f"/api/v1.0/teams/{team.id!s}/accesses/?ordering=-role",
     )
-    assert response.status_code == 200
+    assert response.status_code == HTTP_200_OK
+    assert response.json()["count"] == 21
+
+    results = [team_access["role"] for team_access in response.json()["results"]]
+    assert sorted(results, reverse=True) == results
+
+
+@pytest.mark.parametrize("ordering_fields", ["name", "email"])
+def test_api_team_accesses_list_authenticated_ordering_user(ordering_fields):
+    """Team accesses can be ordered by user's fields "email" or "name"."""
+
+    user = factories.UserFactory()
+    factories.IdentityFactory(user=user, is_main=True)
+
+    team = factories.TeamFactory()
+    models.TeamAccess.objects.create(team=team, user=user)
+
+    # create 20 new team members
+    for _ in range(20):
+        extra_user = factories.IdentityFactory(is_main=True).user
+        factories.TeamAccessFactory(team=team, user=extra_user)
+
+    client = APIClient()
+    client.force_login(user)
+
+    response = client.get(
+        f"/api/v1.0/teams/{team.id!s}/accesses/?ordering={ordering_fields}",
+    )
+    assert response.status_code == HTTP_200_OK
     assert response.json()["count"] == 21
 
     results = [
-        team_access["role"]
+        team_access["user"][ordering_fields]
+        for team_access in response.json()["results"]
+    ]
+    assert sorted(results) == results
+
+    response = client.get(
+        f"/api/v1.0/teams/{team.id!s}/accesses/?ordering=-{ordering_fields}",
+    )
+    assert response.status_code == HTTP_200_OK
+    assert response.json()["count"] == 21
+
+    results = [
+        team_access["user"][ordering_fields]
         for team_access in response.json()["results"]
     ]
     assert sorted(results, reverse=True) == results