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

@@ -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;
     }
   }

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
+            });
           });
       }));
     });

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 = `<?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="selectMultiple">
+                <q1/>
+                <g1><q2/></g1>
+              </data>
+            </instance>
+            <bind nodeset="/data/q1" type="string"/>
+            <bind nodeset="/data/g1/q2" type="string"/>
+          </model>
+        </h:head>
+        <h:body>
+          <select1 ref="/data/q1"><label>one</label></select1>
+          <group ref="/data/g1">
+            <label>group</label>
+            <select1 ref="/data/g1/q2"><label>two</label></select1>
+          </group>
+        </h:body>
+      </h:html>`;
+
+    await asAlice.post('/v1/projects/1/forms?publish=true')
+      .send(selectOne)
+      .set('Content-Type', 'application/xml')
+      .expect(200);
+
+    // <q1>b</q1><g1><q2>x</q2>
+    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);
+
+    //<q1>a b</q1><g1><q2>x y z</q2>
+    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,');
+      });
+  }));
 });
 

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 = `<?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="selectMultiple">
+                <q1/>
+                <g1><q2/></g1>
+              </data>
+            </instance>
+            <bind nodeset="/data/q1" type="string"/>
+            <bind nodeset="/data/g1/q2" type="string"/>
+          </model>
+        </h:head>
+        <h:body>
+          <select1 ref="/data/q1"><label>one</label></select1>
+          <group ref="/data/g1">
+            <label>group</label>
+            <select1 ref="/data/g1/q2"><label>two</label></select1>
+          </group>
+        </h:body>
+      </h:html>`;
+      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', () => {