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

Fix: ensure geopicker is loaded for non-nested setgeopoint actions #904

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Fix: ensure geopicker is loaded for non-nested setgeopoint actions #904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
34af7e6
Fix: setgeopoint actions correctly identify their affected nodeset
eyelidlessness Jun 7, 2022
ddd5e0b
Fix: ensure geopicker is loaded for non-nested setgeopoint actions
eyelidlessness Jun 7, 2022
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
38 changes: 31 additions & 7 deletions src/js/calculate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,14 +204,38 @@ export default {
const nodes = this._getNodesForAction(action, event);

nodes.forEach((actionControl) => {
const name = this.form.input.getName(actionControl);
const dataNodesObj = this.form.model.node(name);
let name = this.form.input.getName(actionControl);

let affectedControl;

if (
name == null ||
name === '' ||
actionControl.matches(
`[name="${CSS.escape(name)}"] ~ [name="${CSS.escape(
name
)}"]`
)
) {
affectedControl = actionControl
.closest('.question')
?.querySelector(
':is([data-name], [name]):not([data-name=""], [data-event], [name=""])'
);

name = this.form.input.getName(affectedControl);
} else {
affectedControl = actionControl;
}

// TODO: should this pass `{ onlyLeaf: true }` option?
const dataNodesObj = this.form.model.node(name, null);
const dataNodes = dataNodesObj.getElements();

const props = {
name,
dataType: this.form.input.getXmlType(actionControl),
relevantExpr: this.form.input.getRelevant(actionControl),
dataType: this.form.input.getXmlType(affectedControl),
relevantExpr: this.form.input.getRelevant(affectedControl),
index:
event.detail &&
typeof event.detail.repeatIndex !== 'undefined'
Expand All @@ -222,7 +246,7 @@ export default {
};

if (action === 'setvalue') {
props.expr = actionControl.dataset.setvalue;
props.expr = affectedControl.dataset.setvalue;
}

if (
Expand All @@ -238,7 +262,7 @@ export default {
*/
dataNodes.forEach((el, index) => {
const obj = Object.create(props);
const control = actionControl;
const control = affectedControl;
obj.index = index;
this._updateCalc(control, obj);
});
Expand All @@ -253,7 +277,7 @@ export default {
}
this._updateCalc(control, props);
} else if (dataNodes[props.index]) {
const control = actionControl;
const control = affectedControl;
this._updateCalc(control, props);
} else {
console.error(
Expand Down
2 changes: 1 addition & 1 deletion src/widget/geo/geopicker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class Geopicker extends Widget {
* @type {string}
*/
static get selector() {
return ':is(.question input[data-type-xml="geopoint"], .question input[data-type-xml="geotrace"], .question input[data-type-xml="geoshape"]):not([data-setvalue], [data-setgeopoint])';
return '.question [name]:not([name=""], [type="hidden"]):is(input[data-type-xml="geopoint"], input[data-type-xml="geotrace"], input[data-type-xml="geoshape"])';
}

/**
Expand Down
33 changes: 27 additions & 6 deletions test/forms/setgeopoint.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@
<instance>
<data id="setgeopoint">
<hidden_first_load/>
<visible_first_load/>
<visible_first_load_adjacent_action/>
<visible_first_load_nested_explicit_ref/>
<visible_first_load_nested_implied_ref/>
<visible_first_load_model_action/>
<changes/>
<location_changed/>
<meta>
Expand All @@ -22,18 +25,36 @@
</instance>

<bind nodeset="/data/hidden_first_load" type="string"/>
<bind nodeset="/data/visible_first_load" type="string"/>
<bind nodeset="/data/visible_first_load_adjacent_action" type="geopoint"/>
<bind nodeset="/data/visible_first_load_nested_explicit_ref" type="geopoint"/>
<bind nodeset="/data/visible_first_load_nested_implied_ref" type="geopoint"/>
<bind nodeset="/data/visible_first_load_model_action" type="geopoint"/>

<bind nodeset="/data/changes" type="int"/>
<bind nodeset="/data/location_changed" type="string"/>
<bind nodeset="/data/location_changed" type="geopoint"/>

<odk:setgeopoint event="some-unsupported-event odk-instance-first-load" ref="/data/hidden_first_load"/>
<odk:setgeopoint event="odk-instance-first-load" ref="/data/visible_first_load_model_action"/>
</model>
</h:head>
<h:body>
<odk:setgeopoint event="odk-instance-first-load" ref="/data/visible_first_load"/>
<input ref="/data/visible_first_load">
<label>odk-instance-first-load</label>
<odk:setgeopoint event="odk-instance-first-load" ref="/data/visible_first_load_adjacent_action"/>
<input ref="/data/visible_first_load_adjacent_action">
<label>visible_first_load_adjacent_action</label>
</input>

<input ref="/data/visible_first_load_nested_explicit_ref">
<label>visible_first_load_nested_explicit_ref</label>
<odk:setgeopoint event="odk-instance-first-load" ref="/data/visible_first_load_nested_explicit_ref"/>
</input>

<input ref="/data/visible_first_load_nested_implied_ref">
<odk:setgeopoint event="odk-instance-first-load"/>
<label>visible_first_load_nested_implied_ref</label>
</input>

<input ref="/data/visible_first_load_model_action">
<label>visible_first_load_model_action</label>
</input>

<select1 ref="/data/changes" appearance="minimal">
Expand Down
176 changes: 95 additions & 81 deletions test/spec/setgeopoint.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,18 @@ import events from '../../src/js/event';
*/

describe('setgeopoint action', () => {
const actionInBodyNodes = {
visible_first_load_adjacent_action: 'action defined adjacent to input',
visible_first_load_nested_explicit_ref:
'action nested in input with explicit ref',
visible_first_load_nested_implied_ref:
'action nested in input with ref determined by parent',
};
const firstLoadeNodes = {
...actionInBodyNodes,
visible_first_load_model_action: 'action defined in model',
};

describe('first load', () => {
const mock = mockGetCurrentPosition(
createTestCoordinates({
Expand All @@ -37,94 +49,96 @@ describe('setgeopoint action', () => {
.then(done, done);
});

it('works for questions with odk-instance-first-load inside of the XForms body', (done) => {
const form1 = loadForm('setgeopoint.xml');
form1.init();

mock.lookup
.then(({ geopoint }) => {
expect(
form1.model.xml.querySelector('visible_first_load')
.textContent
).to.equal(geopoint);
})
.then(done, done);
});
for (const [nodeName, description] of Object.entries(
actionInBodyNodes
)) {
it(`works for questions with odk-instance-first-load inside of the XForms body (${description})`, (done) => {
const form1 = loadForm('setgeopoint.xml');
form1.init();

mock.lookup
.then(({ geopoint }) => {
expect(
form1.model.xml.querySelector(nodeName).textContent
).to.equal(geopoint);
})
.then(done, done);
});
}
});

describe('null `accuracy`', () => {
const mock = mockGetCurrentPosition(
createTestCoordinates({
latitude: 48.66,
longitude: -120.5,
altitude: 123,
}),
{ expectLookup: true }
);

it('substitutes a null `accuracy` value with 0.0', (done) => {
const form1 = loadForm('setgeopoint.xml');
form1.init();

mock.lookup
.then(({ geopoint }) => {
expect(
form1.model.xml.querySelector('visible_first_load')
.textContent
).to.equal(geopoint);
expect(geopoint).to.equal('48.66 -120.5 123 0.0');
})
.then(done, done);
for (const [nodeName, description] of Object.entries(firstLoadeNodes)) {
describe(`null 'accuracy' (${description})`, () => {
const mock = mockGetCurrentPosition(
createTestCoordinates({
latitude: 48.66,
longitude: -120.5,
altitude: 123,
}),
{ expectLookup: true }
);

it('substitutes a null `accuracy` value with 0.0', (done) => {
const form1 = loadForm('setgeopoint.xml');
form1.init();

mock.lookup
.then(({ geopoint }) => {
expect(
form1.model.xml.querySelector(nodeName).textContent
).to.equal(geopoint);
expect(geopoint).to.equal('48.66 -120.5 123 0.0');
})
.then(done, done);
});
});
});

describe('null `altitude`', () => {
const mock = mockGetCurrentPosition(
createTestCoordinates({
latitude: 48.66,
longitude: -120.5,
accuracy: 2500.12,
}),
{ expectLookup: true }
);

it('substitutes a null `altitude` value with 0.0', (done) => {
const form1 = loadForm('setgeopoint.xml');
form1.init();

mock.lookup
.then(({ geopoint }) => {
expect(
form1.model.xml.querySelector('visible_first_load')
.textContent
).to.equal(geopoint);
expect(geopoint).to.include('48.66 -120.5 0.0 2500.12');
})
.then(done, done);
describe(`null 'altitude' (${description})`, () => {
const mock = mockGetCurrentPosition(
createTestCoordinates({
latitude: 48.66,
longitude: -120.5,
accuracy: 2500.12,
}),
{ expectLookup: true }
);

it('substitutes a null `altitude` value with 0.0', (done) => {
const form1 = loadForm('setgeopoint.xml');
form1.init();

mock.lookup
.then(({ geopoint }) => {
expect(
form1.model.xml.querySelector(nodeName).textContent
).to.equal(geopoint);
expect(geopoint).to.include('48.66 -120.5 0.0 2500.12');
})
.then(done, done);
});
});
});

describe('lookup failure', () => {
const mock = mockGetCurrentPosition(
createGeolocationLookupError('PERMISSION_DENIED'),
{ expectLookup: true }
);

it('sets an empty string when lookup fails', (done) => {
const form1 = loadForm('setgeopoint.xml');
form1.init();

mock.lookup
.then(({ geopoint }) => {
expect(
form1.model.xml.querySelector('visible_first_load')
.textContent
).to.equal(geopoint);
expect(geopoint).to.include('');
})
.then(done, done);
describe(`lookup failure (${description})`, () => {
const mock = mockGetCurrentPosition(
createGeolocationLookupError('PERMISSION_DENIED'),
{ expectLookup: true }
);

it('sets an empty string when lookup fails', (done) => {
const form1 = loadForm('setgeopoint.xml');
form1.init();

mock.lookup
.then(({ geopoint }) => {
expect(
form1.model.xml.querySelector(nodeName).textContent
).to.equal(geopoint);
expect(geopoint).to.include('');
})
.then(done, done);
});
});
});
}
});

describe('setgeopoint actions to populate a value if another value changes', () => {
Expand Down
24 changes: 24 additions & 0 deletions test/spec/widget.geo.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,4 +132,28 @@ describe('geoshape widget', () => {

expect(geopickers.length).to.equal(0);
});

it('loads the widget when a setgeopoint odk-instance-first-load action is defined adjacent to the input', () => {
const form = loadForm('setgeopoint.xml');

form.init();

const widget = form.view.html.querySelector(
'[name="/data/visible_first_load_adjacent_action"] ~ .geopicker'
);

expect(widget).not.to.be.null;
});

it('loads the widget when a setgeopoint odk-instance-first-load action is defined in the model', () => {
const form = loadForm('setgeopoint.xml');

form.init();

const widget = form.view.html.querySelector(
'[name="/data/visible_first_load_model_action"] ~ .geopicker'
);

expect(widget).not.to.be.null;
});
});