Fix owner removed when set_permissions passed

diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py
index 6c28f720b2221c46f5645dfdb0a06901eda7a552..a5aec39c453442dc4a64452c56e485d6c0606a4a 100644 (file)
--- a/src/documents/serialisers.py
+++ b/src/documents/serialisers.py
@@ -213,15 +213,12 @@ class OwnedObjectSerializer(serializers.ModelSerializer, SetPermissionsMixin):
     # other methods in mixin
 
     def create(self, validated_data):
-        if self.user and (
-            "owner" not in validated_data or validated_data["owner"] is None
-        ):
+        # default to current user if not set
+        if "owner" not in validated_data and self.user:
             validated_data["owner"] = self.user
         permissions = None
         if "set_permissions" in validated_data:
             permissions = validated_data.pop("set_permissions")
-            if "user" not in permissions or permissions["user"] is None:
-                validated_data["owner"] = None
         instance = super().create(validated_data)
         if permissions is not None:
             self._set_permissions(permissions, instance)

diff --git a/src/documents/tests/test_api.py b/src/documents/tests/test_api.py
index 90684b338f74b07219306d55e9d6f02cb9f3729e..20b9ba7eb8399e2ae85a713532ac386f1d5345e9 100644 (file)
--- a/src/documents/tests/test_api.py
+++ b/src/documents/tests/test_api.py
@@ -28,6 +28,7 @@ from django.contrib.auth.models import User
 from django.test import override_settings
 from django.utils import timezone
 from guardian.shortcuts import assign_perm
+from guardian.shortcuts import get_perms
 from rest_framework import status
 from rest_framework.test import APITestCase
 from whoosh.writing import AsyncWriter
@@ -3855,7 +3856,7 @@ class TestApiAuth(DirectoriesMixin, APITestCase):
             status.HTTP_200_OK,
         )
 
-    def test_object_permissions(self):
+    def test_api_get_object_permissions(self):
         user1 = User.objects.create_user(username="test1")
         user2 = User.objects.create_user(username="test2")
         user1.user_permissions.add(*Permission.objects.filter(codename="view_document"))
@@ -3886,18 +3887,16 @@ class TestApiAuth(DirectoriesMixin, APITestCase):
             status.HTTP_404_NOT_FOUND,
         )
 
-    def test_api_set_permissions(self):
+    def test_api_default_owner(self):
         """
         GIVEN:
-            - API request to create an object (Tag) that supplies set_permissions object
+            - API request to create an object (Tag)
         WHEN:
-            - owner is passed as null or as a user id
-            - view > users is set
+            - owner is not set at all
         THEN:
-            - Object permissions are set appropriately
+            - Object created with current user as owner
         """
         user1 = User.objects.create_superuser(username="user1")
-        user2 = User.objects.create(username="user2")
 
         self.client.force_authenticate(user1)
 
@@ -3907,17 +3906,36 @@ class TestApiAuth(DirectoriesMixin, APITestCase):
                 {
                     "name": "test1",
                     "matching_algorithm": MatchingModel.MATCH_AUTO,
-                    "set_permissions": {
-                        "owner": None,
-                        "view": {
-                            "users": None,
-                            "groups": None,
-                        },
-                        "change": {
-                            "users": None,
-                            "groups": None,
-                        },
-                    },
+                },
+            ),
+            content_type="application/json",
+        )
+
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+
+        tag1 = Tag.objects.filter(name="test1").first()
+        self.assertEqual(tag1.owner, user1)
+
+    def test_api_set_no_owner(self):
+        """
+        GIVEN:
+            - API request to create an object (Tag)
+        WHEN:
+            - owner is passed as None
+        THEN:
+            - Object created with no owner
+        """
+        user1 = User.objects.create_superuser(username="user1")
+
+        self.client.force_authenticate(user1)
+
+        response = self.client.post(
+            "/api/tags/",
+            json.dumps(
+                {
+                    "name": "test1",
+                    "matching_algorithm": MatchingModel.MATCH_AUTO,
+                    "owner": None,
                 },
             ),
             content_type="application/json",
@@ -3928,17 +3946,33 @@ class TestApiAuth(DirectoriesMixin, APITestCase):
         tag1 = Tag.objects.filter(name="test1").first()
         self.assertEqual(tag1.owner, None)
 
+    def test_api_set_owner_w_permissions(self):
+        """
+        GIVEN:
+            - API request to create an object (Tag) that supplies set_permissions object
+        WHEN:
+            - owner is passed as user id
+            - view > users is set & view > groups is set
+        THEN:
+            - Object permissions are set appropriately
+        """
+        user1 = User.objects.create_superuser(username="user1")
+        user2 = User.objects.create(username="user2")
+        group1 = Group.objects.create(name="group1")
+
+        self.client.force_authenticate(user1)
+
         response = self.client.post(
             "/api/tags/",
             json.dumps(
                 {
-                    "name": "test2",
+                    "name": "test1",
                     "matching_algorithm": MatchingModel.MATCH_AUTO,
+                    "owner": user1.id,
                     "set_permissions": {
-                        "owner": user1.id,
                         "view": {
                             "users": [user2.id],
-                            "groups": None,
+                            "groups": [group1.id],
                         },
                         "change": {
                             "users": None,
@@ -3950,12 +3984,15 @@ class TestApiAuth(DirectoriesMixin, APITestCase):
             content_type="application/json",
         )
 
-        tag2 = Tag.objects.filter(name="test2").first()
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+
+        tag1 = Tag.objects.filter(name="test1").first()
 
         from guardian.core import ObjectPermissionChecker
 
         checker = ObjectPermissionChecker(user2)
-        self.assertEqual(checker.has_perm("view_tag", tag2), True)
+        self.assertEqual(checker.has_perm("view_tag", tag1), True)
+        self.assertIn("view_tag", get_perms(group1, tag1))
 
     def test_dynamic_permissions_fields(self):
         user1 = User.objects.create_user(username="user1")