From da683e676388730b2ebfd280e3af4e44a93dfb35 Mon Sep 17 00:00:00 2001 From: Dickson Ukang'a Date: Wed, 24 Jan 2018 05:54:28 +0300 Subject: [PATCH 1/5] Support filter queries for dataviews exports. Fix #1199 Date filter support for dataviews. --- .../tests/viewsets/test_dataview_viewset.py | 64 +++++++++++++++++-- onadata/apps/api/viewsets/dataview_viewset.py | 3 + onadata/libs/utils/export_tools.py | 3 +- 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/onadata/apps/api/tests/viewsets/test_dataview_viewset.py b/onadata/apps/api/tests/viewsets/test_dataview_viewset.py index df1e7a8db3..5a3873937d 100644 --- a/onadata/apps/api/tests/viewsets/test_dataview_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_dataview_viewset.py @@ -2,10 +2,12 @@ import os import csv +from datetime import datetime, timedelta from django.conf import settings from django.test.utils import override_settings from django.core.cache import cache from django.core.files.storage import default_storage +from django.utils.timezone import utc from mock import patch import xlrd @@ -22,7 +24,7 @@ DATAVIEW_COUNT, DATAVIEW_LAST_SUBMISSION_TIME, PROJECT_LINKED_DATAVIEWS) -from onadata.libs.utils.common_tags import EDITED +from onadata.libs.utils.common_tags import EDITED, MONGO_STRFTIME from onadata.apps.api.viewsets.xform_viewset import XFormViewSet from onadata.libs.utils.common_tools import ( filename_from_disposition, get_response_content) @@ -31,7 +33,7 @@ class TestDataViewViewSet(TestAbstractViewSet): def setUp(self): - super(self.__class__, self).setUp() + super(TestDataViewViewSet, self).setUp() xlsform_path = os.path.join( settings.PROJECT_ROOT, 'libs', 'tests', "utils", "fixtures", "tutorial.xls") @@ -703,9 +705,8 @@ def test_export_xls_dataview_with_labels_async(self, async_result): workbook = xlrd.open_workbook(export.full_filepath) main_sheet = workbook.sheets()[0] labels = main_sheet.row_values(1) - self.assertIn( - 'Gender', labels - ) + self.assertIn('Gender', labels) + self.assertEqual(main_sheet.nrows, 5) def _test_csv_export_with_hxl_support(self, columns, expected_output): data = { @@ -1328,3 +1329,56 @@ def test_search_dataview_data(self): def test_invalid_url_parameters(self): response = self.client.get('/api/v1/dataviews/css/ona.css/') self.assertEquals(response.status_code, 404) + + @override_settings(CELERY_ALWAYS_EAGER=True) + @patch('onadata.apps.api.viewsets.dataview_viewset.AsyncResult') + def test_export_xls_dataview_with_date_filter(self, async_result): + """ + Test dataview export with a date filter. + """ + self._create_dataview() + self._publish_xls_form_to_project() + start_date = datetime(2014, 9, 12, tzinfo=utc) + first_datetime = start_date.strftime(MONGO_STRFTIME) + second_datetime = start_date + timedelta(days=1, hours=20) + query_str = '{"_submission_time": {"$gte": "'\ + + first_datetime + '", "$lte": "'\ + + second_datetime.strftime(MONGO_STRFTIME) + '"}}' + + view = DataViewViewSet.as_view({ + 'get': 'export_async', + }) + + request = self.factory.get('/', data={"format": "xls", + 'include_labels': 'true', + 'query': query_str}, + **self.extra) + response = view(request, pk=self.data_view.pk) + self.assertIsNotNone(response.data) + + self.assertEqual(response.status_code, 202) + self.assertTrue('job_uuid' in response.data) + task_id = response.data.get('job_uuid') + + export_pk = Export.objects.all().order_by('pk').reverse()[0].pk + + # metaclass for mocking results + job = type('AsyncResultMock', (), + {'state': 'SUCCESS', 'result': export_pk}) + async_result.return_value = job + + get_data = {'job_uuid': task_id} + request = self.factory.get('/', data=get_data, **self.extra) + response = view(request, pk=self.data_view.pk) + + self.assertIn('export_url', response.data) + + self.assertTrue(async_result.called) + self.assertEqual(response.status_code, 202) + export = Export.objects.get(task_id=task_id) + self.assertTrue(export.is_successful) + workbook = xlrd.open_workbook(export.full_filepath) + main_sheet = workbook.sheets()[0] + labels = main_sheet.row_values(1) + self.assertIn('Gender', labels) + self.assertEqual(main_sheet.nrows, 3) diff --git a/onadata/apps/api/viewsets/dataview_viewset.py b/onadata/apps/api/viewsets/dataview_viewset.py index a169cdbd36..710ed94ce7 100644 --- a/onadata/apps/api/viewsets/dataview_viewset.py +++ b/onadata/apps/api/viewsets/dataview_viewset.py @@ -107,6 +107,7 @@ def export_async(self, request, *args, **kwargs): include_hxl = params.get('include_hxl', False) include_labels = params.get('include_labels', False) include_labels_only = params.get('include_labels_only', False) + query = params.get("query") dataview = self.get_object() xform = dataview.xform @@ -134,6 +135,8 @@ def export_async(self, request, *args, **kwargs): 'include_labels': include_labels, 'include_labels_only': include_labels_only } + if query: + options.update({'query': query}) if job_uuid: job = AsyncResult(job_uuid) diff --git a/onadata/libs/utils/export_tools.py b/onadata/libs/utils/export_tools.py index 42cb561d54..54fbac7429 100644 --- a/onadata/libs/utils/export_tools.py +++ b/onadata/libs/utils/export_tools.py @@ -165,7 +165,8 @@ def generate_export(export_type, xform, export_id=None, options=None, dataview = None if options.get("dataview_pk"): dataview = DataView.objects.get(pk=options.get("dataview_pk")) - records = dataview.query_data(dataview, all_data=True) + records = dataview.query_data(dataview, all_data=True, + filter_query=filter_query) total_records = dataview.query_data(dataview, count=True)[0].get('count') else: From 398176cbe4de427ed2cbfa1a20b8ba1d41a67c2a Mon Sep 17 00:00:00 2001 From: Dickson Ukang'a Date: Wed, 24 Jan 2018 13:53:25 +0300 Subject: [PATCH 2/5] Add support for filter queries for dataviews csv exports.' --- .../tests/viewsets/test_dataview_viewset.py | 87 +++++++++++++++++++ onadata/apps/api/viewsets/dataview_viewset.py | 2 +- onadata/libs/utils/csv_builder.py | 6 +- 3 files changed, 92 insertions(+), 3 deletions(-) diff --git a/onadata/apps/api/tests/viewsets/test_dataview_viewset.py b/onadata/apps/api/tests/viewsets/test_dataview_viewset.py index 5a3873937d..e1c953f05f 100644 --- a/onadata/apps/api/tests/viewsets/test_dataview_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_dataview_viewset.py @@ -1382,3 +1382,90 @@ def test_export_xls_dataview_with_date_filter(self, async_result): labels = main_sheet.row_values(1) self.assertIn('Gender', labels) self.assertEqual(main_sheet.nrows, 3) + + def test_csv_export_dataview_date_filter(self): + """ + Test dataview csv export with a date filter. + """ + self._create_dataview() + self._publish_xls_form_to_project() + start_date = datetime(2014, 9, 12, tzinfo=utc) + first_datetime = start_date.strftime(MONGO_STRFTIME) + second_datetime = start_date + timedelta(days=1, hours=20) + query_str = '{"_submission_time": {"$gte": "'\ + + first_datetime + '", "$lte": "'\ + + second_datetime.strftime(MONGO_STRFTIME) + '"}}' + count = Export.objects.all().count() + + view = DataViewViewSet.as_view({ + 'get': 'data', + }) + + request = self.factory.get('/', data={'query': query_str}, + **self.extra) + response = view(request, pk=self.data_view.pk, format='csv') + self.assertEqual(response.status_code, 200) + + self.assertEquals(count + 1, Export.objects.all().count()) + + headers = dict(response.items()) + self.assertEqual(headers['Content-Type'], 'application/csv') + content_disposition = headers['Content-Disposition'] + filename = filename_from_disposition(content_disposition) + basename, ext = os.path.splitext(filename) + self.assertEqual(ext, '.csv') + + content = get_response_content(response) + self.assertEqual(content, 'name,age,gender\nDennis Wambua,28,male\n') + + @override_settings(CELERY_ALWAYS_EAGER=True) + @patch('onadata.apps.api.viewsets.dataview_viewset.AsyncResult') + def test_csv_export_async_dataview_date_filter(self, async_result): + """ + Test dataview csv export async with a date filter. + """ + self._create_dataview() + self._publish_xls_form_to_project() + start_date = datetime(2014, 9, 12, tzinfo=utc) + first_datetime = start_date.strftime(MONGO_STRFTIME) + second_datetime = start_date + timedelta(days=1, hours=20) + query_str = '{"_submission_time": {"$gte": "'\ + + first_datetime + '", "$lte": "'\ + + second_datetime.strftime(MONGO_STRFTIME) + '"}}' + count = Export.objects.all().count() + + view = DataViewViewSet.as_view({ + 'get': 'export_async', + }) + + request = self.factory.get('/', data={"format": "csv", + 'query': query_str}, + **self.extra) + response = view(request, pk=self.data_view.pk) + self.assertEqual(response.status_code, 202) + self.assertIsNotNone(response.data) + self.assertTrue('job_uuid' in response.data) + task_id = response.data.get('job_uuid') + self.assertEquals(count + 1, Export.objects.all().count()) + + export_pk = Export.objects.all().order_by('pk').reverse()[0].pk + + # metaclass for mocking results + job = type('AsyncResultMock', (), + {'state': 'SUCCESS', 'result': export_pk}) + async_result.return_value = job + + get_data = {'job_uuid': task_id} + request = self.factory.get('/', data=get_data, **self.extra) + response = view(request, pk=self.data_view.pk) + + self.assertIn('export_url', response.data) + + self.assertTrue(async_result.called) + self.assertEqual(response.status_code, 202) + export = Export.objects.get(task_id=task_id) + self.assertTrue(export.is_successful) + with open(export.full_filepath) as csv_file: + self.assertEqual( + csv_file.read(), + 'name,age,gender\nDennis Wambua,28,male\n') diff --git a/onadata/apps/api/viewsets/dataview_viewset.py b/onadata/apps/api/viewsets/dataview_viewset.py index 710ed94ce7..d56ccc5e6f 100644 --- a/onadata/apps/api/viewsets/dataview_viewset.py +++ b/onadata/apps/api/viewsets/dataview_viewset.py @@ -95,7 +95,7 @@ def data(self, request, format='json', **kwargs): return Response(serializer.data) else: - return custom_response_handler(request, self.object.xform, None, + return custom_response_handler(request, self.object.xform, query, export_type, dataview=self.object) diff --git a/onadata/libs/utils/csv_builder.py b/onadata/libs/utils/csv_builder.py index cb4755b78d..5061d4b414 100644 --- a/onadata/libs/utils/csv_builder.py +++ b/onadata/libs/utils/csv_builder.py @@ -525,7 +525,8 @@ def export_to(self, path, dataview=None): self._build_ordered_columns(self.dd.survey, self.ordered_columns) if dataview: - cursor = dataview.query_data(dataview, all_data=True) + cursor = dataview.query_data(dataview, all_data=True, + filter_query=self.filter_query) if isinstance(cursor, QuerySet): cursor = cursor.iterator() self._update_columns_from_data(cursor) @@ -535,7 +536,8 @@ def export_to(self, path, dataview=None): for xpath, cols in self.ordered_columns.iteritems() if [c for c in dataview.columns if xpath.startswith(c)]] )) - cursor = dataview.query_data(dataview, all_data=True) + cursor = dataview.query_data(dataview, all_data=True, + filter_query=self.filter_query) if isinstance(cursor, QuerySet): cursor = cursor.iterator() data = self._format_for_dataframe(cursor) From e4f104a82593ce463886c6c8bc1cca30c339d5b3 Mon Sep 17 00:00:00 2001 From: Dickson Ukang'a Date: Wed, 24 Jan 2018 17:08:05 +0300 Subject: [PATCH 3/5] Refactor test variable to avoid collissions. --- .../tests/viewsets/test_project_viewset.py | 76 ++++++++++--------- 1 file changed, 41 insertions(+), 35 deletions(-) diff --git a/onadata/apps/api/tests/viewsets/test_project_viewset.py b/onadata/apps/api/tests/viewsets/test_project_viewset.py index b1f08e2c66..1a04f65b92 100644 --- a/onadata/apps/api/tests/viewsets/test_project_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_project_viewset.py @@ -1,33 +1,36 @@ -import os +# -*- coding=utf-8 -*- +""" +Test ProjectViewSet module. +""" import json -import requests -from mock import patch -from mock import MagicMock +import os from operator import itemgetter -from httmock import urlmatch, HTTMock +import requests from django.conf import settings +from django.db.models import Q +from httmock import HTTMock, urlmatch +from mock import MagicMock, patch -from onadata.apps.logger.models import Project -from onadata.apps.logger.models import XForm -from onadata.apps.api.tests.viewsets.test_abstract_viewset import\ +from onadata.apps.api import tools +from onadata.apps.api.tests.viewsets.test_abstract_viewset import \ TestAbstractViewSet +from onadata.apps.api.tools import get_organization_owners_team +from onadata.apps.api.viewsets.organization_profile_viewset import \ + OrganizationProfileViewSet from onadata.apps.api.viewsets.project_viewset import ProjectViewSet -from onadata.libs.permissions import ( - OwnerRole, ReadOnlyRole, ManagerRole, DataEntryRole, EditorMinorRole, - EditorRole, ReadOnlyRoleNoDownload, DataEntryMinorRole, DataEntryOnlyRole, - ROLES_ORDERED) -from onadata.libs.serializers.project_serializer import ProjectSerializer,\ - BaseProjectSerializer +from onadata.apps.api.viewsets.team_viewset import TeamViewSet +from onadata.apps.logger.models import Project, XForm +from onadata.apps.main.models import MetaData from onadata.libs import permissions as role from onadata.libs.models.share_project import ShareProject -from django.db.models import Q -from onadata.apps.main.models import MetaData -from onadata.apps.api.viewsets.team_viewset import TeamViewSet -from onadata.apps.api import tools -from onadata.apps.api.viewsets.organization_profile_viewset import\ - OrganizationProfileViewSet -from onadata.apps.api.tools import get_organization_owners_team +from onadata.libs.permissions import (ROLES_ORDERED, DataEntryMinorRole, + DataEntryOnlyRole, DataEntryRole, + EditorMinorRole, EditorRole, ManagerRole, + OwnerRole, ReadOnlyRole, + ReadOnlyRoleNoDownload) +from onadata.libs.serializers.project_serializer import (BaseProjectSerializer, + ProjectSerializer) ROLES = [ReadOnlyRoleNoDownload, ReadOnlyRole, @@ -877,8 +880,12 @@ def test_project_share_remove_user(self): self.project)) def test_project_filter_by_owner(self): + """ + Test projects endpoint filter by owner. + """ self._project_create() - alice_data = {'username': 'alice', 'email': 'alice@localhost.com'} + alice_data = {'username': 'alice', 'email': 'alice@localhost.com', + 'first_name': 'Alice', 'last_name': 'Alice'} self._login_user_and_profile(alice_data) ReadOnlyRole.add(self.user, self.project) @@ -890,9 +897,8 @@ def test_project_filter_by_owner(self): response = view(request, pk=self.project.pk) request.user = self.user self.project.refresh_from_db() - self.project_data = BaseProjectSerializer( + bobs_project_data = BaseProjectSerializer( self.project, context={'request': request}).data - updated_project_data = self.project_data self._project_create({'name': 'another project'}) @@ -902,14 +908,14 @@ def test_project_filter_by_owner(self): self.assertEqual(response.status_code, 200) request = self.factory.get('/', {'owner': 'alice'}, **self.extra) request.user = self.user - self.project_data = BaseProjectSerializer( + alice_project_data = BaseProjectSerializer( self.project, context={'request': request}).data result = [{'owner': p.get('owner'), 'projectid': p.get('projectid')} for p in response.data] bob_data = {'owner': 'http://testserver/api/v1/users/bob', - 'projectid': updated_project_data.get('projectid')} + 'projectid': bobs_project_data.get('projectid')} alice_data = {'owner': 'http://testserver/api/v1/users/alice', - 'projectid': self.project_data.get('projectid')} + 'projectid': alice_project_data.get('projectid')} self.assertIn(bob_data, result) self.assertIn(alice_data, result) @@ -917,15 +923,15 @@ def test_project_filter_by_owner(self): request = self.factory.get('/', {'owner': 'bob'}, **self.extra) response = self.view(request) self.assertEqual(response.status_code, 200) - self.assertIn(updated_project_data, response.data) - self.assertNotIn(self.project_data, response.data) + self.assertIn(bobs_project_data, response.data) + self.assertNotIn(alice_project_data, response.data) # only alice's project request = self.factory.get('/', {'owner': 'alice'}, **self.extra) response = self.view(request) self.assertEqual(response.status_code, 200) - self.assertNotIn(updated_project_data, response.data) - self.assertIn(self.project_data, response.data) + self.assertNotIn(bobs_project_data, response.data) + self.assertIn(alice_project_data, response.data) # none existent user request = self.factory.get('/', {'owner': 'noone'}, **self.extra) @@ -941,20 +947,20 @@ def test_project_filter_by_owner(self): request = self.factory.get('/', {'owner': 'alice'}, **self.extra) response = self.view(request) self.assertEqual(response.status_code, 200) - self.assertNotIn(updated_project_data, response.data) - self.assertNotIn(self.project_data, response.data) + self.assertNotIn(bobs_project_data, response.data) + self.assertNotIn(alice_project_data, response.data) # should show public project when filtered by owner self.project.shared = True self.project.save() request.user = self.user - self.project_data = BaseProjectSerializer( + alice_project_data = BaseProjectSerializer( self.project, context={'request': request}).data request = self.factory.get('/', {'owner': 'alice'}, **self.extra) response = self.view(request) self.assertEqual(response.status_code, 200) - self.assertIn(self.project_data, response.data) + self.assertIn(alice_project_data, response.data) # should show deleted project public project when filtered by owner self.project.soft_delete() From b25b2ae91fb4af37a7dbbc21f05949cd184ca7a4 Mon Sep 17 00:00:00 2001 From: Kelvin Jayanoris Date: Tue, 30 Jan 2018 12:49:47 +0300 Subject: [PATCH 4/5] Define BaseProjectSerializer fields explicitly --- onadata/libs/serializers/project_serializer.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/onadata/libs/serializers/project_serializer.py b/onadata/libs/serializers/project_serializer.py index 8399224096..61bf183fa6 100644 --- a/onadata/libs/serializers/project_serializer.py +++ b/onadata/libs/serializers/project_serializer.py @@ -216,7 +216,10 @@ class BaseProjectSerializer(serializers.HyperlinkedModelSerializer): class Meta: model = Project - exclude = ('shared', 'organization', 'user_stars') + fields = ['url', 'projectid', 'owner', 'created_by', 'metadata', + 'starred', 'users', 'forms', 'public', 'tags', + 'num_datasets', 'last_submission_date', 'teams', 'name', + 'date_created', 'date_modified', 'deleted_at'] def get_starred(self, obj): return get_starred(obj, self.context['request']) From 5aeb0e4aa7fb4b2a7504303944c4205e59d3645f Mon Sep 17 00:00:00 2001 From: Dickson Ukang'a Date: Fri, 2 Feb 2018 12:18:12 +0300 Subject: [PATCH 5/5] Use the ShareProject model to apply role --- onadata/apps/api/tests/viewsets/test_project_viewset.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/onadata/apps/api/tests/viewsets/test_project_viewset.py b/onadata/apps/api/tests/viewsets/test_project_viewset.py index 1a04f65b92..eaf29df7ed 100644 --- a/onadata/apps/api/tests/viewsets/test_project_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_project_viewset.py @@ -888,7 +888,7 @@ def test_project_filter_by_owner(self): 'first_name': 'Alice', 'last_name': 'Alice'} self._login_user_and_profile(alice_data) - ReadOnlyRole.add(self.user, self.project) + ShareProject(self.project, self.user.username, 'readonly').save() view = ProjectViewSet.as_view({ 'get': 'retrieve'