Skip to content

Commit

Permalink
exclude deleting submissions from data list
Browse files Browse the repository at this point in the history
  • Loading branch information
kelvin-muchiri committed Nov 18, 2024
1 parent 90d741c commit 3e57a00
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 3 deletions.
32 changes: 31 additions & 1 deletion onadata/apps/api/tests/viewsets/test_data_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"""
Test /data API endpoint implementation.
"""

from __future__ import unicode_literals

import csv
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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):
"""
Expand Down
26 changes: 25 additions & 1 deletion onadata/apps/api/viewsets/data_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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(
Expand Down
24 changes: 24 additions & 0 deletions onadata/apps/viewer/models/parsed_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"""
ParsedInstance model
"""

import datetime

from django.conf import settings
Expand All @@ -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,
Expand Down Expand Up @@ -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 = [
Expand Down Expand Up @@ -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:
Expand Down
7 changes: 7 additions & 0 deletions onadata/libs/tests/utils/test_logger_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"))
2 changes: 2 additions & 0 deletions onadata/libs/utils/cache_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion onadata/libs/utils/logger_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 3e57a00

Please sign in to comment.