Repository: XLSForm/pyxform
Pull request #450 (2020-06-10 10:22:21)

Fixed <output> inside repeat has absolute path instead of relative
#446 


Patch:
diff --git a/pyxform/question.py b/pyxform/question.py
index 02e7f19..c1633e1 100644
--- a/pyxform/question.py
+++ b/pyxform/question.py
@@ -32,16 +32,6 @@ class Question(SurveyElement):
 
     def xml_control(self):
         if ("calculate" in self.bind or self.trigger) and not (self.label or self.hint):
-            nested_setvalues = self.get_root().get_setvalues_for_question_name(
-                self.name
-            )
-            if nested_setvalues:
-                for setvalue in nested_setvalues:
-                    msg = (
-                        "Trigger was added for ${%s} that refers to hidden question. This is not allowed."
-                        % setvalue[0]
-                    )
-                    raise PyXFormError(msg)
             return None
 
         xml_node = self.build_xml()
diff --git a/pyxform/survey.py b/pyxform/survey.py
index 86245fc..f4c416d 100644
--- a/pyxform/survey.py
+++ b/pyxform/survey.py
@@ -591,19 +591,21 @@ 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"]:
-                    hint_path = d["path"].replace("guidance_hint", "hint")
-                    self._translations[d["lang"]][hint_path] = self._translations[
-                        d["lang"]
-                    ].get(hint_path, {})
-                    self._translations[d["lang"]][hint_path].update(
-                        {"guidance": d["text"]}
-                    )
-                else:
-                    self._translations[d["lang"]][d["path"]] = self._translations[
-                        d["lang"]
-                    ].get(d["path"], {})
-                    self._translations[d["lang"]][d["path"]].update({"long": d["text"]})
+                    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():
@@ -722,12 +724,14 @@ class Survey(Section):
                     raise Exception()
 
                 for media_type, media_value in content.items():
-                    # There is a odk/jr bug where hints can't have a value
-                    # for the "form" attribute.
-                    # This is my workaround.
-                    if label_type == "hint":
+                    if isinstance(media_value, dict):
+                        value, output_inserted = self.insert_output_values(
+                            media_value["text"], context=media_value["output_context"]
+                        )
+                    else:
                         value, output_inserted = self.insert_output_values(media_value)
 
+                    if label_type == "hint":
                         if media_type == "guidance":
                             itext_nodes.append(
                                 node(
@@ -744,14 +748,12 @@ class Survey(Section):
                         continue
 
                     if media_type == "long":
-                        value, output_inserted = self.insert_output_values(media_value)
                         # I'm ignoring long types for now because I don't know
                         # how they are supposed to work.
                         itext_nodes.append(
                             node("value", value, toParseString=output_inserted)
                         )
                     elif media_type == "image":
-                        value, output_inserted = self.insert_output_values(media_value)
                         if value != "-":
                             itext_nodes.append(
                                 node(
@@ -762,7 +764,6 @@ class Survey(Section):
                                 )
                             )
                     else:
-                        value, output_inserted = self.insert_output_values(media_value)
                         if value != "-":
                             itext_nodes.append(
                                 node(
diff --git a/pyxform/survey_element.py b/pyxform/survey_element.py
index 8d671f8..ac28c02 100644
--- a/pyxform/survey_element.py
+++ b/pyxform/survey_element.py
@@ -273,6 +273,7 @@ class SurveyElement(dict):
                         "path": self._translation_path("jr:constraintMsg"),
                         "lang": lang,
                         "text": text,
+                        "output_context": self,
                     }
             required_msg = bind_dict.get("jr:requiredMsg")
             if type(required_msg) is dict:
@@ -281,6 +282,7 @@ class SurveyElement(dict):
                         "path": self._translation_path("jr:requiredMsg"),
                         "lang": lang,
                         "text": text,
+                        "output_context": self,
                     }
             no_app_error_string = bind_dict.get("jr:noAppErrorString")
             if type(no_app_error_string) is dict:
@@ -289,6 +291,7 @@ class SurveyElement(dict):
                         "path": self._translation_path("jr:noAppErrorString"),
                         "lang": lang,
                         "text": text,
+                        "output_context": self,
                     }
 
         for display_element in ["label", "hint", "guidance_hint"]:
@@ -327,6 +330,7 @@ class SurveyElement(dict):
                         "display_element": display_element,  # Not used
                         "path": self._translation_path(display_element),
                         "element": self,  # Not used
+                        "output_context": self,
                         "lang": lang,
                         "text": text,
                     }
diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py
index e372e40..a7b273d 100644
--- a/pyxform/xls2json.py
+++ b/pyxform/xls2json.py
@@ -819,10 +819,7 @@ def workbook_to_json(
 
         if question_type == "calculate":
             calculation = row.get("bind", {}).get("calculate")
-            question_default = row.get("default")
-            if not calculation and not (
-                question_default and default_is_dynamic(question_default, question_type)
-            ):
+            if not calculation:
                 raise PyXFormError(
                     row_format_string % row_number + " Missing calculation."
                 )
@@ -886,11 +883,6 @@ def workbook_to_json(
             constants.LABEL not in row
             and row.get(constants.MEDIA) is None
             and question_type not in aliases.label_optional_types
-            and not row.get("bind", {}).get("calculate")
-            and not (
-                row.get("default")
-                and default_is_dynamic(row.get("default"), question_type)
-            )
         ):
             # TODO: Should there be a default label?
             #      Not sure if we should throw warnings for groups...


Test patch:
diff --git a/pyxform/tests_v1/test_repeat.py b/pyxform/tests_v1/test_repeat.py
index e6bedc1..bbade9b 100644
--- a/pyxform/tests_v1/test_repeat.py
+++ b/pyxform/tests_v1/test_repeat.py
@@ -187,6 +187,70 @@ class TestRepeat(PyxformTestCase):
             ],
         )
 
+    def test_output_with_translation_relative_path(self):
+        md = """
+        | survey |              |             |                |              |               |                             |                           |                           |
+        |        | type         | name        | label::English | calculation  | hint::English | constraint_message::English | required_message::English | noAppErrorString::English |
+        |        | begin repeat | member      |                |              |               |                             |                           |                           |
+        |        | calculate    | pos         |                | position(..) |               |                             |                           |                           |
+        |        | text         | member_name | Name of ${pos} |              |               |                             |                           |                           |
+        |        | text         | a           | A              |              | hint ${pos}   | constraint ${pos}           | required ${pos}           | app error ${pos}          |
+        |        | end repeat   |             |                |              |               |                             |                           |                           |
+        """
+
+        self.assertPyxformXform(
+            md=md,
+            name="inside-repeat-relative-path",
+            xml__contains=[
+                '<translation lang="English">',
+                '<value> Name of <output value=" ../pos "/> </value>',
+                '<value> hint <output value=" ../pos "/> </value>',
+                '<value> constraint <output value=" ../pos "/> </value>',
+                '<value> required <output value=" ../pos "/> </value>',
+                '<value> app error <output value=" ../pos "/>',
+            ],
+        )
+
+    def test_output_with_guidance_hint_translation_relative_path(self):
+        md = """
+        | survey |              |             |                |                        |              |
+        |        | type         | name        | label::English | guidance_hint::English | calculation  |
+        |        | begin repeat | member      |                |                        |              |
+        |        | calculate    | pos         |                |                        | position(..) |
+        |        | text         | member_name | Name of ${pos} | More ${pos}            |              |
+        |        | end repeat   |             |                |                        |              |
+        """
+
+        self.assertPyxformXform(
+            md=md,
+            name="inside-repeat-relative-path",
+            xml__contains=[
+                '<translation lang="English">',
+                '<value> Name of <output value=" ../pos "/> </value>',
+                '<value form="guidance"> More <output value=" ../pos "/> </value>',
+            ],
+        )
+
+    def test_output_with_multiple_translations_relative_path(self):
+        md = """
+        | survey |              |                |                |                  |              | 
+        |        | type         | name           | label::English | label::Indonesia | calculation  |
+        |        | begin repeat | member         |                |                  |              |
+        |        | calculate    | pos            |                |                  | position(..) | 
+        |        | text         | member_name    | Name of ${pos} | Nama ${pos}      |              |
+        |        | text         | member_address |                | Alamat           |              |
+        |        | end repeat   |                |                |                  |              | 
+        """
+
+        self.assertPyxformXform(
+            md=md,
+            name="inside-repeat-relative-path",
+            xml__contains=[
+                '<translation lang="English">',
+                '<value> Name of <output value=" ../pos "/> </value>',
+            ],
+        )
+
     def test_hints_are_not_present_within_repeats(self):
         """Hints are not present within repeats"""
         md = """
diff --git a/pyxform/tests_v1/test_trigger.py b/pyxform/tests_v1/test_trigger.py
index 6da047f..70155be 100644
--- a/pyxform/tests_v1/test_trigger.py
+++ b/pyxform/tests_v1/test_trigger.py
@@ -158,22 +158,6 @@ class TriggerSetvalueTests(PyxformTestCase):
             ],
         )
 
-    def test_trigger_refer_to_hidden_node_ref_gives_error(self):
-        self.assertPyxformXform(
-            name="trigger-invalid-ref",
-            md="""
-            | survey |           |        |                      |         |             |
-            |        | type      | name   | label                | trigger | calculation |
-            |        | calculate | one    |                      |         | 5 + 4       |
-            |        | calculate | one-ts |                      | ${one}  | now()       |
-            |        | note      | note   | timestamp: ${one-ts} |         |             |
-            """,
-            errored=True,
-            error__contains=[
-                "Trigger was added for ${one-ts} that refers to hidden question. This is not allowed."
-            ],
-        )
-
     def test_handling_trigger_column_in_group(self):
         md = """
         | survey |             |      |                    |             |         |
diff --git a/pyxform/tests_v1/test_typed_calculates.py b/pyxform/tests_v1/test_typed_calculates.py
index fbded33..a58ecd6 100644
--- a/pyxform/tests_v1/test_typed_calculates.py
+++ b/pyxform/tests_v1/test_typed_calculates.py
@@ -60,72 +60,6 @@ class TypedCalculatesTest(PyxformTestCase):
             xml__excludes=["input"],
         )
 
-    def test_non_calculate_type_with_calculation_no_warns(self):
-        warnings = []
-
-        self.md_to_pyxform_survey(
-            """
-            | survey |           |      |             |      |             |
-            |        | type      | name | label       | hint | calculation |
-            |        | dateTime  | a    |             |      | now()       |
-            |        | integer   | b    |             |      | 1 div 1     |
-            |        | note      | note | Hello World |      |             |
-            """,
-            warnings=warnings,
-        )
-
-        self.assertTrue(len(warnings) == 0)
-
-    def test_non_calculate_type_with_hint_and_no_calculation_warns(self):
-        warnings = []
-
-        self.md_to_pyxform_survey(
-            """
-            | survey |           |      |             |           |             |
-            |        | type      | name | label       | hint      | calculation |
-            |        | dateTime  | a    |             |           | now()       |
-            |        | integer   | b    |             | Some hint |             |
-            |        | note      | note | Hello World |           |             |
-            """,
-            warnings=warnings,
-        )
-
-        self.assertTrue(len(warnings) == 1)
-        self.assertTrue("Question has no label" in warnings[0])
-
-    def test_non_calculate_type_with_calculation_and_dynamic_default_warns(self):
-        warnings = []
-
-        self.md_to_pyxform_survey(
-            """
-            | survey |           |      |             |      |             |         |
-            |        | type      | name | label       | hint | calculation | default |
-            |        | dateTime  | a    |             |      | now()       |         |
-            |        | integer   | b    |             |      | 1 div 1     | $(a)    |
-            |        | note      | note | Hello World |      |             |         |
-            """,
-            warnings=warnings,
-        )
-
-        self.assertTrue(len(warnings) == 1)
-        self.assertTrue("This form definition contains dynamic defaults" in warnings[0])
-
-    def test_non_calculate_type_with_calculation_and_default_no_warns(self):
-        warnings = []
-
-        self.md_to_pyxform_survey(
-            """
-            | survey |           |      |             |      |             |         |
-            |        | type      | name | label       | hint | calculation | default |
-            |        | dateTime  | a    |             |      | now()       |         |
-            |        | integer   | b    |             |      | 1 div 1     | 1       |
-            |        | note      | note | Hello World |      |             |         |
-            """,
-            warnings=warnings,
-        )
-
-        self.assertTrue(len(warnings) == 0)
-
     def test_select_type_with_calculation_and_no_label_has_no_control(self):
         self.assertPyxformXform(
             name="calculate-select",
@@ -156,39 +90,3 @@ class TypedCalculatesTest(PyxformTestCase):
             errored=True,
             error__contains="The survey element named 'a' has no label or hint.",
         )
-
-    def test_calculate_without_calculation_without_default(self):
-        self.assertPyxformXform(
-            name="calculate-without-calculation-without-default",
-            md="""
-        | survey |            |      |             |             |         |
-        |        | type       | name | label       | calculation | default |
-        |        | calculate  | a    |             |             |         |
-        """,
-            errored=True,
-            error__contains="Missing calculation",
-        )
-
-    def test_calculate_without_calculation_with_default_without_dynamic_default(self):
-        self.assertPyxformXform(
-            name="calculate-without-calculation-with-default-without-dynamic-default",
-            md="""
-        | survey |            |      |             |             |         |
-        |        | type       | name | label       | calculation | default |
-        |        | calculate  | a    |             |             | foo     |
-        """,
-            errored=True,
-            error__contains="Missing calculation",
-        )
-
-    def test_calculate_without_calculation_with_dynamic_default(self):
-        self.assertPyxformXform(
-            name="calculate-without-calculation-with-dynamic-default",
-            md="""
-        | survey |            |      |             |             |          |
-        |        | type       | name | label       | calculation | default  |
-        |        | calculate  | a    |             |             | random() |
-        """,
-            errored=False,
-            instance__contains=["<a/>"],
-        )