Fix verify data entity change

docs/CHANGELOG.rst

@@ -32,6 +32,7 @@ Added
 Fixed
 -----
 - Use ``all_objects`` manager in related lookups in viewsets
+- Veriy change when moving data to another ``entity``
 
 
 ===================

resolwe/flow/models/data.py

@@ -634,6 +634,33 @@ def move_to_entity(self, entity):
                 self.tags = entity.tags
         self.save(update_fields=["permission_group", "entity", "tags"])
 
+    @classmethod
+    def validate_change_containers(cls, data, entity, collection):
+        """Validate changing entity and collection.
+
+        Validate that:
+        - data is not removed from the container.
+        - if entity is given its collection must be the same as given collection
+        """
+        if data and data.in_container():
+            if (data.entity and not entity) or (data.collection and not collection):
+                raise ValidationError(
+                    "Data object can not be removed from the container."
+                )
+        if entity and entity.collection != collection:
+            raise ValidationError(
+                "Entity must belong to the same collection as data object."
+            )
+
+    def move_to_containers(self, entity, collection):
+        """Move the data object to the given entity and collection."""
+        Data.validate_change_containers(self, entity, collection)
+        if self.entity != entity or self.collection != collection:
+            self.collection = collection
+            self.entity = entity
+            self.tags = getattr(collection, "tags", []) or getattr(entity, "tags", [])
+            self.save(update_fields=["collection", "entity", "tags"])
+
     def validate_change_collection(self, collection):
         """Raise validation error if data object cannot change collection."""
         if self.entity and self.entity.collection != collection:
@@ -644,6 +671,17 @@ def validate_change_collection(self, collection):
         if collection is None:
             raise ValidationError("Data object can not be removed from the container.")
 
+    def validate_change_entity(self, entity):
+        """Raise validation error if data object cannot change entity."""
+        if entity is None:
+            raise ValidationError("Data object can not be removed from the container.")
+
+        if self.collection and self.collection != entity.collection:
+            raise ValidationError(
+                "If data is in collection, you can only move it to another entity "
+                "in the same collection."
+            )
+
     def _render_name(self):
         """Render data name.
 

resolwe/flow/serializers/data.py

@@ -99,18 +99,28 @@ def validate_process(self, process):
             )
         return process
 
-    def validate_collection(self, collection):
-        """Verify that changing collection is done in the right place."""
-        if self.instance and self.instance.collection != collection:
-            self.instance.validate_change_collection(collection)
-        return collection
+    def validate(self, validated_data):
+        """Validate collection change."""
+        entity = validated_data.get("entity") or getattr(self.instance, "entity", None)
+        collection = validated_data.get("collection") or getattr(
+            self.instance, "collection", None
+        )
+        Data.validate_change_containers(self.instance, entity, collection)
+        return super().validate(validated_data)
 
     @transaction.atomic
     def update(self, instance, validated_data):
-        """Update."""
-        update_collection = "collection" in validated_data
-        new_collection = validated_data.pop("collection", None)
+        """Update the data object."""
+        container_change = False
+        entity = instance.entity
+        collection = instance.collection
+        if "collection" in validated_data:
+            container_change = True
+            collection = validated_data.pop("collection", None)
+        if "entity" in validated_data:
+            container_change = True
+            entity = validated_data.pop("entity", None)
         instance = super().update(instance, validated_data)
-        if update_collection and new_collection != instance.collection:
-            instance.move_to_collection(new_collection)
+        if container_change:
+            instance.move_to_containers(entity, collection)
         return instance

resolwe/flow/tests/test_api.py

@@ -802,20 +802,22 @@ def test_collection_unassigned(self):
         self.assertEqual(data.entity.id, entity.id)
         self.assertEqual(entity.collection.id, self.collection.id)
 
-        # Assign entity to None
-        data.entity = None
-        data.save()
-        request = factory.patch("/", {"collection": {"id": None}}, format="json")
+        # Assign to new entity without collection.
+        new_entity = Entity.objects.create(
+            contributor=self.contributor, name="My entity"
+        )
+        new_entity.set_permission(Permission.EDIT, self.contributor)
+        request = factory.patch("/", {"entity": {"id": new_entity.id}}, format="json")
         force_authenticate(request, self.contributor)
         response = self.data_detail_viewset(request, pk=data.pk)
-
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-        self.assertEqual(
-            response.data["collection"][0],
-            "Data object can not be removed from the container.",
+        self.assertContains(
+            response,
+            "Entity must belong to the same collection as data object.",
+            status_code=status.HTTP_400_BAD_REQUEST,
         )
 
-    def test_change_collection(self):
+    def test_change_container(self):
         # Create data object. Note that an entity is created as well.
         data = Data.objects.create(
             name="Test data",
@@ -829,15 +831,15 @@ def test_change_collection(self):
         )
         force_authenticate(request, self.contributor)
         response = self.data_detail_viewset(request, pk=data.pk)
-        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-        self.assertEqual(
-            response.data["collection"][0],
-            "If Data is in entity, you can only move it to another collection by moving entire entity.",
+        self.assertContains(
+            response,
+            "Entity must belong to the same collection as data object.",
+            status_code=status.HTTP_400_BAD_REQUEST,
         )
 
         entity = data.entity
 
-        # Data can not be moved from collection.
+        # Data can not be removed from container.
         with self.assertRaises(ValidationError):
             data.move_to_entity(None)
 
@@ -849,6 +851,7 @@ def test_change_collection(self):
         )
         force_authenticate(request, self.contributor)
         response = self.data_detail_viewset(request, pk=data.pk)
+
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         data.refresh_from_db()
         self.assertEqual(data.collection, self.collection)
@@ -879,6 +882,89 @@ def test_change_collection(self):
 
         data.refresh_from_db()
         self.assertEqual(data.tags, collection.tags)
+        self.assertEqual(data.entity, None)
+
+        # Move it in the entity in the same collection.
+        entity = Entity.objects.create(
+            contributor=self.contributor, collection=collection
+        )
+        request = factory.patch("/", {"entity": {"id": entity.pk}}, format="json")
+        force_authenticate(request, self.contributor)
+        response = self.data_detail_viewset(request, pk=data.pk)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        data.refresh_from_db()
+        self.assertEqual(data.tags, collection.tags)
+        self.assertEqual(data.entity, entity)
+
+        # Move it in another entity in the same collection.
+        another_entity = Entity.objects.create(
+            contributor=self.contributor, collection=collection
+        )
+        request = factory.patch(
+            "/", {"entity": {"id": another_entity.pk}}, format="json"
+        )
+        force_authenticate(request, self.contributor)
+        response = self.data_detail_viewset(request, pk=data.pk)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        data.refresh_from_db()
+        self.assertEqual(data.tags, collection.tags)
+        self.assertEqual(data.entity, another_entity)
+
+        # Move it to entity in another collection.
+        another_collection = Collection.objects.create(contributor=self.contributor)
+        entity.move_to_collection(another_collection)
+        another_collection.set_permission(Permission.EDIT, self.contributor)
+        entity.save()
+        entity.collection.set_permission
+        request = factory.patch("/", {"entity": {"id": entity.pk}}, format="json")
+        force_authenticate(request, self.contributor)
+        response = self.data_detail_viewset(request, pk=data.pk)
+        self.assertContains(
+            response,
+            "Entity must belong to the same collection as data object.",
+            status_code=status.HTTP_400_BAD_REQUEST,
+        )
+
+        # Try to remove it from the entity.
+        request = factory.patch("/", {"entity": None}, format="json")
+        force_authenticate(request, self.contributor)
+        response = self.data_detail_viewset(request, pk=data.pk)
+        self.assertContains(
+            response,
+            "Data object can not be removed from the container.",
+            status_code=status.HTTP_400_BAD_REQUEST,
+        )
+
+        # Try to remove it from the collection.
+        request = factory.patch("/", {"collection": None}, format="json")
+        force_authenticate(request, self.contributor)
+        response = self.data_detail_viewset(request, pk=data.pk)
+        self.assertContains(
+            response,
+            "Data object can not be removed from the container.",
+            status_code=status.HTTP_400_BAD_REQUEST,
+        )
+
+        # Move it to another collection and entity.
+        collection = Collection.objects.create(
+            contributor=self.contributor, tags=["test"]
+        )
+        entity = Entity.objects.create(
+            contributor=self.contributor, collection=collection
+        )
+        collection.set_permission(Permission.EDIT, self.contributor)
+        request = factory.patch(
+            "/",
+            {"entity": {"id": entity.pk}, "collection": {"id": collection.pk}},
+            format="json",
+        )
+        force_authenticate(request, self.contributor)
+        response = self.data_detail_viewset(request, pk=data.pk)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        data.refresh_from_db()
+        self.assertEqual(data.tags, collection.tags)
+        self.assertEqual(data.entity, entity)
+        self.assertEqual(data.collection, collection)
 
     def test_process_is_active(self):
         # Do not allow creating data of inactive processes.