Skip to content
This repository has been archived by the owner on Nov 29, 2023. It is now read-only.

Commit

Permalink
Fixed: itemset inside first repeat is not evaluated correctly when m…
Browse files Browse the repository at this point in the history 
…ultiple repeat instances are present, closes #818
  • Loading branch information
@MartijnR
MartijnR authored Oct 1, 2021
1 parent 750c8f8 commit 74f085c
Show file tree
Hide file tree
Showing 5 changed files with 270 additions and 15 deletions.
36 changes: 34 additions & 2 deletions src/js/dom-utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ function empty( element ) {

/**
* @param {Element} el - Target node
* @return {boolean} Whether previous sibling has same node name
* @return {boolean} Whether previous sibling has the same node name
*/
function hasPreviousSiblingElementSameName( el ) {
let found = false;
Expand All @@ -173,6 +173,36 @@ function hasPreviousSiblingElementSameName( el ) {
return found;
}

/**
* @param {Element} el - Target node
* @return {boolean} Whether next sibling has the same node name
*/
function hasNextSiblingElementSameName( el ) {
let found = false;
const nodeName = el.nodeName;
el = el.nextSibling;

while ( el ) {
// Ignore any sibling text and comment nodes (e.g. whitespace with a newline character)
// also deal with repeats that have non-repeat siblings in between them, event though that would be a bug.
if ( el.nodeName && el.nodeName === nodeName ) {
found = true;
break;
}
el = el.nextSibling;
}

return found;
}

/**
* @param {Element} el - Target node
* @return {boolean} Whether a sibling has the same node name
*/
function hasSiblingElementSameName( el ) {
return hasNextSiblingElementSameName( el ) || hasPreviousSiblingElementSameName( el );
}

/**
* @param {Element} node - Target node
* @param {string} content - Text content to look for
Expand Down Expand Up @@ -225,7 +255,7 @@ function getXPath( node, rootNodeName = '#document', includePosition = false ) {
while ( parent && parentName !== rootNodeName && parentName !== '#document' ) {
if ( includePosition ) {
index = getRepeatIndex( parent );
position = ( index > 0 ) ? `[${index + 1}]` : '';
position = hasSiblingElementSameName( parent ) ? `[${index + 1}]` : '';
}
steps.push( parentName + position );
parent = parent.parentElement;
Expand Down Expand Up @@ -417,6 +447,8 @@ export {
getXPath,
hasPreviousCommentSiblingWithContent,
hasPreviousSiblingElementSameName,
hasNextSiblingElementSameName,
hasSiblingElementSameName,
closestAncestorUntil,
empty,
MutationsTracker
Expand Down
123 changes: 123 additions & 0 deletions test/forms/itemset-repeat-relevant-current-new.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
<?xml version="1.0"?>
<h:html
xmlns="http://www.w3.org/2002/xforms"
xmlns:ev="http://www.w3.org/2001/xml-events"
xmlns:h="http://www.w3.org/1999/xhtml"
xmlns:jr="http://openrosa.org/javarosa"
xmlns:odk="http://www.opendatakit.org/xforms"
xmlns:orx="http://openrosa.org/xforms"
xmlns:xsd="http://www.w3.org/2001/XMLSchema">
<h:head>
<h:title>repeat-relative-current</h:title>
<model odk:xforms-version="1.0.0">
<itext>
<translation lang="English">
<text id="/repeat-relative-current/rep/crop/beans:label">
<value>Beans</value>
</text>
<text id="/repeat-relative-current/rep/crop/banana:label">
<value>Banana</value>
</text>
<text id="/repeat-relative-current/rep/crop/coffee:label">
<value>Coffee</value>
</text>
<text id="static_instance-crop_list-2">
<value>Cacao</value>
</text>
<text id="static_instance-crop_list-3">
<value>Coffee</value>
</text>
<text id="static_instance-crop_list-0">
<value>Banana</value>
</text>
<text id="static_instance-crop_list-1">
<value>Beans</value>
</text>
<text id="/repeat-relative-current/rep/crop/cacao:label">
<value>Cacao</value>
</text>
</translation>
</itext>
<instance>
<repeat-relative-current id="repeat-relative-current">
<rep jr:template="">
<crop/>
<sel_a/>
<group>
<sel_b/>
</group>
</rep>
<meta>
<instanceID/>
</meta>
</repeat-relative-current>
</instance>
<instance id="crop_list">
<root>
<item>
<itextId>static_instance-crop_list-0</itextId>
<name>banana</name>
</item>
<item>
<itextId>static_instance-crop_list-1</itextId>
<name>beans</name>
</item>
<item>
<itextId>static_instance-crop_list-2</itextId>
<name>cacao</name>
</item>
<item>
<itextId>static_instance-crop_list-3</itextId>
<name>coffee</name>
</item>
</root>
</instance>
<bind nodeset="/repeat-relative-current/rep/crop" type="select1"/>
<bind nodeset="/repeat-relative-current/rep/sel_a" relevant=" ../crop != ''" type="select1"/>
<bind nodeset="/repeat-relative-current/rep/group/sel_b" type="select1"/>
<bind calculate="concat('uuid:', uuid())" nodeset="/repeat-relative-current/meta/instanceID" readonly="true()" type="string"/>
</model>
</h:head>
<h:body>
<group ref="/repeat-relative-current/rep">
<label></label>
<repeat nodeset="/repeat-relative-current/rep">
<select1 ref="/repeat-relative-current/rep/crop">
<label>Select</label>
<item>
<label ref="jr:itext('/repeat-relative-current/rep/crop/banana:label')"/>
<value>banana</value>
</item>
<item>
<label ref="jr:itext('/repeat-relative-current/rep/crop/beans:label')"/>
<value>beans</value>
</item>
<item>
<label ref="jr:itext('/repeat-relative-current/rep/crop/cacao:label')"/>
<value>cacao</value>
</item>
<item>
<label ref="jr:itext('/repeat-relative-current/rep/crop/coffee:label')"/>
<value>coffee</value>
</item>
</select1>
<select1 ref="/repeat-relative-current/rep/sel_a">
<label>Verify</label>
<itemset nodeset="instance('crop_list')/root/item[name = current()/../crop ]">
<value ref="name"/>
<label ref="jr:itext(itextId)"/>
</itemset>
</select1>
<group ref="/repeat-relative-current/rep/group">
<select1 ref="/repeat-relative-current/rep/group/sel_b">
<label>Verify</label>
<itemset nodeset="instance('crop_list')/root/item[name = current()/../../crop ]">
<value ref="name"/>
<label ref="jr:itext(itextId)"/>
</itemset>
</select1>
</group>
</repeat>
</group>
</h:body>
</h:html>
18 changes: 9 additions & 9 deletions test/forms/reprelcur1.xml → ...s/itemset-repeat-relevant-current-old.xml
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
<?xml version="1.0"?>
<h:html
xmlns="http://www.w3.org/2002/xforms"
xmlns:ev="http://www.w3.org/2001/xml-events"
xmlns:h="http://www.w3.org/1999/xhtml"
xmlns:jr="http://openrosa.org/javarosa"
xmlns:odk="http://www.opendatakit.org/xforms"
xmlns:orx="http://openrosa.org/xforms"
<h:html
xmlns="http://www.w3.org/2002/xforms"
xmlns:ev="http://www.w3.org/2001/xml-events"
xmlns:h="http://www.w3.org/1999/xhtml"
xmlns:jr="http://openrosa.org/javarosa"
xmlns:odk="http://www.opendatakit.org/xforms"
xmlns:orx="http://openrosa.org/xforms"
xmlns:xsd="http://www.w3.org/2001/XMLSchema">
<h:head>
<h:title>repeat-relative-current</h:title>
Expand Down Expand Up @@ -73,7 +73,7 @@
</root>
</instance>
<bind nodeset="/repeat-relative-current/rep/crop" type="select1"/>
<bind nodeset="/repeat-relative-current/rep/sel_a" type="select1"/>
<bind nodeset="/repeat-relative-current/rep/sel_a" relevant=" ../crop != ''" type="select1"/>
<bind nodeset="/repeat-relative-current/rep/group/sel_b" type="select1"/>
<bind calculate="concat('uuid:', uuid())" nodeset="/repeat-relative-current/meta/instanceID" readonly="true()" type="string"/>
</model>
Expand Down Expand Up @@ -120,4 +120,4 @@
</repeat>
</group>
</h:body>
</h:html>
</h:html>
78 changes: 77 additions & 1 deletion test/spec/dom-utils.spec.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getSiblingElements, getSiblingElementsAndSelf, getSiblingElement, getAncestors, closestAncestorUntil, getChildren, getChild, getXPath } from '../../src/js/dom-utils';
import { getSiblingElements, getSiblingElementsAndSelf, getSiblingElement, getAncestors, closestAncestorUntil, getChildren, getChild, getXPath, hasNextSiblingElementSameName, hasSiblingElementSameName, hasPreviousSiblingElementSameName } from '../../src/js/dom-utils';

function getFragment( htmlStr ) {
return document.createRange().createContextualFragment( htmlStr );
Expand Down Expand Up @@ -247,12 +247,15 @@ describe( 'DOM utils', () => {
<path>
<to>
<node/>
<!-- some comment -->
<repeat>
<number/>
</repeat>
<repeat>
<!-- some comment -->
<number/>
<number/>
<!-- some comment -->
</repeat>
</to>
</path>
Expand All @@ -269,24 +272,97 @@ describe( 'DOM utils', () => {
expect( getXPath( node, null ) ).to.equal( '/root/path/to/node' );
} );

it( 'returns same /root/path/to/node if first parameter is null and second parameter is true', () => {
const node = xml.querySelector( 'node' );
expect( getXPath( node, null ) ).to.equal( '/root/path/to/node' );
} );

it( 'returns path from context first node provided as parameter', () => {
const node = xml.querySelector( 'node' );
expect( getXPath( node, 'root' ) ).to.equal( '/path/to/node' );
} );

it( 'returned path includes no positions if there are no siblings with the same name along the path', () => {
const node = xml.querySelector( 'node' );
expect( getXPath( node, 'root', true ) ).to.equal( '/path/to/node' );
} );

it( 'returned path includes positions when asked', () => {
const node = xml.querySelectorAll( 'number' )[ 1 ];
expect( getXPath( node, 'root', true ) ).to.equal( '/path/to/repeat[2]/number' );
} );

it( 'returned path includes position of first repeat if sibling repeat exists', () => {
const node = xml.querySelector( 'number' );
expect( getXPath( node, 'root', true ) ).to.equal( '/path/to/repeat[1]/number' );
} );

it( 'returned path includes positions when asked (multiple levels)', () => {
const node = xml.querySelectorAll( 'number' )[ 2 ];
expect( getXPath( node, 'root', true ) ).to.equal( '/path/to/repeat[2]/number[2]' );
} );
} );

describe( 'siblingElement functions', () => {
const xmlStr = `
<root>
<path>
<to>
<node/>
<!-- some comment -->
<repeat>
<number />
</repeat>
<rogue/>
<!-- some comment -->
<repeat>
<number />
</repeat>
<!-- some comment -->
<rogue/>
</to>
</path>
</root>`;
const xml = new DOMParser().parseFromString( xmlStr, 'text/xml' );
const repeats = xml.querySelectorAll( 'repeat' );
const node = xml.querySelector( 'node' );

describe( 'hasPreviousSiblingElementSameName', () => {
[
[ repeats[0], false ],
[ repeats[1], true ],
[ node, false ]
].forEach( ( [ el, expected ] ) => {
it( 'evaluates correctly', () => {
expect( hasPreviousSiblingElementSameName( el ) ).to.equal( expected );
} );
} );
} );

describe( 'hasNextSiblingElementSameName', () => {
[
[ repeats[0], true ],
[ repeats[1], false ],
[ node, false ]
].forEach( ( [ el, expected ] ) => {
it( 'evaluates correctly', () => {
expect( hasNextSiblingElementSameName( el ) ).to.equal( expected );
} );
} );
} );

describe( 'hasSiblingElementSameName', () => {
[
[ repeats[0], true ],
[ repeats[1], true ],
[ node, false ]
].forEach( ( [ el, expected ] ) => {
it( 'evaluates correctly', () => {
expect( hasSiblingElementSameName( el ) ).to.equal( expected );
} );
} );
} );

} );

} );
30 changes: 27 additions & 3 deletions test/spec/itemset.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,11 +290,11 @@ describe( 'Itemset functionality', () => {
} );
} );

describe( ' in a cloned repeat with a predicate including current()/../', () => {
it( 'works', () => {
describe( 'with repeats and a predicate referring to current()/../', () => {
it( 'works in a cloned repeat in an old form that does not contain odk:xforms-version="1.0.0"', () => {
// This test is added to show that once the makeBugCompliant function has been removed
// itemsets with relative predicates still work.
const form = loadForm( 'reprelcur1.xml' );
const form = loadForm( 'itemset-repeat-relevant-current-old.xml' );
form.init();
form.view.$.find( '[data-name="/repeat-relative-current/rep/crop"][value="banana"]' ).prop( 'checked', true ).trigger( 'change' );
form.view.$.find( '.add-repeat-btn' ).click();
Expand All @@ -306,6 +306,30 @@ describe( 'Itemset functionality', () => {
expect( form.view.$.find( sel1 ).eq( 1 ).val() ).to.equal( 'beans' );
expect( form.view.$.find( sel2 ).eq( 1 ).val() ).to.equal( 'beans' );
} );

describe( 'works in a first repeat if a second repeat exists and an itemset update is triggered', () => {

[
[ 'in an old form that does not contain odk:xforms-version="1.0.0"', 'itemset-repeat-relevant-current-old.xml' ],
[ 'in a form that contains odk:xforms-version="1.0.0"','itemset-repeat-relevant-current-new.xml' ]
].forEach( ( [ itText, filename ] ) => {
it( itText, ()=> {
const form = loadForm( filename );
form.init();
form.view.html.querySelector( '.add-repeat-btn' ).click();
const repeats = form.view.html.querySelectorAll( '.or-repeat' );
const crop1 = repeats[0].querySelector( '[data-name="/repeat-relative-current/rep/crop"][value="banana"]' );
crop1.checked = true;
crop1.dispatchEvent( events.Change() );
const crop2 = repeats[1].querySelector( '[data-name="/repeat-relative-current/rep/crop"][value="beans"]' );
crop2.checked = true;
crop2.dispatchEvent( events.Change() );
const selectorA = 'label:not(.itemset-template) > input[data-name="/repeat-relative-current/rep/sel_a"]';
expect( [ ...repeats[0].querySelectorAll( selectorA ) ].map( el => el.value ) ).to.eql( [ 'banana' ] );
} );
} );

} );
} );

describe( 'with a rank widget', () => {
Expand Down

0 comments on commit 74f085c

Please sign in to comment.