From 3e57a004f4d866edb466fe1f180e4b6a19777a63 Mon Sep 17 00:00:00 2001 From: Kelvin Muchiri Date: Mon, 18 Nov 2024 14:19:46 +0300 Subject: [PATCH] exclude deleting submissions from data list --- .../api/tests/viewsets/test_data_viewset.py | 32 ++++++++++++++++++- onadata/apps/api/viewsets/data_viewset.py | 26 ++++++++++++++- onadata/apps/viewer/models/parsed_instance.py | 24 ++++++++++++++ onadata/libs/tests/utils/test_logger_tools.py | 7 ++++ onadata/libs/utils/cache_tools.py | 2 ++ onadata/libs/utils/logger_tools.py | 3 +- 6 files changed, 91 insertions(+), 3 deletions(-) diff --git a/onadata/apps/api/tests/viewsets/test_data_viewset.py b/onadata/apps/api/tests/viewsets/test_data_viewset.py index f3c4ff33fb..e26137d13f 100644 --- a/onadata/apps/api/tests/viewsets/test_data_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_data_viewset.py @@ -2,6 +2,7 @@ """ Test /data API endpoint implementation. """ + from __future__ import unicode_literals import csv @@ -1796,7 +1797,7 @@ def test_deletion_of_bulk_submissions(self, mock_cache_set): self.assertEqual(current_count, 2) self.assertEqual(self.xform.num_of_submissions, 2) mock_cache_set.assert_called_once_with( - f"xfm-submissions-under-deletion-{formid}", + f"xfm-submissions-deleting-{formid}", [str(i.pk) for i in records_to_be_deleted], 3600, ) @@ -3781,6 +3782,35 @@ def test_merged_dataset_geojson(self): response.data, ) + def test_submissions_deletion_in_progress(self): + """Submissions whose deletion is in progress are excluded from list""" + self._make_submissions() + self.assertEqual(self.xform.instances.count(), 4) + view = DataViewSet.as_view({"get": "list"}) + formid = self.xform.pk + instances = self.xform.instances.all() + cache.set( + f"xfm-submissions-deleting-{self.xform.pk}", + [instances[0].pk, instances[1].pk], + ) + # No query + request = self.factory.get("/", **self.extra) + response = view(request, pk=formid) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data), 2) + # With query + data = {"query": '{"_submission_time":{"$gt":"2018-04-19"}}'} + request = self.factory.get("/", **self.extra, data=data) + response = view(request, pk=formid) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data), 2) + # With sort + data = {"sort": 1} + request = self.factory.get("/", **self.extra, data=data) + response = view(request, pk=formid) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data), 2) + class TestOSM(TestAbstractViewSet): """ diff --git a/onadata/apps/api/viewsets/data_viewset.py b/onadata/apps/api/viewsets/data_viewset.py index ce2923924c..12eb12cec1 100644 --- a/onadata/apps/api/viewsets/data_viewset.py +++ b/onadata/apps/api/viewsets/data_viewset.py @@ -37,6 +37,7 @@ from onadata.apps.viewer.models.parsed_instance import ( ParsedInstance, _get_sort_fields, + exclude_deleting_submissions_clause, get_etag_hash_from_query, get_sql_with_params, get_where_clause, @@ -70,6 +71,11 @@ ) from onadata.libs.serializers.geojson_serializer import GeoJsonSerializer from onadata.libs.utils.api_export_tools import custom_response_handler +from onadata.libs.utils.cache_tools import ( + XFORM_SUBMISSIONS_DELETING, + XFORM_SUBMISSIONS_DELETING_TTL, + safe_cache_set, +) from onadata.libs.utils.common_tools import json_stream, str_to_bool from onadata.libs.utils.viewer_tools import get_enketo_urls, get_form_url @@ -369,18 +375,26 @@ def destroy(self, request, *args, **kwargs): if not instance_ids and not delete_all_submissions: raise ParseError(_("Data id(s) not provided.")) - if instance_ids: + if not delete_all_submissions: instance_ids = [x for x in instance_ids.split(",") if x.isdigit()] if not instance_ids: raise ParseError(_("Invalid data ids were provided.")) + else: + instance_ids = None + delete_xform_submissions_async.delay( self.object.id, instance_ids, not permanent_delete, request.user.id, ) + safe_cache_set( + f"{XFORM_SUBMISSIONS_DELETING}{self.object.id}", + instance_ids, + XFORM_SUBMISSIONS_DELETING_TTL, + ) return Response(status=status.HTTP_200_OK) @@ -655,6 +669,16 @@ def set_object_list(self, query, fields, sort, start, limit, is_public_request): where, where_params = get_where_clause(query) + if not is_public_request: + # Exclude submissions whose deletion is in progress + exclude_del_sql, exclude_del_params = ( + exclude_deleting_submissions_clause(self.get_object().id) + ) + + if exclude_del_sql: + where.append(f" {exclude_del_sql}") + where_params.extend(exclude_del_params) + if where: # pylint: disable=attribute-defined-outside-init self.object_list = self.object_list.extra( diff --git a/onadata/apps/viewer/models/parsed_instance.py b/onadata/apps/viewer/models/parsed_instance.py index f50a218e77..c7c89a25a7 100644 --- a/onadata/apps/viewer/models/parsed_instance.py +++ b/onadata/apps/viewer/models/parsed_instance.py @@ -2,6 +2,7 @@ """ ParsedInstance model """ + import datetime from django.conf import settings @@ -21,6 +22,7 @@ json_order_by_params, sort_from_mongo_sort_str, ) +from onadata.libs.utils.cache_tools import XFORM_SUBMISSIONS_DELETING, safe_cache_get from onadata.libs.utils.common_tags import ( ATTACHMENTS, BAMBOO_DATASET_ID, @@ -179,6 +181,21 @@ def _get_sort_fields(sort): return list(_parse_sort_fields(sort)) +def exclude_deleting_submissions_clause(xform_id: int) -> tuple[str, list[int]]: + """Return SQL clause to exclude submissions whose deletion is in progress + + :param xform_id: XForm ID + :return: SQL and list of submission IDs under deletion + """ + instance_ids = safe_cache_get(f"{XFORM_SUBMISSIONS_DELETING}{xform_id}", []) + + if not instance_ids: + return ("", []) + + placeholders = ", ".join(["%s"] * len(instance_ids)) + return (f"id NOT IN ({placeholders})", instance_ids) + + def build_sql_where(xform, query, start=None, end=None): """Build SQL WHERE clause""" known_integers = [ @@ -209,6 +226,13 @@ def build_sql_where(xform, query, start=None, end=None): sql_where += " AND date_created <= %s" where_params += [end.isoformat()] + exclude_sql, exclude_params = exclude_deleting_submissions_clause(xform.pk) + + if exclude_sql: + # Exclude submissions whose deletion is in progress + sql_where += f" AND {exclude_sql}" + where_params += exclude_params + xform_pks = [xform.pk] if xform.is_merged_dataset: diff --git a/onadata/libs/tests/utils/test_logger_tools.py b/onadata/libs/tests/utils/test_logger_tools.py index b30a6fc128..daf97d5d18 100644 --- a/onadata/libs/tests/utils/test_logger_tools.py +++ b/onadata/libs/tests/utils/test_logger_tools.py @@ -1142,3 +1142,10 @@ def test_hard_delete_enabled(self): """Hard delete should be enabled for hard delete to be successful""" with self.assertRaises(PermissionDenied): delete_xform_submissions(self.xform, soft_delete=False) + + def test_cache_deleted(self): + """Cache tracking submissions being deleted is cleared""" + cache.set(f"xfm-submissions-deleting-{self.xform.id}", [self.instances[0].pk]) + delete_xform_submissions(self.xform) + + self.assertIsNone(cache.get(f"xfm-submissions-deleting-{self.xform.id}")) diff --git a/onadata/libs/utils/cache_tools.py b/onadata/libs/utils/cache_tools.py index 6ed6a27201..ab980d37fc 100644 --- a/onadata/libs/utils/cache_tools.py +++ b/onadata/libs/utils/cache_tools.py @@ -66,6 +66,8 @@ XFORM_MANIFEST_CACHE = "xfm-manifest-" XFORM_LIST_CACHE = "xfm-list-" XFROM_LIST_CACHE_TTL = 10 * 60 # 10 minutes converted to seconds +XFORM_SUBMISSIONS_DELETING = "xfm-submissions-deleting-" +XFORM_SUBMISSIONS_DELETING_TTL = 60 * 60 # 1 hour converted to seconds # Cache timeouts used in XForm model XFORM_REGENERATE_INSTANCE_JSON_TASK_TTL = 24 * 60 * 60 # 24 hrs converted to seconds diff --git a/onadata/libs/utils/logger_tools.py b/onadata/libs/utils/logger_tools.py index 7870af565f..a260c76d2d 100644 --- a/onadata/libs/utils/logger_tools.py +++ b/onadata/libs/utils/logger_tools.py @@ -103,6 +103,7 @@ ELIST_NUM_ENTITIES_CREATED_AT, ELIST_NUM_ENTITIES_IDS, ELIST_NUM_ENTITIES_LOCK, + XFORM_SUBMISSIONS_DELETING, safe_delete, set_cache_with_lock, ) @@ -1511,7 +1512,7 @@ def delete_xform_submissions( xform.project.date_modified = timezone.now() xform.project.save(update_fields=["date_modified"]) - + safe_delete(f"{XFORM_SUBMISSIONS_DELETING}{xform.pk}") send_message( instance_id=instance_ids, target_id=xform.id,