✨ #453 - relax permission model so that all record managers can work …

…on all destruction lists

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

@@ -31,10 +31,7 @@ def has_permission(self, request, view):
         return request.user.has_perm("accounts.can_start_destruction")
 
     def has_object_permission(self, request, view, destruction_list):
-        return (
-            request.user == destruction_list.author
-            and destruction_list.status == ListStatus.new
-        )
+        return destruction_list.status == ListStatus.new
 
 
 class CanMarkListAsFinal(permissions.BasePermission):
@@ -47,10 +44,7 @@ def has_permission(self, request, view):
         return request.user.has_perm("accounts.can_start_destruction")
 
     def has_object_permission(self, request, view, destruction_list):
-        return (
-            request.user == destruction_list.author
-            and destruction_list.status == ListStatus.internally_reviewed
-        )
+        return destruction_list.status == ListStatus.internally_reviewed
 
 
 class CanTriggerDeletion(permissions.BasePermission):
@@ -63,10 +57,7 @@ def has_permission(self, request, view):
         return request.user.has_perm("accounts.can_start_destruction")
 
     def has_object_permission(self, request, view, destruction_list):
-        return (
-            request.user == destruction_list.author
-            and destruction_list.status == ListStatus.ready_to_delete
-        )
+        return destruction_list.status == ListStatus.ready_to_delete
 
 
 class CanReassignDestructionList(permissions.BasePermission):
@@ -76,7 +67,7 @@ def has_permission(self, request, view):
         return request.user.has_perm("accounts.can_start_destruction")
 
     def has_object_permission(self, request, view, destruction_list):
-        return request.user == destruction_list.author and destruction_list.status in [
+        return destruction_list.status in [
             ListStatus.new,
             ListStatus.ready_to_review,
         ]
@@ -89,10 +80,7 @@ def has_permission(self, request, view):
         return request.user.has_perm("accounts.can_start_destruction")
 
     def has_object_permission(self, request, view, destruction_list):
-        return (
-            request.user == destruction_list.author
-            and destruction_list.status == ListStatus.new
-        )
+        return destruction_list.status == ListStatus.new
 
 
 class CanAbortDestruction(permissions.BasePermission):
@@ -103,7 +91,6 @@ def has_permission(self, request, view):
 
     def has_object_permission(self, request, view, destruction_list):
         return (
-            request.user == destruction_list.author
-            and destruction_list.status == ListStatus.ready_to_delete
+            destruction_list.status == ListStatus.ready_to_delete
             and destruction_list.planned_destruction_date
         )

backend/src/openarchiefbeheer/destruction/tests/endpoints/test_abort_destruction.py

@@ -14,9 +14,9 @@
 
 
 class DestructionListAbortDestructionEndpointTest(APITestCase):
-    def test_only_author_can_abort(self):
-        record_manager = UserFactory.create(
-            username="record_manager", post__can_start_destruction=True
+    def test_only_record_manager_can_abort(self):
+        reviewer = UserFactory.create(
+            username="reviewer", post__can_start_destruction=False
         )
         destruction_list = DestructionListFactory.create(
             name="A test list",
@@ -25,7 +25,7 @@ def test_only_author_can_abort(self):
             planned_destruction_date=date(2024, 1, 8),
         )
 
-        self.client.force_authenticate(user=record_manager)
+        self.client.force_authenticate(user=reviewer)
         with freezegun.freeze_time("2024-01-05T12:00:00+01:00"):
             response = self.client.post(
                 reverse(

backend/src/openarchiefbeheer/destruction/tests/endpoints/test_list_destruction.py

@@ -109,7 +109,7 @@ def test_retry_destruction_after_failure_with_planned_date_in_future_raises_erro
             _("This list is already planned to be destroyed on 08/01/2024."),
         )
 
-    def test_cannot_start_destruction_if_not_author(self):
+    def test_can_start_destruction_if_not_author(self):
         record_manager = UserFactory.create(post__can_start_destruction=True)
         destruction_list = DestructionListFactory.create(
             name="A test list",
@@ -120,15 +120,14 @@ def test_cannot_start_destruction_if_not_author(self):
         self.client.force_authenticate(user=record_manager)
         with patch(
             "openarchiefbeheer.destruction.api.viewsets.delete_destruction_list"
-        ) as m_task:
+        ):
             response = self.client.delete(
                 reverse(
                     "api:destructionlist-detail", kwargs={"uuid": destruction_list.uuid}
                 ),
             )
 
-        self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code)
-        m_task.assert_not_called()
+        self.assertEqual(status.HTTP_204_NO_CONTENT, response.status_code)
 
     def test_cannot_start_destruction_if_not_ready_to_delete(self):
         record_manager = UserFactory.create(post__can_start_destruction=True)

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

@@ -277,7 +277,7 @@ def test_cannot_update_destruction_list_if_not_new(self):
 
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
 
-    def test_cannot_update_destruction_list_if_not_author(self):
+    def test_ca_update_destruction_list_if_not_author(self):
         record_manager1 = UserFactory.create(post__can_start_destruction=True)
         record_manager2 = UserFactory.create(post__can_start_destruction=True)
 
@@ -300,7 +300,7 @@ def test_cannot_update_destruction_list_if_not_author(self):
             format="json",
         )
 
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
 
     def test_cannot_reassign_destruction_list_if_not_record_manager(self):
         not_record_manager = UserFactory.create(post__can_start_destruction=False)
@@ -597,7 +597,7 @@ def test_cannot_mark_as_final_if_not_authenticated(self):
 
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
 
-    def test_cannot_mark_as_final_if_not_author(self):
+    def test_can_mark_as_final_if_not_author(self):
         record_manager = UserFactory.create(
             username="record_manager", post__can_start_destruction=True
         )
@@ -615,12 +615,10 @@ def test_cannot_mark_as_final_if_not_author(self):
             "api:destructionlist-make-final", kwargs={"uuid": destruction_list.uuid}
         )
         response = self.client.post(
-            endpoint,
-            data={"user": archivist.pk},
-            format="json",
+            endpoint, data={"user": archivist.pk, "comment": "comment"}, format="json"
         )
 
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
 
     def test_cannot_mark_as_finally_if_not_internally_reviewed(self):
         record_manager = UserFactory.create(

frontend/src/lib/auth/permissions.test.ts

@@ -225,9 +225,9 @@ DESTRUCTION_LIST_STATUSES.forEach((status) => {
       expect(canViewDestructionList(user, destructionList)).toBe(true);
     });
 
-    test("should not allow a user who is not the author to view the destruction list", () => {
+    test("should allow a user who is not the author to view the destruction list", () => {
       destructionList.author = anotherUser;
-      expect(canViewDestructionList(user, destructionList)).toBe(false);
+      expect(canViewDestructionList(user, destructionList)).toBe(true);
     });
   });
 
@@ -253,9 +253,14 @@ DESTRUCTION_LIST_STATUSES.forEach((status) => {
       expect(canMarkListAsFinal(user, destructionList)).toBe(isEligible);
     });
 
-    test("should not allow marking as final if the user is not the author", () => {
+    test("should allow marking as final if the user is not the author", () => {
       destructionList.author = anotherUser;
-      expect(canMarkListAsFinal(user, destructionList)).toBe(false);
+      expect(
+        canMarkListAsFinal(user, {
+          ...destructionList,
+          status: "internally_reviewed",
+        }),
+      ).toBe(true);
     });
 
     test("should not allow marking as final if the status is not internally_reviewed", () => {

frontend/src/lib/auth/permissions.ts

@@ -23,8 +23,7 @@ export function canMarkAsReadyToReview(
   destructionList: DestructionList,
 ) {
   return (
-    user.role.canStartDestruction &&
-    destructionList.author.pk === user.pk &&
+    canStartDestructionList(user) &&
     (destructionList.status === "new" ||
       destructionList.status === "changes_requested")
   );
@@ -80,19 +79,19 @@ export function canUpdateDestructionList(
 
 export function canViewDestructionList(
   user: User,
+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
   destructionList: DestructionList,
 ) {
-  return user.pk === destructionList.author.pk;
+  return canStartDestructionList(user);
 }
 
 export function canMarkListAsFinal(
   user: User,
   destructionList: DestructionList,
 ) {
   return (
-    user.pk === destructionList.author.pk &&
-    destructionList.status === "internally_reviewed" &&
-    user.role.canStartDestruction
+    canStartDestructionList(user) &&
+    destructionList.status === "internally_reviewed"
   );
 }
 
@@ -101,8 +100,7 @@ export function canTriggerDestruction(
   destructionList: DestructionList,
 ) {
   return (
-    user.pk === destructionList.author.pk &&
-    destructionList.status === "ready_to_delete" &&
-    user.role.canStartDestruction
+    canStartDestructionList(user) &&
+    destructionList.status === "ready_to_delete"
   );
 }

frontend/src/pages/destructionlist/detail/DestructionListDetail.action.ts

@@ -132,7 +132,14 @@ export async function destructionListProcessReadyToReviewAction({
   request,
 }: ActionFunctionArgs) {
   const { payload } = await request.json();
-  markDestructionListAsReadyToReview(payload.uuid);
+  try {
+    await markDestructionListAsReadyToReview(payload.uuid);
+  } catch (e: unknown) {
+    if (e instanceof Response) {
+      return await (e as Response).json();
+    }
+    throw e;
+  }
   return redirect("/");
 }