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

[Fix] Ensure nested repeats are shown when relevance is true #807

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
<!-- <script src="//localhost:35729/livereload.js"></script> -->

<!-- all the scripts -->
<script id="main-script" defer module type="text/javascript" src="./js/app.js" charset="utf-8"></script>
<script id="main-script" defer module type="text/javascript" src="./app.js" charset="utf-8"></script>

</body>

Expand Down
11 changes: 9 additions & 2 deletions src/js/relevant.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,15 @@ export default {
* but currently has 0 repeats, the context will not be available. This same logic is applied in output.js.
*/
let context = p.path;
if ( getChild( node, `.or-repeat-info[data-name="${p.path}"]` ) && !getChild( node, `.or-repeat[name="${p.path}"]` ) ) {
context = null;

const repeatInfo = getChild( node, `.or-repeat-info[data-name="${p.path}"]` );

if ( repeatInfo != null && !getChild( node, `.or-repeat[name="${p.path}"]` ) ) {
const count = this.form.repeats.updateViewInstancesFromModel( repeatInfo );
Copy link
Member

@MartijnR MartijnR Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the underlying issue is that the first instance of the nested repeat is not created when the top-level repeat (no. 2) is created (after the user clicks the plus button).

The relevant module should not be responsible for fixing that, as it's the responsibility of the repeat module. It's odd that something so elementary is not done correctly. I wonder if this bug was introduced when we started allowing 0 repeat instances. There seems to be some (still mysterious) special condition in the test form, because in e.g. http://localhost:8005/?xform=nested_repeats.xml the bug doesn't surface.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hesitant to apply this fix here too, I'm glad you agree. To be honest I chose this path because I struggled to understand the repeat initialization flow and wasn't sure how to address it there. But I'll take another look because I agree it would be better solved there.


if ( count === 0 ) {
context = null;
}
}

/*
Expand Down
155 changes: 155 additions & 0 deletions test/forms/core-804-nested-repeat-relevant.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
<?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>enketo-repeat-bug</h:title>
<model odk:xforms-version="1.0.0">
<instance><data id="enketo-repeat-bug" version="2021081700">
<field_asset_exists />
<field_asset_rpt jr:template="">
<field_asset_rpt_grp>
<asset_name />
<documentation_exists />
<documentation_rpt jr:template="">
<documentation_rpt_grp>
<documentation_type />
<documentation_file />
<documentation_photo />
</documentation_rpt_grp>
</documentation_rpt>
<documentation_rpt>
<documentation_rpt_grp>
<documentation_type />
<documentation_file />
<documentation_photo />
</documentation_rpt_grp>
</documentation_rpt>
<instrumentation_exists />
<instrumentation_rpt jr:template="">
<instrumentation_rpt_grp>
<instrumentation_tag />
<instrumentation_measurement />
<instrumentation_measurement_other />
</instrumentation_rpt_grp>
</instrumentation_rpt>
<instrumentation_rpt>
<instrumentation_rpt_grp>
<instrumentation_tag />
<instrumentation_measurement />
<instrumentation_measurement_other />
</instrumentation_rpt_grp>
</instrumentation_rpt>
</field_asset_rpt_grp>
</field_asset_rpt>
<field_asset_rpt>
<field_asset_rpt_grp>
<asset_name />
<documentation_exists />
<documentation_rpt>
<documentation_rpt_grp>
<documentation_type />
<documentation_file />
<documentation_photo />
</documentation_rpt_grp>
</documentation_rpt>
<instrumentation_exists />
<instrumentation_rpt>
<instrumentation_rpt_grp>
<instrumentation_tag />
<instrumentation_measurement />
<instrumentation_measurement_other />
</instrumentation_rpt_grp>
</instrumentation_rpt>
</field_asset_rpt_grp>
</field_asset_rpt>
<meta>
<instanceID />
</meta>
</data></instance>
<bind nodeset="/data/field_asset_exists" required="true()" type="string" />
<bind nodeset="/data/field_asset_rpt" relevant="selected( /data/field_asset_exists , 'yes')" />
<bind nodeset="/data/field_asset_rpt/field_asset_rpt_grp/asset_name" type="string" />
<bind nodeset="/data/field_asset_rpt/field_asset_rpt_grp/documentation_exists" required="true()" type="string" />
<bind nodeset="/data/field_asset_rpt/field_asset_rpt_grp/documentation_rpt" relevant="selected( ../documentation_exists , 'yes')" />
<bind nodeset="/data/field_asset_rpt/field_asset_rpt_grp/documentation_rpt/documentation_rpt_grp/documentation_type" required="true()" type="string" />
<bind nodeset="/data/field_asset_rpt/field_asset_rpt_grp/documentation_rpt/documentation_rpt_grp/documentation_file" type="binary" />
<bind nodeset="/data/field_asset_rpt/field_asset_rpt_grp/documentation_rpt/documentation_rpt_grp/documentation_photo" orx:max-pixels="1000" relevant=" ../documentation_file = ''" type="binary" />
<bind nodeset="/data/field_asset_rpt/field_asset_rpt_grp/instrumentation_exists" required="true()" type="string" />
<bind nodeset="/data/field_asset_rpt/field_asset_rpt_grp/instrumentation_rpt" relevant="selected( ../instrumentation_exists , 'yes')" />
<bind nodeset="/data/field_asset_rpt/field_asset_rpt_grp/instrumentation_rpt/instrumentation_rpt_grp/instrumentation_tag" type="string" />
<bind nodeset="/data/field_asset_rpt/field_asset_rpt_grp/instrumentation_rpt/instrumentation_rpt_grp/instrumentation_measurement" required="true()" type="string" />
<bind nodeset="/data/field_asset_rpt/field_asset_rpt_grp/instrumentation_rpt/instrumentation_rpt_grp/instrumentation_measurement_other" relevant="selected( ../instrumentation_measurement , 'other')" required="true()" type="string" />
<bind jr:preload="uid" nodeset="/data/meta/instanceID" readonly="true()" type="string" />
</model>
</h:head>
<h:body>
<select1 ref="/data/field_asset_exists"><label>Is there at least 1 field asset?</label>
<item><label>Yes</label>
<value>yes</value>
</item>
<item><label>No</label>
<value>no</value>
</item>
</select1>
<group ref="/data/field_asset_rpt"><label>Field asset</label>
<repeat nodeset="/data/field_asset_rpt">
<group ref="/data/field_asset_rpt/field_asset_rpt_grp"><label><output value=" ../field_asset_rpt_grp/asset_name " /></label><input ref="/data/field_asset_rpt/field_asset_rpt_grp/asset_name"><label>Asset name</label></input>
<select1 ref="/data/field_asset_rpt/field_asset_rpt_grp/documentation_exists"><label>Is there at least 1 piece of documentation?</label>
<item><label>Yes</label>
<value>yes</value>
</item>
<item><label>No</label>
<value>no</value>
</item>
</select1>
<group ref="/data/field_asset_rpt/field_asset_rpt_grp/documentation_rpt"><label>Documentation</label>
<repeat nodeset="/data/field_asset_rpt/field_asset_rpt_grp/documentation_rpt">
<group ref="/data/field_asset_rpt/field_asset_rpt_grp/documentation_rpt/documentation_rpt_grp"><label><output value=" ../documentation_rpt_grp/documentation_type " /></label>
<select1 ref="/data/field_asset_rpt/field_asset_rpt_grp/documentation_rpt/documentation_rpt_grp/documentation_type"><label>Documentation type</label>
<item><label>Drawing set</label>
<value>drawing_set</value>
</item>
<item><label>Maintenance schedule</label>
<value>maintenance_schedule</value>
</item>
<item><label>Other</label>
<value>other</value>
</item>
</select1>
<upload mediatype="application/*" ref="/data/field_asset_rpt/field_asset_rpt_grp/documentation_rpt/documentation_rpt_grp/documentation_file"><label>Documentation file</label>
<hint>If no file available, go to next question to take a photo</hint>
</upload>
<upload mediatype="image/*" ref="/data/field_asset_rpt/field_asset_rpt_grp/documentation_rpt/documentation_rpt_grp/documentation_photo"><label>Documentation photo</label></upload>
</group>
</repeat>
</group>
<select1 ref="/data/field_asset_rpt/field_asset_rpt_grp/instrumentation_exists"><label>Is there at least 1 piece of instrumentation?</label>
<item><label>Yes</label>
<value>yes</value>
</item>
<item><label>No</label>
<value>no</value>
</item>
</select1>
<group ref="/data/field_asset_rpt/field_asset_rpt_grp/instrumentation_rpt"><label>Instrumentation</label>
<repeat nodeset="/data/field_asset_rpt/field_asset_rpt_grp/instrumentation_rpt">
<group ref="/data/field_asset_rpt/field_asset_rpt_grp/instrumentation_rpt/instrumentation_rpt_grp"><label><output value=" ../instrumentation_rpt_grp/instrumentation_tag " /></label><input ref="/data/field_asset_rpt/field_asset_rpt_grp/instrumentation_rpt/instrumentation_rpt_grp/instrumentation_tag"><label>Instrumentation tag</label>
<hint>If available</hint></input>
<select1 ref="/data/field_asset_rpt/field_asset_rpt_grp/instrumentation_rpt/instrumentation_rpt_grp/instrumentation_measurement"><label>Instrumentation measurement</label>
<item><label>Conductivity</label>
<value>conductivity</value>
</item>
<item><label>Flow</label>
<value>flow</value>
</item>
<item><label>Other</label>
<value>other</value>
</item>
</select1><input ref="/data/field_asset_rpt/field_asset_rpt_grp/instrumentation_rpt/instrumentation_rpt_grp/instrumentation_measurement_other"><label>Specify other instrumentation measurement</label></input>
</group>
</repeat>
</group>
</group>
</repeat>
</group>
</h:body>
</h:html>
Loading