Skip to content

Commit

Permalink
fix: translations not generated for choice list used with search() (X…
Browse files Browse the repository at this point in the history
…LSForm#690)

- existing test case search_and_select didn't cover the multi-language
  case, which requires that itext is emitted for the inline selects.
- approach and details described in Survey._redirect_is_search_itext.
  • Loading branch information
lindsay-stevens authored Jan 26, 2024
1 parent c870049 commit 847d9c8
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 15 deletions.
6 changes: 6 additions & 0 deletions pyxform/question.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,12 @@ def validate(self):
def xml_control(self):
raise NotImplementedError()

def _translation_path(self, display_element):
choice_itext_id = self.get("_choice_itext_id")
if choice_itext_id is not None:
return choice_itext_id
return super()._translation_path(display_element=display_element)


class MultipleChoiceQuestion(Question):
def __init__(self, **kwargs):
Expand Down
34 changes: 33 additions & 1 deletion pyxform/survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
RE_PULLDATA = re.compile(r"(pulldata\s*\(\s*)(.*?),")
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)
SEARCH_APPEARANCE_REGEX = re.compile(r"search\(.*?\)")


class InstanceInfo:
Expand Down Expand Up @@ -671,6 +672,35 @@ 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:
"""
For selects using the "search()" appearance, redirect itext for in-line items.
External selects from a "search" appearance alone don't work in Enketo. In Collect
they must have the "item" elements in the body, rather than in an "itemset".
The "itemset" reference is cleared below, so that the element will get in-line
items instead of an itemset reference to a secondary instance. The itext ref is
passed to the options/choices so they can use the generated translations. This
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.
"""
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] = ""
for i, opt in enumerate(element.get(constants.CHILDREN, [])):
opt["_choice_itext_id"] = f"{element['list_name']}-{i}"

def _setup_translations(self):
"""
set up the self._translations dict which will be referenced in the
Expand Down Expand Up @@ -740,8 +770,10 @@ def get_choices():
element._itemset_has_media = itemset in itemsets_has_media
element._itemset_dyn_label = itemset in itemsets_has_dyn_label

if element[constants.TYPE] in select_types:
self._redirect_is_search_itext(element=element)
# Skip creation of translations for choices in selects. The creation of these
# translations is done futher below in this function.
# translations is done above in this function.
parent = element.get("parent")
if parent is not None and parent[constants.TYPE] not in select_types:
for d in element.get_translations(self.default_language):
Expand Down
3 changes: 2 additions & 1 deletion pyxform/survey_element.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,8 @@ def __eq__(self, y):
and self.to_json_dict() == y.to_json_dict()
)

def _translation_path(self, display_element):
def _translation_path(self, display_element: str) -> str:
"""Get an itextId based on the element XPath and display type."""
return self.get_xpath() + ":" + display_element

def get_translations(self, default_language):
Expand Down
14 changes: 1 addition & 13 deletions pyxform/xls2json.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
from pyxform.xlsparseutils import find_sheet_misspellings, is_valid_xml_tag

SMART_QUOTES = {"\u2018": "'", "\u2019": "'", "\u201c": '"', "\u201d": '"'}
SEARCH_APPEARANCE_REGEX = re.compile(r"search\(.*?\)")


def print_pyobj_to_json(pyobj, path=None):
Expand Down Expand Up @@ -362,19 +361,8 @@ def add_choices_info_to_question(
choice_filter = ""
if file_extension is None:
file_extension = ""
try:
is_search = bool(
SEARCH_APPEARANCE_REGEX.search(
question[constants.CONTROL][constants.APPEARANCE]
)
)
except (KeyError, TypeError):
is_search = False

# External selects from a "search" appearance alone don't work in Enketo. In Collect
# they must have the "item" elements in the body, rather than in an "itemset".
if not is_search:
question[constants.ITEMSET] = list_name
question[constants.ITEMSET] = list_name

if choice_filter:
# External selects e.g. type = "select_one_external city".
Expand Down
8 changes: 8 additions & 0 deletions tests/test_expected_output/search_and_select.xml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@
</meta>
</search_and_select>
</instance>
<instance id="fruits">
<root>
<item>
<name>name_key</name>
<label>name</label>
</item>
</root>
</instance>
<bind nodeset="/search_and_select/fruit" type="string"/>
<bind nodeset="/search_and_select/note_fruit" readonly="true()" type="string"/>
<bind jr:preload="uid" nodeset="/search_and_select/meta/instanceID" readonly="true()" type="string"/>
Expand Down
69 changes: 69 additions & 0 deletions tests/test_translations.py
Original file line number Diff line number Diff line change
Expand Up @@ -1553,6 +1553,75 @@ def test_choice_name_containing_dash_output_itext(self):
)


class TestTranslationsSearchAppearance(PyxformTestCase):
"""Translations behaviour with the search() appearance."""

def test_shared_choice_list(self):
"""Should include translation for search() items, sharing the choice list"""
md = """
| survey | | | | | |
| | type | name | label::en | label::fr | appearance |
| | select_one c1 | q1 | Question 1 | Chose 1 | search('my_file') |
| | select_one c2 | q2 | Question 2 | Chose 2 | |
| choices | | | | |
| | list_name | name | label::en | label::fr |
| | c1 | na | la-e | la-f |
| | c1 | nb | lb-e | lb-f |
| | c2 | na | la-e | la-f |
"""
self.assertPyxformXform(
md=md,
run_odk_validate=True,
xml__xpath_match=[
"/h:html/h:body/x:select1/x:item[./x:value/text()='na']",
xpc.model_itext_choice_text_label_by_pos("en", "c1", ("la-e", "lb-e")),
xpc.model_itext_choice_text_label_by_pos("fr", "c1", ("la-f", "lb-f")),
xpc.model_itext_choice_text_label_by_pos("en", "c2", ("la-e",)),
xpc.model_itext_choice_text_label_by_pos("fr", "c2", ("la-f",)),
],
)

def test_single_question_single_choice(self):
"""Should include translation for search() items, edge case of single elements"""
md = """
| survey | | | | | |
| | type | name | label::en | label::fr | appearance |
| | select_one c1 | q1 | Question 1 | Chose 1 | search('my_file') |
| choices | | | | |
| | list_name | name | label::en | label::fr |
| | c1 | na | la-e | la-f |
"""
self.assertPyxformXform(
md=md,
run_odk_validate=True,
xml__xpath_match=[
"/h:html/h:body/x:select1/x:item[./x:value/text()='na']",
xpc.model_itext_choice_text_label_by_pos("en", "c1", ("la-e",)),
xpc.model_itext_choice_text_label_by_pos("fr", "c1", ("la-f",)),
],
)

def test_name_clashes(self):
"""Should include translation for search() items, avoids any name clashes."""
md = """
| survey | | | | | |
| | type | name | label::en | label::fr | appearance |
| | select_one c1-0 | c1-0 | Question 1 | Chose 1 | search('my_file') |
| choices | | | | |
| | list_name | name | label::en | label::fr |
| | c1-0 | na | la-e | la-f |
"""
self.assertPyxformXform(
md=md,
run_odk_validate=True,
xml__xpath_match=[
"/h:html/h:body/x:select1/x:item[./x:value/text()='na']",
xpc.model_itext_choice_text_label_by_pos("en", "c1-0", ("la-e",)),
xpc.model_itext_choice_text_label_by_pos("fr", "c1-0", ("la-f",)),
],
)


class TestTranslationsOrOther(PyxformTestCase):
"""Translations behaviour with or_other."""

Expand Down

0 comments on commit 847d9c8

Please sign in to comment.