diff --git a/pyxform/builder.py b/pyxform/builder.py index d7edad2c..8837593a 100644 --- a/pyxform/builder.py +++ b/pyxform/builder.py @@ -205,8 +205,28 @@ def _add_other_option_to_multiple_choice_question(d: Dict[str, Any]) -> None: choice_list = d.get(const.CHOICES, d.get(const.CHILDREN, [])) if len(choice_list) <= 0: raise PyXFormError("There should be choices for this question.") - if OR_OTHER_CHOICE not in choice_list: - choice_list.append(OR_OTHER_CHOICE) + if not any(c[const.NAME] == OR_OTHER_CHOICE[const.NAME] for c in choice_list): + choice_list.append(SurveyElementBuilder._get_or_other_choice(choice_list)) + + @staticmethod + def _get_or_other_choice( + choice_list: List[Dict[str, Any]] + ) -> Dict[str, Union[str, Dict]]: + """ + If the choices have any translations, return an OR_OTHER choice for each lang. + """ + if any(isinstance(c.get(const.LABEL), dict) for c in choice_list): + langs = set( + lang + for c in choice_list + for lang in c[const.LABEL] + if isinstance(c.get(const.LABEL), dict) + ) + return { + const.NAME: OR_OTHER_CHOICE[const.NAME], + const.LABEL: {lang: OR_OTHER_CHOICE[const.LABEL] for lang in langs}, + } + return OR_OTHER_CHOICE @staticmethod def _add_none_option_to_select_all_that_apply(d_copy): @@ -268,9 +288,12 @@ def _create_section_from_dict(self, d): if ( itemset_choices is not None and isinstance(itemset_choices, list) - and OR_OTHER_CHOICE not in itemset_choices + and not any( + c[const.NAME] == OR_OTHER_CHOICE[const.NAME] + for c in itemset_choices + ) ): - itemset_choices.append(OR_OTHER_CHOICE) + itemset_choices.append(self._get_or_other_choice(itemset_choices)) # This is required for builder_tests.BuilderTests.test_loop to pass. self._add_other_option_to_multiple_choice_question(d=child) if survey_element: diff --git a/tests/builder_tests.py b/tests/builder_tests.py index 5a5a6d55..19cfaed3 100644 --- a/tests/builder_tests.py +++ b/tests/builder_tests.py @@ -130,7 +130,7 @@ def test_specify_other(self): "sexes": [ {"label": {"English": "Male"}, "name": "male"}, {"label": {"English": "Female"}, "name": "female"}, - {"label": "Other", "name": "other"}, + {"label": {"English": "Other"}, "name": "other"}, ] }, "children": [ @@ -145,7 +145,7 @@ def test_specify_other(self): # json2xform half that will need to change) {"name": "male", "label": {"English": "Male"}}, {"name": "female", "label": {"English": "Female"}}, - {"name": "other", "label": "Other"}, + {"name": "other", "label": {"English": "Other"}}, ], }, { @@ -233,7 +233,7 @@ def test_loop(self): "name": "open_pit_latrine", }, {"label": {"english": "Bucket system"}, "name": "bucket_system"}, - {"label": "Other", "name": "other"}, + {"label": {"english": "Other"}, "name": "other"}, ] }, "children": [ @@ -262,7 +262,7 @@ def test_loop(self): # u'name': u'none', # u'label': u'None', # }, - {"name": "other", "label": "Other"}, + {"name": "other", "label": {"english": "Other"}}, ], }, { @@ -330,7 +330,7 @@ def test_loop(self): "type": "integer", } ], - "label": "Other", + "label": {"english": "Other"}, "name": "other", "type": "group", }, diff --git a/tests/test_translations.py b/tests/test_translations.py index 6afa817c..e2cd0b50 100644 --- a/tests/test_translations.py +++ b/tests/test_translations.py @@ -1520,6 +1520,42 @@ def test_missing_translation__two_lang__warn__default(self): warnings__not_contains=[OR_OTHER_WARNING], ) + def test_choice_name_containing_dash_output_itext(self): + """Should output itext when list_name contains a dash (itextId separator).""" + md = """ + | survey | | | | + | | type | name | label:en | label:fr | + | | select_one with_us | q0 | Q1 EN | Q1 FR | + | | select_one with-dash | q1 | Q2 EN | Q2 FR | + | choices | | | | + | | list name | name | label:en | label:fr | + | | with_us | na | l1a-en | l1a-fr | + | | with_us | nb | l1b-en | l1b-fr | + | | with-dash | na | l2a-en | l2a-fr | + | | with-dash | nb | l2b-en | l2b-fr | + """ + self.assertPyxformXform( + md=md, + xml__xpath_match=[ + xpc.model_itext_choice_text_label_by_pos( + "en", "with_us", ("l1a-en", "l1b-en") + ), + xpc.model_itext_choice_text_label_by_pos( + "en", "with-dash", ("l2a-en", "l2b-en") + ), + xpc.model_itext_choice_text_label_by_pos( + "fr", "with_us", ("l1a-fr", "l1b-fr") + ), + xpc.model_itext_choice_text_label_by_pos( + "fr", "with-dash", ("l2a-fr", "l2b-fr") + ), + ], + ) + + +class TestTranslationsOrOther(PyxformTestCase): + """Translations behaviour with or_other.""" + def test_specify_other__with_translations(self): """Should add an "other" choice to the itemset instance and an itext label.""" md = """ @@ -1527,6 +1563,107 @@ def test_specify_other__with_translations(self): | | type | name | label | label::eng | | | select_one c1 or_other | q1 | Question 1 | Question A | | choices | | | | | | + | | list name | name | label | label::eng | label::fr | media::image::eng | + | | c1 | na | la | la-e | la-f | a.jpg | + | | c1 | nb | lb | lb-e | | b.jpg | + """ + self.assertPyxformXform( + md=md, + xml__xpath_match=[ + xpc.model_itext_choice_text_label_by_pos( + "eng", "c1", ("la-e", "lb-e", "Other") + ), + xpc.model_itext_choice_text_label_by_pos( + "fr", "c1", ("la-f", "-", "Other") + ), + xpc.model_itext_choice_text_label_by_pos( + DEFAULT_LANG, "c1", ("la", "lb", "Other") + ), + xpq.body_select1_itemset("q1"), + """ + /h:html/h:body/x:input[@ref='/test_name/q1_other']/ + x:label[text() = 'Specify other.'] + """, + ], + warnings__contains=[OR_OTHER_WARNING], + ) + + def test_specify_other__with_translations_only(self): + """Should add an "other" choice to the itemset instance and an itext label.""" + md = """ + | survey | | | | | + | | type | name | label::en | label::fr | + | | select_one c1 or_other | q1 | Question 1 | Question A | + | choices | | | | | + | | list name | name | label::en | label::fr | + | | c1 | na | la-e | la-f | + | | c1 | nb | lb-e | | + """ + self.assertPyxformXform( + md=md, + xml__xpath_match=[ + xpc.model_itext_choice_text_label_by_pos( + "en", "c1", ("la-e", "lb-e", "Other") + ), + xpc.model_itext_choice_text_label_by_pos( + "fr", "c1", ("la-f", "-", "Other") + ), + """ + /h:html/h:head/x:model/x:itext[ + not(descendant::x:translation[@lang='default']) + ] + """, + xpq.body_select1_itemset("q1"), + """ + /h:html/h:body/x:input[@ref='/test_name/q1_other']/ + x:label[text() = 'Specify other.'] + """, + ], + warnings__contains=[OR_OTHER_WARNING], + ) + + def test_specify_other__with_translations_only__existing_other_choice(self): + """Should not add an extra "other" choice if already defined for some reason.""" + # Blank translations for existing "other" choices are not replaced with "Other". + md = """ + | survey | | | | | + | | type | name | label::en | label::fr | + | | select_one c1 or_other | q1 | Question 1 | Question A | + | choices | | | | | + | | list name | name | label::en | label::fr | + | | c1 | na | la-e | la-f | + | | c1 | nb | lb-e | | + | | c1 | other | Other | | + """ + self.assertPyxformXform( + md=md, + xml__xpath_match=[ + xpc.model_itext_choice_text_label_by_pos( + "en", "c1", ("la-e", "lb-e", "Other") + ), + xpc.model_itext_choice_text_label_by_pos("fr", "c1", ("la-f", "-", "-")), + """ + /h:html/h:head/x:model/x:itext[ + not(descendant::x:translation[@lang='default']) + ] + """, + xpq.body_select1_itemset("q1"), + """ + /h:html/h:body/x:input[@ref='/test_name/q1_other']/ + x:label[text() = 'Specify other.'] + """, + ], + warnings__contains=[OR_OTHER_WARNING], + ) + + def test_specify_other__with_translations_only__missing_first_translation(self): + """Should add an "other" choice to the itemset instance and an itext label.""" + # xls2json validation would raise an error if a choice has no label at all. + md = """ + | survey | | | | | | + | | type | name | label | label::eng | label:fr | + | | select_one c1 or_other | q1 | Question 1 | Question A | QA fr | + | choices | | | | | | | | list name | name | label | label::eng | label::fr | | | c1 | na | la | la-e | la-f | | | c1 | nb | lb | lb-e | | @@ -1535,9 +1672,11 @@ def test_specify_other__with_translations(self): md=md, xml__xpath_match=[ xpc.model_itext_choice_text_label_by_pos( - "eng", "c1", ("la-e", "lb-e", "-") + "eng", "c1", ("la-e", "lb-e", "Other") + ), + xpc.model_itext_choice_text_label_by_pos( + "fr", "c1", ("la-f", "-", "Other") ), - xpc.model_itext_choice_text_label_by_pos("fr", "c1", ("la-f", "-", "-")), xpc.model_itext_choice_text_label_by_pos( DEFAULT_LANG, "c1", ("la", "lb", "Other") ), @@ -1560,16 +1699,18 @@ def test_specify_other__with_translations__with_group(self): | | end group | g1 | | | | choices | | | | | | | | list name | name | label | label::eng | label::fr | - | | c1 | na | la | la-e | la-f | - | | c1 | nb | lb | lb-e | | + | | c1 | na | la | | | + | | c1 | nb | lb | lb-e | lb-f | """ self.assertPyxformXform( md=md, xml__xpath_match=[ xpc.model_itext_choice_text_label_by_pos( - "eng", "c1", ("la-e", "lb-e", "-") + "eng", "c1", ("-", "lb-e", "Other") + ), + xpc.model_itext_choice_text_label_by_pos( + "fr", "c1", ("-", "lb-f", "Other") ), - xpc.model_itext_choice_text_label_by_pos("fr", "c1", ("la-f", "-", "-")), xpc.model_itext_choice_text_label_by_pos( DEFAULT_LANG, "c1", ("la", "lb", "Other") ), @@ -1600,9 +1741,11 @@ def test_specify_other__with_translations__with_repeat(self): md=md, xml__xpath_match=[ xpc.model_itext_choice_text_label_by_pos( - "eng", "c1", ("la-e", "lb-e", "-") + "eng", "c1", ("la-e", "lb-e", "Other") + ), + xpc.model_itext_choice_text_label_by_pos( + "fr", "c1", ("la-f", "-", "Other") ), - xpc.model_itext_choice_text_label_by_pos("fr", "c1", ("la-f", "-", "-")), xpc.model_itext_choice_text_label_by_pos( DEFAULT_LANG, "c1", ("la", "lb", "Other") ), @@ -1636,9 +1779,11 @@ def test_specify_other__with_translations__with_nested_group(self): md=md, xml__xpath_match=[ xpc.model_itext_choice_text_label_by_pos( - "eng", "c1", ("la-e", "lb-e", "-") + "eng", "c1", ("la-e", "lb-e", "Other") + ), + xpc.model_itext_choice_text_label_by_pos( + "fr", "c1", ("la-f", "-", "Other") ), - xpc.model_itext_choice_text_label_by_pos("fr", "c1", ("la-f", "-", "-")), xpc.model_itext_choice_text_label_by_pos( DEFAULT_LANG, "c1", ("la", "lb", "Other") ), @@ -1679,9 +1824,11 @@ def test_specify_other__with_translations__with_nested_repeat(self): md=md, xml__xpath_match=[ xpc.model_itext_choice_text_label_by_pos( - "eng", "c1", ("la-e", "lb-e", "-") + "eng", "c1", ("la-e", "lb-e", "Other") + ), + xpc.model_itext_choice_text_label_by_pos( + "fr", "c1", ("la-f", "-", "Other") ), - xpc.model_itext_choice_text_label_by_pos("fr", "c1", ("la-f", "-", "-")), xpc.model_itext_choice_text_label_by_pos( DEFAULT_LANG, "c1", ("la", "lb", "Other") ), @@ -1752,35 +1899,3 @@ def test_specify_other__choice_filter(self): errored=True, error__contains=["[row : 3] Choice filter not supported with or_other."], ) - - def test_choice_name_containing_dash_output_itext(self): - """Should output itext when list_name contains a dash (itextId separator).""" - md = """ - | survey | | | | - | | type | name | label:en | label:fr | - | | select_one with_us | q0 | Q1 EN | Q1 FR | - | | select_one with-dash | q1 | Q2 EN | Q2 FR | - | choices | | | | - | | list name | name | label:en | label:fr | - | | with_us | na | l1a-en | l1a-fr | - | | with_us | nb | l1b-en | l1b-fr | - | | with-dash | na | l2a-en | l2a-fr | - | | with-dash | nb | l2b-en | l2b-fr | - """ - self.assertPyxformXform( - md=md, - xml__xpath_match=[ - xpc.model_itext_choice_text_label_by_pos( - "en", "with_us", ("l1a-en", "l1b-en") - ), - xpc.model_itext_choice_text_label_by_pos( - "en", "with-dash", ("l2a-en", "l2b-en") - ), - xpc.model_itext_choice_text_label_by_pos( - "fr", "with_us", ("l1a-fr", "l1b-fr") - ), - xpc.model_itext_choice_text_label_by_pos( - "fr", "with-dash", ("l2a-fr", "l2b-fr") - ), - ], - )