Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce that entity field is always structure type #1053

Merged
merged 10 commits into from
Dec 7, 2023
Merged

Enforce that entity field is always structure type #1053

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6110d9e
Tests showing failure
ktuite Nov 29, 2023
b0d1813
Separated entity-specific submission parsing code
ktuite Nov 29, 2023
58c1df5
Test demonstrating issue #522
ktuite Dec 1, 2023
ed58fe2
A test demonstrating issue #553
ktuite Dec 1, 2023
3bab8dd
Forcing /meta/entity field to be type structure
ktuite Dec 1, 2023
fb95b8c
Tidying up tests to be clearer to read in the future
ktuite Dec 5, 2023
529f444
Tests to start looking at impact of empty structure entity field
ktuite Dec 5, 2023
6f34d99
More tests of odata
ktuite Dec 5, 2023
76bfdb8
Removed console.log from exploratory test to make circleci happy
ktuite Dec 5, 2023
4b56426
Small final test changes
ktuite Dec 6, 2023
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
4 changes: 4 additions & 0 deletions lib/data/schema.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ const _recurseFormFields = (instance, bindings, repeats, selectMultiples, envelo
// if we have no binding node but we have children, assume this is a
// structural node with no repeat or direct data binding; recurse.
field.type = 'structure';
} else if (tag.name === 'entity' && joinedPath === '/meta/entity') {
ktuite marked this conversation as resolved.
Show resolved Hide resolved
// if we have encountered the entity tag, but it has no children (e.g. label)
// still mark it as type structure.
field.type = 'structure';
} else {
field.type = 'unknown'; // something is wrong.
}
Expand Down
237 changes: 237 additions & 0 deletions test/integration/api/datasets.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2864,6 +2864,243 @@ describe('datasets and entities', () => {
person.currentVersion.should.have.property('data').which.is.eql({ first_name: 'Robert', hometown: 'Seattle' });
});
}));

// cb#551 issue, <entity/> tag has no children
it('should allow update where no label or no properties are updated', testService(async (service, container) => {
const asAlice = await service.login('alice');

const form = `<?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:entities="http://www.opendatakit.org/xforms">
<h:head>
<model entities:entities-version="2023.1.0">
<instance>
<data id="brokenForm" orx:version="1.0">
<age foo="bar"/>
<meta>
<entity dataset="people" id="" create="" update="" baseVersion="" />
</meta>
</data>
</instance>
<bind nodeset="/data/age" type="int" entities:saveto="age"/>
</model>
</h:head>
</h:html>`;

await asAlice.post('/v1/projects/1/forms?publish=true')
.send(form)
.set('Content-Type', 'application/xml')
.expect(200);

await asAlice.post('/v1/projects/1/datasets/people/entities')
.send({
uuid: '12345678-1234-4123-8234-123456789abc',
label: 'First Label',
data: { age: '11' }
})
.expect(200);

const sub = `<data xmlns:jr="http://openrosa.org/javarosa" xmlns:entities="http://www.opendatakit.org/xforms" id="brokenForm" version="1.0">
<meta>
<instanceID>one</instanceID>
<orx:instanceName>one</orx:instanceName>
<entity baseVersion="1" dataset="people" id="12345678-1234-4123-8234-123456789abc" update="1" />
</meta>
</data>`;

await asAlice.post('/v1/projects/1/forms/brokenForm/submissions')
.send(sub)
.expect(200);

await exhaust(container);

await asAlice.get('/v1/projects/1/datasets/people/entities/12345678-1234-4123-8234-123456789abc/versions')
.expect(200)
.then(({ body: versions }) => {
versions[1].version.should.equal(2);
versions[1].baseVersion.should.equal(1);
versions[1].label.should.equal('First Label');
versions[1].data.should.eql({ age: '11' });
versions[1].dataReceived.should.eql({});
});

await asAlice.get('/v1/projects/1/forms/brokenForm/submissions/one/audits')
.expect(200)
.then(({ body: logs }) => {
logs[0].action.should.equal('entity.update.version');
});
}));

// cb#552 issue, can't add label to entity update form that previously didnt have label
it('should allow update where no label or no properties are updated', testService(async (service) => {
ktuite marked this conversation as resolved.
Show resolved Hide resolved
const asAlice = await service.login('alice');

const form = `<?xml version="1.0"?>
<h:html xmlns:entities="http://www.opendatakit.org/xforms">
<h:head>
<model entities:entities-version="2023.1.0">
<instance>
<data id="brokenForm" orx:version="1.0">
<age foo="bar"/>
<meta>
<entity dataset="people" id="" create="" update="" baseVersion="" />
</meta>
</data>
</instance>
<bind nodeset="/data/age" type="int" entities:saveto="age"/>
</model>
</h:head>
</h:html>`;

const form2 = `<?xml version="1.0"?>
<h:html xmlns:entities="http://www.opendatakit.org/xforms">
<h:head>
<model entities:entities-version="2023.1.0">
<instance>
<data id="brokenForm" orx:version="2.0">
<age foo="bar"/>
<meta>
<entity dataset="people" id="" create="" update="" baseVersion="">
<label/>
</entity>
</meta>
</data>
</instance>
<bind nodeset="/data/age" type="int" entities:saveto="age"/>
</model>
</h:head>
</h:html>`;

await asAlice.post('/v1/projects/1/forms?publish=true')
.send(form)
.set('Content-Type', 'application/xml')
.expect(200);

await asAlice.post('/v1/projects/1/forms/brokenForm/draft')
.send(form2)
.set('Content-Type', 'application/xml')
.expect(200);
}));

// cb#553 issue, forms with and without entity label show different fields
// (because entity has type 'unknown' instead of 'structure')
ktuite marked this conversation as resolved.
Show resolved Hide resolved
it('should allow update where no label or no properties are updated', testService(async (service) => {
ktuite marked this conversation as resolved.
Show resolved Hide resolved
const asAlice = await service.login('alice');

const form = `<?xml version="1.0"?>
<h:html xmlns:entities="http://www.opendatakit.org/xforms">
<h:head>
<model entities:entities-version="2023.1.0">
<instance>
<data id="updateWithoutLabel" orx:version="1.0">
<age foo="bar"/>
<meta>
<entity dataset="people" id="" update="" baseVersion="" />
</meta>
</data>
</instance>
<bind nodeset="/data/age" type="int" entities:saveto="age"/>
</model>
</h:head>
</h:html>`;

await asAlice.post('/v1/projects/1/forms?publish=true')
.send(form)
.set('Content-Type', 'application/xml')
.expect(200);

// Form with label nested under entity
const form2 = `<?xml version="1.0"?>
<h:html xmlns:entities="http://www.opendatakit.org/xforms">
<h:head>
<model entities:entities-version="2023.1.0">
<instance>
<data id="updateWithLabel" orx:version="1.0">
<age foo="bar"/>
<meta>
<entity dataset="people" id="" update="" baseVersion="">
<label/>
</entity>
</meta>
</data>
</instance>
<bind nodeset="/data/age" type="int" entities:saveto="age"/>
</model>
</h:head>
</h:html>`;

await asAlice.post('/v1/projects/1/forms?publish=true')
.send(form2)
.set('Content-Type', 'application/xml')
.expect(200);

// Compare form fields
await asAlice.get('/v1/projects/1/forms/updateWithoutLabel/fields?odata=true')
.then(({ body }) => {
body[2].path.should.equal('/meta/entity');
body[2].type.should.equal('structure');
});

await asAlice.get('/v1/projects/1/forms/updateWithLabel/fields?odata=true')
.then(({ body }) => {
body[2].path.should.equal('/meta/entity');
body[2].type.should.equal('structure');
ktuite marked this conversation as resolved.
Show resolved Hide resolved
});
}));

it('should gracefully handle error if incoming entity tag in sub has no attributes', testService(async (service, container) => {
const asAlice = await service.login('alice');

const form = `<?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:entities="http://www.opendatakit.org/xforms">
<h:head>
<model entities:entities-version="2023.1.0">
<instance>
<data id="brokenForm" orx:version="1.0">
<age foo="bar"/>
<meta>
<entity dataset="people" id="" update="" baseVersion="" />
</meta>
</data>
</instance>
<bind nodeset="/data/age" type="int" entities:saveto="age"/>
</model>
</h:head>
</h:html>`;

await asAlice.post('/v1/projects/1/forms?publish=true')
.send(form)
.set('Content-Type', 'application/xml')
.expect(200);

await asAlice.post('/v1/projects/1/datasets/people/entities')
.send({
uuid: '12345678-1234-4123-8234-123456789abc',
label: 'First Label',
data: { age: '11' }
})
.expect(200);

const sub = `<data xmlns:jr="http://openrosa.org/javarosa" xmlns:entities="http://www.opendatakit.org/xforms" id="brokenForm" version="1.0">
<meta>
<instanceID>one</instanceID>
<orx:instanceName>one</orx:instanceName>
<entity/>
</meta>
</data>`;

await asAlice.post('/v1/projects/1/forms/brokenForm/submissions')
.send(sub)
.expect(200);

await exhaust(container);

await asAlice.get('/v1/projects/1/forms/brokenForm/submissions/one/audits')
.expect(200)
.then(({ body: logs }) => {
logs[0].action.should.equal('entity.error');
logs[0].details.errorMessage.should.equal('Required parameter dataset missing.');
});
}));
});

describe('dataset and entities should have isolated lifecycle', () => {
Expand Down
87 changes: 87 additions & 0 deletions test/unit/data/dataset.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,93 @@ describe('parsing dataset from entity block', () => {
fields[2].path.should.equal('/hometown');
should.not.exist(fields[2].propertyName);
}));

it('should figure out type of entity block from form def', async () => {
const form = (entityBlock) => `<?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:entities="http://www.opendatakit.org/xforms">
<h:head>
<model entities:entities-version="2023.1.0">
<instance>
<data id="brokenForm" orx:version="1.0">
<age/>
<meta>
${entityBlock}
</meta>
</data>
</instance>
<bind nodeset="/data/age" type="int" entities:saveto="age"/>
</model>
</h:head>
</h:html>`;

const noChildEntity = '<entity dataset="people" id="" create="" update="" baseVersion=""/>';
await getFormFields(form(noChildEntity)).then((fields) => {
fields[0].name.should.equal('age');
fields[0].path.should.equal('/age');
fields[0].type.should.equal('int');

fields[1].name.should.equal('meta');
fields[1].path.should.equal('/meta');
fields[1].type.should.equal('structure');

fields[2].name.should.equal('entity');
fields[2].path.should.equal('/meta/entity');
fields[2].type.should.equal('structure');
});

const emptyEntity = '<entity dataset="people" id="" create="" update="" baseVersion=""></entity>';
await getFormFields(form(emptyEntity)).then((fields) => {
fields[0].name.should.equal('age');
fields[0].path.should.equal('/age');
fields[0].type.should.equal('int');

fields[1].name.should.equal('meta');
fields[1].path.should.equal('/meta');
fields[1].type.should.equal('structure');

fields[2].name.should.equal('entity');
fields[2].path.should.equal('/meta/entity');
fields[2].type.should.equal('structure');
});

const nonEmptyEntity = '<entity dataset="people" id="" create="" update="" baseVersion=""><label/></entity>';
await getFormFields(form(nonEmptyEntity)).then((fields) => {
fields[0].name.should.equal('age');
fields[0].path.should.equal('/age');
fields[0].type.should.equal('int');

fields[1].name.should.equal('meta');
fields[1].path.should.equal('/meta');
fields[1].type.should.equal('structure');

fields[2].name.should.equal('entity');
fields[2].path.should.equal('/meta/entity');
fields[2].type.should.equal('structure'); // is type structure because it contains a child

fields[3].name.should.equal('label');
fields[3].path.should.equal('/meta/entity/label');
fields[3].type.should.equal('unknown'); // should possibly be something other than unknown (unknown bc no binding)
});

const nonEmptyLabel = '<entity dataset="people" id="" create="" update="" baseVersion=""><label></label></entity>';
await getFormFields(form(nonEmptyLabel)).then((fields) => {
fields[0].name.should.equal('age');
fields[0].path.should.equal('/age');
fields[0].type.should.equal('int');

fields[1].name.should.equal('meta');
fields[1].path.should.equal('/meta');
fields[1].type.should.equal('structure');

fields[2].name.should.equal('entity');
fields[2].path.should.equal('/meta/entity');
fields[2].type.should.equal('structure'); // is type structure because it contains a child

fields[3].name.should.equal('label');
fields[3].path.should.equal('/meta/entity/label');
fields[3].type.should.equal('unknown'); // should possibly be something other than unknown (unknown bc no children and no binding)
});
});
});

describe('dataset name validation', () => {
Expand Down
Loading