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

Commit

Permalink
Fix: don't use removed repeat instance as context for relevance (#909)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lognaturel
lognaturel committed Jun 22, 2022
1 parent 6936b39 commit e80306a
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 2 deletions.
1 change: 1 addition & 0 deletions src/js/nodeset.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ Nodeset.prototype.remove = function () {
nodes: null,
repeatPath,
repeatIndex,
removed: true, // Introduced to handle relevance on model nodes with no form controls (calculates)
})
);

Expand Down
12 changes: 10 additions & 2 deletions src/js/relevant.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,16 @@ export default {
*/
let context = p.path;
if (
getChild(node, `.or-repeat-info[data-name="${p.path}"]`) &&
!getChild(node, `.or-repeat[name="${p.path}"]`)
(getChild(node, `.or-repeat-info[data-name="${p.path}"]`) &&
!getChild(node, `.or-repeat[name="${p.path}"]`)) ||
// Special cases below for model nodes with no visible form control: if repeat instance removed or if
// no instances at all (e.g. during load with `jr:count="0"`)
(insideRepeat &&
repeatParent == null &&
(options.removed ||
this.form.view.html.querySelector(
`.or-repeat[name="${CSS.escape(repeatPath)}"]`
) == null))
) {
context = null;
}
Expand Down
37 changes: 37 additions & 0 deletions test/forms/repeat-count-relevant-on-calculate.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?xml version="1.0"?>
<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa" xmlns:odk="http://www.opendatakit.org/xforms">
<h:head>
<h:title>Repeat count 0 with relevant on calculate</h:title>
<model odk:xforms-version="1.0.0">
<instance>
<data id="relevance_on_calc_repeat_count">
<count>1</count>
<repeat>
<q1/>
<q1_x2/>
</repeat>
<meta>
<instanceID/>
</meta>
</data>
</instance>
<bind nodeset="/data/count" type="int"/>
<bind nodeset="/data/repeat/q1" type="int"/>
<bind calculate=" ../q1 * 2" nodeset="/data/repeat/q1_x2" relevant=" ../q1 &gt; 0" type="string"/>
<bind jr:preload="uid" nodeset="/data/meta/instanceID" readonly="true()" type="string"/>
</model>
</h:head>
<h:body>
<input ref="/data/count">
<label>Count</label>
</input>
<group ref="/data/repeat">
<label>Repeat</label>
<repeat jr:count=" /data/count " nodeset="/data/repeat">
<input ref="/data/repeat/q1">
<label>Q1</label>
</input>
</repeat>
</group>
</h:body>
</h:html>
32 changes: 32 additions & 0 deletions test/forms/repeat-relevant-on-calculate.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?xml version="1.0"?>
<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa" xmlns:odk="http://www.opendatakit.org/xforms">
<h:head>
<h:title>Relevant on calculate in repeat</h:title>
<model odk:xforms-version="1.0.0">
<instance>
<data id="relevance_on_calc">
<repeat>
<q1/>
<q1_x2/>
</repeat>
<meta>
<instanceID/>
</meta>
</data>
</instance>
<bind nodeset="/data/repeat/q1" type="int"/>
<bind calculate=" ../q1 * 2" nodeset="/data/repeat/q1_x2" relevant=" ../q1 &gt; 0" type="string"/>
<bind jr:preload="uid" nodeset="/data/meta/instanceID" readonly="true()" type="string"/>
</model>
</h:head>
<h:body>
<group ref="/data/repeat">
<label>Repeat</label>
<repeat nodeset="/data/repeat">
<input ref="/data/repeat/q1">
<label>Q1</label>
</input>
</repeat>
</group>
</h:body>
</h:html>
59 changes: 59 additions & 0 deletions test/spec/repeat.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,43 @@ describe('repeat functionality', () => {
});
});

describe('instance removal', () => {
it('removes an instance that contains a field without a form control', async () => {
const form = loadForm('repeat-relevant-on-calculate.xml');
const errors = form.init();

expect(
form.view.html.querySelectorAll(
'.or-repeat[name="/data/repeat"]'
).length
).to.equal(1);

const q1 = form.view.html.querySelector(
'input[name="/data/repeat/q1"]'
);
q1.value = 2;
q1.dispatchEvent(event.Change());

expect(form.model.xml.querySelector('q1_x2').textContent).to.equal(
'4'
);

form.view.html
.querySelector('.or-repeat[name="/data/repeat"]')
.querySelector('button.remove')
.click();

await timers.runAllAsync();

expect(
form.view.html.querySelectorAll(
'.or-repeat[name="/data/repeat"]'
).length
).to.equal(0);
expect(errors.length).to.equal(0);
});
});

describe('fixes unique ids in cloned repeats', () => {
// Avoiding problems in the autocomplete widget, https://github.com/enketo/enketo-core/issues/521
it('ensures uniqueness of datalist ids, so cascading selects inside repeats work', () => {
Expand Down Expand Up @@ -479,6 +516,28 @@ describe('repeat functionality', () => {
).to.equal('2');
});

it('and works with relevance on a field without a form control when repeat count is 0', () => {
const form = loadForm('repeat-count-relevant-on-calculate.xml');
const errors = form.init();
expect(errors.length).to.equal(0);

const count = form.view.html.querySelector(
'input[name="/data/count'
);
count.value = 1;
count.dispatchEvent(event.Change());

const q1 = form.view.html.querySelector(
'input[name="/data/repeat/q1"]'
);
q1.value = 2;
q1.dispatchEvent(event.Change());

expect(form.model.xml.querySelector('q1_x2').textContent).to.equal(
'4'
);
});

it('and correctly deals with nested repeats that have a repeat count', () => {
const form = loadForm('repeat-count-nested-2.xml');
const a = '.or-repeat[name="/data/repeat_A"]';
Expand Down

0 comments on commit e80306a

Please sign in to comment.