Skip to content

Commit

Permalink
fix: support of or_other with selects in itemsets
Browse files Browse the repository at this point in the history
- builder.py:
  - add or_other choice to survey-level choice dict
  - add type annotations
- constants.py: add new constant for "or specify other" internal suffix
- xls2json.py: add error for choice_filter + or_other. Main reason I
  suppose is that it's unknown how to filter the "other" choice so the
  user might never see it.
- xlsform_spec_test.py:
  - fix or_other expected XML to include "other" and match sorted
    element names.
  - move or_other test into this testcase since it is the same as others
    there already.
- builder_tests.py: update expected output to match or_other output.
  • Loading branch information
lindsay-stevens committed May 12, 2023
1 parent c2dd549 commit 3275b6c
Show file tree
Hide file tree
Showing 8 changed files with 179 additions and 97 deletions.
45 changes: 30 additions & 15 deletions pyxform/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
import copy
import os
import re
from typing import Any, Dict, Optional
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union

from pyxform import file_utils, utils
from pyxform import constants, file_utils, utils
from pyxform.entities.entity_declaration import EntityDeclaration
from pyxform.errors import PyXFormError
from pyxform.external_instance import ExternalInstance
Expand All @@ -25,6 +25,14 @@
from pyxform.survey import Survey
from pyxform.xls2json import SurveyReader

if TYPE_CHECKING:
from pyxform.survey_element import SurveyElement

OR_OTHER_CHOICE = {
"name": "other",
"label": "Other",
}


def copy_json_dict(json_dict):
"""
Expand Down Expand Up @@ -88,7 +96,9 @@ def set_sections(self, sections):
assert type(sections) == dict
self._sections = sections

def create_survey_element_from_dict(self, d):
def create_survey_element_from_dict(
self, d: Dict[str, Any]
) -> Union["SurveyElement", List["SurveyElement"]]:
"""
Convert from a nested python dictionary/array structure (a json dict I
call it because it corresponds directly with a json object)
Expand Down Expand Up @@ -143,7 +153,11 @@ def _save_trigger_as_setvalue_and_remove_calculate(self, d):
self.setvalues_by_triggering_ref[triggering_ref] = [(d["name"], value)]

@staticmethod
def _create_question_from_dict(d, question_type_dictionary, add_none_option=False):
def _create_question_from_dict(
d: Dict[str, Any],
question_type_dictionary: Dict[str, Any],
add_none_option: bool = False,
) -> Union[Question, List[Question]]:
question_type_str = d["type"]
d_copy = d.copy()

Expand All @@ -152,11 +166,9 @@ def _create_question_from_dict(d, question_type_dictionary, add_none_option=Fals
SurveyElementBuilder._add_none_option_to_select_all_that_apply(d_copy)

# Handle or_other on select type questions
or_other_str = " or specify other"
if question_type_str.endswith(or_other_str):
question_type_str = question_type_str[
: len(question_type_str) - len(or_other_str)
]
or_other_len = len(constants.SELECT_OR_OTHER_SUFFIX)
if question_type_str.endswith(constants.SELECT_OR_OTHER_SUFFIX):
question_type_str = question_type_str[: len(question_type_str) - or_other_len]
d_copy["type"] = question_type_str
SurveyElementBuilder._add_other_option_to_multiple_choice_question(d_copy)
return [
Expand All @@ -173,20 +185,18 @@ def _create_question_from_dict(d, question_type_dictionary, add_none_option=Fals
# todo: clean up this spaghetti code
d_copy["question_type_dictionary"] = question_type_dictionary
if question_class:

return question_class(**d_copy)

return []

@staticmethod
def _add_other_option_to_multiple_choice_question(d):
def _add_other_option_to_multiple_choice_question(d: Dict[str, Any]) -> None:
# ideally, we'd just be pulling from children
choice_list = d.get("choices", d.get("children", []))
if len(choice_list) <= 0:
raise PyXFormError("There should be choices for this question.")
other_choice = {"name": "other", "label": "Other"}
if other_choice not in choice_list:
choice_list.append(other_choice)
if OR_OTHER_CHOICE not in choice_list:
choice_list.append(OR_OTHER_CHOICE)

@staticmethod
def _add_none_option_to_select_all_that_apply(d_copy):
Expand Down Expand Up @@ -220,7 +230,7 @@ def _get_question_class(question_type_str, question_type_dictionary):
return SurveyElementBuilder.QUESTION_CLASSES[control_tag]

@staticmethod
def _create_specify_other_question_from_dict(d):
def _create_specify_other_question_from_dict(d: Dict[str, Any]) -> InputQuestion:
kwargs = {
"type": "text",
"name": "%s_other" % d["name"],
Expand All @@ -242,6 +252,11 @@ def _create_section_from_dict(self, d):
# And I hope it doesn't break something else.
# I think the good solution would be to rewrite this class.
survey_element = self.create_survey_element_from_dict(copy.deepcopy(child))
if child["type"].endswith(" or specify other"):
select_question = survey_element[0]
itemset_choices = result["choices"][select_question["itemset"]]
if OR_OTHER_CHOICE not in itemset_choices:
itemset_choices.append(OR_OTHER_CHOICE)
if survey_element:
result.add_children(survey_element)

Expand Down
1 change: 1 addition & 0 deletions pyxform/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
SELECT_ONE = "select one"
SELECT_ONE_EXTERNAL = "select one external"
SELECT_ALL_THAT_APPLY = "select all that apply"
SELECT_OR_OTHER_SUFFIX = " or specify other"
RANK = "rank"
CHOICES = "choices"

Expand Down
8 changes: 7 additions & 1 deletion pyxform/xls2json.py
Original file line number Diff line number Diff line change
Expand Up @@ -1158,7 +1158,13 @@ def workbook_to_json(

specify_other_question = None
if parse_dict.get("specify_other") is not None:
select_type += " or specify other"
select_type += constants.SELECT_OR_OTHER_SUFFIX
if row.get(constants.CHOICE_FILTER):
msg = (
ROW_FORMAT_STRING % row_number
+ " Choice filter not supported with or_other."
)
raise PyXFormError(msg)

new_json_dict = row.copy()
new_json_dict[constants.TYPE] = select_type
Expand Down
17 changes: 17 additions & 0 deletions tests/builder_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ def test_specify_other(self):
"sexes": [
{"label": {"English": "Male"}, "name": "male"},
{"label": {"English": "Female"}, "name": "female"},
{"label": "Other", "name": "other"},
]
},
"children": [
Expand Down Expand Up @@ -167,6 +168,7 @@ def test_specify_other(self):
],
}
self.maxDiff = None
observed = survey.to_json_dict()
self.assertEqual(survey.to_json_dict(), expected_dict)

def test_select_one_question_with_identical_choice_name(self):
Expand Down Expand Up @@ -231,6 +233,7 @@ def test_loop(self):
"name": "open_pit_latrine",
},
{"label": {"english": "Bucket system"}, "name": "bucket_system"},
{"label": "Other", "name": "other"},
]
},
"children": [
Expand Down Expand Up @@ -317,6 +320,20 @@ def test_loop(self):
}
],
},
{
"children": [
{
"label": {
"english": "How many Other are on the premises?"
},
"name": "number",
"type": "integer",
}
],
"label": "Other",
"name": "other",
"type": "group",
},
],
},
{
Expand Down
24 changes: 14 additions & 10 deletions tests/test_expected_output/or_other.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,29 +16,33 @@
<instance id="colors">
<root>
<item>
<pastel>no</pastel>
<name>red</name>
<label>red</label>
<name>red</name>
<pastel>no</pastel>
</item>
<item>
<pastel>no</pastel>
<name>green</name>
<label>green</label>
<name>green</name>
<pastel>no</pastel>
</item>
<item>
<pastel>no</pastel>
<name>blue</name>
<label>blue</label>
<name>blue</name>
<pastel>no</pastel>
</item>
<item>
<pastel>yes</pastel>
<name>mauve</name>
<label>mauve</label>
<name>mauve</name>
<pastel>yes</pastel>
</item>
<item>
<pastel>yes</pastel>
<name>apricot</name>
<label>apricot</label>
<name>apricot</name>
<pastel>yes</pastel>
</item>
<item>
<label>Other</label>
<name>other</name>
</item>
</root>
</instance>
Expand Down
Loading

0 comments on commit 3275b6c

Please sign in to comment.