Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for choice from previous repeat answers #472

Merged
merged 16 commits into from
Nov 6, 2020
Merged

Add support for choice from previous repeat answers #472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
01412df
Add support for choice from previous repeat answers
ukanga Oct 2, 2019
4ca43f3
Test choices from previous repeat answers with choice filter
DavisRayM Sep 22, 2020
ba03680
Add support for choices from previous repeat answers with choice filter
DavisRayM Sep 24, 2020
41c36ad
Update default predicate for choices from previous repeat answers
DavisRayM Sep 24, 2020
206f6ab
Test choices from previous repeat answers in nested repeat
DavisRayM Sep 24, 2020
a635a5a
Set default value for 'multi_language' variable
DavisRayM Sep 28, 2020
aa7d9b5
Test that a relative nodeset is generated for select from previous
DavisRayM Oct 17, 2020
9cd3825
Test that choice from previous answers in child repeat work
DavisRayM Oct 17, 2020
3390a3b
Test that current is used when select from previous question is a sib…
DavisRayM Oct 17, 2020
aff5ba7
Check if xpath_parent and context_parent share a common repeat ancestor
DavisRayM Oct 23, 2020
4d719a0
Replace nodeset if within choice_filter
DavisRayM Oct 23, 2020
2d020ba
Update choice from previous repeat answers with choice_filter test case
DavisRayM Oct 24, 2020
8eb48d0
Populate itemset instead of list_name value
DavisRayM Oct 28, 2020
37d0312
Disable "run_odk_validate" functionality for select from previous tests
DavisRayM Oct 28, 2020
4aebb2c
Reference shared parent if context is a child of the target xpath parent
DavisRayM Oct 28, 2020
32b17c7
Remove Validate run
DavisRayM Oct 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions pyxform/question.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
XForm Survey element classes for different question types.
"""
import os.path
import re

from pyxform.errors import PyXFormError
from pyxform.question_type_dictionary import QUESTION_TYPE_DICT
Expand Down Expand Up @@ -186,6 +187,7 @@ def build_xml(self):
result.appendChild(element)

choices = survey.get("choices")
multi_language = False
if choices is not None and len(choices) > 0:
first_choices = next(iter(choices.values()))
multi_language = isinstance(first_choices[0].get("label"), dict)
Expand All @@ -196,6 +198,7 @@ def build_xml(self):
choice_filter = self.get("choice_filter")
itemset, file_extension = os.path.splitext(self["itemset"])
has_media = False
is_previous_question = bool(re.match(r"^\${.*}$", self.get("itemset")))

if choices.get(itemset):
has_media = bool(choices[itemset][0].get("media"))
Expand All @@ -210,8 +213,28 @@ def build_xml(self):
else:
itemset = self["itemset"]
itemset_label_ref = "jr:itext(itextId)"
nodeset = "instance('" + itemset + "')/root/item"
choice_filter = survey.insert_xpaths(choice_filter, self, True)

choice_filter = survey.insert_xpaths(choice_filter, self, True, True)
if is_previous_question:
path = (
survey.insert_xpaths(self["itemset"], self, reference_parent=True)
.strip()
.split("/")
)
nodeset = "/".join(path[:-1])
itemset_label_ref = path[-1]
if choice_filter:
choice_filter = choice_filter.replace(
"current()/" + nodeset, "."
).replace(nodeset, ".")
else:
# Choices must have a value. Filter out repeat instances without
# an answer for the linked question
name = path[-1]
choice_filter = f"./{name} != ''"
else:
nodeset = "instance('" + itemset + "')/root/item"

if choice_filter:
nodeset += "[" + choice_filter + "]"

Expand Down Expand Up @@ -239,8 +262,9 @@ def build_xml(self):
]
result.appendChild(node("itemset", *itemset_children, nodeset=nodeset))
else:
for n in [o.xml() for o in self.children]:
result.appendChild(n)
for child in self.children:
result.appendChild(child.xml())

return result


Expand Down
64 changes: 52 additions & 12 deletions pyxform/survey.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import codecs
import os
import re
from re import split
import tempfile
import xml.etree.ElementTree as ETree
from collections import defaultdict
Expand Down Expand Up @@ -67,7 +68,7 @@ def is_parent_a_repeat(survey, xpath):


@lru_cache(maxsize=None)
def share_same_repeat_parent(survey, xpath, context_xpath):
def share_same_repeat_parent(survey, xpath, context_xpath, reference_parent=False):
"""
Returns a tuple of the number of steps from the context xpath to the shared
repeat parent and the xpath to the target xpath from the shared repeat
Expand All @@ -79,14 +80,20 @@ def share_same_repeat_parent(survey, xpath, context_xpath):

returns (2, '/group_a/name')'
"""
context_parent = is_parent_a_repeat(survey, context_xpath)
xpath_parent = is_parent_a_repeat(survey, xpath)
if context_parent and xpath_parent and xpath_parent in context_parent:
context_parts = context_xpath[len(xpath_parent) + 1 :].split("/")

def _get_steps_and_target_xpath(context_parent, xpath_parent, include_parent=False):
parts = []
steps = 1
remainder_xpath = xpath[len(xpath_parent) :]
xpath_parts = xpath[len(xpath_parent) + 1 :].split("/")
if not include_parent:
remainder_xpath = xpath[len(xpath_parent) :]
context_parts = context_xpath[len(xpath_parent) + 1 :].split("/")
xpath_parts = xpath[len(xpath_parent) + 1 :].split("/")
else:
split_idx = len(xpath_parent.split("/"))
context_parts = context_xpath.split("/")[split_idx - 1 :]
xpath_parts = xpath.split("/")[split_idx - 1 :]
remainder_xpath = "/".join(xpath_parts)

for index, item in enumerate(context_parts[:-1]):
try:
if xpath[len(context_parent) + 1 :].split("/")[index] != item:
Expand All @@ -99,9 +106,36 @@ def share_same_repeat_parent(survey, xpath, context_xpath):
steps = len(context_parts[index - 1 :])
parts = xpath_parts[index - 1 :]
break

return (steps, "/" + "/".join(parts) if parts else remainder_xpath)

context_parent = is_parent_a_repeat(survey, context_xpath)
lognaturel marked this conversation as resolved.
Show resolved Hide resolved
xpath_parent = is_parent_a_repeat(survey, xpath)
if context_parent and xpath_parent and xpath_parent in context_parent:
include_parent = False
if not context_parent == xpath_parent and reference_parent:
context_shared_ancestor = is_parent_a_repeat(survey, context_parent)
if context_shared_ancestor == xpath_parent:
# Check if context_parent is a child repeat of the xpath_parent
# If the context_parent is a child of the xpath_parent reference the entire
# xpath_parent in the generated nodeset
include_parent = True
context_parent = context_shared_ancestor
return _get_steps_and_target_xpath(context_parent, xpath_parent, include_parent)
elif context_parent and xpath_parent:
# Check if context_parent and xpath_parent share a common
# repeat ancestor
context_shared_ancestor = is_parent_a_repeat(survey, context_parent)
xpath_shared_ancestor = is_parent_a_repeat(survey, xpath_parent)

if (
xpath_shared_ancestor
and context_shared_ancestor
and xpath_shared_ancestor == context_shared_ancestor
):
return _get_steps_and_target_xpath(
context_shared_ancestor, xpath_shared_ancestor
)

return (None, None)


Expand Down Expand Up @@ -836,7 +870,9 @@ def _setup_xpath_dictionary(self):
else:
self._xpath[element.name] = element.get_xpath()

def _var_repl_function(self, matchobj, context, use_current=False):
def _var_repl_function(
self, matchobj, context, use_current=False, reference_parent=False
):
"""
Given a dictionary of xpaths, return a function we can use to
replace ${varname} with the xpath to varname.
Expand Down Expand Up @@ -869,7 +905,9 @@ def _var_repl_function(self, matchobj, context, use_current=False):
):
# if context xpath and target xpath fall under the same
# repeat use relative xpath referencing.
steps, ref_path = share_same_repeat_parent(self, xpath, context_xpath)
steps, ref_path = share_same_repeat_parent(
self, xpath, context_xpath, reference_parent
)
if steps:
ref_path = ref_path if ref_path.endswith(name) else "/%s" % name
prefix = " current()/" if use_current else " "
Expand All @@ -881,13 +919,15 @@ def _var_repl_function(self, matchobj, context, use_current=False):
)
return " " + last_saved_prefix + self._xpath[name] + " "

def insert_xpaths(self, text, context, use_current=False):
def insert_xpaths(self, text, context, use_current=False, reference_parent=False):
"""
Replace all instances of ${var} with the xpath to var.
"""

def _var_repl_function(matchobj):
return self._var_repl_function(matchobj, context, use_current)
return self._var_repl_function(
matchobj, context, use_current, reference_parent
)

return re.sub(BRACKETED_TAG_REGEX, _var_repl_function, unicode(text))

Expand Down
134 changes: 131 additions & 3 deletions pyxform/tests_v1/test_repeat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,13 +233,13 @@ def test_output_with_guidance_hint_translation_relative_path(self):

def test_output_with_multiple_translations_relative_path(self):
md = """
| survey | | | | | |
| survey | | | | | |
| | type | name | label::English | label::Indonesia | calculation |
| | begin repeat | member | | | |
| | calculate | pos | | | position(..) |
| | calculate | pos | | | position(..) |
| | text | member_name | Name of ${pos} | Nama ${pos} | |
| | text | member_address | | Alamat | |
| | end repeat | | | | |
| | end repeat | | | | |
"""

self.assertPyxformXform(
Expand Down Expand Up @@ -330,3 +330,131 @@ def test_hints_are_present_within_groups(self):
</group>""" # noqa

self.assertPyxformXform(md=md, xml__contains=[expected], run_odk_validate=True)

def test_choice_from_previous_repeat_answers(self):
"""Select one choices from previous repeat answers."""
xlsform_md = """
| survey | | | |
| | type | name | label |
| | begin repeat | rep | Repeat |
| | text | name | Enter name |
| | end repeat | | |
| | select one fruits | fruit | Choose a fruit |
| | select one ${name} | choice | Choose name |
| choices | | | |
| | list name | name | label |
| | fruits | banana | Banana |
| | fruits | mango | Mango |
"""
self.assertPyxformXform(
md=xlsform_md,
xml__contains=[
"<itemset nodeset=\"/pyxform_autotestname/rep[./name != '']\">"
],
run_odk_validate=False,
)

def test_choice_from_previous_repeat_answers_with_choice_filter(self):
"""Select one choices from previous repeat answers with choice filter"""
xlsform_md = """
| survey | | | | |
| | type | name | label | choice_filter |
| | begin repeat | rep | Repeat | |
| | text | name | Enter name | |
| | begin group | demographics | Demographics | |
| | integer | age | Enter age | |
| | end group | demographics | | |
| | end repeat | | | |
| | select one fruits | fruit | Choose a fruit | |
| | select one ${name} | choice | Choose name | starts-with(${name}, "b") |
| | select one ${name} | choice_18_over | Choose name | ${age} > 18 |
| choices | | | | |
| | list name | name | label | |
| | fruits | banana | Banana | |
| | fruits | mango | Mango | |
"""
self.assertPyxformXform(
name="data",
id_string="some-id",
md=xlsform_md,
xml__contains=[
'<itemset nodeset="/data/rep[starts-with( ./name , &quot;b&quot;)]">',
'<itemset nodeset="/data/rep[ ./demographics/age &gt; 18]">',
],
run_odk_validate=False,
)

def test_choice_from_previous_repeat_answers_in_child_repeat(self):
"""
Select one choice from previous repeat answers when within a child of a repeat
"""
xlsform_md = """
| survey | | | | |
| | type | name | label | choice_filter |
| | begin repeat | household | Household Repeat | |
| | begin repeat | member | Household member repeat | |
| | text | name | Enter name of a household member | |
| | integer | age | Enter age of the household member | |
| | begin repeat | adult | Select a representative | |
| | select one ${name} | adult_name | Choose a name | ${age} > 18 |
| | end repeat | adult | | |
| | end repeat | member | | |
| | end repeat | household | | |
"""
self.assertPyxformXform(
name="data",
id_string="some-id",
md=xlsform_md,
xml__contains=['<itemset nodeset="../../../member[ ./age &gt; 18]">',],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is tricky indeed. The rule is that if you want to refer to an ancestor repeat, you need to parent above it and include its name. I'm not sure where the best place for that is and let's talk if you get stuck.

Copy link
Contributor

@lognaturel lognaturel Oct 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the changes at https://github.com/XLSForm/pyxform/pull/472/files#r511057165 to address this case? I think basically you'll need to address #453. Does that sound right, @MartijnR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this was kicking around my brain and I believe https://github.com/XLSForm/pyxform/pull/472/files#discussion_r511920303 addresses the case in #453. The case that's now missing is referencing a parent repeat. That is, a reference with a relative terminal is being generated (../../) when the parent repeat name needs to be included (../../../member).

Copy link
Contributor Author

@DavisRayM DavisRayM Oct 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made a few changes in the latest commits; Now checking if the context_parent is a child of the xpath_path and only referencing the parent repeat when this is the case

)

def test_choice_from_previous_repeat_answers_in_nested_repeat(self):
"""Select one choices from previous repeat answers within a nested repeat"""
xlsform_md = """
| survey | | | | |
| | type | name | label | choice_filter |
| | begin repeat | household | Household Repeat | |
| | begin repeat | person | Household member repeat | |
| | text | name | Enter name of a household member | |
| | integer | age | Enter age of the household member | |
| | end repeat | person | | |
| | begin repeat | adult | Select a representative | |
| | select one ${name} | adult_name | Choose a name | ${age} > 18 |
| | end repeat | adult | | |
| | end repeat | household | | |
"""
self.assertPyxformXform(
name="data",
id_string="some-id",
md=xlsform_md,
xml__contains=['<itemset nodeset="../../person[ ./age &gt; 18]">',],
)

def test_choice_from_previous_repeat_answers_in_nested_repeat_uses_current(self):
"""
Select one choices from previous repeat answers within a nested repeat should use current if a sibling node of a select is used
"""
xlsform_md = """
| survey | | | | |
| | type | name | label | choice_filter |
| | text | enumerators_name | Enter enumerators name | |
| | begin repeat | household_rep | Household Repeat | |
| | integer | household_id | Enter household ID | |
| | begin repeat | household_mem_rep | Household member repeat | |
| | text | name | Enter name of a household member | |
| | integer | age | Enter age of the household member | |
| | end repeat | household_mem_rep | | |
| | begin repeat | selected | Select a representative | |
| | integer | target_min_age | Minimum age requirement | |
| | select one ${name} | selected_name | Choose a name | ${age} > ${target_min_age} |
| | end repeat | selected | | |
| | end repeat | household_rep | | |
"""
self.assertPyxformXform(
name="data",
id_string="some-id",
md=xlsform_md,
xml__contains=[
'<itemset nodeset="../../household_mem_rep[ ./age &gt; current()/../target_min_age ]">',
],
)
Loading