Merge branch 'dev' into l10n_dev

src/documents/permissions.py

@@ -70,6 +70,7 @@ 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_to_add = User.objects.filter(id__in=permissions[action]["users"])
             users_to_remove = (
@@ -96,6 +97,7 @@ def set_permissions_for_object(permissions: list[str], object, *, merge: bool =
                             user,
                             object,
                         )
+        if "groups" in permissions[action]:
             # groups
             groups_to_add = Group.objects.filter(id__in=permissions[action]["groups"])
             groups_to_remove = (

src/documents/serialisers.py

@@ -160,24 +160,24 @@ class SetPermissionsMixin:
 
     def validate_set_permissions(self, set_permissions=None):
         permissions_dict = {
-            "view": {
+            "view": {},
-                "users": User.objects.none(),
+            "change": {},
-                "groups": Group.objects.none(),
-            },
-            "change": {
-                "users": User.objects.none(),
-                "groups": Group.objects.none(),
-            },
         }
         if set_permissions is not None:
-            for action, _ in permissions_dict.items():
+            for action in ["view", "change"]:
                 if action in set_permissions:
+                    if "users" in set_permissions[action]:
                         users = set_permissions[action]["users"]
-                    permissions_dict[action]["users"] = self._validate_user_ids(users)
+                        permissions_dict[action]["users"] = self._validate_user_ids(
+                            users,
+                        )
+                    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": (
-                            document.original_filename,
+                            filename,
                             f.read(),
                             document.mime_type,
                         ),

src/documents/tests/test_api_permissions.py

@@ -395,6 +395,52 @@ 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"))