Repository: XLSForm/pyxform (python)

Issue:
If constraint_message includes a ${} reference and no translations, expanded XPath path should be in <output>
#### Software and hardware versions 
pyxform v1.x.x, Python v1.2.0

#### Problem description
If a form only uses one language and has a `constraint_message` with a ${} reference, no `output` is generated. This is a side effect of https://github.com/XLSForm/pyxform/issues/285.

#### Steps to reproduce the problem
1. Build a single-language form with a constraint message that has a ${} reference in it:

type | name | label | constraint | constraint_message
-- | -- | -- | -- | --
select_one fruits | Q1 | Select the fruit that you like the most: |   |  
select_one fruits | Q2 | Select (again) the (same) fruit that you like the most (that   you selected in Q1): | .=${Q1} | You chose ${Q1} in Q1 but now it's different!

1. Convert it and see that the constraint message is set to `jr:constraintMsg="You chose  /data/Q1  in Q1 but now it's different!"`. This means the `/data/Q1` reference is not evaluated by clients.

#### Expected behavior
`/data/Q1` gets evaluated by clients.

#### Other information 
A form design workaround is to set a language for the `constraint_message` column to force the generation of an `itext` block. I think the only approach we could take is detect whether there's a `${}` reference in a `constraint_message` and if so generate `itext` for it.

Pull request:
Support ref in constraint and required messages with single language
Closes #474

#### Why is this the best possible solution? Were any other approaches considered?
Discussed approach in the issue and @MartijnR agreed identifying `${}` references and forcing itext creation in that case was our best bet.

In terms of implementation, I tried to reduce redundancy in this section but I couldn't come up with any approach that would be easy to read. I think this is ok because the conditions are linear and read well.

I considered adding tests for relative references in repeats but I think that's sufficiently covered elsewhere and should Just Work.

#### What are the regression risks?
Everything I added is in a conditional so I think risks are minimal. The worst risk is probably that I would have forgotten some cases.

#### 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.
No, this is a bug fix.

#### 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/survey.py b/pyxform/survey.py
index de0f5f9..6ba328c 100644
--- a/pyxform/survey.py
+++ b/pyxform/survey.py
@@ -7,13 +7,17 @@ from __future__ import print_function
 import codecs
 import os
 import re
-from re import split
 import tempfile
 import xml.etree.ElementTree as ETree
 from collections import defaultdict
 from datetime import datetime
 
 from pyxform import constants
+from pyxform.utils import (
+    BRACKETED_TAG_REGEX,
+    LAST_SAVED_REGEX,
+    LAST_SAVED_INSTANCE_NAME,
+)
 from pyxform.errors import PyXFormError, ValidationError
 from pyxform.external_instance import ExternalInstance
 from pyxform.instance import SurveyInstance
@@ -36,10 +40,6 @@ try:
 except ImportError:
     from functools32 import lru_cache
 
-LAST_SAVED_INSTANCE_NAME = "__last-saved"
-BRACKETED_TAG_REGEX = re.compile(r"\${(last-saved#)?(.*?)}")
-LAST_SAVED_REGEX = re.compile(r"\${last-saved#(.*?)}")
-
 
 def register_nsmap():
     """Function to register NSMAP namespaces with ETree"""
diff --git a/pyxform/survey_element.py b/pyxform/survey_element.py
index ac28c02..5e95960 100644
--- a/pyxform/survey_element.py
+++ b/pyxform/survey_element.py
@@ -10,10 +10,10 @@ from pyxform.errors import PyXFormError
 from pyxform.question_type_dictionary import QUESTION_TYPE_DICT
 from pyxform.utils import (
     INVALID_XFORM_TAG_REGEXP,
+    BRACKETED_TAG_REGEX,
     is_valid_xml_tag,
     node,
     unicode,
-    basestring,
     default_is_dynamic,
 )
 from pyxform.xls2json import print_pyobj_to_json
@@ -275,6 +275,14 @@ class SurveyElement(dict):
                         "text": text,
                         "output_context": self,
                     }
+            elif constraint_msg and re.search(BRACKETED_TAG_REGEX, constraint_msg):
+                yield {
+                    "path": self._translation_path("jr:constraintMsg"),
+                    "lang": default_language,
+                    "text": constraint_msg,
+                    "output_context": self,
+                }
+
             required_msg = bind_dict.get("jr:requiredMsg")
             if type(required_msg) is dict:
                 for lang, text in required_msg.items():
@@ -284,6 +292,13 @@ class SurveyElement(dict):
                         "text": text,
                         "output_context": self,
                     }
+            elif required_msg and re.search(BRACKETED_TAG_REGEX, required_msg):
+                yield {
+                    "path": self._translation_path("jr:requiredMsg"),
+                    "lang": default_language,
+                    "text": required_msg,
+                    "output_context": self,
+                }
             no_app_error_string = bind_dict.get("jr:noAppErrorString")
             if type(no_app_error_string) is dict:
                 for lang, text in no_app_error_string.items():
@@ -426,9 +441,13 @@ class SurveyElement(dict):
                     and k in self.CONVERTIBLE_BIND_ATTRIBUTES
                 ):
                     v = self.binding_conversions[v]
-                if k == "jr:constraintMsg" and type(v) is dict:
+                if k == "jr:constraintMsg" and (
+                    type(v) is dict or re.search(BRACKETED_TAG_REGEX, v)
+                ):
                     v = "jr:itext('%s')" % self._translation_path("jr:constraintMsg")
-                if k == "jr:requiredMsg" and type(v) is dict:
+                if k == "jr:requiredMsg" and (
+                    type(v) is dict or re.search(BRACKETED_TAG_REGEX, v)
+                ):
                     v = "jr:itext('%s')" % self._translation_path("jr:requiredMsg")
                 if k == "jr:noAppErrorString" and type(v) is dict:
                     v = "jr:itext('%s')" % self._translation_path("jr:noAppErrorString")
diff --git a/pyxform/utils.py b/pyxform/utils.py
index 7e553b3..03820a0 100644
--- a/pyxform/utils.py
+++ b/pyxform/utils.py
@@ -43,6 +43,10 @@ XFORM_TAG_REGEXP = "%(start)s%(char)s*" % {"start": TAG_START_CHAR, "char": TAG_
 
 INVALID_XFORM_TAG_REGEXP = r"[^a-zA-Z:_][^a-zA-Z:_0-9\-.]*"
 
+LAST_SAVED_INSTANCE_NAME = "__last-saved"
+BRACKETED_TAG_REGEX = re.compile(r"\${(last-saved#)?(.*?)}")
+LAST_SAVED_REGEX = re.compile(r"\${last-saved#(.*?)}")
+
 NSMAP = {
     "xmlns": "http://www.w3.org/2002/xforms",
     "xmlns:h": "http://www.w3.org/1999/xhtml",


Test patch:
diff --git a/pyxform/tests_v1/test_bind_conversions.py b/pyxform/tests_v1/test_bind_conversions.py
index 4b2bd99..6b5eec4 100644
--- a/pyxform/tests_v1/test_bind_conversions.py
+++ b/pyxform/tests_v1/test_bind_conversions.py
@@ -32,6 +32,21 @@ class BindConversionsTest(PyxformTestCase):
             xml__contains=['<bind nodeset="/data/text" required="false()"'],
         )
 
+    def test_bind_required_message_with_reference(self):
+        self.assertPyxformXform(
+            name="data",
+            md="""
+            | survey |       |      |       |            |                    |
+            |        | type  | name | label | required   | required_message |
+            |        | int   | foo  | foo   |            |                    |
+            |        | text  | text | text  | true()     | required, ${foo}   |
+            """,
+            xml__contains=[
+                '<bind nodeset="/data/text" required="true()" type="string" jr:requiredMsg="jr:itext(\'/data/text:jr:requiredMsg\')"',
+                '<value> required, <output value=" /data/foo "/> </value>',
+            ],
+        )
+
     def test_bind_constraint_conversion(self):
         self.assertPyxformXform(
             name="data",
@@ -45,6 +60,21 @@ class BindConversionsTest(PyxformTestCase):
             ],
         )
 
+    def test_bind_constraint_message_with_reference(self):
+        self.assertPyxformXform(
+            name="data",
+            md="""
+            | survey |       |      |       |                      |                    |
+            |        | type  | name | label | constraint           | constraint_message |
+            |        | int   | foo  | foo   |                      |                    |
+            |        | text  | text | text  | string-length(.) > 1 | too short ${foo}   |
+            """,
+            xml__contains=[
+                '<bind constraint="string-length(.) &gt; 1" nodeset="/data/text" type="string" jr:constraintMsg="jr:itext(\'/data/text:jr:constraintMsg\')"',
+                '<value> too short <output value=" /data/foo "/> </value>',
+            ],
+        )
+
     def test_bind_custom_conversion(self):
         self.assertPyxformXform(
             name="data",