diff --git a/onadata/apps/api/viewsets/attachment_viewset.py b/onadata/apps/api/viewsets/attachment_viewset.py
index 0d63153ca6..af38671c40 100644
--- a/onadata/apps/api/viewsets/attachment_viewset.py
+++ b/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
diff --git a/onadata/apps/logger/migrations/0059_attachment_deleted_by.py b/onadata/apps/logger/migrations/0059_attachment_deleted_by.py
new file mode 100644
index 0000000000..4a0cd2ac86
--- /dev/null
+++ b/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),
+ ),
+ ]
diff --git a/onadata/apps/logger/models/attachment.py b/onadata/apps/logger/models/attachment.py
index 72f6399305..9ea5520bfb 100644
--- a/onadata/apps/logger/models/attachment.py
+++ b/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'
diff --git a/onadata/apps/logger/models/instance.py b/onadata/apps/logger/models/instance.py
index eba834e846..0618e61f20 100644
--- a/onadata/apps/logger/models/instance.py
+++ b/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:
diff --git a/onadata/libs/tests/utils/test_logger_tools.py b/onadata/libs/tests/utils/test_logger_tools.py
index 2418db2024..ad0f8be87b 100644
--- a/onadata/libs/tests/utils/test_logger_tools.py
+++ b/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 = """
+
+
+ uuid:UJ5jz4EszdgH8uhy8nss1AsKaqBPO5VN7
+
+ 1300221157303.jpg
+
+ """.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 = """
+
+
+ uuid:UJ5jz4EszdgH8uhy8nss1AsKaqBPO5VN7
+
+
+ 1300375832136.jpg
+
+
+ """.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
diff --git a/onadata/libs/utils/logger_tools.py b/onadata/libs/utils/logger_tools.py
index 2c3b1dcc6d..aa17ca0ff9 100644
--- a/onadata/libs/utils/logger_tools.py
+++ b/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()