Improve and augment tests around relative paths, especially in choice filters

[lognaturel]

#335 introduced some tests on using current() in an itemset context. These are pretty low-level and not entirely targeted (e.g. they always do a "round trip" with verification that values are properly written back out).

On that PR, @ggalmazor discussed a few directions that could be taken to make these tests easier to read. For example, a method answerQuestion(string, string) could be introduced. The first string would be interpreted as an XPath path that a reference can be built from. Then this could be converted to a formindex to jump to that desired question. The second string would be the answer to set.

Similarly, answerOf(string) could take an XPath path as a string and return the answer to that question / value of the node.

To augment these tests, introduce more cases with relative paths (from #335 (comment)):

• submission inside the model of a form XFormParser.java#755
• setvalue action XFormParser.java#690
• A form field with a binding with a relative path XFormParser.java#969
  • Specifically, a group with a binding with a relative path XFormParser.java#1468 and XFormParser.java#1477
• A <label> that shows an <itext> related to the selected value from a previous field

[lognaturel]

@ggalmazor I know you have some of this already in a branch and that there's overlap with #334 so I think it makes sense for you to take on.

The additional tests must be in before the next JR release to add more confidence around #335.

[lognaturel]

Overlap with #336

[ggalmazor]

I have a question about the optional <submission> element inside a form's model. According to the documentation, we can expect it to have the action, method, base64RsaPublicKey, orx:auto-send, and orx:auto-delete attributes.

XFormParser.parseSubmission(Element submission) checks these attributes, but it also checks for an allegedly optional bind attribute, requiring a call to `FormDef.getAbsRef(IDataReference ref, TreeReference parentRef), which is one of the cases we want to augment the tests with.

What could be the use of a bind attribute inside a submission element?

[ggalmazor]

I have another question about the setvalue action. I've tried to think about creative uses of this action that would involve relative xpaths to siblings inside a repeat group. Something that could justify the use of current():

<?xml version="1.0"?>
<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:h="http://www.w3.org/1999/xhtml">
    <h:head>
        <h:title>relative-current-ref</h:title>
        <model>
            <instance>
                <data id="relative-current-ref-repeat">
                    <meta>
                        <instanceID/>
                    </meta>
                    <repeat_group>
                        <person/>
                        <selected_person/>
                    </repeat_group>
                </data>
            </instance>
            <bind nodeset="/data/repeat_group/person" type="string"/>
            <bind nodeset="/data/repeat_group/selected_person" type="string"/>
            <bind calculate="concat('uuid:', uuid())" nodeset="/data/meta/instanceID" readonly="true()" type="string"/>
        </model>
    </h:head>
    <h:body>
        <repeat nodeset="/data/repeat_group">
            <group>
                <label>Top level group</label>
                <input ref="/data/repeat_group/person">
                    <label>Person</label>
                </input>
                <input ref="/data/repeat_group/selected_person">
                    <label>Selected Person</label>
                    <setvalue event="xforms-value-changed" ref="curren()/../person">Value changed!</setvalue>
                </input>
            </group>
        </repeat>
    </h:body>
</h:html>

(don't mind the field names, etc. I just want the second text field to update with some fixed value when the value of the first one changes)

The parser is complaining about this form with this error: XPath evaluation: Attempt to parent past the root node current()../person.

Any ideas?

[lognaturel]

What could be the use of a bind attribute inside a submission element?

I've never seen this before and didn't know it was possible. My guess would be that a client could use that to only send the subtree described by that node. I don't believe Collect does that but would be good to check if it does use it. I think this is safe to ignore.

The parser is complaining about this form with this error: XPath evaluation: Attempt to parent past the root node current()../person

We already actually have a good form for this -- nested-setvalue-action-with-repeats.xml. But it abuses the issue at https://opendatakit.github.io/xforms-spec/#a-big-deviation-with-xforms and uses an absolute reference (a big reason we want to make the change to using relative paths in repeats in XLSForm/pyxform#187).

I changed /data/repeat/cost_timestamp to ../cost_timestamp, debugged and see that the problem is in XFormParser.parseSetValueAction -- dataRef = FormDef.getAbsRef(dataRef, TreeReference.rootRef()) shouldn't always use the root ref for anchoring, it should use the current context. I haven't looked at a fix yet.

So these tests are already paying off. 🎉

[ggalmazor]

I changed /data/repeat/cost_timestamp to ../cost_timestamp, debugged and see that the problem is in XFormParser.parseSetValueAction -- dataRef = FormDef.getAbsRef(dataRef, TreeReference.rootRef()) shouldn't always use the root ref for anchoring, it should use the current context. I haven't looked at a fix yet.

Should we create an issue for this?

[lognaturel]

Should we create an issue for this?

Yes, good idea. I guess you can omit that test here and the test can go with the fix for easy tracking.

[ggalmazor]

A <label> that shows an <itext> related to the selected value from a previous field

@lognaturel, it turns out that this last check we figured out doesn't make sense:

• A label's ref attribute only accepts a call to jr:itext(string id)
• Dynamic itext ids are not supported. We try to match the id literally against all the defined translation ids

After seeing this, I think it doesn't leave much room for augmenting the test suite for current() around labels and itext.

[lognaturel]

@ggalmazor When you wrote that, I was imagining something like XLSForm/pyxform#187 (comment) where the label of the select uses a relative expression with current(). It's another one of those places where current() isn't strictly necessary but I think it's slightly different than the existing cases because label is a child of the select1 but has the same context.

[ggalmazor]

Ooooh, that's a good one I didn't think about!

I'll explore that <output> example. If it turns out it doesn't add coverage, would you want to still leave it as an example use case of current()?

[lognaturel]

I think it'd be good to have an example like that with a relative reference. Not sure if it should really use current().

Maybe something to consider is to have a series of tests that demonstrate where expressions with current() evaluate to the same thing as expressions with . and where they don't. That might be a better place for comments we've been discussing at https://github.com/opendatakit/javarosa/pull/361/files#r211469938 (though you're right that adding one by the current case in XPathPathExpr would be helpful regardless).