-
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 6 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,84 @@ 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_nested_repeat(self): | ||
"""Select one choices from previous repeat answers within a nested repeat""" | ||
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="/data/household_rep/household_mem_rep[ ./age > current()/../target_min_age ]">', | ||
MartijnR marked this conversation as resolved.
Show resolved
Hide resolved
|
||
], | ||
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.