Skip to content

Commit

Permalink
Revert to have data exports default sorting by id (#2474)
Browse files Browse the repository at this point in the history
* revert to have data exports default sorting by id

sort export data by id by default

* add test case

* remove assertions in test case

when fields are provided in the query no ordering is applied

* fix fetching data on select columns not sorting

sorting when fetching data is not applied when fetching data on select columns that are not json fields

* refactor code

* fix lint error

Unexpected keyword argument 'json_only' in function call

* refactor code

get rid of ctypes.ArgumentError: error raised when working with GIS fields with Manager.raw() by selecting only the columns we need

* fix failing tests
  • Loading branch information
kelvin-muchiri authored Sep 13, 2023
1 parent 7b8d0d7 commit 170c8ca
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 51 deletions.
16 changes: 1 addition & 15 deletions docs/data.rst
Original file line number Diff line number Diff line change
Expand Up @@ -372,9 +372,7 @@ Sample response with link header

Sort submitted data of a specific form using existing fields
-------------------------------------------------------------
Provides a sorted list of json submitted data for a specific form by specifing the order in which the query returns matching data.

No ordering is applied by default -- the data is returned in any arbitrary order. Use the `sort` parameter to filter the list of submissions. The sort parameter has field and value pairs.
Provides a sorted list of json submitted data for a specific form by specifing the order in which the query returns matching data. Use the `sort` parameter to filter the list of submissions.The sort parameter has field and value pairs.

::

Expand All @@ -392,18 +390,6 @@ Descending sort query using the age field:

{"age":-1}

Query sorted by id field in ascending.

::

{"_id":1}

Query sorted by id field in descending.

::

{"_id":-1}


Example of Ascending Sort
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
11 changes: 4 additions & 7 deletions onadata/apps/api/tests/viewsets/test_data_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -3565,22 +3565,19 @@ def test_csv_export(self):
number_records = len(list(csv_reader))
self.assertEqual(number_records, 4)

def test_sort_query_param(self):
"""sort query param works with exports"""

def test_default_ordering(self):
"""Export data is sorted by id by default"""
self._make_submissions()
formid = self.xform.pk
# sort csv export data by id in descending order
request = self.factory.get(
"/", data={"format": "csv", "sort": '{"_id": -1}'}, **self.extra
)
request = self.factory.get("/", data={"format": "csv"}, **self.extra)
response = self.view(request, pk=formid)
self.assertEqual(response.status_code, 200)
csv_file_obj = StringIO(
"".join([c.decode("utf-8") for c in response.streaming_content])
)
csv_reader = csv.reader(csv_file_obj)
instances = Instance.objects.filter(xform_id=formid).order_by("-id")
instances = Instance.objects.filter(xform_id=formid).order_by("id")
self.assertEqual(instances.count(), 4)
headers = next(csv_reader)
expected_headers = [
Expand Down
3 changes: 2 additions & 1 deletion onadata/apps/api/tests/viewsets/test_tableau_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
OpenData tests.
"""
import os
import sys
import json
from re import search
from django.test import RequestFactory
Expand Down Expand Up @@ -396,7 +397,7 @@ def test_gt_id_query_param(self):
self.view = TableauViewSet.as_view({"get": "data"})
_open_data = get_or_create_opendata(self.xform)
uuid = _open_data[0].uuid
request = self.factory.get("/", data={"gt_id": 100000}, **self.extra)
request = self.factory.get("/", data={"gt_id": sys.maxsize}, **self.extra)
response = self.view(request, uuid=uuid)
self.assertEqual(response.status_code, 200)
row_data = streaming_data(response)
Expand Down
4 changes: 2 additions & 2 deletions onadata/apps/api/tests/viewsets/test_xform_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -3859,7 +3859,7 @@ def test_csv_export_with_win_excel_utf8(self):
self.assertEqual(response.status_code, 200)
self.assertTrue(response.data.get("has_hxl_support"))
# sort csv data in ascending order
data = {"win_excel_utf8": True, "sort": '{"_id": 1}'}
data = {"win_excel_utf8": True}
request = self.factory.get("/", data=data, **self.extra)
response = view(request, pk=self.xform.pk, format="csv")
self.assertEqual(response.status_code, 200)
Expand Down Expand Up @@ -3888,7 +3888,7 @@ def test_csv_export_with_win_excel_utf8(self):
basename, ext = os.path.splitext(filename)
self.assertEqual(ext, ".csv")
# sort csv data in ascending order
data = {"win_excel_utf8": False, "sort": '{"_id": 1}'}
data = {"win_excel_utf8": False}
request = self.factory.get("/", data=data, **self.extra)
response = view(request, pk=self.xform.pk, format="csv")
self.assertEqual(response.status_code, 200)
Expand Down
5 changes: 0 additions & 5 deletions onadata/apps/api/viewsets/data_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -720,11 +720,6 @@ def set_object_list(self, query, fields, sort, start, limit, is_public_request):
start = (page - 1) * page_size
limit = page_size

if sort is None:
# Paginated data needs to be sorted. We order by
# id ascending if sort is empty
sort = '{"_id": 1}'

if should_query_json_fields:
data = query_fields_data(
xform,
Expand Down
5 changes: 0 additions & 5 deletions onadata/apps/main/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -598,11 +598,6 @@ def api(request, username=None, id_string=None): # noqa C901
args["start_index"] = start_index
args["limit"] = page_size

if args.get("sort") is None:
# Paginated data needs to be sorted. We order by id ascending if
# sort is empty
args["sort"] = '{"_id": 1}'

if "start" in request.GET:
args["start_index"] = int(request.GET.get("start"))

Expand Down
31 changes: 15 additions & 16 deletions onadata/apps/viewer/models/parsed_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ def _start_index_limit(sql, params, start_index, limit):


def _get_sort_fields(sort):
sort = [] if sort is None else sort_from_mongo_sort_str(sort)
sort = ["id"] if sort is None else sort_from_mongo_sort_str(sort)

return list(_parse_sort_fields(sort))

Expand Down Expand Up @@ -258,7 +258,7 @@ def get_sql_with_params(
sql = "SELECT id,json FROM logger_instance"

else:
sql = "SELECT * FROM logger_instance"
sql = "SELECT id,json,xml FROM logger_instance"

sql_where, params = build_sql_where(xform, query, start, end)
sql += f" {sql_where}"
Expand All @@ -275,20 +275,19 @@ def get_sql_with_params(
)
sql = f"{sql} {_json_order_by}"
else:
if not fields:
sql += " ORDER BY"

for index, sort_field in enumerate(sort):
if sort_field.startswith("-"):
sort_field = sort_field.removeprefix("-")
# It's safe to use string interpolation since this
# is a column and not a value
sql += f" {sort_field} DESC"
else:
sql += f" {sort_field} ASC"

if index != len(sort) - 1:
sql += ","
sql += " ORDER BY"

for index, sort_field in enumerate(sort):
if sort_field.startswith("-"):
sort_field = sort_field.removeprefix("-")
# It's safe to use string interpolation since this
# is a column and not a value
sql += f" {sort_field} DESC"
else:
sql += f" {sort_field} ASC"

if index != len(sort) - 1:
sql += ","

sql, params = _start_index_limit(sql, params, start_index, limit)

Expand Down

0 comments on commit 170c8ca

Please sign in to comment.