Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

694: search and pulldata secondary instance conflict #697

Merged
merged 4 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pyxform/aliases.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 = {
Expand All @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion pyxform/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
46 changes: 39 additions & 7 deletions pyxform/survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ class Survey(Section):
"namespaces": str,
constants.ENTITY_FEATURES: list,
}
) # yapf: disable
)

def validate(self):
if self.id_string in [None, "None"]:
Expand Down Expand Up @@ -385,7 +385,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), "
Expand Down Expand Up @@ -655,8 +655,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.

Expand All @@ -669,7 +668,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(
Expand All @@ -680,9 +679,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."
lindsay-stevens marked this conversation as resolved.
Show resolved Hide resolved
)
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):
"""
Expand Down Expand Up @@ -745,6 +755,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:
Expand All @@ -753,7 +765,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")
Expand All @@ -780,6 +797,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 "
lindsay-stevens marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
2 changes: 1 addition & 1 deletion pyxform/survey_element.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
"autoplay": str,
"flat": lambda: False,
"action": str,
"list_name": str,
const.LIST_NAME_U: str,
"trigger": str,
}

Expand Down
20 changes: 10 additions & 10 deletions pyxform/xls2json.py
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand All @@ -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]


Expand Down Expand Up @@ -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 ##########
Expand All @@ -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.
Expand Down Expand Up @@ -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
)
# #################################

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"))
Expand Down
Loading