Complete removal of deleted_at. Closes #696

onadata/apps/api/viewsets/data_viewset.py

@@ -111,7 +111,6 @@ class DataViewSet(AnonymousUserPublicFormsMixin, ModelViewSet):
 >        [
 >            {
 >                "_id": 4503,
->                "_deleted_at": null,
 >                "expense_type": "service",
 >                "_xform_id_string": "exp",
 >                "_geolocation": [
@@ -159,7 +158,6 @@ class DataViewSet(AnonymousUserPublicFormsMixin, ModelViewSet):
 >
 >            {
 >                "_id": 4503,
->                "_deleted_at": null,
 >                "expense_type": "service",
 >                "_xform_id_string": "exp",
 >                "_geolocation": [
@@ -215,7 +213,6 @@ class DataViewSet(AnonymousUserPublicFormsMixin, ModelViewSet):
 >        [
 >            {
 >                "_id": 4503,
->                "_deleted_at": null,
 >                "expense_type": "service",
 >                "_xform_id_string": "exp",
 >                "_geolocation": [

onadata/apps/logger/models/instance.py

@@ -24,7 +24,6 @@
     clean_and_parse_xml, get_uuid_from_xml
 from onadata.libs.utils.common_tags import (
     ATTACHMENTS,
-    DELETEDAT,
     GEOLOCATION,
     ID,
     MONGO_STRFTIME,
@@ -91,6 +90,28 @@ def update_xform_submission_count(sender, instance, created, **kwargs):
         )
 
 
+def nullify_exports_time_of_last_submission(sender, instance, **kwargs):
+    """
+    Formerly, "deleting" a submission would set a flag on the `Instance`,
+    causing the `date_modified` attribute to be set to the current timestamp.
+    `Export.exports_outdated()` relied on this to detect when a new `Export`
+    needed to be generated due to submission deletion, but now that we always
+    delete `Instance`s outright, this trick doesn't work. This kludge simply
+    makes every `Export` for a form appear stale by nulling out its
+    `time_of_last_submission` attribute.
+    """
+    # Avoid circular import
+    try:
+        export_model = instance.xform.export_set.model
+    except XForm.DoesNotExist:
+        return
+    f = instance.xform.export_set.filter(
+        # Match the statuses considered by `Export.exports_outdated()`
+        internal_status__in=[export_model.SUCCESSFUL, export_model.PENDING],
+    )
+    f.update(time_of_last_submission=None)
+
+
 def update_xform_submission_count_delete(sender, instance, **kwargs):
     try:
         xform = XForm.objects.select_for_update().get(pk=instance.xform.pk)
@@ -133,7 +154,8 @@ class Instance(models.Model):
     # this will end up representing "date last parsed"
     date_modified = models.DateTimeField(auto_now=True)
 
-    # this will end up representing "date instance was deleted"
+    # this formerly represented "date instance was deleted".
+    # do not use it anymore.
     deleted_at = models.DateTimeField(null=True, default=None)
 
     # ODK keeps track of three statuses for an instance:
@@ -172,10 +194,6 @@ def asset(self):
         """
         return self.xform
 
-    @classmethod
-    def set_deleted_at(cls, instance_id, deleted_at=timezone.now()):
-        raise Exception('This method MUST NOT be used.')
-
     def _check_active(self, force):
         """Check that form is active and raise exception if not.
 
@@ -331,9 +349,6 @@ def get_full_dict(self):
             NOTES: self.get_notes()
         }
 
-        if isinstance(self.instance.deleted_at, datetime):
-            data[DELETEDAT] = self.deleted_at.strftime(MONGO_STRFTIME)
-
         d.update(data)
 
         return d
@@ -386,9 +401,6 @@ def save(self, *args, **kwargs):
 
         super(Instance, self).save(*args, **kwargs)
 
-    def set_deleted(self, deleted_at=timezone.now()):
-        raise Exception('This method MUST NOT be used.')
-
     def get_validation_status(self):
         """
         Returns instance validation status.
@@ -405,6 +417,9 @@ def get_validation_status(self):
 post_save.connect(update_xform_submission_count, sender=Instance,
                   dispatch_uid='update_xform_submission_count')
 
+post_delete.connect(nullify_exports_time_of_last_submission, sender=Instance,
+                  dispatch_uid='nullify_exports_time_of_last_submission')
+
 post_delete.connect(update_xform_submission_count_delete, sender=Instance,
                     dispatch_uid='update_xform_submission_count_delete')
 

onadata/apps/main/tests/test_form_api_delete.py

@@ -34,15 +34,15 @@ def _get_data(self):
 
     def test_get_request_does_not_delete(self):
         # not allowed 405
-        count = Instance.objects.filter(deleted_at=None).count()
+        count = Instance.objects.count()
         response = self.anon.get(self.delete_url)
         self.assertEqual(response.status_code, 405)
         self.assertEqual(
-            Instance.objects.filter(deleted_at=None).count(), count)
+            Instance.objects.count(), count)
 
     def test_anon_user_cant_delete(self):
         # Only authenticated user are allowed to access the url
-        count = Instance.objects.filter(deleted_at=None).count()
+        count = Instance.objects.count()
         instance = Instance.objects.filter(
             xform=self.xform).latest('date_created')
         # delete
@@ -51,63 +51,57 @@ def test_anon_user_cant_delete(self):
         self.assertEqual(response.status_code, 302)
         self.assertIn("accounts/login/?next=", response["Location"])
         self.assertEqual(
-            Instance.objects.filter(deleted_at=None).count(), count)
+            Instance.objects.count(), count)
 
     def test_delete_shared(self):
         # Test if someone can delete data from a shared form
         self.xform.shared = True
         self.xform.save()
         self._create_user_and_login("jo")
-        count = Instance.objects.filter(deleted_at=None).count()
+        count = Instance.objects.count()
         instance = Instance.objects.filter(
             xform=self.xform).latest('date_created')
         # delete
         params = {'id': instance.id}
         response = self.client.post(self.delete_url, params)
         self.assertEqual(response.status_code, 403)
         self.assertEqual(
-            Instance.objects.filter(deleted_at=None).count(), count)
+            Instance.objects.count(), count)
 
     def test_owner_can_delete(self):
         # Test if Form owner can delete
         # check record exist before delete and after delete
-        count = Instance.objects.filter(deleted_at=None).count()
+        count = Instance.objects.count()
         instance = Instance.objects.filter(
             xform=self.xform).latest('date_created')
-        self.assertEqual(instance.deleted_at, None)
         # delete
         params = {'id': instance.id}
         response = self.client.post(self.delete_url, params)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(
-            Instance.objects.filter(deleted_at=None).count(), count - 1)
-        instance = Instance.objects.get(id=instance.id)
-        self.assertTrue(isinstance(instance.deleted_at, datetime))
-        self.assertNotEqual(instance.deleted_at, None)
+            Instance.objects.count(), count - 1)
+        self.assertFalse(Instance.objects.filter(id=instance.id).exists())
         query = '{"_id": %s}' % instance.id
         self.mongo_args.update({"query": query})
         # check that query_mongo will not return the deleted record
         after = list(ParsedInstance.query_mongo(**self.mongo_args))
         self.assertEqual(len(after), count - 1)
 
     def test_delete_updates_mongo(self):
-        count = Instance.objects.filter(
-            xform=self.xform, deleted_at=None).count()
+        count = Instance.objects.filter(xform=self.xform).count()
         instance = Instance.objects.filter(
             xform=self.xform).latest('date_created')
         # delete
         params = {'id': instance.id}
         response = self.client.post(self.delete_url, params)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(
-            Instance.objects.filter(
-                xform=self.xform, deleted_at=None).count(), count - 1)
-        # check that instance's deleted_at is set
-        instance = Instance.objects.get(id=instance.id)
-        self.assertTrue(isinstance(instance.deleted_at, datetime))
-        # check mongo record was marked as deleted
+            Instance.objects.filter(xform=self.xform).count(),
+            count - 1,
+        )
+        # check that instance was removed from postgres
+        self.assertFalse(Instance.objects.filter(id=instance.id).exists())
+        # check that mongo record was also removed
         cursor = settings.MONGO_DB.instances.find(
             {common_tags.ID: instance.id})
-        self.assertEqual(cursor.count(), 1)
-        record = cursor.next()
-        self.assertIsNotNone(record[common_tags.DELETEDAT])
+        self.assertEqual(cursor.count(), 0)

onadata/apps/viewer/models/parsed_instance.py

@@ -22,7 +22,6 @@
     GEOLOCATION,
     SUBMISSION_TIME,
     MONGO_STRFTIME,
-    DELETEDAT,
     TAGS,
     NOTES,
     SUBMITTED_BY,
@@ -250,9 +249,6 @@ def to_dict_for_mongo(self):
             if self.instance.user else None
         }
 
-        if isinstance(self.instance.deleted_at, datetime.datetime):
-            data[DELETEDAT] = self.instance.deleted_at.strftime(MONGO_STRFTIME)
-
         d.update(data)
 
         return MongoHelper.to_safe_dict(d)

onadata/apps/viewer/pandas_mongo_bridge.py

@@ -87,8 +87,15 @@ def get_prefix_from_xpath(xpath):
 
 
 class AbstractDataFrameBuilder(object):
-    IGNORED_COLUMNS = [XFORM_ID_STRING, STATUS, ID, ATTACHMENTS, GEOLOCATION,
-                       DELETEDAT, SUBMITTED_BY]
+    IGNORED_COLUMNS = [
+        XFORM_ID_STRING,
+        STATUS,
+        ID,
+        ATTACHMENTS,
+        GEOLOCATION,
+        DELETEDAT,  # no longer used but may persist in old submissions
+        SUBMITTED_BY,
+    ]
     # fields NOT within the form def that we want to include
     ADDITIONAL_COLUMNS = [UUID, SUBMISSION_TIME, TAGS, NOTES, VALIDATION_STATUS]
     BINARY_SELECT_MULTIPLES = False

onadata/apps/viewer/tests/test_pandas_mongo_bridge.py

@@ -253,6 +253,7 @@ def test_csv_columns_for_gps_within_groups(self):
         ] + AbstractDataFrameBuilder.ADDITIONAL_COLUMNS +\
             AbstractDataFrameBuilder.IGNORED_COLUMNS
         try:
+            # `_deleted_at` is no longer used but may persist in old submissions
             expected_columns.remove('_deleted_at')
         except ValueError:
             pass

onadata/libs/data/query.py

@@ -64,7 +64,6 @@ def _postgres_count_group(field, name, xform):
 
     return "SELECT %(json)s AS \"%(name)s\", COUNT(*) AS count FROM "\
            "%(table)s WHERE %(restrict_field)s=%(restrict_value)s "\
-           "AND %(exclude_deleted)s "\
            "GROUP BY %(json)s" % string_args
 
 
@@ -81,8 +80,7 @@ def _query_args(field, name, xform):
         'json': _json_query(field),
         'name': name,
         'restrict_field': 'xform_id',
-        'restrict_value': xform.pk,
-        'exclude_deleted': 'deleted_at IS NULL'}
+        'restrict_value': xform.pk}
 
 
 def _select_key(field, name, xform):

onadata/libs/utils/common_tags.py

@@ -34,7 +34,7 @@
 DATE = "_date"
 GEOLOCATION = "_geolocation"
 SUBMISSION_TIME = '_submission_time'
-DELETEDAT = "_deleted_at"  # marker for delete surveys
+DELETEDAT = "_deleted_at"  # no longer used but may persist in old submissions
 SUBMITTED_BY = "_submitted_by"
 VALIDATION_STATUS = "_validation_status"
 

onadata/libs/utils/export_tools.py

@@ -178,8 +178,13 @@ def dict_to_joined_export(data, index, indices, name):
 
 
 class ExportBuilder(object):
-    IGNORED_COLUMNS = [XFORM_ID_STRING, STATUS, ATTACHMENTS, GEOLOCATION,
-                       DELETEDAT]
+    IGNORED_COLUMNS = [
+        XFORM_ID_STRING,
+        STATUS,
+        ATTACHMENTS,
+        GEOLOCATION,
+        DELETEDAT,  # no longer used but may persist in old submissions
+    ]
     # fields we export but are not within the form's structure
     EXTRA_FIELDS = [ID, UUID, SUBMISSION_TIME, INDEX, PARENT_TABLE_NAME,
                     PARENT_INDEX, TAGS, NOTES]
@@ -767,9 +772,10 @@ def query_mongo(username, id_string, query=None):
 
 
 def should_create_new_export(xform, export_type):
-    if Export.objects.filter(
-            xform=xform, export_type=export_type).count() == 0\
-            or Export.exports_outdated(xform, export_type=export_type):
+    if (
+        not Export.objects.filter(xform=xform, export_type=export_type).exists()
+        or Export.exports_outdated(xform, export_type=export_type)
+    ):
         return True
     return False