From b9b03954ee919c6d6a2f1731af4834fb418fd239 Mon Sep 17 00:00:00 2001 From: Dickson Ukang'a Date: Thu, 16 Aug 2018 10:24:07 +0300 Subject: [PATCH] Raise ValidationError on PyXFormError exceptions This should allow the user to see what the problem with the merged form is. --- .../viewsets/test_merged_xform_viewset.py | 69 ++++++++++++++++--- .../serializers/merged_xform_serializer.py | 29 +++++--- 2 files changed, 77 insertions(+), 21 deletions(-) diff --git a/onadata/apps/api/tests/viewsets/test_merged_xform_viewset.py b/onadata/apps/api/tests/viewsets/test_merged_xform_viewset.py index 437c30bc55..66cda70eb6 100644 --- a/onadata/apps/api/tests/viewsets/test_merged_xform_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_merged_xform_viewset.py @@ -26,7 +26,6 @@ from onadata.libs.utils.export_tools import get_osm_data_kwargs from onadata.libs.utils.user_auth import get_user_default_project - MD = """ | survey | | | type | name | label | @@ -49,13 +48,26 @@ | | fruits | mango | Mango | """ +# https://github.com/onaio/onadata/issues/1153 +REFERENCE_ISSUE = """ +| survey | +| | type | name | label | +| | select one fruits | tunda | Tunda | +| | select one fruits | fruit | Fruit ${tunda} | + +| choices | +| | list name | name | label | +| | fruits | orange | Orange | +| | fruits | mango | Mango | +""" + def streaming_data(response): """ Iterates through a streaming response to return a json list object """ - return json.loads(u''.join([ - i.decode('utf-8') for i in response.streaming_content])) + return json.loads(u''.join( + [i.decode('utf-8') for i in response.streaming_content])) def _make_submissions_merged_datasets(merged_xform): @@ -372,8 +384,10 @@ def test_deleted_forms(self): merged_xform.xforms.all().delete() request = self.factory.get( '/', - data={'sort': '{"_submission_time":1}', - 'limit': '10'}, + data={ + 'sort': '{"_submission_time":1}', + 'limit': '10' + }, **self.extra) data_view = DataViewSet.as_view({ 'get': 'list', @@ -405,8 +419,8 @@ def test_md_csv_export(self): response = view(request, pk=merged_dataset['id'], format='csv') self.assertEqual(response.status_code, 200) - csv_file_obj = StringIO(''.join([ - c.decode('utf-8') for c in response.streaming_content])) + csv_file_obj = StringIO(''.join( + [c.decode('utf-8') for c in response.streaming_content])) csv_reader = csv.reader(csv_file_obj) # jump over headers first headers = next(csv_reader) @@ -574,9 +588,9 @@ def test_md_has_deleted_xforms(self): request = self.factory.post('/', data=data, **self.extra) response = view(request) self.assertEqual(response.status_code, 400) - self.assertEqual(response.data, { - 'xforms': [u'Invalid hyperlink - Object does not exist.'] - }) + self.assertEqual( + response.data, + {'xforms': [u'Invalid hyperlink - Object does not exist.']}) def test_md_has_no_matching_fields(self): """ @@ -661,3 +675,38 @@ def test_md_data_viewset_deleted_form(self): }) response = data_view(request, pk=merged_dataset['id'], dataid=dataid) self.assertEqual(response.status_code, 404) + + def test_xform_has_uncommon_reference(self): + """ + Test creating a merged dataset that has matching fields but with + uncommon reference variable. + """ + view = MergedXFormViewSet.as_view({ + 'post': 'create', + }) + # pylint: disable=attribute-defined-outside-init + self.project = get_user_default_project(self.user) + xform1 = self._publish_markdown(MD, self.user, id_string='a') + xform2 = self._publish_markdown( + REFERENCE_ISSUE, self.user, id_string='b') + + data = { + 'xforms': [ + "http://testserver/api/v1/forms/%s" % xform2.pk, + "http://testserver/api/v1/forms/%s" % xform1.pk, + ], + 'name': + 'Merged Dataset', + 'project': + "http://testserver/api/v1/projects/%s" % self.project.pk, + } + + request = self.factory.post('/', data=data, **self.extra) + response = view(request) + self.assertEqual(response.status_code, 400) + error_message = ( + "There has been a problem trying to replace ${tunda} with the " + "XPath to the survey element named 'tunda'. There is no survey " + "element with this name.") + self.assertIn('xforms', response.data) + self.assertIn(error_message, response.data['xforms']) diff --git a/onadata/libs/serializers/merged_xform_serializer.py b/onadata/libs/serializers/merged_xform_serializer.py index 8f3717375a..8b8946885f 100644 --- a/onadata/libs/serializers/merged_xform_serializer.py +++ b/onadata/libs/serializers/merged_xform_serializer.py @@ -11,10 +11,12 @@ from django.utils.translation import ugettext as _ from rest_framework import serializers +from pyxform.builder import create_survey_element_from_dict +from pyxform.errors import PyXFormError + from onadata.apps.logger.models import MergedXForm, XForm from onadata.apps.logger.models.xform import XFORM_TITLE_LENGTH from onadata.libs.utils.common_tags import MULTIPLE_SELECT_TYPE, SELECT_ONE -from pyxform.builder import create_survey_element_from_dict SELECTS = [SELECT_ONE, MULTIPLE_SELECT_TYPE] @@ -34,9 +36,9 @@ def _get_elements(elements, intersect, parent_prefix=None): if name in intersect: k = element.copy() if 'children' in element and element['type'] not in SELECTS: - k['children'] = _get_elements(element['children'], [ - __ for __ in intersect if __.startswith(name) - ], name) + k['children'] = _get_elements( + element['children'], + [__ for __ in intersect if __.startswith(name)], name) if not k['children']: continue new_elements.append(k) @@ -120,8 +122,7 @@ class XFormListField(serializers.ManyRelatedField): def to_representation(self, iterable): return [ - dict(i) - for i in XFormSerializer( + dict(i) for i in XFormSerializer( iterable, many=True, context=self.context).data ] @@ -136,8 +137,8 @@ class MergedXFormSerializer(serializers.HyperlinkedModelSerializer): allow_empty=False, child_relation=serializers.HyperlinkedRelatedField( allow_empty=False, - queryset=XForm.objects.filter(is_merged_dataset=False, - deleted_at__isnull=True), + queryset=XForm.objects.filter( + is_merged_dataset=False, deleted_at__isnull=True), view_name='xform-detail'), validators=[minimum_two_xforms, has_matching_fields]) num_of_submissions = serializers.SerializerMethodField() @@ -174,12 +175,18 @@ def create(self, validated_data): xforms = validated_data['xforms'] # create merged xml, json with non conflicting id_string survey = get_merged_xform_survey(xforms) - survey['id_string'] = base64.b64encode(uuid.uuid4().hex[:6].encode( - 'utf-8')).decode('utf-8') + survey['id_string'] = base64.b64encode( + uuid.uuid4().hex[:6].encode('utf-8')).decode('utf-8') survey['sms_keyword'] = survey['id_string'] survey['title'] = validated_data.pop('name') validated_data['json'] = survey.to_json() - validated_data['xml'] = survey.to_xml() + try: + validated_data['xml'] = survey.to_xml() + except PyXFormError as error: + raise serializers.ValidationError({ + 'xforms': + _("Problem Merging the Form: {}".format(error)) + }) validated_data['user'] = validated_data['project'].user validated_data['created_by'] = request.user validated_data['is_merged_dataset'] = True