From 437e40228e22be2a1548e6571d61e984a660229e Mon Sep 17 00:00:00 2001 From: ozer550 Date: Thu, 20 Jun 2024 12:03:01 +0530 Subject: [PATCH 1/5] support ordering from the backend for facility users --- kolibri/core/auth/api.py | 24 ++++++++- kolibri/core/auth/test/test_api.py | 85 ++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 2 deletions(-) diff --git a/kolibri/core/auth/api.py b/kolibri/core/auth/api.py index 4bef4681f3b..6d576af5492 100644 --- a/kolibri/core/auth/api.py +++ b/kolibri/core/auth/api.py @@ -72,6 +72,7 @@ from kolibri.core import error_constants from kolibri.core.api import ReadOnlyValuesViewset from kolibri.core.api import ValuesViewset +from kolibri.core.api import ValuesViewsetOrderingFilter from kolibri.core.auth.constants.demographics import NOT_SPECIFIED from kolibri.core.auth.permissions.general import _user_is_admin_for_own_facility from kolibri.core.auth.permissions.general import DenyAll @@ -90,7 +91,6 @@ from kolibri.core.utils.pagination import ValuesViewsetPageNumberPagination from kolibri.plugins.app.utils import interface - logger = logging.getLogger(__name__) @@ -394,6 +394,7 @@ class FacilityUserViewSet(ValuesViewset): KolibriAuthPermissionsFilter, DjangoFilterBackend, filters.SearchFilter, + ValuesViewsetOrderingFilter, ) order_by_field = "username" @@ -415,6 +416,16 @@ class FacilityUserViewSet(ValuesViewset): "gender", "birth_year", "extra_demographics", + "date_joined", + ) + + ordering_fields = ( + "id", + "username", + "full_name", + "gender", + "birth_year", + "date_joined", ) field_map = { @@ -438,7 +449,16 @@ def consolidate(self, items, queryset): roles.append(role) item["roles"] = roles output.append(item) - output = sorted(output, key=lambda x: x[self.order_by_field]) + ordering_param = self.request.query_params.get( + "ordering", self.order_by_field + ) + if ordering_param.startswith("-"): + self.order_by_field = ordering_param[1:] + reverse = True + else: + self.order_by_field = ordering_param + reverse = False + output = sorted(output, key=lambda x: x[self.order_by_field], reverse=reverse) return output def perform_update(self, serializer): diff --git a/kolibri/core/auth/test/test_api.py b/kolibri/core/auth/test/test_api.py index d7562f9abaf..744e7e3eb14 100644 --- a/kolibri/core/auth/test/test_api.py +++ b/kolibri/core/auth/test/test_api.py @@ -1170,6 +1170,91 @@ def test_anonymous_no_retrieve_user(self): self.assertEqual(response.status_code, 404) +class FacilityUserOrderingTestCase(APITestCase): + @classmethod + def setUpTestData(cls): + provision_device() + cls.facility = FacilityFactory.create() + cls.superuser = create_superuser(cls.facility) + cls.facility.add_admin(cls.superuser) + cls.user1 = FacilityUserFactory.create(facility=cls.facility, username="mario") + cls.user2 = FacilityUserFactory.create(facility=cls.facility, username="luigi") + cls.user3 = FacilityUserFactory.create(facility=cls.facility, username="batman") + + def setUp(self): + self.client.login( + username=self.superuser.username, + password=DUMMY_PASSWORD, + facility=self.facility, + ) + + def _sort_by_field(self, data, field, reverse=False): + return sorted(data, key=lambda x: x[field], reverse=reverse) + + def test_default_ordering(self): + response = self.client.get(reverse("kolibri:core:facilityuser-list")) + self.assertEqual(response.status_code, 200) + data = response.data + self.assertEqual(data[0]["username"], "batman") + sorted_data = self._sort_by_field(data, "username") + self.assertEqual(data, sorted_data) + + def test_ordering_by_username(self): + response = self.client.get( + reverse("kolibri:core:facilityuser-list") + "?ordering=username" + ) + self.assertEqual(response.status_code, 200) + data = response.data + sorted_data = self._sort_by_field(data, "username") + self.assertEqual(data, sorted_data) + + def test_ordering_by_username_desc(self): + response = self.client.get( + reverse("kolibri:core:facilityuser-list") + "?ordering=-username" + ) + self.assertEqual(response.status_code, 200) + data = response.data + self.assertEqual(data[0]["username"], "superuser") + sorted_data = self._sort_by_field(data, "username", reverse=True) + self.assertEqual(data, sorted_data) + + def test_ordering_by_date_joined(self): + response = self.client.get( + reverse("kolibri:core:facilityuser-list") + "?ordering=date_joined" + ) + self.assertEqual(response.status_code, 200) + data = response.data + sorted_data = self._sort_by_field(data, "date_joined") + self.assertEqual(data, sorted_data) + + def test_ordering_by_date_joined_desc(self): + response = self.client.get( + reverse("kolibri:core:facilityuser-list") + "?ordering=-date_joined" + ) + self.assertEqual(response.status_code, 200) + data = response.data + sorted_data = self._sort_by_field(data, "date_joined", reverse=True) + self.assertEqual(data, sorted_data) + + def test_ordering_by_full_name(self): + response = self.client.get( + reverse("kolibri:core:facilityuser-list") + "?ordering=full_name" + ) + self.assertEqual(response.status_code, 200) + data = response.data + sorted_data = self._sort_by_field(data, "full_name") + self.assertEqual(data, sorted_data) + + def test_ordering_by_full_name_desc(self): + response = self.client.get( + reverse("kolibri:core:facilityuser-list") + "?ordering=-full_name" + ) + self.assertEqual(response.status_code, 200) + data = response.data + sorted_data = self._sort_by_field(data, "full_name", reverse=True) + self.assertEqual(data, sorted_data) + + class FacilityUserFilterTestCase(APITestCase): @classmethod def setUpTestData(cls): From c52ca81c71e9e715c235c2eebe9ff0218eba7523 Mon Sep 17 00:00:00 2001 From: ozer550 Date: Thu, 20 Jun 2024 12:51:50 +0530 Subject: [PATCH 2/5] fix reverse variable initialization error --- kolibri/core/auth/api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/kolibri/core/auth/api.py b/kolibri/core/auth/api.py index 6d576af5492..c66625ece7f 100644 --- a/kolibri/core/auth/api.py +++ b/kolibri/core/auth/api.py @@ -452,6 +452,7 @@ def consolidate(self, items, queryset): ordering_param = self.request.query_params.get( "ordering", self.order_by_field ) + reverse = False if ordering_param.startswith("-"): self.order_by_field = ordering_param[1:] reverse = True From c39181ba8177d101ee82f29ff6ecadef1a4669c6 Mon Sep 17 00:00:00 2001 From: ozer550 Date: Thu, 20 Jun 2024 16:50:31 +0530 Subject: [PATCH 3/5] fix reverse variable initialization error --- kolibri/core/auth/api.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/kolibri/core/auth/api.py b/kolibri/core/auth/api.py index c66625ece7f..79d544580b6 100644 --- a/kolibri/core/auth/api.py +++ b/kolibri/core/auth/api.py @@ -435,6 +435,8 @@ class FacilityUserViewSet(ValuesViewset): def consolidate(self, items, queryset): output = [] items = sorted(items, key=lambda x: x["id"]) + ordering_param = self.request.query_params.get("ordering", self.order_by_field) + reverse = False for key, group in groupby(items, lambda x: x["id"]): roles = [] for item in group: @@ -449,16 +451,11 @@ def consolidate(self, items, queryset): roles.append(role) item["roles"] = roles output.append(item) - ordering_param = self.request.query_params.get( - "ordering", self.order_by_field - ) - reverse = False if ordering_param.startswith("-"): self.order_by_field = ordering_param[1:] reverse = True else: self.order_by_field = ordering_param - reverse = False output = sorted(output, key=lambda x: x[self.order_by_field], reverse=reverse) return output From 776d9f4fa5344391390e4280b8be213d5943341f Mon Sep 17 00:00:00 2001 From: ozer550 Date: Thu, 20 Jun 2024 17:31:47 +0530 Subject: [PATCH 4/5] fix failing tests --- kolibri/core/auth/test/test_api.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kolibri/core/auth/test/test_api.py b/kolibri/core/auth/test/test_api.py index 744e7e3eb14..40fc43803e3 100644 --- a/kolibri/core/auth/test/test_api.py +++ b/kolibri/core/auth/test/test_api.py @@ -1083,6 +1083,7 @@ def test_user_list(self): "id_number": self.user.id_number, "gender": self.user.gender, "birth_year": self.user.birth_year, + "date_joined": self.user.date_joined, "is_superuser": False, "roles": [], "extra_demographics": None, @@ -1094,6 +1095,7 @@ def test_user_list(self): "facility": self.superuser.facility_id, "id_number": self.superuser.id_number, "gender": self.superuser.gender, + "date_joined": self.superuser.date_joined, "birth_year": self.superuser.birth_year, "is_superuser": True, "roles": [ @@ -1127,6 +1129,7 @@ def test_user_list_self(self): "id_number": self.user.id_number, "gender": self.user.gender, "birth_year": self.user.birth_year, + "date_joined": self.user.date_joined, "is_superuser": False, "roles": [], "extra_demographics": None, From 3b95738c20ffb2f42ae1aa964f4fb6cb0420817d Mon Sep 17 00:00:00 2001 From: ozer550 Date: Fri, 26 Jul 2024 16:53:52 +0530 Subject: [PATCH 5/5] Add reviewed changes --- kolibri/core/auth/api.py | 7 +++---- kolibri/core/auth/test/test_api.py | 19 ++++++++++++++++--- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/kolibri/core/auth/api.py b/kolibri/core/auth/api.py index 79d544580b6..4eeb714d42b 100644 --- a/kolibri/core/auth/api.py +++ b/kolibri/core/auth/api.py @@ -452,11 +452,10 @@ def consolidate(self, items, queryset): item["roles"] = roles output.append(item) if ordering_param.startswith("-"): - self.order_by_field = ordering_param[1:] + ordering_param = ordering_param[1:] reverse = True - else: - self.order_by_field = ordering_param - output = sorted(output, key=lambda x: x[self.order_by_field], reverse=reverse) + + output = sorted(output, key=lambda x: x[ordering_param], reverse=reverse) return output def perform_update(self, serializer): diff --git a/kolibri/core/auth/test/test_api.py b/kolibri/core/auth/test/test_api.py index 40fc43803e3..1c77e3d423d 100644 --- a/kolibri/core/auth/test/test_api.py +++ b/kolibri/core/auth/test/test_api.py @@ -3,6 +3,7 @@ import time import uuid from datetime import datetime +from datetime import timedelta from importlib import import_module import factory @@ -1180,9 +1181,21 @@ def setUpTestData(cls): cls.facility = FacilityFactory.create() cls.superuser = create_superuser(cls.facility) cls.facility.add_admin(cls.superuser) - cls.user1 = FacilityUserFactory.create(facility=cls.facility, username="mario") - cls.user2 = FacilityUserFactory.create(facility=cls.facility, username="luigi") - cls.user3 = FacilityUserFactory.create(facility=cls.facility, username="batman") + + base_time = datetime.now() - timedelta(days=3) + cls.user1 = FacilityUserFactory.create( + facility=cls.facility, username="mario", date_joined=base_time + ) + cls.user2 = FacilityUserFactory.create( + facility=cls.facility, + username="luigi", + date_joined=base_time + timedelta(days=1), + ) + cls.user3 = FacilityUserFactory.create( + facility=cls.facility, + username="batman", + date_joined=base_time + timedelta(days=4), + ) def setUp(self): self.client.login(