Make new attachment fields available in private crud endpoints (Amste…

…rdam#1349)

* Add fields to serializer and make sure the read only fields are handled correctly by drf spectacular

* Expose PUT and PATCH methods and only have multipart/form available in openapi spec for creates

* Allow updating public and caption fields of attachments through the private endpoint

* Add test cases

app/signals/apps/api/serializers/attachment.py

@@ -44,7 +44,7 @@ def create(self, validated_data):
 class PublicSignalAttachmentSerializer(SignalAttachmentSerializerMixin, HALSerializer):
  serializer_url_field = PublicSignalAttachmentLinksField
  _display = DisplayField()
- location = serializers.FileField(source='file', required=False)
+ location = serializers.FileField(source='file', required=False, read_only=True)
 
  class Meta:
  model = Attachment
@@ -57,7 +57,7 @@ class Meta:
  'file',
  )
 
- read_only = (
+ read_only_fields = (
  '_display',
  '_links',
  'location',
@@ -94,7 +94,9 @@ class PrivateSignalAttachmentSerializer(SignalAttachmentSerializerMixin, HALSeri
  serializer_url_field = PrivateSignalAttachmentLinksField
 
  _display = DisplayField()
- location = serializers.FileField(source='file', required=False)
+ location = serializers.FileField(source='file', required=False, read_only=True)
+ public = serializers.BooleanField(required=False)
+ caption = serializers.CharField(required=False)
 
  class Meta:
  model = Attachment
@@ -106,9 +108,11 @@ class Meta:
  'created_at',
  'file',
  'created_by',
+ 'public',
+ 'caption',
  )
 
- read_only = (
+ read_only_fields = (
  '_display',
  '_links',
  'location',
@@ -118,3 +122,9 @@ class Meta:
  )
 
  extra_kwargs = {'file': {'write_only': True}}
+
+
+class PrivateSignalAttachmentUpdateSerializer(serializers.ModelSerializer):
+ class Meta:
+ model = Attachment
+ fields = ('public', 'caption')

app/signals/apps/api/tests/test_attachment_validation.py

@@ -10,7 +10,7 @@
 
 from django.core.files.uploadedfile import SimpleUploadedFile
 
-from signals.apps.signals.factories import SignalFactory
+from signals.apps.signals.factories import AttachmentFactory, SignalFactory
 from signals.test.utils import SignalsBaseApiTestCase
 
 THIS_DIR = os.path.dirname(__file__)
@@ -61,6 +61,90 @@ def test_upload_allowed_private(self):
  response = self.client.post(self.private_upload_url, data={'file': allowed})
  self.assertEqual(response.status_code, 201)
 
+ def test_upload_allowed_private_with_public_field(self):
+ self.client.force_authenticate(user=self.superuser)
+ for filename, content_type in ALLOWED:
+ with open(filename, 'rb') as allowed_file:
+ allowed = SimpleUploadedFile(filename, allowed_file.read(), content_type=content_type)
+ response = self.client.post(self.private_upload_url, data={'file': allowed, 'public': True})
+ self.assertEqual(response.status_code, 201)
+
+ def test_upload_allowed_private_with_caption_field(self):
+ self.client.force_authenticate(user=self.superuser)
+ for filename, content_type in ALLOWED:
+ with open(filename, 'rb') as allowed_file:
+ allowed = SimpleUploadedFile(filename, allowed_file.read(), content_type=content_type)
+ response = self.client.post(self.private_upload_url, data={'file': allowed, 'caption': 'Allowed'})
+ self.assertEqual(response.status_code, 201)
+
+ def test_upload_allowed_private_with_public_and_caption_fields(self):
+ self.client.force_authenticate(user=self.superuser)
+ for filename, content_type in ALLOWED:
+ with open(filename, 'rb') as allowed_file:
+ allowed = SimpleUploadedFile(filename, allowed_file.read(), content_type=content_type)
+ response = self.client.post(
+ self.private_upload_url,
+ data={'file': allowed, 'public': True, 'caption': 'Allowed'}
+ )
+ self.assertEqual(response.status_code, 201)
+
+ def test_put_attachment_public_and_caption_fields(self):
+ caption = 'Allowed'
+ attachment = AttachmentFactory.create(_signal=self.signal, public=False, caption=None)
+ self.client.force_authenticate(user=self.superuser)
+ response = self.client.put(
+ self.private_upload_url + f'{attachment.pk}',
+ data={'public': True, 'caption': caption}
+ )
+
+ self.assertEqual(response.status_code, 200)
+
+ body = response.json()
+ self.assertTrue(body.get('public'))
+ self.assertEqual(body.get('caption'), caption)
+
+ def test_patch_attachment_public_field(self):
+ attachment = AttachmentFactory.create(_signal=self.signal, public=False, caption=None)
+ self.client.force_authenticate(user=self.superuser)
+ response = self.client.patch(
+ self.private_upload_url + f'{attachment.pk}',
+ data={'public': True}
+ )
+
+ self.assertEqual(response.status_code, 200)
+
+ body = response.json()
+ self.assertTrue(body.get('public'))
+
+ def test_patch_attachment_caption_field(self):
+ caption = 'Allowed'
+ attachment = AttachmentFactory.create(_signal=self.signal, public=False, caption=None)
+ self.client.force_authenticate(user=self.superuser)
+ response = self.client.patch(
+ self.private_upload_url + f'{attachment.pk}',
+ data={'caption': caption}
+ )
+
+ self.assertEqual(response.status_code, 200)
+
+ body = response.json()
+ self.assertEqual(body.get('caption'), caption)
+
+ def test_patch_attachment_public_and_caption_field(self):
+ caption = 'Allowed'
+ attachment = AttachmentFactory.create(_signal=self.signal, public=False, caption=None)
+ self.client.force_authenticate(user=self.superuser)
+ response = self.client.patch(
+ self.private_upload_url + f'{attachment.pk}',
+ data={'caption': caption, 'public': True}
+ )
+
+ self.assertEqual(response.status_code, 200)
+
+ body = response.json()
+ self.assertEqual(body.get('caption'), caption)
+ self.assertTrue(body.get('public'))
+
  def test_upload_disallowed_public(self):
  # Test uploads of disfiles with allowed filetypes with correct content
  # and filename extensions - test public endpoint.

app/signals/apps/api/views/attachment.py

@@ -6,18 +6,20 @@
 import os
 
 from drf_spectacular.utils import extend_schema, extend_schema_view
+from rest_framework import serializers
 from rest_framework.exceptions import PermissionDenied
 from rest_framework.generics import get_object_or_404
-from rest_framework.mixins import CreateModelMixin, DestroyModelMixin
+from rest_framework.mixins import CreateModelMixin
 from rest_framework.response import Response
 from rest_framework.status import HTTP_204_NO_CONTENT
-from rest_framework.viewsets import GenericViewSet, ReadOnlyModelViewSet
-from rest_framework_extensions.mixins import DetailSerializerMixin, NestedViewSetMixin
+from rest_framework.viewsets import GenericViewSet, ModelViewSet
+from rest_framework_extensions.mixins import NestedViewSetMixin
 
 from signals.apps.api.serializers import (
  PrivateSignalAttachmentSerializer,
  PublicSignalAttachmentSerializer
 )
+from signals.apps.api.serializers.attachment import PrivateSignalAttachmentUpdateSerializer
 from signals.apps.services.domain.permissions.signal import SignalPermissionService
 from signals.apps.signals.models import Attachment, Signal
 from signals.auth.backend import JWTAuthBackend
@@ -48,13 +50,17 @@ def get_signal(self):
  return self.get_object()
 
 
-class PrivateSignalAttachmentsViewSet(NestedViewSetMixin, DetailSerializerMixin, CreateModelMixin, DestroyModelMixin,
- ReadOnlyModelViewSet):
+@extend_schema_view(
+ create=extend_schema(
+ request={
+ 'multipart/form-data': PrivateSignalAttachmentSerializer
+ }
+ ),
+ update=extend_schema(request=PrivateSignalAttachmentUpdateSerializer)
+)
+class PrivateSignalAttachmentsViewSet(NestedViewSetMixin, ModelViewSet):
  queryset = Attachment.objects.all()
-
  serializer_class = PrivateSignalAttachmentSerializer
- serializer_detail_class = PrivateSignalAttachmentSerializer
-
  authentication_classes = (JWTAuthBackend,)
 
  def get_queryset(self, *args, **kwargs):
@@ -72,6 +78,12 @@ def get_signal(self):
  signal = get_object_or_404(Signal.objects.filter_for_user(self.request.user), pk=pk)
  return signal
 
+ def get_serializer(self, *args, **kwargs) -> serializers.BaseSerializer:
+ if self.request.method in ['PUT', 'PATCH']:
+ return PrivateSignalAttachmentUpdateSerializer(*args, **kwargs)
+
+ return super().get_serializer(*args, **kwargs)
+
  def destroy(self, *args, **kwargs):
  user = self.request.user
  signal = self.get_signal()