From 33714c10a6c946d0f4ae70e6dc40d0351fdd0614 Mon Sep 17 00:00:00 2001 From: Kipchirchir Sigei Date: Fri, 14 Jul 2023 15:19:16 +0300 Subject: [PATCH 1/3] Add project and form level submission and download list filtering Signed-off-by: Kipchirchir Sigei --- .../apps/api/viewsets/briefcase_viewset.py | 34 +++++++++++++++++-- onadata/apps/main/urls.py | 20 +++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/onadata/apps/api/viewsets/briefcase_viewset.py b/onadata/apps/api/viewsets/briefcase_viewset.py index 7056d80163..67b1efa65c 100644 --- a/onadata/apps/api/viewsets/briefcase_viewset.py +++ b/onadata/apps/api/viewsets/briefcase_viewset.py @@ -98,6 +98,19 @@ def get_object(self, queryset=None): id_string = _extract_id_string(form_id) uuid = _extract_uuid(form_id) username = self.kwargs.get("username") + form_pk = self.kwargs.get("xform_pk") + project_pk = self.kwargs.get("project_pk") + + if not username: + if form_pk: + queryset = self.queryset.filter(pk=form_pk) + if queryset.first(): + username = queryset.first().user.username + elif project_pk: + queryset = queryset.filter(project__pk=project_pk) + if queryset.first(): + username = queryset.first().user.username + obj = get_object_or_404( Instance, @@ -115,18 +128,35 @@ def filter_queryset(self, queryset): Filters an XForm submission instances using ODK Aggregate query parameters. """ username = self.kwargs.get("username") - if username is None and self.request.user.is_anonymous: + form_pk = self.kwargs.get("xform_pk") + project_pk = self.kwargs.get("project_pk") + + if (not username or not form_pk or not project_pk) and self.request.user.is_anonymous: # raises a permission denied exception, forces authentication self.permission_denied(self.request) if username is not None and self.request.user.is_anonymous: - profile = get_object_or_404(UserProfile, user__username__iexact=username) + profile = None + if username: + profile = get_object_or_404(UserProfile, user__username__iexact=username) + elif form_pk: + queryset = queryset.filter(pk=form_pk) + if queryset.first(): + profile = queryset.first().user.profile + elif project_pk: + queryset = queryset.filter(project__pk=project_pk) + if queryset.first(): + profile = queryset.first().user.profile if profile.require_auth: # raises a permission denied exception, forces authentication self.permission_denied(self.request) else: queryset = queryset.filter(user=profile.user) + elif form_pk: + queryset = queryset.filter(pk=form_pk) + elif project_pk: + queryset = queryset.filter(project__pk=project_pk) else: queryset = super().filter_queryset(queryset) diff --git a/onadata/apps/main/urls.py b/onadata/apps/main/urls.py index 0c622491df..57ecdbfa9e 100644 --- a/onadata/apps/main/urls.py +++ b/onadata/apps/main/urls.py @@ -201,11 +201,31 @@ BriefcaseViewset.as_view({"get": "list", "head": "list"}), name="view-submission-list", ), + re_path( + r"^forms/(?P\w+)/view/submissionList$", + BriefcaseViewset.as_view({"get": "list", "head": "list"}), + name="view-submission-list", + ), + re_path( + r"^projects/(?P\d+)/view/submissionList$", + BriefcaseViewset.as_view({"get": "list", "head": "list"}), + name="view-submission-list", + ), re_path( r"^(?P\w+)/view/downloadSubmission$", BriefcaseViewset.as_view({"get": "retrieve", "head": "retrieve"}), name="view-download-submission", ), + re_path( + r"^forms/(?P\w+)/view/downloadSubmission$", + BriefcaseViewset.as_view({"get": "retrieve", "head": "retrieve"}), + name="view-download-submission", + ), + re_path( + r"^projects/(?P\d+)/view/downloadSubmission$", + BriefcaseViewset.as_view({"get": "retrieve", "head": "retrieve"}), + name="view-download-submission", + ), re_path( r"^(?P\w+)/formUpload$", BriefcaseViewset.as_view({"post": "create", "head": "create"}), From f98e91085e5483178252c3ac110ae2d9cd4b713b Mon Sep 17 00:00:00 2001 From: Kipchirchir Sigei Date: Wed, 19 Jul 2023 16:20:18 +0300 Subject: [PATCH 2/3] WIP tests Signed-off-by: Kipchirchir Sigei --- .../tests/viewsets/test_briefcase_viewset.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/onadata/apps/api/tests/viewsets/test_briefcase_viewset.py b/onadata/apps/api/tests/viewsets/test_briefcase_viewset.py index bfb984a611..979731e738 100644 --- a/onadata/apps/api/tests/viewsets/test_briefcase_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_briefcase_viewset.py @@ -130,6 +130,35 @@ def test_view_submission_list(self): '{{resumptionCursor}}', '%s' % last_index) self.assertContains(response, expected_submission_list) + def test_view_submission_list_w_xformid(self): + view = BriefcaseViewset.as_view({'get': 'list'}) + self._publish_xml_form() + self._make_submissions() + request = self.factory.get( + self._submission_list_url, + data={'formId': self.xform.id_string}) + response = view(request, xform_pk=self.xform.pk) + self.assertEqual(response.status_code, 401) + auth = DigestAuth(self.login_username, self.login_password) + request.META.update(auth(request.META, response)) + response = view(request, xform_pk=self.xform.pk) + self.assertEqual(response.status_code, 200) + import ipdb; ipdb.set_trace() + submission_list_path = os.path.join( + self.main_directory, 'fixtures', 'transportation', + 'view', 'submissionList.xml') + instances = ordered_instances(self.xform) + + self.assertEqual(instances.count(), NUM_INSTANCES) + + last_index = instances[instances.count() - 1].pk + with codecs.open(submission_list_path, 'rb', encoding='utf-8') as f: + expected_submission_list = f.read() + expected_submission_list = \ + expected_submission_list.replace( + '{{resumptionCursor}}', '%s' % last_index) + self.assertContains(response, expected_submission_list) + def test_view_submission_list_w_soft_deleted_submission(self): view = BriefcaseViewset.as_view({'get': 'list'}) self._publish_xml_form() From 404829b4db755c51cd433b4cb5cf17d0f52d3e35 Mon Sep 17 00:00:00 2001 From: Kipchirchir Sigei Date: Mon, 4 Sep 2023 11:53:50 +0300 Subject: [PATCH 3/3] Add more tests Signed-off-by: Kipchirchir Sigei --- .../tests/viewsets/test_briefcase_viewset.py | 107 +++++++++++++++++- .../apps/api/viewsets/briefcase_viewset.py | 13 ++- 2 files changed, 114 insertions(+), 6 deletions(-) diff --git a/onadata/apps/api/tests/viewsets/test_briefcase_viewset.py b/onadata/apps/api/tests/viewsets/test_briefcase_viewset.py index 979731e738..fb95b669f3 100644 --- a/onadata/apps/api/tests/viewsets/test_briefcase_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_briefcase_viewset.py @@ -134,6 +134,9 @@ def test_view_submission_list_w_xformid(self): view = BriefcaseViewset.as_view({'get': 'list'}) self._publish_xml_form() self._make_submissions() + self._submission_list_url = reverse( + 'view-submission-list', + kwargs={'xform_pk': self.xform.pk}) request = self.factory.get( self._submission_list_url, data={'formId': self.xform.id_string}) @@ -143,7 +146,37 @@ def test_view_submission_list_w_xformid(self): request.META.update(auth(request.META, response)) response = view(request, xform_pk=self.xform.pk) self.assertEqual(response.status_code, 200) - import ipdb; ipdb.set_trace() + submission_list_path = os.path.join( + self.main_directory, 'fixtures', 'transportation', + 'view', 'submissionList.xml') + instances = ordered_instances(self.xform) + + self.assertEqual(instances.count(), NUM_INSTANCES) + + last_index = instances[instances.count() - 1].pk + with codecs.open(submission_list_path, 'rb', encoding='utf-8') as f: + expected_submission_list = f.read() + expected_submission_list = \ + expected_submission_list.replace( + '{{resumptionCursor}}', '%s' % last_index) + self.assertContains(response, expected_submission_list) + + def test_view_submission_list_w_projectid(self): + view = BriefcaseViewset.as_view({'get': 'list'}) + self._publish_xml_form() + self._make_submissions() + self._submission_list_url = reverse( + 'view-submission-list', + kwargs={'project_pk': self.xform.project.pk}) + request = self.factory.get( + self._submission_list_url, + data={'formId': self.xform.id_string}) + response = view(request, project_pk=self.xform.project.pk) + self.assertEqual(response.status_code, 401) + auth = DigestAuth(self.login_username, self.login_password) + request.META.update(auth(request.META, response)) + response = view(request, project_pk=self.xform.project.pk) + self.assertEqual(response.status_code, 200) submission_list_path = os.path.join( self.main_directory, 'fixtures', 'transportation', 'view', 'submissionList.xml') @@ -336,6 +369,78 @@ def test_view_downloadSubmission(self): self.assertContains(response, instanceId, status_code=200) self.assertMultiLineEqual(response.content.decode('utf-8'), text) + def test_view_downloadSubmission_w_xformid(self): + view = BriefcaseViewset.as_view({'get': 'retrieve'}) + self._publish_xml_form() + self.maxDiff = None + self._submit_transport_instance_w_attachment() + instanceId = u'5b2cc313-fc09-437e-8149-fcd32f695d41' + instance = Instance.objects.get(uuid=instanceId) + formId = u'%(formId)s[@version=null and @uiVersion=null]/' \ + u'%(formId)s[@key=uuid:%(instanceId)s]' % { + 'formId': self.xform.id_string, + 'instanceId': instanceId} + params = {'formId': formId} + auth = DigestAuth(self.login_username, self.login_password) + self._download_submission_url = reverse( + 'view-download-submission', + kwargs={'xform_pk': self.xform.pk}) + request = self.factory.get( + self._download_submission_url, data=params) + response = view(request, xform_pk=self.xform.pk) + self.assertEqual(response.status_code, 401) + request.META.update(auth(request.META, response)) + response = view(request, xform_pk=self.xform.pk) + text = "uuid:%s" % instanceId + download_submission_path = os.path.join( + self.main_directory, 'fixtures', 'transportation', + 'view', 'downloadSubmission.xml') + with codecs.open(download_submission_path, encoding='utf-8') as f: + text = f.read() + for var in ((u'{{submissionDate}}', + instance.date_created.isoformat()), + (u'{{form_id}}', str(self.xform.id)), + (u'{{media_id}}', str(self.attachment.id))): + text = text.replace(*var) + self.assertContains(response, instanceId, status_code=200) + self.assertMultiLineEqual(response.content.decode('utf-8'), text) + + def test_view_downloadSubmission_w_projectid(self): + view = BriefcaseViewset.as_view({'get': 'retrieve'}) + self._publish_xml_form() + self.maxDiff = None + self._submit_transport_instance_w_attachment() + instanceId = u'5b2cc313-fc09-437e-8149-fcd32f695d41' + instance = Instance.objects.get(uuid=instanceId) + formId = u'%(formId)s[@version=null and @uiVersion=null]/' \ + u'%(formId)s[@key=uuid:%(instanceId)s]' % { + 'formId': self.xform.id_string, + 'instanceId': instanceId} + params = {'formId': formId} + auth = DigestAuth(self.login_username, self.login_password) + self._download_submission_url = reverse( + 'view-download-submission', + kwargs={'project_pk': self.xform.project.pk}) + request = self.factory.get( + self._download_submission_url, data=params) + response = view(request, project_pk=self.xform.project.pk) + self.assertEqual(response.status_code, 401) + request.META.update(auth(request.META, response)) + response = view(request, project_pk=self.xform.project.pk) + text = "uuid:%s" % instanceId + download_submission_path = os.path.join( + self.main_directory, 'fixtures', 'transportation', + 'view', 'downloadSubmission.xml') + with codecs.open(download_submission_path, encoding='utf-8') as f: + text = f.read() + for var in ((u'{{submissionDate}}', + instance.date_created.isoformat()), + (u'{{form_id}}', str(self.xform.id)), + (u'{{media_id}}', str(self.attachment.id))): + text = text.replace(*var) + self.assertContains(response, instanceId, status_code=200) + self.assertMultiLineEqual(response.content.decode('utf-8'), text) + def test_view_downloadSubmission_OtherUser(self): view = BriefcaseViewset.as_view({'get': 'retrieve'}) self._publish_xml_form() diff --git a/onadata/apps/api/viewsets/briefcase_viewset.py b/onadata/apps/api/viewsets/briefcase_viewset.py index 67b1efa65c..7e0648a7a5 100644 --- a/onadata/apps/api/viewsets/briefcase_viewset.py +++ b/onadata/apps/api/viewsets/briefcase_viewset.py @@ -107,11 +107,10 @@ def get_object(self, queryset=None): if queryset.first(): username = queryset.first().user.username elif project_pk: - queryset = queryset.filter(project__pk=project_pk) + queryset = self.queryset.filter(project__pk=project_pk) if queryset.first(): username = queryset.first().user.username - obj = get_object_or_404( Instance, xform__user__username__iexact=username, @@ -122,7 +121,7 @@ def get_object(self, queryset=None): return obj - # pylint: disable=too-many-branches + # pylint: disable=too-many-branches,too-many-statements def filter_queryset(self, queryset): """ Filters an XForm submission instances using ODK Aggregate query parameters. @@ -131,14 +130,18 @@ def filter_queryset(self, queryset): form_pk = self.kwargs.get("xform_pk") project_pk = self.kwargs.get("project_pk") - if (not username or not form_pk or not project_pk) and self.request.user.is_anonymous: + if ( + not username or not form_pk or not project_pk + ) and self.request.user.is_anonymous: # raises a permission denied exception, forces authentication self.permission_denied(self.request) if username is not None and self.request.user.is_anonymous: profile = None if username: - profile = get_object_or_404(UserProfile, user__username__iexact=username) + profile = get_object_or_404( + UserProfile, user__username__iexact=username + ) elif form_pk: queryset = queryset.filter(pk=form_pk) if queryset.first():