Fix: don't use removed repeat instance as context for relevance (#909)

enketo · Jun 22, 2022 · e80306a · e80306a
1 parent 6936b39
commit e80306a
Show file tree

Hide file tree

Showing 5 changed files with 139 additions and 2 deletions.
diff --git a/src/js/nodeset.js b/src/js/nodeset.js
@@ -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)
             })
         );
 

diff --git a/src/js/relevant.js b/src/js/relevant.js
@@ -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;
             }

diff --git a/test/forms/repeat-count-relevant-on-calculate.xml b/test/forms/repeat-count-relevant-on-calculate.xml
@@ -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>
diff --git a/test/forms/repeat-relevant-on-calculate.xml b/test/forms/repeat-relevant-on-calculate.xml
@@ -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>
diff --git a/test/spec/repeat.spec.js b/test/spec/repeat.spec.js
@@ -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', () => {
@@ -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"]';