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.

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

When indexed-repeat is used within a repeat the first argument nodeset points to single node using relative path #484

Closed
MartijnR opened this issue Nov 25, 2020 · 8 comments · Fixed by #485 or #499

Comments

@MartijnR
Copy link
Contributor

MartijnR commented Nov 25, 2020

Software and hardware versions

2.2.1

Problem description

The first argument of indexed-repeat() calls is relative when called from inside a repeat and referring to a question in that repeat. It should be absolute.

A ../relative/path/to/question always returns a single node (in ODK XForms) and according to the spec it should refer to all nodes with that question name, i.e. the nodeset returned by /absolute/path/to/question.

Steps to reproduce the problem

A. using regular calculation with relative index:

type name label calculation
begin_repeat person Person  
integer pos position(..)
text name Enter name  
text prev_name Name in previous repeat instance indexed-repeat(${name}, ${person}, ${pos}-1)
end_repeat      

outputs:

<bind calculate="indexed-repeat( ../name ,  /data/person , ../pos -1)"  ... />

B. using dynamic default:

type name label default
begin_repeat person Person  
text name Enter name  
text prev_name Name in previous repeat instance indexed-repeat(${name}, ${person}, position(..)-1)
end_repeat      

outputs:

<setvalue ... value="indexed-repeat( ../name ,  /data/person , position(..)-1)" />

C. using dreaded nested repeats:

type name label default
begin_repeat family Family  
integer members_number How many members in this family?  
begin_repeat person Person  
text name Enter name  
text prev_name Non-sensible previous name in first family, 2nd person indexed-repeat(${name}, ${family}, 1, ${person}, 2)
end_repeat      
end_repeat      

outputs:

<setvalue .. value="indexed-repeat( ../name ,  /data/family , 1,  ../../person , 2)" />

Expected behavior

A:

<bind calculate="indexed-repeat( /data/person/name ,  /data/person , ../pos - 1)" ... />

B:

<setvalue ... value="indexed-repeat( /data/person/name ,  /data/person , position(..)-1)" />

C:

<setvalue .. value="indexed-repeat( /data/family/person/name ,  /data/family , 1, /data/family/person , 2)" />

So the first argument of indexed-repeat() is an exception to the relative-path rule used to transform ${node} syntax (and the second argument too but that seems to already be the case).

Or actually since indexed-repeat() has either 3, 5, or 7 arguments, this exception applies to the 1st, 2nd, 4th, and 6th arguments.

Other information

Previously using indexed-repeat() inside a repeat was a bit of an edge case. Since the introduction of dynamic defaults it is becoming more common, as you could use the answer of a previous repeat question as the dynamic default of that same question in a new repeat instance (example B).

I'm guessing this issue was introduced in 0.12.0 when we changed to relative node references inside repeats.

@MartijnR
Copy link
Contributor Author

ping @pbowen-oc

@pbowen-oc
Copy link

Thanks for logging this, @MartijnR

@gushil - Can you please work on this?

@gushil
Copy link
Contributor

gushil commented Nov 27, 2020

@pbowen-oc Working on it.

@gushil
Copy link
Contributor

gushil commented Dec 2, 2020

Hi @MartijnR, I've created PR #485 to fix this issue. Waiting for review.
Thanks

@MartijnR
Copy link
Contributor Author

Looks like the form posted here slips through the cracks: OpenClinica/enketo-express-oc#398 (comment) . See both (similar) indexed-repeat() calls.

One of them is:
if(position(..) = 1, '', indexed-repeat(${bp_sys}, ${bp_rg}, position(..) - 1))

@pbowen-oc
Copy link

@gushil - Can you take a look at this?

@gushil
Copy link
Contributor

gushil commented Dec 17, 2020

@pbowen-oc working on it

@gushil
Copy link
Contributor

gushil commented Dec 17, 2020

@MartijnR I've created a new PR (#499) to solve the issue @pbowen-oc mentioned.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants