🚑 - fix: fix a bug where record manager was unable to access destruct…

…ion list with status change_requested created by another user
maykinmedia · Nov 19, 2024 · b1f0507 · b1f0507
1 parent f9901ce
commit b1f0507
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 18 deletions.
diff --git a/backend/src/openarchiefbeheer/destruction/api/serializers.py b/backend/src/openarchiefbeheer/destruction/api/serializers.py
@@ -748,12 +748,8 @@ class Meta:
 
     def validate(self, attrs: dict) -> dict:
         destruction_list = attrs["review"].destruction_list
-        request = self.context["request"]
 
-        if not (
-            request.user == destruction_list.author
-            and destruction_list.status == ListStatus.changes_requested
-        ):
+        if not (destruction_list.status == ListStatus.changes_requested):
             raise ValidationError(
                 _(
                     "This user is either not allowed to update the destruction list or "

diff --git a/backend/src/openarchiefbeheer/destruction/tests/endpoints/test_reviewresponse.py b/backend/src/openarchiefbeheer/destruction/tests/endpoints/test_reviewresponse.py
@@ -133,7 +133,7 @@ def test_create_review_response(self):
 
         self.assertEqual(item_response3.action_zaak["archiefactiedatum"], "2030-01-01")
 
-    def test_cannot_create_response_if_not_author(self):
+    def test_can_create_response_if_not_author(self):
         record_manager1 = UserFactory.create(post__can_start_destruction=True)
         record_manager2 = UserFactory.create(post__can_start_destruction=True)
 
@@ -156,14 +156,7 @@ def test_cannot_create_response_if_not_author(self):
             format="json",
         )
 
-        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-        self.assertEqual(
-            response.json()["nonFieldErrors"][0],
-            _(
-                "This user is either not allowed to update the destruction list or "
-                "the destruction list cannot currently be updated."
-            ),
-        )
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
 
     def test_cannot_create_response_if_not_changes_requested(self):
         record_manager = UserFactory.create(post__can_start_destruction=True)

diff --git a/frontend/src/lib/auth/permissions.test.ts b/frontend/src/lib/auth/permissions.test.ts
@@ -310,9 +310,13 @@ DESTRUCTION_LIST_STATUSES.forEach((status) => {
       expect(canUpdateDestructionList(user, destructionList)).toBe(false);
     });
 
-    test("should not allow a user to update if they are not the assignee", () => {
-      destructionList.assignee = anotherUser;
-      expect(canUpdateDestructionList(user, destructionList)).toBe(false);
+    test("should allow a user to update if they are not the author", () => {
+      expect(
+        canUpdateDestructionList(
+          user,
+          destructionListFactory({ status: "changes_requested" }),
+        ),
+      ).toBe(true);
     });
   });
 

diff --git a/frontend/src/lib/auth/permissions.ts b/frontend/src/lib/auth/permissions.ts
@@ -97,7 +97,7 @@ export function canUpdateDestructionList(
     return false;
   }
 
-  return user.pk === destructionList.assignee.pk;
+  return true;
 }
 
 export function canViewDestructionList(