Skip to content

Commit 80d60d0

Browse files
SilviaAmAmSilviaAmAm
authored
Merge pull request #331 from maykinmedia/feature/318-archivist-requests-changes
[#318] Archivist requests changes
2 parents 7c9a484 + a3e0948 commit 80d60d0

File tree

4 files changed

+133
-7
lines changed

4 files changed

+133
-7
lines changed

‎backend/src/openarchiefbeheer/destruction/assignment_logic.py

+16-7
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from django.db.models import Count, Min
44

55
from .constants import ListRole, ListStatus, ReviewDecisionChoices
6+
from .exceptions import NoReviewFoundError
67

78
if TYPE_CHECKING:
89
from .models import DestructionList, DestructionListAssignee
@@ -73,11 +74,15 @@ def reassign(self, destruction_list: "DestructionList") -> None:
7374

7475
class ChangesRequested:
7576
def assign_next(self, destruction_list: "DestructionList") -> None:
76-
destruction_list.set_status(ListStatus.ready_to_review)
77-
78-
# The reviewer who rejected the list reviews first
7977
last_review = destruction_list.reviews.order_by("created").last()
80-
destruction_list.assign(destruction_list.get_assignee(last_review.author))
78+
last_reviewer = destruction_list.get_assignee(last_review.author)
79+
80+
if last_reviewer.role == ListRole.archivist:
81+
destruction_list.set_status(ListStatus.ready_for_archivist)
82+
else:
83+
destruction_list.set_status(ListStatus.ready_to_review)
84+
85+
destruction_list.assign(last_reviewer)
8186

8287
def reassign(self, destruction_list: "DestructionList") -> None:
8388
# When a list has requested changes, it is assigned to the author. No action needed.
@@ -103,11 +108,15 @@ def reassign(self, destruction_list: "DestructionList") -> None:
103108
class ReadyForArchivist:
104109
def assign_next(self, destruction_list: "DestructionList") -> None:
105110
last_review = destruction_list.reviews.order_by("created").last()
106-
if last_review and last_review.decision == ReviewDecisionChoices.accepted:
111+
if not last_review:
112+
raise NoReviewFoundError()
113+
114+
if last_review.decision == ReviewDecisionChoices.accepted:
107115
destruction_list.set_status(ListStatus.ready_to_delete)
108-
destruction_list.assign(destruction_list.get_author())
116+
else:
117+
destruction_list.set_status(ListStatus.changes_requested)
109118

110-
# TODO in the case where the archivist rejects it is not clear yet what should happen!
119+
destruction_list.assign(destruction_list.get_author())
111120

112121
def reassign(self, destruction_list: "DestructionList") -> None:
113122
# TODO

‎backend/src/openarchiefbeheer/destruction/exceptions.py

+4
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,7 @@ class DeletionProcessingError(Exception):
44

55
class ZaakNotFound(Exception):
66
pass
7+
8+
9+
class NoReviewFoundError(Exception):
10+
pass

‎backend/src/openarchiefbeheer/destruction/tests/e2e/features/test_feature_list_review.py

+66
Original file line numberDiff line numberDiff line change
@@ -91,3 +91,69 @@ async def test_scenario_reviewer_rejects_list(self):
9191
await self.then.path_should_be(page, "/destruction-lists")
9292
await self.then.page_should_contain_text(page, "Destruction list to review")
9393
await self.then.list_should_have_status(page, list, ListStatus.changes_requested)
94+
95+
async def test_scenario_archivist_approves_list(self):
96+
async with browser_page() as page:
97+
record_manager = await self.given.record_manager_exists()
98+
archivist = await self.given.archivist_exists()
99+
100+
assignees = [
101+
await self.given.assignee_exists(user=record_manager, role=ListRole.author),
102+
await self.given.assignee_exists(user=archivist, role=ListRole.archivist),
103+
]
104+
105+
list = await self.given.list_exists(
106+
assignee=archivist,
107+
assignees=assignees,
108+
uuid="00000000-0000-0000-0000-000000000000",
109+
name="Destruction list to review",
110+
status=ListStatus.ready_for_archivist,
111+
)
112+
113+
await self.when.archivist_logs_in(page)
114+
await self.then.path_should_be(page, "/destruction-lists")
115+
116+
await self.when.user_clicks_button(page, "Destruction list to review")
117+
await self.then.path_should_be(page, "/destruction-lists/00000000-0000-0000-0000-000000000000/review")
118+
119+
await self.when.user_clicks_button(page, "Accorderen")
120+
await self.when.user_fills_form_field(page, "Opmerking", "Looks good to me👍🏻")
121+
await self.when.user_clicks_button(page, "Accorderen")
122+
123+
await self.then.path_should_be(page, "/destruction-lists")
124+
await self.then.page_should_contain_text(page, "Destruction list to review")
125+
await self.then.list_should_have_status(page, list, ListStatus.ready_to_delete)
126+
127+
async def test_scenario_archivist_rejects_list(self):
128+
async with browser_page() as page:
129+
record_manager = await self.given.record_manager_exists()
130+
archivist = await self.given.archivist_exists()
131+
132+
assignees = [
133+
await self.given.assignee_exists(user=record_manager, role=ListRole.author),
134+
await self.given.assignee_exists(user=archivist, role=ListRole.archivist),
135+
]
136+
137+
list = await self.given.list_exists(
138+
assignee=archivist,
139+
assignees=assignees,
140+
uuid="00000000-0000-0000-0000-000000000000",
141+
name="Destruction list to review",
142+
status=ListStatus.ready_for_archivist,
143+
)
144+
145+
await self.when.archivist_logs_in(page)
146+
await self.then.path_should_be(page, "/destruction-lists")
147+
148+
await self.when.user_clicks_button(page, "Destruction list to review")
149+
await self.then.path_should_be(page, "/destruction-lists/00000000-0000-0000-0000-000000000000/review")
150+
await self.when.user_clicks_checkbox(page, "(de)selecteer rij")
151+
await self.when.user_fills_form_field(page, "Reden van uitzondering", "Please reconsider this zaak")
152+
await self.when.user_clicks_button(page, "Uitzonderen")
153+
await self.when.user_clicks_button(page, "Beoordelen")
154+
await self.when.user_fills_form_field(page, "Opmerking", "Please reconsider the zaak on this list")
155+
await self.when.user_clicks_button(page, "Beoordelen")
156+
157+
await self.then.path_should_be(page, "/destruction-lists")
158+
await self.then.page_should_contain_text(page, "Destruction list to review")
159+
await self.then.list_should_have_status(page, list, ListStatus.changes_requested)

‎backend/src/openarchiefbeheer/destruction/tests/test_assignement_logic.py

+47
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
DestructionListFactory,
1212
DestructionListItemFactory,
1313
DestructionListReviewFactory,
14+
ReviewResponseFactory,
1415
)
1516

1617

@@ -267,3 +268,49 @@ def test_reassign_reviewers_noop(self):
267268

268269
self.assertEqual(destruction_list.status, ListStatus.internally_reviewed)
269270
self.assertEqual(destruction_list.assignee, record_manager)
271+
272+
def test_archivist_requested_changes(self):
273+
destruction_list = DestructionListFactory.create(
274+
status=ListStatus.ready_for_archivist
275+
)
276+
record_manager = DestructionListAssigneeFactory.create(
277+
role=ListRole.author,
278+
destruction_list=destruction_list,
279+
user=destruction_list.author,
280+
)
281+
archivist = DestructionListAssigneeFactory.create(
282+
role=ListRole.archivist, destruction_list=destruction_list
283+
)
284+
DestructionListReviewFactory.create(
285+
author=archivist.user,
286+
destruction_list=destruction_list,
287+
decision=ReviewDecisionChoices.rejected,
288+
)
289+
290+
destruction_list.assign_next()
291+
292+
destruction_list.refresh_from_db()
293+
294+
self.assertEqual(destruction_list.status, ListStatus.changes_requested)
295+
self.assertEqual(destruction_list.assignee, record_manager.user)
296+
297+
def test_record_manager_processed_changes_after_archivist(self):
298+
destruction_list = DestructionListFactory.create(
299+
status=ListStatus.changes_requested
300+
)
301+
archivist = DestructionListAssigneeFactory.create(
302+
role=ListRole.archivist, destruction_list=destruction_list
303+
)
304+
review = DestructionListReviewFactory.create(
305+
author=archivist.user,
306+
destruction_list=destruction_list,
307+
decision=ReviewDecisionChoices.rejected,
308+
)
309+
ReviewResponseFactory.create(review=review)
310+
311+
destruction_list.assign_next()
312+
313+
destruction_list.refresh_from_db()
314+
315+
self.assertEqual(destruction_list.status, ListStatus.ready_for_archivist)
316+
self.assertEqual(destruction_list.assignee, archivist.user)

0 commit comments

Comments
 (0)