Repository: XLSForm/pyxform (python)

Issue:
Select with single-language labels and choice filter can't specify choice images
#### Software and hardware versions 
all

#### Problem description
Images in ODK XForms are displayed using the `itext` machinery. When a select has labels in a single language and a choice filter, the choices are represented as a secondary instance and the labels are taken from the `label` element for each `item` in the instance. In other words, `itext` isn't used. This means there's no way for images to be displayed by clients.

#### Steps to reproduce the problem
Create an XLSForm with a select that has single-language labels and a choice filter such as [this one](https://docs.google.com/spreadsheets/d/13RlgVIT3U70kmXOWVoWaDsXSFv2UAE7QGnbLwS2ARPQ/edit#gid=1903566534).

#### Expected behavior
It should be possible to specify images for selects with single-language labels.

#### Other information 
If there's media specified for choices, `itext` should always be used for specifying the choice labels. #463 is in the same general space.

Pull request description:
Fixed duplicate label translations for secondary itemsets and use itextID for selects with choices that have media specified
Based on #467 
Closes #464, #463 

#### Why is this the best possible solution? Were any other approaches considered?

The simplest approach(_Least amount of modification to already existing functionality_). Ensures that we only generate translations for choices in filtered selects in a controlled manner and that itext is used for selects with media

Considered utilizing uuids, but decided otherwise since uuid generation is unpredictable; _Translation entries are created/linked in multiple areas i.e `_setup_media` & `_setup_translations` using uuids may lead to more duplicate entries and I'm not entirely sure of an easy way to implement uuids and correctly link them to the question elements_

#### What are the regression risks?

None

#### Does this change require updates to documentation? If so, please file an issue [here](https://github.com/XLSForm/xlsform.github.io) and include the link below.

None

#### Before submitting this PR, please make sure you have:
- [x] included test cases for core behavior and edge cases in `tests_v1`
- [x] run `nosetests` and verified all tests pass
- [x] run `black pyxform` to format code
- [x] verified that any code or assets from external sources are properly credited in comments

Patch:
diff --git a/pyxform/question.py b/pyxform/question.py
index 579b7cf..945af4b 100644
--- a/pyxform/question.py
+++ b/pyxform/question.py
@@ -195,11 +195,16 @@ class MultipleChoiceQuestion(Question):
         if self["itemset"] and isinstance(self["itemset"], basestring):
             choice_filter = self.get("choice_filter")
             itemset, file_extension = os.path.splitext(self["itemset"])
+            has_media = False
+
+            if choices.get(itemset):
+                has_media = bool(choices[itemset][0].get("media"))
+
             if file_extension in [".csv", ".xml"]:
                 itemset = itemset
                 itemset_label_ref = "label"
             else:
-                if not multi_language:
+                if not multi_language and not has_media:
                     itemset = self["itemset"]
                     itemset_label_ref = "label"
                 else:
diff --git a/pyxform/survey.py b/pyxform/survey.py
index f4c416d..4ec4100 100644
--- a/pyxform/survey.py
+++ b/pyxform/survey.py
@@ -227,12 +227,13 @@ class Survey(Section):
         """
         instance_element_list = []
         multi_language = isinstance(choice_list[0].get("label"), dict)
+        has_media = bool(choice_list[0].get("media"))
         for idx, choice in enumerate(choice_list):
             choice_element_list = []
             # Add a unique id to the choice element in case there is itext
             # it references
-            if multi_language:
-                itext_id = "-".join(["static_instance", list_name, str(idx)])
+            if multi_language or has_media:
+                itext_id = "-".join([list_name, str(idx)])
                 choice_element_list.append(node("itextId", itext_id))
 
             for name, value in sorted(choice.items()):
@@ -590,31 +591,41 @@ class Survey(Section):
 
         self._translations = defaultdict(dict)  # pylint: disable=W0201
         for element in self.iter_descendants():
-            for d in element.get_translations(self.default_language):
-
-                translation_path = d["path"]
-                form = "long"
-
-                if "guidance_hint" in d["path"]:
-                    translation_path = d["path"].replace("guidance_hint", "hint")
-                    form = "guidance"
-
-                self._translations[d["lang"]][translation_path] = self._translations[
-                    d["lang"]
-                ].get(translation_path, {})
-
-                self._translations[d["lang"]][translation_path].update(
-                    {form: {"text": d["text"], "output_context": d["output_context"],}}
-                )
+            # Skip creation of translations for choices in filtered selects
+            # The creation of these translations is done futher below in this
+            # function
+            parent = element.get("parent")
+            if parent and not parent.get("choice_filter"):
+                for d in element.get_translations(self.default_language):
+                    translation_path = d["path"]
+                    form = "long"
+
+                    if "guidance_hint" in d["path"]:
+                        translation_path = d["path"].replace("guidance_hint", "hint")
+                        form = "guidance"
+
+                    self._translations[d["lang"]][
+                        translation_path
+                    ] = self._translations[d["lang"]].get(translation_path, {})
+
+                    self._translations[d["lang"]][translation_path].update(
+                        {
+                            form: {
+                                "text": d["text"],
+                                "output_context": d["output_context"],
+                            }
+                        }
+                    )
 
         # This code sets up translations for choices in filtered selects.
         for list_name, choice_list in self.choices.items():
             multi_language = isinstance(choice_list[0].get("label"), dict)
-            if not multi_language:
+            has_media = bool(choice_list[0].get("media"))
+            if not multi_language and not has_media:
                 continue
             for idx, choice in zip(range(len(choice_list)), choice_list):
                 for name, choice_value in choice.items():
-                    itext_id = "-".join(["static_instance", list_name, str(idx)])
+                    itext_id = "-".join([list_name, str(idx)])
                     if isinstance(choice_value, dict):
                         _setup_choice_translations(name, choice_value, itext_id)
                     elif name == "label":
@@ -651,14 +662,8 @@ class Survey(Section):
         {language : {element_xpath : {media_type : media}}}
         It matches the xform nesting order.
         """
-        if not self._translations:
-            self._translations = defaultdict(dict)  # pylint: disable=W0201
-
-        for survey_element in self.iter_descendants():
-
-            translation_key = survey_element.get_xpath() + ":label"
-            media_dict = survey_element.get("media")
 
+        def _set_up_media_translations(media_dict, translation_key):
             # This is probably papering over a real problem, but anyway,
             # in py3, sometimes if an item is on an xform with multiple
             # languages and the item only has media defined in # "default"
@@ -702,6 +707,19 @@ class Survey(Section):
 
                     translations_trans_key[media_type] = media
 
+        if not self._translations:
+            self._translations = defaultdict(dict)  # pylint: disable=W0201
+
+        for survey_element in self.iter_descendants():
+            # Skip set up of media for choices in filtered selects.
+            # Translations for the media content should have been set up
+            # in _setup_translations
+            parent = survey_element.get("parent")
+            if parent and not parent.get("choice_filter"):
+                translation_key = survey_element.get_xpath() + ":label"
+                media_dict = survey_element.get("media")
+                _set_up_media_translations(media_dict, translation_key)
+
     def itext(self):
         """
         This function creates the survey's itext nodes from _translations


Test patch:
diff --git a/pyxform/tests_v1/test_inline_translations.py b/pyxform/tests_v1/test_inline_translations.py
index 2c4fc47..3cd82a4 100644
--- a/pyxform/tests_v1/test_inline_translations.py
+++ b/pyxform/tests_v1/test_inline_translations.py
@@ -30,12 +30,12 @@ class InlineTranslationsTest(PyxformTestCase):
                 "<label>c</label>",
             ],
             model__excludes=[
-                '<text id="static_instance-states-0">',
-                '<text id="static_instance-states-1">',
-                '<text id="static_instance-states-2">',
-                "<itextId>static_instance-states-0</itextId>",
-                "<itextId>static_instance-states-1</itextId>",
-                "<itextId>static_instance-states-2</itextId>",
+                '<text id="states-0">',
+                '<text id="states-1">',
+                '<text id="states-2">',
+                "<itextId>states-0</itextId>",
+                "<itextId>states-1</itextId>",
+                "<itextId>states-2</itextId>",
             ],
             xml__contains=['<label ref="label"/>'],
             xml__excludes=['<label ref="jr:itext(itextId)"/>'],
@@ -59,12 +59,12 @@ class InlineTranslationsTest(PyxformTestCase):
             name="data",
             id_string="some-id",
             model__contains=[
-                '<text id="static_instance-states-0">',
-                '<text id="static_instance-states-1">',
-                '<text id="static_instance-states-2">',
-                "<itextId>static_instance-states-0</itextId>",
-                "<itextId>static_instance-states-1</itextId>",
-                "<itextId>static_instance-states-2</itextId>",
+                '<text id="states-0">',
+                '<text id="states-1">',
+                '<text id="states-2">',
+                "<itextId>states-0</itextId>",
+                "<itextId>states-1</itextId>",
+                "<itextId>states-2</itextId>",
             ],
             model__excludes=[
                 "<label>a</label>",
@@ -74,3 +74,75 @@ class InlineTranslationsTest(PyxformTestCase):
             xml__contains=['<label ref="jr:itext(itextId)"/>'],
             xml__excludes=['<label ref="label"/>'],
         )
+
+    def test_select_with_media_and_choice_filter_and_no_translations_generates_media(
+        self,
+    ):
+        """
+        Selects with media and choice filter should generate itext fields for the media.
+        """
+        xform_md = """
+        | survey |                    |                 |                                 |                           |
+        |        | type               | name            | label                           | choice_filter             |
+        |        | select_one consent | consent         | Would you like to participate ? |                           |
+        |        | select_one mood    | enumerator_mood | How are you feeling today ?     | selected(${consent}, 'y') |
+        | choices |
+        |         | list_name | name | label | media::image |
+        |         | mood      | h    | Happy | happy.jpg    |
+        |         | mood      | s    | Sad   | sad.jpg      |
+        |         | consent   | y    | Yes   |              |
+        |         | consent   | n    | No    |              |
+        """
+        self.assertPyxformXform(
+            name="data",
+            id_string="some-id",
+            md=xform_md,
+            errored=False,
+            debug=False,
+            model__contains=[
+                '<text id="mood-0">',
+                '<text id="mood-1">',
+                "<itextId>mood-0</itextId>",
+                "<itextId>mood-1</itextId>",
+            ],
+            model__excludes=[
+                '<text id="/data/enumerator_mood/h:label">',
+                '<text id="/data/enumerator_mood/s:label">',
+            ],
+            xml__contains=['<label ref="jr:itext(itextId)"/>'],
+            xml__excludes=['<label ref="label"/>'],
+        )
+
+    def test_select_with_choice_filter_and_translations_generates_single_translation(
+        self,
+    ):
+        """
+        Selects with choice filter and translations should only have a single itext entry.
+        """
+        xform_md = """
+        | survey |                    |      |       |               |
+        |        | type               | name | label | choice_filter |
+        |        | select_one list    | foo  | Foo   | name != "     |
+        | choices |
+        |         | list_name | name | label | image | label::French |
+        |         | list      | a    | A     | a.jpg | Ah            |
+        |         | list      | b    | B     | b.jpg | Bé            |
+        |         | list      | c    | C     | c.jpg | Cé            |
+        """
+        self.assertPyxformXform(
+            name="data",
+            id_string="some-id",
+            md=xform_md,
+            errored=False,
+            debug=False,
+            itext__contains=[
+                '<text id="list-0">',
+                '<text id="list-1">',
+                '<text id="list-2">',
+            ],
+            itext__excludes=[
+                '<text id="/data/foo/a:label">',
+                '<text id="/data/foo/b:label">',
+                '<text id="/data/foo/c:label">',
+            ],
+        )