From 1bbf4f327ecccd9151cc312f6ec335b82fe65023 Mon Sep 17 00:00:00 2001 From: SilviaAmAm Date: Thu, 9 Jan 2025 15:15:07 +0100 Subject: [PATCH 1/2] :test_tube: [#599] Test replacing reviewer with author --- .../tests/endpoints/test_destructionlist.py | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/backend/src/openarchiefbeheer/destruction/tests/endpoints/test_destructionlist.py b/backend/src/openarchiefbeheer/destruction/tests/endpoints/test_destructionlist.py index bb052bcd6..7d8916b06 100644 --- a/backend/src/openarchiefbeheer/destruction/tests/endpoints/test_destructionlist.py +++ b/backend/src/openarchiefbeheer/destruction/tests/endpoints/test_destructionlist.py @@ -1,3 +1,5 @@ +from django.utils.translation import gettext as _ + from requests_mock import Mocker from rest_framework import status from rest_framework.reverse import reverse @@ -7,9 +9,9 @@ from openarchiefbeheer.accounts.tests.factories import UserFactory -from ...constants import ListStatus +from ...constants import ListRole, ListStatus from ...models import DestructionList -from ..factories import DestructionListFactory +from ..factories import DestructionListAssigneeFactory, DestructionListFactory class DestructionListViewsetTests(APITestCase): @@ -182,3 +184,43 @@ def test_destruction_report_url_retrieved_from_openzaak(self, m): ) self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_assign_author_as_reviewer_when_logged_in_as_other_record_manager(self): + record_manager1 = UserFactory.create( + post__can_start_destruction=True, post__can_review_destruction=True + ) + record_manager2 = UserFactory.create(post__can_start_destruction=True) + reviewer = UserFactory.create(post__can_review_destruction=True) + + destruction_list = DestructionListFactory.create( + status=ListStatus.ready_to_review, + author=record_manager1, # First record manager is author + assignee=reviewer, + ) + DestructionListAssigneeFactory.create( + destruction_list=destruction_list, + user=record_manager1, + role=ListRole.author, + ) + DestructionListAssigneeFactory.create( + destruction_list=destruction_list, + user=reviewer, + role=ListRole.main_reviewer, + ) + + # Second record manager tries to assign first record manager as reviewer + self.client.force_login(record_manager2) + endpoint = reverse( + "api:destructionlist-reassign", kwargs={"uuid": destruction_list.uuid} + ) + response = self.client.post( + endpoint, + data={"assignee": {"user": record_manager1.pk}, "comment": "Tralala"}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual( + response.json()["assignee"]["user"][0], + _("The author of a list cannot also be a reviewer."), + ) From 1601d63504d46bc30d9a5f630c671abd518976f1 Mon Sep 17 00:00:00 2001 From: SilviaAmAm Date: Thu, 9 Jan 2025 15:16:37 +0100 Subject: [PATCH 2/2] :bug: [#599] Fix replacing the reviewer with author of list --- .../src/openarchiefbeheer/destruction/api/serializers.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/backend/src/openarchiefbeheer/destruction/api/serializers.py b/backend/src/openarchiefbeheer/destruction/api/serializers.py index 26c2f0c47..ea2bacf25 100644 --- a/backend/src/openarchiefbeheer/destruction/api/serializers.py +++ b/backend/src/openarchiefbeheer/destruction/api/serializers.py @@ -61,7 +61,13 @@ def validate(self, attrs: dict) -> dict: if self.parent.instance: return attrs - author = self.parent.context["request"].user + if destruction_list := self.parent.context.get("destruction_list"): + # Case in which an existing reviewer is replaced + author = destruction_list.author + else: + # Case in which a new list is created + author = self.parent.context["request"].user + if author.pk == attrs["user"].pk: raise ValidationError( {"user": _("The author of a list cannot also be a reviewer.")}