694: search and pulldata secondary instance conflict

pyxform/aliases.py

@@ -73,7 +73,7 @@
     "SMS Response": constants.SMS_RESPONSE,
     "compact_tag": "instance::odk:tag",  # used for compact representation
     "Type": "type",
-    "List_name": "list_name",
+    "List_name": constants.LIST_NAME_U,
     # u"repeat_count": u"jr:count",  duplicate key
     "read_only": "bind::readonly",
     "readonly": "bind::readonly",
@@ -111,7 +111,7 @@
     constants.ENTITIES_SAVETO: "bind::entities:saveto",
 }
 
-entities_header = {"list_name": "dataset"}
+entities_header = {constants.LIST_NAME_U: "dataset"}
 
 # Key is the pyxform internal name, Value is the name used in error/warning messages.
 TRANSLATABLE_SURVEY_COLUMNS = {
@@ -135,7 +135,7 @@
 }
 list_header = {
     "caption": constants.LABEL,
-    "list_name": constants.LIST_NAME,
+    constants.LIST_NAME_U: constants.LIST_NAME_S,
     "value": constants.NAME,
     "image": "media::image",
     "big-image": "media::big-image",

pyxform/constants.py

@@ -67,7 +67,8 @@
 CHOICES = "choices"
 
 # XLS Specific constants
-LIST_NAME = "list name"
+LIST_NAME_S = "list name"
+LIST_NAME_U = "list_name"
 CASCADING_SELECT = "cascading_select"
 TABLE_LIST = "table-list"  # hyphenated because it goes in appearance, and convention for appearance column is dashes
 FIELD_LIST = "field-list"
@@ -155,3 +156,12 @@ class EntityColumns(StrEnum):
     "constraint",
     "calculate",
 )
+NSMAP = {
+    "xmlns": "http://www.w3.org/2002/xforms",
+    "xmlns:h": "http://www.w3.org/1999/xhtml",
+    "xmlns:ev": "http://www.w3.org/2001/xml-events",
+    "xmlns:xsd": "http://www.w3.org/2001/XMLSchema",
+    "xmlns:jr": "http://openrosa.org/javarosa",
+    "xmlns:orx": "http://openrosa.org/xforms",
+    "xmlns:odk": "http://www.opendatakit.org/xforms",
+}

pyxform/survey.py

@@ -11,7 +11,7 @@
 from typing import Generator, Iterator, List, Optional, Tuple
 
 from pyxform import aliases, constants
-from pyxform.constants import EXTERNAL_INSTANCE_EXTENSIONS
+from pyxform.constants import EXTERNAL_INSTANCE_EXTENSIONS, NSMAP
 from pyxform.errors import PyXFormError, ValidationError
 from pyxform.external_instance import ExternalInstance
 from pyxform.instance import SurveyInstance
@@ -23,7 +23,6 @@
     BRACKETED_TAG_REGEX,
     LAST_SAVED_INSTANCE_NAME,
     LAST_SAVED_REGEX,
-    NSMAP,
     DetachableElement,
     PatchedText,
     get_languages_with_bad_tags,
@@ -216,7 +215,7 @@ class Survey(Section):
             "namespaces": str,
             constants.ENTITY_FEATURES: list,
         }
-    )  # yapf: disable
+    )
 
     def validate(self):
         if self.id_string in [None, "None"]:
@@ -385,7 +384,7 @@ def _validate_external_instances(instances) -> None:
         errors = []
         for element, copies in seen.items():
             if len(copies) > 1:
-                contexts = ", ".join(x.context for x in copies)
+                contexts = ", ".join(f"{x.context}({x.type})" for x in copies)
                 errors.append(
                     "Instance names must be unique within a form. "
                     f"The name '{element}' was found {len(copies)} time(s), "
@@ -655,8 +654,7 @@ def _add_to_nested_dict(self, dicty, path, value):
             dicty[path[0]] = {}
         self._add_to_nested_dict(dicty[path[0]], path[1:], value)
 
-    @staticmethod
-    def _redirect_is_search_itext(element: SurveyElement) -> None:
+    def _redirect_is_search_itext(self, element: SurveyElement) -> bool:
         """
         For selects using the "search()" appearance, redirect itext for in-line items.
 
@@ -669,7 +667,7 @@ def _redirect_is_search_itext(element: SurveyElement) -> None:
         accounts for questions with and without a "search()" appearance sharing choices.
 
         :param element: A select type question.
-        :return: None, the question/children are modified in-place.
+        :return: If True, the element has a search appearance.
         """
         try:
             is_search = bool(
@@ -680,9 +678,20 @@ def _redirect_is_search_itext(element: SurveyElement) -> None:
         except (KeyError, TypeError):
             is_search = False
         if is_search:
+            file_id, ext = os.path.splitext(element[constants.ITEMSET])
+            if ext in EXTERNAL_INSTANCE_EXTENSIONS:
+                msg = (
+                    f"Question '{element[constants.NAME]}' is a select from file type, "
+                    "with a 'search' appearance. This combination is not supported. "
+                    "Remove the 'search' appearance, or change the select type."
+                )
+                raise PyXFormError(msg)
+            itemset = element[constants.ITEMSET]
+            self.choices.pop(itemset, None)
             element[constants.ITEMSET] = ""
             for i, opt in enumerate(element.get(constants.CHILDREN, [])):
-                opt["_choice_itext_id"] = f"{element['list_name']}-{i}"
+                opt["_choice_itext_id"] = f"{element[constants.LIST_NAME_U]}-{i}"
+        return is_search
 
     def _setup_translations(self):
         """
@@ -745,6 +754,8 @@ def get_choices():
             self._add_to_nested_dict(self._translations, path, leaf_value)
 
         select_types = set(aliases.select.keys())
+        search_lists = set()
+        non_search_lists = set()
         for element in self.iter_descendants():
             itemset = element.get("itemset")
             if itemset is not None:
@@ -753,7 +764,12 @@ def get_choices():
                 element._itemset_dyn_label = itemset in itemsets_has_dyn_label
 
             if element[constants.TYPE] in select_types:
-                self._redirect_is_search_itext(element=element)
+                select_ref = (element[constants.NAME], element[constants.LIST_NAME_U])
+                if self._redirect_is_search_itext(element=element):
+                    search_lists.add(select_ref)
+                else:
+                    non_search_lists.add(select_ref)
+
             # Skip creation of translations for choices in selects. The creation of these
             # translations is done above in this function.
             parent = element.get("parent")
@@ -780,6 +796,21 @@ def get_choices():
                         }
                     )
 
+        for q_name, list_name in search_lists:
+            choice_refs = [f"'{q}'" for q, c in non_search_lists if c == list_name]
+            if len(choice_refs) > 0:
+                choice_refs_str = ", ".join(choice_refs)
+                msg = (
+                    f"Question '{q_name}' uses the 'search' appearance, and its select "
+                    f"type references the choice list name '{list_name}'. This choice "
+                    "list name is referenced by at least one other non-'search' "
+                    f"question, which will not work: {choice_refs_str}. "
+                    "Either 1) use the 'search' appearance for all questions using "
+                    f"this choice list name, or 2) use a different choice list name "
+                    "for the 'search' question."
+                )
+                raise PyXFormError(msg)
+
     def _add_empty_translations(self):
         """
         Adds translations so that every itext element has the same elements across every

pyxform/survey_element.py

@@ -51,7 +51,7 @@
     "autoplay": str,
     "flat": lambda: False,
     "action": str,
-    "list_name": str,
+    const.LIST_NAME_U: str,
     "trigger": str,
 }
 

pyxform/utils.py

@@ -29,17 +29,6 @@
 NODE_TYPE_TEXT = (Node.TEXT_NODE, Node.CDATA_SECTION_NODE)
 
 
-NSMAP = {
-    "xmlns": "http://www.w3.org/2002/xforms",
-    "xmlns:h": "http://www.w3.org/1999/xhtml",
-    "xmlns:ev": "http://www.w3.org/2001/xml-events",
-    "xmlns:xsd": "http://www.w3.org/2001/XMLSchema",
-    "xmlns:jr": "http://openrosa.org/javarosa",
-    "xmlns:orx": "http://openrosa.org/xforms",
-    "xmlns:odk": "http://www.opendatakit.org/xforms",
-}
-
-
 class DetachableElement(Element):
     """
     Element classes are not meant to be instantiated directly.   This

pyxform/xform2json.py

@@ -13,8 +13,8 @@
 from defusedxml.ElementTree import ParseError, XMLParser, fromstring, parse
 
 from pyxform import builder
+from pyxform.constants import NSMAP
 from pyxform.errors import PyXFormError
-from pyxform.utils import NSMAP
 
 logger = logging.getLogger(__name__)
 logger.addHandler(logging.NullHandler())

pyxform/xls2json.py

@@ -370,7 +370,7 @@ def add_choices_info_to_question(
             question["query"] = list_name
         elif choices.get(list_name):
             # Reference to list name for data dictionary tools (ilri/odktools).
-            question["list_name"] = list_name
+            question[constants.LIST_NAME_U] = list_name
             # Copy choices for data export tools (onaio/onadata).
             # TODO: could onadata use the list_name to look up the list for
             #  export, instead of pyxform internally duplicating choices data?
@@ -386,7 +386,7 @@ def add_choices_info_to_question(
         # Select from previous answers e.g. type = "select_one ${q1}".
         or bool(PYXFORM_REFERENCE_REGEX.search(list_name))
     ):
-        question["list_name"] = list_name
+        question[constants.LIST_NAME_U] = list_name
         question[constants.CHOICES] = choices[list_name]
 
 
@@ -529,7 +529,7 @@ def workbook_to_json(
         default_language=default_language,
     )
     external_choices = group_dictionaries_by_key(
-        list_of_dicts=external_choices_sheet.data, key=constants.LIST_NAME
+        list_of_dicts=external_choices_sheet.data, key=constants.LIST_NAME_S
     )
 
     # ########## Choices sheet ##########
@@ -543,7 +543,7 @@ def workbook_to_json(
         default_language=default_language,
     )
     combined_lists = group_dictionaries_by_key(
-        list_of_dicts=choices_sheet.data, key=constants.LIST_NAME
+        list_of_dicts=choices_sheet.data, key=constants.LIST_NAME_S
     )
     # To combine the warning into one message, the check for missing choices translation
     # columns is run with Survey sheet below.
@@ -654,7 +654,7 @@ def workbook_to_json(
         use_double_colons=True,
     )
     osm_tags = group_dictionaries_by_key(
-        list_of_dicts=osm_sheet.data, key=constants.LIST_NAME
+        list_of_dicts=osm_sheet.data, key=constants.LIST_NAME_S
     )
     # #################################
 
@@ -1025,13 +1025,13 @@ def workbook_to_json(
                 child_list = []
                 new_json_dict[constants.CHILDREN] = child_list
                 if control_type is constants.LOOP:
-                    if not parse_dict.get("list_name"):
+                    if not parse_dict.get(constants.LIST_NAME_U):
                         # TODO: Perhaps warn and make repeat into a group?
                         raise PyXFormError(
                             ROW_FORMAT_STRING % row_number
                             + " Repeat loop without list name."
                         )
-                    list_name = parse_dict["list_name"]
+                    list_name = parse_dict[constants.LIST_NAME_U]
                     if list_name not in choices:
                         raise PyXFormError(
                             ROW_FORMAT_STRING % row_number
@@ -1127,7 +1127,7 @@ def workbook_to_json(
                         + " select one external is only meant for"
                         " filtered selects."
                     )
-                list_name = parse_dict["list_name"]
+                list_name = parse_dict[constants.LIST_NAME_U]
                 file_extension = os.path.splitext(list_name)[1]
                 if (
                     select_type == constants.SELECT_ONE_EXTERNAL
@@ -1323,8 +1323,8 @@ def workbook_to_json(
             new_dict = row.copy()
             new_dict["type"] = constants.OSM
 
-            if parse_dict.get("list_name") is not None:
-                tags = osm_tags.get(parse_dict.get("list_name"))
+            if parse_dict.get(constants.LIST_NAME_U) is not None:
+                tags = osm_tags.get(parse_dict.get(constants.LIST_NAME_U))
                 for tag in tags:
                     if osm_tags.get(tag.get("name")):
                         tag["choices"] = osm_tags.get(tag.get("name"))

tests/pyxform_test_case.py

@@ -14,8 +14,9 @@
 # noinspection PyProtectedMember
 from lxml.etree import _Element
 from pyxform.builder import create_survey_element_from_dict
+from pyxform.constants import NSMAP
 from pyxform.errors import PyXFormError
-from pyxform.utils import NSMAP, coalesce
+from pyxform.utils import coalesce
 from pyxform.validators.odk_validate import ODKValidateError, check_xform
 from pyxform.xls2json import workbook_to_json
 
@@ -113,13 +114,17 @@ def _ss_structure_to_pyxform_survey(
     ):
         # using existing methods from the builder
         imported_survey_json = workbook_to_json(
-            workbook_dict=ss_structure, warnings=warnings
+            workbook_dict=ss_structure, form_name=name, warnings=warnings
         )
         # ideally, when all these tests are working, this would be refactored as well
         survey = create_survey_element_from_dict(imported_survey_json)
-        survey.name = coalesce(name, "data")
-        survey.title = title
-        survey.id_string = id_string
+        # Due to str(name) in workbook_to_json
+        if survey.name is None or survey.name == "None":
+            survey.name = coalesce(name, "data")
+        if survey.title is None:
+            survey.title = title
+        if survey.id_string is None:
+            survey.id_string = id_string
 
         return survey
 

tests/test_builder.py

@@ -545,7 +545,7 @@ def test_style_column(self):
     STRIP_NS_FROM_TAG_RE = re.compile(r"\{.+\}")
 
     def test_style_not_added_to_body_if_not_present(self):
-        survey = utils.create_survey_from_fixture("settings", filetype=FIXTURE_FILETYPE)
+        survey = utils.create_survey_from_fixture("widgets", filetype=FIXTURE_FILETYPE)
         xml = survey.to_xml()
         # find the body tag
         root_elm = ETree.fromstring(xml.encode("utf-8"))

tests/test_form_name.py

@@ -4,7 +4,7 @@
 from tests.pyxform_test_case import PyxformTestCase
 
 
-class FormNameTest(PyxformTestCase):
+class TestFormName(PyxformTestCase):
     def test_default_to_data_when_no_name(self):
         """
         Test no form_name will default to survey name to 'data'.
@@ -14,9 +14,6 @@ def test_default_to_data_when_no_name(self):
             | survey   |           |      |           |
             |          | type      | name | label     |
             |          | text      | city | City Name |
-            | settings |           |      |           |
-            |          | id_string | name |
-            |          | some-id   | data |
             """,
             autoname=False,
         )