Merge pull request #1450 from onaio/raise-validation-error-on-pyxform…

…-exceptions

Raise ValidationError on PyXFormError exceptions

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'])

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