Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return correct geojson for polygons and geotrace data #2348

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 60 additions & 8 deletions onadata/apps/api/tests/viewsets/test_data_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -1118,11 +1118,11 @@ def test_data_with_query_parameter(self):
second_datetime = start_time + timedelta(days=1, hours=20)

query_str = (
'{"_submission_time": {"$gte": "'
+ first_datetime
+ '", "$lte": "'
+ second_datetime.strftime(MONGO_STRFTIME)
+ '"}}'
'{"_submission_time": {"$gte": "' +
first_datetime +
'", "$lte": "' +
second_datetime.strftime(MONGO_STRFTIME) +
'"}}'
)

request = self.factory.get("/?query=%s" % query_str, **self.extra)
Expand Down Expand Up @@ -2110,7 +2110,7 @@ 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()
Expand All @@ -2129,7 +2129,7 @@ def test_instances_with_geopoints(self):

@patch("onadata.apps.viewer.signals._post_process_submissions")
def test_instances_with_empty_geopoints(self, mock_signal):
# publish sample geo submissions
# publish sample geo submissions with polygons and polylines
self._publish_submit_geojson(has_empty_geoms=True)

view = DataViewSet.as_view({"delete": "destroy", "get": "list"})
Expand All @@ -2141,6 +2141,57 @@ def test_instances_with_empty_geopoints(self, mock_signal):
self.assertEqual(self.xform.instances.count(), 2)

# check if instances_with_geopoints is False for the form
self.assertFalse(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
# Update this test
request = self.factory.get("/", **self.extra)
response = view(request, pk=self.xform.pk, format="geojson")
self.assertEqual(response.status_code, 200)
self.assertEqual(
json.dumps(response.data)[:94],
'{"type": "FeatureCollection", "features":' +
' [{"type": "Feature", "geometry": null, "properties":')
self.assertEqual(len(response.data['features']), 1)
feature = dict(response.data['features'][0])
self.assertEqual(feature['type'], 'Feature')
self.assertEqual(feature['geometry'], None)
self.assertTrue(isinstance(feature['properties'], dict))
self.assertEqual(self.xform.instances.count(), 2)
self.assertEqual(self.xform.polygon_xpaths(), ['shape'])
self.assertEqual(self.xform.geotrace_xpaths(), ['path'])

# 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_no_polygons(self, mock_signal):

# publish sample geo submissions
self._publish_submit_geojson(has_empty_geoms=True, only_geopoints=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 True for the form
self.xform.refresh_from_db()
self.assertTrue(self.xform.instances_with_geopoints)

Expand All @@ -2159,8 +2210,9 @@ def test_instances_with_empty_geopoints(self, mock_signal):
# 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(self.xform.polygon_xpaths(), [])
self.assertEqual(self.xform.geotrace_xpaths(), [])
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()
Expand Down
9 changes: 5 additions & 4 deletions onadata/apps/api/viewsets/data_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,9 @@ def get_serializer_class(self):
elif fmt == "xml":
serializer_class = DataInstanceXMLSerializer
elif (
form_pk is not None
and dataid is None
and form_pk != self.public_data_endpoint
form_pk is not None and
dataid is None and
form_pk != self.public_data_endpoint
):
if sort or fields:
serializer_class = JsonDataSerializer
Expand Down Expand Up @@ -605,7 +605,8 @@ def list(self, request, *args, **kwargs):

if export_type == "geojson":
# raise 404 if all instances dont have geoms
if not xform.instances_with_geopoints:
if not xform.instances_with_geopoints and not (
xform.polygon_xpaths() or xform.geotrace_xpaths()):
raise Http404(_("Not Found"))

# add pagination when fetching geojson features
Expand Down
13 changes: 8 additions & 5 deletions onadata/apps/logger/models/instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,10 +320,13 @@ def _update_xform_submission_count_delete(instance):
if (
instance.xform.instances.filter(
deleted_at__isnull=True
).exclude(geom=None).count()
< 1
).exclude(geom=None).count() <
1
):
instance.xform.instances_with_geopoints = False
if (instance.xform.polygon_xpaths() or instance.xform.geotrace_xpaths()):
instance.xform.instances_with_geopoints = True
else:
instance.xform.instances_with_geopoints = False
instance.xform.save()


Expand Down Expand Up @@ -735,8 +738,8 @@ def get_expected_media(self):
media_list.extend([i["media/file"] for i in data["media"]])
else:
media_xpaths = (
self.xform.get_media_survey_xpaths()
+ self.xform.get_osm_survey_xpaths()
self.xform.get_media_survey_xpaths() +
self.xform.get_osm_survey_xpaths()
)
for media_xpath in media_xpaths:
media_list.extend(get_values_matching_key(data, media_xpath))
Expand Down
20 changes: 20 additions & 0 deletions onadata/apps/logger/models/xform.py
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,26 @@ def geopoint_xpaths(self):
if e.bind.get("type") == "geopoint"
]

def polygon_xpaths(self):
"""Returns the abbreviated_xpath of all fields of type `geoshape`."""
survey_elements = self.get_survey_elements()

return [
e.get_abbreviated_xpath()
for e in survey_elements
if e.bind.get("type") == "geoshape"
]

def geotrace_xpaths(self):
"""Returns the abbreviated_xpath of all fields of type `geotrace`."""
survey_elements = self.get_survey_elements()

return [
e.get_abbreviated_xpath()
for e in survey_elements
if e.bind.get("type") == "geotrace"
]

def xpath_of_first_geopoint(self):
"""Returns the abbreviated_xpath of the first field of type `geopoint`."""
geo_xpaths = self.geopoint_xpaths()
Expand Down
Binary file not shown.
9 changes: 5 additions & 4 deletions onadata/apps/main/tests/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,8 +360,8 @@ def _internet_on(self, url="http://74.125.113.99"):

def _set_auth_headers(self, username, password):
return {
"HTTP_AUTHORIZATION": "Basic "
+ base64.b64encode(f"{username}:{password}".encode("utf-8")).decode(
"HTTP_AUTHORIZATION": "Basic " +
base64.b64encode(f"{username}:{password}".encode("utf-8")).decode(
"utf-8"
),
}
Expand Down Expand Up @@ -393,15 +393,16 @@ def _get_digest_client(self):
client.set_authorization("bob", "bob", "Digest")
return client

def _publish_submit_geojson(self, has_empty_geoms=False):
def _publish_submit_geojson(self, has_empty_geoms=False, only_geopoints=False):
path = os.path.join(
settings.PROJECT_ROOT,
"apps",
"main",
"tests",
"fixtures",
"geolocation",
"GeoLocationForm.xlsx",
("GeoLocationFormNoPolylineOrPolygon.xlsx"
if only_geopoints else "GeoLocationForm.xlsx"),
)

self._publish_xls_file_and_set_xform(path)
Expand Down
2 changes: 1 addition & 1 deletion onadata/libs/serializers/geojson_serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def to_representation(self, instance):
geometry = (
geometry_from_string(points, simple_style)
if points
else geojson.Feature()
else None
)

ret["geometry"] = geometry
Expand Down