Add ability to remove attachments

src/sa_api_v2/migrations/0008_attachment_visible.py

@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('sa_api_v2', '0007_auto_20180420_2202'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='attachment',
+            name='visible',
+            field=models.BooleanField(default=True, db_index=True),
+            preserve_default=True,
+        ),
+    ]

src/sa_api_v2/models/core.py

@@ -319,6 +319,7 @@ class Attachment (CacheClearingModel, TimeStampedModel):
     file = models.FileField(upload_to=timestamp_filename, storage=AttachmentStorage())
     name = models.CharField(max_length=128, null=True, blank=True)
     thing = models.ForeignKey('SubmittedThing', related_name='attachments')
+    visible = models.BooleanField(default=True, blank=True, db_index=True)
 
     COVER = 'CO'
     RICH_TEXT = 'RT'

src/sa_api_v2/serializers.py

@@ -111,6 +111,7 @@ def api_reverse(view_name, kwargs={}, request=None, format=None):
         'dataset-detail': '/{owner_username}/datasets/{dataset_slug}',
         'user-detail': '/{owner_username}',
         'dataset-submission-list': '/{owner_username}/datasets/{dataset_slug}/{submission_set_name}',
+        'attachment-detail': '/{owner_username}/datasets/{dataset_slug}/places/{place_id}/attachments/{attachment_id}',
     }
 
     try:
@@ -202,6 +203,11 @@ class PlaceIdentityField (ShareaboutsIdentityField):
     url_arg_names = ('owner_username', 'dataset_slug', 'place_id')
 
 
+class AttachmentIdentityField (ShareaboutsIdentityField):
+    url_arg_names = ('owner_username', 'dataset_slug', 'place_id', 'attachment_id')
+    view_name = 'attachment-detail'
+
+
 class SubmissionSetIdentityField (ShareaboutsIdentityField):
     url_arg_names = ('owner_username', 'dataset_slug', 'place_id', 'submission_set_name')
     view_name = 'submission-list'
@@ -330,6 +336,33 @@ def to_native(self, obj):
         return data
 
 
+class AttachmentSerializerMixin (EmptyModelSerializer, serializers.ModelSerializer):
+    url = AttachmentIdentityField()
+
+    def to_native(self, obj):
+        obj = self.ensure_obj(obj)
+        data = {
+            'id': obj.pk,
+            'created_datetime': obj.created_datetime,
+            'updated_datetime': obj.updated_datetime,
+            'file': obj.file.storage.url(obj.file.name),
+            'name': obj.name,
+            'type': obj.type,
+            'visible': obj.visible,
+        }
+        fields = self.get_fields()
+        data['url'] = fields['url'].field_to_native(obj, 'pk')
+
+        # Construct a SortedDictWithMetaData to get the browsable API form
+        ret = self._dict_class(data)
+        ret.fields = self._dict_class()
+        for field_name, field in fields.iteritems():
+            value = data[field_name]
+            ret.fields[field_name] = self.augment_field(field, field_name, field_name, value)
+
+        return ret
+
+
 ###############################################################################
 #
 # User Data Strategies
@@ -425,34 +458,16 @@ def extract_bio(self, user_info):
 #    hyperlinked serializer. This is more useful for bulk data dumps where all
 #    of the related data is included in a package.
 #
-
-
-class AttachmentSerializer (EmptyModelSerializer, serializers.ModelSerializer):
-    file = AttachmentFileField()
-
+
+class AttachmentListSerializer (AttachmentSerializerMixin):
     class Meta:
         model = models.Attachment
-        exclude = ('id', 'thing',)
-
-    def to_native(self, obj):
-        obj = self.ensure_obj(obj)
-        data = {
-            'created_datetime': obj.created_datetime,
-            'updated_datetime': obj.updated_datetime,
-            'file': obj.file.storage.url(obj.file.name),
-            'name': obj.name,
-            'type': obj.type,
-        }
-        fields = self.get_fields()
-
-        # Construct a SortedDictWithMetaData to get the brosable API form
-        ret = self._dict_class(data)
-        ret.fields = self._dict_class()
-        for field_name, field in fields.iteritems():
-            value = data[field_name]
-            ret.fields[field_name] = self.augment_field(field, field_name, field_name, value)
-        return ret
+        exclude = ('thing', 'id')
 
+class AttachmentInstanceSerializer (AttachmentSerializerMixin):
+    class Meta:
+        model = models.Attachment
+        exclude = ('thing', 'file', 'id')
 
 class DataSetPermissionSerializer (serializers.ModelSerializer):
     class Meta:
@@ -718,7 +733,7 @@ def restore_fields(self, data, files):
 # Place serializers
 class BasePlaceSerializer (SubmittedThingSerializer, serializers.ModelSerializer):
     geometry = GeometryField(format='wkt')
-    attachments = AttachmentSerializer(read_only=True, many=True)
+    attachments = AttachmentListSerializer(read_only=True, many=True)
     submitter = SimpleUserSerializer(read_only=False)
 
     class Meta:
@@ -794,7 +809,7 @@ def get_detailed_submission_sets(self, place):
         return details
 
     def attachments_to_native(self, obj):
-        return [AttachmentSerializer(a).data for a in obj.attachments.all()]
+        return [AttachmentListSerializer(a, context=self.context).data for a in obj.attachments.filter(visible=True)]
 
     def submitter_to_native(self, obj):
         return SimpleUserSerializer(obj.submitter).data if obj.submitter else None
@@ -871,7 +886,7 @@ def submitter_to_native(self, obj):
 # Submission serializers
 class BaseSubmissionSerializer (SubmittedThingSerializer, serializers.ModelSerializer):
     id = serializers.IntegerField(read_only=True)
-    attachments = AttachmentSerializer(read_only=True, many=True)
+    attachments = AttachmentListSerializer(read_only=True, many=True)
     submitter = SimpleUserSerializer()
 
     class Meta:

src/sa_api_v2/tests/test_serializers.py

@@ -7,33 +7,57 @@
 from nose.tools import istest
 from sa_api_v2.cache import cache_buffer
 from sa_api_v2.models import Attachment, Action, User, DataSet, Place, Submission, Group
-from sa_api_v2.serializers import AttachmentSerializer, ActionSerializer, UserSerializer, FullUserSerializer, PlaceSerializer, DataSetSerializer, SubmissionSerializer
+from sa_api_v2.serializers import AttachmentListSerializer, AttachmentInstanceSerializer, ActionSerializer, UserSerializer, FullUserSerializer, PlaceSerializer, DataSetSerializer, SubmissionSerializer
 from social.apps.django_app.default.models import UserSocialAuth
 import json
 from os import path
 from mock import patch
 
 
-class TestAttachmentSerializer (TestCase):
+class TestAttachmentListSerializer (TestCase):
 
     def setUp(self):
         f = ContentFile('this is a test')
         f.name = 'my_file.txt'
         self.attachment_model = Attachment(name='my_file', file=f, type='RT')
 
     def test_attributes(self):
-        serializer = AttachmentSerializer(self.attachment_model)
-        self.assertNotIn('id', serializer.data)
+        serializer = AttachmentListSerializer(self.attachment_model)
         self.assertNotIn('thing', serializer.data)
 
         self.assertIn('created_datetime', serializer.data)
         self.assertIn('updated_datetime', serializer.data)
         self.assertIn('file', serializer.data)
         self.assertIn('name', serializer.data)
         self.assertIn('type', serializer.data)
+        self.assertIn('id', serializer.data)
+
+    def test_can_serlialize_a_null_instance(self):
+        serializer = AttachmentListSerializer(None)
+        data = serializer.data
+        self.assertIsInstance(data, dict)
+
+
+class TestAttachmentInstanceSerializer (TestCase):
+
+    def setUp(self):
+        f = ContentFile('this is a test')
+        f.name = 'my_file.txt'
+        self.attachment_model = Attachment(name='my_file', file=f, type='RT')
+
+    def test_attributes(self):
+        serializer = AttachmentInstanceSerializer(self.attachment_model)
+        self.assertNotIn('thing', serializer.data)
+
+        self.assertIn('created_datetime', serializer.data)
+        self.assertIn('updated_datetime', serializer.data)
+        self.assertIn('file', serializer.data)
+        self.assertIn('name', serializer.data)
+        self.assertIn('type', serializer.data)
+        self.assertIn('id', serializer.data)
 
     def test_can_serlialize_a_null_instance(self):
-        serializer = AttachmentSerializer(None)
+        serializer = AttachmentInstanceSerializer(None)
         data = serializer.data
         self.assertIsInstance(data, dict)
 

src/sa_api_v2/tests/test_views.py

@@ -517,7 +517,9 @@ def test_GET_from_cache(self):
         # - SELECT * FROM sa_api_attachment AS a
         #    WHERE a.thing_id IN (<self.place.id>);
         #
-        with self.assertNumQueries(11):
+
+        # TODO: https://github.com/mapseed/api/issues/137
+        with self.assertNumQueries(15):
             response = self.view(request, **self.request_kwargs)
             self.assertStatusCode(response, 200)
 
@@ -577,7 +579,9 @@ def test_GET_from_cache_with_api_key(self):
         # - SELECT * FROM sa_api_attachment AS a
         #    WHERE a.thing_id IN (<self.place.id>);
         #
-        with self.assertNumQueries(11):
+
+        # TODO: https://github.com/mapseed/api/issues/137
+        with self.assertNumQueries(15):
             response = self.view(request, **self.request_kwargs)
             self.assertStatusCode(response, 200)
 
@@ -634,7 +638,9 @@ def test_GET_differently_from_cache_by_user_group(self):
         # - SELECT * FROM sa_api_attachment AS a
         #    WHERE a.thing_id IN (<self.place.id>);
         #
-        with self.assertNumQueries(16):
+
+        # TODO: https://github.com/mapseed/api/issues/137
+        with self.assertNumQueries(24):
             response = self.view(anon_request, **self.request_kwargs)
             self.assertStatusCode(response, 200)
             response = self.view(auth_request, **self.request_kwargs)
@@ -1767,7 +1773,9 @@ def test_model_update_clears_GET_cache_for_multiple_specific_objects(self):
         # SELECT * FROM att WHERE thing_id IN <place ids>
         #
         request = factory.get(path)
-        with self.assertNumQueries(12):
+
+        # TODO: https://github.com/mapseed/api/issues/137
+        with self.assertNumQueries(17):
             view(request, **request_kwargs)
 
         # Second call should hardly hit the database
@@ -1786,7 +1794,9 @@ def test_model_update_clears_GET_cache_for_multiple_specific_objects(self):
 
         # Run same queries as above (except for permissions)
         request = factory.get(path)
-        with self.assertNumQueries(6):
+
+        # TODO: https://github.com/mapseed/api/issues/137
+        with self.assertNumQueries(11):
             view(request, **request_kwargs)
 
 

src/sa_api_v2/urls.py

@@ -12,6 +12,11 @@
     url(r'^(?P<owner_username>[^/]+)/datasets/(?P<dataset_slug>[^/]+)/places/(?P<thing_id>\d+)/attachments$',
         views.AttachmentListView.as_view(),
         name='place-attachments'),
+
+    url(r'^(?P<owner_username>[^/]+)/datasets/(?P<dataset_slug>[^/]+)/places/(?P<place_id>\d+)/attachments/(?P<attachment_id>\d+)$',
+        views.AttachmentInstanceView.as_view(),
+        name='attachment-detail'),
+
     url(r'^(?P<owner_username>[^/]+)/datasets/(?P<dataset_slug>[^/]+)/places/(?P<place_id>\d+)/(?P<submission_set_name>[^/]+)/(?P<thing_id>\d+)/attachments$',
         views.AttachmentListView.as_view(),
         name='submission-attachments'),

src/sa_api_v2/views/base_views.py

@@ -1891,6 +1891,48 @@ class AdminDataSetListView (CachedResourceMixin, DataSetListMixin, generics.List
     content_negotiation_class = ShareaboutsContentNegotiation
 
 
+class AttachmentInstanceView (ProtectedOwnedResourceMixin, generics.RetrieveUpdateAPIView):
+    """
+
+    GET
+    ---
+    Get a particular attachment
+
+    **Authentication**: Basic, session, or key auth *(optional)*
+
+    PATCH
+    -----
+    Update an attachment's metadata, though not the attachment itself
+
+    **Authentication**: Basic, session, or key auth *(required)*
+
+    ------------------------------------------------------------
+    """
+
+    model = models.Attachment
+    serializer_class = serializers.AttachmentInstanceSerializer
+
+    def partial_update(self, *args, **kwargs):
+        attachment_id = self.kwargs['attachment_id']
+        attachment = self.get_object_or_404(attachment_id)
+        attachment.clear_instance_cache()
+        return super(AttachmentInstanceView, self).partial_update(*args, **kwargs) 
+
+    def get_object_or_404(self, pk):
+        try:
+            return self.model.objects\
+                .filter(pk=pk)\
+                .get()
+        except self.model.DoesNotExist:
+            raise Http404
+
+    def get_object(self, queryset=None):
+        attachment_id = self.kwargs['attachment_id']
+        obj = self.get_object_or_404(attachment_id)
+        self.verify_object(obj)
+        return obj
+
+
 class AttachmentListView (OwnedResourceMixin, FilteredResourceMixin, generics.ListCreateAPIView):
     """
 
@@ -1902,7 +1944,6 @@ class AttachmentListView (OwnedResourceMixin, FilteredResourceMixin, generics.Li
 
     POST
     ----
-
     Attach a file to a place or submission
 
     **Authentication**: Basic, session, or key auth *(required)*
@@ -1911,7 +1952,7 @@ class AttachmentListView (OwnedResourceMixin, FilteredResourceMixin, generics.Li
     """
 
     model = models.Attachment
-    serializer_class = serializers.AttachmentSerializer
+    serializer_class = serializers.AttachmentListSerializer
 
     thing_id_kwarg = 'thing_id'
     submission_set_name_kwarg = 'submission_set_name'