From 3f4543cf6ca0291efe7202979726a20c66688578 Mon Sep 17 00:00:00 2001 From: Benjamin Monjoie Date: Wed, 28 Aug 2024 11:10:12 +0200 Subject: [PATCH 1/3] Allow the mobile app to download all groups This adds a query parameter `showDeleted` that allows the mobile app to download groups from all source_version. IA-3379 --- iaso/api/mobile/groups.py | 63 ++++++++++++++++++++--- iaso/tests/api/test_mobile_groups.py | 77 ++++++++++++++++++++++------ 2 files changed, 116 insertions(+), 24 deletions(-) diff --git a/iaso/api/mobile/groups.py b/iaso/api/mobile/groups.py index dfb4d541f9..3f5ddd1238 100644 --- a/iaso/api/mobile/groups.py +++ b/iaso/api/mobile/groups.py @@ -1,3 +1,5 @@ +from typing import Dict, Any + from django.db.models.query import QuerySet from drf_yasg import openapi from drf_yasg.utils import swagger_auto_schema @@ -8,17 +10,34 @@ from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet -from iaso.api.query_params import APP_ID +from iaso.api.query_params import APP_ID, SHOW_DELETED from iaso.api.serializers import AppIdSerializer -from iaso.models import Group, Project +from iaso.models import Group, Project, SourceVersion class MobileGroupSerializer(serializers.ModelSerializer): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.source_version = kwargs["context"].get("source_version") + + erased = serializers.SerializerMethodField() + + def get_erased(self, obj: Group): + return obj.source_version.id != self.source_version + + @staticmethod + def include_source_version( + context: Dict[str, Any], source_version: SourceVersion + ) -> Dict[str, Any]: + context["source_version"] = source_version.id + return context + class Meta: model = Group fields = [ "id", "name", + "erased", ] @@ -43,19 +62,49 @@ class MobileGroupsViewSet(ListModelMixin, GenericViewSet): type=openapi.TYPE_STRING, ) + show_deleted_param = openapi.Parameter( + name=SHOW_DELETED, + in_=openapi.IN_QUERY, + required=False, + description="Include deleted groups", + type=openapi.TYPE_BOOLEAN, + default=False, + ) + @swagger_auto_schema( responses={ 200: f"List of groups for the given '{APP_ID}'.", 400: f"Parameter '{APP_ID}' is required.", 404: f"Project for given '{APP_ID}' doesn't exist.", }, - manual_parameters=[app_id_param], + manual_parameters=[app_id_param, show_deleted_param], ) def list(self, request: Request, *args, **kwargs) -> Response: return super().list(request, *args, **kwargs) + def get_serializer_context(self) -> Dict[str, Any]: + context = super().get_serializer_context() + return self.serializer_class.include_source_version( + context, self.get_project().account.default_version + ) + + def get_project(self): + app_id = AppIdSerializer(data=self.request.query_params).get_app_id( + raise_exception=True + ) + project_qs = Project.objects.select_related( + "account__default_version__data_source" + ) + return get_object_or_404(project_qs, app_id=app_id) + def get_queryset(self) -> QuerySet: - app_id = AppIdSerializer(data=self.request.query_params).get_app_id(raise_exception=True) - project_qs = Project.objects.select_related("account__default_version") - project = get_object_or_404(project_qs, app_id=app_id) - return Group.objects.filter(source_version=project.account.default_version) + objects = Group.objects + if self.request.query_params.get(SHOW_DELETED) != "true": + objects = objects.filter( + source_version=self.get_project().account.default_version + ) + else: + objects = objects.filter( + source_version__data_source=self.get_project().account.default_version.data_source + ) + return objects diff --git a/iaso/tests/api/test_mobile_groups.py b/iaso/tests/api/test_mobile_groups.py index b42daf4c37..9551f4db8d 100644 --- a/iaso/tests/api/test_mobile_groups.py +++ b/iaso/tests/api/test_mobile_groups.py @@ -1,7 +1,7 @@ from django.utils.timezone import now from iaso import models as m -from iaso.api.query_params import APP_ID +from iaso.api.query_params import APP_ID, SHOW_DELETED from iaso.test import APITestCase @@ -11,29 +11,51 @@ def setUpTestData(cls): cls.now = now() cls.data_source = m.DataSource.objects.create(name="Default source") - cls.source_version_1 = m.SourceVersion.objects.create(data_source=cls.data_source, number=1) - cls.source_version_2 = m.SourceVersion.objects.create(data_source=cls.data_source, number=2) + cls.source_version_1 = m.SourceVersion.objects.create( + data_source=cls.data_source, number=1 + ) + cls.source_version_2 = m.SourceVersion.objects.create( + data_source=cls.data_source, number=2 + ) - account_nigeria = m.Account.objects.create(name="Nigeria", default_version=cls.source_version_2) - account_cameroon = m.Account.objects.create(name="Cameroon", default_version=cls.source_version_1) + account_nigeria = m.Account.objects.create( + name="Nigeria", default_version=cls.source_version_2 + ) + account_cameroon = m.Account.objects.create( + name="Cameroon", default_version=cls.source_version_1 + ) cls.user_nigeria = cls.create_user_with_profile( - username="user_nigeria", account=account_nigeria, permissions=["iaso_org_units"] + username="user_nigeria", + account=account_nigeria, + permissions=["iaso_org_units"], ) cls.user_cameroon = cls.create_user_with_profile( - username="user_cameroon", account=account_cameroon, permissions=["iaso_org_units"] + username="user_cameroon", + account=account_cameroon, + permissions=["iaso_org_units"], ) cls.project_nigeria = m.Project.objects.create( - name="Nigeria health pyramid", app_id="nigeria.health.pyramid", account=account_nigeria + name="Nigeria health pyramid", + app_id="nigeria.health.pyramid", + account=account_nigeria, ) cls.project_cameroon = m.Project.objects.create( - name="Cameroon health map", app_id="cameroon.health.map", account=account_cameroon + name="Cameroon health map", + app_id="cameroon.health.map", + account=account_cameroon, ) - cls.group_nigeria_1 = m.Group.objects.create(name="Hospitals", source_version=cls.source_version_1) - cls.group_nigeria_2 = m.Group.objects.create(name="Villages", source_version=cls.source_version_2) - cls.group_cameroon = m.Group.objects.create(name="North", source_version=cls.source_version_1) + cls.group_nigeria_1 = m.Group.objects.create( + name="Hospitals", source_version=cls.source_version_1 + ) + cls.group_nigeria_2 = m.Group.objects.create( + name="Villages", source_version=cls.source_version_2 + ) + cls.group_cameroon = m.Group.objects.create( + name="North", source_version=cls.source_version_1 + ) def test_api_mobile_groups_list_without_app_id(self): """GET /api/mobile/groups/ without app_id""" @@ -49,18 +71,39 @@ def test_api_mobile_groups_list_with_app_id(self): """GET /api/mobile/groups/ with app_id""" # Groups with `source_version_1`. - response = self.client.get("/api/mobile/groups/", {APP_ID: self.project_cameroon.app_id}) + response = self.client.get( + "/api/mobile/groups/", {APP_ID: self.project_cameroon.app_id} + ) self.assertJSONResponse(response, 200) self.assertEqual(len(response.data), 2) expected_data = [ - {"id": self.group_nigeria_1.pk, "name": "Hospitals"}, - {"id": self.group_cameroon.pk, "name": "North"}, + {"id": self.group_nigeria_1.pk, "name": "Hospitals", "erased": False}, + {"id": self.group_cameroon.pk, "name": "North", "erased": False}, ] self.assertCountEqual(response.data, expected_data) # Groups with `source_version_2`. - response = self.client.get("/api/mobile/groups/", {APP_ID: self.project_nigeria.app_id}) + ## Without all versions + response = self.client.get( + "/api/mobile/groups/", {APP_ID: self.project_nigeria.app_id} + ) self.assertJSONResponse(response, 200) self.assertEqual(len(response.data), 1) - expected_data = [{"id": self.group_nigeria_2.pk, "name": "Villages"}] + expected_data = [ + {"id": self.group_nigeria_2.pk, "name": "Villages", "erased": False} + ] + self.assertCountEqual(response.data, expected_data) + + ## With all versions + response = self.client.get( + "/api/mobile/groups/", + {APP_ID: self.project_nigeria.app_id, SHOW_DELETED: "true"}, + ) + self.assertJSONResponse(response, 200) + self.assertEqual(len(response.data), 3) + expected_data = [ + {"id": self.group_nigeria_1.pk, "name": "Hospitals", "erased": True}, + {"id": self.group_cameroon.pk, "name": "North", "erased": True}, + {"id": self.group_nigeria_2.pk, "name": "Villages", "erased": False}, + ] self.assertCountEqual(response.data, expected_data) From 3947f8812bafb2ee01c1e1e8e3a089ef282eb1d8 Mon Sep 17 00:00:00 2001 From: Benjamin Monjoie Date: Wed, 28 Aug 2024 11:54:05 +0200 Subject: [PATCH 2/3] Use ./venv/black instead of the one in docker... --- iaso/api/mobile/groups.py | 24 +++++------------ iaso/tests/api/test_mobile_groups.py | 40 +++++++--------------------- 2 files changed, 16 insertions(+), 48 deletions(-) diff --git a/iaso/api/mobile/groups.py b/iaso/api/mobile/groups.py index 3f5ddd1238..ef6c79b299 100644 --- a/iaso/api/mobile/groups.py +++ b/iaso/api/mobile/groups.py @@ -26,9 +26,7 @@ def get_erased(self, obj: Group): return obj.source_version.id != self.source_version @staticmethod - def include_source_version( - context: Dict[str, Any], source_version: SourceVersion - ) -> Dict[str, Any]: + def include_source_version(context: Dict[str, Any], source_version: SourceVersion) -> Dict[str, Any]: context["source_version"] = source_version.id return context @@ -84,27 +82,17 @@ def list(self, request: Request, *args, **kwargs) -> Response: def get_serializer_context(self) -> Dict[str, Any]: context = super().get_serializer_context() - return self.serializer_class.include_source_version( - context, self.get_project().account.default_version - ) + return self.serializer_class.include_source_version(context, self.get_project().account.default_version) def get_project(self): - app_id = AppIdSerializer(data=self.request.query_params).get_app_id( - raise_exception=True - ) - project_qs = Project.objects.select_related( - "account__default_version__data_source" - ) + app_id = AppIdSerializer(data=self.request.query_params).get_app_id(raise_exception=True) + project_qs = Project.objects.select_related("account__default_version__data_source") return get_object_or_404(project_qs, app_id=app_id) def get_queryset(self) -> QuerySet: objects = Group.objects if self.request.query_params.get(SHOW_DELETED) != "true": - objects = objects.filter( - source_version=self.get_project().account.default_version - ) + objects = objects.filter(source_version=self.get_project().account.default_version) else: - objects = objects.filter( - source_version__data_source=self.get_project().account.default_version.data_source - ) + objects = objects.filter(source_version__data_source=self.get_project().account.default_version.data_source) return objects diff --git a/iaso/tests/api/test_mobile_groups.py b/iaso/tests/api/test_mobile_groups.py index 9551f4db8d..0e1dcf4267 100644 --- a/iaso/tests/api/test_mobile_groups.py +++ b/iaso/tests/api/test_mobile_groups.py @@ -11,19 +11,11 @@ def setUpTestData(cls): cls.now = now() cls.data_source = m.DataSource.objects.create(name="Default source") - cls.source_version_1 = m.SourceVersion.objects.create( - data_source=cls.data_source, number=1 - ) - cls.source_version_2 = m.SourceVersion.objects.create( - data_source=cls.data_source, number=2 - ) + cls.source_version_1 = m.SourceVersion.objects.create(data_source=cls.data_source, number=1) + cls.source_version_2 = m.SourceVersion.objects.create(data_source=cls.data_source, number=2) - account_nigeria = m.Account.objects.create( - name="Nigeria", default_version=cls.source_version_2 - ) - account_cameroon = m.Account.objects.create( - name="Cameroon", default_version=cls.source_version_1 - ) + account_nigeria = m.Account.objects.create(name="Nigeria", default_version=cls.source_version_2) + account_cameroon = m.Account.objects.create(name="Cameroon", default_version=cls.source_version_1) cls.user_nigeria = cls.create_user_with_profile( username="user_nigeria", @@ -47,15 +39,9 @@ def setUpTestData(cls): account=account_cameroon, ) - cls.group_nigeria_1 = m.Group.objects.create( - name="Hospitals", source_version=cls.source_version_1 - ) - cls.group_nigeria_2 = m.Group.objects.create( - name="Villages", source_version=cls.source_version_2 - ) - cls.group_cameroon = m.Group.objects.create( - name="North", source_version=cls.source_version_1 - ) + cls.group_nigeria_1 = m.Group.objects.create(name="Hospitals", source_version=cls.source_version_1) + cls.group_nigeria_2 = m.Group.objects.create(name="Villages", source_version=cls.source_version_2) + cls.group_cameroon = m.Group.objects.create(name="North", source_version=cls.source_version_1) def test_api_mobile_groups_list_without_app_id(self): """GET /api/mobile/groups/ without app_id""" @@ -71,9 +57,7 @@ def test_api_mobile_groups_list_with_app_id(self): """GET /api/mobile/groups/ with app_id""" # Groups with `source_version_1`. - response = self.client.get( - "/api/mobile/groups/", {APP_ID: self.project_cameroon.app_id} - ) + response = self.client.get("/api/mobile/groups/", {APP_ID: self.project_cameroon.app_id}) self.assertJSONResponse(response, 200) self.assertEqual(len(response.data), 2) expected_data = [ @@ -84,14 +68,10 @@ def test_api_mobile_groups_list_with_app_id(self): # Groups with `source_version_2`. ## Without all versions - response = self.client.get( - "/api/mobile/groups/", {APP_ID: self.project_nigeria.app_id} - ) + response = self.client.get("/api/mobile/groups/", {APP_ID: self.project_nigeria.app_id}) self.assertJSONResponse(response, 200) self.assertEqual(len(response.data), 1) - expected_data = [ - {"id": self.group_nigeria_2.pk, "name": "Villages", "erased": False} - ] + expected_data = [{"id": self.group_nigeria_2.pk, "name": "Villages", "erased": False}] self.assertCountEqual(response.data, expected_data) ## With all versions From d3adab0ed74372087a984a51f068c8d46390166d Mon Sep 17 00:00:00 2001 From: kemar Date: Wed, 28 Aug 2024 16:00:33 +0200 Subject: [PATCH 3/3] Use `annotate` to avoid relying on `get_serializer_context` --- iaso/api/mobile/groups.py | 43 ++++++++++----------------------------- 1 file changed, 11 insertions(+), 32 deletions(-) diff --git a/iaso/api/mobile/groups.py b/iaso/api/mobile/groups.py index 3f5ddd1238..197aa0b5eb 100644 --- a/iaso/api/mobile/groups.py +++ b/iaso/api/mobile/groups.py @@ -1,5 +1,4 @@ -from typing import Dict, Any - +from django.db.models import Q, ExpressionWrapper, BooleanField, Value from django.db.models.query import QuerySet from drf_yasg import openapi from drf_yasg.utils import swagger_auto_schema @@ -12,25 +11,11 @@ from iaso.api.query_params import APP_ID, SHOW_DELETED from iaso.api.serializers import AppIdSerializer -from iaso.models import Group, Project, SourceVersion +from iaso.models import Group, Project class MobileGroupSerializer(serializers.ModelSerializer): - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.source_version = kwargs["context"].get("source_version") - - erased = serializers.SerializerMethodField() - - def get_erased(self, obj: Group): - return obj.source_version.id != self.source_version - - @staticmethod - def include_source_version( - context: Dict[str, Any], source_version: SourceVersion - ) -> Dict[str, Any]: - context["source_version"] = source_version.id - return context + erased = serializers.BooleanField(read_only=True, source="annotated_erased") class Meta: model = Group @@ -82,29 +67,23 @@ class MobileGroupsViewSet(ListModelMixin, GenericViewSet): def list(self, request: Request, *args, **kwargs) -> Response: return super().list(request, *args, **kwargs) - def get_serializer_context(self) -> Dict[str, Any]: - context = super().get_serializer_context() - return self.serializer_class.include_source_version( - context, self.get_project().account.default_version - ) - def get_project(self): - app_id = AppIdSerializer(data=self.request.query_params).get_app_id( - raise_exception=True - ) - project_qs = Project.objects.select_related( - "account__default_version__data_source" - ) + app_id = AppIdSerializer(data=self.request.query_params).get_app_id(raise_exception=True) + project_qs = Project.objects.select_related("account__default_version__data_source") return get_object_or_404(project_qs, app_id=app_id) def get_queryset(self) -> QuerySet: objects = Group.objects if self.request.query_params.get(SHOW_DELETED) != "true": - objects = objects.filter( - source_version=self.get_project().account.default_version + objects = objects.filter(source_version=self.get_project().account.default_version).annotate( + annotated_erased=Value(False, output_field=BooleanField()) ) else: objects = objects.filter( source_version__data_source=self.get_project().account.default_version.data_source + ).annotate( + annotated_erased=ExpressionWrapper( + ~Q(source_version=self.get_project().account.default_version), output_field=BooleanField() + ) ) return objects