Merge pull request #116 from maykinmedia/fix/107-filter-zaken-on-dl

[#107] Filter zaken on destruction list

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

@@ -156,7 +156,6 @@ def update(
 
 class DestructionListResponseSerializer(serializers.ModelSerializer):
     assignees = DestructionListAssigneeResponseSerializer(many=True)
-    items = DestructionListItemSerializer(many=True)
     author = UserSerializer(read_only=True)
     assignee = UserSerializer(read_only=True)
 
@@ -169,7 +168,6 @@ class Meta:
             "contains_sensitive_info",
             "assignees",
             "assignee",
-            "items",
             "status",
             "created",
             "status_changed",
@@ -185,12 +183,21 @@ class Meta:
         )
 
 
+class ZakenReviewSerializer(serializers.Serializer):
+    zaak_url = serializers.URLField(
+        required=True, help_text="The URL of the case for which changes are requested."
+    )
+    feedback = serializers.CharField(
+        required=True, help_text="Feedback about what should be done with this case."
+    )
+
+
 class DestructionListReviewSerializer(serializers.ModelSerializer):
     author = UserSerializer(read_only=True)
     destruction_list = SlugRelatedField(
         slug_field="uuid", queryset=DestructionList.objects.all()
     )
-    item_reviews = DestructionListItemReviewSerializer(
+    zaken_reviews = ZakenReviewSerializer(
         many=True,
         required=False,
         help_text="This field is required if changes are requested to the destruction list.",
@@ -203,7 +210,7 @@ class Meta:
             "author",
             "decision",
             "list_feedback",
-            "item_reviews",
+            "zaken_reviews",
         )
 
     def validate(self, attrs: dict) -> dict:
@@ -218,46 +225,77 @@ def validate(self, attrs: dict) -> dict:
                 }
             )
 
+        zaken_reviews = attrs.get("zaken_reviews", [])
         if (
             attrs["decision"] == ReviewDecisionChoices.rejected
-            and len(attrs.get("item_reviews", [])) == 0
+            and len(zaken_reviews) == 0
         ):
             raise ValidationError(
                 {
-                    "item_reviews": _(
+                    "zaken_reviews": _(
                         "This field cannot be empty if changes are requested on the list."
                     )
                 }
             )
 
         if (
             attrs["decision"] == ReviewDecisionChoices.accepted
-            and len(attrs.get("item_reviews", [])) != 0
+            and len(zaken_reviews) != 0
         ):
             raise ValidationError(
                 {
-                    "item_reviews": _(
+                    "zaken_reviews": _(
                         "There cannot be feedback on the cases if the list is approved."
                     )
                 }
             )
 
+        if len(zaken_reviews) > 0:
+            destruction_list_items = (
+                attrs["destruction_list"]
+                .items.filter(status=ListItemStatus.suggested)
+                .values_list("zaak", flat=True)
+            )
+
+            for zaak_review in zaken_reviews:
+                if zaak_review["zaak_url"] not in destruction_list_items:
+                    raise ValidationError(
+                        {
+                            "zaken_reviews": _(
+                                "You can only provide feedback about cases that are part of the destruction list."
+                            )
+                        }
+                    )
+
         return attrs
 
     def create(self, validated_data: dict) -> DestructionListReview:
-        item_reviews = validated_data.pop("item_reviews", [])
+        zaken_reviews = validated_data.pop("zaken_reviews", [])
+        destruction_list_items_with_changes = (
+            validated_data["destruction_list"]
+            .items.filter(
+                zaak__in=[zaak_review["zaak_url"] for zaak_review in zaken_reviews]
+            )
+            .distinct("zaak")
+            .in_bulk(field_name="zaak")
+        )
 
         validated_data["author"] = self.context["request"].user
         review = DestructionListReview.objects.create(**validated_data)
 
-        extra_data_per_item = {
-            "destruction_list": validated_data["destruction_list"],
-            "review": review,
-        }
-        items_feedback_to_create = [
-            DestructionListItemReview(**{**item, **extra_data_per_item})
-            for item in item_reviews
+        review_items_data = [
+            DestructionListItemReview(
+                **{
+                    "destruction_list_item": destruction_list_items_with_changes[
+                        zaak_review["zaak_url"]
+                    ],
+                    "destruction_list": validated_data["destruction_list"],
+                    "review": review,
+                    "feedback": zaak_review["feedback"],
+                }
+            )
+            for zaak_review in zaken_reviews
         ]
-        DestructionListItemReview.objects.bulk_create(items_feedback_to_create)
+        DestructionListItemReview.objects.bulk_create(review_items_data)
 
         return review

backend/src/openarchiefbeheer/destruction/tests/test_endpoints.py

@@ -457,13 +457,13 @@ def test_create_review_rejected(self):
             "destruction_list": destruction_list.uuid,
             "decision": ReviewDecisionChoices.rejected,
             "list_feedback": "I disagree with this list",
-            "item_reviews": [
+            "zaken_reviews": [
                 {
-                    "destruction_list_item": items[0].pk,
+                    "zaak_url": items[0].zaak,
                     "feedback": "This item should not be deleted.",
                 },
                 {
-                    "destruction_list_item": items[1].pk,
+                    "zaak_url": items[1].zaak,
                     "feedback": "We should wait to delete this.",
                 },
             ],

backend/src/openarchiefbeheer/destruction/tests/test_serializers.py

@@ -457,9 +457,9 @@ def test_create_review_accepted_cannot_have_item_reviews(self):
             "destruction_list": destruction_list.uuid,
             "decision": ReviewDecisionChoices.accepted,
             "list_feedback": "This is a list with inconsisten feedback.",
-            "item_reviews": [
+            "zaken_reviews": [
                 {
-                    "destruction_list_item": item.pk,
+                    "zaak_url": item.zaak,
                     "feedback": "This item should not be deleted.",
                 },
             ],
@@ -473,7 +473,7 @@ def test_create_review_accepted_cannot_have_item_reviews(self):
 
         self.assertFalse(serializer.is_valid())
         self.assertEqual(
-            serializer.errors["item_reviews"][0],
+            serializer.errors["zaken_reviews"][0],
             _("There cannot be feedback on the cases if the list is approved."),
         )
 
@@ -498,7 +498,7 @@ def test_create_review_rejected_must_have_item_reviews(self):
 
         self.assertFalse(serializer.is_valid())
         self.assertEqual(
-            serializer.errors["item_reviews"][0],
+            serializer.errors["zaken_reviews"][0],
             _("This field cannot be empty if changes are requested on the list."),
         )
 
@@ -517,13 +517,13 @@ def test_create_review_rejected(self):
             "destruction_list": destruction_list.uuid,
             "decision": ReviewDecisionChoices.rejected,
             "list_feedback": "I disagree with this list",
-            "item_reviews": [
+            "zaken_reviews": [
                 {
-                    "destruction_list_item": items[0].pk,
+                    "zaak_url": items[0].zaak,
                     "feedback": "This item should not be deleted.",
                 },
                 {
-                    "destruction_list_item": items[1].pk,
+                    "zaak_url": items[1].zaak,
                     "feedback": "We should wait to delete this.",
                 },
             ],
@@ -541,3 +541,76 @@ def test_create_review_rejected(self):
 
         self.assertEqual(DestructionListReview.objects.count(), 1)
         self.assertEqual(DestructionListItemReview.objects.count(), 2)
+
+    def test_reviewing_cases_not_in_destruction_list(self):
+        reviewer = UserFactory.create(
+            username="reviewer",
+            email="[email protected]",
+            role__can_review_destruction=True,
+        )
+        destruction_list = DestructionListFactory.create(assignee=reviewer)
+        # Not part of the destruction list
+        item = DestructionListItemFactory.create(status=ListItemStatus.suggested)
+
+        data = {
+            "destruction_list": destruction_list.uuid,
+            "decision": ReviewDecisionChoices.rejected,
+            "list_feedback": "I disagree with this list",
+            "zaken_reviews": [
+                {
+                    "zaak_url": item.zaak,
+                    "feedback": "This item should not be deleted.",
+                },
+            ],
+        }
+
+        request = factory.get("/foo")
+        request.user = reviewer
+        serializer = DestructionListReviewSerializer(
+            data=data, context={"request": request}
+        )
+
+        self.assertFalse(serializer.is_valid())
+        self.assertEqual(
+            serializer.errors["zaken_reviews"][0],
+            _(
+                "You can only provide feedback about cases that are part of the destruction list."
+            ),
+        )
+
+    def test_reviewing_cases_removed_from_destruction_list(self):
+        reviewer = UserFactory.create(
+            username="reviewer",
+            email="[email protected]",
+            role__can_review_destruction=True,
+        )
+        destruction_list = DestructionListFactory.create(assignee=reviewer)
+        item = DestructionListItemFactory.create(
+            status=ListItemStatus.removed, destruction_list=destruction_list
+        )
+
+        data = {
+            "destruction_list": destruction_list.uuid,
+            "decision": ReviewDecisionChoices.rejected,
+            "list_feedback": "I disagree with this list",
+            "zaken_reviews": [
+                {
+                    "zaak_url": item.zaak,
+                    "feedback": "This item should not be deleted.",
+                },
+            ],
+        }
+
+        request = factory.get("/foo")
+        request.user = reviewer
+        serializer = DestructionListReviewSerializer(
+            data=data, context={"request": request}
+        )
+
+        self.assertFalse(serializer.is_valid())
+        self.assertEqual(
+            serializer.errors["zaken_reviews"][0],
+            _(
+                "You can only provide feedback about cases that are part of the destruction list."
+            ),
+        )

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

@@ -36,6 +36,11 @@ class ZaakFilter(FilterSet):
             "If True, only cases not already included in a destruction list are returned."
         ),
     )
+    in_destruction_list = UUIDFilter(
+        field_name="in_destruction_list",
+        method="filter_in_destruction_list",
+        help_text=_("All cases included in a destruction list are returned."),
+    )
     not_in_destruction_list_except = UUIDFilter(
         field_name="not_in_destruction_list_except",
         method="filter_not_in_destruction_list_except",
@@ -161,9 +166,17 @@ def filter_not_in_destruction_list(
 
         return queryset.exclude(url__in=Subquery(zaken_to_exclude))
 
+    def filter_in_destruction_list(
+        self, queryset: QuerySet[Zaak], name: str, value: str
+    ) -> QuerySet[Zaak]:
+        list_items = DestructionListItem.objects.filter(
+            Q(destruction_list__uuid=value) & ~Q(status=ListItemStatus.removed)
+        ).values_list("zaak", flat=True)
+        return queryset.filter(url__in=Subquery(list_items))
+
     def filter_not_in_destruction_list_except(
         self, queryset: QuerySet[Zaak], name: str, value: str
-    ):
+    ) -> QuerySet[Zaak]:
         zaken_to_exclude = DestructionListItem.objects.filter(
             ~Q(status=ListItemStatus.removed) & ~Q(destruction_list__uuid=value)
         ).values_list("zaak", flat=True)