🐛 #420 - fix: turn off broken sorting in edit page and fix bug causin…

…g missing zaken on edit page

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

@@ -0,0 +1,49 @@
+# fmt: off
+from django.test import tag
+
+from openarchiefbeheer.utils.tests.e2e import browser_page
+from openarchiefbeheer.utils.tests.gherkin import GherkinLikeTestCase
+
+
+@tag("e2e")
+class Issue420EditDestructionList(GherkinLikeTestCase):
+    async def test_scenario_user_edits_multi_page_destruction_list(self):
+        async with browser_page() as page:
+            await self.given.record_manager_exists()
+            await self.given.zaken_are_indexed(300, recreate=True)
+            reviewer = await self.given.reviewer_exists()
+
+            # Create destruction list
+            await self.when.record_manager_logs_in(page)
+            await self.when.user_clicks_button(page, "Vernietigingslijst opstellen", 0)
+            await self.then.path_should_be(page, "/destruction-lists/create")
+
+            await self.when.user_clicks_button(page, "volgende")
+            await self.when.user_clicks_checkbox(page, "(de)selecteer 100 rijen")  # All zaken on second page
+            await self.when.user_clicks_button(page, "volgende")
+            await self.then.path_should_be(page, "/destruction-lists/create?page=3")
+
+            await self.when.user_selects_zaak(page, "ZAAK-200")  # First zaak on third (last) page
+            await self.when.user_clicks_button(page, "Vernietigingslijst opstellen", 1)
+
+            await self.when.user_fills_form_field(page, "Naam", "Destruction list to edit")
+            await self.when.user_fills_form_field(page, "Reviewer", reviewer.username)
+            await self.when.user_clicks_button(page, "Vernietigingslijst opstellen", 2)
+            await self.then.path_should_be(page, "/destruction-lists")
+
+            # View destruction list
+            destruction_list = await self.then.list_should_exist(page, "Destruction list to edit")
+            await self.when.user_clicks_button(page, "Destruction list to edit")
+            await self.then.path_should_be(page, f"/destruction-lists/{str(destruction_list.uuid)}")
+
+            await self.when.user_clicks_button(page, "2")
+            await self.then.path_should_be(page, f"/destruction-lists/{str(destruction_list.uuid)}?page=2")
+            await self.then.page_should_contain_text(page, "ZAAK-200")
+
+            # Edit destruction list
+            await self.when.user_clicks_button(page, "Bewerken", 1)
+            await self.then.path_should_be(page, f"/destruction-lists/{str(destruction_list.uuid)}?page=1&is_editing=true")
+            await self.when.user_clicks_button(page, "2")
+            await self.then.path_should_be(page, f"/destruction-lists/{str(destruction_list.uuid)}?page=2&is_editing=true")
+            await self.then.zaak_should_be_selected(page, "ZAAK-200")
+            await self.then.not_.zaak_should_be_selected(page, "ZAAK-100")  # First unselected zaak

backend/src/openarchiefbeheer/utils/tests/gherkin.py

@@ -5,6 +5,7 @@
 from playwright.async_api import expect
 
 from openarchiefbeheer.accounts.tests.factories import UserFactory
+from openarchiefbeheer.destruction.models import DestructionList
 from openarchiefbeheer.destruction.tests.factories import (
     DestructionListAssigneeFactory,
     DestructionListFactory,
@@ -268,17 +269,23 @@ async def _get_or_create(self, factory, **kwargs):
             except factory._meta.model.DoesNotExist:
                 return await self._factory_create(factory, **kwargs)
 
-        async def _get_or_create_batch(self, factory, amount, **kwargs):
+        async def _get_or_create_batch(self, factory, amount, recreate=False, **kwargs):
             # Remove any traits of the factory
             orm_params = {
                 key: value
                 for key, value in kwargs.items()
                 if key not in factory._meta.parameters
             }
 
-            queryset = await self._orm_filter(factory._meta.model, **orm_params)
-            if queryset:
-                return queryset
+            if recreate:
+                await factory._meta.model.objects.all().adelete()
+                factory.reset_sequence()
+
+            else:
+                queryset = await self._orm_filter(factory._meta.model, **orm_params)
+                if queryset:
+                    return queryset
+
             return await self._factory_create_batch(factory, amount, **kwargs)
 
     class When:
@@ -344,6 +351,12 @@ async def user_clicks_checkbox(self, page, name, index=0):
         async def user_clicks_radio(self, page, name, index=0):
             await self._user_clicks("radio", page, name, index=index)
 
+        async def user_selects_zaak(self, page, identificatie):
+            locator = page.get_by_role(
+                "row", name=f"(de)selecteer rij {identificatie}"
+            ).get_by_label("(de)selecteer rij")
+            await locator.click()
+
         async def _user_clicks(self, role, page, name, index=0):
             locator = page.get_by_role(role, name=name)
             await locator.first.wait_for()
@@ -419,6 +432,13 @@ async def inverted_method(*args, **kwargs):
 
             return InvertedThen(self)
 
+        async def list_should_exist(self, page, name):
+            @sync_to_async()
+            def get_list():
+                return DestructionList.objects.get(name=name)
+
+            return await get_list()
+
         async def list_should_have_assignee(self, page, destruction_list, assignee):
             @sync_to_async()
             def refresh_list():
@@ -451,9 +471,8 @@ def get_number_of_items():
             self.testcase.assertEqual(number_of_items, count)
 
         async def page_should_contain_text(self, page, text):
-            locator = page.get_by_text(text)
-            count = await locator.count()
-            self.testcase.assertTrue(bool(count), f"{text} not found in {page}")
+            locator = page.get_by_text(text).nth(0)
+            await expect(locator).to_be_attached()
 
         async def path_should_be(self, page, path):
             await self.url_should_be(page, self.testcase.live_server_url + path)
@@ -464,6 +483,12 @@ async def url_should_be(self, page, url):
         async def url_should_contain_text(self, page, text):
             await expect(page).to_have_url(re.compile(text))
 
+        async def zaak_should_be_selected(self, page, identificatie):
+            locator = page.get_by_role(
+                "row", name=f"(de)selecteer rij {identificatie}"
+            ).get_by_label("(de)selecteer rij")
+            await expect(locator).to_be_checked()
+
         async def zaaktype_filters_are(self, page, expected_filters):
             select = page.get_by_label('filter veld "zaaktype"')
 

backend/src/openarchiefbeheer/zaken/api/filtersets.py

@@ -206,16 +206,20 @@ def filter_not_in_destruction_list_except(
             destruction_list__uuid=value
         ).values_list("zaak__url", flat=True)
 
-        return (
-            queryset.exclude(url__in=Subquery(zaken_to_exclude))
-            .annotate(
-                in_exception_list=Case(
-                    When(url__in=Subquery(exception_list), then=Value(True))
-                )
+        qs = queryset.exclude(url__in=Subquery(zaken_to_exclude)).annotate(
+            in_exception_list=Case(
+                When(url__in=Subquery(exception_list), then=Value(True))
             )
-            .order_by("in_exception_list")
         )
 
+        # Allow edit view to check for selected zaken (destruction list items) based on index.
+        # This requires the zaken (with filter_not_in_destruction_list_except) and destruction list items endpoint (with
+        # filter_in_destruction_list) to start with exactly the same zaken (zaken endpoint starts with zaken on
+        # destruction list).
+        #
+        # FIXME: This won't work if the ordering is overridden with a custom value.
+        return qs.order_by("in_exception_list", "pk")
+
     def filter_in_review(
         self, queryset: QuerySet[Zaak], name: str, value: Decimal
     ) -> QuerySet[Zaak]:

frontend/src/pages/destructionlist/abstract/BaseListView.tsx

@@ -44,6 +44,7 @@ export type BaseListViewProps = React.PropsWithChildren<{
   allowSelectAllPages?: boolean;
   selectionActions?: ButtonProps[];
   initiallySelectedZakenOnPage?: Zaak[];
+  sortable?: boolean;
 
   extraFields?: TypedField[];
   filterSelectionZaken?: ZaakSelectionZaakFilter;
@@ -69,6 +70,7 @@ export function BaseListView({
   allowSelectAllPages,
   selectionActions,
   initiallySelectedZakenOnPage = [],
+  sortable = true,
 
   extraFields,
   filterSelectionZaken,
@@ -196,7 +198,7 @@ export function BaseListView({
         loading: state === "loading",
         objectList: objectList,
         page,
-        sort: sort,
+        sort: sortable && sort,
         selected: selectable
           ? (selectedZakenOnPage as unknown as AttributeData[])
           : [],

frontend/src/pages/destructionlist/detail/components/DestructionListEdit/DestructionListEdit.tsx

@@ -180,6 +180,7 @@ export function DestructionListEdit() {
       secondaryNavigationItems={secondaryNavigationItems}
       selectable={editingState}
       selectionActions={selectionActions}
+      sortable={false}
       storageKey={storageKey}
       onClearZaakSelection={handleClearSelection}
     ></BaseListView>