From d6f954b02364787d1329a56d80c571e0aa53d9a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9l=C3=A8ne=20Martin?= Date: Tue, 16 Jan 2024 12:20:48 -0800 Subject: [PATCH] Add 'Other' label text for all languages defined by form --- pyxform/builder.py | 34 ++- pyxform/xls2json.py | 6 - tests/builder_tests.py | 12 +- .../example_xls/~$attribute_columns_test.xlsx | Bin 0 -> 165 bytes tests/test_repeat.py | 1 - tests/test_translations.py | 217 +++++++++++------- 6 files changed, 168 insertions(+), 102 deletions(-) create mode 100644 tests/example_xls/~$attribute_columns_test.xlsx diff --git a/pyxform/builder.py b/pyxform/builder.py index d7edad2c..3254c85a 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(OR_OTHER_CHOICE[const.NAME] in choice[const.NAME] for choice in choice_list): + choice_list.append(SurveyElementBuilder._get_translated_other_label(choice_list)) + + @staticmethod + def _get_translated_other_label(choice_list) -> Dict[str, Any]: + translated_other_choice = OR_OTHER_CHOICE.copy() + + if isinstance(choice_list[0][const.LABEL], Dict): + # Get all langs defined across all existing choices in this list and give them all a label of "Other" + langs = set().union( + *map( + set, + [ + itemset_choice[const.LABEL].keys() + for itemset_choice in choice_list + ], + ) + ) + translated_other_choice[const.LABEL] = dict( + map(lambda l: (l, OR_OTHER_CHOICE[const.LABEL]), langs) + ) + return translated_other_choice @staticmethod def _add_none_option_to_select_all_that_apply(d_copy): @@ -265,13 +285,17 @@ def _create_section_from_dict(self, d): if child[const.TYPE].endswith(const.SELECT_OR_OTHER_SUFFIX): select_question = survey_element[0] itemset_choices = self._choices.get(select_question[const.ITEMSET], None) + translated_other_choice = OR_OTHER_CHOICE.copy() if ( itemset_choices is not None and isinstance(itemset_choices, list) - and OR_OTHER_CHOICE not in itemset_choices + and not any( + OR_OTHER_CHOICE[const.NAME] in choice[const.NAME] + for choice in itemset_choices + ) ): - itemset_choices.append(OR_OTHER_CHOICE) - # This is required for builder_tests.BuilderTests.test_loop to pass. + itemset_choices.append(self._get_translated_other_label(itemset_choices)) + # Select choices need to be both in choices and children. See the loop test in builder_tests self._add_other_option_to_multiple_choice_question(d=child) if survey_element: result.add_children(survey_element) diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index 6d987b61..6a3d126b 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -1213,12 +1213,6 @@ def workbook_to_json( if parse_dict.get("specify_other") is not None: sheet_translations.or_other_seen = True 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 diff --git a/tests/builder_tests.py b/tests/builder_tests.py index 0eac39a7..b514a023 100644 --- a/tests/builder_tests.py +++ b/tests/builder_tests.py @@ -129,7 +129,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": [ @@ -144,7 +144,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"}}, ], }, { @@ -232,7 +232,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": [ @@ -261,7 +261,7 @@ def test_loop(self): # u'name': u'none', # u'label': u'None', # }, - {"name": "other", "label": "Other"}, + {"name": "other", "label": {"english": "Other"}}, ], }, { @@ -329,7 +329,9 @@ def test_loop(self): "type": "integer", } ], - "label": "Other", + "label": { + "english": "Other" + }, "name": "other", "type": "group", }, diff --git a/tests/example_xls/~$attribute_columns_test.xlsx b/tests/example_xls/~$attribute_columns_test.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..eae17e07523bef514561f6ecc5979bb750e4de1a GIT binary patch literal 165 ycmd<}NX<#jOI7eqEGo&&Qy>=bGI%hgGUNbp9uO-q_%b9i6fu-AWCHmLREGdm)f5u| literal 0 HcmV?d00001 diff --git a/tests/test_repeat.py b/tests/test_repeat.py index 35627b5a..ae6d31c8 100644 --- a/tests/test_repeat.py +++ b/tests/test_repeat.py @@ -997,7 +997,6 @@ def test_repeat_count_item_with_same_suffix_as_repeat_is_ok(self): """ self.assertPyxformXform( md=md, - debug=True, xml__xpath_match=[ # repeat references existing count element directly. """ diff --git a/tests/test_translations.py b/tests/test_translations.py index 4fff2d48..c66e442d 100644 --- a/tests/test_translations.py +++ b/tests/test_translations.py @@ -522,7 +522,6 @@ def test_translation_detection__survey_columns_present_with_media(self): """ self.assertPyxformXform( md=md, - debug=True, xml__xpath_match=[ xpq.body_select1_itemset("f"), xpq.body_label_inline("select1", "f", "f"), @@ -1521,26 +1520,92 @@ 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") + ), + ], + ) + + # region or_other + def test_specify_other__with_translations(self): """Should add an "other" choice to the itemset instance and an itext label.""" md = """ - | survey | | | | | - | | type | name | label | label::eng | - | | select_one c1 or_other | q1 | Question 1 | Question A | - | choices | | | | | | - | | list name | name | label | label::eng | label::fr | - | | c1 | na | la | la-e | la-f | - | | c1 | nb | lb | lb-e | | + | survey | | | | | + | | type | name | label::eng | label::fr | + | | select_one c1 or_other | q1 | Question A | A FR | + | choices | | | | | + | | list name | name | label::eng | label::fr | + | | c1 | na | la-e | la-f | + | | c1 | nb | 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", ("la-e", "lb-e", "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") + "fr", "c1", ("la-f", "lb-f", "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_and_existing_other(self): + """Should not add "other" choice again.""" + md = """ + | survey | | | | | + | | type | name | label::eng | label::fr | + | | select_one c1 or_other | q1 | Question A | A FR | + | choices | | | | | + | | list name | name | label::eng | label::fr | + | | c1 | na | la-e | la-f | + | | c1 | nb | lb-e | lb-f | + | | c1 | other| Other | Autre | + """ + self.assertPyxformXform( + md=md, + xml__xpath_count=[ + ("/h:html/h:head/x:model/x:instance[@id='c1']/x:root/x:item", 3) + ], + 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", "lb-f", "Autre") ), xpq.body_select1_itemset("q1"), """ @@ -1554,25 +1619,24 @@ def test_specify_other__with_translations(self): def test_specify_other__with_translations__with_group(self): """Should add an "other" choice to the itemset instance and an itext label.""" md = """ - | survey | | | | | - | | type | name | label | label::eng | - | | begin group | g1 | Group 1 | Group 1 | - | | select_one c1 or_other | q1 | Question 1 | Question A | - | | end group | g1 | | | - | choices | | | | | | - | | list name | name | label | label::eng | label::fr | - | | c1 | na | la | la-e | la-f | - | | c1 | nb | lb | lb-e | | + | survey | | | | + | | type | name | label::eng | + | | begin group | g1 | Group 1 | + | | select_one c1 or_other | q1 | Question A | + | | end group | g1 | | + | choices | | | | | + | | list name | name | label::eng | label::fr | + | | c1 | na | la-e | la-f | + | | c1 | nb | 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", ("la-e", "lb-e", "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") + "fr", "c1", ("la-f", "lb-f", "Other") ), xpq.body_group_select1_itemset("g1", "q1"), """ @@ -1587,25 +1651,24 @@ def test_specify_other__with_translations__with_group(self): def test_specify_other__with_translations__with_repeat(self): """Should add an "other" choice to the itemset instance and an itext label.""" md = """ - | survey | | | | | - | | type | name | label | label::eng | - | | begin repeat | r1 | Repeat 1 | Repeat 1 | - | | select_one c1 or_other | q1 | Question 1 | Question A | - | | end repeat | r1 | | | - | choices | | | | | | - | | list name | name | label | label::eng | label::fr | - | | c1 | na | la | la-e | la-f | - | | c1 | nb | lb | lb-e | | + | survey | | | | | + | | type | name | label::fr | label::eng | + | | begin repeat | r1 | Repeat 1 fr | Repeat 1 | + | | select_one c1 or_other | q1 | Question 1 fr | Question A | + | | end repeat | r1 | | | + | choices | | | | | + | | list name | name | label::eng | label::fr | + | | c1 | na | la-e | la-f | + | | c1 | nb | 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", ("la-e", "lb-e", "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") + "fr", "c1", ("la-f", "lb-f", "Other") ), xpq.body_repeat_select1_itemset("r1", "q1"), """ @@ -1630,16 +1693,18 @@ def test_specify_other__with_translations__with_nested_group(self): | | end group | g1 | | | | choices | | | | | | | | list name | name | label | label::eng | label::fr | - | | c1 | na | la | la-e | la-f | + | | c1 | na | la | | la-f | | | c1 | nb | lb | lb-e | | """ 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", ("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") ), @@ -1664,27 +1729,26 @@ def test_specify_other__with_translations__with_nested_group(self): def test_specify_other__with_translations__with_nested_repeat(self): """Should add an "other" choice to the itemset instance and an itext label.""" md = """ - | survey | | | | | - | | type | name | label | label::eng | - | | begin group | g1 | Group 1 | Group 1 | - | | begin repeat | r1 | Repeat 1 | Repeat 1 | - | | select_one c1 or_other | q1 | Question 1 | Question A | - | | end repeat | r1 | | | - | | end group | g1 | | | - | choices | | | | | | - | | list name | name | label | label::eng | label::fr | - | | c1 | na | la | la-e | la-f | - | | c1 | nb | lb | lb-e | | + | survey | | | | + | | type | name | label::eng | + | | begin group | g1 | Group 1 | + | | begin repeat | r1 | Repeat 1 | + | | select_one c1 or_other | q1 | Question A | + | | end repeat | r1 | | + | | end group | g1 | | + | choices | | | | | + | | list name | name | label::eng | 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( - "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", "-", "-")), xpc.model_itext_choice_text_label_by_pos( - DEFAULT_LANG, "c1", ("la", "lb", "Other") + "fr", "c1", ("la-f", "-", "Other") ), """ /h:html/h:body/x:group[@ref='/test_name/g1'] @@ -1737,51 +1801,34 @@ def test_specify_other__no_translations(self): ) def test_specify_other__choice_filter(self): - """Should raise an error since these features are unsupported together.""" + """Should work starting in pyxform 2""" md = """ | survey | | | | | | type | name | label | choice_filter | - | | input | q0 | Question 0 | | + | | text | q0 | Question 0 | | | | select_one c1 or_other | q1 | Question 1 | ${q0} = cf | | choices | | | | | | list name | name | label | cf | | | c1 | na | la | 1 | | | c1 | nb | lb | 2 | """ - self.assertPyxformXform( - md=md, - 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") + """ + /h:html/h:head/x:model[not(descendant::x:itext)] + """, + xpc.model_instance_choices_label( + "c1", (("na", "la"), ("nb", "lb"), ("other", "Other")) ), + xpq.body_select1_itemset("q1"), + """ + /h:html/h:body/x:input[@ref='/test_name/q1_other']/ + x:label[text() = 'Specify other.'] + """, ], + warnings__not_contains=[OR_OTHER_WARNING], ) + + +# endregion