Get additional children #663
Conversation
Clearly the design here isn't great. This class represents choices differently for all of the combinations of static vs dynamic and localizable vs not localizable.
| assertThat(children, hasEntry("special-property", "AA")); | ||
|
|
||
| scenario.answer("/data/repeat[0]/special-property", "changed"); | ||
| children = scenario.choicesOf("/data/select").get(0).getAdditionalChildren(); |
There was a problem hiding this comment.
I hadn't really thought about this previously -- because getChild and getAdditionalChildren use the display text of the children, they're not dynamic. That is, you have to call getChild and getAdditionalChildren again after the form changes to get new values. I think that's ok -- I can't think of a usage where we'd hold on to one of these while other parts of the form are changing but I still wanted to flag it.
The alternative would be to keep a reference to an AnswerData object instead. That would reflect changes to the model.
|
@seadowg May this be of value to you. 🤞🤞😬😬 |
| if (child != null) { | ||
| return child.getValue().getDisplayText(); | ||
| } | ||
| if (item == null) { |
There was a problem hiding this comment.
One thing I find hard about JR is that it can be unclear what's a deliberate precondition vs a case that wasn't explicitly considered vs ...? I think this precondition is now much clearer even though it's equivalent to the terser code it replaces. (As I mentioned, this thinking about preconditions is what had nudged me in the direction of runtime exceptions.)
|
Thanks for all the good discussion! I think the two things left are
|
lognaturel
force-pushed
the
getAddtlChildren
branch
from
50e2713
to
feae17b
Compare
lognaturel
force-pushed
the
getAddtlChildren
branch
from
feae17b
to
a541ffe
Compare
| @@ -38,6 +38,8 @@ dependencies { | |||
| // Upgrade to version higher than 1.4 when Collect minSDK >= 26 | |||
| implementation 'org.apache.commons:commons-csv:1.4' | |||
|
|
|||
| implementation 'org.jetbrains.kotlin:kotlin-stdlib:1.6.10' | |||
Adds access to additional children from an itemset item in support of getodk/collect#4948
What has been done to verify that this works as intended?
Tests!
Why is this the best possible solution? Were any other approaches considered?
Comments inline for a few alternatives I considered.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
Additive change.
I did put all the
copyNodestuff on a deprecation path. It's not documented. If ever we have a use case for it we can bring it back with tests.Do we need any specific form for testing your changes? If so, please attach one.
See tests.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
Not user-facing so no.