Make small changes related to requesting Enketo IDs after code review

lib/model/query/assignments.js

@@ -64,7 +64,7 @@ select ${fields} from assignments
   where ${equals(options.condition)}`);
 const getByActeeId = (acteeId, options = QueryOptions.none) => ({ all }) =>
   _get(all, options.withCondition({ 'assignments.acteeId': acteeId }));
-const getByActeeAndRoleId = (acteeId, roleId, options) => ({ all }) =>
+const getByActeeAndRoleId = (acteeId, roleId, options = QueryOptions.none) => ({ all }) =>
   _get(all, options.withCondition({ 'assignments.acteeId': acteeId, roleId }));
 
 const _getForForms = extender(Assignment, Assignment.FormSummary)(Actor)((fields, extend, options) => sql`

lib/model/query/forms.js

@@ -45,17 +45,21 @@ const fromXls = (stream, contentType, formIdFallback, ignoreWarnings) => ({ Blob
 ////////////////////////////////////////////////////////////////////////////////
 // PUSHING TO ENKETO
 
+// Time-bounds a request from enketo.create(). If the request times out or
+// results in an error, then an empty object is returned.
 const timeboundEnketo = (request, bound) =>
   (bound != null ? timebound(request, bound).catch(() => ({})) : request);
 
 // Accepts either a Form or an object with a top-level draftToken property. Also
 // accepts an optional bound on the amount of time for the request to Enketo to
 // complete (in seconds). If a bound is specified, and the request to Enketo
-// times out or results in an error, then a nullish value is returned.
+// times out or results in an error, then `null` is returned.
 const pushDraftToEnketo = ({ projectId, xmlFormId, def, draftToken = def?.draftToken }, bound = undefined) => async ({ enketo, env }) => {
   const encodedFormId = encodeURIComponent(xmlFormId);
   const path = `${env.domain}/v1/test/${draftToken}/projects/${projectId}/forms/${encodedFormId}/draft`;
-  return (await timeboundEnketo(enketo.create(path, xmlFormId), bound)).enketoId;
+  const { enketoId } = await timeboundEnketo(enketo.create(path, xmlFormId), bound);
+  // Return `null` if enketoId is `undefined`.
+  return enketoId ?? null;
 };
 
 // Pushes a form that is published or about to be published to Enketo. Accepts

test/integration/api/forms/draft.js

@@ -1265,6 +1265,7 @@ describe('api: /projects/:id/forms (drafts)', () => {
         should.not.exist(beforeWorker.enketoOnceId);
 
         // Second request, from worker
+        global.enketo.callCount.should.equal(2);
         await exhaust(container);
         global.enketo.callCount.should.equal(3);
         global.enketo.receivedUrl.should.equal(`${container.env.domain}/v1/projects/1`);

test/integration/fixtures/02-forms.js

@@ -1,21 +1,22 @@
 const appRoot = require('app-root-path');
 const { Form } = require(appRoot + '/lib/model/frames');
-const { QueryOptions } = require(appRoot + '/lib/util/db');
 const { simple, withrepeat } = require('../../data/xml').forms;
 const forms = [ simple, withrepeat ];
 
-module.exports = async ({ Actors, Assignments, Forms, Projects, Roles }) => {
+module.exports = async ({ Assignments, Forms, Projects, Roles }) => {
   const project = (await Projects.getById(1)).get();
+  const { id: formview } = (await Roles.getBySystemName('formview')).get();
   /* eslint-disable no-await-in-loop */
   for (const xml of forms) {
     const partial = await Form.fromXml(xml);
+    // Create the form without Enketo IDs in order to maintain existing tests.
+    global.enketo.state = 'error';
     const { acteeId } = await Forms.createNew(partial, project, true);
 
-    // Delete the formview actor created by Forms.createNew() in order to
-    // maintain existing tests.
-    const { id: roleId } = (await Roles.getBySystemName('formview')).get();
-    const [{ actor }] = await Assignments.getByActeeAndRoleId(acteeId, roleId, QueryOptions.extended);
-    await Actors.del(actor);
+    // Delete the assignment of the formview actor created by Forms.createNew()
+    // in order to maintain existing tests.
+    const [{ actorId }] = await Assignments.getByActeeAndRoleId(acteeId, formview);
+    await Assignments.revokeByActorId(actorId);
   }
   /* eslint-enable no-await-in-loop */
 };

test/integration/setup.js

@@ -41,7 +41,10 @@ const bcrypt = require(appRoot + '/lib/util/crypto').password(_bcrypt);
 
 // set up our enketo mock.
 const { reset: resetEnketo, ...enketo } = require(appRoot + '/test/util/enketo');
+// Initialize the mock before other setup that uses the mock, then reset the
+// mock after setup is complete and after each test.
 before(resetEnketo);
+after(resetEnketo);
 afterEach(resetEnketo);
 
 // set up odk analytics mock.
@@ -84,12 +87,7 @@ const initialize = async () => {
     await migrator.destroy();
   }
 
-  // When creating fixtures, create forms without Enketo IDs in order to
-  // maintain existing tests.
-  global.enketo.state = 'error';
-  return withDefaults({ db, bcrypt, context, enketo, env })
-    .transacting(populate)
-    .finally(resetEnketo);
+  return withDefaults({ db, bcrypt, context, enketo, env }).transacting(populate);
 };
 
 // eslint-disable-next-line func-names, space-before-function-paren

test/integration/task/reap-sessions.js

@@ -10,7 +10,9 @@ describe('task: reap-sessions', () => {
       .then((actor) => Promise.all([ 2000, 2001, 2002, 2003, 3000, 3001, 3002, 3003 ]
         .map((year) => Sessions.create(actor, new Date(`${year}-01-01`)))))
       .then(() => reapSessions())
-      .then(() => oneFirst(sql`select count(*) from sessions`))
+      .then(() => oneFirst(sql`
+SELECT count(*) FROM sessions
+JOIN actors ON actors.id = sessions."actorId" AND actors.type = 'actor'`))
       .then((count) => { count.should.equal(4); })));
 });
 

test/util/enketo.js

@@ -7,7 +7,8 @@ const Problem = require(appRoot + '/lib/util/problem');
 const { without } = require(appRoot + '/lib/util/util');
 
 const defaults = {
-  // Properties that each test can set to determine the behavior of the mock
+  // Properties that can be set to change the behavior of the mock. These
+  // properties are reset after each mock request.
 
   // If `state` is set to 'error', the mock will pretend that Enketo has
   // misbehaved and will return a rejected promise for the next call.
@@ -20,7 +21,8 @@ const defaults = {
   // Properties that the mock may update after being called. These properties
   // are how the mock communicates back to the test.
 
-  // The total number of times that the mock has been called during the test
+  // The number of times that the mock has been called during the test, that is,
+  // the number of requests that would be sent to Enketo
   callCount: 0,
   // The OpenRosa URL that was passed to the create() method
   receivedUrl: undefined,