✨ [#227] Reassign assignees

Author: SilviaAmAm

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

@@ -22,6 +22,7 @@
     ReviewResponse,
 )
 from ..tasks import delete_destruction_list
+from ..utils import process_new_assignees
 from .filtersets import (
     DestructionListFilterset,
     DestructionListItemFilterset,
@@ -265,14 +266,12 @@ def reassign(self, request, *args, **kwargs):
         )
         serialiser.is_valid(raise_exception=True)
 
-        with transaction.atomic():
-            destruction_list.assignees.filter(
-                role=serialiser.validated_data["role"]
-            ).delete()
-            new_assignees = destruction_list.bulk_create_assignees(
-                serialiser.validated_data["assignees"],
-                serialiser.validated_data["role"],
-            )
+        new_assignees = process_new_assignees(
+            destruction_list,
+            serialiser.validated_data["assignees"],
+            serialiser.validated_data["role"],
+        )
+        destruction_list.reassign()
 
         logevent.destruction_list_reassigned(
             destruction_list,

backend/src/openarchiefbeheer/destruction/assignment_logic.py

@@ -5,12 +5,14 @@
 from .constants import ListRole, ListStatus, ReviewDecisionChoices
 
 if TYPE_CHECKING:
-    from .models import DestructionList
+    from .models import DestructionList, DestructionListAssignee
 
 
 class State(Protocol):
     def assign_next(self, destruction_list: "DestructionList") -> None: ...
 
+    def reassign(self, destruction_list: "DestructionList") -> None: ...
+
 
 class NewList:
     def assign_next(self, destruction_list: "DestructionList") -> None:
@@ -23,8 +25,27 @@ def assign_next(self, destruction_list: "DestructionList") -> None:
         destruction_list.set_status(ListStatus.ready_to_review)
         destruction_list.assign(assignee)
 
+    def reassign(self, destruction_list: "DestructionList") -> None:
+        # When a list is new, it is assigned to the author. No action needed.
+        pass
+
 
 class ReadyToReview:
+    def _deduce_next_reviewer(
+        self, destruction_list: "DestructionList"
+    ) -> "DestructionListAssignee":
+        # Find the reviewers who have given fewer reviews
+        reviewers = destruction_list.assignees.filter(role=ListRole.reviewer).annotate(
+            num_reviews=Count("user__created_reviews")
+        )
+        min_number_reviews = reviewers.aggregate(min_reviews=Min("num_reviews"))
+        next_reviewer = (
+            reviewers.filter(num_reviews=min_number_reviews["min_reviews"])
+            .order_by("pk")
+            .first()
+        )
+        return next_reviewer
+
     def assign_next(self, destruction_list: "DestructionList") -> None:
         last_review = destruction_list.reviews.order_by("created").last()
         if last_review and last_review.decision == ReviewDecisionChoices.rejected:
@@ -42,16 +63,11 @@ def assign_next(self, destruction_list: "DestructionList") -> None:
             destruction_list.assign(destruction_list.get_author())
             return
 
-        # Find the reviewers who have given fewer reviews
-        reviewers = destruction_list.assignees.filter(role=ListRole.reviewer).annotate(
-            num_reviews=Count("user__created_reviews")
-        )
-        min_number_reviews = reviewers.aggregate(min_reviews=Min("num_reviews"))
-        next_reviewer = (
-            reviewers.filter(num_reviews=min_number_reviews["min_reviews"])
-            .order_by("pk")
-            .first()
-        )
+        next_reviewer = self._deduce_next_reviewer(destruction_list)
+        destruction_list.assign(next_reviewer)
+
+    def reassign(self, destruction_list: "DestructionList") -> None:
+        next_reviewer = self._deduce_next_reviewer(destruction_list)
         destruction_list.assign(next_reviewer)
 
 
@@ -63,6 +79,10 @@ def assign_next(self, destruction_list: "DestructionList") -> None:
         last_review = destruction_list.reviews.order_by("created").last()
         destruction_list.assign(destruction_list.get_assignee(last_review.author))
 
+    def reassign(self, destruction_list: "DestructionList") -> None:
+        # When a list has requested changes, it is assigned to the author. No action needed.
+        pass
+
 
 class InternallyReviewed:
     def assign_next(self, destruction_list: "DestructionList") -> None:
@@ -75,6 +95,10 @@ def assign_next(self, destruction_list: "DestructionList") -> None:
         )
         destruction_list.assign(archivist)
 
+    def reassign(self, destruction_list: "DestructionList") -> None:
+        # When a list has been internally reviewed, it is assigned to the author. No action needed.
+        pass
+
 
 class ReadyForArchivist:
     def assign_next(self, destruction_list: "DestructionList") -> None:
@@ -85,6 +109,10 @@ def assign_next(self, destruction_list: "DestructionList") -> None:
 
         # TODO in the case where the archivist rejects it is not clear yet what should happen!
 
+    def reassign(self, destruction_list: "DestructionList") -> None:
+        # TODO
+        raise NotImplementedError
+
 
 STATE_MANAGER = {
     ListStatus.new: NewList(),

backend/src/openarchiefbeheer/destruction/models.py

@@ -147,6 +147,9 @@ def get_current_assignee(self) -> Optional["DestructionListAssignee"]:
     def assign_next(self) -> None:
         STATE_MANAGER[self.status].assign_next(self)
 
+    def reassign(self) -> None:
+        STATE_MANAGER[self.status].reassign(self)
+
     def all_reviewers_approved(self) -> bool:
         number_of_reviewers = self.assignees.filter(role=ListRole.reviewer).count()
         latest_reviews = self.reviews.order_by("created")

backend/src/openarchiefbeheer/destruction/utils.py

@@ -3,13 +3,15 @@
 
 from django.conf import settings
 from django.core.mail import send_mail
+from django.db import transaction
+from django.db.models import Q
 
 from openarchiefbeheer.accounts.models import User
 from openarchiefbeheer.emails.models import EmailConfig
 from openarchiefbeheer.emails.render_backend import get_sandboxed_backend
 
 from .constants import InternalStatus
-from .models import DestructionList
+from .models import DestructionList, DestructionListAssignee
 
 
 def notify(subject: str, body: str, context: dict, recipients: list[str]) -> None:
@@ -124,3 +126,38 @@ def mark_as_failed_on_error(object_with_status: ObjectWithStatus):
     except Exception as exc:
         object_with_status.set_processing_status(InternalStatus.failed)
         raise exc
+
+
+def process_new_assignees(
+    destruction_list: DestructionList,
+    assignees: list[dict],
+    role: str,
+) -> list[DestructionListAssignee]:
+    """
+    Remove any assignees that are not present in the new assignees and create the new ones.
+
+    Example:
+    Before reassigning there are reviewerA, reviewerB and reviewerC.
+    The record manager requests that the new reviewers are reviewerB and reviewerD.
+    This function deletes reviewerA and reviewerC and creates reviewerD.
+    """
+    users = [assignee["user"] for assignee in assignees]
+
+    with transaction.atomic():
+        destruction_list.assignees.filter(~Q(user__in=users), role=role).delete()
+
+        existing_assignees_users = [
+            assignee.user.pk
+            for assignee in destruction_list.assignees.filter(role=role)
+        ]
+        assignees_to_create = []
+        for assignee in assignees:
+            if assignee["user"].pk not in existing_assignees_users:
+                assignees_to_create.append(assignee)
+
+        new_assignees = destruction_list.bulk_create_assignees(
+            assignees_to_create,
+            role,
+        )
+
+    return new_assignees