Skip to content

Commit

Permalink
Prevent instance name conflicts with search and pulldata
Browse files Browse the repository at this point in the history
  • Loading branch information
lognaturel committed Feb 8, 2024
1 parent 8f75605 commit b3e0972
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 15 deletions.
15 changes: 4 additions & 11 deletions pyxform/survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
PatchedText,
get_languages_with_bad_tags,
has_dynamic_label,
has_search_appearance_function,
node,
)
from pyxform.validators import enketo_validate, odk_validate
Expand All @@ -42,7 +43,8 @@
r"(instance\(.*\)\/root\/item\[.*?(\$\{.*\})\]\/.*?)\s"
)
RE_PULLDATA = re.compile(r"(pulldata\s*\(\s*)(.*?),")
SEARCH_APPEARANCE_REGEX = re.compile(r"search\(.*?\)")
RE_XML_OUTPUT = re.compile(r"\n.*(<output.*>)\n(\s\s)*")
RE_XML_TEXT = re.compile(r"(>)\n\s*(\s[^<>\s].*?)\n\s*(\s</)", re.DOTALL)


class InstanceInfo:
Expand Down Expand Up @@ -686,16 +688,7 @@ def _redirect_is_search_itext(element: SurveyElement) -> None:
:param element: A select type question.
:return: None, the question/children are modified in-place.
"""
try:
is_search = bool(
SEARCH_APPEARANCE_REGEX.search(
element[constants.CONTROL][constants.APPEARANCE]
)
)
except (KeyError, TypeError):
is_search = False
if is_search:
element[constants.ITEMSET] = ""
if has_search_appearance_function(element):
for i, opt in enumerate(element.get(constants.CHILDREN, [])):
opt["_choice_itext_id"] = f"{element['list_name']}-{i}"

Expand Down
18 changes: 16 additions & 2 deletions pyxform/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@
import re
from collections import namedtuple
from json.decoder import JSONDecodeError
from typing import Dict, List, Tuple
from typing import Any, Dict, List, Tuple
from xml.dom import Node
from xml.dom.minidom import Element, Text, _write_data, parseString

import openpyxl
import xlrd
from pyxform import constants

from pyxform.xls2json_backends import is_empty, xls_value_to_unicode, xlsx_value_to_str

Expand All @@ -29,7 +30,6 @@
PYXFORM_REFERENCE_REGEX = re.compile(r"\$\{(.*?)\}")
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",
Expand Down Expand Up @@ -327,6 +327,20 @@ def has_dynamic_label(choice_list: "List[Dict[str, str]]") -> bool:
return False


def has_search_appearance_function(question: Dict[str, Any]) -> bool:
"""
The search() appearance can be applied to selects to use a Collect-only database-backed select implementation.
"""
try:
return bool(
re.compile(r"search\(.*?\)").search(
question[constants.CONTROL][constants.APPEARANCE]
)
)
except (KeyError, TypeError):
return False


def levenshtein_distance(a: str, b: str) -> int:
"""
Calculate Levenshtein distance between two strings.
Expand Down
9 changes: 7 additions & 2 deletions pyxform/xls2json.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@
)
from pyxform.errors import PyXFormError
from pyxform.parsing.expression import is_single_token_expression
from pyxform.utils import PYXFORM_REFERENCE_REGEX, default_is_dynamic
from pyxform.utils import (
PYXFORM_REFERENCE_REGEX,
default_is_dynamic,
has_search_appearance_function,
)
from pyxform.validators.pyxform import parameters_generic, select_from_file
from pyxform.validators.pyxform.android_package_name import validate_android_package_name
from pyxform.validators.pyxform.translations_checks import SheetTranslations
Expand Down Expand Up @@ -362,7 +366,8 @@ def add_choices_info_to_question(
if file_extension is None:
file_extension = ""

question[constants.ITEMSET] = list_name
if not has_search_appearance_function(question):
question[constants.ITEMSET] = list_name

if choice_filter:
# External selects e.g. type = "select_one_external city".
Expand Down
17 changes: 17 additions & 0 deletions tests/test_external_instances.py
Original file line number Diff line number Diff line change
Expand Up @@ -610,3 +610,20 @@ def test_mixed_quotes_and_functions_in_pulldata(self):
'<instance id="instance4" src="jr://file-csv/instance4.csv"/>',
],
)

def test_search_and_pulldata(self):
"""Should not generate secondary instance for choice list used to configure search()"""
md = """
| survey | | | | | |
| | type | name | label | appearance | calculation |
| | select_one fruits | fruit | Question 1 | search('fruits') | |
| | calculate | calc | | | pulldata('fruits', 'this', 'that', ${fruit}) |
| choices | | | |
| | list_name | name | label |
| | fruits | na | la |
"""
self.assertPyxformXform(
md=md,
xml__xpath_match=["/h:html/h:body/x:select1/x:item[./x:value/text()='na']"],
xml__excludes=['<instance id="fruits">']
)

0 comments on commit b3e0972

Please sign in to comment.