From 841f215ff83106e2aa9c8c45ae70c420dd8a052f Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Tue, 22 Aug 2023 16:15:38 -0700 Subject: [PATCH] Validate savetos on structural fields (#952) * Add more dataset validation tests * Disallowed savetos in repeat groups * Updated tests a little bit --- lib/data/schema.js | 5 +- test/integration/api/datasets.js | 108 ++++++++++++++++++++ test/unit/data/schema.js | 165 +++++++++++++++++++++++++++++++ 3 files changed, 277 insertions(+), 1 deletion(-) diff --git a/lib/data/schema.js b/lib/data/schema.js index ea7cf1b6a..e3b242b5c 100644 --- a/lib/data/schema.js +++ b/lib/data/schema.js @@ -82,8 +82,11 @@ const _recurseFormFields = (instance, bindings, repeats, selectMultiples, envelo if (binding != null) { field.type = binding.type || 'unknown'; // binding should have a type. const prop = saveToAttr(binding); - if (prop) + if (prop) { + if (Array.from(repeats).find((repeat) => bindingPath.startsWith(repeat))) + throw Problem.user.invalidEntityForm({ reason: 'Currently, entities cannot be populated from fields in repeat groups.' }); field.propertyName = binding[prop]; + } } else if (tag.children != null) { // if we have no binding node but we have children, assume this is a // structural node with no repeat or direct data binding; recurse. diff --git a/test/integration/api/datasets.js b/test/integration/api/datasets.js index dbea89bf9..23c7595c1 100644 --- a/test/integration/api/datasets.js +++ b/test/integration/api/datasets.js @@ -2030,6 +2030,114 @@ describe('datasets and entities', () => { })); })); + it('should ignore a saveto incorrrectly placed on a bind on a structural field', testService(async (service) => { + const alice = await service.login('alice'); + const xml = ` + + + + + + + + + + + + + + + + + + + + + `; + await alice.post('/v1/projects/1/forms') + .send(xml) + .set('Content-Type', 'application/xml') + .expect(200); + await alice.get('/v1/projects/1/forms/validate_structure/draft/dataset-diff') + .expect(200) + .then(({ body }) => { + const { properties } = body[0]; + properties.length.should.equal(2); + properties[0].name.should.equal('prop1'); + properties[1].name.should.equal('prop3'); + }); + await alice.post('/v1/projects/1/forms/validate_structure/draft/publish') + .expect(200); + await alice.get('/v1/projects/1/datasets/things') + .expect(200) + .then(({ body }) => { + body.name.should.be.eql('things'); + const { properties } = body; + properties.length.should.equal(2); + properties[0].name.should.equal('prop1'); + properties[1].name.should.equal('prop3'); + }); + })); + + it('should throw an error when a saveto is on a field inside a repeat group', testService(async (service) => { + // Entities made from repeat groups are not yet supported. pyxform also throws an error about this. + const form = ` + + + Repeat Children Entities + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + `; + const alice = await service.login('alice'); + await alice.post('/v1/projects/1/forms?publish=true') + .send(form) + .set('Content-Type', 'application/xml') + .expect(400) + .then(({ body }) => { + body.code.should.equal(400.25); + body.details.reason.should.equal('Currently, entities cannot be populated from fields in repeat groups.'); + }); + })); + it('should publish dataset when any dataset creating form is published', testService(async (service) => { const alice = await service.login('alice'); diff --git a/test/unit/data/schema.js b/test/unit/data/schema.js index b32cb5168..6eab1454d 100644 --- a/test/unit/data/schema.js +++ b/test/unit/data/schema.js @@ -524,6 +524,171 @@ describe('form schema', () => { ]); }); }); + + describe('datasets', () => { + it('should ignore entities:saveto in bindings on structural nodes', () => { // gh cb#670 + // binds must have a 'type' attribute to be picked up by XML parsing. + const xml = ` + + + + + + + + + + <dates> + <joined/> + <departed/> + </dates> + <salary/> + </occupation> + </data> + </instance> + <bind nodeset="/data/name" type="string"/> + <bind nodeset="/data/occupation" relevant="/data/name='liz'" entities:saveto="occupation"/> + <bind nodeset="/data/occupation/title" type="string"/> + <bind nodeset="/data/occupation/dates" relevant="true()"/> + <bind nodeset="/data/occupation/dates/joined" type="date"/> + <bind nodeset="/data/occupation/dates/departed" type="date"/> + <bind nodeset="/data/occupation/salary" type="decimal"/> + </model> + </h:head> + </h:html>`; + return getFormFields(xml).then((schema) => { + schema.should.eql([ + { name: 'name', path: '/name', type: 'string', order: 0 }, + { name: 'occupation', path: '/occupation', type: 'structure', order: 1 }, + { name: 'title', path: '/occupation/title', type: 'string', order: 2 }, + { name: 'dates', path: '/occupation/dates', type: 'structure', order: 3 }, + { name: 'joined', path: '/occupation/dates/joined', type: 'date', order: 4 }, + { name: 'departed', path: '/occupation/dates/departed', type: 'date', order: 5 }, + { name: 'salary', path: '/occupation/salary', type: 'decimal', order: 6 } + ]); + }); + }); + + it('should reject binds on fields in repeats', () => { // gh cb#670 + const xml = ` + <?xml version="1.0"?> + <h:html xmlns="http://www.w3.org/2002/xforms" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:jr="http://openrosa.org/javarosa"> + <h:head> + <model> + <instance> + <data id="form"> + <name/> + <children> + <child> + <name/> + <toy> + <name/> + </toy> + </child> + </children> + </data> + </instance> + <bind nodeset="/data/name" type="string" entities:saveto="parent_name"/> + <bind nodeset="/data/children/child/name" type="string" entities:saveto="child_name"/> + <bind nodeset="/data/children/child/toy/name" type="string"/> + </model> + </h:head> + <h:body> + <input ref="/data/name"> + <label>What is your name?</label> + </input> + <group ref="/data/children/child"> + <label>Child</label> + <repeat nodeset="/data/children/child"> + <input ref="/data/children/child/name"> + <label>What is the child's name?</label> + </input> + <group ref="/data/children/child/toy"> + <label>Child</label> + <repeat nodeset="/data/children/child/toy"> + <input ref="/data/children/child/toy/name"> + <label>What is the toy's name?</label> + </input> + </repeat> + </group> + </repeat> + </group> + </h:body> + </h:html>`; + return getFormFields(xml).should.be.rejected().then((p) => p.problemCode.should.equal(400.25)); + }); + + it('should reject binds on fields in nested repeats inside groups', () => { // gh cb#670 + const xml = ` + <?xml version="1.0"?> + <h:html xmlns="http://www.w3.org/2002/xforms" xmlns:entities="http://www.opendatakit.org/xforms/entities" 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 Children Entities</h:title> + <model entities:entities-version="2022.1.0" odk:xforms-version="1.0.0"> + <instance> + <data id="repeat_entity" version="1"> + <num_children/> + <children> + <child jr:template=""> + <child_name/> + <possessions> + <toys jr:template=""> + <toy/> + </toys> + </possessions> + </child> + </children> + <meta> + <instanceID/> + <instanceName/> + <entity create="1" dataset="children" id=""> + <label/> + </entity> + </meta> + </data> + </instance> + <bind nodeset="/data/num_children" type="int"/> + <bind nodeset="/data/children/child/child_name" type="string"/> + <bind entities:saveto="toy_name" nodeset="/data/children/child/possessions/toys/toy" type="string"/> + <bind jr:preload="uid" nodeset="/data/meta/instanceID" readonly="true()" type="string"/> + <bind calculate=" /data/num_children " nodeset="/data/meta/instanceName" type="string"/> + <bind calculate="1" nodeset="/data/meta/entity/@create" readonly="true()" type="string"/> + <bind nodeset="/data/meta/entity/@id" readonly="true()" type="string"/> + <setvalue event="odk-instance-first-load" readonly="true()" ref="/data/meta/entity/@id" type="string" value="uuid()"/> + <bind calculate="concat("Num children:", /data/num_children )" nodeset="/data/meta/entity/label" readonly="true()" type="string"/> + </model> + </h:head> + <h:body> + <input ref="/data/num_children"> + <label>Num Children</label> + </input> + <group ref="/data/children"> + <label>Children</label> + <group ref="/data/children/child"> + <label>Child</label> + <repeat nodeset="/data/children/child"> + <input ref="/data/children/child/child_name"> + <label>Child Name</label> + </input> + <group ref="/data/children/child/possessions"> + <label>Posessions</label> + <group ref="/data/children/child/possessions/toys"> + <label>Toys</label> + <repeat nodeset="/data/children/child/possessions/toys"> + <input ref="/data/children/child/possessions/toys/toy"> + <label>Toy</label> + </input> + </repeat> + </group> + </group> + </repeat> + </group> + </group> + </h:body> + </h:html>`; + return getFormFields(xml).should.be.rejected().then((p) => p.problemCode.should.equal(400.25)); + }); + }); }); describe('SchemaStack', () => {