Repository: XLSForm/pyxform (python)

Issue:
When indexed-repeat is used within a repeat the first argument nodeset points to single node using relative path
#### Software and hardware versions 
2.2.1

#### Problem description
The first argument of indexed-repeat() calls is relative when called from inside a repeat and referring to a question in that repeat. It should be absolute.

A `../relative/path/to/question` always returns a single node (in ODK XForms) and according to [the spec](https://getodk.github.io/xforms-spec/#fn:indexed-repeat) it should refer to all nodes with that question name, i.e. the nodeset returned by `/absolute/path/to/question`.

#### Steps to reproduce the problem

_A. using regular calculation with relative index:_

type | name | label | calculation
-- | -- | -- | --
begin_repeat | person | Person |  
integer | pos | | position(..) |
text | name | Enter name |  
text | prev_name | Name in previous repeat instance | indexed-repeat(${name}, ${person}, ${pos}-1)
end_repeat |   |   |  

outputs: 

```xml
<bind calculate="indexed-repeat( ../name ,  /data/person , ../pos -1)"  ... />
```

_B. using dynamic default:_

type | name | label | default
-- | -- | -- | --
begin_repeat | person | Person |  
text | name | Enter name |  
text | prev_name | Name in previous repeat instance | indexed-repeat(${name}, ${person}, position(..)-1)
end_repeat |   |   |  

outputs:

```xml
<setvalue ... value="indexed-repeat( ../name ,  /data/person , position(..)-1)" />
```

_C. using dreaded nested repeats:_


type | name | label | default
-- | -- | -- | --
begin_repeat | family | Family |  
integer | members_number | How many members in this family? |  
begin_repeat | person | Person |  
text | name | Enter name |  
text | prev_name | Non-sensible previous name in first family, 2nd person | indexed-repeat(${name}, ${family}, 1, ${person}, 2)
end_repeat |   |   |  
end_repeat |   |   |  

outputs:

```xml
<setvalue .. value="indexed-repeat( ../name ,  /data/family , 1,  ../../person , 2)" />
```

#### Expected behavior

A:

```xml
<bind calculate="indexed-repeat( /data/person/name ,  /data/person , ../pos - 1)" ... />
```

B: 
```xml
<setvalue ... value="indexed-repeat( /data/person/name ,  /data/person , position(..)-1)" />
```

C: 
```xml
<setvalue .. value="indexed-repeat( /data/family/person/name ,  /data/family , 1, /data/family/person , 2)" />
```

So the first argument of indexed-repeat() is an exception to the relative-path rule used to transform ${node} syntax (and the second argument too but that seems to already be the case).

Or actually since indexed-repeat() has either 3, 5, or 7 arguments, this exception applies to the 1st, 2nd, 4th, and 6th  arguments.

#### Other information 

Previously using indexed-repeat() inside a repeat was a bit of an edge case. Since the introduction of dynamic defaults it is becoming more common, as you could use the answer of a previous repeat question as the dynamic default of that same question in a new repeat instance (example B).

I'm guessing this issue was introduced in 0.12.0 when we changed to relative node references inside repeats.


Pull request:
When indexed-repeat is used within a repeat, 1st, 2nd, 4th, and 6th a…
…rgument not using relative path.

Closes #
Fix #484 
#### Why is this the best possible solution? Were any other approaches considered?
The one that works and passed tests.
#### What are the regression risks?
All tests passed, including new ones.
#### 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
#### 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..c56f8c1 100644
--- a/pyxform/survey.py
+++ b/pyxform/survey.py
@@ -877,26 +877,16 @@ class Survey(Section):
         Given a dictionary of xpaths, return a function we can use to
         replace ${varname} with the xpath to varname.
         """
+
         name = matchobj.group(2)
         last_saved = matchobj.group(1) is not None
-        intro = (
-            "There has been a problem trying to replace %s with the "
-            "XPath to the survey element named '%s'." % (matchobj.group(0), name)
-        )
-        if name not in self._xpath:
-            raise PyXFormError(intro + " There is no survey element with this name.")
-        if self._xpath[name] is None:
-            raise PyXFormError(
-                intro + " There are multiple survey elements" " with this name."
-            )
-        if (
-            not last_saved
-            and context
-            and not (
-                context["type"] == "calculate"
-                and "indexed-repeat" in context["bind"]["calculate"]
-            )
-        ):
+        is_indexed_repeat = matchobj.string.find("indexed-repeat(") > -1
+        indexed_repeat_regex = re.compile(r"indexed-repeat\([^)]+\)")
+        function_args_regex = re.compile(r"\b[^()]+\((.*)\)$")
+
+        def _relative_path(name):
+            """Given name in ${name}, return relative xpath to ${name}."""
+            return_path = None
             xpath, context_xpath = self._xpath[name], context.get_xpath()
             # share same root i.e repeat_a from /data/repeat_a/...
             if (
@@ -912,7 +902,81 @@ class Survey(Section):
                     ref_path = ref_path if ref_path.endswith(name) else "/%s" % name
                     prefix = " current()/" if use_current else " "
 
-                    return prefix + "/".join([".."] * steps) + ref_path + " "
+                    return_path = prefix + "/".join([".."] * steps) + ref_path + " "
+
+            return return_path
+
+        def _is_return_relative_path():
+            """Determine condition to return relative xpath of current ${name}."""
+            indexed_repeat_relative_path_args_index = [0, 1, 3, 5]
+            current_matchobj = matchobj
+
+            if not last_saved and context:
+                if context["type"] == "text":
+
+                    if not is_indexed_repeat:
+                        return True
+
+                    # It is possible to have multiple indexed-repeat in an expression
+                    indexed_repeats_iter = indexed_repeat_regex.finditer(
+                        matchobj.string
+                    )
+                    for indexed_repeat in indexed_repeats_iter:
+
+                        # Make sure current ${name} is in the correct indexed-repeat
+                        if current_matchobj.end() > indexed_repeat.end():
+                            try:
+                                next(indexed_repeats_iter)
+                                continue
+                            except StopIteration:
+                                return True
+
+                        # ${name} outside of indexed-repeat always using relative path
+                        if (
+                            current_matchobj.end() < indexed_repeat.start()
+                            or current_matchobj.start() > indexed_repeat.end()
+                        ):
+                            return True
+
+                        indexed_repeat_name_index = None
+                        indexed_repeat_args = (
+                            function_args_regex.match(indexed_repeat.group())
+                            .group(1)
+                            .split(",")
+                        )
+                        name_arg = "${{{0}}}".format(name)
+                        for idx, arg in enumerate(indexed_repeat_args):
+                            if name_arg in arg.strip():
+                                indexed_repeat_name_index = idx
+
+                        return (
+                            indexed_repeat_name_index is not None
+                            and indexed_repeat_name_index
+                            not in indexed_repeat_relative_path_args_index
+                        )
+                else:
+                    return not (
+                        context["type"] == "calculate"
+                        and "indexed-repeat" in context["bind"]["calculate"]
+                    )
+
+            return False
+
+        intro = (
+            "There has been a problem trying to replace %s with the "
+            "XPath to the survey element named '%s'." % (matchobj.group(0), name)
+        )
+        if name not in self._xpath:
+            raise PyXFormError(intro + " There is no survey element with this name.")
+        if self._xpath[name] is None:
+            raise PyXFormError(
+                intro + " There are multiple survey elements" " with this name."
+            )
+
+        if _is_return_relative_path():
+            relative_path = _relative_path(name)
+            if relative_path:
+                return relative_path
 
         last_saved_prefix = (
             "instance('" + LAST_SAVED_INSTANCE_NAME + "')" if last_saved else ""


Test patch:
diff --git a/pyxform/tests_v1/test_repeat.py b/pyxform/tests_v1/test_repeat.py
index 92f8089..dc100d1 100644
--- a/pyxform/tests_v1/test_repeat.py
+++ b/pyxform/tests_v1/test_repeat.py
@@ -458,3 +458,155 @@ class TestRepeat(PyxformTestCase):
                 '<itemset nodeset="../../household_mem_rep[ ./age  &gt;  current()/../target_min_age ]">',
             ],
         )
+
+    def test_indexed_repeat_regular_calculation_relative_path_exception(self):
+        """Test relative path exception (absolute path) in indexed-repeat() using regular calculation."""
+        self.assertPyxformXform(
+            name="data",
+            title="regular calculation indexed-repeat 1st, 2nd, 4th, and 6th argument is using absolute path",
+            md="""
+                | survey  |                |           |                                  |                                              |
+                |         | type           | name      | label                            | calculation                                  |
+                |         | begin_repeat   | person    | Person                           |                                              |
+                |         | integer        | pos       |                                  | position(..)                                 |
+                |         | text           | name      | Enter name                       |                                              |
+                |         | text           | prev_name | Name in previous repeat instance | indexed-repeat(${name}, ${person}, ${pos}-1) |
+                |         | end repeat     |           |                                  |                                              |
+            """,  # noqa pylint: disable=line-too-long
+            model__contains=[
+                """<bind calculate="indexed-repeat( /data/person/name ,  /data/person ,  ../pos -1)" nodeset="/data/person/prev_name" type="string"/>"""  # noqa pylint: disable=line-too-long
+            ],
+        )
+
+    def test_indexed_repeat_dynamic_default_relative_path_exception(self):
+        """Test relative path exception (absolute path) in indexed-repeat() using dynamic default."""
+        self.assertPyxformXform(
+            name="data",
+            title="dynamic default indexed-repeat 1st, 2nd, 4th, and 6th argument is using absolute path",
+            md="""
+                | survey  |                |           |                                  |                                                    |
+                |         | type           | name      | label                            | default                                            |
+                |         | begin_repeat   | person    | Person                           |                                                    |
+                |         | text           | name      | Enter name                       |                                                    |
+                |         | text           | prev_name | Name in previous repeat instance | indexed-repeat(${name}, ${person}, position(..)-1) |
+                |         | end repeat     |           |                                  |                                                    |
+            """,  # noqa pylint: disable=line-too-long
+            xml__contains=[
+                """<setvalue event="odk-instance-first-load odk-new-repeat" ref="/data/person/prev_name" value="indexed-repeat( /data/person/name ,  /data/person , position(..)-1)"/>"""  # noqa pylint: disable=line-too-long
+            ],
+        )
+
+    def test_indexed_repeat_nested_repeat_relative_path_exception(self):
+        """Test relative path exception (absolute path) in indexed-repeat() using nested repeat."""
+        self.assertPyxformXform(
+            name="data",
+            title="In nested repeat, indexed-repeat 1st, 2nd, 4th, and 6th argument is using absolute path",
+            md="""
+                | survey  |                |                |                                                        |                                                     |
+                |         | type           | name           | label                                                  | default                                             |
+                |         | begin_repeat   | family         | Family                                                 |                                                     |
+                |         | integer        | members_number | How many members in this family?                       |                                                     |
+                |         | begin_repeat   | person         | Person                                                 |                                                     |
+                |         | text           | name           | Enter name                                             |                                                     |
+                |         | text           | prev_name      | Non-sensible previous name in first family, 2nd person | indexed-repeat(${name}, ${family}, 1, ${person}, 2) |
+                |         | end repeat     |                |                                                        |                                                     |
+                |         | end repeat     |                |                                                        |                                                     |
+            """,  # noqa pylint: disable=line-too-long
+            xml__contains=[
+                """<setvalue event="odk-instance-first-load odk-new-repeat" ref="/data/family/person/prev_name" value="indexed-repeat( /data/family/person/name ,  /data/family , 1,  /data/family/person , 2)"/>"""  # noqa pylint: disable=line-too-long
+            ],
+        )
+
+    def test_indexed_repeat_math_epression_nested_repeat_relative_path_exception(self,):
+        """Test relative path exception (absolute path) in indexed-repeat() with math expression using nested repeat."""
+        self.assertPyxformXform(
+            name="data",
+            title="In nested repeat, indexed-repeat 1st, 2nd, 4th, and 6th argument is using absolute path",
+            md="""
+                | survey  |                |                |                                  |                                                        |
+                |         | type           | name           | label                            | calculation                                            |
+                |         | begin_repeat   | family         | Family                           |                                                        |
+                |         | integer        | members_number | How many members in this family? |                                                        |
+                |         | begin_repeat   | person         | Person                           |                                                        |
+                |         | text           | name           | Enter name                       |                                                        |
+                |         | integer        | age            | Enter age                        |                                                        |
+                |         | text           | prev_name      | Expression label                 | 7 * indexed-repeat(${age}, ${family}, 1, ${person}, 2) |
+                |         | end repeat     |                |                                  |                                                        |
+                |         | end repeat     |                |                                  |                                                        |
+            """,  # noqa pylint: disable=line-too-long
+            xml__contains=[
+                """<bind calculate="7 * indexed-repeat( /data/family/person/age ,  /data/family , 1,  /data/family/person , 2)" nodeset="/data/family/person/prev_name" type="string"/>"""  # noqa pylint: disable=line-too-long
+            ],
+        )
+
+    def test_multiple_indexed_repeat_in_epression_nested_repeat_relative_path_exception(
+        self,
+    ):
+        """Test relative path exception (absolute path) in multiple indexed-repeat() inside an expression using nested repeat."""
+        self.assertPyxformXform(
+            name="data",
+            title="In nested repeat, indexed-repeat 1st, 2nd, 4th, and 6th argument is using absolute path",
+            md="""
+                | survey  |                |                |                                  |                                                                                                                 |
+                |         | type           | name           | label                            | required                                                                                                        |
+                |         | begin_repeat   | family         | Family                           |                                                                                                                 |
+                |         | integer        | members_number | How many members in this family? |                                                                                                                 |
+                |         | begin_repeat   | person         | Person                           |                                                                                                                 |
+                |         | text           | name           | Enter name                       |                                                                                                                 |
+                |         | integer        | age            | Enter age                        |                                                                                                                 |
+                |         | text           | prev_name      | Expression label                 | concat(indexed-repeat(${name}, ${family}, 1, ${person}, 2), indexed-repeat(${age}, ${family}, 1, ${person}, 2)) |
+                |         | end repeat     |                |                                  |                                                                                                                 |
+                |         | end repeat     |                |                                  |                                                                                                                 |
+            """,  # noqa pylint: disable=line-too-long
+            xml__contains=[
+                """<bind nodeset="/data/family/person/prev_name" required="concat(indexed-repeat( /data/family/person/name ,  /data/family , 1,  /data/family/person , 2), indexed-repeat( /data/family/person/age ,  /data/family , 1,  /data/family/person , 2))" type="string"/>"""  # noqa pylint: disable=line-too-long
+            ],
+        )
+
+    def test_mixed_variables_and_indexed_repeat_in_epression_nested_repeat_relative_path_exception(
+        self,
+    ):
+        """Test relative path exception (absolute path) in an expression contains variables and indexed-repeat() using nested repeat."""
+        self.assertPyxformXform(
+            name="data",
+            title="In nested repeat, indexed-repeat 1st, 2nd, 4th, and 6th argument is using absolute path",
+            md="""
+                | survey  |                |                |                                  |                                                                                                                 |
+                |         | type           | name           | label                            | calculation                                                                                                        |
+                |         | begin_repeat   | family         | Family                           |                                                                                                                 |
+                |         | integer        | members_number | How many members in this family? |                                                                                                                 |
+                |         | begin_repeat   | person         | Person                           |                                                                                                                 |
+                |         | text           | name           | Enter name                       |                                                                                                                 |
+                |         | integer        | age            | Enter age                        |                                                                                                                 |
+                |         | text           | prev_name      | Expression label                 | concat(${name}, indexed-repeat(${age}, ${family}, 1, ${person}, 2), ${age})                                     |
+                |         | end repeat     |                |                                  |                                                                                                                 |
+                |         | end repeat     |                |                                  |                                                                                                                 |
+            """,  # noqa pylint: disable=line-too-long
+            xml__contains=[
+                """<bind calculate="concat( ../name , indexed-repeat( /data/family/person/age ,  /data/family , 1,  /data/family/person , 2),  ../age )" nodeset="/data/family/person/prev_name" type="string"/>"""  # noqa pylint: disable=line-too-long
+            ],
+        )
+
+    def test_indexed_repeat_math_epression_with_double_variable_in_nested_repeat_relative_path_exception(
+        self,
+    ):
+        """Test relative path exception (absolute path) in indexed-repeat() with math expression and double variable using nested repeat."""
+        self.assertPyxformXform(
+            name="data",
+            title="In nested repeat, indexed-repeat 1st, 2nd, 4th, and 6th argument is using absolute path",
+            md="""
+                | survey  |                |                |                                  |                                                             |
+                |         | type           | name           | label                            | relevant                                                    |
+                |         | begin_repeat   | family         | Family                           |                                                             |
+                |         | integer        | members_number | How many members in this family? |                                                             |
+                |         | begin_repeat   | person         | Person                           |                                                             |
+                |         | text           | name           | Enter name                       |                                                             |
+                |         | integer        | age            | Enter age                        |                                                             |
+                |         | text           | prev_name      | Expression label                 | ${age} > indexed-repeat(${age}, ${family}, 1, ${person}, 2) |
+                |         | end repeat     |                |                                  |                                                             |
+                |         | end repeat     |                |                                  |                                                             |
+            """,  # noqa pylint: disable=line-too-long
+            xml__contains=[
+                """<bind nodeset="/data/family/person/prev_name" relevant=" ../age  &gt; indexed-repeat( /data/family/person/age ,  /data/family , 1,  /data/family/person , 2)" type="string"/>"""  # noqa pylint: disable=line-too-long
+            ],
+        )