Merge pull request #389 from maykinmedia/fix/386-blijven-bewaren

[#386] Checks for selectielijstklasse waardering "blijvend bewaren" [Backend]

Author: SilviaAmAm

backend/docs/developers/logic.rst

@@ -10,5 +10,17 @@ For each zaak that has been "rejected" by the reviewer, the record manager can:
 - Keep the zaak in the destruction list (go against the suggestion of the reviewer). In this case, no details of the zaak can be updated.
 - Remove the zaak from the destruction list. When removing the zaak, the record manager should either:
 
-   - Update the selectielijstklasse of the zaak. This should automatically update the archiefactiedatum of the zaak (this is currently done by the frontend). This is for example desirable in the case that the zaak has the wrong selectielijstklasse by accident.
-   - Update the archiefactiedatum of the zaak. In this case the selectielijstklasse is correct, but for whatever reason the zaak needs to be kept for longer.
+   - Update the selectielijstklasse of the zaak. 
+
+     This should automatically update the archiefactiedatum of the zaak (this is currently done by the frontend). 
+     *Example*: case where the zaak has the wrong selectielijstklasse by accident. 
+
+     The record manager needs to still be able to update the archiefactiedatum manually. 
+
+     In the case where the selectielijstklasse has "waardering" equal to "blijven_bewaren", 
+     then the archiefactiedatum must be empty and the record manager cannot change it.
+   
+   - Update the archiefactiedatum of the zaak. 
+
+     In this case the selectielijstklasse is correct, but for whatever reason the zaak needs to be kept for longer. 
+     Changing the archiefactiedatum for zaken with "waardering" equal to "blijven_bewaren" should not be possible.

backend/src/openarchiefbeheer/config/exceptions.py

@@ -0,0 +1,2 @@
+class ServiceNotConfigured(Exception):
+    pass

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

@@ -5,6 +5,7 @@
 from django.utils.translation import gettext_lazy as _
 
 from drf_spectacular.utils import 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
@@ -16,6 +17,7 @@
 from openarchiefbeheer.zaken.api.filtersets import ZaakFilter
 from openarchiefbeheer.zaken.api.serializers import ZaakSerializer
 from openarchiefbeheer.zaken.models import Zaak
+from openarchiefbeheer.zaken.utils import retrieve_selectielijstklasse_resultaat
 
 from ..constants import (
     DestructionListItemAction,
@@ -497,6 +499,11 @@ def to_internal_value(self, data: dict) -> dict:
 
 
 class ReviewItemResponseSerializer(serializers.ModelSerializer):
+    review_item = serializers.PrimaryKeyRelatedField(
+        queryset=DestructionListItemReview.objects.all().select_related(
+            "destruction_list_item__zaak"
+        ),
+    )
     action_zaak = ActionZaakSerializer(required=False)
 
     class Meta:
@@ -511,8 +518,79 @@ class Meta:
             "comment",
         )
 
+    def _get_selectielijst_resultaat(self, resultaat_url: str) -> dict:
+        try:
+            resultaat = retrieve_selectielijstklasse_resultaat(resultaat_url)
+        except HTTPError:
+            raise ValidationError(
+                _(
+                    "Could not validate the selectielijstklasse waardering "
+                    "due to an unexpected response from the Selectielijst API."
+                )
+            )
+
+        return resultaat
+
+    def _validate_action_zaak_with_type(
+        self,
+        action_zaak: dict,
+        action_zaak_type: str,
+        review_item: DestructionListItemReview,
+    ) -> None:
+        # TODO this could be refactored using a polymorphic serializer
+        selectielijstklasse = action_zaak.get("selectielijstklasse")
+        archiefactiedatum = action_zaak.get("archiefactiedatum")
+
+        match action_zaak_type:
+            case ZaakActionType.selectielijstklasse_and_bewaartermijn:
+                if not selectielijstklasse:
+                    raise ValidationError(
+                        {
+                            "selectielijstklasse": _(
+                                "The selectielijstklasse is required for action type "
+                                "is 'selectielijstklasse_and_bewaartermijn'."
+                            )
+                        }
+                    )
+
+                resultaat = self._get_selectielijst_resultaat(selectielijstklasse)
+                if resultaat["waardering"] == "blijvend_bewaren" and archiefactiedatum:
+                    raise ValidationError(
+                        {
+                            "selectielijstklasse": _(
+                                "The selectielijstklasse has waardering 'blijvend_bewaren', "
+                                "so the archiefactiedatum should be null."
+                            )
+                        }
+                    )
+
+            case ZaakActionType.bewaartermijn:
+                if selectielijstklasse:
+                    raise ValidationError(
+                        {
+                            "action_zaak_type": _(
+                                "The selectielijstklasse cannot be changed if the case action type is 'bewaartermijn'."
+                            )
+                        }
+                    )
+
+                resultaat = self._get_selectielijst_resultaat(
+                    review_item.destruction_list_item.zaak.selectielijstklasse
+                )
+                if resultaat["waardering"] == "blijvend_bewaren":
+                    raise ValidationError(
+                        {
+                            "archiefactiedatum": _(
+                                "The selectielijstklasse has waardering 'blijvend_bewaren', "
+                                "so an archiefactiedatum cannot be set."
+                            )
+                        }
+                    )
+
     def validate(self, attrs: dict) -> dict:
         action_zaak = attrs.get("action_zaak", {})
+        action_zaak_type = attrs.get("action_zaak_type", {})
+
         if attrs["action_item"] == DestructionListItemAction.keep and action_zaak:
             raise ValidationError(
                 {
@@ -522,32 +600,17 @@ def validate(self, attrs: dict) -> dict:
                 }
             )
 
-        zaak_action_type = attrs.get("action_zaak_type")
-        selectielijstklasse = action_zaak.get("selectielijstklasse")
-        if (
-            attrs["action_item"] == DestructionListItemAction.remove
-            and zaak_action_type == ZaakActionType.bewaartermijn
-            and selectielijstklasse
-        ):
-            raise ValidationError(
-                {
-                    "action_zaak_type": _(
-                        "The selectielijstklasse cannot be changed if the case action type is 'bewaartermijn'."
+        if attrs["action_item"] == DestructionListItemAction.remove:
+            if not action_zaak_type or not action_zaak:
+                raise ValidationError(
+                    _(
+                        "When removing an item from a destruction list, "
+                        "the fields action_zaak_type and action_zaak are required."
                     )
-                }
-            )
+                )
 
-        if (
-            attrs["action_item"] == DestructionListItemAction.remove
-            and zaak_action_type == ZaakActionType.selectielijstklasse_and_bewaartermijn
-            and not selectielijstklasse
-        ):
-            raise ValidationError(
-                {
-                    "selectielijstklasse": _(
-                        "The selectielijstklasse is required for action type is 'selectielijstklasse_and_bewaartermijn'."
-                    )
-                }
+            self._validate_action_zaak_with_type(
+                action_zaak, action_zaak_type, attrs["review_item"]
             )
 
         return attrs

backend/src/openarchiefbeheer/destruction/tests/e2e/features/test_feature_list_process_review.py

@@ -1,4 +1,6 @@
 # fmt: off
+from unittest.mock import patch
+
 from django.test import tag
 
 from asgiref.sync import sync_to_async
@@ -24,6 +26,13 @@
 @tag("e2e")
 class FeatureProcessReviewTests(GherkinLikeTestCase):
     async def test_scenario_record_manager_process_review(self):
+        patcher = patch(
+            "openarchiefbeheer.destruction.api.serializers.retrieve_selectielijstklasse_resultaat", 
+            return_value={"waardering": "vernietigen"}
+        )
+        patcher.start()
+        self.addCleanup(patcher.stop)
+
         async with browser_page() as page:
             await self.given.selectielijstklasse_choices_are_available(page)
             record_manager = await self.given.record_manager_exists()

backend/src/openarchiefbeheer/destruction/tests/endpoints/test_reviewresponse.py

@@ -1,3 +1,5 @@
+from unittest.mock import patch
+
 from django.utils.translation import gettext_lazy as _
 
 import freezegun
@@ -61,46 +63,52 @@ def test_create_review_response(self):
         items_reviews = DestructionListItemReviewFactory.create_batch(
             3,
             destruction_list_item__destruction_list=review.destruction_list,
+            destruction_list_item__with_zaak=True,
+            destruction_list_item__zaak__selectielijstklasse="http://some-url.nl",
             review=review,
         )
 
         endpoint = reverse("api:review-responses-list")
         self.client.force_authenticate(user=record_manager)
 
-        response = self.client.post(
-            endpoint,
-            data={
-                "review": review.pk,
-                "comment": "A comment about the review.",
-                "itemsResponses": [
-                    {
-                        "reviewItem": items_reviews[0].pk,
-                        "actionItem": DestructionListItemAction.keep,
-                        "comment": "This zaak needs to stay in the list.",
-                    },
-                    {
-                        "reviewItem": items_reviews[1].pk,
-                        "actionItem": DestructionListItemAction.remove,
-                        "actionZaak": {
-                            "action_type": ZaakActionType.selectielijstklasse_and_bewaartermijn,
-                            "selectielijstklasse": "http://some-url.nl",
-                            "archiefactiedatum": "2030-01-01",
+        with patch(
+            "openarchiefbeheer.destruction.api.serializers.retrieve_selectielijstklasse_resultaat",
+            return_value={"waardering": "vernietigen"},
+        ):
+            response = self.client.post(
+                endpoint,
+                data={
+                    "review": review.pk,
+                    "comment": "A comment about the review.",
+                    "itemsResponses": [
+                        {
+                            "reviewItem": items_reviews[0].pk,
+                            "actionItem": DestructionListItemAction.keep,
+                            "comment": "This zaak needs to stay in the list.",
                         },
-                        "comment": "Changed the selectielijstklasse and removed from the list.",
-                    },
-                    {
-                        "reviewItem": items_reviews[2].pk,
-                        "actionItem": DestructionListItemAction.remove,
-                        "actionZaak": {
-                            "action_type": ZaakActionType.bewaartermijn,
-                            "archiefactiedatum": "2030-01-01",
+                        {
+                            "reviewItem": items_reviews[1].pk,
+                            "actionItem": DestructionListItemAction.remove,
+                            "actionZaakType": ZaakActionType.selectielijstklasse_and_bewaartermijn,
+                            "actionZaak": {
+                                "selectielijstklasse": "http://some-url.nl",
+                                "archiefactiedatum": "2030-01-01",
+                            },
+                            "comment": "Changed the selectielijstklasse and removed from the list.",
                         },
-                        "comment": "Changed the archiefactiedatum and removed from the list.",
-                    },
-                ],
-            },
-            format="json",
-        )
+                        {
+                            "reviewItem": items_reviews[2].pk,
+                            "actionItem": DestructionListItemAction.remove,
+                            "actionZaakType": ZaakActionType.bewaartermijn,
+                            "actionZaak": {
+                                "archiefactiedatum": "2030-01-01",
+                            },
+                            "comment": "Changed the archiefactiedatum and removed from the list.",
+                        },
+                    ],
+                },
+                format="json",
+            )
 
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
         self.assertEqual(ReviewResponse.objects.filter(review=review).count(), 1)

backend/src/openarchiefbeheer/destruction/tests/serializers/test_reviewresponse.py

@@ -1,12 +1,19 @@
+from unittest.mock import patch
+
 from django.test import TestCase
 from django.utils.translation import gettext_lazy as _
 
+from requests.exceptions import HTTPError
+from rest_framework.test import APIRequestFactory
+
 from openarchiefbeheer.accounts.tests.factories import UserFactory
 
 from ...api.serializers import ReviewResponseSerializer
 from ...constants import DestructionListItemAction, ListStatus, ZaakActionType
 from ..factories import DestructionListItemReviewFactory, DestructionListReviewFactory
 
+factory = APIRequestFactory()
+
 
 class ReviewResponseSerializerTests(TestCase):
     def test_rejecting_suggestion_cannot_have_zaak_action(self):
@@ -121,3 +128,53 @@ def test_selectielijst_zaak_action_requires_selectielijst(self):
                 "The selectielijstklasse is required for action type is 'selectielijstklasse_and_bewaartermijn'."
             ),
         )
+
+    def test_update_archiefactiedatum_selectielijst_api_error(self):
+        record_manager = UserFactory.create(role__can_start_destruction=True)
+        review = DestructionListReviewFactory.create(
+            destruction_list__author=record_manager,
+            destruction_list__status=ListStatus.changes_requested,
+            destruction_list__assignee=record_manager,
+        )
+        items_review = DestructionListItemReviewFactory.create(
+            destruction_list_item__with_zaak=True,
+            destruction_list_item__zaak__selectielijstklasse="https://selectielijst.openzaak.nl/api/v1/resultaten/8320ab7d-3a8d-4c8b-b94a-14b4fa374d0a",
+            destruction_list_item__destruction_list=review.destruction_list,
+            review=review,
+        )
+
+        data = {
+            "review": review.pk,
+            "comment": "A response about the review.",
+            "items_responses": [
+                {
+                    "review_item": items_review.pk,
+                    "action_item": DestructionListItemAction.remove,
+                    "action_zaak_type": ZaakActionType.bewaartermijn,
+                    "action_zaak": {
+                        "archiefactiedatum": "2030-01-01",
+                    },
+                    "comment": "This zaak needs to kept.",
+                },
+            ],
+        }
+
+        request = factory.get("/foo")
+        request.user = record_manager
+
+        with patch(
+            "openarchiefbeheer.destruction.api.serializers.retrieve_selectielijstklasse_resultaat",
+            side_effect=HTTPError,
+        ):
+            serializer = ReviewResponseSerializer(
+                data=data, context={"request": request}
+            )
+
+            self.assertFalse(serializer.is_valid())
+
+        self.assertEqual(
+            serializer.errors["items_responses"][0]["non_field_errors"][0],
+            _(
+                "Could not validate the selectielijstklasse waardering due to an unexpected response from the Selectielijst API."
+            ),
+        )