From 88756a96384bfbb34780524e3b4be8cc4be1641d Mon Sep 17 00:00:00 2001 From: kemar Date: Tue, 23 Apr 2024 16:14:07 +0200 Subject: [PATCH 01/17] Add a new `/api/orgunits/tree/` route --- iaso/api/org_unit_tree/__init__.py | 0 iaso/api/org_unit_tree/filters.py | 53 +++++++++++++++++++++++++++ iaso/api/org_unit_tree/pagination.py | 5 +++ iaso/api/org_unit_tree/serializers.py | 37 +++++++++++++++++++ iaso/api/org_unit_tree/views.py | 24 ++++++++++++ iaso/urls.py | 2 + 6 files changed, 121 insertions(+) create mode 100644 iaso/api/org_unit_tree/__init__.py create mode 100644 iaso/api/org_unit_tree/filters.py create mode 100644 iaso/api/org_unit_tree/pagination.py create mode 100644 iaso/api/org_unit_tree/serializers.py create mode 100644 iaso/api/org_unit_tree/views.py diff --git a/iaso/api/org_unit_tree/__init__.py b/iaso/api/org_unit_tree/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/iaso/api/org_unit_tree/filters.py b/iaso/api/org_unit_tree/filters.py new file mode 100644 index 0000000000..f083ad59d1 --- /dev/null +++ b/iaso/api/org_unit_tree/filters.py @@ -0,0 +1,53 @@ +from django.db.models.query import QuerySet +from django.utils.translation import gettext_lazy as _ + +import django_filters +from rest_framework.exceptions import ValidationError + +from iaso.models import OrgUnit, DataSource + + +class OrgUnitTreeFilter(django_filters.rest_framework.FilterSet): + default_version = django_filters.BooleanFilter(method="filter_default_version", label=_("Default version")) + ignore_empty_names = django_filters.BooleanFilter(method="filter_empty_names", label=_("Ignore empty names")) + parent_id = django_filters.NumberFilter(method="filter_parent_id", label=_("Parent ID")) + roots_for_user = django_filters.BooleanFilter(method="filter_roots_for_user", label=_("Roots for user")) + source_id = django_filters.NumberFilter(method="filter_source_id", label=_("Source ID")) + + class Meta: + model = OrgUnit + fields = ["validation_status", "version"] + + def filter_default_version(self, queryset: QuerySet, _, use_default_version: bool) -> QuerySet: + if not use_default_version or self.request.user.is_anonymous: + return queryset + return queryset.filter(version=self.request.user.iaso_profile.account.default_version) + + def filter_empty_names(self, queryset: QuerySet, _, use_empty_names: bool) -> QuerySet: + if use_empty_names: + return queryset.exclude(name="") + return queryset + + def filter_parent_id(self, queryset: QuerySet, _, parent_id: int) -> QuerySet: + try: + parent = OrgUnit.objects.get(id=parent_id) + except OrgUnit.DoesNotExist: + raise ValidationError({"parent_id": [f"OrgUnit with id {parent_id} does not exist."]}) + return queryset.filter(parent=parent.pk) + + def filter_roots_for_user(self, queryset: QuerySet, _, use_roots_for_user: bool) -> QuerySet: + if not use_roots_for_user or self.request.user.is_anonymous: + return queryset + org_unit_ids_for_profile = self.request.user.iaso_profile.org_units.only("id") + if org_unit_ids_for_profile and not self.request.user.is_superuser: + return queryset.filter(id__in=org_unit_ids_for_profile) + return queryset.filter(parent__isnull=True) + + def filter_source_id(self, queryset: QuerySet, _, source_id: int) -> QuerySet: + try: + source = DataSource.objects.get(id=source_id) + except OrgUnit.DoesNotExist: + raise ValidationError({"source_id": [f"DataSource with id {source_id} does not exist."]}) + if source.default_version: + return queryset.filter(version=source.default_version) + return queryset.filter(version__data_source_id=source_id) diff --git a/iaso/api/org_unit_tree/pagination.py b/iaso/api/org_unit_tree/pagination.py new file mode 100644 index 0000000000..708e0936f7 --- /dev/null +++ b/iaso/api/org_unit_tree/pagination.py @@ -0,0 +1,5 @@ +from iaso.api.common import Paginator + + +class OrgUnitTreePagination(Paginator): + page_size = 10 diff --git a/iaso/api/org_unit_tree/serializers.py b/iaso/api/org_unit_tree/serializers.py new file mode 100644 index 0000000000..3b2f6db3f3 --- /dev/null +++ b/iaso/api/org_unit_tree/serializers.py @@ -0,0 +1,37 @@ +from typing import Union + +from rest_framework import serializers + +from iaso.api.common import DynamicFieldsModelSerializer +from iaso.models import OrgUnit + + +class OrgUnitTreeSerializer(DynamicFieldsModelSerializer, serializers.ModelSerializer): + has_children = serializers.SerializerMethodField() + org_unit_type_short_name = serializers.SerializerMethodField() + + @classmethod + def get_has_children(cls, org_unit: OrgUnit) -> bool: + return org_unit.children_count > 0 + + def get_org_unit_type_short_name(self, org_unit: OrgUnit) -> Union[str, None]: + return org_unit.org_unit_type.short_name if org_unit.org_unit_type else None + + class Meta: + model = OrgUnit + fields = [ + "id", + "name", + "validation_status", + "has_children", + "org_unit_type_id", + "org_unit_type_short_name", + ] + default_fields = [ + "id", + "name", + "validation_status", + "has_children", + "org_unit_type_id", + "org_unit_type_short_name", + ] diff --git a/iaso/api/org_unit_tree/views.py b/iaso/api/org_unit_tree/views.py new file mode 100644 index 0000000000..088140e217 --- /dev/null +++ b/iaso/api/org_unit_tree/views.py @@ -0,0 +1,24 @@ +import django_filters +from django.db.models import Count + +from rest_framework import filters, viewsets + +from iaso.api.org_unit_tree.filters import OrgUnitTreeFilter +from iaso.api.org_unit_tree.pagination import OrgUnitTreePagination +from iaso.api.org_unit_tree.serializers import OrgUnitTreeSerializer +from iaso.models import OrgUnit + + +class OrgUnitTreeViewSet(viewsets.ModelViewSet): + filter_backends = [filters.OrderingFilter, django_filters.rest_framework.DjangoFilterBackend] + filterset_class = OrgUnitTreeFilter + http_method_names = ["get", "options", "head", "trace"] + ordering_fields = ["id", "name"] + pagination_class = OrgUnitTreePagination + serializer_class = OrgUnitTreeSerializer + + def get_queryset(self): + qs = OrgUnit.objects.filter_for_user(self.request.user) + qs = qs.only("id", "name", "validation_status", "version", "org_unit_type", "parent") + qs = qs.annotate(children_count=Count("orgunit__id")) + return qs diff --git a/iaso/urls.py b/iaso/urls.py index 732b10a879..baeec2d778 100644 --- a/iaso/urls.py +++ b/iaso/urls.py @@ -68,6 +68,7 @@ from .api.mobile.storage import MobileStoragePasswordViewSet from .api.org_unit_change_requests.views import OrgUnitChangeRequestViewSet from .api.org_unit_change_requests.views_mobile import MobileOrgUnitChangeRequestViewSet +from .api.org_unit_tree.views import OrgUnitTreeViewSet from .api.org_unit_types import OrgUnitTypeViewSet from .api.org_unit_types.viewsets import OrgUnitTypeViewSetV2 from .api.org_units import OrgUnitViewSet @@ -101,6 +102,7 @@ router = routers.DefaultRouter() router.register(r"orgunits/changes", OrgUnitChangeRequestViewSet, basename="orgunitschanges") router.register(r"mobile/orgunits/changes", MobileOrgUnitChangeRequestViewSet, basename="mobileorgunitschanges") +router.register(r"orgunits/tree", OrgUnitTreeViewSet, basename="orgunitstree") router.register(r"orgunits", OrgUnitViewSet, basename="orgunits") router.register(r"orgunittypes", OrgUnitTypeViewSet, basename="orgunittypes") router.register(r"v2/orgunittypes", OrgUnitTypeViewSetV2, basename="orgunittypes") From 2184ce7b87d702babda13cca7625e53c5caa3a07 Mon Sep 17 00:00:00 2001 From: kemar Date: Thu, 25 Apr 2024 12:05:57 +0200 Subject: [PATCH 02/17] Override `validation_status` --- iaso/api/org_unit_tree/filters.py | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/iaso/api/org_unit_tree/filters.py b/iaso/api/org_unit_tree/filters.py index f083ad59d1..64e38a51b5 100644 --- a/iaso/api/org_unit_tree/filters.py +++ b/iaso/api/org_unit_tree/filters.py @@ -13,20 +13,22 @@ class OrgUnitTreeFilter(django_filters.rest_framework.FilterSet): parent_id = django_filters.NumberFilter(method="filter_parent_id", label=_("Parent ID")) roots_for_user = django_filters.BooleanFilter(method="filter_roots_for_user", label=_("Roots for user")) source_id = django_filters.NumberFilter(method="filter_source_id", label=_("Source ID")) + validation_status = django_filters.ChoiceFilter( + method="filter_validation_status", label=_("Validation status"), choices=OrgUnit.VALIDATION_STATUS_CHOICES + ) class Meta: model = OrgUnit - fields = ["validation_status", "version"] + fields = ["version"] def filter_default_version(self, queryset: QuerySet, _, use_default_version: bool) -> QuerySet: - if not use_default_version or self.request.user.is_anonymous: + user = self.request.user + if user.is_anonymous or not use_default_version: return queryset - return queryset.filter(version=self.request.user.iaso_profile.account.default_version) + return queryset.filter(version=user.iaso_profile.account.default_version) def filter_empty_names(self, queryset: QuerySet, _, use_empty_names: bool) -> QuerySet: - if use_empty_names: - return queryset.exclude(name="") - return queryset + return queryset.exclude(name="") if use_empty_names else queryset def filter_parent_id(self, queryset: QuerySet, _, parent_id: int) -> QuerySet: try: @@ -36,10 +38,11 @@ def filter_parent_id(self, queryset: QuerySet, _, parent_id: int) -> QuerySet: return queryset.filter(parent=parent.pk) def filter_roots_for_user(self, queryset: QuerySet, _, use_roots_for_user: bool) -> QuerySet: - if not use_roots_for_user or self.request.user.is_anonymous: + user = self.request.user + if user.is_anonymous or not use_roots_for_user: return queryset - org_unit_ids_for_profile = self.request.user.iaso_profile.org_units.only("id") - if org_unit_ids_for_profile and not self.request.user.is_superuser: + org_unit_ids_for_profile = user.iaso_profile.org_units.only("id") + if org_unit_ids_for_profile and not user.is_superuser: return queryset.filter(id__in=org_unit_ids_for_profile) return queryset.filter(parent__isnull=True) @@ -51,3 +54,6 @@ def filter_source_id(self, queryset: QuerySet, _, source_id: int) -> QuerySet: if source.default_version: return queryset.filter(version=source.default_version) return queryset.filter(version__data_source_id=source_id) + + def filter_validation_status(self, queryset: QuerySet, _, validation_status: str) -> QuerySet: + return queryset From e9d74acc65b68e628cb082964c3528a71e2ecdd7 Mon Sep 17 00:00:00 2001 From: kemar Date: Mon, 29 Apr 2024 09:56:33 +0200 Subject: [PATCH 03/17] Fetch children of `REJECTED` org units which are not themselves `REJECTED` --- iaso/api/org_unit_tree/filters.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/iaso/api/org_unit_tree/filters.py b/iaso/api/org_unit_tree/filters.py index 64e38a51b5..6b00fe4157 100644 --- a/iaso/api/org_unit_tree/filters.py +++ b/iaso/api/org_unit_tree/filters.py @@ -1,7 +1,7 @@ +import django_filters from django.db.models.query import QuerySet from django.utils.translation import gettext_lazy as _ -import django_filters from rest_framework.exceptions import ValidationError from iaso.models import OrgUnit, DataSource @@ -56,4 +56,22 @@ def filter_source_id(self, queryset: QuerySet, _, source_id: int) -> QuerySet: return queryset.filter(version__data_source_id=source_id) def filter_validation_status(self, queryset: QuerySet, _, validation_status: str) -> QuerySet: - return queryset + if validation_status == OrgUnit.VALIDATION_VALID: + # Fetch children of `REJECTED` org units which are not themselves `REJECTED`. + children_of_rejected_org_units_sql = """ +WITH RECURSIVE rejected AS ( + SELECT id, validation_status + FROM iaso_orgunit + WHERE validation_status = 'REJECTED' + UNION + SELECT descendant.id, descendant.validation_status + FROM iaso_orgunit as descendant, rejected + WHERE descendant.parent_id = rejected.id +) +SELECT 1 as id, array_agg(id) as ids +FROM rejected +WHERE validation_status != 'REJECTED';""" + ids_to_exclude = OrgUnit.objects.raw(children_of_rejected_org_units_sql)[0].ids + queryset = queryset.exclude(id__in=ids_to_exclude) + + return queryset.filter(validation_status=validation_status) From 9d4015cae16eb315dc047718d13d0ceacd4f4e69 Mon Sep 17 00:00:00 2001 From: kemar Date: Tue, 30 Apr 2024 17:52:47 +0200 Subject: [PATCH 04/17] Improve query --- iaso/api/org_unit_tree/filters.py | 86 +++++++++++++++++++------------ iaso/api/org_unit_tree/views.py | 10 ++-- 2 files changed, 58 insertions(+), 38 deletions(-) diff --git a/iaso/api/org_unit_tree/filters.py b/iaso/api/org_unit_tree/filters.py index 6b00fe4157..0526b5b5e1 100644 --- a/iaso/api/org_unit_tree/filters.py +++ b/iaso/api/org_unit_tree/filters.py @@ -1,5 +1,6 @@ import django_filters from django.db.models.query import QuerySet +from django.shortcuts import get_object_or_404 from django.utils.translation import gettext_lazy as _ from rest_framework.exceptions import ValidationError @@ -11,7 +12,6 @@ class OrgUnitTreeFilter(django_filters.rest_framework.FilterSet): default_version = django_filters.BooleanFilter(method="filter_default_version", label=_("Default version")) ignore_empty_names = django_filters.BooleanFilter(method="filter_empty_names", label=_("Ignore empty names")) parent_id = django_filters.NumberFilter(method="filter_parent_id", label=_("Parent ID")) - roots_for_user = django_filters.BooleanFilter(method="filter_roots_for_user", label=_("Roots for user")) source_id = django_filters.NumberFilter(method="filter_source_id", label=_("Source ID")) validation_status = django_filters.ChoiceFilter( method="filter_validation_status", label=_("Validation status"), choices=OrgUnit.VALIDATION_STATUS_CHOICES @@ -21,30 +21,26 @@ class Meta: model = OrgUnit fields = ["version"] + @property + def qs(self): + parent_qs = super().qs + # If `parent_id` is not provided, return the tree's first level. + parent_id = self.form.cleaned_data["parent_id"] + if not parent_id: + return parent_qs.filter(parent__isnull=True) + return parent_qs + def filter_default_version(self, queryset: QuerySet, _, use_default_version: bool) -> QuerySet: user = self.request.user - if user.is_anonymous or not use_default_version: + if not use_default_version or user.is_anonymous: return queryset return queryset.filter(version=user.iaso_profile.account.default_version) def filter_empty_names(self, queryset: QuerySet, _, use_empty_names: bool) -> QuerySet: return queryset.exclude(name="") if use_empty_names else queryset - def filter_parent_id(self, queryset: QuerySet, _, parent_id: int) -> QuerySet: - try: - parent = OrgUnit.objects.get(id=parent_id) - except OrgUnit.DoesNotExist: - raise ValidationError({"parent_id": [f"OrgUnit with id {parent_id} does not exist."]}) - return queryset.filter(parent=parent.pk) - - def filter_roots_for_user(self, queryset: QuerySet, _, use_roots_for_user: bool) -> QuerySet: - user = self.request.user - if user.is_anonymous or not use_roots_for_user: - return queryset - org_unit_ids_for_profile = user.iaso_profile.org_units.only("id") - if org_unit_ids_for_profile and not user.is_superuser: - return queryset.filter(id__in=org_unit_ids_for_profile) - return queryset.filter(parent__isnull=True) + def filter_parent_id(self, queryset: QuerySet, _, parent_id: str) -> QuerySet: + return queryset.filter(parent=parent_id) def filter_source_id(self, queryset: QuerySet, _, source_id: int) -> QuerySet: try: @@ -56,22 +52,44 @@ def filter_source_id(self, queryset: QuerySet, _, source_id: int) -> QuerySet: return queryset.filter(version__data_source_id=source_id) def filter_validation_status(self, queryset: QuerySet, _, validation_status: str) -> QuerySet: - if validation_status == OrgUnit.VALIDATION_VALID: - # Fetch children of `REJECTED` org units which are not themselves `REJECTED`. - children_of_rejected_org_units_sql = """ -WITH RECURSIVE rejected AS ( - SELECT id, validation_status - FROM iaso_orgunit - WHERE validation_status = 'REJECTED' - UNION - SELECT descendant.id, descendant.validation_status - FROM iaso_orgunit as descendant, rejected - WHERE descendant.parent_id = rejected.id -) -SELECT 1 as id, array_agg(id) as ids -FROM rejected -WHERE validation_status != 'REJECTED';""" - ids_to_exclude = OrgUnit.objects.raw(children_of_rejected_org_units_sql)[0].ids - queryset = queryset.exclude(id__in=ids_to_exclude) + """ + If we filter on VALID or NEW, the children of a REJECTED org unit should not be returned. + """ + parent_id = self.form.cleaned_data["parent_id"] + if parent_id and validation_status in [OrgUnit.VALIDATION_NEW, OrgUnit.VALIDATION_VALID]: + try: + # Get parent's top ancestor. + top_ancestor = OrgUnit.objects.raw( + ( + "SELECT id, path FROM iaso_orgunit " + "WHERE path = SUBPATH((SELECT path FROM iaso_orgunit WHERE id = %s), 0, 1)" + ), + [parent_id], + )[0] + except IndexError: + raise ValidationError({"parent_id": [f"OrgUnit with id {parent_id} does not exist."]}) + + # Find the children of `REJECTED` org units which are not `REJECTED` themselves. + rejected_children = OrgUnit.objects.raw( + """ + WITH RECURSIVE rejected AS ( + SELECT id, validation_status + FROM iaso_orgunit + WHERE path <@ %s -- Limit to all descendants of the top_ancestor node (including self). + AND validation_status = 'REJECTED' + UNION + SELECT descendant.id, descendant.validation_status + FROM iaso_orgunit as descendant, rejected + WHERE descendant.parent_id = rejected.id + ) + SELECT 1 as id, array_agg(id) as ids + FROM rejected + WHERE validation_status != 'REJECTED'; + """, + [str(top_ancestor.path)], + )[0] + + if rejected_children.ids: + queryset = queryset.exclude(id__in=rejected_children.ids) return queryset.filter(validation_status=validation_status) diff --git a/iaso/api/org_unit_tree/views.py b/iaso/api/org_unit_tree/views.py index 088140e217..ea4150408e 100644 --- a/iaso/api/org_unit_tree/views.py +++ b/iaso/api/org_unit_tree/views.py @@ -1,10 +1,9 @@ import django_filters from django.db.models import Count -from rest_framework import filters, viewsets +from rest_framework import filters, permissions, viewsets from iaso.api.org_unit_tree.filters import OrgUnitTreeFilter -from iaso.api.org_unit_tree.pagination import OrgUnitTreePagination from iaso.api.org_unit_tree.serializers import OrgUnitTreeSerializer from iaso.models import OrgUnit @@ -14,11 +13,14 @@ class OrgUnitTreeViewSet(viewsets.ModelViewSet): filterset_class = OrgUnitTreeFilter http_method_names = ["get", "options", "head", "trace"] ordering_fields = ["id", "name"] - pagination_class = OrgUnitTreePagination + permission_classes = [permissions.AllowAny] serializer_class = OrgUnitTreeSerializer def get_queryset(self): - qs = OrgUnit.objects.filter_for_user(self.request.user) + if self.request.user.is_anonymous: + qs = OrgUnit.objects.all() + else: + qs = OrgUnit.objects.filter_for_user(self.request.user) qs = qs.only("id", "name", "validation_status", "version", "org_unit_type", "parent") qs = qs.annotate(children_count=Count("orgunit__id")) return qs From c81f222dfeab3f93bc9935d6a63f3c7c7297902c Mon Sep 17 00:00:00 2001 From: kemar Date: Thu, 2 May 2024 15:46:59 +0200 Subject: [PATCH 05/17] Add a `search` view to `OrgUnitTreeViewSet` --- iaso/api/org_unit_tree/filters.py | 33 +++++++++++++++++++++++++++++++ iaso/api/org_unit_tree/views.py | 18 ++++++++++++++++- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/iaso/api/org_unit_tree/filters.py b/iaso/api/org_unit_tree/filters.py index 0526b5b5e1..96b7486c35 100644 --- a/iaso/api/org_unit_tree/filters.py +++ b/iaso/api/org_unit_tree/filters.py @@ -93,3 +93,36 @@ def filter_validation_status(self, queryset: QuerySet, _, validation_status: str queryset = queryset.exclude(id__in=rejected_children.ids) return queryset.filter(validation_status=validation_status) + + +class OrgUnitTreeSearchFilter(django_filters.rest_framework.FilterSet): + class Meta: + model = OrgUnit + fields = ["name"] + + @property + def qs(self): + parent_qs = super().qs + + # Find the children of `REJECTED` org units which are not `REJECTED` themselves. + rejected_children = OrgUnit.objects.raw( + """ + WITH RECURSIVE rejected AS ( + SELECT id, validation_status + FROM iaso_orgunit + WHERE validation_status = 'REJECTED' + UNION + SELECT descendant.id, descendant.validation_status + FROM iaso_orgunit as descendant, rejected + WHERE descendant.parent_id = rejected.id + ) + SELECT 1 as id, array_agg(id) as ids + FROM rejected + WHERE validation_status != 'REJECTED'; + """, + )[0] + + if rejected_children.ids: + parent_qs = parent_qs.exclude(id__in=rejected_children.ids) + + return parent_qs diff --git a/iaso/api/org_unit_tree/views.py b/iaso/api/org_unit_tree/views.py index ea4150408e..89392caabd 100644 --- a/iaso/api/org_unit_tree/views.py +++ b/iaso/api/org_unit_tree/views.py @@ -2,8 +2,11 @@ from django.db.models import Count from rest_framework import filters, permissions, viewsets +from rest_framework.decorators import action +from rest_framework.response import Response -from iaso.api.org_unit_tree.filters import OrgUnitTreeFilter +from iaso.api.org_unit_tree.filters import OrgUnitTreeFilter, OrgUnitTreeSearchFilter +from iaso.api.org_unit_tree.pagination import OrgUnitTreePagination from iaso.api.org_unit_tree.serializers import OrgUnitTreeSerializer from iaso.models import OrgUnit @@ -13,6 +16,7 @@ class OrgUnitTreeViewSet(viewsets.ModelViewSet): filterset_class = OrgUnitTreeFilter http_method_names = ["get", "options", "head", "trace"] ordering_fields = ["id", "name"] + pagination_class = None # The tree is not paginated (but the search view is paginated). permission_classes = [permissions.AllowAny] serializer_class = OrgUnitTreeSerializer @@ -24,3 +28,15 @@ def get_queryset(self): qs = qs.only("id", "name", "validation_status", "version", "org_unit_type", "parent") qs = qs.annotate(children_count=Count("orgunit__id")) return qs + + @action(detail=False, methods=["get"]) + def search(self, request): + """ + TODO: ?page=2&limit=2 + """ + org_units = self.get_queryset() + paginator = OrgUnitTreePagination() + filtered_org_units = OrgUnitTreeSearchFilter(request.query_params, org_units).qs + paginated_org_units = paginator.paginate_queryset(filtered_org_units, request) + serializer = OrgUnitTreeSerializer(paginated_org_units, many=True, context={"request": request}) + return paginator.get_paginated_response(serializer.data) From 7a2de746f9d2c7479edbed1ebff39f191de570d1 Mon Sep 17 00:00:00 2001 From: kemar Date: Tue, 7 May 2024 17:28:06 +0200 Subject: [PATCH 06/17] Implement design doc changes --- iaso/api/org_unit_tree/filters.py | 105 +---------------- iaso/api/org_unit_tree/views.py | 50 ++++++-- iaso/api/org_units.py | 3 +- iaso/tests/api/org_unit_tree/__init__.py | 0 iaso/tests/api/org_unit_tree/test_views.py | 129 +++++++++++++++++++++ 5 files changed, 180 insertions(+), 107 deletions(-) create mode 100644 iaso/tests/api/org_unit_tree/__init__.py create mode 100644 iaso/tests/api/org_unit_tree/test_views.py diff --git a/iaso/api/org_unit_tree/filters.py b/iaso/api/org_unit_tree/filters.py index 96b7486c35..d2f36022a8 100644 --- a/iaso/api/org_unit_tree/filters.py +++ b/iaso/api/org_unit_tree/filters.py @@ -1,6 +1,7 @@ import django_filters + +from django import forms from django.db.models.query import QuerySet -from django.shortcuts import get_object_or_404 from django.utils.translation import gettext_lazy as _ from rest_framework.exceptions import ValidationError @@ -9,39 +10,21 @@ class OrgUnitTreeFilter(django_filters.rest_framework.FilterSet): - default_version = django_filters.BooleanFilter(method="filter_default_version", label=_("Default version")) ignore_empty_names = django_filters.BooleanFilter(method="filter_empty_names", label=_("Ignore empty names")) - parent_id = django_filters.NumberFilter(method="filter_parent_id", label=_("Parent ID")) + parent_id = django_filters.NumberFilter(field_name="parent_id", label=_("Parent ID")) source_id = django_filters.NumberFilter(method="filter_source_id", label=_("Source ID")) - validation_status = django_filters.ChoiceFilter( - method="filter_validation_status", label=_("Validation status"), choices=OrgUnit.VALIDATION_STATUS_CHOICES + validation_status = django_filters.MultipleChoiceFilter( + choices=OrgUnit.VALIDATION_STATUS_CHOICES, label=_("Validation status"), widget=forms.CheckboxSelectMultiple ) + search = django_filters.CharFilter(field_name="name", lookup_expr="icontains") class Meta: model = OrgUnit fields = ["version"] - @property - def qs(self): - parent_qs = super().qs - # If `parent_id` is not provided, return the tree's first level. - parent_id = self.form.cleaned_data["parent_id"] - if not parent_id: - return parent_qs.filter(parent__isnull=True) - return parent_qs - - def filter_default_version(self, queryset: QuerySet, _, use_default_version: bool) -> QuerySet: - user = self.request.user - if not use_default_version or user.is_anonymous: - return queryset - return queryset.filter(version=user.iaso_profile.account.default_version) - def filter_empty_names(self, queryset: QuerySet, _, use_empty_names: bool) -> QuerySet: return queryset.exclude(name="") if use_empty_names else queryset - def filter_parent_id(self, queryset: QuerySet, _, parent_id: str) -> QuerySet: - return queryset.filter(parent=parent_id) - def filter_source_id(self, queryset: QuerySet, _, source_id: int) -> QuerySet: try: source = DataSource.objects.get(id=source_id) @@ -50,79 +33,3 @@ def filter_source_id(self, queryset: QuerySet, _, source_id: int) -> QuerySet: if source.default_version: return queryset.filter(version=source.default_version) return queryset.filter(version__data_source_id=source_id) - - def filter_validation_status(self, queryset: QuerySet, _, validation_status: str) -> QuerySet: - """ - If we filter on VALID or NEW, the children of a REJECTED org unit should not be returned. - """ - parent_id = self.form.cleaned_data["parent_id"] - if parent_id and validation_status in [OrgUnit.VALIDATION_NEW, OrgUnit.VALIDATION_VALID]: - try: - # Get parent's top ancestor. - top_ancestor = OrgUnit.objects.raw( - ( - "SELECT id, path FROM iaso_orgunit " - "WHERE path = SUBPATH((SELECT path FROM iaso_orgunit WHERE id = %s), 0, 1)" - ), - [parent_id], - )[0] - except IndexError: - raise ValidationError({"parent_id": [f"OrgUnit with id {parent_id} does not exist."]}) - - # Find the children of `REJECTED` org units which are not `REJECTED` themselves. - rejected_children = OrgUnit.objects.raw( - """ - WITH RECURSIVE rejected AS ( - SELECT id, validation_status - FROM iaso_orgunit - WHERE path <@ %s -- Limit to all descendants of the top_ancestor node (including self). - AND validation_status = 'REJECTED' - UNION - SELECT descendant.id, descendant.validation_status - FROM iaso_orgunit as descendant, rejected - WHERE descendant.parent_id = rejected.id - ) - SELECT 1 as id, array_agg(id) as ids - FROM rejected - WHERE validation_status != 'REJECTED'; - """, - [str(top_ancestor.path)], - )[0] - - if rejected_children.ids: - queryset = queryset.exclude(id__in=rejected_children.ids) - - return queryset.filter(validation_status=validation_status) - - -class OrgUnitTreeSearchFilter(django_filters.rest_framework.FilterSet): - class Meta: - model = OrgUnit - fields = ["name"] - - @property - def qs(self): - parent_qs = super().qs - - # Find the children of `REJECTED` org units which are not `REJECTED` themselves. - rejected_children = OrgUnit.objects.raw( - """ - WITH RECURSIVE rejected AS ( - SELECT id, validation_status - FROM iaso_orgunit - WHERE validation_status = 'REJECTED' - UNION - SELECT descendant.id, descendant.validation_status - FROM iaso_orgunit as descendant, rejected - WHERE descendant.parent_id = rejected.id - ) - SELECT 1 as id, array_agg(id) as ids - FROM rejected - WHERE validation_status != 'REJECTED'; - """, - )[0] - - if rejected_children.ids: - parent_qs = parent_qs.exclude(id__in=rejected_children.ids) - - return parent_qs diff --git a/iaso/api/org_unit_tree/views.py b/iaso/api/org_unit_tree/views.py index 89392caabd..3bee05eaf4 100644 --- a/iaso/api/org_unit_tree/views.py +++ b/iaso/api/org_unit_tree/views.py @@ -1,42 +1,78 @@ import django_filters + from django.db.models import Count from rest_framework import filters, permissions, viewsets +from rest_framework import serializers from rest_framework.decorators import action -from rest_framework.response import Response -from iaso.api.org_unit_tree.filters import OrgUnitTreeFilter, OrgUnitTreeSearchFilter +from iaso.api.org_unit_tree.filters import OrgUnitTreeFilter from iaso.api.org_unit_tree.pagination import OrgUnitTreePagination from iaso.api.org_unit_tree.serializers import OrgUnitTreeSerializer from iaso.models import OrgUnit +class OrgUnitTreeQuerystringSerializer(serializers.Serializer): + force_full_tree = serializers.BooleanField(required=False) + parent_id = serializers.IntegerField(required=False) + + class OrgUnitTreeViewSet(viewsets.ModelViewSet): + """ + Explore the OrgUnit tree level by level. + + Since results are displayed level by level, results are not paginated. + """ + filter_backends = [filters.OrderingFilter, django_filters.rest_framework.DjangoFilterBackend] filterset_class = OrgUnitTreeFilter http_method_names = ["get", "options", "head", "trace"] ordering_fields = ["id", "name"] - pagination_class = None # The tree is not paginated (but the search view is paginated). + pagination_class = None permission_classes = [permissions.AllowAny] serializer_class = OrgUnitTreeSerializer def get_queryset(self): - if self.request.user.is_anonymous: + user = self.request.user + + querystring_serializer = OrgUnitTreeQuerystringSerializer(data=self.request.query_params) + querystring_serializer.is_valid(raise_exception=True) + force_full_tree = querystring_serializer.validated_data.get("force_full_tree") + parent_id = querystring_serializer.validated_data.get("parent_id") + + can_view_full_tree = any([user.is_anonymous, user.is_superuser, force_full_tree]) + if can_view_full_tree: qs = OrgUnit.objects.all() else: - qs = OrgUnit.objects.filter_for_user(self.request.user) + qs = OrgUnit.objects.filter_for_user(user) + + display_root_level = self.action == "list" and not parent_id + if display_root_level: + if can_view_full_tree: + qs = qs.filter(parent__isnull=True) + elif user.is_authenticated: + if user.iaso_profile.org_units.exists(): + qs = qs.filter(id__in=user.iaso_profile.org_units.all()) + qs = qs.only("id", "name", "validation_status", "version", "org_unit_type", "parent") qs = qs.annotate(children_count=Count("orgunit__id")) + qs = qs.order_by("name") + qs = qs.select_related("org_unit_type") # Avoid N+1 in the serializer. + return qs @action(detail=False, methods=["get"]) def search(self, request): """ - TODO: ?page=2&limit=2 + Search the OrgUnit tree. + + ``` + /api/orgunits/tree/search/?search=congo&page=2&limit=10 + ``` """ org_units = self.get_queryset() paginator = OrgUnitTreePagination() - filtered_org_units = OrgUnitTreeSearchFilter(request.query_params, org_units).qs + filtered_org_units = self.filterset_class(request.query_params, org_units).qs paginated_org_units = paginator.paginate_queryset(filtered_org_units, request) serializer = OrgUnitTreeSerializer(paginated_org_units, many=True, context={"request": request}) return paginator.get_paginated_response(serializer.data) diff --git a/iaso/api/org_units.py b/iaso/api/org_units.py index a97bb115b5..7dfbeefa52 100644 --- a/iaso/api/org_units.py +++ b/iaso/api/org_units.py @@ -365,7 +365,8 @@ def treesearch(self, request, **kwargs): if roots_for_user: org_unit_for_profile = request.user.iaso_profile.org_units.only("id") - if org_unit_for_profile and not request.user.is_superuser: + # if org_unit_for_profile and not request.user.is_superuser: + if org_unit_for_profile: queryset = queryset.filter(id__in=org_unit_for_profile) else: queryset = queryset.filter(parent__isnull=True) diff --git a/iaso/tests/api/org_unit_tree/__init__.py b/iaso/tests/api/org_unit_tree/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/iaso/tests/api/org_unit_tree/test_views.py b/iaso/tests/api/org_unit_tree/test_views.py new file mode 100644 index 0000000000..f1d5b59661 --- /dev/null +++ b/iaso/tests/api/org_unit_tree/test_views.py @@ -0,0 +1,129 @@ +from iaso.test import APITestCase +from iaso import models as m + + +class OrgUnitTreeViewsAPITestCase(APITestCase): + @classmethod + def setUpTestData(cls): + cls.data_source = m.DataSource.objects.create(name="Data source") + cls.source_version = m.SourceVersion.objects.create(number=1, data_source=cls.data_source) + + # Angola. + cls.angola = m.OrgUnit.objects.create( + name="Angola", + org_unit_type=m.OrgUnitType.objects.create(category="COUNTRY"), + validation_status=m.OrgUnit.VALIDATION_VALID, + path=["1"], + version=cls.source_version, + ) + cls.angola_region = m.OrgUnit.objects.create( + name="Huila", + parent=cls.angola, + org_unit_type=m.OrgUnitType.objects.create(category="REGION"), + validation_status=m.OrgUnit.VALIDATION_VALID, + path=["1", "2"], + version=cls.source_version, + ) + cls.angola_district = m.OrgUnit.objects.create( + name="Cuvango", + parent=cls.angola_region, + org_unit_type=m.OrgUnitType.objects.create(category="DISTRICT"), + validation_status=m.OrgUnit.VALIDATION_VALID, + path=["1", "2", "3"], + version=cls.source_version, + ) + cls.angola_district.calculate_paths() + + # Burkina. + cls.burkina = m.OrgUnit.objects.create( + name="Burkina Faso", + org_unit_type=m.OrgUnitType.objects.create(category="COUNTRY"), + validation_status=m.OrgUnit.VALIDATION_VALID, + path=["2"], + version=cls.source_version, + ) + cls.burkina_region = m.OrgUnit.objects.create( + name="Boucle du Mouhon", + parent=cls.burkina, + org_unit_type=m.OrgUnitType.objects.create(category="REGION"), + validation_status=m.OrgUnit.VALIDATION_VALID, + path=["2", "2"], + version=cls.source_version, + ) + cls.burkina_district = m.OrgUnit.objects.create( + name="Banwa", + parent=cls.burkina_region, + org_unit_type=m.OrgUnitType.objects.create(category="DISTRICT"), + validation_status=m.OrgUnit.VALIDATION_VALID, + path=["2", "2", "3"], + version=cls.source_version, + ) + cls.burkina_district.calculate_paths() + + cls.account = m.Account.objects.create(name="Account", default_version=cls.source_version) + cls.project = m.Project.objects.create(name="Project", account=cls.account, app_id="foo.bar.baz") + cls.user = cls.create_user_with_profile(username="user", account=cls.account) + + cls.data_source.projects.set([cls.project]) + cls.user.iaso_profile.org_units.set([cls.burkina]) # Restrict access to a subset of org units for user. + + def test_tree_root_for_anonymous(self): + with self.assertNumQueries(1): + response = self.client.get("/api/orgunits/tree/") + self.assertJSONResponse(response, 200) + self.assertEqual(2, len(response.data)) + self.assertEqual(response.data[0]["name"], "Angola") + self.assertEqual(response.data[1]["name"], "Burkina Faso") + + def test_tree_level_for_anonymous(self): + with self.assertNumQueries(1): + response = self.client.get(f"/api/orgunits/tree/?parent_id={self.angola.pk}") + self.assertJSONResponse(response, 200) + self.assertEqual(1, len(response.data)) + self.assertEqual(response.data[0]["name"], "Huila") + + def test_restricted_tree_root(self): + self.client.force_authenticate(self.user) + with self.assertNumQueries(3): + response = self.client.get("/api/orgunits/tree/") + self.assertJSONResponse(response, 200) + self.assertEqual(1, len(response.data)) + self.assertEqual(response.data[0]["name"], "Burkina Faso") + + def test_restricted_tree_level(self): + self.client.force_authenticate(self.user) + + with self.assertNumQueries(2): + response = self.client.get(f"/api/orgunits/tree/?parent_id={self.angola.pk}") + self.assertJSONResponse(response, 200) + self.assertEqual(0, len(response.data)) + + with self.assertNumQueries(2): + response = self.client.get(f"/api/orgunits/tree/?parent_id={self.burkina.pk}") + self.assertJSONResponse(response, 200) + self.assertEqual(1, len(response.data)) + self.assertEqual(response.data[0]["name"], "Boucle du Mouhon") + + def test_force_full_tree_root(self): + self.client.force_authenticate(self.user) + with self.assertNumQueries(1): + response = self.client.get("/api/orgunits/tree/?force_full_tree=true") + self.assertJSONResponse(response, 200) + self.assertEqual(2, len(response.data)) + self.assertEqual(response.data[0]["name"], "Angola") + self.assertEqual(response.data[1]["name"], "Burkina Faso") + + def test_force_full_tree_level(self): + self.client.force_authenticate(self.user) + + with self.assertNumQueries(1): + response = self.client.get(f"/api/orgunits/tree/?parent_id={self.angola.pk}&force_full_tree=true") + self.assertJSONResponse(response, 200) + self.assertEqual(1, len(response.data)) + self.assertEqual(response.data[0]["name"], "Huila") + + with self.assertNumQueries(1): + response = self.client.get(f"/api/orgunits/tree/?parent_id={self.burkina.pk}&force_full_tree=true") + self.assertJSONResponse(response, 200) + self.assertEqual(1, len(response.data)) + self.assertEqual(response.data[0]["name"], "Boucle du Mouhon") From 6a4fa97dfffcef4f0f775a07c579ad76fe843463 Mon Sep 17 00:00:00 2001 From: kemar Date: Tue, 7 May 2024 17:52:34 +0200 Subject: [PATCH 07/17] Remove comment --- iaso/api/org_units.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/iaso/api/org_units.py b/iaso/api/org_units.py index 7dfbeefa52..a97bb115b5 100644 --- a/iaso/api/org_units.py +++ b/iaso/api/org_units.py @@ -365,8 +365,7 @@ def treesearch(self, request, **kwargs): if roots_for_user: org_unit_for_profile = request.user.iaso_profile.org_units.only("id") - # if org_unit_for_profile and not request.user.is_superuser: - if org_unit_for_profile: + if org_unit_for_profile and not request.user.is_superuser: queryset = queryset.filter(id__in=org_unit_for_profile) else: queryset = queryset.filter(parent__isnull=True) From e54f9b255a4aac2ba850f524809176f1effad5a9 Mon Sep 17 00:00:00 2001 From: kemar Date: Mon, 13 May 2024 17:32:26 +0200 Subject: [PATCH 08/17] Add the `show_invalid_parents` field to `DataSource` --- .../0280_datasource_show_invalid_parents.py | 20 +++++++++++++++++++ iaso/models/data_source.py | 3 +++ 2 files changed, 23 insertions(+) create mode 100644 iaso/migrations/0280_datasource_show_invalid_parents.py diff --git a/iaso/migrations/0280_datasource_show_invalid_parents.py b/iaso/migrations/0280_datasource_show_invalid_parents.py new file mode 100644 index 0000000000..fc2f588020 --- /dev/null +++ b/iaso/migrations/0280_datasource_show_invalid_parents.py @@ -0,0 +1,20 @@ +# Generated by Django 4.2.13 on 2024-05-13 15:19 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("iaso", "0279_merge_20240417_1319"), + ] + + operations = [ + migrations.AddField( + model_name="datasource", + name="show_invalid_parents", + field=models.BooleanField( + default=False, + help_text="Designates whether invalid parents should be displayed in the OrgUnit tree.", + ), + ), + ] diff --git a/iaso/models/data_source.py b/iaso/models/data_source.py index 6beba26daf..bed9cb45ba 100644 --- a/iaso/models/data_source.py +++ b/iaso/models/data_source.py @@ -19,6 +19,9 @@ class DataSource(models.Model): created_at = models.DateTimeField(auto_now_add=True) updated_at = models.DateTimeField(auto_now=True) default_version = models.ForeignKey("SourceVersion", null=True, blank=True, on_delete=models.SET_NULL) + show_invalid_parents = models.BooleanField( + default=False, help_text="Designates whether invalid parents should be displayed in the OrgUnit tree." + ) def __str__(self): return "%s " % (self.name,) From c4f4d6442b04b42cb865d7ffd23c0361cb5d9353 Mon Sep 17 00:00:00 2001 From: kemar Date: Mon, 13 May 2024 17:32:55 +0200 Subject: [PATCH 09/17] Fix some N+1 --- iaso/api/data_sources.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/iaso/api/data_sources.py b/iaso/api/data_sources.py index 3031df0148..177778d77d 100644 --- a/iaso/api/data_sources.py +++ b/iaso/api/data_sources.py @@ -31,6 +31,7 @@ class Meta: "projects", "default_version", "credentials", + "show_invalid_parents", ] url = serializers.SerializerMethodField() @@ -243,7 +244,12 @@ def get_queryset(self): linked_to = self.kwargs.get("linkedTo", None) profile = self.request.user.iaso_profile order = self.request.GET.get("order", "name").split(",") - sources = DataSource.objects.filter(projects__account=profile.account).distinct() + sources = ( + DataSource.objects.select_related("default_version", "credentials") + .prefetch_related("projects", "versions") + .filter(projects__account=profile.account) + .distinct() + ) if linked_to: org_unit = OrgUnit.objects.get(pk=linked_to) useful_sources = org_unit.source_set.values_list("algorithm_run__version_2__data_source_id", flat=True) From 89d883f3e6b6b5912072bf40ba462430efc74412 Mon Sep 17 00:00:00 2001 From: kemar Date: Mon, 13 May 2024 17:43:23 +0200 Subject: [PATCH 10/17] Fix tests --- iaso/api/profiles.py | 1 + 1 file changed, 1 insertion(+) diff --git a/iaso/api/profiles.py b/iaso/api/profiles.py index d6235998a4..78f5bc3ac8 100644 --- a/iaso/api/profiles.py +++ b/iaso/api/profiles.py @@ -305,6 +305,7 @@ def get_row(profile: Profile, **_) -> List[Any]: filename = "users" response: Union[HttpResponse, StreamingHttpResponse] + queryset = queryset.order_by("id") if file_format == FileFormatEnum.XLSX: filename = filename + ".xlsx" From 803cbb999f8f0939ff2d938b9a9c4aa02d10ad85 Mon Sep 17 00:00:00 2001 From: kemar Date: Tue, 14 May 2024 16:43:37 +0200 Subject: [PATCH 11/17] Add the `tree_config_status_fields` field to `DataSource` --- iaso/api/data_sources.py | 2 +- .../0280_datasource_show_invalid_parents.py | 20 ---------------- ...80_datasource_tree_config_status_fields.py | 24 +++++++++++++++++++ iaso/models/data_source.py | 22 +++++++++++++++-- 4 files changed, 45 insertions(+), 23 deletions(-) delete mode 100644 iaso/migrations/0280_datasource_show_invalid_parents.py create mode 100644 iaso/migrations/0280_datasource_tree_config_status_fields.py diff --git a/iaso/api/data_sources.py b/iaso/api/data_sources.py index 177778d77d..a87d3b58c6 100644 --- a/iaso/api/data_sources.py +++ b/iaso/api/data_sources.py @@ -31,7 +31,7 @@ class Meta: "projects", "default_version", "credentials", - "show_invalid_parents", + "tree_config_status_fields", ] url = serializers.SerializerMethodField() diff --git a/iaso/migrations/0280_datasource_show_invalid_parents.py b/iaso/migrations/0280_datasource_show_invalid_parents.py deleted file mode 100644 index fc2f588020..0000000000 --- a/iaso/migrations/0280_datasource_show_invalid_parents.py +++ /dev/null @@ -1,20 +0,0 @@ -# Generated by Django 4.2.13 on 2024-05-13 15:19 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - dependencies = [ - ("iaso", "0279_merge_20240417_1319"), - ] - - operations = [ - migrations.AddField( - model_name="datasource", - name="show_invalid_parents", - field=models.BooleanField( - default=False, - help_text="Designates whether invalid parents should be displayed in the OrgUnit tree.", - ), - ), - ] diff --git a/iaso/migrations/0280_datasource_tree_config_status_fields.py b/iaso/migrations/0280_datasource_tree_config_status_fields.py new file mode 100644 index 0000000000..085d769d35 --- /dev/null +++ b/iaso/migrations/0280_datasource_tree_config_status_fields.py @@ -0,0 +1,24 @@ +# Generated by Django 4.2.13 on 2024-05-14 14:23 + +import django.contrib.postgres.fields +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("iaso", "0279_merge_20240417_1319"), + ] + + operations = [ + migrations.AddField( + model_name="datasource", + name="tree_config_status_fields", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField(blank=True, choices=[], max_length=30), + blank=True, + default=list, + help_text="List of statuses used for display configuration of the OrgUnit tree.", + size=None, + ), + ), + ] diff --git a/iaso/models/data_source.py b/iaso/models/data_source.py index bed9cb45ba..c5fada5c66 100644 --- a/iaso/models/data_source.py +++ b/iaso/models/data_source.py @@ -1,3 +1,4 @@ +from django.contrib.postgres.fields import ArrayField from django.db import models @@ -19,13 +20,30 @@ class DataSource(models.Model): created_at = models.DateTimeField(auto_now_add=True) updated_at = models.DateTimeField(auto_now=True) default_version = models.ForeignKey("SourceVersion", null=True, blank=True, on_delete=models.SET_NULL) - show_invalid_parents = models.BooleanField( - default=False, help_text="Designates whether invalid parents should be displayed in the OrgUnit tree." + tree_config_status_fields = ArrayField( + models.CharField(max_length=30, blank=True, choices=[]), + default=list, + blank=True, + help_text="List of statuses used for display configuration of the OrgUnit tree.", ) def __str__(self): return "%s " % (self.name,) + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + # Override `__init__` to avoid a circular import. + # This could be replaced by a callable in Django ≥ 5. + # https://docs.djangoproject.com/en/5.0/releases/5.0/#more-options-for-declaring-field-choices + from iaso.models import OrgUnit + + self._meta.get_field("tree_config_status_fields").base_field.choices = OrgUnit.VALIDATION_STATUS_CHOICES + + def clean(self, *args, **kwargs): + super().clean() + self.tree_config_status_fields = list(set(self.tree_config_status_fields)) + def as_dict(self): versions = SourceVersion.objects.filter(data_source_id=self.id) return { From dd2c572332add6cc8d83483ce96f87048e44d901 Mon Sep 17 00:00:00 2001 From: kemar Date: Wed, 15 May 2024 10:36:48 +0200 Subject: [PATCH 12/17] Add tests for the `DataSource` model --- iaso/tests/models/test_datasource.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 iaso/tests/models/test_datasource.py diff --git a/iaso/tests/models/test_datasource.py b/iaso/tests/models/test_datasource.py new file mode 100644 index 0000000000..06d608c4f1 --- /dev/null +++ b/iaso/tests/models/test_datasource.py @@ -0,0 +1,28 @@ +from django.core.exceptions import ValidationError + +from iaso import models as m +from iaso.test import TestCase + + +class DataSourceModelTestCase(TestCase): + """ + Test DataSource model. + """ + + @classmethod + def setUpTestData(cls): + cls.data_source = m.DataSource.objects.create( + name="DataSource 1", + description="Description 1", + ) + + def test_full_clean_should_raise_for_invalid_choice(self): + self.data_source.tree_config_status_fields = ["FOO"] + with self.assertRaises(ValidationError) as error: + self.data_source.full_clean() + self.assertIn("Value 'FOO' is not a valid choice.", error.exception.messages[0]) + + def test_clean_should_remove_duplicates(self): + self.data_source.tree_config_status_fields = [m.OrgUnit.VALIDATION_NEW, m.OrgUnit.VALIDATION_NEW] + self.data_source.clean() + self.assertEqual(self.data_source.tree_config_status_fields, [m.OrgUnit.VALIDATION_NEW]) From 6672da20c63ce4e8e8a4aa2b7cb898d73ebce222 Mon Sep 17 00:00:00 2001 From: kemar Date: Wed, 15 May 2024 11:21:04 +0200 Subject: [PATCH 13/17] Fix perms --- iaso/api/org_unit_tree/filters.py | 10 +++++----- iaso/api/org_unit_tree/views.py | 23 ++++++++++++++-------- iaso/tests/api/org_unit_tree/test_views.py | 14 +++++++++---- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/iaso/api/org_unit_tree/filters.py b/iaso/api/org_unit_tree/filters.py index d2f36022a8..459265a22c 100644 --- a/iaso/api/org_unit_tree/filters.py +++ b/iaso/api/org_unit_tree/filters.py @@ -12,7 +12,7 @@ class OrgUnitTreeFilter(django_filters.rest_framework.FilterSet): ignore_empty_names = django_filters.BooleanFilter(method="filter_empty_names", label=_("Ignore empty names")) parent_id = django_filters.NumberFilter(field_name="parent_id", label=_("Parent ID")) - source_id = django_filters.NumberFilter(method="filter_source_id", label=_("Source ID")) + data_source_id = django_filters.NumberFilter(method="filter_data_source_id", label=_("Data source ID")) validation_status = django_filters.MultipleChoiceFilter( choices=OrgUnit.VALIDATION_STATUS_CHOICES, label=_("Validation status"), widget=forms.CheckboxSelectMultiple ) @@ -25,11 +25,11 @@ class Meta: def filter_empty_names(self, queryset: QuerySet, _, use_empty_names: bool) -> QuerySet: return queryset.exclude(name="") if use_empty_names else queryset - def filter_source_id(self, queryset: QuerySet, _, source_id: int) -> QuerySet: + def filter_data_source_id(self, queryset: QuerySet, _, data_source_id: int) -> QuerySet: try: - source = DataSource.objects.get(id=source_id) + source = DataSource.objects.get(id=data_source_id) except OrgUnit.DoesNotExist: - raise ValidationError({"source_id": [f"DataSource with id {source_id} does not exist."]}) + raise ValidationError({"data_source_id": [f"DataSource with id {data_source_id} does not exist."]}) if source.default_version: return queryset.filter(version=source.default_version) - return queryset.filter(version__data_source_id=source_id) + return queryset.filter(version__data_source_id=data_source_id) diff --git a/iaso/api/org_unit_tree/views.py b/iaso/api/org_unit_tree/views.py index 3bee05eaf4..e0b50c4f7c 100644 --- a/iaso/api/org_unit_tree/views.py +++ b/iaso/api/org_unit_tree/views.py @@ -5,6 +5,7 @@ from rest_framework import filters, permissions, viewsets from rest_framework import serializers from rest_framework.decorators import action +from rest_framework.exceptions import ValidationError from iaso.api.org_unit_tree.filters import OrgUnitTreeFilter from iaso.api.org_unit_tree.pagination import OrgUnitTreePagination @@ -15,20 +16,19 @@ class OrgUnitTreeQuerystringSerializer(serializers.Serializer): force_full_tree = serializers.BooleanField(required=False) parent_id = serializers.IntegerField(required=False) + data_source_id = serializers.IntegerField(required=False) class OrgUnitTreeViewSet(viewsets.ModelViewSet): """ Explore the OrgUnit tree level by level. - - Since results are displayed level by level, results are not paginated. """ filter_backends = [filters.OrderingFilter, django_filters.rest_framework.DjangoFilterBackend] filterset_class = OrgUnitTreeFilter http_method_names = ["get", "options", "head", "trace"] ordering_fields = ["id", "name"] - pagination_class = None + pagination_class = None # Since results are displayed level by level, results are not paginated in the list view. permission_classes = [permissions.AllowAny] serializer_class = OrgUnitTreeSerializer @@ -39,18 +39,25 @@ def get_queryset(self): querystring_serializer.is_valid(raise_exception=True) force_full_tree = querystring_serializer.validated_data.get("force_full_tree") parent_id = querystring_serializer.validated_data.get("parent_id") - - can_view_full_tree = any([user.is_anonymous, user.is_superuser, force_full_tree]) - if can_view_full_tree: - qs = OrgUnit.objects.all() + data_source_id = querystring_serializer.validated_data.get("data_source_id") + + if user.is_anonymous: + if not data_source_id: + raise ValidationError({"data_source_id": ["DataSource must be provided for anonymous users."]}) + qs = OrgUnit.objects.all() # `qs` will be filtered by `data_source_id` in `OrgUnitTreeFilter`. + elif user.is_superuser or force_full_tree: + # Filter by the account. + qs = OrgUnit.objects.filter(version=user.iaso_profile.account.default_version) else: qs = OrgUnit.objects.filter_for_user(user) + can_view_full_tree = any([user.is_anonymous, user.is_superuser, force_full_tree]) display_root_level = self.action == "list" and not parent_id + if display_root_level: if can_view_full_tree: qs = qs.filter(parent__isnull=True) - elif user.is_authenticated: + elif user.is_authenticated: # The user might be restricted to a subpart of the tree. if user.iaso_profile.org_units.exists(): qs = qs.filter(id__in=user.iaso_profile.org_units.all()) diff --git a/iaso/tests/api/org_unit_tree/test_views.py b/iaso/tests/api/org_unit_tree/test_views.py index f1d5b59661..79617b5ceb 100644 --- a/iaso/tests/api/org_unit_tree/test_views.py +++ b/iaso/tests/api/org_unit_tree/test_views.py @@ -68,16 +68,22 @@ def setUpTestData(cls): cls.user.iaso_profile.org_units.set([cls.burkina]) # Restrict access to a subset of org units for user. def test_tree_root_for_anonymous(self): - with self.assertNumQueries(1): - response = self.client.get("/api/orgunits/tree/") + with self.assertNumQueries(2): + # Select `DataSource`. + # Select `OrgUnit`s. + response = self.client.get(f"/api/orgunits/tree/?data_source_id={self.data_source.pk}") self.assertJSONResponse(response, 200) self.assertEqual(2, len(response.data)) self.assertEqual(response.data[0]["name"], "Angola") self.assertEqual(response.data[1]["name"], "Burkina Faso") def test_tree_level_for_anonymous(self): - with self.assertNumQueries(1): - response = self.client.get(f"/api/orgunits/tree/?parent_id={self.angola.pk}") + with self.assertNumQueries(2): + # Select `DataSource`. + # Select `OrgUnit`s. + response = self.client.get( + f"/api/orgunits/tree/?parent_id={self.angola.pk}&data_source_id={self.data_source.pk}" + ) self.assertJSONResponse(response, 200) self.assertEqual(1, len(response.data)) self.assertEqual(response.data[0]["name"], "Huila") From 42f7348daffff86bf4f0548b6ac9c779ccbd3924 Mon Sep 17 00:00:00 2001 From: kemar Date: Wed, 15 May 2024 16:56:01 +0200 Subject: [PATCH 14/17] Exclude invalid children when we filter only by VALID org units --- iaso/api/org_unit_tree/views.py | 34 ++++++++++------- iaso/tests/api/org_unit_tree/test_views.py | 44 +++++++++++++++++++--- 2 files changed, 59 insertions(+), 19 deletions(-) diff --git a/iaso/api/org_unit_tree/views.py b/iaso/api/org_unit_tree/views.py index e0b50c4f7c..2c6dd97a60 100644 --- a/iaso/api/org_unit_tree/views.py +++ b/iaso/api/org_unit_tree/views.py @@ -1,6 +1,6 @@ import django_filters -from django.db.models import Count +from django.db.models import Count, Q from rest_framework import filters, permissions, viewsets from rest_framework import serializers @@ -17,6 +17,7 @@ class OrgUnitTreeQuerystringSerializer(serializers.Serializer): force_full_tree = serializers.BooleanField(required=False) parent_id = serializers.IntegerField(required=False) data_source_id = serializers.IntegerField(required=False) + validation_status = serializers.MultipleChoiceField(choices=OrgUnit.VALIDATION_STATUS_CHOICES) class OrgUnitTreeViewSet(viewsets.ModelViewSet): @@ -35,36 +36,43 @@ class OrgUnitTreeViewSet(viewsets.ModelViewSet): def get_queryset(self): user = self.request.user - querystring_serializer = OrgUnitTreeQuerystringSerializer(data=self.request.query_params) - querystring_serializer.is_valid(raise_exception=True) - force_full_tree = querystring_serializer.validated_data.get("force_full_tree") - parent_id = querystring_serializer.validated_data.get("parent_id") - data_source_id = querystring_serializer.validated_data.get("data_source_id") + querystring = OrgUnitTreeQuerystringSerializer(data=self.request.query_params) + querystring.is_valid(raise_exception=True) + force_full_tree = querystring.validated_data.get("force_full_tree") + parent_id = querystring.validated_data.get("parent_id") + data_source_id = querystring.validated_data.get("data_source_id") + validation_status = querystring.validated_data.get("validation_status", set()) if user.is_anonymous: if not data_source_id: - raise ValidationError({"data_source_id": ["DataSource must be provided for anonymous users."]}) + raise ValidationError({"data_source_id": ["A `data_source_id` must be provided for anonymous users."]}) qs = OrgUnit.objects.all() # `qs` will be filtered by `data_source_id` in `OrgUnitTreeFilter`. elif user.is_superuser or force_full_tree: - # Filter by the account. qs = OrgUnit.objects.filter(version=user.iaso_profile.account.default_version) else: qs = OrgUnit.objects.filter_for_user(user) can_view_full_tree = any([user.is_anonymous, user.is_superuser, force_full_tree]) - display_root_level = self.action == "list" and not parent_id + display_root_level = not parent_id - if display_root_level: + if display_root_level and self.action == "list": if can_view_full_tree: qs = qs.filter(parent__isnull=True) - elif user.is_authenticated: # The user might be restricted to a subpart of the tree. + elif user.is_authenticated: if user.iaso_profile.org_units.exists(): + # Root level of the tree for this user (the user may be restricted to a subpart of the tree). qs = qs.filter(id__in=user.iaso_profile.org_units.all()) qs = qs.only("id", "name", "validation_status", "version", "org_unit_type", "parent") - qs = qs.annotate(children_count=Count("orgunit__id")) qs = qs.order_by("name") - qs = qs.select_related("org_unit_type") # Avoid N+1 in the serializer. + qs = qs.select_related("org_unit_type") + + if validation_status == {OrgUnit.VALIDATION_VALID}: + exclude_filter = ~Q(orgunit__validation_status__in=[OrgUnit.VALIDATION_REJECTED, OrgUnit.VALIDATION_NEW]) + children_count = Count("orgunit", filter=exclude_filter) + else: + children_count = Count("orgunit") + qs = qs.annotate(children_count=children_count) return qs diff --git a/iaso/tests/api/org_unit_tree/test_views.py b/iaso/tests/api/org_unit_tree/test_views.py index 79617b5ceb..b7dd171795 100644 --- a/iaso/tests/api/org_unit_tree/test_views.py +++ b/iaso/tests/api/org_unit_tree/test_views.py @@ -67,7 +67,7 @@ def setUpTestData(cls): cls.data_source.projects.set([cls.project]) cls.user.iaso_profile.org_units.set([cls.burkina]) # Restrict access to a subset of org units for user. - def test_tree_root_for_anonymous(self): + def test_root_for_anonymous(self): with self.assertNumQueries(2): # Select `DataSource`. # Select `OrgUnit`s. @@ -77,7 +77,7 @@ def test_tree_root_for_anonymous(self): self.assertEqual(response.data[0]["name"], "Angola") self.assertEqual(response.data[1]["name"], "Burkina Faso") - def test_tree_level_for_anonymous(self): + def test_specific_level_for_anonymous(self): with self.assertNumQueries(2): # Select `DataSource`. # Select `OrgUnit`s. @@ -88,7 +88,7 @@ def test_tree_level_for_anonymous(self): self.assertEqual(1, len(response.data)) self.assertEqual(response.data[0]["name"], "Huila") - def test_restricted_tree_root(self): + def test_root_for_authenticated_user(self): self.client.force_authenticate(self.user) with self.assertNumQueries(3): response = self.client.get("/api/orgunits/tree/") @@ -96,7 +96,7 @@ def test_restricted_tree_root(self): self.assertEqual(1, len(response.data)) self.assertEqual(response.data[0]["name"], "Burkina Faso") - def test_restricted_tree_level(self): + def test_specific_level_for_authenticated_user(self): self.client.force_authenticate(self.user) with self.assertNumQueries(2): @@ -110,7 +110,7 @@ def test_restricted_tree_level(self): self.assertEqual(1, len(response.data)) self.assertEqual(response.data[0]["name"], "Boucle du Mouhon") - def test_force_full_tree_root(self): + def test_root_with_force_full_tree(self): self.client.force_authenticate(self.user) with self.assertNumQueries(1): response = self.client.get("/api/orgunits/tree/?force_full_tree=true") @@ -119,7 +119,7 @@ def test_force_full_tree_root(self): self.assertEqual(response.data[0]["name"], "Angola") self.assertEqual(response.data[1]["name"], "Burkina Faso") - def test_force_full_tree_level(self): + def test_specific_level_with_force_full_tree(self): self.client.force_authenticate(self.user) with self.assertNumQueries(1): @@ -133,3 +133,35 @@ def test_force_full_tree_level(self): self.assertJSONResponse(response, 200) self.assertEqual(1, len(response.data)) self.assertEqual(response.data[0]["name"], "Boucle du Mouhon") + + def test_children_count(self): + """ + When we filter only by VALID org units, then REJECTED or NEW children should not be included in `has_children`. + """ + self.client.force_authenticate(self.user) + + url = f"/api/orgunits/tree/?parent_id={self.burkina.pk}&validation_status={m.OrgUnit.VALIDATION_VALID}" + + response = self.client.get(url) + self.assertJSONResponse(response, 200) + self.assertEqual(1, len(response.data)) + self.assertEqual(response.data[0]["name"], "Boucle du Mouhon") + self.assertEqual(response.data[0]["has_children"], True) + + # Mark the child of "Boucle du Mouhon" as "rejected". + self.burkina_district.validation_status = m.OrgUnit.VALIDATION_REJECTED + self.burkina_district.save() + + response = self.client.get(url) + self.assertJSONResponse(response, 200) + self.assertEqual(1, len(response.data)) + self.assertEqual(response.data[0]["has_children"], False) # It should be excluded from `has_children`. + + # Mark the child of "Boucle du Mouhon" as "new". + self.burkina_district.validation_status = m.OrgUnit.VALIDATION_NEW + self.burkina_district.save() + + response = self.client.get(url) + self.assertJSONResponse(response, 200) + self.assertEqual(1, len(response.data)) + self.assertEqual(response.data[0]["has_children"], False) # It should be excluded from `has_children`. From 028131c11d1d75e2b6a9a52718073fbd44858252 Mon Sep 17 00:00:00 2001 From: kemar Date: Wed, 15 May 2024 17:01:17 +0200 Subject: [PATCH 15/17] Add tests for the search view --- iaso/tests/api/org_unit_tree/test_views.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/iaso/tests/api/org_unit_tree/test_views.py b/iaso/tests/api/org_unit_tree/test_views.py index b7dd171795..ca963540bc 100644 --- a/iaso/tests/api/org_unit_tree/test_views.py +++ b/iaso/tests/api/org_unit_tree/test_views.py @@ -165,3 +165,19 @@ def test_children_count(self): self.assertJSONResponse(response, 200) self.assertEqual(1, len(response.data)) self.assertEqual(response.data[0]["has_children"], False) # It should be excluded from `has_children`. + + def test_search(self): + self.client.force_authenticate(self.user) + + response = self.client.get("/api/orgunits/tree/search/?search=b") + self.assertJSONResponse(response, 200) + + self.assertEqual(3, len(response.data["results"])) + self.assertEqual(response.data["results"][0]["name"], "Banwa") + self.assertEqual(response.data["results"][1]["name"], "Boucle du Mouhon") + self.assertEqual(response.data["results"][2]["name"], "Burkina Faso") + + response = self.client.get("/api/orgunits/tree/search/?search=BURKINA") + self.assertJSONResponse(response, 200) + self.assertEqual(1, len(response.data["results"])) + self.assertEqual(response.data["results"][0]["name"], "Burkina Faso") From 7622bc19df9c5735835f2462e312dc3d50581fb9 Mon Sep 17 00:00:00 2001 From: kemar Date: Wed, 15 May 2024 17:03:16 +0200 Subject: [PATCH 16/17] Add comment --- iaso/api/org_units.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/iaso/api/org_units.py b/iaso/api/org_units.py index a97bb115b5..3904a28b3b 100644 --- a/iaso/api/org_units.py +++ b/iaso/api/org_units.py @@ -336,6 +336,9 @@ def list_to_gpkg(self, queryset, filename): @action(methods=["GET"], detail=False) def treesearch(self, request, **kwargs): + """ + TODO: delete this route when it's been replaced by `OrgUnitTreeViewSet`. + """ queryset = self.get_queryset().order_by("name") params = request.GET parent_id = params.get("parent_id") From 85f84f9a5415e62c27c84163c302b76ffa8dd64b Mon Sep 17 00:00:00 2001 From: kemar Date: Wed, 15 May 2024 18:06:18 +0200 Subject: [PATCH 17/17] Display `ArrayField` with a `CheckboxSelectMultiple` widget in admin --- iaso/admin.py | 85 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 60 insertions(+), 25 deletions(-) diff --git a/iaso/admin.py b/iaso/admin.py index 68d61f7017..5090519fcc 100644 --- a/iaso/admin.py +++ b/iaso/admin.py @@ -1,38 +1,17 @@ from typing import Any from typing import Protocol +from django import forms as django_forms from django.contrib.admin import widgets from django.contrib.gis import admin, forms from django.contrib.gis.db import models as geomodels +from django.contrib.postgres.fields import ArrayField from django.db import models from django.utils.html import format_html_join, format_html from django.utils.safestring import mark_safe from django_json_widget.widgets import JSONEditorWidget from iaso.models.json_config import Config # type: ignore - - -class IasoJSONEditorWidget(JSONEditorWidget): - class Media: - css = {"all": ("css/admin-json-widget.css",)} - - def __init__(self, attrs=None, mode="code", options=None, width=None, height=None): - if height == None: - height = "400px" - - default_options = { - "modes": ["text", "code"], - "mode": mode, - "search": True, - } - if options: - default_options.update(options) - - super(IasoJSONEditorWidget, self).__init__( - attrs=attrs, mode=mode, options=default_options, width=width, height=height - ) - - from .models import ( Account, AccountFeatureFlag, @@ -87,11 +66,58 @@ def __init__(self, attrs=None, mode="code", options=None, width=None, height=Non Payment, PaymentLot, ) -from .models.microplanning import Team, Planning, Assignment from .models.data_store import JsonDataStore +from .models.microplanning import Team, Planning, Assignment from .utils.gis import convert_2d_point_to_3d +class IasoJSONEditorWidget(JSONEditorWidget): + class Media: + css = {"all": ("css/admin-json-widget.css",)} + + def __init__(self, attrs=None, mode="code", options=None, width=None, height=None): + if height == None: + height = "400px" + + default_options = { + "modes": ["text", "code"], + "mode": mode, + "search": True, + } + if options: + default_options.update(options) + + super(IasoJSONEditorWidget, self).__init__( + attrs=attrs, mode=mode, options=default_options, width=width, height=height + ) + + +class ArrayFieldMultipleChoiceField(django_forms.MultipleChoiceField): + """ + Display a multi-select field for ArrayField: + + formfield_overrides = { + ArrayField: { + "form_class": ArrayFieldMultipleChoiceField, + } + } + + formfield_overrides = { + ArrayField: { + "form_class": ArrayFieldMultipleChoiceField, + "widget": forms.CheckboxSelectMultiple, + } + } + """ + + def __init__(self, *args, **kwargs): + kwargs.pop("max_length", None) + base_field = kwargs.pop("base_field", None) + kwargs["choices"] = base_field.choices + kwargs["choices"].pop(0) + super().__init__(*args, **kwargs) + + class AdminAttributes(Protocol): """Workaround to avoid mypy errors, see https://github.com/python/mypy/issues/2087#issuecomment-462726600""" @@ -786,10 +812,19 @@ class PaymentLotAdmin(admin.ModelAdmin): formfield_overrides = {models.JSONField: {"widget": IasoJSONEditorWidget}} +@admin.register(DataSource) +class DataSourceAdmin(admin.ModelAdmin): + formfield_overrides = { + ArrayField: { + "form_class": ArrayFieldMultipleChoiceField, + "widget": forms.CheckboxSelectMultiple, + } + } + + admin.site.register(Account) admin.site.register(AccountFeatureFlag) admin.site.register(Device) -admin.site.register(DataSource) admin.site.register(DeviceOwnership) admin.site.register(MatchingAlgorithm) admin.site.register(ExternalCredentials)