Check for meta group on form upload (#759)

* Check for meta in form xml and update tests

* Only check for missingMeta on new forms

lib/model/query/forms.js

@@ -75,6 +75,9 @@ const createNew = (partial, project, publish = false) => async ({ run, Datasets,
     (partial.aux.key.isDefined() ? resolve(Option.none()) : getDataset(partial.xml)) // Don't parse dataset schema if Form has encryption key
   ]);
 
+  // Check that meta field (group containing instanceId and name) exists
+  await Forms.checkMeta(fields);
+
   // Check for xmlFormId collisions with previously deleted forms
   await Forms.checkDeletedForms(partial.xmlFormId, project.id);
   await Forms.rejectIfWarnings();
@@ -617,6 +620,13 @@ order by actors."displayName" asc`)
 ////////////////////////////////////////////////////////////////////////////////
 // CHECKING CONSTRAINTS, STRUCTURAL CHANGES, ETC.
 
+// This will check if a form contains a meta group (for capturing instanceID).
+// It is only to be used for newly uploaded forms.
+const checkMeta = (fields) => () =>
+  (fields.some((f) => (f.name === 'meta' && f.type === 'structure'))
+    ? resolve()
+    : reject(Problem.user.missingMeta()));
+
 const checkDeletedForms = (xmlFormId, projectId) => ({ maybeOne, context }) => (context.query.ignoreWarnings ? resolve() : maybeOne(sql`SELECT 1 FROM forms WHERE "xmlFormId" = ${xmlFormId} AND "projectId" = ${projectId} AND "deletedAt" IS NOT NULL LIMIT 1`)
   .then(deletedForm => {
     if (deletedForm.isDefined()) {
@@ -681,7 +691,7 @@ module.exports = {
   getByProjectId, getByProjectAndXmlFormId, getByProjectAndNumericId,
   getAllByAuth,
   getFields, getBinaryFields, getStructuralFields, getMergedFields,
-  rejectIfWarnings, checkDeletedForms, checkStructuralChange, checkFieldDowncast,
+  rejectIfWarnings, checkMeta, checkDeletedForms, checkStructuralChange, checkFieldDowncast,
   _newSchema,
   lockDefs, getAllSubmitters
 };

lib/util/problem.js

@@ -108,6 +108,9 @@ const problems = {
     // problem parsing the entity form
     datasetLinkNotAllowed: problem(400.26, () => 'Dataset can only be linked to attachments with "Data File" type.'),
 
+    // meta group in form definition is missing
+    missingMeta: problem(400.27, () => 'The form does not contain a \'meta\' group.'),
+
     // no detail information for security reasons.
     authenticationFailed: problem(401.2, () => 'Could not authenticate with the provided credentials.'),
 

test/data/xml.js

@@ -173,11 +173,15 @@ module.exports = {
     <model>
       <instance>
         <data id="withAttachments">
+          <meta>
+            <instanceID/>
+          </meta>
           <name/>
           <age/>
           <hometown/>
         </data>
       </instance>
+      <bind nodeset="/data/meta/instanceID" type="string" readonly="true()" calculate="concat('uuid:', uuid())"/>
       <instance id="mydata" src="badnoroot.xls"/>
       <instance id="seconddata" src="jr://files/badsubpath.csv"/>
       <instance id="thirddata" src="jr://file/goodone.csv"/>
@@ -202,10 +206,14 @@ module.exports = {
     <model>
       <instance>
         <data id="itemsets">
+          <meta>
+            <instanceID/>
+          </meta>
           <name/>
         </data>
       </instance>
       <instance id="itemsets" src="jr://file/itemsets.csv"/>
+      <bind nodeset="/data/meta/instanceID" type="string" readonly="true()" calculate="concat('uuid:', uuid())"/>
       <bind nodeset="/data/name" type="string"/>
     </model>
   </h:head>
@@ -316,10 +324,14 @@ module.exports = {
     <model>
       <instance>
         <data id="selectMultiple">
+          <meta>
+            <instanceID/>
+          </meta>
           <q1/>
           <g1><q2/></g1>
         </data>
       </instance>
+      <bind nodeset="/data/meta/instanceID" type="string" readonly="true()" calculate="concat('uuid:', uuid())"/>
       <bind nodeset="/data/q1" type="string"/>
       <bind nodeset="/data/g1/q2" type="string"/>
     </model>

test/integration/api/forms/draft.js

@@ -406,6 +406,35 @@ describe('api: /projects/:id/forms (drafts)', () => {
               .set('Content-Type', 'application/xml')
               .expect(200)))));
 
+
+      it('should allow new draft with missing meta group', testService(async (service) => {
+        // This case is not expected, but it mimics a different scenario where there are
+        // already meta-less forms in a user's central repo and they need to be able to update them
+        // without breaking their workflows.
+        const asAlice = await service.login('alice');
+
+        const simpleMissingMeta = `<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:h="http://www.w3.org/1999/xhtml">
+          <h:head>
+            <h:title>Simple</h:title>
+            <model>
+              <instance>
+                <data id="simple">
+                  <name/>
+                  <age/>
+                </data>
+              </instance>
+              <bind nodeset="/data/name" type="string"/>
+              <bind nodeset="/data/age" type="int"/>
+            </model>
+          </h:head>
+        </h:html>`;
+
+        await asAlice.post('/v1/projects/1/forms/simple/draft?ignoreWarnings=true')
+          .send(simpleMissingMeta)
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+      }));
+
       it('should identify attachments', testService((service) =>
         service.login('alice', (asAlice) =>
           asAlice.post('/v1/projects/1/forms/simple/draft?ignoreWarnings=true')

test/integration/api/forms/forms.js

@@ -438,6 +438,59 @@ describe('api: /projects/:id/forms (create, read, update)', () => {
         });
     }));
 
+    it('should reject form with missing meta group', testService(async (service) => {
+      const asAlice = await service.login('alice');
+
+      const missingMeta = `<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:h="http://www.w3.org/1999/xhtml">
+      <h:head>
+        <h:title>Missing Meta</h:title>
+        <model>
+          <instance>
+            <data id="missingMeta">
+              <name/>
+              <age/>
+            </data>
+          </instance>
+          <bind nodeset="/data/name" type="string"/>
+          <bind nodeset="/data/age" type="int"/>
+        </model>
+      </h:head>
+
+    </h:html>`;
+
+      await asAlice.post('/v1/projects/1/forms')
+        .send(missingMeta)
+        .set('Content-Type', 'application/xml')
+        .expect(400);
+    }));
+
+    it('should reject form with meta field that is not a group', testService(async (service) => {
+      const asAlice = await service.login('alice');
+
+      const missingMeta = `<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:h="http://www.w3.org/1999/xhtml">
+      <h:head>
+        <h:title>Non Group Meta</h:title>
+        <model>
+          <instance>
+            <data id="missingMeta">
+              <meta/>
+              <name/>
+              <age/>
+            </data>
+          </instance>
+          <bind nodeset="/data/name" type="string"/>
+          <bind nodeset="/data/age" type="int"/>
+        </model>
+      </h:head>
+
+    </h:html>`;
+
+      await asAlice.post('/v1/projects/1/forms')
+        .send(missingMeta)
+        .set('Content-Type', 'application/xml')
+        .expect(400);
+    }));
+
     it('should create the form for xml files with warnings given ignoreWarnings', testService(async (service) => {
       const asAlice = await service.login('alice', identity);
 
@@ -809,6 +862,8 @@ describe('api: /projects/:id/forms (create, read, update)', () => {
               .expect(200)
               .then(({ body }) => {
                 body.should.eql([
+                  { name: 'meta', path: '/meta', type: 'structure', binary: null, selectMultiple: null },
+                  { name: 'instanceID', path: '/meta/instanceID', type: 'string', binary: null, selectMultiple: null },
                   { name: 'q1', path: '/q1', type: 'string', binary: null, selectMultiple: true },
                   { name: 'g1', path: '/g1', type: 'structure', binary: null, selectMultiple: null },
                   { name: 'q2', path: '/g1/q2', type: 'string', binary: null, selectMultiple: true }
@@ -821,13 +876,16 @@ describe('api: /projects/:id/forms (create, read, update)', () => {
       <model>
         <instance>
           <data id="sanitize">
+            <meta>
+              <instanceID>
+            </meta>
             <q1.8>
               <17/>
             </q1.8>
             <4.2/>
           </data>
         </instance>
-
+        <bind nodeset="/data/meta/instanceID" type="string" readonly="true()" calculate="concat('uuid:', uuid())"/>
         <bind nodeset="/data/q1.8/17" type="string" readonly="true()" calculate="concat('uuid:', uuid())"/>
         <bind nodeset="/data/4.2" type="number"/>
       </model>
@@ -850,6 +908,8 @@ describe('api: /projects/:id/forms (create, read, update)', () => {
               .expect(200)
               .then(({ body }) => {
                 body.should.eql([
+                  { name: 'meta', path: '/meta', type: 'structure', binary: null, selectMultiple: null },
+                  { name: 'instanceID', path: '/meta/instanceID', type: 'string', binary: null, selectMultiple: null },
                   { name: 'q1_8', path: '/q1_8', type: 'structure', binary: null, selectMultiple: null },
                   { name: '_17', path: '/q1_8/_17', type: 'string', binary: null, selectMultiple: null },
                   { name: '_4_2', path: '/_4_2', type: 'number', binary: null, selectMultiple: null }

test/integration/api/forms/list.js

@@ -328,7 +328,7 @@ describe('api: /projects/:id/forms (listing forms)', () => {
       <formID>withAttachments</formID>
       <name>withAttachments</name>
       <version></version>
-      <hash>md5:7eb21b5b123b0badcf2b8f50bcf1cbd0</hash>
+      <hash>md5:dda89055c8fec222458f702aead30a83</hash>
       <downloadUrl>${domain}/v1/projects/1/forms/withAttachments.xml</downloadUrl>
       <manifestUrl>${domain}/v1/projects/1/forms/withAttachments/manifest</manifestUrl>
     </xform>

test/integration/api/forms/test.js

@@ -120,7 +120,7 @@ describe('api: /projects/:id/forms (testing drafts)', () => {
       <formID>withAttachments</formID>
       <name>withAttachments</name>
       <version></version>
-      <hash>md5:7eb21b5b123b0badcf2b8f50bcf1cbd0</hash>
+      <hash>md5:dda89055c8fec222458f702aead30a83</hash>
       <downloadUrl>${domain}/v1/test/${token}/projects/1/forms/withAttachments/draft.xml</downloadUrl>
       <manifestUrl>${domain}/v1/test/${token}/projects/1/forms/withAttachments/draft/manifest</manifestUrl>
     </xform>

test/integration/api/submissions.js

@@ -1575,11 +1575,11 @@ describe('api: /forms/:id/submissions', () => {
             .then((result) => {
               result.filenames.should.containDeep([ 'selectMultiple.csv' ]);
               const lines = result['selectMultiple.csv'].split('\n');
-              lines[0].should.equal('SubmissionDate,q1,q1/a,q1/b,g1-q2,g1-q2/m,g1-q2/x,g1-q2/y,g1-q2/z,KEY,SubmitterID,SubmitterName,AttachmentsPresent,AttachmentsExpected,Status,ReviewState,DeviceID,Edits,FormVersion');
+              lines[0].should.equal('SubmissionDate,meta-instanceID,q1,q1/a,q1/b,g1-q2,g1-q2/m,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(',b,0,1,m x,1,1,0,0,two,5,Alice,0,0,,,,0,');
+                .should.equal(',two,b,0,1,m x,1,1,0,0,two,5,Alice,0,0,,,,0,');
               lines[2].slice('yyyy-mm-ddThh:mm:ss._msZ'.length)
-                .should.equal(',a b,1,1,x y z,0,1,1,1,one,5,Alice,0,0,,,,0,');
+                .should.equal(',one,a b,1,1,x y z,0,1,1,1,one,5,Alice,0,0,,,,0,');
             })))));
 
     it('should omit multiples it does not know about', testService((service) =>
@@ -1597,11 +1597,11 @@ describe('api: /forms/:id/submissions', () => {
             .then((result) => {
               result.filenames.should.containDeep([ 'selectMultiple.csv' ]);
               const lines = result['selectMultiple.csv'].split('\n');
-              lines[0].should.equal('SubmissionDate,q1,g1-q2,KEY,SubmitterID,SubmitterName,AttachmentsPresent,AttachmentsExpected,Status,ReviewState,DeviceID,Edits,FormVersion');
+              lines[0].should.equal('SubmissionDate,meta-instanceID,q1,g1-q2,KEY,SubmitterID,SubmitterName,AttachmentsPresent,AttachmentsExpected,Status,ReviewState,DeviceID,Edits,FormVersion');
               lines[1].slice('yyyy-mm-ddThh:mm:ss._msZ'.length)
-                .should.equal(',b,m x,two,5,Alice,0,0,,,,0,');
+                .should.equal(',two,b,m x,two,5,Alice,0,0,,,,0,');
               lines[2].slice('yyyy-mm-ddThh:mm:ss._msZ'.length)
-                .should.equal(',a b,x y z,one,5,Alice,0,0,,,,0,');
+                .should.equal(',one,a b,x y z,one,5,Alice,0,0,,,,0,');
             })))));
 
     it('should split select multiples and filter given both options', testService((service, container) =>
@@ -1741,11 +1741,11 @@ describe('api: /forms/:id/submissions', () => {
             .then((result) => {
               result.filenames.should.containDeep([ 'selectMultiple.csv' ]);
               const lines = result['selectMultiple.csv'].split('\n');
-              lines[0].should.equal('SubmissionDate,q1,q1/a,q1/b,q2,q2/m,q2/x,q2/y,q2/z,KEY,SubmitterID,SubmitterName,AttachmentsPresent,AttachmentsExpected,Status,ReviewState,DeviceID,Edits,FormVersion');
+              lines[0].should.equal('SubmissionDate,instanceID,q1,q1/a,q1/b,q2,q2/m,q2/x,q2/y,q2/z,KEY,SubmitterID,SubmitterName,AttachmentsPresent,AttachmentsExpected,Status,ReviewState,DeviceID,Edits,FormVersion');
               lines[1].slice('yyyy-mm-ddThh:mm:ss._msZ'.length)
-                .should.equal(',b,0,1,m x,1,1,0,0,two,5,Alice,0,0,,,,0,');
+                .should.equal(',two,b,0,1,m x,1,1,0,0,two,5,Alice,0,0,,,,0,');
               lines[2].slice('yyyy-mm-ddThh:mm:ss._msZ'.length)
-                .should.equal(',a b,1,1,x y z,0,1,1,1,one,5,Alice,0,0,,,,0,');
+                .should.equal(',one,a b,1,1,x y z,0,1,1,1,one,5,Alice,0,0,,,,0,');
             })))));
 
     it('should properly count present attachments', testService((service) =>
@@ -2186,11 +2186,11 @@ one,h,/data/h,2000-01-01T00:06,2000-01-01T00:07,-5,-6,,ee,ff
             // eslint-disable-next-line no-multi-spaces
             .then(({ text }) =>  {
               const lines = text.split('\n');
-              lines[0].should.equal('SubmissionDate,q1,q1/a,q1/b,g1-q2,g1-q2/m,g1-q2/x,g1-q2/y,g1-q2/z,KEY,SubmitterID,SubmitterName,AttachmentsPresent,AttachmentsExpected,Status,ReviewState,DeviceID,Edits,FormVersion');
+              lines[0].should.equal('SubmissionDate,meta-instanceID,q1,q1/a,q1/b,g1-q2,g1-q2/m,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(',b,0,1,m x,1,1,0,0,two,5,Alice,0,0,,,,0,');
+                .should.equal(',two,b,0,1,m x,1,1,0,0,two,5,Alice,0,0,,,,0,');
               lines[2].slice('yyyy-mm-ddThh:mm:ss._msZ'.length)
-                .should.equal(',a b,1,1,x y z,0,1,1,1,one,5,Alice,0,0,,,,0,');
+                .should.equal(',one,a b,1,1,x y z,0,1,1,1,one,5,Alice,0,0,,,,0,');
             })))));
 
     it('should omit group paths if ?groupPaths=false is given', testService((service) =>
@@ -2279,11 +2279,13 @@ one,h,/data/h,2000-01-01T00:06,2000-01-01T00:07,-5,-6,,ee,ff
                     <model>
                       <instance>
                         <data id="selectMultiple" version='3'>
+                          <meta><instanceID/></meta>
                           <q1/>
                           <g1><q2/></g1>
                           <q3/>
                         </data>
                       </instance>
+                      <bind nodeset="/data/meta/instanceID" type="string"/>
                       <bind nodeset="/data/q1" type="string"/>
                       <bind nodeset="/data/g1/q2" type="string"/>
                       <bind nodeset="/data/q3" type="string"/>
@@ -2323,13 +2325,13 @@ one,h,/data/h,2000-01-01T00:06,2000-01-01T00:07,-5,-6,,ee,ff
               .expect(200)
               .then(({ text }) => {
                 const lines = text.split('\n');
-                lines[0].should.equal('SubmissionDate,q1,q1/a,q1/b,g1-q2,g1-q2/m,g1-q2/x,g1-q2/y,g1-q2/z,q3,q3/z,KEY,SubmitterID,SubmitterName,AttachmentsPresent,AttachmentsExpected,Status,ReviewState,DeviceID,Edits,FormVersion');
+                lines[0].should.equal('SubmissionDate,meta-instanceID,q1,q1/a,q1/b,g1-q2,g1-q2/m,g1-q2/x,g1-q2/y,g1-q2/z,q3,q3/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,m,1,0,0,0,z,1,three,5,Alice,0,0,,,,0,3');
+                  .should.equal(',three,a b,1,1,m,1,0,0,0,z,1,three,5,Alice,0,0,,,,0,3');
                 lines[2].slice('yyyy-mm-ddThh:mm:ss._msZ'.length)
-                  .should.equal(',b,0,1,m x,1,1,0,0,,0,two,5,Alice,0,0,,,,0,');
+                  .should.equal(',two,b,0,1,m x,1,1,0,0,,0,two,5,Alice,0,0,,,,0,');
                 lines[3].slice('yyyy-mm-ddThh:mm:ss._msZ'.length)
-                  .should.equal(',a b,1,1,x y z,0,1,1,1,,0,one,5,Alice,0,0,,,,0,');
+                  .should.equal(',one,a b,1,1,x y z,0,1,1,1,,0,one,5,Alice,0,0,,,,0,');
               })))));
     });
   });
@@ -2471,11 +2473,11 @@ one,h,/data/h,2000-01-01T00:06,2000-01-01T00:07,-5,-6,,ee,ff
             .then((result) => {
               result.filenames.should.containDeep([ 'selectMultiple.csv' ]);
               const lines = result['selectMultiple.csv'].split('\n');
-              lines[0].should.equal('SubmissionDate,q1,q1/a,q1/b,g1-q2,g1-q2/m,g1-q2/x,g1-q2/y,g1-q2/z,KEY,SubmitterID,SubmitterName,AttachmentsPresent,AttachmentsExpected,Status,ReviewState,DeviceID,Edits,FormVersion');
+              lines[0].should.equal('SubmissionDate,meta-instanceID,q1,q1/a,q1/b,g1-q2,g1-q2/m,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(',b,0,1,m x,1,1,0,0,two,,,0,0,,,,0,');
+                .should.equal(',two,b,0,1,m x,1,1,0,0,two,,,0,0,,,,0,');
               lines[2].slice('yyyy-mm-ddThh:mm:ss._msZ'.length)
-                .should.equal(',a b,1,1,x y z,0,1,1,1,one,,,0,0,,,,0,');
+                .should.equal(',one,a b,1,1,x y z,0,1,1,1,one,,,0,0,,,,0,');
             })))));
   });
 

test/integration/other/select-many.js

@@ -94,10 +94,12 @@ describe('select many value processing', () => {
           <model>
             <instance>
               <data id="selectMultiple">
+                <meta><instanceID/></meta>
                 <q1/>
                 <g1><q2/></g1>
               </data>
             </instance>
+            <bind nodeset="/data/meta/instanceID" type="string"/>
             <bind nodeset="/data/q1" type="string"/>
             <bind nodeset="/data/g1/q2" type="string"/>
           </model>
@@ -143,11 +145,11 @@ describe('select many value processing', () => {
       .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[0].should.equal('SubmissionDate,meta-instanceID,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');
+          .should.equal(',one,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,');
+          .should.equal(',two,b,0,1,x,1,0,0,two,5,Alice,0,0,,,,0,');
       });
   }));
 });