Copy forward enketoId from existing draft

Addresses the "existing draft is updated" case of getodk/central#385.

Author: matthew-white

lib/model/query/forms.js

@@ -9,7 +9,7 @@
 
 const config = require('config');
 const { sql } = require('slonik');
-const { map, compose } = require('ramda');
+const { map } = require('ramda');
 const { Frame, into } = require('../frame');
 const { Actor, Blob, Form, Dataset } = require('../frames');
 const { getFormFields, merge, compare } = require('../../data/schema');
@@ -18,7 +18,7 @@ const { generateToken } = require('../../util/crypto');
 const { unjoiner, extender, updater, equals, insert, insertMany, markDeleted, markUndeleted, QueryOptions } = require('../../util/db');
 const { resolve, reject } = require('../../util/promise');
 const { splitStream } = require('../../util/stream');
-const { construct, noop } = require('../../util/util');
+const { construct, noop, pickAll } = require('../../util/util');
 const Option = require('../../util/option');
 const Problem = require('../../util/problem');
 
@@ -42,7 +42,7 @@ const fromXls = (stream, contentType, formIdFallback, ignoreWarnings) => ({ Blob
     });
 
 ////////////////////////////////////////////////////////////////////////////////
-// CREATING NEW FORMS+VERSIONS
+// CREATING NEW FORMS
 
 const _createNew = (form, def, project, publish) => ({ oneFirst, Actees, Forms }) =>
   Actees.provision('form', project)
@@ -117,6 +117,32 @@ createNew.audit = (form, partial, _, publish) => (log) =>
     : null));
 createNew.audit.withResult = true;
 
+////////////////////////////////////////////////////////////////////////////////
+// CREATING NEW VERSIONS
+
+// Inserts a new form def into the database for createVersion() below, setting
+// fields on the def according to whether the def will be the current def or the
+// draft def.
+const _createNewDef = (partial, form, publish, data) => ({ one }) => {
+  const insertWith = (moreData) => one(insert(partial.def.with({
+    formId: form.id,
+    xlsBlobId: partial.xls.xlsBlobId,
+    xml: partial.xml,
+    ...data,
+    ...moreData
+  })));
+
+  if (publish === true) return insertWith({ publishedAt: new Date() });
+
+  // Check whether there is an existing draft that we have access to. If not,
+  // generate a draft token.
+  if (form.def.id == null || form.def.id !== form.draftDefId)
+    return insertWith({ draftToken: generateToken() });
+
+  // Copy forward fields from the existing draft.
+  return insertWith(pickAll(['draftToken', 'enketoId'], form.def));
+};
+
 // creates a new version (formDef) of an existing form.
 //
 // if publish is true, the new version supplants the published (currentDefId)
@@ -126,20 +152,15 @@ createNew.audit.withResult = true;
 // exists.
 //
 // if field paths/types collide, the database will complain.
-
-const _getDraftToken = (form) => {
-  if ((form.def.id != null) && (form.draftDefId === form.def.id)) return form.def.draftToken;
-  return generateToken();
-};
-
+//
 // Parameters:
 // ===========
 // partial:     Partial form definition of the new version
 // form:        Form frame of existing form
 // publish:     set true if you want new version to be published
 // duplicating: set true if copying form definition from previously uploaded definition, in that cases we don't check for structural change
 //              as user has already been warned otherwise set false
-const createVersion = (partial, form, publish = false, duplicating = false) => async ({ run, one, Datasets, FormAttachments, Forms, Keys }) => {
+const createVersion = (partial, form, publish, duplicating = false) => async ({ run, Datasets, FormAttachments, Forms, Keys }) => {
   // Check xmlFormId match
   if (form.xmlFormId !== partial.xmlFormId)
     return reject(Problem.user.unexpectedValue({ field: 'xmlFormId', value: partial.xmlFormId, reason: 'does not match the form you are updating' }));
@@ -185,20 +206,13 @@ const createVersion = (partial, form, publish = false, duplicating = false) => a
     }
   }
 
-  // Make sure our new def is in the database, and mark it as either draft or current.
-  let def = partial.def.with({ formId: form.id, keyId, xlsBlobId: partial.xls.xlsBlobId });
-  def = ((publish === true)
-    ? def.with({ publishedAt: new Date(), xml: partial.xml })
-    : def.with({ draftToken: _getDraftToken(form), xml: partial.xml }));
-  const savedDef = await compose(one, insert)(def);
+  const savedDef = await Forms._createNewDef(partial, form, publish, { schemaId, keyId });
 
   // Update corresponding form
   await ((publish === true)
     ? Forms._update(form, { currentDefId: savedDef.id })
     : Forms._update(form, { draftDefId: savedDef.id }));
 
-  await Forms._updateDef(new Form.Def(savedDef), { schemaId });
-
   // Prepare the form fields
   const ids = { formId: form.id, schemaId };
   const fieldsForInsert = new Array(fields.length);
@@ -691,7 +705,7 @@ const _newSchema = () => ({ one }) =>
     .then((s) => s.id);
 
 module.exports = {
-  fromXls, _createNew, createNew, createVersion,
+  fromXls, _createNew, createNew, _createNewDef, createVersion,
   publish, clearDraft,
   _update, update, _updateDef, del, restore, purge,
   clearUnneededDrafts,

lib/worker/form.js

@@ -15,6 +15,10 @@ const pushDraftToEnketo = ({ Forms, enketo, env }, event) =>
       // if there was no draft or this form isn't the draft anymore just bail.
       if ((form.def.id == null) || (form.draftDefId !== form.def.id)) return;
 
+      // if the enketoId has been carried forward from the previous draft, then
+      // bail.
+      if (form.def.enketoId != null) return;
+
       // if this form doesn't have a draft testing key something is broken
       // and wrong. still want to log a fail but bail early.
       if (form.def.draftToken == null) throw new Error('Could not find a draft token!');

test/integration/api/forms/draft.js

@@ -138,7 +138,7 @@ describe('api: /projects/:id/forms (drafts)', () => {
                 body.version.should.equal('drafty2');
               })))));
 
-      it('should keep the draft token while replacing the draft version', testService((service) =>
+      it('should keep the draft token while replacing the draft', testService((service) =>
         service.login('alice', (asAlice) =>
           asAlice.post('/v1/projects/1/forms/simple/draft')
             .send(testData.forms.simple.replace('id="simple"', 'id="simple" version="drafty"'))
@@ -161,6 +161,43 @@ describe('api: /projects/:id/forms (drafts)', () => {
                     }));
               })))));
 
+      it('should keep the enketoId while replacing the draft', testService(async (service) => {
+        const asAlice = await service.login('alice');
+        await asAlice.post('/v1/projects/1/forms/simple/draft')
+          .send(testData.forms.simple.replace('id="simple"', 'id="simple" version="drafty"'))
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+        await exhaust(container);
+        const { body: draft1 } = await asAlice.get('/v1/projects/1/forms/simple/draft')
+          .expect(200);
+        draft1.enketoId.should.equal('::abcdefgh');
+        await asAlice.post('/v1/projects/1/forms/simple/draft')
+          .send(testData.forms.simple.replace('id="simple"', 'id="simple" version="drafty2"'))
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+        const { body: draft2 } = await asAlice.get('/v1/projects/1/forms/simple/draft')
+          .expect(200);
+        draft2.enketoId.should.equal('::abcdefgh');
+      }));
+
+      it('should not request an enketoId from the worker while replacing the draft', testService(async (service, container) => {
+        const asAlice = await service.login('alice');
+        await asAlice.post('/v1/projects/1/forms/simple/draft')
+          .send(testData.forms.simple.replace('id="simple"', 'id="simple" version="drafty"'))
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+        await exhaust(container);
+        await asAlice.post('/v1/projects/1/forms/simple/draft')
+          .send(testData.forms.simple.replace('id="simple"', 'id="simple" version="drafty2"'))
+          .set('Content-Type', 'application/xml')
+          .expect(200);
+        global.enketoToken = '::ijklmnop';
+        await exhaust(container);
+        const { body: draft } = await asAlice.get('/v1/projects/1/forms/simple/draft')
+          .expect(200);
+        draft.enketoId.should.equal('::abcdefgh');
+      }));
+
       it('should copy the published form definition if not given one', testService((service) =>
         service.login('alice', (asAlice) =>
           asAlice.post('/v1/projects/1/forms/simple/draft')

test/integration/api/submissions.js

@@ -2724,7 +2724,7 @@ one,h,/data/h,2000-01-01T00:06,2000-01-01T00:07,-5,-6,,ee,ff
               .replace(/PublicKey="[a-z0-9+/]+"/i, 'PublicKey="keytwo"')
               .replace('working3', 'working4'))
           ]))
-          .then(([ form, partial ]) => Forms.createVersion(partial, form))
+          .then(([ form, partial ]) => Forms.createVersion(partial, form, false))
           .then(() => asAlice.get('/v1/projects/1/forms/encrypted/submissions/keys')
             .expect(200)
             .then(({ body }) => {