From 73dd7cfeab6404e9462d0fe9e387152bfab675aa Mon Sep 17 00:00:00 2001 From: Julian Roeland Date: Fri, 20 Dec 2024 11:51:54 +0100 Subject: [PATCH 1/5] :bug: #568 - fix: correct count --- .../destruction/api/serializers.py | 49 +++++++++---------- .../openarchiefbeheer/destruction/models.py | 34 +++++-------- .../e2e/issues/test_568_correct_count.py | 31 ++++++++++++ 3 files changed, 65 insertions(+), 49 deletions(-) create mode 100644 backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_568_correct_count.py diff --git a/backend/src/openarchiefbeheer/destruction/api/serializers.py b/backend/src/openarchiefbeheer/destruction/api/serializers.py index fc825618..88ebcf5b 100644 --- a/backend/src/openarchiefbeheer/destruction/api/serializers.py +++ b/backend/src/openarchiefbeheer/destruction/api/serializers.py @@ -3,42 +3,29 @@ from django.db import transaction from django.db.models import F, Q, QuerySet from django.utils.translation import gettext_lazy as _ - from drf_spectacular.plumbing import build_basic_type from drf_spectacular.utils import OpenApiTypes, extend_schema_field -from requests.exceptions import HTTPError -from rest_framework import serializers -from rest_framework.exceptions import ValidationError -from rest_framework.relations import SlugRelatedField - from openarchiefbeheer.accounts.api.serializers import UserSerializer from openarchiefbeheer.accounts.models import User from openarchiefbeheer.logging import logevent from openarchiefbeheer.zaken.api.filtersets import ZaakFilterSet from openarchiefbeheer.zaken.api.serializers import ZaakSerializer from openarchiefbeheer.zaken.models import Zaak -from openarchiefbeheer.zaken.utils import retrieve_selectielijstklasse_resultaat +from openarchiefbeheer.zaken.utils import \ + retrieve_selectielijstklasse_resultaat +from requests.exceptions import HTTPError +from rest_framework import serializers +from rest_framework.exceptions import ValidationError +from rest_framework.relations import SlugRelatedField from ..api.constants import MAX_NUMBER_CO_REVIEWERS -from ..constants import ( - DestructionListItemAction, - InternalStatus, - ListItemStatus, - ListRole, - ListStatus, - ReviewDecisionChoices, - ZaakActionType, -) -from ..models import ( - DestructionList, - DestructionListAssignee, - DestructionListCoReview, - DestructionListItem, - DestructionListItemReview, - DestructionListReview, - ReviewItemResponse, - ReviewResponse, -) +from ..constants import (DestructionListItemAction, InternalStatus, + ListItemStatus, ListRole, ListStatus, + ReviewDecisionChoices, ZaakActionType) +from ..models import (DestructionList, DestructionListAssignee, + DestructionListCoReview, DestructionListItem, + DestructionListItemReview, DestructionListReview, + ReviewItemResponse, ReviewResponse) from ..signals import co_reviewers_added from ..tasks import process_review_response @@ -426,6 +413,11 @@ class DestructionListReadSerializer(serializers.ModelSerializer): assignees = DestructionListAssigneeReadSerializer(many=True) author = UserSerializer(read_only=True) assignee = UserSerializer(read_only=True) + deletable_items_count = serializers.SerializerMethodField( + help_text=_("Number of items to be deleted"), + allow_null=True, + + ) class Meta: model = DestructionList @@ -442,8 +434,13 @@ class Meta: "created", "status_changed", "planned_destruction_date", + "deletable_items_count" ) + def get_deletable_items_count(self, instance) -> int: + succeeded_count = instance.items.filter(processing_status=InternalStatus.succeeded).count() + total_count = instance.items.all().count() + return total_count - succeeded_count class ZakenReviewSerializer(serializers.Serializer): zaak_url = serializers.URLField( diff --git a/backend/src/openarchiefbeheer/destruction/models.py b/backend/src/openarchiefbeheer/destruction/models.py index e37d1366..92bd69f2 100644 --- a/backend/src/openarchiefbeheer/destruction/models.py +++ b/backend/src/openarchiefbeheer/destruction/models.py @@ -11,29 +11,19 @@ from django.db.models import QuerySet from django.utils import timezone from django.utils.translation import gettext_lazy as _ - -from privates.fields import PrivateMediaFileField -from slugify import slugify -from timeline_logger.models import TimelineLog - from openarchiefbeheer.accounts.models import User from openarchiefbeheer.config.models import ArchiveConfig from openarchiefbeheer.utils.results_store import ResultStore -from openarchiefbeheer.zaken.utils import ( - delete_zaak_and_related_objects, - get_zaak_metadata, -) +from openarchiefbeheer.zaken.utils import (delete_zaak_and_related_objects, + get_zaak_metadata) +from privates.fields import PrivateMediaFileField +from slugify import slugify +from timeline_logger.models import TimelineLog from .assignment_logic import STATE_MANAGER -from .constants import ( - DestructionListItemAction, - InternalStatus, - ListItemStatus, - ListRole, - ListStatus, - ReviewDecisionChoices, - ZaakActionType, -) +from .constants import (DestructionListItemAction, InternalStatus, + ListItemStatus, ListRole, ListStatus, + ReviewDecisionChoices, ZaakActionType) from .exceptions import ZaakArchiefactiedatumInFuture, ZaakNotFound from .managers import DestructionListManager @@ -244,11 +234,9 @@ def generate_destruction_report(self) -> None: self.save() def create_report_zaak(self) -> None: - from .utils import ( - attach_report_to_zaak, - create_eio_destruction_report, - create_zaak_for_report, - ) + from .utils import (attach_report_to_zaak, + create_eio_destruction_report, + create_zaak_for_report) if self.processing_status == InternalStatus.succeeded: return diff --git a/backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_568_correct_count.py b/backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_568_correct_count.py new file mode 100644 index 00000000..eea0d3e8 --- /dev/null +++ b/backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_568_correct_count.py @@ -0,0 +1,31 @@ +import asyncio + +from asgiref.sync import sync_to_async +from django.test import tag +from openarchiefbeheer.utils.tests.e2e import browser_page +from openarchiefbeheer.utils.tests.gherkin import GherkinLikeTestCase + +from ....constants import InternalStatus, ListStatus + + +@tag("e2e") +@tag("gh-568") +class Issue568CorrectCount(GherkinLikeTestCase): + async def test_destruction_fails_with_incorrect_count(self): + async with browser_page() as page: + zaken = await self.given.zaken_are_indexed(amount=5) + await self.given.record_manager_exists() + + destruction_list = await self.given.list_exists(name="Destruction list to check count for", status=ListStatus.ready_to_delete, processing_status=InternalStatus.failed, zaken=[]) + await self.given.list_item_exists(destruction_list=destruction_list, processing_status=InternalStatus.new, zaak=zaken[0]) + await self.given.list_item_exists(destruction_list=destruction_list, processing_status=InternalStatus.failed, zaak=zaken[1]) + await self.given.list_item_exists(destruction_list=destruction_list, processing_status=InternalStatus.processing, zaak=zaken[2]) + await self.given.list_item_exists(destruction_list=destruction_list, processing_status=InternalStatus.queued, zaak=zaken[3]) + await self.given.list_item_exists(destruction_list=destruction_list, processing_status=InternalStatus.succeeded, zaak=zaken[4]) + + await self.when.record_manager_logs_in(page) + await self.then.path_should_be(page, "/destruction-lists") + + await self.when.user_clicks_button(page, "Destruction list to check count for") + await self.when.user_clicks_button(page, "Vernietigen herstarten") + await self.then.page_should_contain_text(page, "U staat op het punt om 4 zaken definitief te vernietigen") From ec34130d4a0cc71eec072fb6238bb8d068903c2f Mon Sep 17 00:00:00 2001 From: Julian Roeland Date: Fri, 20 Dec 2024 12:10:46 +0100 Subject: [PATCH 2/5] :bug: - fix: count frontend --- frontend/src/fixtures/destructionList.ts | 1 + frontend/src/lib/api/destructionLists.ts | 3 ++- .../destructionlist/detail/hooks/useSecondaryNavigation.tsx | 3 +-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/frontend/src/fixtures/destructionList.ts b/frontend/src/fixtures/destructionList.ts index a1457bbe..f0ef8782 100644 --- a/frontend/src/fixtures/destructionList.ts +++ b/frontend/src/fixtures/destructionList.ts @@ -20,6 +20,7 @@ export const FIXTURE_DESTRUCTION_LIST: DestructionList = { assignee: defaultAssignees[0].user, created: "2024-07-11T16:57", statusChanged: "2024-07-11:16:57", + deletableItemsCount: 0, }; export const destructionListFactory = createObjectFactory( diff --git a/frontend/src/lib/api/destructionLists.ts b/frontend/src/lib/api/destructionLists.ts index 11f921f3..eedf0d29 100644 --- a/frontend/src/lib/api/destructionLists.ts +++ b/frontend/src/lib/api/destructionLists.ts @@ -13,12 +13,13 @@ export type DestructionList = { author: User; comment: string; containsSensitiveInfo: boolean; - plannedDestructionDate: string | null; created: string; + plannedDestructionDate: string | null; name: string; status: DestructionListStatus; processingStatus: ProcessingStatus; statusChanged: string | null; + deletableItemsCount: number; uuid: string; }; diff --git a/frontend/src/pages/destructionlist/detail/hooks/useSecondaryNavigation.tsx b/frontend/src/pages/destructionlist/detail/hooks/useSecondaryNavigation.tsx index 6471f736..ee066b1b 100644 --- a/frontend/src/pages/destructionlist/detail/hooks/useSecondaryNavigation.tsx +++ b/frontend/src/pages/destructionlist/detail/hooks/useSecondaryNavigation.tsx @@ -63,7 +63,6 @@ export function useSecondaryNavigation(): ToolbarItem[] { } = useRouteLoaderData( "destruction-list:detail", ) as DestructionListDetailContext; - const confirm = useConfirm(); const prompt = usePrompt(); const formDialog = useFormDialog(); @@ -284,7 +283,7 @@ export function useSecondaryNavigation(): ToolbarItem[] { onClick: () => formDialog( "Zaken definitief vernietigen", - `U staat op het punt om ${destructionListItems.count} zaken definitief te vernietigen`, + `U staat op het punt om ${destructionList.deletableItemsCount} zaken definitief te vernietigen`, [ { label: "Type naam van de lijst ter bevestiging", From 5f829ceed9ef05a18286e843f941f4cfb09e73df Mon Sep 17 00:00:00 2001 From: Julian Roeland Date: Fri, 20 Dec 2024 12:12:45 +0100 Subject: [PATCH 3/5] :art: - refactor: formatting/sorting --- .../destruction/api/serializers.py | 37 ++++++++++----- .../openarchiefbeheer/destruction/models.py | 26 ++++++---- .../e2e/issues/test_568_correct_count.py | 47 +++++++++++++++---- 3 files changed, 81 insertions(+), 29 deletions(-) diff --git a/backend/src/openarchiefbeheer/destruction/api/serializers.py b/backend/src/openarchiefbeheer/destruction/api/serializers.py index 88ebcf5b..3ffc41c0 100644 --- a/backend/src/openarchiefbeheer/destruction/api/serializers.py +++ b/backend/src/openarchiefbeheer/destruction/api/serializers.py @@ -11,21 +11,32 @@ from openarchiefbeheer.zaken.api.filtersets import ZaakFilterSet from openarchiefbeheer.zaken.api.serializers import ZaakSerializer from openarchiefbeheer.zaken.models import Zaak -from openarchiefbeheer.zaken.utils import \ - retrieve_selectielijstklasse_resultaat +from openarchiefbeheer.zaken.utils import retrieve_selectielijstklasse_resultaat from requests.exceptions import HTTPError from rest_framework import serializers from rest_framework.exceptions import ValidationError from rest_framework.relations import SlugRelatedField from ..api.constants import MAX_NUMBER_CO_REVIEWERS -from ..constants import (DestructionListItemAction, InternalStatus, - ListItemStatus, ListRole, ListStatus, - ReviewDecisionChoices, ZaakActionType) -from ..models import (DestructionList, DestructionListAssignee, - DestructionListCoReview, DestructionListItem, - DestructionListItemReview, DestructionListReview, - ReviewItemResponse, ReviewResponse) +from ..constants import ( + DestructionListItemAction, + InternalStatus, + ListItemStatus, + ListRole, + ListStatus, + ReviewDecisionChoices, + ZaakActionType, +) +from ..models import ( + DestructionList, + DestructionListAssignee, + DestructionListCoReview, + DestructionListItem, + DestructionListItemReview, + DestructionListReview, + ReviewItemResponse, + ReviewResponse, +) from ..signals import co_reviewers_added from ..tasks import process_review_response @@ -416,7 +427,6 @@ class DestructionListReadSerializer(serializers.ModelSerializer): deletable_items_count = serializers.SerializerMethodField( help_text=_("Number of items to be deleted"), allow_null=True, - ) class Meta: @@ -434,14 +444,17 @@ class Meta: "created", "status_changed", "planned_destruction_date", - "deletable_items_count" + "deletable_items_count", ) def get_deletable_items_count(self, instance) -> int: - succeeded_count = instance.items.filter(processing_status=InternalStatus.succeeded).count() + succeeded_count = instance.items.filter( + processing_status=InternalStatus.succeeded + ).count() total_count = instance.items.all().count() return total_count - succeeded_count + class ZakenReviewSerializer(serializers.Serializer): zaak_url = serializers.URLField( required=True, help_text="The URL of the case for which changes are requested." diff --git a/backend/src/openarchiefbeheer/destruction/models.py b/backend/src/openarchiefbeheer/destruction/models.py index 92bd69f2..d8581993 100644 --- a/backend/src/openarchiefbeheer/destruction/models.py +++ b/backend/src/openarchiefbeheer/destruction/models.py @@ -14,16 +14,24 @@ from openarchiefbeheer.accounts.models import User from openarchiefbeheer.config.models import ArchiveConfig from openarchiefbeheer.utils.results_store import ResultStore -from openarchiefbeheer.zaken.utils import (delete_zaak_and_related_objects, - get_zaak_metadata) +from openarchiefbeheer.zaken.utils import ( + delete_zaak_and_related_objects, + get_zaak_metadata, +) from privates.fields import PrivateMediaFileField from slugify import slugify from timeline_logger.models import TimelineLog from .assignment_logic import STATE_MANAGER -from .constants import (DestructionListItemAction, InternalStatus, - ListItemStatus, ListRole, ListStatus, - ReviewDecisionChoices, ZaakActionType) +from .constants import ( + DestructionListItemAction, + InternalStatus, + ListItemStatus, + ListRole, + ListStatus, + ReviewDecisionChoices, + ZaakActionType, +) from .exceptions import ZaakArchiefactiedatumInFuture, ZaakNotFound from .managers import DestructionListManager @@ -234,9 +242,11 @@ def generate_destruction_report(self) -> None: self.save() def create_report_zaak(self) -> None: - from .utils import (attach_report_to_zaak, - create_eio_destruction_report, - create_zaak_for_report) + from .utils import ( + attach_report_to_zaak, + create_eio_destruction_report, + create_zaak_for_report, + ) if self.processing_status == InternalStatus.succeeded: return diff --git a/backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_568_correct_count.py b/backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_568_correct_count.py index eea0d3e8..6f5ec540 100644 --- a/backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_568_correct_count.py +++ b/backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_568_correct_count.py @@ -15,17 +15,46 @@ async def test_destruction_fails_with_incorrect_count(self): async with browser_page() as page: zaken = await self.given.zaken_are_indexed(amount=5) await self.given.record_manager_exists() - - destruction_list = await self.given.list_exists(name="Destruction list to check count for", status=ListStatus.ready_to_delete, processing_status=InternalStatus.failed, zaken=[]) - await self.given.list_item_exists(destruction_list=destruction_list, processing_status=InternalStatus.new, zaak=zaken[0]) - await self.given.list_item_exists(destruction_list=destruction_list, processing_status=InternalStatus.failed, zaak=zaken[1]) - await self.given.list_item_exists(destruction_list=destruction_list, processing_status=InternalStatus.processing, zaak=zaken[2]) - await self.given.list_item_exists(destruction_list=destruction_list, processing_status=InternalStatus.queued, zaak=zaken[3]) - await self.given.list_item_exists(destruction_list=destruction_list, processing_status=InternalStatus.succeeded, zaak=zaken[4]) + + destruction_list = await self.given.list_exists( + name="Destruction list to check count for", + status=ListStatus.ready_to_delete, + processing_status=InternalStatus.failed, + zaken=[], + ) + await self.given.list_item_exists( + destruction_list=destruction_list, + processing_status=InternalStatus.new, + zaak=zaken[0], + ) + await self.given.list_item_exists( + destruction_list=destruction_list, + processing_status=InternalStatus.failed, + zaak=zaken[1], + ) + await self.given.list_item_exists( + destruction_list=destruction_list, + processing_status=InternalStatus.processing, + zaak=zaken[2], + ) + await self.given.list_item_exists( + destruction_list=destruction_list, + processing_status=InternalStatus.queued, + zaak=zaken[3], + ) + await self.given.list_item_exists( + destruction_list=destruction_list, + processing_status=InternalStatus.succeeded, + zaak=zaken[4], + ) await self.when.record_manager_logs_in(page) await self.then.path_should_be(page, "/destruction-lists") - await self.when.user_clicks_button(page, "Destruction list to check count for") + await self.when.user_clicks_button( + page, "Destruction list to check count for" + ) await self.when.user_clicks_button(page, "Vernietigen herstarten") - await self.then.page_should_contain_text(page, "U staat op het punt om 4 zaken definitief te vernietigen") + await self.then.page_should_contain_text( + page, "U staat op het punt om 4 zaken definitief te vernietigen" + ) From 9e5ea4a56ae0fc013261d28cd5563edd76cbf3de Mon Sep 17 00:00:00 2001 From: Julian Roeland Date: Fri, 20 Dec 2024 12:15:02 +0100 Subject: [PATCH 4/5] :art: - refactor: formatting/sorting --- .../openarchiefbeheer/destruction/api/serializers.py | 10 ++++++---- backend/src/openarchiefbeheer/destruction/models.py | 8 +++++--- .../tests/e2e/issues/test_568_correct_count.py | 4 +--- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/backend/src/openarchiefbeheer/destruction/api/serializers.py b/backend/src/openarchiefbeheer/destruction/api/serializers.py index 3ffc41c0..e4065a90 100644 --- a/backend/src/openarchiefbeheer/destruction/api/serializers.py +++ b/backend/src/openarchiefbeheer/destruction/api/serializers.py @@ -3,8 +3,14 @@ from django.db import transaction from django.db.models import F, Q, QuerySet from django.utils.translation import gettext_lazy as _ + from drf_spectacular.plumbing import build_basic_type from drf_spectacular.utils import OpenApiTypes, extend_schema_field +from requests.exceptions import HTTPError +from rest_framework import serializers +from rest_framework.exceptions import ValidationError +from rest_framework.relations import SlugRelatedField + from openarchiefbeheer.accounts.api.serializers import UserSerializer from openarchiefbeheer.accounts.models import User from openarchiefbeheer.logging import logevent @@ -12,10 +18,6 @@ from openarchiefbeheer.zaken.api.serializers import ZaakSerializer from openarchiefbeheer.zaken.models import Zaak from openarchiefbeheer.zaken.utils import retrieve_selectielijstklasse_resultaat -from requests.exceptions import HTTPError -from rest_framework import serializers -from rest_framework.exceptions import ValidationError -from rest_framework.relations import SlugRelatedField from ..api.constants import MAX_NUMBER_CO_REVIEWERS from ..constants import ( diff --git a/backend/src/openarchiefbeheer/destruction/models.py b/backend/src/openarchiefbeheer/destruction/models.py index d8581993..e37d1366 100644 --- a/backend/src/openarchiefbeheer/destruction/models.py +++ b/backend/src/openarchiefbeheer/destruction/models.py @@ -11,6 +11,11 @@ from django.db.models import QuerySet from django.utils import timezone from django.utils.translation import gettext_lazy as _ + +from privates.fields import PrivateMediaFileField +from slugify import slugify +from timeline_logger.models import TimelineLog + from openarchiefbeheer.accounts.models import User from openarchiefbeheer.config.models import ArchiveConfig from openarchiefbeheer.utils.results_store import ResultStore @@ -18,9 +23,6 @@ delete_zaak_and_related_objects, get_zaak_metadata, ) -from privates.fields import PrivateMediaFileField -from slugify import slugify -from timeline_logger.models import TimelineLog from .assignment_logic import STATE_MANAGER from .constants import ( diff --git a/backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_568_correct_count.py b/backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_568_correct_count.py index 6f5ec540..af8204fc 100644 --- a/backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_568_correct_count.py +++ b/backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_568_correct_count.py @@ -1,7 +1,5 @@ -import asyncio - -from asgiref.sync import sync_to_async from django.test import tag + from openarchiefbeheer.utils.tests.e2e import browser_page from openarchiefbeheer.utils.tests.gherkin import GherkinLikeTestCase From 71cacb0a589919d7e93dc12ddf6b59618990b603 Mon Sep 17 00:00:00 2001 From: Julian Roeland Date: Fri, 20 Dec 2024 12:50:04 +0100 Subject: [PATCH 5/5] :ok_hand: - fix: pr fixes --- .../openarchiefbeheer/destruction/api/serializers.py | 4 ++-- .../tests/e2e/issues/test_568_correct_count.py | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/backend/src/openarchiefbeheer/destruction/api/serializers.py b/backend/src/openarchiefbeheer/destruction/api/serializers.py index e4065a90..f57d8fbe 100644 --- a/backend/src/openarchiefbeheer/destruction/api/serializers.py +++ b/backend/src/openarchiefbeheer/destruction/api/serializers.py @@ -449,11 +449,11 @@ class Meta: "deletable_items_count", ) - def get_deletable_items_count(self, instance) -> int: + def get_deletable_items_count(self, instance: DestructionList) -> int: succeeded_count = instance.items.filter( processing_status=InternalStatus.succeeded ).count() - total_count = instance.items.all().count() + total_count = instance.items.filter(status=ListItemStatus.suggested).count() return total_count - succeeded_count diff --git a/backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_568_correct_count.py b/backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_568_correct_count.py index af8204fc..02d400e5 100644 --- a/backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_568_correct_count.py +++ b/backend/src/openarchiefbeheer/destruction/tests/e2e/issues/test_568_correct_count.py @@ -3,7 +3,7 @@ from openarchiefbeheer.utils.tests.e2e import browser_page from openarchiefbeheer.utils.tests.gherkin import GherkinLikeTestCase -from ....constants import InternalStatus, ListStatus +from ....constants import InternalStatus, ListItemStatus, ListStatus @tag("e2e") @@ -11,7 +11,7 @@ class Issue568CorrectCount(GherkinLikeTestCase): async def test_destruction_fails_with_incorrect_count(self): async with browser_page() as page: - zaken = await self.given.zaken_are_indexed(amount=5) + zaken = await self.given.zaken_are_indexed(amount=6) await self.given.record_manager_exists() destruction_list = await self.given.list_exists( @@ -45,6 +45,12 @@ async def test_destruction_fails_with_incorrect_count(self): processing_status=InternalStatus.succeeded, zaak=zaken[4], ) + await self.given.list_item_exists( + destruction_list=destruction_list, + status=ListItemStatus.removed, + processing_status=InternalStatus.new, + zaak=zaken[5], + ) await self.when.record_manager_logs_in(page) await self.then.path_should_be(page, "/destruction-lists")