Merge branch 'master' into 1212-logging

.eslintrc.json

@@ -28,7 +28,7 @@
     "no-else-return": "off",
     "no-multiple-empty-lines": "off",
     "no-nested-ternary": "off",
-    "no-only-tests/no-only-tests": "error",
+    "no-only-tests/no-only-tests": [ "error", { "block": [ "describe", "it", "describeMigration" ] } ],
     "no-restricted-syntax": "off",
     "no-underscore-dangle": "off",
     "nonblock-statement-body-position": "off",

lib/model/migrations/.eslintrc.json

@@ -0,0 +1,6 @@
+{
+  "extends": "../../../.eslintrc.json",
+  "rules": {
+    "no-restricted-modules": [ "error", { "patterns": [ "../*" ] } ]
+  }
+}

lib/model/migrations/20180727-02-add-md5-to-blobs.js

@@ -8,7 +8,7 @@
 // except according to the terms contained in the LICENSE file.
 //
 
-const { md5sum } = require('../../util/crypto');
+const { md5sum } = require('../../util/crypto'); // eslint-disable-line no-restricted-modules
 
 const up = (knex) =>
   knex.schema.table('blobs', (blobs) => { blobs.string('md5', 32); })

lib/model/migrations/20180727-03-add-form-attachments-table.js

@@ -23,7 +23,7 @@ const up = (knex) =>
 
     fa.index([ 'formId' ]);
   }).then(() => {
-    const { expectedFormAttachments } = require('../../data/schema');
+    const { expectedFormAttachments } = require('../../data/schema'); // eslint-disable-line no-restricted-modules
     const { uniq, pluck } = require('ramda');
 
     // now add all expected attachments on extant forms.

lib/model/migrations/20190520-01-add-form-versioning.js

@@ -7,7 +7,7 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
-const { shasum, sha256sum } = require('../../util/crypto');
+const { shasum, sha256sum } = require('../../util/crypto'); // eslint-disable-line no-restricted-modules
 const assert = require('assert').strict;
 
 const check = (message, query) =>

lib/model/migrations/20191007-01-backfill-client-audits.js

@@ -7,9 +7,9 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
-const { parseClientAudits } = require('../../data/client-audits');
-const { getFormFields } = require('../../data/schema');
-const { traverseXml, findOne, root, node, text } = require('../../util/xml');
+const { parseClientAudits } = require('../../data/client-audits'); // eslint-disable-line no-restricted-modules
+const { getFormFields } = require('../../data/schema'); // eslint-disable-line no-restricted-modules
+const { traverseXml, findOne, root, node, text } = require('../../util/xml'); // eslint-disable-line no-restricted-modules
 
 const up = (db) => new Promise((resolve, reject) => {
   const work = [];

lib/model/migrations/20191231-02-add-schema-storage.js

@@ -7,7 +7,7 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
-const { getFormFields } = require('../../data/schema');
+const { getFormFields } = require('../../data/schema'); // eslint-disable-line no-restricted-modules
 
 const up = async (db) => {
   await db.schema.createTable('form_fields', (fields) => {
@@ -51,7 +51,7 @@ const up = async (db) => {
               // this config hardcoding would be dangerous with tests except that
               // tests will never invoke this path.
               const config = require('config').get('default.database');
-              const db2 = require('../migrate').knexConnect(config);
+              const db2 = require('../migrate').knexConnect(config); // eslint-disable-line no-restricted-modules
               return db2.select('projectId', 'xmlFormId').from('forms').where({ currentDefId: formDefId })
                 .then(([{ projectId, xmlFormId }]) => {
                   process.stderr.write(`\n!!!!\nThe database upgrade to v0.8 has failed because the Form '${xmlFormId}' in Project ${projectId} has an invalid schema. It tries to bind multiple instance nodes at the path ${path}.\n!!!!\n\n`);

lib/model/migrations/20200220-01-repair-submission-parsing.js

@@ -7,7 +7,7 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
-const { Submission } = require('../frames');
+const { Submission } = require('../frames'); // eslint-disable-line no-restricted-modules
 
 const up = async (db) => {
   const work = [];

lib/model/migrations/20210120-01-instance-names.js

@@ -7,7 +7,7 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
-const { Submission } = require('../frames');
+const { Submission } = require('../frames'); // eslint-disable-line no-restricted-modules
 
 const up = async (db) => {
   await db.schema.table('submission_defs', (sds) => {

lib/model/migrations/20211008-01-track-select-many-options.js

@@ -8,10 +8,10 @@
 // except according to the terms contained in the LICENSE file.
 
 const { map } = require('ramda');
-const { getFormFields } = require('../../data/schema');
-const { getSelectMultipleResponses } = require('../../data/submission');
-const { Form } = require('../frames');
-const { construct } = require('../../util/util');
+const { getFormFields } = require('../../data/schema'); // eslint-disable-line no-restricted-modules
+const { getSelectMultipleResponses } = require('../../data/submission'); // eslint-disable-line no-restricted-modules
+const { Form } = require('../frames'); // eslint-disable-line no-restricted-modules
+const { construct } = require('../../util/util'); // eslint-disable-line no-restricted-modules
 
 const up = async (db) => {
   // add select many flag, options field to fields

lib/model/migrations/20230109-01-add-form-schema.js

@@ -7,7 +7,7 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
-const { getFormFields, compare } = require('../../data/schema');
+const { getFormFields, compare } = require('../../data/schema'); // eslint-disable-line no-restricted-modules
 
 /* Steps of this migration
   1. remove check field collision trigger

lib/model/query/forms.js

@@ -242,14 +242,16 @@ const createVersion = (partial, form, publish, duplicating = false) => async ({
       // skip checking for structural change if duplicating because user has already
       // been warning at the time of form definition upload
       if (!duplicating) {
-        await Forms.checkStructuralChange(prevFields, fields)
-          .then(Forms.rejectIfWarnings);
+        await Forms.checkStructuralChange(prevFields, fields);
       }
       // If we haven't been rejected or warned yet, make a new schema id
       schemaId = await Forms._newSchema();
     }
   }
 
+  // Let's check for warnings before pushing to Enketo or to DB
+  await Forms.rejectIfWarnings();
+
   // If not publishing, check whether there is an existing draft that we have access to.
   // If not, generate a draft token and enketoId.
   let { draftToken, enketoId } = form.def;

lib/util/db.js

@@ -117,7 +117,7 @@ const unjoiner = (...frames) => {
     return new frames[0](primary, bag);
   };
 
-  unjoin.fields = sql`${sql.join(fields, sql`,`)}`; // FIXME remove wrapping sql``
+  unjoin.fields = sql.join(fields, sql`,`);
   return unjoin;
 };
 

package.json

@@ -27,7 +27,7 @@
     "htmlparser2": "~3.9",
     "knex": "~0.21",
     "luxon": "~0.3",
-    "minio": "^7.1.3",
+    "minio": "^8.0.3",
     "morgan": "~1.9",
     "multer": "^1.4.5-lts.1",
     "mustache": "~2.3",

test/db-migrations/utils.js

@@ -77,22 +77,22 @@ function assertRowsMatch(actualRows, expectedRows) {
 
   const remainingRows = [...actualRows];
   for (let i=0; i<expectedRows.length; ++i) { // eslint-disable-line no-plusplus
-    const x = expectedRows[i];
+    const expectedRow = expectedRows[i];
     let found = false;
     for (let j=0; j<remainingRows.length; ++j) { // eslint-disable-line no-plusplus
       const rr = remainingRows[j];
       try {
-        assertIncludes(rr, x); // eslint-disable-line no-use-before-define
+        assertIncludes(rr, expectedRow); // eslint-disable-line no-use-before-define
         remainingRows.splice(j, 1);
         found = true;
         break;
       } catch (err) { /* keep searching */ }
     }
     if (!found) {
-      const filteredRemainingRows = remainingRows.map(r => _.pick(r, Object.keys(x)));
+      const filteredRemainingRows = remainingRows.map(r => _.pick(r, Object.keys(expectedRow)));
       assert.fail(
         `Expected row ${i} not found:\njson=` +
-        JSON.stringify({ remainingRows, filteredRemainingRows, expectedRow: x }),
+        JSON.stringify({ remainingRows, filteredRemainingRows, expectedRow }),
       );
     }
   }

test/integration/api/forms/forms.js

@@ -158,6 +158,25 @@ describe('api: /projects/:id/forms (create, read, update)', () => {
           }));
     }));
 
+    it('should fail on warnings even for valid xlsx files', testService(async (service) => {
+      await service.login('alice', (asAlice) =>
+        asAlice.post('/v1/projects/1/forms')
+          .send(readFileSync(appRoot + '/test/data/simple.xlsx'))
+          .set('Content-Type', 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet')
+          .expect(200));
+
+      global.xlsformTest = 'warning'; // set up the mock service to warn.
+      return service.login('alice', (asAlice) =>
+        asAlice.post('/v1/projects/1/forms/simple2/draft')
+          .send(readFileSync(appRoot + '/test/data/simple.xlsx'))
+          .set('Content-Type', 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet')
+          .expect(400)
+          .then(({ body }) => {
+            body.code.should.equal(400.16);
+            body.details.warnings.xlsFormWarnings.should.eql([ 'warning 1', 'warning 2' ]);
+          }));
+    }));
+
     it('should create the form for xlsx files with warnings given ignoreWarnings', testService((service) => {
       global.xlsformTest = 'warning'; // set up the mock service to warn.
       return service.login('alice', (asAlice) =>