Skip to content

Commit

Permalink
fix: clarify 'search()' vs. 'search', use documented settings names
Browse files Browse the repository at this point in the history
  • Loading branch information
lindsay-stevens committed Feb 28, 2024
1 parent 667942f commit e008ffd
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 25 deletions.
31 changes: 15 additions & 16 deletions pyxform/survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
r"(instance\(.*\)\/root\/item\[.*?(\$\{.*\})\]\/.*?)\s"
)
RE_PULLDATA = re.compile(r"(pulldata\s*\(\s*)(.*?),")
SEARCH_APPEARANCE_REGEX = re.compile(r"search\(.*?\)")
SEARCH_FUNCTION_REGEX = re.compile(r"search\(.*?\)")


class InstanceInfo:
Expand Down Expand Up @@ -656,22 +656,22 @@ def _add_to_nested_dict(self, dicty, path, value):

def _redirect_is_search_itext(self, element: SurveyElement) -> bool:
"""
For selects using the "search()" appearance, redirect itext for in-line items.
For selects using the "search()" function, redirect itext for in-line items.
External selects from a "search" appearance alone don't work in Enketo. In Collect
External selects from a "search" function 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.
accounts for questions with and without a "search()" function sharing choices.
:param element: A select type question.
:return: If True, the element has a search appearance.
:return: If True, the element uses the search function.
"""
try:
is_search = bool(
SEARCH_APPEARANCE_REGEX.search(
SEARCH_FUNCTION_REGEX.search(
element[constants.CONTROL][constants.APPEARANCE]
)
)
Expand All @@ -682,8 +682,8 @@ def _redirect_is_search_itext(self, element: SurveyElement) -> bool:
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."
"using 'search()'. This combination is not supported. "
"Remove the 'search()' usage, or change the select type."
)
raise PyXFormError(msg)
itemset = element[constants.ITEMSET]
Expand Down Expand Up @@ -799,15 +799,14 @@ 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)
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."
f"Question '{q_name}' uses 'search()', and its select type references"
f" the choice list name '{list_name}'. This choice list name is "
f"referenced by at least one other question that is not using "
f"'search()', which will not work: {refs_str}. Either 1) use "
f"'search()' for all questions using this choice list name, or 2) "
f"use a different choice list name for the question using 'search()'."
)
raise PyXFormError(msg)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
"""
Tests about the 'search()' function, which pulls data from CSV, optionally filtering it.
Although both go in the 'appearance' column, 'search()' is not the same as 'search'. The
latter is a style which enables a choice filtering UI.
"""

from tests.pyxform_test_case import PyxformTestCase
from tests.xpath_helpers.choices import xpc
from tests.xpath_helpers.questions import xpq
Expand Down Expand Up @@ -127,7 +134,7 @@ def test_usage_with_other_selects__invalid_list_reuse_by_non_search_question(sel
self.assertPyxformXform(
md=md,
errored=True,
error__contains=["Question 'q1' uses the 'search' appearance"],
error__contains=["Question 'q1' uses 'search()',"],
)

def test_single_question_usage(self):
Expand Down Expand Up @@ -566,6 +573,6 @@ def test_select_from_file__search_on_same_question(self):
md=md,
errored=True,
error__contains=[
"Question 'q1' is a select from file type, with a 'search' appearance."
"Question 'q1' is a select from file type, using 'search()'."
],
)
20 changes: 14 additions & 6 deletions tests/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,19 @@


class TestSettings(PyxformTestCase):
def test_title(self):
"""
Test form settings.
Use the documented setting name, even if it's an alias.
"""

#
def test_form_title(self):
"""Should find the title set in the XForm."""
md = """
| settings |
| | title |
| | My Form |
| | form_title |
| | My Form |
| survey | | | |
| | type | name | label |
| | text | q1 | hello |
Expand All @@ -19,18 +26,19 @@ def test_title(self):
xml__xpath_match=["/h:html/h:head/h:title[.='My Form']"],
)

def test_id_string(self):
def test_form_id(self):
"""Should find the instance id set in the XForm."""
md = """
| settings |
| | id_string |
| | my_form |
| | form_id |
| | my_form |
| survey | | | |
| | type | name | label |
| | text | q1 | hello |
"""
self.assertPyxformXform(
md=md,
debug=True,
xml__xpath_match=[
"/h:html/h:head/x:model/x:instance/x:test_name[@id='my_form']"
],
Expand Down
2 changes: 1 addition & 1 deletion tests/test_xlsform_spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def test_warnings__count(self):
| | animals | zebra | | |
| | animals | buffalo | | |
| settings | | | | | |
| | title | id_string | public_key | submission_url | default_language |
| | form_title | form_id | public_key | submission_url | default_language |
| | spec_test | spec_test | | | |
"""
warnings = []
Expand Down

0 comments on commit e008ffd

Please sign in to comment.