From c7448455c1d442175498beaef67c0a7dbe7d65e1 Mon Sep 17 00:00:00 2001 From: Lincoln Simba Date: Mon, 21 Jan 2019 11:02:47 +0300 Subject: [PATCH 1/4] Optional fields in formList such as manifestUrl should be empty incase none exists. fixes #1519 Signed-off-by: Lincoln Simba --- onadata/libs/renderers/renderers.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/onadata/libs/renderers/renderers.py b/onadata/libs/renderers/renderers.py index 45994cbff9..f1d2082981 100644 --- a/onadata/libs/renderers/renderers.py +++ b/onadata/libs/renderers/renderers.py @@ -7,13 +7,11 @@ import math from io import BytesIO -from future.utils import iteritems - +import pytz from django.utils.dateparse import parse_datetime from django.utils.encoding import smart_text from django.utils.xmlutils import SimplerXMLGenerator - -import pytz +from future.utils import iteritems from rest_framework import negotiation from rest_framework.compat import six from rest_framework.renderers import (BaseRenderer, JSONRenderer, @@ -31,6 +29,14 @@ 'meta/sessionID', ] +FORMLIST_MANDATORY_FIELDS = [ + 'formID', + 'name', + 'version', + 'hash', + 'downloadUrl' +] + def pairing(val1, val2): """ @@ -244,9 +250,12 @@ def _to_xml(self, xml, data): elif isinstance(data, dict): for (key, value) in iteritems(data): - xml.startElement(key, {}) - self._to_xml(xml, value) - xml.endElement(key) + if key not in FORMLIST_MANDATORY_FIELDS and value is None: + pass + else: + xml.startElement(key, {}) + self._to_xml(xml, value) + xml.endElement(key) elif data is None: # Don't output any value From 0405bc0767c4046204913d7b133840ad33b3a0c3 Mon Sep 17 00:00:00 2001 From: Lincoln Simba Date: Mon, 21 Jan 2019 11:03:28 +0300 Subject: [PATCH 2/4] tests Signed-off-by: Lincoln Simba --- .../apps/api/tests/viewsets/test_xform_list_viewset.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/onadata/apps/api/tests/viewsets/test_xform_list_viewset.py b/onadata/apps/api/tests/viewsets/test_xform_list_viewset.py index 25334cd39d..672ed69c91 100644 --- a/onadata/apps/api/tests/viewsets/test_xform_list_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_xform_list_viewset.py @@ -1,7 +1,7 @@ import os -from builtins import open from hashlib import md5 +from builtins import open from django.conf import settings from django.core.urlresolvers import reverse from django.test import TransactionTestCase @@ -894,9 +894,9 @@ def test_get_xform_anonymous_user_xform_require_auth(self): # success with authentication self.assertEqual(response.status_code, 200) - def test_manifest_url_tag_is_empty_when_no_media(self): + def test_manifest_url_tag_is_not_present_when_no_media(self): """ - Test that content contains an empty tag for manifest url + Test that content does not contain a manifest url only when the form has no media """ request = self.factory.get('/') @@ -905,7 +905,7 @@ def test_manifest_url_tag_is_empty_when_no_media(self): self.assertEqual(response.status_code, 200) content = response.render().content.decode('utf-8') manifest_url = ('') - self.assertTrue(manifest_url in content) + self.assertNotIn(manifest_url, content) # Add media and test that manifest url exists data_type = 'media' From eb9496d7658402c24866b2c8a5587d705f38770f Mon Sep 17 00:00:00 2001 From: Lincoln Simba Date: Mon, 21 Jan 2019 11:46:52 +0300 Subject: [PATCH 3/4] rid formList.xml of optional fields (manifestUrl) Update tests Signed-off-by: Lincoln Simba --- onadata/apps/api/tests/fixtures/formList.xml | 2 +- onadata/apps/main/tests/test_process.py | 17 ++++++++--------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/onadata/apps/api/tests/fixtures/formList.xml b/onadata/apps/api/tests/fixtures/formList.xml index 1987c4ea7b..2953022fad 100644 --- a/onadata/apps/api/tests/fixtures/formList.xml +++ b/onadata/apps/api/tests/fixtures/formList.xml @@ -1,2 +1,2 @@ -transportation_2011_07_25transportation_2011_07_252014111%(hash)shttp://testserver/bob/forms/%(pk)s/form.xml +transportation_2011_07_25transportation_2011_07_252014111%(hash)shttp://testserver/bob/forms/%(pk)s/form.xml diff --git a/onadata/apps/main/tests/test_process.py b/onadata/apps/main/tests/test_process.py index 41758385ce..d4f5d30752 100644 --- a/onadata/apps/main/tests/test_process.py +++ b/onadata/apps/main/tests/test_process.py @@ -3,19 +3,19 @@ import json import os import re -import pytz -from builtins import open from datetime import datetime -from future.utils import iteritems from hashlib import md5 -from mock import patch +from xml.dom import minidom, Node -from django.core.urlresolvers import reverse +import pytz +from builtins import open from django.conf import settings -from django_digest.test import Client as DigestClient from django.core.files.uploadedfile import UploadedFile +from django.core.urlresolvers import reverse +from django_digest.test import Client as DigestClient +from future.utils import iteritems +from mock import patch from xlrd import open_workbook -from xml.dom import minidom, Node from onadata.apps.logger.models import XForm from onadata.apps.logger.models.xform import XFORM_TITLE_LENGTH @@ -26,7 +26,6 @@ from onadata.libs.utils.common_tags import MONGO_STRFTIME from onadata.libs.utils.common_tools import get_response_content - uuid_regex = re.compile( r'(.*uuid[^//]+="\')([^\']+)(\'".*)', re.DOTALL) @@ -217,7 +216,7 @@ def _check_formlist(self): % (self.user.username, self.xform.pk) md5_hash = md5(self.xform.xml.encode('utf-8')).hexdigest() expected_content = """ -transportation_2011_07_25transportation_2011_07_252014111md5:%(hash)s%(download_url)s""" # noqa +transportation_2011_07_25transportation_2011_07_252014111md5:%(hash)s%(download_url)s""" # noqa expected_content = expected_content % { 'download_url': self.download_url, 'hash': md5_hash From a01625996c106410c8011b7cfca6308db36167c5 Mon Sep 17 00:00:00 2001 From: Lincoln Simba Date: Tue, 22 Jan 2019 09:51:34 +0300 Subject: [PATCH 4/4] improve code Signed-off-by: Lincoln Simba --- onadata/libs/renderers/renderers.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/onadata/libs/renderers/renderers.py b/onadata/libs/renderers/renderers.py index f1d2082981..440cd84ded 100644 --- a/onadata/libs/renderers/renderers.py +++ b/onadata/libs/renderers/renderers.py @@ -251,11 +251,10 @@ def _to_xml(self, xml, data): elif isinstance(data, dict): for (key, value) in iteritems(data): if key not in FORMLIST_MANDATORY_FIELDS and value is None: - pass - else: - xml.startElement(key, {}) - self._to_xml(xml, value) - xml.endElement(key) + continue + xml.startElement(key, {}) + self._to_xml(xml, value) + xml.endElement(key) elif data is None: # Don't output any value