Merge pull request #1698 from onaio/attachment_endpoint_fixes

Add deleted_at to attachments

onadata/apps/api/viewsets/attachment_viewset.py

@@ -47,7 +47,8 @@ class AttachmentViewSet(AuthenticateHeaderMixin, CacheControlMixin, ETagsMixin,
     content_negotiation_class = MediaFileContentNegotiation
     filter_backends = (filters.AttachmentFilter, filters.AttachmentTypeFilter)
     lookup_field = 'pk'
-    queryset = Attachment.objects.filter(instance__deleted_at__isnull=True)
+    queryset = Attachment.objects.filter(
+        instance__deleted_at__isnull=True, deleted_at__isnull=True)
     permission_classes = (AttachmentObjectPermissions,)
     serializer_class = AttachmentSerializer
     pagination_class = StandardPageNumberPagination

onadata/apps/logger/migrations/0059_attachment_deleted_by.py

@@ -0,0 +1,21 @@
+# Generated by Django 2.1.7 on 2020-02-12 08:35
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('logger', '0058_auto_20191211_0900'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='attachment',
+            name='deleted_by',
+            field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='deleted_attachments', to=settings.AUTH_USER_MODEL),
+        ),
+    ]

onadata/apps/logger/models/attachment.py

@@ -3,6 +3,7 @@
 from hashlib import md5
 
 from django.db import models
+from django.contrib.auth.models import User
 
 
 def get_original_filename(filename):
@@ -52,6 +53,8 @@ class Attachment(models.Model):
     deleted_at = models.DateTimeField(null=True, default=None)
     file_size = models.PositiveIntegerField(default=0)
     name = models.CharField(max_length=100, null=True, blank=True)
+    deleted_by = models.ForeignKey(User, related_name='deleted_attachments',
+                                   null=True, on_delete=models.SET_NULL)
 
     class Meta:
         app_label = 'logger'

onadata/apps/logger/models/instance.py

@@ -4,6 +4,7 @@
 """
 import math
 from datetime import datetime
+from django.db.models import Q
 
 from future.utils import python_2_unicode_compatible
 
@@ -66,7 +67,7 @@ def get_attachment_url(attachment, suffix=None):
 
 def _get_attachments_from_instance(instance):
     attachments = []
-    for a in instance.attachments.all():
+    for a in instance.attachments.filter(deleted_at__isnull=True):
         attachment = dict()
         attachment['download_url'] = get_attachment_url(a)
         attachment['small_download_url'] = get_attachment_url(a, 'small')
@@ -583,6 +584,17 @@ def set_deleted(self, deleted_at=timezone.now(), user=None):
         self.xform.submission_count(force_update=True)
         self.parsed_instance.save()
 
+    def soft_delete_attachments(self, user=None):
+        """
+        Soft deletes an attachment by adding a deleted_at timestamp.
+        """
+        queryset = self.attachments.filter(
+            ~Q(name__in=self.get_expected_media()))
+        kwargs = {'deleted_at': timezone.now()}
+        if user:
+            kwargs.update({'deleted_by': user})
+        queryset.update(**kwargs)
+
 
 def post_save_submission(sender, instance=None, created=False, **kwargs):
     if ASYNC_POST_SUBMISSION_PROCESSING_ENABLED:

onadata/libs/tests/utils/test_logger_tools.py

@@ -2,6 +2,7 @@
 from io import BytesIO
 
 from django.conf import settings
+from mock import patch
 
 from pyxform.tests_v1.pyxform_test_case import PyxformTestCase
 
@@ -236,6 +237,83 @@ def test_attachment_tracking_for_nested_repeats(self):
         self.assertEquals(instance2.json[MEDIA_ALL_RECEIVED],
                           instance2.media_all_received)
 
+    def test_replaced_attachments_not_tracked(self):
+        """
+        Test that when a submission with an attachments is made,
+        and later edited, whereby the attachment is replaced,
+        the replaced attachment is no longer tracked for that
+        submission
+        """
+        md = """
+        | survey |       |        |       |
+        |        | type  | name   | label |
+        |        | image | image1 | Photo |
+        """
+        self._create_user_and_login()
+        self.xform = self._publish_markdown(md, self.user)
+
+        xml_string = """
+        <data id="{}">
+            <meta>
+                <instanceID>uuid:UJ5jz4EszdgH8uhy8nss1AsKaqBPO5VN7</instanceID>
+            </meta>
+            <image1>1300221157303.jpg</image1>
+        </data>
+        """.format(self.xform.id_string)
+        file_path = "{}/apps/logger/tests/Health_2011_03_13."\
+                    "xml_2011-03-15_20-30-28/1300221157303"\
+                    ".jpg".format(settings.PROJECT_ROOT)
+        media_file = django_file(
+            path=file_path, field_name="image1", content_type="image/jpeg")
+        instance = create_instance(
+            self.user.username,
+            BytesIO(xml_string.strip().encode('utf-8')),
+            media_files=[media_file])
+        self.assertTrue(instance.json[MEDIA_ALL_RECEIVED])
+        self.assertEquals(instance.json[TOTAL_MEDIA], 1)
+        self.assertEquals(instance.json[MEDIA_COUNT], 1)
+        self.assertEquals(instance.json[TOTAL_MEDIA], instance.total_media)
+        self.assertEquals(instance.json[MEDIA_COUNT], instance.media_count)
+        self.assertEquals(instance.json[MEDIA_ALL_RECEIVED],
+                          instance.media_all_received)
+        patch_value = 'onadata.apps.logger.models.Instance.get_expected_media'
+        with patch(patch_value) as get_expected_media:
+            get_expected_media.return_value = ['1300375832136.jpg']
+            updated_xml_string = """
+            <data id="{}">
+                <meta>
+                    <instanceID>uuid:UJ5jz4EszdgH8uhy8nss1AsKaqBPO5VN7</instanceID>
+                </meta>
+                <images>
+                    <image1>1300375832136.jpg</image1>
+                </images>
+            </data>
+            """.format(self.xform.id_string)
+            file2_path = "{}/apps/logger/tests/Water_2011_03_17_2011"\
+                "-03-17_16-29-59/1300375832136.jpg".format(
+                    settings.PROJECT_ROOT)
+            media2_file = django_file(
+                path=file2_path,
+                field_name="image1",
+                content_type="image/jpeg")
+            create_instance(
+                self.user.username,
+                BytesIO(updated_xml_string.strip().encode('utf-8')),
+                media_files=[media2_file])
+
+            instance2 = Instance.objects.get(pk=instance.pk)
+            self.assertTrue(instance2.json[MEDIA_ALL_RECEIVED])
+            # Test that only one attachment is recognised for this submission
+            self.assertEquals(instance2.json[TOTAL_MEDIA], 1)
+            self.assertEquals(instance2.json[MEDIA_COUNT], 1)
+            self.assertEquals(
+                instance2.json[TOTAL_MEDIA], instance2.total_media)
+            self.assertEquals(
+                instance2.json[MEDIA_COUNT], instance2.media_count)
+            self.assertEquals(
+                instance2.json[MEDIA_ALL_RECEIVED],
+                instance2.media_all_received)
+
     def test_attachment_tracking_duplicate(self):
         """
         Test that duplicate attachments does not affect if all attachments were

onadata/libs/utils/logger_tools.py

@@ -214,7 +214,7 @@ def update_attachment_tracking(instance):
                                  'media_all_received', 'json'])
 
 
-def save_attachments(xform, instance, media_files):
+def save_attachments(xform, instance, media_files, remove_deleted_media=False):
     """
     Saves attachments for the given instance/submission.
     """
@@ -241,6 +241,9 @@ def save_attachments(xform, instance, media_files):
                 mimetype=content_type,
                 name=filename,
                 extension=extension)
+    if remove_deleted_media:
+        instance.soft_delete_attachments()
+
     update_attachment_tracking(instance)
 
 
@@ -251,7 +254,11 @@ def save_submission(xform, xml, media_files, new_uuid, submitted_by, status,
 
     instance = _get_instance(xml, new_uuid, submitted_by, status, xform,
                              checksum)
-    save_attachments(xform, instance, media_files)
+    save_attachments(
+        xform,
+        instance,
+        media_files,
+        remove_deleted_media=True)
 
     # override date created if required
     if date_created_override:
@@ -314,7 +321,11 @@ def create_instance(username,
             (new_uuid or existing_instance.xform.has_start_time):
         # ensure we have saved the extra attachments
         with transaction.atomic():
-            save_attachments(xform, existing_instance, media_files)
+            save_attachments(
+                xform,
+                existing_instance,
+                media_files,
+                remove_deleted_media=True)
             existing_instance.save(update_fields=['json', 'date_modified'])
 
         # Ignore submission as a duplicate IFF
@@ -333,7 +344,11 @@ def create_instance(username,
         duplicate_instance = history.xform_instance
         # ensure we have saved the extra attachments
         with transaction.atomic():
-            save_attachments(xform, duplicate_instance, media_files)
+            save_attachments(
+                xform,
+                duplicate_instance,
+                media_files,
+                remove_deleted_media=True)
             duplicate_instance.save()
 
         return DuplicateInstance()