🎨 - refactor: translations and improvementent of audit log text

Julian Roeland · svenvandescheur · commit 5e1cc0b88923 · 2024-11-19T09:01:01.000+01:00
diff --git a/backend/src/openarchiefbeheer/conf/locale/en/LC_MESSAGES/django.po b/backend/src/openarchiefbeheer/conf/locale/en/LC_MESSAGES/django.po
diff --git a/backend/src/openarchiefbeheer/conf/locale/nl/LC_MESSAGES/django.po b/backend/src/openarchiefbeheer/conf/locale/nl/LC_MESSAGES/django.po
diff --git a/backend/src/openarchiefbeheer/destruction/tests/endpoints/test_abort_destruction.py b/backend/src/openarchiefbeheer/destruction/tests/endpoints/test_abort_destruction.py
@@ -127,8 +127,8 @@ def test_abort_list_destruction(self):
         self.assertEqual(
             message,
             _(
-                'The destruction of destruction list "%(list_name)s" was aborted by record '
-                'manager %(record_manager)s with reason: "%(comment)s".'
+                'User "%(record_manager)s" with the role of "" has aborted the destruction of destruction list "%(list_name)s" '
+                'with reason: "%(comment)s".'
             )
             % {
                 "list_name": "A test list",
diff --git a/backend/src/openarchiefbeheer/destruction/tests/endpoints/test_co_reviewers.py b/backend/src/openarchiefbeheer/destruction/tests/endpoints/test_co_reviewers.py
@@ -130,10 +130,11 @@ def test_fully_update_co_reviewers(self):
         message = logs[0].get_message()
         self.assertIn(
             _(
-                'User "%(user)s" has replaced all the co-reviewers of the list '
+                'User "%(user)s" with the role of "%(role)s" has replaced all the co-reviewers of the list '
                 '"%(list_name)s" with: %(added_co_reviewers)s.'
             )
             % {
+                "role": "",
                 "user": main_reviewer.user,
                 "list_name": "A beautiful list",
                 "added_co_reviewers": ", ".join(
@@ -245,10 +246,11 @@ def test_partially_update_co_reviewers(self):
         message = logs[0].get_message()
         self.assertIn(
             _(
-                'User "%(user)s" has added these users as co-reviewers to the list '
+                'User "%(user)s" with the role of "%(role)s" has added these users as co-reviewers to the list '
                 '"%(list_name)s": %(added_co_reviewers)s.'
             )
             % {
+                "role": "",
                 "user": main_reviewer.user,
                 "list_name": "A beautiful list",
                 "added_co_reviewers": ", ".join(
diff --git a/backend/src/openarchiefbeheer/destruction/tests/endpoints/test_reviewresponse.py b/backend/src/openarchiefbeheer/destruction/tests/endpoints/test_reviewresponse.py
@@ -237,8 +237,10 @@ def test_audit_log(self):
         )
         self.assertEqual(
             data[0]["message"],
-            _('Destruction list "%(list_name)s" was reassigned.')
-            % {"list_name": "Test audittrail"},
+            _(
+                'User "%(user)s" with the role of "" has reassigned destruction list "%(list_name)s".'
+            )
+            % {"list_name": "Test audittrail", "user": record_manager},
         )
         self.assertEqual(
             data[0]["extra_data"]["assignee"]["user"],
diff --git a/backend/src/openarchiefbeheer/destruction/tests/test_endpoints.py b/backend/src/openarchiefbeheer/destruction/tests/test_endpoints.py
@@ -567,10 +567,11 @@ def test_mark_as_final(self):
         self.assertEqual(
             message.strip("\n"),
             _(
-                'Destruction list "%(list_name)s" was made final by the record manager '
-                "%(record_manager)s who assigned it to the archivist %(archivist)s."
+                'User "%(user)s" with the role of "" has made destruction list "%(list_name)s" final and '
+                "assigned it to the archivist %(archivist)s."
             )
             % {
+                "user": record_manager,
                 "list_name": "A test list",
                 "record_manager": "record_manager",
                 "archivist": "archivist",
diff --git a/backend/src/openarchiefbeheer/destruction/tests/test_serializers.py b/backend/src/openarchiefbeheer/destruction/tests/test_serializers.py
@@ -111,7 +111,9 @@ def test_create_destruction_list(self):
 
         self.assertEqual(
             message,
-            _('Destruction list "%(list_name)s" created by user %(author)s.')
+            _(
+                'User "%(author)s" with the role of "" has created destruction list "%(list_name)s".'
+            )
             % {"list_name": "A test list", "author": "Jeffrey Jones (record_manager)"},
         )
 
@@ -264,8 +266,10 @@ def test_full_list_update(self):
 
         self.assertEqual(
             message,
-            _('Destruction list "%(list_name)s" was updated.')
-            % {"list_name": "An updated test list"},
+            _(
+                'User "%(user)s" with the role of "" has updated destruction list "%(list_name)s".'
+            )
+            % {"list_name": "An updated test list", "user": record_manager},
         )
 
     def test_partial_list_update(self):
@@ -1044,7 +1048,7 @@ def test_create_review_rejected(self):
         self.assertFalse(logs[0].extra_data["approved"])
         self.assertEqual(
             logs[0].get_message(),
-            'User "Jeffrey Jones (reviewer)" has reviewed the list "Test list". The destruction list was rejected.',
+            'User "Jeffrey Jones (reviewer)" with the role of "" has reviewed the list "Test list". The destruction list was rejected.',
         )
 
     def test_reviewing_cases_not_in_destruction_list(self):
diff --git a/backend/src/openarchiefbeheer/logging/logevent.py b/backend/src/openarchiefbeheer/logging/logevent.py
@@ -1,9 +1,8 @@
 import traceback
 
-from django.db.models import Model
-
 from timeline_logger.models import TimelineLog
 
+from openarchiefbeheer.accounts.api.serializers import UserSerializer
 from openarchiefbeheer.accounts.models import User
 from openarchiefbeheer.destruction.models import (
     DestructionList,
@@ -13,22 +12,34 @@
 )
 
 
-def _create_log(
-    model: Model, event: str, extra_data: dict | None = None, user: User | None = None
+def _create_log_destruction_list(
+    destruction_list: DestructionList,
+    event: str,
+    extra_data: dict | None = None,
+    user: User | None = None,
 ) -> TimelineLog:
+    serialized_user = UserSerializer(user).data if user else {}
+    user_groups = [group.name for group in user.groups.all()] if user else []
+
+    combined_extra_data = {
+        **(extra_data or {}),
+        "user": serialized_user,
+        "groups": user_groups,
+    }
+
     return TimelineLog.objects.create(
-        content_object=model,
+        content_object=destruction_list,
         template=f"logging/{event}.txt",
-        extra_data=extra_data,
+        extra_data=combined_extra_data,
         user=user,
     )
 
 
 def destruction_list_created(
     destruction_list: DestructionList, author: User, reviewer: User
 ) -> None:
-    _create_log(
-        model=destruction_list,
+    _create_log_destruction_list(
+        destruction_list=destruction_list,
         event="destruction_list_created",
         user=author,
         extra_data={
@@ -44,7 +55,9 @@ def destruction_list_created(
 
 
 def destruction_list_updated(destruction_list: DestructionList, user: User) -> None:
-    _create_log(model=destruction_list, event="destruction_list_updated", user=user)
+    _create_log_destruction_list(
+        destruction_list=destruction_list, event="destruction_list_updated", user=user
+    )
 
 
 def destruction_list_reassigned(
@@ -53,8 +66,8 @@ def destruction_list_reassigned(
     comment: str,
     user: User,
 ) -> None:
-    _create_log(
-        model=destruction_list,
+    _create_log_destruction_list(
+        destruction_list=destruction_list,
         event="destruction_list_reassigned",
         user=user,
         extra_data={
@@ -78,8 +91,8 @@ def destruction_list_co_reviewers_added(
     comment: str,
     user: User,
 ):
-    _create_log(
-        model=destruction_list,
+    _create_log_destruction_list(
+        destruction_list=destruction_list,
         event="destruction_list_co_reviewers_added",
         user=user,
         extra_data={
@@ -100,8 +113,8 @@ def destruction_list_co_reviewers_added(
 def destruction_list_reviewed(
     destruction_list: DestructionList, review: DestructionListReview, user: User
 ) -> None:
-    _create_log(
-        model=destruction_list,
+    _create_log_destruction_list(
+        destruction_list=destruction_list,
         event="destruction_list_reviewed",
         user=user,
         extra_data={"approved": review.decision == ReviewDecisionChoices.accepted},
@@ -114,8 +127,8 @@ def destruction_list_finalized(
     archivist: User,
     record_manager: User,
 ) -> None:
-    _create_log(
-        model=destruction_list,
+    _create_log_destruction_list(
+        destruction_list=destruction_list,
         event="destruction_list_finalized",
         user=record_manager,
         extra_data={
@@ -136,8 +149,8 @@ def destruction_list_aborted(
     comment: str,
     record_manager: User,
 ):
-    _create_log(
-        model=destruction_list,
+    _create_log_destruction_list(
+        destruction_list=destruction_list,
         event="destruction_list_aborted",
         user=record_manager,
         extra_data={
diff --git a/backend/src/openarchiefbeheer/logging/templates/logging/destruction_list_aborted.txt b/backend/src/openarchiefbeheer/logging/templates/logging/destruction_list_aborted.txt
@@ -1,3 +1,3 @@
-{% load i18n %}{% blocktranslate trimmed with list_name=log.content_object.name record_manager=log.user comment=log.extra_data.comment %}
-The destruction of destruction list "{{ list_name }}" was aborted by record manager {{ record_manager }} with reason: "{{ comment }}".
+{% load i18n %}{% blocktranslate trimmed with list_name=log.content_object.name role=log.extra_data.groups.0 record_manager=log.user comment=log.extra_data.comment %}
+User "{{ record_manager }}" with the role of "{{ role }}" has aborted the destruction of destruction list "{{ list_name }}" with reason: "{{ comment }}".
 {% endblocktranslate %}
diff --git a/backend/src/openarchiefbeheer/logging/templates/logging/destruction_list_co_reviewers_added.txt b/backend/src/openarchiefbeheer/logging/templates/logging/destruction_list_co_reviewers_added.txt
@@ -1,16 +1,16 @@
 {% load i18n %}{% with added_co_reviewers=log.extra_data.added_co_reviewers|join:", "  %}{% with removed_co_reviewers=log.extra_data.removed_co_reviewers|join:", "  %}
 {% if log.extra_data.partial %}
-{% blocktranslate trimmed with list_name=log.content_object.name user=log.user added_co_reviewers=added_co_reviewers %}
-User "{{ user }}" has added these users as co-reviewers to the list "{{ list_name }}": {{ added_co_reviewers }}.
+{% blocktranslate trimmed with role=log.extra_data.groups.0 list_name=log.content_object.name user=log.user added_co_reviewers=added_co_reviewers %}
+User "{{ user }}" with the role of "{{ role }}" has added these users as co-reviewers to the list "{{ list_name }}": {{ added_co_reviewers }}.
 {% endblocktranslate %}
 {% if removed_co_reviewers %}
 {% blocktranslate trimmed with removed_co_reviewers=removed_co_reviewers %}
 They also removed these co-reviewers: {{ removed_co_reviewers }}.
 {% endblocktranslate %}
 {% endif %}
 {% else %}
-{% blocktranslate trimmed with list_name=log.content_object.name user=log.user added_co_reviewers=added_co_reviewers %}
-User "{{ user }}" has replaced all the co-reviewers of the list "{{ list_name }}" with: {{ added_co_reviewers }}.
+{% blocktranslate trimmed with list_name=log.content_object.name role=log.extra_data.groups.0 user=log.user added_co_reviewers=added_co_reviewers %}
+User "{{ user }}" with the role of "{{ role }}" has replaced all the co-reviewers of the list "{{ list_name }}" with: {{ added_co_reviewers }}.
 {% endblocktranslate %}
 {% endif %}
 
diff --git a/backend/src/openarchiefbeheer/logging/templates/logging/destruction_list_created.txt b/backend/src/openarchiefbeheer/logging/templates/logging/destruction_list_created.txt
@@ -1,3 +1,3 @@
-{% load i18n %}{% blocktranslate trimmed with list_name=log.content_object.name author=log.user %}
-Destruction list "{{ list_name }}" created by user {{ author }}.
+{% load i18n %}{% blocktranslate trimmed with list_name=log.content_object.name role=log.extra_data.groups.0 author=log.user %}
+User "{{ author }}" with the role of "{{ role }}" has created destruction list "{{ list_name }}".
 {% endblocktranslate %}
diff --git a/backend/src/openarchiefbeheer/logging/templates/logging/destruction_list_finalized.txt b/backend/src/openarchiefbeheer/logging/templates/logging/destruction_list_finalized.txt
@@ -1,4 +1,4 @@
 {% load i18n %}
-{% blocktranslate trimmed with list_name=log.content_object.name record_manager=log.user.username archivist=log.extra_data.archivist.username %}
-   Destruction list "{{ list_name }}" was made final by the record manager {{ record_manager }} who assigned it to the archivist {{ archivist }}.
+{% blocktranslate trimmed with list_name=log.content_object.name role=log.extra_data.groups.0 user=log.user record_manager=log.user.username archivist=log.extra_data.archivist.username %}
+    User "{{ user }}" with the role of "{{ role }}" has made destruction list "{{ list_name }}" final and assigned it to the archivist {{ archivist }}.
 {% endblocktranslate %}
diff --git a/backend/src/openarchiefbeheer/logging/templates/logging/destruction_list_reassigned.txt b/backend/src/openarchiefbeheer/logging/templates/logging/destruction_list_reassigned.txt
@@ -1,3 +1,3 @@
-{% load i18n %}{% blocktranslate trimmed with timestamp=log.timestamp|date:"c" list_name=log.content_object.name %}
-   Destruction list "{{ list_name }}" was reassigned.
+{% load i18n %}{% blocktranslate trimmed with timestamp=log.timestamp|date:"c" list_name=log.content_object.name role=log.extra_data.groups.0 user=log.user %}
+    User "{{ user }}" with the role of "{{ role }}" has reassigned destruction list "{{ list_name }}".
 {% endblocktranslate %}
diff --git a/backend/src/openarchiefbeheer/logging/templates/logging/destruction_list_reviewed.txt b/backend/src/openarchiefbeheer/logging/templates/logging/destruction_list_reviewed.txt
@@ -1,2 +1,2 @@
-{% load i18n %}{% blocktranslate trimmed with timestamp=log.timestamp|date:"c" list_name=log.content_object.name user=log.user %}
-   User "{{ user }}" has reviewed the list "{{ list_name }}". {% endblocktranslate %}{% if log.extra_data.approved %}{% translate " The destruction list was approved." %}{% else %}{% translate " The destruction list was rejected." %}{% endif %}
+{% load i18n %}{% blocktranslate trimmed with timestamp=log.timestamp|date:"c" list_name=log.content_object.name role=log.extra_data.groups.0 user=log.user %}
+   User "{{ user }}" with the role of "{{ role }}" has reviewed the list "{{ list_name }}". {% endblocktranslate %}{% if log.extra_data.approved %}{% translate " The destruction list was approved." %}{% else %}{% translate " The destruction list was rejected." %}{% endif %}
diff --git a/backend/src/openarchiefbeheer/logging/templates/logging/destruction_list_updated.txt b/backend/src/openarchiefbeheer/logging/templates/logging/destruction_list_updated.txt
@@ -1,3 +1,3 @@
-{% load i18n %}{% blocktranslate trimmed with list_name=log.content_object.name %}
-   Destruction list "{{ list_name }}" was updated.
+{% load i18n %}{% blocktranslate trimmed with list_name=log.content_object.name role=log.extra_data.groups.0 user=log.user %}
+    User "{{ user }}" with the role of "{{ role }}" has updated destruction list "{{ list_name }}".
 {% endblocktranslate %}
diff --git a/frontend/src/components/DestructionListAuditLog/DestructionListAuditLog.tsx b/frontend/src/components/DestructionListAuditLog/DestructionListAuditLog.tsx
@@ -2,7 +2,7 @@ import { DataGrid } from "@maykin-ui/admin-ui";
 
 import { useAuditLog } from "../../hooks";
 import { DestructionList } from "../../lib/api/destructionLists";
-import { formatDate } from "../../lib/format/date";
+import { formatDateAndTime } from "../../lib/format/date";
 import { formatUser } from "../../lib/format/user";
 
 /**
@@ -16,7 +16,7 @@ export function DestructionListAuditLog({
   const logItems = useAuditLog(destructionList);
 
   const objectList = logItems.map((logItem) => ({
-    Datum: formatDate(logItem.timestamp),
+    Datum: formatDateAndTime(logItem.timestamp),
     "Gewijzigd door": formatUser(logItem.user),
     Wijziging: logItem.message,
   }));
diff --git a/frontend/src/lib/format/date.ts b/frontend/src/lib/format/date.ts
@@ -14,6 +14,23 @@ export function formatDate(date: Date | string, format: "nl" | "iso" = "nl") {
   return _format(_date, "dd/MM/yyyy");
 }
 
+/**
+ * Formats date and time.
+ * @param date
+ * @param format
+ * @returns Formatted date and time.
+ */
+export function formatDateAndTime(
+  date: Date | string,
+  format: "nl" | "iso" = "nl",
+) {
+  const _date = new Date(date);
+  if (format === "iso") {
+    return formatISO(date, { representation: "date" });
+  }
+  return _format(_date, "dd/MM/yyyy HH:mm");
+}
+
 /**
  * Options for customizing the output of the timeAgo function.
  */

Original file line number	Diff line number	Diff line change
`@@ -237,8 +237,10 @@ def test_audit_log(self):`
`237`	`237`	`)`
`238`	`238`	`self.assertEqual(`
`239`	`239`	`data[0]["message"],`
`240`		`- _('Destruction list "%(list_name)s" was reassigned.')`
`241`		`- % {"list_name": "Test audittrail"},`
	`240`	`+ _(`
	`241`	`+ 'User "%(user)s" with the role of "" has reassigned destruction list "%(list_name)s".'`
	`242`	`+ )`
	`243`	`+ % {"list_name": "Test audittrail", "user": record_manager},`
`242`	`244`	`)`
`243`	`245`	`self.assertEqual(`
`244`	`246`	`data[0]["extra_data"]["assignee"]["user"],`