Merge pull request #787 from lindsay-stevens/pyxform-754

754: fix 'NoneType is not iterable' error for group with no children

Author: lognaturel

pyxform/section.py

@@ -56,7 +56,7 @@ def __init__(
     ):
         # Structure
         self.bind: dict | None = bind
-        self.children: list[Section | Question] | None = None
+        self.children: list[Section | Question] = []
         self.control: dict | None = control
         # instance is for custom instance attrs from survey e.g. instance::abc:xyz
         self.instance: dict | None = instance
@@ -73,6 +73,23 @@ def __init__(
         kwargs.pop(constants.CHILDREN, None)
         super().__init__(name=name, label=label, fields=fields, **kwargs)
 
+        self._link_children()
+
+    def _link_children(self):
+        for child in self.children:
+            child.parent = self
+
+    def add_child(self, child):
+        self.children.append(child)
+        child.parent = self
+
+    def add_children(self, children):
+        if isinstance(children, list | tuple):
+            for child in children:
+                self.add_child(child)
+        else:
+            self.add_child(children)
+
     def validate(self):
         super().validate()
         for element in self.children:

pyxform/survey_element.py

@@ -121,9 +121,6 @@ def __init__(
         if len(kwargs) > 0:
             self.extra_data = kwargs
 
-        if hasattr(self, const.CHILDREN):
-            self._link_children()
-
         # Create a space label for unlabeled elements with the label
         # appearance tag. # This is because such elements are used to label the
         # options for selects in a field-list and might want blank labels for
@@ -141,24 +138,6 @@ def __init__(
     def name_for_xpath(self) -> str:
         return self.name
 
-    def _link_children(self):
-        if self.children is not None:
-            for child in self.children:
-                child.parent = self
-
-    def add_child(self, child):
-        if self.children is None:
-            self.children = []
-        self.children.append(child)
-        child.parent = self
-
-    def add_children(self, children):
-        if isinstance(children, list | tuple):
-            for child in children:
-                self.add_child(child)
-        else:
-            self.add_child(children)
-
     def validate(self):
         if not is_xml_tag(self.name):
             invalid_char = re.search(INVALID_XFORM_TAG_REGEXP, self.name)

tests/test_fieldlist_labels.py

@@ -79,6 +79,40 @@ def test_group_relevant_included_in_bind(self):
             ],
         )
 
+    def test_table_list_appearance(self):
+        md = """
+        | survey  |
+        | | type              | name       | label     | hint       | appearance         |
+        | | begin_group       | tablelist1 | Table_Y_N |            | table-list minimal |
+        | | select_one yes_no | options1a  | Q1        | first row! |                    |
+        | | select_one yes_no | options1b  | Q2        |            |                    |
+        | | end_group         |            |           |            |                    |
+        | choices |
+        | | list_name | name | label |
+        | | yes_no    | yes  | Yes   |
+        """
+        xml_contains = """
+    <group appearance="field-list minimal" ref="/table-list-appearance-mod/tablelist1">
+      <input ref="/table-list-appearance-mod/tablelist1/generated_table_list_label_2">
+        <label>Table_Y_N</label>
+      </input>
+      <select1 appearance="label" ref="/table-list-appearance-mod/tablelist1/reserved_name_for_field_list_labels_3">
+        <label> </label>
+        <itemset nodeset="instance('yes_no')/root/item">
+          <value ref="name"/>
+          <label ref="label"/>
+        </itemset>
+      </select1>
+      <select1 appearance="list-nolabel" ref="/table-list-appearance-mod/tablelist1/options1a">
+        <label>Q1</label>
+        <hint>first row!</hint>
+""".strip()
+        self.assertPyxformXform(
+            name="table-list-appearance-mod",
+            md=md,
+            xml__contains=[xml_contains],
+        )
+
 
 class TestGroupParsing(PyxformTestCase):
     def test_names__group_basic_case__ok(self):
@@ -576,6 +610,96 @@ def test_group__no_begin_error__with_another_closed_repeat(self):
             error__contains=[SURVEY_001.format(row=4, type="group")],
         )
 
+    def test_empty_group__no_question__error(self):
+        """Should raise an error for an empty group with no questions."""
+        md = """
+        | survey |
+        | | type        | name | label |
+        | | begin group | g1   | G1    |
+        | | end group   |      |       |
+        """
+        self.assertPyxformXform(
+            md=md,
+            run_odk_validate=True,  # Error about empty groups is from Validate only.
+            odk_validate_error__contains=[
+                "Group has no children! Group: ${g1}. The XML is invalid."
+            ],
+        )
+
+    def test_empty_group__no_question_control__error(self):
+        """Should raise an error for an empty group with no question controls."""
+        md = """
+        | survey |
+        | | type        | name | label | calculation |
+        | | begin group | g1   | G1    |             |
+        | | text        | q1   |       | 0 + 0       |
+        | | end group   |      |       |             |
+        """
+        self.assertPyxformXform(
+            md=md,
+            run_odk_validate=True,  # Error about empty groups is from Validate only.
+            odk_validate_error__contains=[
+                "Group has no children! Group: ${g1}. The XML is invalid."
+            ],
+        )
+
+    def test_unlabeled_group(self):
+        self.assertPyxformXform(
+            md="""
+            | survey |
+            | | type        | name     | label   |
+            | | begin_group | my-group |         |
+            | | text        | my-text  | my-text |
+            | | end_group   |          |         |
+            """,
+            warnings_count=1,
+            warnings__contains=["[row : 2] Group has no label"],
+        )
+
+    def test_unlabeled_group_alternate_syntax(self):
+        self.assertPyxformXform(
+            md="""
+            | survey |
+            | | type        | name     | label::English (en) |
+            | | begin group | my-group |                     |
+            | | text        | my-text  | my-text             |
+            | | end group   |          |                     |
+            """,
+            warnings_count=1,
+            warnings__contains=["[row : 2] Group has no label"],
+        )
+
+    def test_unlabeled_group_fieldlist(self):
+        self.assertPyxformXform(
+            md="""
+            | survey |
+            | | type         | name      | label   | appearance |
+            | | begin_group  | my-group  |         | field-list |
+            | | text         | my-text   | my-text |            |
+            | | end_group    |           |         |            |
+            """,
+            warnings_count=0,
+            xml__xpath_match=[
+                """
+                /h:html/h:body/x:group[
+                  @ref = '/test_name/my-group' and @appearance='field-list'
+                ]
+                """
+            ],
+        )
+
+    def test_unlabeled_group_fieldlist_alternate_syntax(self):
+        self.assertPyxformXform(
+            md="""
+            | survey |
+            | | type        | name     | label::English (en) | appearance |
+            | | begin group | my-group |                     | field-list |
+            | | text        | my-text  | my-text             |            |
+            | | end group   |          |                     |            |
+            """,
+            warnings_count=0,
+        )
+
 
 class TestGroupInternalRepresentations(TestCase):
     maxDiff = None