From d1c74ef9bf6af943f81fcc9785736dbac3bfcf73 Mon Sep 17 00:00:00 2001 From: Kathleen Tuite Date: Mon, 30 Jan 2023 15:22:09 -0800 Subject: [PATCH] Fix compare to not match select and select1 fields (#757) * Fix field compare to not match select and select1 fields * Refining non-true comparison --- lib/data/schema.js | 5 +- test/integration/api/datasets.js | 7 +-- test/integration/other/select-many.js | 67 ++++++++++++++++++++++ test/unit/data/schema.js | 80 +++++++++++++++++++++++++++ 4 files changed, 154 insertions(+), 5 deletions(-) diff --git a/lib/data/schema.js b/lib/data/schema.js index 73e59e4c7..ea7cf1b6a 100644 --- a/lib/data/schema.js +++ b/lib/data/schema.js @@ -432,7 +432,10 @@ const compare = (prevFields, fields) => { for (let i = 0; i < fields.length; i += 1) { if (!(prevFields[i].path === fields[i].path && prevFields[i].order === fields[i].order && - prevFields[i].type === fields[i].type)) { + prevFields[i].type === fields[i].type && + // field.selectMultiple may be true, null, or undefined. + // should match if both are true, or both are something other than true. + (prevFields[i].selectMultiple === true) === (fields[i].selectMultiple === true))) { return false; } } diff --git a/test/integration/api/datasets.js b/test/integration/api/datasets.js index 7762f42a6..810311b38 100644 --- a/test/integration/api/datasets.js +++ b/test/integration/api/datasets.js @@ -257,10 +257,9 @@ describe('datasets and entities', () => { createdAt.should.not.be.null(); lastEntity.should.not.be.null(); return d; - }).should.eql([ - { name: 'people', projectId: 1, entities: 2 }, - { name: 'trees', projectId: 1, entities: 1 } - ]); + }).reduce((a, v) => ({ ...a, [v.name]: v.entities }), {}).should.eql({ + people: 2, trees: 1 + }); }); })); }); diff --git a/test/integration/other/select-many.js b/test/integration/other/select-many.js index ae521faa7..5c31ea232 100644 --- a/test/integration/other/select-many.js +++ b/test/integration/other/select-many.js @@ -83,5 +83,72 @@ describe('select many value processing', () => { { formId, submissionDefId: one2, path: '/g1/q2', value: 'xyz' } ]); })))); + + it('should handle when field changes from select1 to select (selectMultiple)', testService(async (service, container) => { + const asAlice = await service.login('alice'); + + // the select1 version of forms.selectMultple + const selectOne = ` + + + + + + + + + + + + + + + + + + + + + `; + + await asAlice.post('/v1/projects/1/forms?publish=true') + .send(selectOne) + .set('Content-Type', 'application/xml') + .expect(200); + + // bx + await asAlice.post('/v1/projects/1/forms/selectMultiple/submissions') + .send(testData.instances.selectMultiple.two.replace('m x', 'x')) // just send one value for each field + .set('Content-Type', 'application/xml') + .expect(200); + + // upload new version of form where select multiple is now allowed + await asAlice.post('/v1/projects/1/forms/selectMultiple/draft') + .set('Content-Type', 'application/xml') + .send(testData.forms.selectMultiple) + .expect(200); + + await asAlice.post('/v1/projects/1/forms/selectMultiple/draft/publish?version=2') + .expect(200); + + //a bx y z + await asAlice.post('/v1/projects/1/forms/selectMultiple/submissions') + .send(testData.instances.selectMultiple.one.replace('id="selectMultiple"', 'id="selectMultiple" version="2"')) + .set('Content-Type', 'text/xml') + .expect(200); + + await exhaust(container); + + await asAlice.get('/v1/projects/1/forms/selectMultiple/submissions.csv?splitSelectMultiples=true') + .expect(200) + .then(({ text }) => { + const lines = text.split('\n'); + 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'); + lines[1].slice('yyyy-mm-ddThh:mm:ss._msZ'.length) + .should.equal(',a b,1,1,x y z,1,1,1,one,5,Alice,0,0,,,,0,2'); + lines[2].slice('yyyy-mm-ddThh:mm:ss._msZ'.length) + .should.equal(',b,0,1,x,1,0,0,two,5,Alice,0,0,,,,0,'); + }); + })); }); diff --git a/test/unit/data/schema.js b/test/unit/data/schema.js index 2af0dbc06..b32cb5168 100644 --- a/test/unit/data/schema.js +++ b/test/unit/data/schema.js @@ -1157,6 +1157,86 @@ describe('form schema', () => { compare(a, b).should.be.false(); compare(b, a).should.be.false(); // try both directions })); + + it('should say selectMultiple matches selectMultiple', () => Promise.all([ + fieldsFor(testData.forms.selectMultiple), + fieldsFor(testData.forms.selectMultiple) + ]).then(([ a, b ]) => { + compare(a, b).should.be.true(); + compare(b, a).should.be.true(); // try both directions + })); + + // this doesn't actually come up, but compare() ought to handle it + it('should compare fields with selectMultiple=false and =null or undefined', () => { + // comparing false and null (should match) + // comparing false and undefined (should match) + const a = [ + { + name: 'q1', + path: '/q1', + order: 0, + type: 'string', + selectMultiple: false + }, + { + name: 'q2', + path: '/q2', + order: 0, + type: 'string', + selectMultiple: false + } + ]; + const b = [ + { + name: 'q1', + path: '/q1', + order: 0, + type: 'string', + selectMultiple: null + }, + { + name: 'q2', + path: '/q2', + order: 0, + type: 'string' + // selectMultple is undefined + } + ]; + compare(a, b).should.be.true(); + compare(b, a).should.be.true(); // try both directions + }); + + it('should say select1 and selectMultiple are different', () => { + const selectOne = ` + + + + + + + + + + + + + + + + + + + + + `; + return Promise.all([ + fieldsFor(testData.forms.selectMultiple), + fieldsFor(selectOne) + ]).then(([ a, b ]) => { + compare(a, b).should.be.false(); + compare(b, a).should.be.false(); // try both directions + }); + }); }); describe('expectedFormAttachments', () => {