From 4d6c95c35dcb0cc446bc9b03cb9c7f3325f7f55f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gregor=20Jer=C5=A1e?= Date: Fri, 20 Oct 2023 14:09:24 +0200 Subject: [PATCH] Allow modifying annotation values via api --- docs/CHANGELOG.rst | 1 + resolwe/flow/filters.py | 5 +- resolwe/flow/models/annotations.py | 10 +- resolwe/flow/serializers/annotations.py | 14 ++- resolwe/flow/tests/test_annotations.py | 126 +++++++++++++++++++++++- resolwe/flow/views/annotations.py | 31 +++--- 6 files changed, 157 insertions(+), 30 deletions(-) diff --git a/docs/CHANGELOG.rst b/docs/CHANGELOG.rst index 96b061c0f..0321652ea 100644 --- a/docs/CHANGELOG.rst +++ b/docs/CHANGELOG.rst @@ -25,6 +25,7 @@ Changed - Allow filtering ``AnnotationField`` by ids - Allow filtering ``AnnotationField`` by entity - Allow filtering ``AnnotationValue`` by group name +- Allow create/update/delete REST API calls for ``AnnotationValue`` objects =================== diff --git a/resolwe/flow/filters.py b/resolwe/flow/filters.py index 2eb2d6717..37bb5b30b 100644 --- a/resolwe/flow/filters.py +++ b/resolwe/flow/filters.py @@ -683,7 +683,10 @@ def clean(self, original_clean): raise ValidationError("At least one of the entity filters must be set.") form = super().get_form_class() - form.clean = partialmethod(clean, original_clean=form.clean) + # Allow patch/delete without the entity filter. + if self.request.method not in ["PATCH", "DELETE"]: + form.clean = partialmethod(clean, original_clean=form.clean) + return form def filter_by_label(self, queryset: QuerySet, name: str, value: str): diff --git a/resolwe/flow/models/annotations.py b/resolwe/flow/models/annotations.py index 2c18b121c..a9d08f91e 100644 --- a/resolwe/flow/models/annotations.py +++ b/resolwe/flow/models/annotations.py @@ -399,16 +399,10 @@ def __init__(self, *args, **kwargs): intercept the value when given as kwarg, since args are used by Django when constructing class from database value. """ - recompute_label = False if "value" in kwargs: - value = kwargs.pop("value") - kwargs["_value"] = { - "value": value, - } - recompute_label = True - + kwargs["_value"] = {"value": kwargs.pop("value")} super().__init__(*args, **kwargs) - if recompute_label: + if "label" not in self._value: self.recompute_label() @property diff --git a/resolwe/flow/serializers/annotations.py b/resolwe/flow/serializers/annotations.py index 1cdc8b1a6..4d198a514 100644 --- a/resolwe/flow/serializers/annotations.py +++ b/resolwe/flow/serializers/annotations.py @@ -2,6 +2,7 @@ from django.db import models from rest_framework import serializers +from rest_framework.fields import empty from resolwe.flow.models.annotations import ( AnnotationField, @@ -94,10 +95,17 @@ class Meta: class AnnotationValueSerializer(ResolweBaseSerializer): """Serializer for AnnotationValue objects.""" + def __init__(self, instance=None, data=empty, **kwargs): + """Rewrite value -> _value.""" + if data is not empty and "value" in data: + data["_value"] = {"value": data.pop("value", None)} + super().__init__(instance, data, **kwargs) + class Meta: """AnnotationValueSerializer Meta options.""" model = AnnotationValue - read_only_fields = ("id", "field", "label") - update_protected_fields = ("entity", "field") - fields = read_only_fields + update_protected_fields + ("value",) + read_only_fields = ("label",) + update_protected_fields = ("id", "entity", "field") + fields = read_only_fields + update_protected_fields + ("value", "_value") + extra_kwargs = {"_value": {"write_only": True}} diff --git a/resolwe/flow/tests/test_annotations.py b/resolwe/flow/tests/test_annotations.py index 5a8b75d5c..ed520f0d3 100644 --- a/resolwe/flow/tests/test_annotations.py +++ b/resolwe/flow/tests/test_annotations.py @@ -3,10 +3,11 @@ from django.core.exceptions import ValidationError from django.http import HttpResponse +from django.urls import reverse from rest_framework import status from rest_framework.response import Response -from rest_framework.test import APIRequestFactory, force_authenticate +from rest_framework.test import APIClient, APIRequestFactory, force_authenticate from resolwe.flow.models import AnnotationField, Collection, Entity from resolwe.flow.models.annotations import ( @@ -321,7 +322,7 @@ def setUp(self): sort_order=2, group=self.annotation_group1, type="STRING", - vocabulary={"string": "label string"}, + vocabulary={"string": "label string", "another": "Another one"}, ) self.annotation_field2: AnnotationField = AnnotationField.objects.create( name="field2", @@ -349,7 +350,12 @@ def setUp(self): ) self.annotationvalue_viewset = AnnotationValueViewSet.as_view( - actions={"get": "list", "post": "create", "patch": "partial_update"} + actions={ + "get": "list", + "post": "create", + "patch": "partial_update", + "delete": "destroy", + } ) self.annotation_value1: AnnotationValue = AnnotationValue.objects.create( entity=self.entity1, field=self.annotation_field1, value="string" @@ -358,8 +364,120 @@ def setUp(self): entity=self.entity2, field=self.annotation_field2, value=2 ) - def test_create(self): + def test_create_annotation_value(self): """Test creating new annotation value objects.""" + field = AnnotationField.objects.create( + name="field3", + label="Annotation field 3", + sort_order=1, + group=self.annotation_group1, + type=AnnotationType.INTEGER.value, + ) + + self.client = APIClient() + path = reverse("resolwe-api:annotationvalue-list") + values = {"entity": self.entity1.pk, "field": field.pk, "value": -1} + values_count = AnnotationValue.objects.count() + + # Unauthenticated request. + response = self.client.post(path, values, format="json") + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.data, {"detail": "Not found."}) + self.assertAlmostEqual(values_count, AnnotationValue.objects.count()) + + # Authenticated request. + self.client.force_authenticate(self.contributor) + response = self.client.post(path, values, format="json") + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + created_value = AnnotationValue.objects.get(pk=response.data["id"]) + self.assertEqual(created_value.entity, self.entity1) + self.assertEqual(created_value.field, field) + self.assertEqual(created_value.value, -1) + self.assertAlmostEqual(values_count + 1, AnnotationValue.objects.count()) + + # Authenticated request, no permission. + self.entity1.collection.set_permission(Permission.NONE, self.contributor) + self.client.force_authenticate(self.contributor) + response = self.client.post(path, values, format="json") + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.data, {"detail": "Not found."}) + self.assertAlmostEqual(values_count + 1, AnnotationValue.objects.count()) + + def test_update_annotation_value(self): + """Test updating new annotation value objects.""" + client = APIClient() + path = reverse( + "resolwe-api:annotationvalue-detail", args=[self.annotation_value1.pk] + ) + values = {"id": self.annotation_value1.pk, "value": "another"} + + # Unauthenticated request. + response = client.patch(path, values, format="json") + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.data, {"detail": "Not found."}) + + # Authenticated request. + client.force_authenticate(self.contributor) + response = client.patch(path, values, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["id"], self.annotation_value1.pk) + self.annotation_value1.refresh_from_db() + self.assertEqual(self.annotation_value1.value, "another") + self.assertEqual(self.annotation_value1.label, "Another one") + self.assertEqual(self.annotation_value1.entity, self.entity1) + + # Authenticated request, entity should not be changed + values = { + "id": self.annotation_value1.pk, + "value": "string", + "entity": self.entity2.pk, + } + response = client.patch(path, values, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.annotation_value1.refresh_from_db() + self.assertEqual(self.annotation_value1.value, "string") + self.assertEqual(self.annotation_value1.entity, self.entity1) + + # Authenticated request, no permission. + self.entity1.collection.set_permission(Permission.NONE, self.contributor) + response = client.patch(path, values, format="json") + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.data, {"detail": "Not found."}) + + def test_delete_annotation_value(self): + """Test deleting annotation value objects.""" + client = APIClient() + path = reverse( + "resolwe-api:annotationvalue-detail", args=[self.annotation_value1.pk] + ) + # Unauthenticated request. + response = client.delete(path, format="json") + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual( + response.data, + {"detail": "You do not have permission to perform this action."}, + ) + + # Authenticated request. + client.force_authenticate(self.contributor) + response = client.delete(path, format="json") + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + with self.assertRaises(AnnotationValue.DoesNotExist): + self.annotation_value1.refresh_from_db() + + # Authenticated request, no permission. + path = reverse( + "resolwe-api:annotationvalue-detail", args=[self.annotation_value2.pk] + ) + self.annotation_value2.entity.collection.set_permission( + Permission.NONE, self.contributor + ) + response = client.delete(path, format="json") + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual( + response.data, + {"detail": "You do not have permission to perform this action."}, + ) def test_annotate_path(self): """Test annotate entity queryset.""" diff --git a/resolwe/flow/views/annotations.py b/resolwe/flow/views/annotations.py index 3f0fe5474..1a27a36c3 100644 --- a/resolwe/flow/views/annotations.py +++ b/resolwe/flow/views/annotations.py @@ -65,6 +65,7 @@ class AnnotationValueViewSet( mixins.RetrieveModelMixin, ResolweUpdateModelMixin, mixins.ListModelMixin, + mixins.DestroyModelMixin, viewsets.GenericViewSet, ): """Annotation value viewset.""" @@ -84,11 +85,11 @@ def _has_permissions_on_entity(self, entity: Entity) -> bool: """Has the authenticated user EDIT permission on the associated entity.""" return ( Entity.objects.filter(pk=entity.pk) - .filter_for_user(request.user, Permission.EDIT) + .filter_for_user(self.request.user, Permission.EDIT) .exists() ) - def _get_annotation_value(self, request: request.Request) -> AnnotationValue: + def _get_entity(self, request: request.Request) -> AnnotationValue: """Get annotation value from request. :raises ValidationError: if the annotation value is not valid. @@ -97,30 +98,32 @@ def _get_annotation_value(self, request: request.Request) -> AnnotationValue: if not request.user.is_authenticated: raise exceptions.NotFound - serializer = self.get_serializer(data=request.data) + serializer = self.get_serializer(data=request.data, partial=True) serializer.is_valid(raise_exception=True) - return serializer.validated_data + return serializer.validated_data["entity"] def create(self, request, *args, **kwargs): """Create annotation value. Authenticated users with edit permissions on the entity can create annotations. """ - annotation_value = self._get_annotation_value(request) - - if self._has_permissions_on_entity(annotation_value.entity): + if self._has_permissions_on_entity(self._get_entity(request)): return super().create(request, *args, **kwargs) - - raise exceptions.PermissionDenied() + raise exceptions.NotFound() def update(self, request, *args, **kwargs): """Update annotation values. Authenticated users with edit permission on the entity can update annotations. """ - annotation_value = self._get_annotation_value(request) - - if self._has_permissions_on_entity(annotation_value.entity): - return super().create(request, *args, **kwargs) - + entity = AnnotationValue.objects.get(pk=kwargs["pk"]).entity + if self._has_permissions_on_entity(entity): + return super().update(request, *args, **kwargs) + raise exceptions.NotFound() + + def destroy(self, request, *args, **kwargs): + """Destroy the annotation value.""" + entity = AnnotationValue.objects.get(pk=kwargs["pk"]).entity + if self._has_permissions_on_entity(entity): + return super().destroy(request, *args, **kwargs) raise exceptions.PermissionDenied()