Skip to content

Commit 8b872e0

Browse files
✨ - feat: allow single reviewer for list with short process
1 parent 18eb2ac commit 8b872e0

File tree

4 files changed

+119
-20
lines changed

4 files changed

+119
-20
lines changed

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

+41-16
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from rest_framework.relations import SlugRelatedField
99

1010
from openarchiefbeheer.accounts.api.serializers import UserSerializer
11+
from openarchiefbeheer.config.models import ArchiveConfig
1112
from openarchiefbeheer.logging import logevent
1213
from openarchiefbeheer.zaken.api.serializers import ZaakSerializer
1314
from openarchiefbeheer.zaken.models import Zaak
@@ -131,25 +132,49 @@ def validate(self, attrs: dict) -> dict:
131132
"""
132133
assignees = attrs.get("assignees")
133134

134-
if assignees:
135-
# Validate that assignees can only be set when comment is provided.
136-
current_reviewers = (
137-
self.instance.assignees.filter(role=ListRole.reviewer)
138-
if self.instance
139-
else DestructionListAssignee.objects.none()
140-
)
141-
current_reviewer_pks = list(
142-
current_reviewers.values_list("user__pk", flat=True)
143-
)
144-
assignee_pks = [assignee["user"].pk for assignee in assignees]
135+
if not assignees:
136+
return attrs
145137

146-
if current_reviewer_pks and current_reviewer_pks != assignee_pks:
147-
comment = attrs.get("comment", "").strip()
138+
# Validate that assignees can only be set when comment is provided.
139+
current_reviewers = (
140+
self.instance.assignees.filter(role=ListRole.reviewer)
141+
if self.instance
142+
else DestructionListAssignee.objects.none()
143+
)
144+
current_reviewer_pks = list(
145+
current_reviewers.values_list("user__pk", flat=True)
146+
)
147+
assignee_pks = [assignee["user"].pk for assignee in assignees]
148148

149-
if not comment:
150-
raise ValidationError(
151-
_("A comment should be provided when changing assignees.")
149+
if current_reviewer_pks and current_reviewer_pks != assignee_pks:
150+
comment = attrs.get("comment", "").strip()
151+
152+
if not comment:
153+
raise ValidationError(
154+
_("A comment should be provided when changing assignees.")
155+
)
156+
157+
if len(assignees) == 1:
158+
items = self.initial_data["items"]
159+
zaken_urls = [i["zaak"] for i in items]
160+
zaaktypes_urls = (
161+
Zaak.objects.filter(url__in=zaken_urls)
162+
.values_list("zaaktype", flat=True)
163+
.distinct()
164+
)
165+
config = ArchiveConfig.get_solo()
166+
167+
if not all(
168+
[
169+
zaaktype in config.zaaktypes_short_process
170+
for zaaktype in zaaktypes_urls
171+
]
172+
):
173+
raise ValidationError(
174+
_(
175+
"A destruction list without a short reviewing process must have more than 1 reviewer."
152176
)
177+
)
153178

154179
return attrs
155180

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

+72
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@
1111
from timeline_logger.models import TimelineLog
1212

1313
from openarchiefbeheer.accounts.tests.factories import UserFactory
14+
from openarchiefbeheer.config.models import ArchiveConfig
1415
from openarchiefbeheer.emails.models import EmailConfig
1516

17+
from ...zaken.tests.factories import ZaakFactory
1618
from ..api.serializers import DestructionListReviewSerializer, DestructionListSerializer
1719
from ..constants import ListItemStatus, ListRole, ListStatus, ReviewDecisionChoices
1820
from ..models import (
@@ -122,6 +124,76 @@ def test_create_destruction_list(self):
122124
'[2024-05-02T16:00:00+02:00]: Destruction list "A test list" created by user record_manager.',
123125
)
124126

127+
def test_destruction_list_with_short_procedure_requires_multiple_reviewers(self):
128+
reviewer = UserFactory.create(
129+
username="reviewer1",
130+
131+
role__can_review_destruction=True,
132+
)
133+
record_manager = UserFactory.create(
134+
username="record_manager", role__can_start_destruction=True
135+
)
136+
zaak = ZaakFactory.create()
137+
request = factory.get("/foo")
138+
request.user = record_manager
139+
140+
with patch(
141+
"openarchiefbeheer.config.models.ArchiveConfig.get_solo",
142+
return_value=ArchiveConfig(zaaktypes_short_process=[zaak.zaaktype]),
143+
):
144+
data = {
145+
"name": "A test list",
146+
"contains_sensitive_info": True,
147+
"assignees": [
148+
{"user": reviewer.pk, "order": 0},
149+
],
150+
"items": [
151+
{
152+
"zaak": zaak.url,
153+
"extra_zaak_data": {},
154+
},
155+
],
156+
}
157+
serializer = DestructionListSerializer(
158+
data=data, context={"request": request}
159+
)
160+
self.assertTrue(serializer.is_valid())
161+
162+
def test_destruction_list_without_short_procedure_requires_multiple_reviewers(self):
163+
reviewer = UserFactory.create(
164+
username="reviewer1",
165+
166+
role__can_review_destruction=True,
167+
)
168+
record_manager = UserFactory.create(
169+
username="record_manager", role__can_start_destruction=True
170+
)
171+
zaak = ZaakFactory.create()
172+
request = factory.get("/foo")
173+
request.user = record_manager
174+
175+
data = {
176+
"name": "A test list",
177+
"contains_sensitive_info": True,
178+
"assignees": [
179+
{"user": reviewer.pk, "order": 0},
180+
],
181+
"items": [
182+
{
183+
"zaak": zaak.url,
184+
"extra_zaak_data": {},
185+
},
186+
],
187+
}
188+
189+
serializer = DestructionListSerializer(data=data, context={"request": request})
190+
serializer.is_valid()
191+
192+
self.assertIn(
193+
"A destruction list without a short reviewing process must have more than 1 reviewer.",
194+
serializer.errors["non_field_errors"],
195+
)
196+
125197
def test_zaak_already_included_in_other_list(self):
126198
user1 = UserFactory.create(
127199
username="reviewer1", role__can_review_destruction=True

frontend/src/lib/api/destructionLists.ts

+5-3
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,11 @@ export async function createDestructionList(
6868
assignees: string[] | number[] | User[],
6969
) {
7070
const urls = zaken.map((zaak) => (isPrimitive(zaak) ? zaak : zaak.url));
71-
const assigneeIds = assignees.map((assignee) =>
72-
isPrimitive(assignee) ? assignee.toString() : assignee.pk.toString(),
73-
);
71+
const assigneeIds = assignees
72+
.map((assignee) =>
73+
isPrimitive(assignee) ? assignee.toString() : assignee.pk.toString(),
74+
)
75+
.filter((v) => v);
7476

7577
const destructionList = {
7678
name,

frontend/src/pages/destructionlist/create/DestructionListCreate.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ export function DestructionListCreatePage() {
8181
value: String(user.pk),
8282
label: user.username,
8383
})),
84-
required: true,
84+
required: false,
8585
},
8686
];
8787

0 commit comments

Comments
 (0)