Merge pull request #671 from maykinmedia/issue/#667-ordering

🧪 #667 - test: add (failing) tests for ordering

backend/src/openarchiefbeheer/destruction/api/backends.py

@@ -1,5 +1,6 @@
 from django_filters.rest_framework import DjangoFilterBackend
 from django_filters.utils import translate_validation
+from rest_framework.filters import OrderingFilter
 
 
 class NestedFilterBackend(DjangoFilterBackend):
@@ -35,3 +36,77 @@ def get_nested_filterset(self, request, view):
             "request": request,
         }
         return nested_filterset_class(**kwargs)
+
+
+class NestedOrderingFilterBackend(OrderingFilter):
+    def get_valid_fields(self, queryset, view, context={}) -> list[tuple[str, str]]:
+        valid_fields = getattr(
+            view, context.get("ordering_fields_view_attr", "ordering_fields"), []
+        )
+
+        if valid_fields is None:
+            # Default to allowing filtering on serializer fields
+            return self.get_default_valid_fields(queryset, view, {})
+
+        elif valid_fields == "__all__":
+            # View explicitly allows filtering on any model field
+            valid_fields = [
+                (field.name, field.verbose_name)
+                for field in queryset.model._meta.fields
+            ]
+            valid_fields += [
+                (key, key.title().split("__")) for key in queryset.query.annotations
+            ]
+        else:
+            valid_fields = [
+                (item, item) if isinstance(item, str) else item for item in valid_fields
+            ]
+        return valid_fields
+
+    def get_valid_fields_keys(self, valid_fields) -> list[str]:
+        return [item[0] for item in valid_fields]
+
+    def is_term_valid(self, term, valid_fields) -> bool:
+        if term.startswith("-"):
+            term = term[1:]
+        return term in valid_fields
+
+    def get_ordering(self, request, queryset, view) -> list[str]:
+        base_ordering_param = (
+            f"{view.nested_ordering_prefix}-{self.ordering_param}"
+            if view.nested_ordering_prefix
+            else self.ordering_param
+        )
+        base_params = request.query_params.get(base_ordering_param)
+        formatted_fields = []
+        if base_params:
+            fields = [param.strip() for param in base_params.split(",")]
+            valid_fields = self.get_valid_fields_keys(
+                self.get_valid_fields(queryset, view)
+            )
+            formatted_fields = [
+                term for term in fields if self.is_term_valid(term, valid_fields)
+            ]
+
+        nested_params = request.query_params.get(self.ordering_param)
+        if nested_params:
+            fields = [param.strip() for param in nested_params.split(",")]
+            nested_queryset = view.nested_ordering_model.objects.all()
+            valid_fields = self.get_valid_fields_keys(
+                self.get_valid_fields(
+                    nested_queryset,
+                    view,
+                    context={"ordering_fields_view_attr": "nested_ordering_fields"},
+                )
+            )
+
+            for term in fields:
+                if not self.is_term_valid(term, valid_fields):
+                    continue
+
+                if is_reverse := term.startswith("-"):
+                    term = term[1:]
+                formatted_fields.append(
+                    f"{"-" if is_reverse else ""}{view.nested_ordering_relation_field}__{term}"
+                )
+        return formatted_fields

backend/src/openarchiefbeheer/destruction/api/viewsets.py

@@ -22,6 +22,7 @@
 from openarchiefbeheer.logging import logevent
 from openarchiefbeheer.utils.paginators import PageNumberPagination
 from openarchiefbeheer.zaken.api.filtersets import ZaakFilterSet
+from openarchiefbeheer.zaken.models import Zaak
 
 from ..constants import DestructionListItemAction, InternalStatus, ListRole, ListStatus
 from ..models import (
@@ -36,7 +37,7 @@
 )
 from ..tasks import delete_destruction_list
 from ..utils import process_new_reviewer
-from .backends import NestedFilterBackend
+from .backends import NestedFilterBackend, NestedOrderingFilterBackend
 from .filtersets import (
     DestructionListCoReviewFilterset,
     DestructionListFilterset,
@@ -465,12 +466,17 @@ class DestructionListItemsViewSet(
     viewsets.GenericViewSet,
 ):
     serializer_class = DestructionListItemReadSerializer
-    filter_backends = (NestedFilterBackend,)
+    filter_backends = (NestedFilterBackend, NestedOrderingFilterBackend)
     filterset_class = DestructionListItemFilterset
     filterset_kwargs = {"prefix": "item"}
     nested_filterset_class = ZaakFilterSet
     nested_filterset_relation_field = "zaak"
     pagination_class = PageNumberPagination
+    ordering_fields = "__all__"
+    nested_ordering_fields = "__all__"
+    nested_ordering_model = Zaak
+    nested_ordering_relation_field = "zaak"
+    nested_ordering_prefix = "item"
 
     def get_queryset(self):
         review_response_items = ReviewItemResponse.objects.filter(
@@ -593,10 +599,15 @@ class DestructionListItemReviewViewSet(mixins.ListModelMixin, viewsets.GenericVi
         "destruction_list_item__zaak"
     )
     filterset_class = DestructionListReviewItemFilterset
-    filter_backends = (NestedFilterBackend,)
+    filter_backends = (NestedFilterBackend, NestedOrderingFilterBackend)
     filterset_kwargs = {"prefix": "item-review"}
     nested_filterset_class = ZaakFilterSet
     nested_filterset_relation_field = "destruction_list_item__zaak"
+    ordering_fields = "__all__"
+    nested_ordering_fields = "__all__"
+    nested_ordering_model = Zaak
+    nested_ordering_relation_field = "destruction_list_item__zaak"
+    nested_ordering_prefix = "item-review"
 
 
 @extend_schema_view(

backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_667_sorting_seems_to_be_broken.py

@@ -0,0 +1,221 @@
+# fmt: off
+from django.test import tag
+
+from asgiref.sync import sync_to_async
+
+from openarchiefbeheer.accounts.tests.factories import UserFactory
+from openarchiefbeheer.destruction.constants import (
+    ListRole,
+    ListStatus,
+    ReviewDecisionChoices,
+)
+from openarchiefbeheer.destruction.tests.factories import (
+    DestructionListFactory,
+    DestructionListItemFactory,
+    DestructionListItemReviewFactory,
+    DestructionListReviewFactory,
+)
+from openarchiefbeheer.utils.tests.e2e import browser_page
+from openarchiefbeheer.utils.tests.gherkin import GherkinLikeTestCase
+from openarchiefbeheer.zaken.tests.factories import ZaakFactory
+
+
+@tag("e2e")
+@tag("gh-667")
+class Issue667CancelFilteredEditMode(GherkinLikeTestCase):
+    async def test_scenario_sort_destruction_list_create_page(self):
+        async with browser_page() as page:
+            await self.given.record_manager_exists()
+            await self.given.zaken_are_indexed(amount=3, recreate=True)
+
+            await self.when.record_manager_logs_in(page)
+            await self.when.user_clicks_button(page, "Vernietigingslijst opstellen")
+            await self.then.zaken_should_have_order(page, ["ZAAK-0", "ZAAK-1", "ZAAK-2"])
+
+            await self.when.user_clicks_button(page, "Identificatie")
+            await self.then.path_should_be(page, "/destruction-lists/create?ordering=identificatie")
+            await self.then.zaken_should_have_order(page, ["ZAAK-0", "ZAAK-1", "ZAAK-2"])
+
+            await self.when.user_clicks_button(page, "Identificatie")
+            await self.then.path_should_be(page, "/destruction-lists/create?ordering=-identificatie")
+            await self.then.zaken_should_have_order(page, ["ZAAK-2", "ZAAK-1", "ZAAK-0"])
+
+    async def test_scenario_sort_destruction_list_detail_page(self):
+        async with browser_page() as page:
+            await self.given.record_manager_exists()
+            zaken = await self.given.zaken_are_indexed(amount=3, recreate=True)
+            await self.given.list_exists(
+                name="Destruction list to sort",
+                status=ListStatus.new,
+                uuid="00000000-0000-0000-0000-000000000000",
+                zaken=zaken)
+
+            await self.when.record_manager_logs_in(page)
+            await self.when.user_clicks_button(page, "Destruction list to sort")
+            await self.then.zaken_should_have_order(page, ["ZAAK-0", "ZAAK-1", "ZAAK-2"])
+
+            await self.when.user_clicks_button(page, "Identificatie")
+            await self.then.path_should_be(page, "/destruction-lists/00000000-0000-0000-0000-000000000000/edit?ordering=identificatie")
+            await self.then.zaken_should_have_order(page, ["ZAAK-0", "ZAAK-1", "ZAAK-2"])
+
+            await self.when.user_clicks_button(page, "Identificatie")
+            await self.then.path_should_be(page, "/destruction-lists/00000000-0000-0000-0000-000000000000/edit?ordering=-identificatie")
+            await self.then.zaken_should_have_order(page, ["ZAAK-2", "ZAAK-1", "ZAAK-0"])
+
+    async def test_scenario_sort_destruction_list_edit_page(self):
+        async with browser_page() as page:
+            await self.given.record_manager_exists()
+            zaken = await self.given.zaken_are_indexed(amount=3, recreate=True)
+            await self.given.list_exists(
+                name="Destruction list to sort",
+                status=ListStatus.new,
+                uuid="00000000-0000-0000-0000-000000000000",
+                zaken=zaken)
+
+            await self.when.record_manager_logs_in(page)
+            await self.when.user_clicks_button(page, "Destruction list to sort")
+            await self.when.user_clicks_button(page, "Bewerken", 2)
+            await self.then.zaken_should_have_order(page, ["ZAAK-0", "ZAAK-1", "ZAAK-2"])
+            await self.then.path_should_be(page, "/destruction-lists/00000000-0000-0000-0000-000000000000/edit?page=1&is_editing=true")
+
+            await self.when.user_clicks_button(page, "Identificatie")
+            await self.then.path_should_be(page, "/destruction-lists/00000000-0000-0000-0000-000000000000/edit?page=1&is_editing=true&ordering=identificatie")
+            await self.then.zaken_should_have_order(page, ["ZAAK-0", "ZAAK-1", "ZAAK-2"])
+
+            await self.when.user_clicks_button(page, "Identificatie")
+            await self.then.path_should_be(page, "/destruction-lists/00000000-0000-0000-0000-000000000000/edit?page=1&is_editing=true&ordering=-identificatie")
+            await self.then.zaken_should_have_order(page, ["ZAAK-2", "ZAAK-1", "ZAAK-0"])
+
+    async def test_scenario_sort_destruction_list_review_page(self):
+        async with browser_page() as page:
+            record_manager = await self.given.record_manager_exists()
+            reviewer = await self.given.reviewer_exists()
+            archivist = await self.given.archivist_exists()
+
+            assignees = [
+                await self.given.assignee_exists(user=record_manager, role=ListRole.author),
+                await self.given.assignee_exists(user=reviewer, role=ListRole.main_reviewer),
+                await self.given.assignee_exists(user=archivist, role=ListRole.archivist),
+            ]
+
+            zaken = await self.given.zaken_are_indexed(amount=3, recreate=True)
+            await self.given.list_exists(
+                assignee=reviewer,
+                assignees=assignees,
+                uuid="00000000-0000-0000-0000-000000000000",
+                name="Destruction list to sort",
+                status=ListStatus.ready_to_review,
+                zaken=zaken
+            )
+
+            await self.when.reviewer_logs_in(page)
+            await self.when.user_clicks_button(page, "Destruction list to sort")
+            await self.then.zaken_should_have_order(page, ["ZAAK-0", "ZAAK-1", "ZAAK-2"])
+            await self.then.path_should_be(page, "/destruction-lists/00000000-0000-0000-0000-000000000000/review")
+
+            await self.when.user_clicks_button(page, "Identificatie")
+            await self.then.path_should_be(page, "/destruction-lists/00000000-0000-0000-0000-000000000000/review?ordering=identificatie")
+            await self.then.zaken_should_have_order(page, ["ZAAK-0", "ZAAK-1", "ZAAK-2"])
+
+            await self.when.user_clicks_button(page, "Identificatie")
+            await self.then.path_should_be(page, "/destruction-lists/00000000-0000-0000-0000-000000000000/review?ordering=-identificatie")
+            await self.then.zaken_should_have_order(page, ["ZAAK-2", "ZAAK-1", "ZAAK-0"])
+
+    async def test_scenario_sort_destruction_list_process_review_page(self):
+        @sync_to_async
+        def create_data():
+            record_manager = UserFactory.create(
+                password="ANic3Password", post__can_start_destruction=True
+            )
+            destruction_list = DestructionListFactory.create(
+                assignee=record_manager,
+                author=record_manager,
+                uuid="00000000-0000-0000-0000-000000000000",
+                status=ListStatus.changes_requested
+            )
+            zaak1 = ZaakFactory.create(
+                identificatie="ZAAK-0",
+                post___expand={
+                    "zaaktype": {
+                        "identificatie": "ZAAKTYPE-01",
+                        "omschrijving": "ZAAKTYPE-01",
+                        "url": "http://catalogue-api.nl/zaaktypen/111-111-111",
+                        "selectielijst_procestype": {
+                            "url": "http://selectielijst.nl/api/v1/procestype/1"
+                        },
+                        "versiedatum": "2024-01-01"
+                    }
+                },
+            )
+            item1 = DestructionListItemFactory.create(zaak=zaak1, destruction_list=destruction_list)
+
+            zaak2 = ZaakFactory.create(
+                identificatie="ZAAK-1",
+                post___expand={
+                    "zaaktype": {
+                        "identificatie": "ZAAKTYPE-02",
+                        "omschrijving": "ZAAKTYPE-02",
+                        "url": "http://catalogue-api.nl/zaaktypen/222-222-222",
+                        "selectielijst_procestype": {
+                            "url": "http://selectielijst.nl/api/v1/procestype/1"
+                        },
+                        "versiedatum": "2024-01-01"
+                    }
+                },
+            )
+            item2 = DestructionListItemFactory.create(zaak=zaak2, destruction_list=destruction_list)
+
+            zaak3 = ZaakFactory.create(
+                identificatie="ZAAK-2",
+                post___expand={
+                    "zaaktype": {
+                        "identificatie": "ZAAKTYPE-03",
+                        "omschrijving": "ZAAKTYPE-03",
+                        "url": "http://catalogue-api.nl/zaaktypen/333-333-333",
+                        "selectielijst_procestype": {
+                            "url": "http://selectielijst.nl/api/v1/procestype/1"
+                        },
+                        "versiedatum": "2024-01-01"
+                    }
+                },
+            )
+            item3 = DestructionListItemFactory.create(zaak=zaak3, destruction_list=destruction_list)
+
+            review = DestructionListReviewFactory.create(
+                destruction_list=destruction_list,
+                decision=ReviewDecisionChoices.rejected,
+            )
+            DestructionListItemReviewFactory.create(
+                destruction_list=destruction_list,
+                destruction_list_item=item1,
+                review=review
+            )
+            DestructionListItemReviewFactory.create(
+                destruction_list=destruction_list,
+                destruction_list_item=item2,
+                review=review
+            )
+            DestructionListItemReviewFactory.create(
+                destruction_list=destruction_list,
+                destruction_list_item=item3,
+                review=review
+            )
+
+            self.destruction_list = destruction_list
+
+        async with browser_page() as page:
+            await self.given.data_exists(create_data)
+            await self.when.user_logs_in(page, self.destruction_list.assignee)
+            await self.then.path_should_be(page, "/destruction-lists")
+
+            await self.when.user_clicks_button(page, self.destruction_list.name)
+            await self.then.path_should_be(page, "/destruction-lists/00000000-0000-0000-0000-000000000000/process-review")
+            await self.then.zaken_should_have_order(page, ["ZAAK-0", "ZAAK-1", "ZAAK-2"])
+
+            await self.when.user_clicks_button(page, "Identificatie")
+            await self.then.path_should_be(page, "/destruction-lists/00000000-0000-0000-0000-000000000000/process-review?ordering=identificatie")
+            await self.then.zaken_should_have_order(page, ["ZAAK-0", "ZAAK-1", "ZAAK-2"])
+
+            await self.when.user_clicks_button(page, "Identificatie")
+            await self.then.path_should_be(page, "/destruction-lists/00000000-0000-0000-0000-000000000000/process-review?ordering=-identificatie")
+            await self.then.zaken_should_have_order(page, ["ZAAK-2", "ZAAK-1", "ZAAK-0"])