From 1f9eef857ef9b22dd99e99b03dd67d8be7ec179f Mon Sep 17 00:00:00 2001
From: Oliver Roick <oliver.roick@gmail.com>
Date: Thu, 22 Dec 2016 12:42:03 +0100
Subject: [PATCH] Fix repeat groups and question order in XForms (#1014)

---
 cadasta/questionnaires/serializers.py         |  7 ++++++-
 .../questionnaires/tests/test_serializers.py  | 17 +++++++++++++++
 cadasta/xforms/renderers.py                   |  8 +++++--
 cadasta/xforms/tests/test_renderer.py         | 21 ++++++++++++++++---
 4 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/cadasta/questionnaires/serializers.py b/cadasta/questionnaires/serializers.py
index 4c626470a..8e29f628c 100644
--- a/cadasta/questionnaires/serializers.py
+++ b/cadasta/questionnaires/serializers.py
@@ -149,7 +149,7 @@ class QuestionnaireSerializer(serializers.ModelSerializer):
     )
     version = serializers.IntegerField(required=False, default=1)
     questions = serializers.SerializerMethodField()
-    question_groups = QuestionGroupSerializer(many=True, read_only=True)
+    question_groups = serializers.SerializerMethodField()
 
     class Meta:
         model = models.Questionnaire
@@ -211,3 +211,8 @@ def get_questions(self, instance):
         questions = instance.questions.filter(question_group__isnull=True)
         serializer = QuestionSerializer(questions, many=True)
         return serializer.data
+
+    def get_question_groups(self, instance):
+        groups = instance.question_groups.filter(question_group__isnull=True)
+        serializer = QuestionGroupSerializer(groups, many=True)
+        return serializer.data
diff --git a/cadasta/questionnaires/tests/test_serializers.py b/cadasta/questionnaires/tests/test_serializers.py
index 3860efa6a..bd0be0e87 100644
--- a/cadasta/questionnaires/tests/test_serializers.py
+++ b/cadasta/questionnaires/tests/test_serializers.py
@@ -113,6 +113,17 @@ def test_invalid_deserialize_json(self):
 
     def test_serialize(self):
         questionnaire = factories.QuestionnaireFactory()
+
+        factories.QuestionFactory.create(questionnaire=questionnaire,
+                                         question_group=None)
+        group = factories.QuestionGroupFactory.create(
+            questionnaire=questionnaire,
+            question_group=None)
+        factories.QuestionGroupFactory.create(questionnaire=questionnaire,
+                                              question_group=group)
+        factories.QuestionFactory.create(questionnaire=questionnaire,
+                                         question_group=group)
+
         serializer = serializers.QuestionnaireSerializer(questionnaire)
 
         assert serializer.data['id'] == questionnaire.id
@@ -123,6 +134,12 @@ def test_serialize(self):
         assert serializer.data['version'] == questionnaire.version
         assert 'project' not in serializer.data
 
+        assert len(serializer.data['questions']) == 1
+        assert len(serializer.data['question_groups']) == 1
+        assert len(serializer.data['question_groups'][0]['questions']) == 1
+        assert (
+            len(serializer.data['question_groups'][0]['question_groups']) == 1)
+
     def test_huge(self):
         data = {
             "filename": "wa6hrqr4e4vcf49q6kxjc443",
diff --git a/cadasta/xforms/renderers.py b/cadasta/xforms/renderers.py
index 95d6228c7..84198f521 100644
--- a/cadasta/xforms/renderers.py
+++ b/cadasta/xforms/renderers.py
@@ -96,11 +96,15 @@ def transform_questions(self, questions):
     def transform_groups(self, groups):
         transformed_groups = []
         for g in groups:
+            questions = self.transform_questions(g.get('questions', []))
+            groups = self.transform_groups(g.get('question_groups', []))
+            children = sorted(questions + groups,
+                              key=lambda x: x['index'])
             group = {
-                'type': 'group',
+                'type': g.get('type', 'group'),
                 'name': g.get('name'),
                 'label': g.get('label'),
-                'children': self.transform_questions(g.get('questions')),
+                'children': children,
                 'index': g.get('index')
             }
             if group['label'] is None:
diff --git a/cadasta/xforms/tests/test_renderer.py b/cadasta/xforms/tests/test_renderer.py
index 916211a7d..7ae4befbb 100644
--- a/cadasta/xforms/tests/test_renderer.py
+++ b/cadasta/xforms/tests/test_renderer.py
@@ -50,35 +50,50 @@ def test_transform_groups(self):
         groups = [{
             'id': '123',
             'name': 'group_1',
-            'label': 'Group 2',
+            'label': 'Group 1',
+            'type': 'group',
             'relevant': "${party_type}='IN'",
+            'index': 0,
             'questions': [{
                 'id': "bzs2984c3gxgwcjhvambdt3w",
                 'name': "start",
                 'label': None,
                 'type': "ST",
+                'index': 0
             }]
         }, {
             'id': '456',
             'name': 'group_2',
             'label': None,
+            'type': 'repeat',
+            'index': 1,
             'questions': [{
                 'id': "xp8vjr6dsk46p47u22fft7bg",
                 'name': "tenure_type",
                 'label': "What is the social tenure type?",
                 'type': "TX",
+                'index': 0
+            }],
+            'question_groups': [{
+                'id': "xp8vjr6dsk46p47u22fft7aa",
+                'name': 'group_3',
+                'label': 'Group 3',
+                'type': 'group',
+                'index': 1
             }]
         }]
         renderer = XFormRenderer()
         transformed = renderer.transform_groups(groups)
         assert len(transformed) == 2
         for g in transformed:
-            assert g['type'] == 'group'
-            assert len(g['children']) == 1
             if g['name'] == 'group_1':
                 assert g['bind']['relevant'] == "${party_type}='IN'"
+                assert g['type'] == 'group'
+                assert len(g['children']) == 1
             if g['name'] == 'group_2':
                 assert 'label' not in g
+                assert g['type'] == 'repeat'
+                assert len(g['children']) == 2
 
     def test_transform_to_xform_json(self):
         data = {