src/documents/permissions.py

@@ -70,59 +70,57 @@ def set_permissions_for_object(permissions: list[str], object, *, merge: bool =
 
     for action in permissions:
         permission = f"{action}_{object.__class__.__name__.lower()}"
-        if "users" in permissions[action]:
+        # users
-            # users
+        users_to_add = User.objects.filter(id__in=permissions[action]["users"])
-            users_to_add = User.objects.filter(id__in=permissions[action]["users"])
+        users_to_remove = (
-            users_to_remove = (
+            get_users_with_perms(
-                get_users_with_perms(
+                object,
-                    object,
+                only_with_perms_in=[permission],
-                    only_with_perms_in=[permission],
+                with_group_users=False,
-                    with_group_users=False,
-                )
-                if not merge
-                else User.objects.none()
             )
-            if len(users_to_add) > 0 and len(users_to_remove) > 0:
+            if not merge
-                users_to_remove = users_to_remove.exclude(id__in=users_to_add)
+            else User.objects.none()
-            if len(users_to_remove) > 0:
+        )
-                for user in users_to_remove:
+        if len(users_to_add) > 0 and len(users_to_remove) > 0:
-                    remove_perm(permission, user, object)
+            users_to_remove = users_to_remove.exclude(id__in=users_to_add)
-            if len(users_to_add) > 0:
+        if len(users_to_remove) > 0:
-                for user in users_to_add:
+            for user in users_to_remove:
-                    assign_perm(permission, user, object)
+                remove_perm(permission, user, object)
-                    if action == "change":
+        if len(users_to_add) > 0:
-                        # change gives view too
+            for user in users_to_add:
-                        assign_perm(
+                assign_perm(permission, user, object)
-                            f"view_{object.__class__.__name__.lower()}",
+                if action == "change":
-                            user,
+                    # change gives view too
-                            object,
+                    assign_perm(
-                        )
+                        f"view_{object.__class__.__name__.lower()}",
-        if "groups" in permissions[action]:
+                        user,
-            # groups
+                        object,
-            groups_to_add = Group.objects.filter(id__in=permissions[action]["groups"])
+                    )
-            groups_to_remove = (
+        # groups
-                get_groups_with_only_permission(
+        groups_to_add = Group.objects.filter(id__in=permissions[action]["groups"])
-                    object,
+        groups_to_remove = (
-                    permission,
+            get_groups_with_only_permission(
-                )
+                object,
-                if not merge
+                permission,
-                else Group.objects.none()
             )
-            if len(groups_to_add) > 0 and len(groups_to_remove) > 0:
+            if not merge
-                groups_to_remove = groups_to_remove.exclude(id__in=groups_to_add)
+            else Group.objects.none()
-            if len(groups_to_remove) > 0:
+        )
-                for group in groups_to_remove:
+        if len(groups_to_add) > 0 and len(groups_to_remove) > 0:
-                    remove_perm(permission, group, object)
+            groups_to_remove = groups_to_remove.exclude(id__in=groups_to_add)
-            if len(groups_to_add) > 0:
+        if len(groups_to_remove) > 0:
-                for group in groups_to_add:
+            for group in groups_to_remove:
-                    assign_perm(permission, group, object)
+                remove_perm(permission, group, object)
-                    if action == "change":
+        if len(groups_to_add) > 0:
-                        # change gives view too
+            for group in groups_to_add:
-                        assign_perm(
+                assign_perm(permission, group, object)
-                            f"view_{object.__class__.__name__.lower()}",
+                if action == "change":
-                            group,
+                    # change gives view too
-                            object,
+                    assign_perm(
-                        )
+                        f"view_{object.__class__.__name__.lower()}",
+                        group,
+                        object,
+                    )
 
 
 def get_objects_for_user_owner_aware(user, perms, Model) -> QuerySet:

src/documents/serialisers.py

@@ -160,24 +160,24 @@ class SetPermissionsMixin:
 
     def validate_set_permissions(self, set_permissions=None):
         permissions_dict = {
-            "view": {},
+            "view": {
-            "change": {},
+                "users": User.objects.none(),
+                "groups": Group.objects.none(),
+            },
+            "change": {
+                "users": User.objects.none(),
+                "groups": Group.objects.none(),
+            },
         }
         if set_permissions is not None:
-            for action in ["view", "change"]:
+            for action, _ in permissions_dict.items():
                 if action in set_permissions:
-                    if "users" in set_permissions[action]:
+                    users = set_permissions[action]["users"]
-                        users = set_permissions[action]["users"]
+                    permissions_dict[action]["users"] = self._validate_user_ids(users)
-                        permissions_dict[action]["users"] = self._validate_user_ids(
+                    groups = set_permissions[action]["groups"]
-                            users,
+                    permissions_dict[action]["groups"] = self._validate_group_ids(
-                        )
+                        groups,
-                    if "groups" in set_permissions[action]:
+                    )
-                        groups = set_permissions[action]["groups"]
-                        permissions_dict[action]["groups"] = self._validate_group_ids(
-                            groups,
-                        )
-                else:
-                    del permissions_dict[action]
         return permissions_dict
 
     def _set_permissions(self, permissions, object):

src/documents/signals/handlers.py

@@ -1162,7 +1162,7 @@ def run_workflows(
                 ) as f:
                     files = {
                         "file": (
-                            filename,
+                            document.original_filename,
                             f.read(),
                             document.mime_type,
                         ),

src/documents/tests/test_api_permissions.py

@@ -395,52 +395,6 @@ class TestApiAuth(DirectoriesMixin, APITestCase):
         self.assertTrue(checker.has_perm("view_document", doc))
         self.assertIn("view_document", get_perms(group1, doc))
 
-    def test_patch_doesnt_remove_permissions(self):
-        """
-        GIVEN:
-            - existing document with permissions set
-        WHEN:
-            - PATCH API request to update doc that is not json
-        THEN:
-            - Object permissions are not removed
-        """
-        doc = Document.objects.create(
-            title="test",
-            mime_type="application/pdf",
-            content="this is a document",
-        )
-        user1 = User.objects.create_superuser(username="user1")
-        user2 = User.objects.create(username="user2")
-        group1 = Group.objects.create(name="group1")
-        doc.owner = user1
-        doc.save()
-
-        assign_perm("view_document", user2, doc)
-        assign_perm("change_document", user2, doc)
-        assign_perm("view_document", group1, doc)
-        assign_perm("change_document", group1, doc)
-
-        self.client.force_authenticate(user1)
-
-        response = self.client.patch(
-            f"/api/documents/{doc.id}/",
-            {
-                "archive_serial_number": "123",
-            },
-        )
-
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        doc = Document.objects.get(pk=doc.id)
-
-        self.assertEqual(doc.owner, user1)
-        from guardian.core import ObjectPermissionChecker
-
-        checker = ObjectPermissionChecker(user2)
-        self.assertTrue(checker.has_perm("view_document", doc))
-        self.assertIn("view_document", get_perms(group1, doc))
-        self.assertTrue(checker.has_perm("change_document", doc))
-        self.assertIn("change_document", get_perms(group1, doc))
-
     def test_dynamic_permissions_fields(self):
         user1 = User.objects.create_user(username="user1")
         user1.user_permissions.add(*Permission.objects.filter(codename="view_document"))