Update GeoJSON endpoint to filter by instances with geoms

onadata/apps/api/tests/viewsets/test_data_viewset.py

@@ -2110,6 +2110,61 @@ def test_geojson_format(self):
         }
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data, data)
+
+    def test_instances_with_geopoints(self):
+        # publish sample geo submissions
+        self._publish_submit_geojson()
+
+        view = DataViewSet.as_view({"get": "list"})
+        request = self.factory.get("/", **self.extra)
+        response = view(request, pk=self.xform.pk, format="geojson")
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(self.xform.instances.count(), 4)
+        self.assertEqual(len(response.data["features"]), 4)
+
+        # check if instances_with_geopoints is True for the form
+        self.xform.refresh_from_db()
+        self.assertTrue(self.xform.instances_with_geopoints)
+
+    @patch("onadata.apps.viewer.signals._post_process_submissions")
+    def test_instances_with_empty_geopoints(self, mock_signal):
+        # publish sample geo submissions
+        self._publish_submit_geojson(has_empty_geoms=True)
+
+        view = DataViewSet.as_view({"delete": "destroy", "get": "list"})
+        request = self.factory.get("/", **self.extra)
+        response = view(request, pk=self.xform.pk, format="geojson")
+
+        # should return 200 if it has atleast one valid geom
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(self.xform.instances.count(), 2)
+
+        # check if instances_with_geopoints is False for the form
+        self.xform.refresh_from_db()
+        self.assertTrue(self.xform.instances_with_geopoints)
+
+        # soft delete instance with geoms
+        dataid = self.xform.instances.all().order_by("id")[0].pk
+        request = self.factory.delete("/", **self.extra)
+        response = view(request, pk=self.xform.pk, dataid=dataid)
+
+        # test that signal to update instances_with_geopoints is sent
+        self.assertTrue(mock_signal.called)
+
+        # get the soft deleted instance
+        first_xform_instance = self.xform.instances.get(pk=dataid)
+        self.assertEqual(first_xform_instance.deleted_by, request.user)
+
+        # return 404 if all instances dont have geoms
+        request = self.factory.get("/", **self.extra)
+        response = view(request, pk=self.xform.pk, format="geojson")
+        self.assertEqual(response.status_code, 404)
+        self.assertEqual(self.xform.instances.count(), 2)
+
+        # check if instances_with_geopoints is False for the form
+        self.xform.refresh_from_db()
+        self.assertFalse(self.xform.instances_with_geopoints)
 
     @patch("onadata.apps.api.viewsets.data_viewset" ".DataViewSet.paginate_queryset")
     def test_retry_on_operational_error(self, mock_paginate_queryset):

onadata/apps/api/viewsets/data_viewset.py

@@ -393,6 +393,7 @@ def destroy(self, request, *args, **kwargs):
             if request.user.has_perm(CAN_DELETE_SUBMISSION, self.object.xform):
                 instance_id = self.object.pk
                 delete_instance(self.object, request.user)
+
                 # send message
                 send_message(
                     instance_id=instance_id,
@@ -603,6 +604,10 @@ def list(self, request, *args, **kwargs):
             return super().list(request, *args, **kwargs)
 
         if export_type == "geojson":
+            # raise 404 if all instances dont have geoms
+            if not xform.instances_with_geopoints:
+                raise Http404(_("Not Found"))
+
             # add pagination when fetching geojson features
             page = self.paginate_queryset(self.object_list)
             serializer = self.get_serializer(page, many=True)

onadata/apps/logger/migrations/0004_update_instance_geoms.py

@@ -0,0 +1,27 @@
+# Generated by Django 3.2.16 on 2022-11-22 14:06
+
+from django.db import migrations
+from onadata.apps.logger.models.instance import Instance
+
+
+def update_instance_geoms(apps, schema_editor):
+    """
+    Update instance geom field with valid geom values
+    """
+    for inst in Instance.objects.filter(
+        deleted_at__isnull=True,
+        xform__downloadable=True,
+        xform__deleted_at__isnull=True,
+    ):
+        if inst.geom.empty:
+            inst.geom = None
+            inst.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('logger', '0003_alter_instance_media_all_received'),
+    ]
+
+    operations = [migrations.RunPython(update_instance_geoms)]

onadata/apps/logger/models/instance.py

@@ -281,8 +281,8 @@ def update_xform_submission_count(instance_id, created):
                 clear_project_cache(instance.xform.project_id)
 
 
-# pylint: disable=unused-argument,invalid-name
-def update_xform_submission_count_delete(sender, instance, **kwargs):
+@transaction.atomic()
+def _update_xform_submission_count_delete(instance):
     """Updates the XForm submissions count on deletion of a submission."""
     try:
         xform = XForm.objects.select_for_update().get(pk=instance.xform.pk)
@@ -316,9 +316,22 @@ def update_xform_submission_count_delete(sender, instance, **kwargs):
         safe_delete(f"{DATAVIEW_COUNT}{xform.pk}")
         safe_delete(f"{XFORM_COUNT}{xform.pk}")
 
-        if xform.instances.exclude(geom=None).count() < 1:
-            xform.instances_with_geopoints = False
-            xform.save()
+        # update xform if no instance has geoms
+        if (
+            instance.xform.instances.filter(
+                deleted_at__isnull=True
+            ).exclude(geom=None).count()
+            < 1
+        ):
+            instance.xform.instances_with_geopoints = False
+            instance.xform.save()
+
+
+# pylint: disable=unused-argument,invalid-name
+def update_xform_submission_count_delete(sender, instance, **kwargs):
+    """Updates the XForm submissions count on deletion of a submission."""
+    if instance:
+        _update_xform_submission_count_delete(instance)
 
 
 @app.task(bind=True, max_retries=3)
@@ -450,7 +463,10 @@ def _set_geom(self):
                 xform.save()
 
             # pylint: disable=attribute-defined-outside-init
-            self.geom = GeometryCollection(points)
+            if points:
+                self.geom = GeometryCollection(points)
+            else:
+                self.geom = None
 
     def _set_json(self):
         # pylint: disable=attribute-defined-outside-init
@@ -794,14 +810,12 @@ def soft_delete_attachments(self, user=None):
 def post_save_submission(sender, instance=None, created=False, **kwargs):
     """Update XForm, Project, JSON field
 
-    - XForm submission coun
+    - XForm submission count & instances_with_geopoints field
     - Project date modified
     - Update the submission JSON field data
     """
     if instance.deleted_at is not None:
-        _update_submission_count_for_today(
-            instance.xform_id, incr=False, date_created=instance.date_created
-        )
+        _update_xform_submission_count_delete(instance)
 
     if ASYNC_POST_SUBMISSION_PROCESSING_ENABLED:
         update_xform_submission_count_async.apply_async(args=[instance.pk, created])

onadata/apps/main/tests/fixtures/geolocation/GeoLocationForm_empty_geoms.csv

@@ -0,0 +1,3 @@
+today,location,_location_latitude,_location_longitude,_location_altitude,_location_precision,path,shape,meta/instanceID,_uuid,_submission_time,_tags,_notes,_version,_duration,_submitted_by
+2015-01-15,-1.294197 36.787219 0 34,-1.294197,36.787219,0,34,-1.294234 36.78722 0 30;-1.292286 36.787152 0 0;-1.295182 36.800439 0 0;-1.295289 36.802215 0 0;-1.286708 36.817235 0 0,-1.2942 36.78723 0 22;-1.292415 36.787356 0 0;-1.292586 36.779084 0 0;-1.29988 36.779546 0 0;-1.299666 36.788096 0 0;-1.2942 36.78723 0 22,uuid:7c0d8b2d-7ad0-4100-923d-f2767d532258,7c0d8b2d-7ad0-4100-923d-f2767d532258,2015-01-15T06:19:23,,,201501150616,,
+2015-01-15,n/a,n/a,n/a,n/a,n/a,-1.294234 36.78722 0 30;-1.292286 36.787152 0 0;-1.295182 36.800439 0 0;-1.295289 36.802215 0 0;-1.286708 36.817235 0 0,-1.2942 36.78723 0 22;-1.292415 36.787356 0 0;-1.292586 36.779084 0 0;-1.29988 36.779546 0 0;-1.299666 36.788096 0 0;-1.2942 36.78723 0 22,uuid:7c0d8b2d-7ad0-4100-923d-f2767d532259,7c0d8b2d-7ad0-4100-923d-f2767d532259,2015-01-15T06:19:23,,,201501150616,,

onadata/apps/main/tests/test_base.py

@@ -393,7 +393,7 @@ def _get_digest_client(self):
         client.set_authorization("bob", "bob", "Digest")
         return client
 
-    def _publish_submit_geojson(self):
+    def _publish_submit_geojson(self, has_empty_geoms=False):
         path = os.path.join(
             settings.PROJECT_ROOT,
             "apps",
@@ -406,6 +406,8 @@ def _publish_submit_geojson(self):
 
         self._publish_xls_file_and_set_xform(path)
 
+        csv_sub = "empty_geoms" if has_empty_geoms else "2015_01_15_01_28_45"
+
         view = XFormViewSet.as_view({"post": "csv_import"})
         with open(
             os.path.join(
@@ -415,7 +417,7 @@ def _publish_submit_geojson(self):
                 "tests",
                 "fixtures",
                 "geolocation",
-                "GeoLocationForm_2015_01_15_01_28_45.csv",
+                f"GeoLocationForm_{csv_sub}.csv",
             ),
             encoding="utf-8",
         ) as csv_import: