Merge pull request #416 from getodk/issa/v1.3-patches

v1.3 patches

lib/data/briefcase.js

@@ -50,12 +50,7 @@ const writeMetadata = (out, idxs, submission, submitter, attachments, status) =>
     out[idxs.reviewState] = submission.reviewState;
   if (submission.deviceId != null)
     out[idxs.deviceID] = submission.deviceId;
-
-  // we don't output a number unless there is a count sort of because all the
-  // columns before this are sparse and optional and it's awkward for some of
-  // our tests to deal with a 0 always being here.
-  const editCount = submission.aux.edit.count;
-  if (editCount > 0) out[idxs.edits] = editCount;
+  out[idxs.edits] = submission.aux.edit.count;
 };
 /* eslint-enable no-param-reassign */
 
@@ -84,10 +79,11 @@ const keyForStacks = (instanceId, schemaStack, slicer = identity) => {
 // deals with each submission. runs the entire submission xml through an evented
 // parser, transforms the data to tabular format, and automatically streams out
 // substream rows. returns the root row as a Promise result.
-const processRow = (xml, instanceId, fields) => new Promise((resolve, reject) => {
+// fields is an array in depth-first tree order, and length is the width of the root table.
+const processRow = (xml, instanceId, fields, length) => new Promise((resolve, reject) => {
   // set up our state machine stacks:
   const schemaStack = new SchemaStack(fields);
-  const dataStack = [ [] ];
+  const dataStack = [ new Array(length) ];
 
   // now spin up our XML parser and let its SAX-like tree events drive our traversal.
   const parser = new hparser.Parser({
@@ -98,7 +94,7 @@ const processRow = (xml, instanceId, fields) => new Promise((resolve, reject) =>
         dataStack.push(null);
       } else if (field.type === 'repeat') {
         // we are going to be writing to a new subrow:
-        const subrow = [];
+        const subrow = new Array(field.header.length);
         subrow[field.meta.key] = keyForStacks(instanceId, schemaStack);
         subrow[field.meta.parentKey] =
           keyForStacks(instanceId, schemaStack, schemaStack.repeatContextSlicer());
@@ -281,13 +277,13 @@ const streamBriefcaseCsvs = (inStream, inFields, xmlFormId, decryptor, rootOnly
           (xml === 'missing') ? 'missing encrypted form data' :
           (xml === null) ? 'not decrypted' : null; // eslint-disable-line indent
         if (status != null) {
-          const result = [];
+          const result = new Array(rootHeader.length);
           writeMetadata(result, rootMeta, submission, submission.aux.submitter, submission.aux.attachment, status);
           return done(null, result);
         }
 
         // write the root row we get back from parsing the xml.
-        processRow(xml, submission.instanceId, fields, rootOnly).then((result) => {
+        processRow(xml, submission.instanceId, fields, rootHeader.length).then((result) => {
           writeMetadata(result, rootMeta, submission, submission.aux.submitter, submission.aux.attachment);
           done(null, result);
         }, done); // pass through errors.

lib/model/query/submissions.js

@@ -131,6 +131,12 @@ where submissions."instanceId"=${instanceId} and current=true
 limit 1`)
   .then(map(construct(Submission.Def)));
 
+const getDefById = (submissionDefId) => ({ maybeOne }) => maybeOne(sql`
+select submission_defs.* from submission_defs
+inner join submissions on submissions.id = submission_defs."submissionId" and submissions."deletedAt" is null
+where submission_defs.id=${submissionDefId}`)
+  .then(map(construct(Submission.Def)));
+
 const _getDef = extender(Submission.Def)(Actor.into('submitter'))((fields, extend, options, formId, draft) => sql`
 select ${fields} from submission_defs
 inner join
@@ -219,7 +225,7 @@ module.exports = {
   createNew, createVersion,
   update, clearDraftSubmissions,
   getByIds, getAllForFormByIds, countByFormId, verifyVersion,
-  getCurrentDefByIds, getAnyDefByFormAndInstanceId, getDefsByFormAndLogicalId, getRootForInstanceId,
+  getDefById, getCurrentDefByIds, getAnyDefByFormAndInstanceId, getDefsByFormAndLogicalId, getRootForInstanceId,
   streamForExport, getForExport
 };
 

lib/util/http.js

@@ -89,7 +89,7 @@ const sanitizeToAscii = (str) => str.replace(/[^\x00-\x7F]+/g, '-'); //  eslint-
 
 const contentDisposition = (filename) => {
   const sanitized = sanitize(filename);
-  return `attachment; filename="${sanitizeToAscii(sanitized)}" filename*=UTF-8''${encodeRFC5987ValueChars(sanitized)}`;
+  return `attachment; filename="${sanitizeToAscii(sanitized)}"; filename*=UTF-8''${encodeRFC5987ValueChars(sanitized)}`;
 };
 
 //  content-disposition helpers

lib/worker/submission.attachment.update.js

@@ -9,22 +9,27 @@
 
 const { parseClientAudits } = require('../data/client-audits');
 
-const worker = ({ ClientAudits, Blobs, SubmissionAttachments }, event) =>
-  SubmissionAttachments.getBySubmissionDefIdAndName(event.details.submissionDefId, event.details.name)
-    .then((maybeAttachment) => maybeAttachment
-      .map((attachment) => (((attachment.blobId == null) || (attachment.isClientAudit !== true))
+const worker = ({ ClientAudits, Blobs, Submissions, SubmissionAttachments }, event) =>
+  Promise.all([
+    Submissions.getDefById(event.details.submissionDefId),
+    SubmissionAttachments.getBySubmissionDefIdAndName(event.details.submissionDefId, event.details.name)
+  ])
+    .then(([ maybeSubmission, maybeAttachment ]) =>
+      (maybeSubmission.map((s) => s.localKey != null).orElse(false)
         ? null
-        : ClientAudits.existsForBlob(attachment.blobId)
-          .then((exists) => ((exists === true)
-            ? null // do nothing
-            : Blobs.getById(attachment.blobId)
-              .then((maybeBlob) => maybeBlob.get()) // blobs are immutable
-              .then((blob) => parseClientAudits(blob.content))
-              .then((audits) => {
-                const withBlobIds = audits.map((audit) => audit.with({ blobId: attachment.blobId }));
-                return ClientAudits.createMany(withBlobIds);
-              })))))
-      .orNull());
+        : maybeAttachment.map((attachment) => (((attachment.blobId == null) || (attachment.isClientAudit !== true))
+          ? null
+          : ClientAudits.existsForBlob(attachment.blobId)
+            .then((exists) => ((exists === true)
+              ? null // do nothing
+              : Blobs.getById(attachment.blobId)
+                .then((maybeBlob) => maybeBlob.get()) // blobs are immutable
+                .then((blob) => parseClientAudits(blob.content))
+                .then((audits) => {
+                  const withBlobIds = audits.map((audit) => audit.with({ blobId: attachment.blobId }));
+                  return ClientAudits.createMany(withBlobIds);
+                })))))
+          .orNull()));
 
 module.exports = worker;
 

lib/worker/worker.js

@@ -14,6 +14,8 @@ const { sql } = require('slonik');
 const { timebound, resolve } = require('../util/promise');
 const defaultJobMap = require('./jobs').jobs;
 
+const hostname = require('os').hostname();
+
 // TODO: domain catch/restart? << investigated and seems unlikely.
 
 // given an event, attempts to run the appropriate jobs for the event and then
@@ -37,7 +39,7 @@ const runner = (container, jobMap) => {
         // but more importantly we need to unclaim this job, and most importantly
         // we want to be sure that the worker loop gets rescheduled.
         /* eslint-disable */ // i don't like anything it suggests here.
-        try { Sentry.captureException(err); }
+        try { Sentry.captureException(err, { server_name: hostname }); }
         catch (ierr) {
           try { process.stderr.write(inspect(err) + '\n'); process.stderr.write(inspect(ierr) + '\n'); }
           catch (_) { /* too scared to try anything at this point */ }

test/assertions.js

@@ -249,11 +249,11 @@ should.Assertion.add('SimpleCsv', function() {
   csv.length.should.equal(5); // header + 3 data rows + newline
   csv[0].should.eql([ 'SubmissionDate', 'meta-instanceID', 'name', 'age', 'KEY', 'SubmitterID', 'SubmitterName', 'AttachmentsPresent', 'AttachmentsExpected', 'Status', 'ReviewState', 'DeviceID', 'Edits' ]);
   csv[1].shift().should.be.an.recentIsoDate();
-  csv[1].should.eql([ 'three','Chelsea','38','three','5','Alice','0','0' ]);
+  csv[1].should.eql([ 'three','Chelsea','38','three','5','Alice','0','0','','','','0' ]);
   csv[2].shift().should.be.an.recentIsoDate();
-  csv[2].should.eql([ 'two','Bob','34','two','5','Alice','0','0' ]);
+  csv[2].should.eql([ 'two','Bob','34','two','5','Alice','0','0','','','','0' ]);
   csv[3].shift().should.be.an.recentIsoDate();
-  csv[3].should.eql([ 'one','Alice','30','one','5','Alice','0','0' ]);
+  csv[3].should.eql([ 'one','Alice','30','one','5','Alice','0','0','','','','0' ]);
   csv[4].should.eql([ '' ]);
 });
 
@@ -264,11 +264,11 @@ should.Assertion.add('EncryptedSimpleCsv', function() {
   csv.length.should.equal(5); // header + 3 data rows + newline
   csv[0].should.eql([ 'SubmissionDate', 'meta-instanceID', 'name', 'age', 'KEY', 'SubmitterID', 'SubmitterName', 'AttachmentsPresent', 'AttachmentsExpected', 'Status', 'ReviewState', 'DeviceID', 'Edits' ]);
   csv[1].shift().should.be.an.recentIsoDate();
-  csv[1].should.eql([ 'three','Chelsea','38','three','5','Alice','1','1' ]);
+  csv[1].should.eql([ 'three','Chelsea','38','three','5','Alice','1','1','','','','0' ]);
   csv[2].shift().should.be.an.recentIsoDate();
-  csv[2].should.eql([ 'two','Bob','34','two','5','Alice','1','1' ]);
+  csv[2].should.eql([ 'two','Bob','34','two','5','Alice','1','1','','','','0' ]);
   csv[3].shift().should.be.an.recentIsoDate();
-  csv[3].should.eql([ 'one','Alice','30','one','5','Alice','1','1' ]);
+  csv[3].should.eql([ 'one','Alice','30','one','5','Alice','1','1','','','','0' ]);
   csv[4].should.eql([ '' ]);
 });
 

test/integration/api/forms.js

@@ -732,7 +732,7 @@ describe('api: /projects/:id/forms', () => {
               .expect(200)
               .then(({ headers, body }) => {
                 headers['content-type'].should.equal('application/vnd.openxmlformats-officedocument.spreadsheetml.sheet');
-                headers['content-disposition'].should.equal('attachment; filename="simple2.xlsx" filename*=UTF-8\'\'simple2.xlsx');
+                headers['content-disposition'].should.equal('attachment; filename="simple2.xlsx"; filename*=UTF-8\'\'simple2.xlsx');
                 Buffer.compare(input, body).should.equal(0);
               })));
       }));
@@ -753,7 +753,7 @@ describe('api: /projects/:id/forms', () => {
               .expect(200)
               .then(({ headers, body }) => {
                 headers['content-type'].should.equal('application/vnd.openxmlformats-officedocument.spreadsheetml.sheet');
-                headers['content-disposition'].should.equal('attachment; filename="simple2.xlsx" filename*=UTF-8\'\'simple2.xlsx');
+                headers['content-disposition'].should.equal('attachment; filename="simple2.xlsx"; filename*=UTF-8\'\'simple2.xlsx');
                 Buffer.compare(input, body).should.equal(0);
               })));
       }));
@@ -772,7 +772,7 @@ describe('api: /projects/:id/forms', () => {
               .expect(200)
               .then(({ headers, body }) => {
                 headers['content-type'].should.equal('application/vnd.openxmlformats-officedocument.spreadsheetml.sheet');
-                headers['content-disposition'].should.equal('attachment; filename="simple2.xlsx" filename*=UTF-8\'\'simple2.xlsx');
+                headers['content-disposition'].should.equal('attachment; filename="simple2.xlsx"; filename*=UTF-8\'\'simple2.xlsx');
                 Buffer.compare(input, body).should.equal(0);
               })));
       }));
@@ -1229,7 +1229,7 @@ describe('api: /projects/:id/forms', () => {
               .then(() => asAlice.get('/v1/projects/1/forms/withAttachments/attachments/goodone.csv')
                 .expect(200)
                 .then(({ headers, text }) => {
-                  headers['content-disposition'].should.equal('attachment; filename="goodone.csv" filename*=UTF-8\'\'goodone.csv');
+                  headers['content-disposition'].should.equal('attachment; filename="goodone.csv"; filename*=UTF-8\'\'goodone.csv');
                   headers['content-type'].should.equal('text/csv; charset=utf-8');
                   text.should.equal('test,csv\n1,2');
                 })))));
@@ -2815,7 +2815,7 @@ describe('api: /projects/:id/forms', () => {
                 .buffer(true).parse(superagent.parse['application/octet-stream'])
                 .then(({ headers, body }) => {
                   headers['content-type'].should.equal('application/vnd.openxmlformats-officedocument.spreadsheetml.sheet');
-                  headers['content-disposition'].should.equal('attachment; filename="simple2.xlsx" filename*=UTF-8\'\'simple2.xlsx');
+                  headers['content-disposition'].should.equal('attachment; filename="simple2.xlsx"; filename*=UTF-8\'\'simple2.xlsx');
                   Buffer.compare(input, body).should.equal(0);
                 })));
         }));