Merge pull request #526 from maykinmedia/feature/502-review-process-report

[#502] Add review process to destruction report

Author: SilviaAmAm

backend/src/openarchiefbeheer/accounts/models.py

@@ -5,6 +5,7 @@
 from django.utils.translation import gettext_lazy as _
 
 from .managers import UserManager
+from .utils import format_user
 
 
 class User(AbstractBaseUser, PermissionsMixin):
@@ -68,9 +69,4 @@ def get_short_name(self):
         return self.first_name
 
     def get_name_with_username(self):
-        username = self.get_username()
-        if self.first_name or self.last_name:
-            full_name = self.get_full_name()
-            return f"{full_name} ({username})"
-
-        return username
+        return format_user(self)

backend/src/openarchiefbeheer/accounts/utils.py

@@ -1,11 +1,15 @@
+from dataclasses import dataclass
+from typing import TYPE_CHECKING, Union
+
 from django.core.exceptions import ValidationError
 from django.utils.translation import gettext_lazy as _
 
-from .models import User
+if TYPE_CHECKING:
+    from .models import User
 
 
 def validate_max_permissions(
-    current_user: User, permissions=None, groups=None, is_superuser=None
+    current_user: "User", permissions=None, groups=None, is_superuser=None
 ):
     """
     Validate that the ``permissions``, ``groups`` and ``superuser``-flag are
@@ -45,7 +49,7 @@ def validate_max_permissions(
         )
 
 
-def validate_max_user_permissions(current_user: User, target_user: User):
+def validate_max_user_permissions(current_user: "User", target_user: "User") -> None:
     """
     Validate that the ``target_user`` permissions are never more than the
     permissions of the ``current_user``.
@@ -56,3 +60,31 @@ def validate_max_user_permissions(current_user: User, target_user: User):
         target_user.groups,
         target_user.is_superuser,
     )
+
+
+@dataclass
+class UserData:
+    username: str
+    first_name: str = ""
+    last_name: str = ""
+
+
+def format_user(user: Union[dict, "User"]) -> str:
+    if isinstance(user, dict):
+        user = UserData(
+            **{
+                "username": user["username"],
+                "first_name": user.get("first_name"),
+                "last_name": user.get("last_name"),
+            }
+        )
+
+    if user.first_name or user.last_name:
+        full_name = "%s %s" % (user.first_name, user.last_name)
+        return f"{full_name.strip()} ({user.username})"
+
+    return user.username
+
+
+def format_user_groups(groups: list[str]) -> str:
+    return ", ".join(groups)

backend/src/openarchiefbeheer/destruction/destruction_report.py

@@ -1,12 +1,16 @@
 from dataclasses import dataclass
 from typing import IO
 
-from django.utils.translation import gettext
+from django.utils.translation import gettext as _
 
 import xlsxwriter
 from glom import glom
+from timeline_logger.models import TimelineLog
 from xlsxwriter.worksheet import Worksheet
 
+from openarchiefbeheer.accounts.utils import format_user, format_user_groups
+from openarchiefbeheer.logging.logevent import destruction_list_reviewed
+from openarchiefbeheer.logging.utils import get_event_template, get_readable_timestamp
 from openarchiefbeheer.zaken.api.constants import ZAAK_METADATA_FIELDS_MAPPINGS
 
 from .constants import InternalStatus
@@ -17,6 +21,33 @@
 class DestructionReportGenerator:
     destruction_list: DestructionList
 
+    def add_review_process_table(
+        self, worksheet: Worksheet, start_row: int = 0
+    ) -> None:
+        column_names = [
+            _("Group"),
+            _("Name"),
+            _("Date/Time"),
+            _("Changes"),
+        ]
+        worksheet.write_row(start_row, 0, column_names)
+
+        logs = TimelineLog.objects.for_object(self.destruction_list).filter(
+            template=get_event_template(destruction_list_reviewed),
+            extra_data__approved=True,
+        )
+        for row_count, log in enumerate(logs):
+            # Not using the FK because the user might have been deleted in the mean time
+            data = [
+                format_user_groups(log.extra_data["user_groups"]),
+                format_user(log.extra_data["user"]),
+                get_readable_timestamp(log),
+                # This column is not useful, since we are filtering on approved reviews.
+                # But it was specifically requested.
+                _("Has approved"),
+            ]
+            worksheet.write_row(start_row + row_count + 1, 0, data)
+
     def add_zaken_table(self, worksheet: Worksheet, start_row: int = 0) -> None:
         worksheet.write_row(
             start_row, 0, [field["name"] for field in ZAAK_METADATA_FIELDS_MAPPINGS]
@@ -36,8 +67,10 @@ def add_zaken_table(self, worksheet: Worksheet, start_row: int = 0) -> None:
     def generate_destruction_report(self, file: IO) -> None:
         workbook = xlsxwriter.Workbook(file.name, options={"in_memory": False})
 
-        worksheet = workbook.add_worksheet(name=gettext("Deleted zaken"))
+        worksheet_zaken = workbook.add_worksheet(name=_("Deleted zaken"))
+        worksheet_review_process = workbook.add_worksheet(name=_("Review process"))
 
-        self.add_zaken_table(worksheet)
+        self.add_zaken_table(worksheet_zaken)
+        self.add_review_process_table(worksheet_review_process)
 
         workbook.close()

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

@@ -1,6 +1,7 @@
 from datetime import date, datetime
 from unittest.mock import patch
 
+from django.contrib.auth.models import Group
 from django.core.exceptions import ObjectDoesNotExist
 from django.test import TestCase
 from django.utils import timezone
@@ -15,16 +16,23 @@
 from zgw_consumers.test.factories import ServiceFactory
 
 from openarchiefbeheer.config.models import ArchiveConfig
+from openarchiefbeheer.logging import logevent
 from openarchiefbeheer.utils.results_store import ResultStore
 from openarchiefbeheer.zaken.models import Zaak
 from openarchiefbeheer.zaken.tests.factories import ZaakFactory
 
 from ...accounts.tests.factories import UserFactory
-from ..constants import InternalStatus, ListItemStatus, ListStatus
+from ..constants import (
+    InternalStatus,
+    ListItemStatus,
+    ListStatus,
+    ReviewDecisionChoices,
+)
 from .factories import (
     DestructionListCoReviewFactory,
     DestructionListFactory,
     DestructionListItemFactory,
+    DestructionListReviewFactory,
     ReviewResponseFactory,
 )
 
@@ -408,6 +416,82 @@ def test_generate_destruction_report(self):
             ),
         )
 
+    def test_generate_destruction_report_review_process(self):
+        author = UserFactory.create(post__can_start_destruction=True)
+        reviewer = UserFactory.create(
+            first_name="John",
+            last_name="Doe",
+            username="jdoe1",
+            post__can_review_destruction=True,
+        )
+        reviewer_group, created = Group.objects.get_or_create(name="Reviewer")
+        reviewer.groups.add(reviewer_group)
+        archivist = UserFactory.create(
+            first_name="Alice",
+            last_name="Wonderland",
+            username="awonderland1",
+            post__can_review_final_list=True,
+        )
+        archivist_group, created = Group.objects.get_or_create(name="Archivist")
+        archivist.groups.add(archivist_group)
+
+        destruction_list = DestructionListFactory.create(
+            status=ListStatus.deleted, author=author
+        )
+
+        review_reviewer_rejected = DestructionListReviewFactory.create(
+            author=reviewer, decision=ReviewDecisionChoices.rejected
+        )
+        review_reviewer_accepted = DestructionListReviewFactory.create(
+            author=reviewer, decision=ReviewDecisionChoices.accepted
+        )
+        review_archivist_accepted = DestructionListReviewFactory.create(
+            author=archivist, decision=ReviewDecisionChoices.accepted
+        )
+
+        logevent.destruction_list_created(
+            destruction_list, author, reviewer
+        )  # A log with a different template (should NOT be present in the report)
+
+        logevent.destruction_list_reviewed(
+            destruction_list, review_reviewer_rejected, reviewer
+        )  # A rejection (should NOT be present in the report)
+        with freeze_time("2024-05-02T16:00:00+02:00"):
+            logevent.destruction_list_reviewed(
+                destruction_list, review_reviewer_accepted, reviewer
+            )
+        with freeze_time("2024-05-08T09:45:00+02:00"):
+            logevent.destruction_list_reviewed(
+                destruction_list, review_archivist_accepted, archivist
+            )
+
+        destruction_list.generate_destruction_report()
+
+        wb = load_workbook(filename=destruction_list.destruction_report.path)
+        sheet_deleted_zaken = wb[gettext("Review process")]
+        rows = list(sheet_deleted_zaken.iter_rows(values_only=True))
+
+        self.assertEqual(len(rows), 3)
+
+        self.assertEqual(
+            rows[1],
+            (
+                "Reviewer",
+                "John Doe (jdoe1)",
+                "2024-05-02 16:00+02:00",
+                gettext("Has approved"),
+            ),
+        )
+        self.assertEqual(
+            rows[2],
+            (
+                "Archivist",
+                "Alice Wonderland (awonderland1)",
+                "2024-05-08 09:45+02:00",
+                gettext("Has approved"),
+            ),
+        )
+
     def test_zaak_creation_skipped_if_internal_status_succeeded(self):
         destruction_list = DestructionListFactory.create(
             processing_status=InternalStatus.succeeded

backend/src/openarchiefbeheer/logging/logevent.py

@@ -19,6 +19,8 @@
     format_zaaktype_choices,
 )
 
+TEMPLATE_FORMAT = "logging/%(event)s.txt"
+
 
 def _create_log(
     model: Model, event: str, extra_data: dict | None = None, user: User | None = None
@@ -34,7 +36,7 @@ def _create_log(
 
     return TimelineLog.objects.create(
         content_object=model,
-        template=f"logging/{event}.txt",
+        template=TEMPLATE_FORMAT % {"event": event},
         extra_data=extra_data,
         user=user,
     )

backend/src/openarchiefbeheer/logging/utils.py

@@ -0,0 +1,19 @@
+from typing import Callable
+
+from django.utils import timezone
+
+from timeline_logger.models import TimelineLog
+
+from .logevent import TEMPLATE_FORMAT
+
+
+def get_event_template(logging_func: Callable) -> str:
+    return TEMPLATE_FORMAT % {"event": logging_func.__name__}
+
+
+def get_readable_timestamp(
+    log: TimelineLog, separator: str = " ", timespec: str = "minutes"
+) -> str:
+    return log.timestamp.astimezone(tz=timezone.get_default_timezone()).isoformat(
+        sep=separator, timespec=timespec
+    )