-
Notifications
You must be signed in to change notification settings - Fork 136
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.
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
Changes from 11 commits
01412df
4ca43f3
ba03680
41c36ad
206f6ab
a635a5a
aa7d9b5
9cd3825
3390a3b
aff5ba7
4d719a0
2d020ba
8eb48d0
37d0312
4aebb2c
32b17c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -330,3 +330,133 @@ 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=True, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great that you tried all with Validate. We typically turn this flag off for the test suite because it takes time. |
||
) | ||
|
||
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") | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To make this more realistic, use |
||
| | 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, "b")]">', | ||
'<itemset nodeset="/data/rep[ ./demographics/age > 18]">', | ||
], | ||
run_odk_validate=True, | ||
) | ||
|
||
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 > 18]">',], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made a few changes in the latest commits; Now checking if the |
||
) | ||
|
||
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 > 18]">',], | ||
run_odk_validate=True, | ||
) | ||
|
||
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 > current()/../target_min_age ]">', | ||
], | ||
run_odk_validate=True, | ||
lognaturel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1077,6 +1077,7 @@ def replace_prefix(d, prefix): | |
list_name not in choices | ||
and select_type != "select one external" | ||
and file_extension not in [".csv", ".xml"] | ||
and not re.match(r"\$\{(.*?)\}", list_name) | ||
): | ||
if not choices: | ||
raise PyXFormError( | ||
|
@@ -1187,6 +1188,8 @@ def replace_prefix(d, prefix): | |
json_dict["choices"] = choices | ||
elif file_extension in [".csv", ".xml"]: | ||
new_json_dict["itemset"] = list_name | ||
elif re.match(r"\$\{(.*?)\}", list_name): | ||
new_json_dict["list_name"] = list_name | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if you populate There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is possible. I've made these changes in the latest commits |
||
else: | ||
new_json_dict["list_name"] = list_name | ||
new_json_dict[constants.CHOICES] = choices[list_name] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially thought it would be clearer to have this whole "select from repeat" case separated from the existing cases but I see now that with having
itemset
vs.list_name
attributes it gets messy. So I think the way you have it is fine but it'd be helpful to have a comment indicating which case this is -- a select from a previous repeat with no choice filter specified.Or see another possible option at #472 (comment)
My goal would be to unify the "select from repeat" case so it's easier to understand and detangle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the option described in #472 (comment) is a bit more cleaner as you've stated. I've implemented the option in the latest commits.