Skip to content

Commit d1c74ef

Browse files
ktuitektuite
authored
Fix compare to not match select and select1 fields (getodk#757)
* Fix field compare to not match select and select1 fields * Refining non-true comparison
1 parent 2089f63 commit d1c74ef

File tree

4 files changed

+154
-5
lines changed

4 files changed

+154
-5
lines changed

‎lib/data/schema.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,10 @@ const compare = (prevFields, fields) => {
432432
for (let i = 0; i < fields.length; i += 1) {
433433
if (!(prevFields[i].path === fields[i].path &&
434434
prevFields[i].order === fields[i].order &&
435-
prevFields[i].type === fields[i].type)) {
435+
prevFields[i].type === fields[i].type &&
436+
// field.selectMultiple may be true, null, or undefined.
437+
// should match if both are true, or both are something other than true.
438+
(prevFields[i].selectMultiple === true) === (fields[i].selectMultiple === true))) {
436439
return false;
437440
}
438441
}

‎test/integration/api/datasets.js‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -257,10 +257,9 @@ describe('datasets and entities', () => {
257257
createdAt.should.not.be.null();
258258
lastEntity.should.not.be.null();
259259
return d;
260-
}).should.eql([
261-
{ name: 'people', projectId: 1, entities: 2 },
262-
{ name: 'trees', projectId: 1, entities: 1 }
263-
]);
260+
}).reduce((a, v) => ({ ...a, [v.name]: v.entities }), {}).should.eql({
261+
people: 2, trees: 1
262+
});
264263
});
265264
}));
266265
});

‎test/integration/other/select-many.js‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,5 +83,72 @@ describe('select many value processing', () => {
8383
{ formId, submissionDefId: one2, path: '/g1/q2', value: 'xyz' }
8484
]);
8585
}))));
86+
87+
it('should handle when field changes from select1 to select (selectMultiple)', testService(async (service, container) => {
88+
const asAlice = await service.login('alice');
89+
90+
// the select1 version of forms.selectMultple
91+
const selectOne = `<?xml version="1.0"?>
92+
<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">
93+
<h:head>
94+
<model>
95+
<instance>
96+
<data id="selectMultiple">
97+
<q1/>
98+
<g1><q2/></g1>
99+
</data>
100+
</instance>
101+
<bind nodeset="/data/q1" type="string"/>
102+
<bind nodeset="/data/g1/q2" type="string"/>
103+
</model>
104+
</h:head>
105+
<h:body>
106+
<select1 ref="/data/q1"><label>one</label></select1>
107+
<group ref="/data/g1">
108+
<label>group</label>
109+
<select1 ref="/data/g1/q2"><label>two</label></select1>
110+
</group>
111+
</h:body>
112+
</h:html>`;
113+
114+
await asAlice.post('/v1/projects/1/forms?publish=true')
115+
.send(selectOne)
116+
.set('Content-Type', 'application/xml')
117+
.expect(200);
118+
119+
// <q1>b</q1><g1><q2>x</q2>
120+
await asAlice.post('/v1/projects/1/forms/selectMultiple/submissions')
121+
.send(testData.instances.selectMultiple.two.replace('m x', 'x')) // just send one value for each field
122+
.set('Content-Type', 'application/xml')
123+
.expect(200);
124+
125+
// upload new version of form where select multiple is now allowed
126+
await asAlice.post('/v1/projects/1/forms/selectMultiple/draft')
127+
.set('Content-Type', 'application/xml')
128+
.send(testData.forms.selectMultiple)
129+
.expect(200);
130+
131+
await asAlice.post('/v1/projects/1/forms/selectMultiple/draft/publish?version=2')
132+
.expect(200);
133+
134+
//<q1>a b</q1><g1><q2>x y z</q2>
135+
await asAlice.post('/v1/projects/1/forms/selectMultiple/submissions')
136+
.send(testData.instances.selectMultiple.one.replace('id="selectMultiple"', 'id="selectMultiple" version="2"'))
137+
.set('Content-Type', 'text/xml')
138+
.expect(200);
139+
140+
await exhaust(container);
141+
142+
await asAlice.get('/v1/projects/1/forms/selectMultiple/submissions.csv?splitSelectMultiples=true')
143+
.expect(200)
144+
.then(({ text }) => {
145+
const lines = text.split('\n');
146+
lines[0].should.equal('SubmissionDate,q1,q1/a,q1/b,g1-q2,g1-q2/x,g1-q2/y,g1-q2/z,KEY,SubmitterID,SubmitterName,AttachmentsPresent,AttachmentsExpected,Status,ReviewState,DeviceID,Edits,FormVersion');
147+
lines[1].slice('yyyy-mm-ddThh:mm:ss._msZ'.length)
148+
.should.equal(',a b,1,1,x y z,1,1,1,one,5,Alice,0,0,,,,0,2');
149+
lines[2].slice('yyyy-mm-ddThh:mm:ss._msZ'.length)
150+
.should.equal(',b,0,1,x,1,0,0,two,5,Alice,0,0,,,,0,');
151+
});
152+
}));
86153
});
87154

‎test/unit/data/schema.js‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1157,6 +1157,86 @@ describe('form schema', () => {
11571157
compare(a, b).should.be.false();
11581158
compare(b, a).should.be.false(); // try both directions
11591159
}));
1160+
1161+
it('should say selectMultiple matches selectMultiple', () => Promise.all([
1162+
fieldsFor(testData.forms.selectMultiple),
1163+
fieldsFor(testData.forms.selectMultiple)
1164+
]).then(([ a, b ]) => {
1165+
compare(a, b).should.be.true();
1166+
compare(b, a).should.be.true(); // try both directions
1167+
}));
1168+
1169+
// this doesn't actually come up, but compare() ought to handle it
1170+
it('should compare fields with selectMultiple=false and =null or undefined', () => {
1171+
// comparing false and null (should match)
1172+
// comparing false and undefined (should match)
1173+
const a = [
1174+
{
1175+
name: 'q1',
1176+
path: '/q1',
1177+
order: 0,
1178+
type: 'string',
1179+
selectMultiple: false
1180+
},
1181+
{
1182+
name: 'q2',
1183+
path: '/q2',
1184+
order: 0,
1185+
type: 'string',
1186+
selectMultiple: false
1187+
}
1188+
];
1189+
const b = [
1190+
{
1191+
name: 'q1',
1192+
path: '/q1',
1193+
order: 0,
1194+
type: 'string',
1195+
selectMultiple: null
1196+
},
1197+
{
1198+
name: 'q2',
1199+
path: '/q2',
1200+
order: 0,
1201+
type: 'string'
1202+
// selectMultple is undefined
1203+
}
1204+
];
1205+
compare(a, b).should.be.true();
1206+
compare(b, a).should.be.true(); // try both directions
1207+
});
1208+
1209+
it('should say select1 and selectMultiple are different', () => {
1210+
const selectOne = `<?xml version="1.0"?>
1211+
<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">
1212+
<h:head>
1213+
<model>
1214+
<instance>
1215+
<data id="selectMultiple">
1216+
<q1/>
1217+
<g1><q2/></g1>
1218+
</data>
1219+
</instance>
1220+
<bind nodeset="/data/q1" type="string"/>
1221+
<bind nodeset="/data/g1/q2" type="string"/>
1222+
</model>
1223+
</h:head>
1224+
<h:body>
1225+
<select1 ref="/data/q1"><label>one</label></select1>
1226+
<group ref="/data/g1">
1227+
<label>group</label>
1228+
<select1 ref="/data/g1/q2"><label>two</label></select1>
1229+
</group>
1230+
</h:body>
1231+
</h:html>`;
1232+
return Promise.all([
1233+
fieldsFor(testData.forms.selectMultiple),
1234+
fieldsFor(selectOne)
1235+
]).then(([ a, b ]) => {
1236+
compare(a, b).should.be.false();
1237+
compare(b, a).should.be.false(); // try both directions
1238+
});
1239+
});
11601240
});
11611241

11621242
describe('expectedFormAttachments', () => {

0 commit comments

Comments
 (0)