diff --git a/lib/model/query/datasets.js b/lib/model/query/datasets.js index 6a64c59e0..e41107653 100644 --- a/lib/model/query/datasets.js +++ b/lib/model/query/datasets.js @@ -43,7 +43,7 @@ const _insertDatasetDef = (dataset, acteeId, actions) => sql` const _insertProperties = (fields) => sql` ,fields("propertyName", "formDefId", "schemaId", path) AS (VALUES - ${sql.join(fields.map(p => sql`( ${sql.join([p.aux.propertyName, p.aux.formDefId, p.schemaId, p.path], sql`,`)} )`), sql`,`)} + ${sql.join(fields.map(p => sql`( ${sql.join([p.aux.propertyName, p.aux.formDefId, p.aux.schemaId, p.path], sql`,`)} )`), sql`,`)} ), insert_properties AS ( INSERT INTO ds_properties (id, name, "datasetId") @@ -82,6 +82,7 @@ SELECT "action", "id" FROM ds // * information about the dataset parsed from the form XML // * form (a Form frame or object containing projectId, formDefId, and schemaId) // * array of field objects and property names that were parsed from the form xml +// (form fields do not need any extra IDs of the form, form def, or schema.) const createOrMerge = (parsedDataset, form, fields) => async ({ one, Actees, Datasets, Projects }) => { const { projectId } = form; const { id: formDefId, schemaId } = form.def; diff --git a/lib/model/query/forms.js b/lib/model/query/forms.js index 86cf7cacb..d72d9750a 100644 --- a/lib/model/query/forms.js +++ b/lib/model/query/forms.js @@ -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, timebound } = require('../../util/promise'); const { splitStream } = require('../../util/stream'); -const { construct, noop, pickAll } = require('../../util/util'); +const { construct } = require('../../util/util'); const Option = require('../../util/option'); const Problem = require('../../util/problem'); @@ -84,40 +84,50 @@ const pushFormToEnketo = ({ projectId, xmlFormId, acteeId }, bound = undefined) return timeboundEnketo(enketo.create(path, xmlFormId, token), bound); }; +//////////////////////////////////////////////////////////////////////////////// +// COMMON FORM UTILITY FUNCTIONS + +const _parseFormXml = (partial) => Promise.all([ + getFormFields(partial.xml), + (partial.aux.key.isDefined() ? resolve(Option.none()) : getDataset(partial.xml)) // Don't parse dataset schema if Form has encryption key +]); + +const _insertFormFields = (fields, formId, schemaId) => async ({ run }) => { + // Combine form fields as parsed from the form XML with the top level formId + // and the schemaId (rather than formDefId) + const ids = { formId, schemaId }; + const fieldsForInsert = fields.map((field) => new Form.Field(Object.assign({}, { ...field, ...ids }))); + await run(insertMany(fieldsForInsert)); +}; //////////////////////////////////////////////////////////////////////////////// // CREATING NEW FORMS -const _createNew = (form, def, project, publish) => ({ oneFirst, Forms }) => +const _createNew = (form, def, project) => ({ oneFirst, Forms }) => oneFirst(sql` with sch as (insert into form_schemas (id) values (default) returning *), def as - (insert into form_defs ("formId", "schemaId", xml, name, hash, sha, sha256, version, "keyId", "xlsBlobId", "draftToken", "enketoId", "createdAt", "publishedAt") - select nextval(pg_get_serial_sequence('forms', 'id')), sch.id, ${form.xml}, ${def.name}, ${def.hash}, ${def.sha}, ${def.sha256}, ${def.version}, ${def.keyId}, ${form.xls.xlsBlobId || null}, ${def.draftToken || null}, ${def.enketoId || null}, clock_timestamp(), ${(publish === true) ? sql`clock_timestamp()` : null} + (insert into form_defs ("formId", "schemaId", xml, name, hash, sha, sha256, version, "keyId", "xlsBlobId", "draftToken", "enketoId", "createdAt") + select nextval(pg_get_serial_sequence('forms', 'id')), sch.id, ${form.xml}, ${def.name}, ${def.hash}, ${def.sha}, ${def.sha256}, ${def.version}, ${def.keyId}, ${form.xls.xlsBlobId || null}, ${def.draftToken || null}, ${def.enketoId || null}, clock_timestamp() from sch returning *), form as - (insert into forms (id, "xmlFormId", state, "projectId", ${sql.identifier([ (publish === true) ? 'currentDefId' : 'draftDefId' ])}, "acteeId", "enketoId", "enketoOnceId", "createdAt") + (insert into forms (id, "xmlFormId", state, "projectId", "draftDefId", "acteeId", "enketoId", "enketoOnceId", "createdAt") select def."formId", ${form.xmlFormId}, ${form.state || 'open'}, ${project.id}, def.id, ${form.acteeId}, ${form.enketoId || null}, ${form.enketoOnceId || null}, def."createdAt" from def returning forms.*) select id from form`) - .then(() => Forms.getByProjectAndXmlFormId(project.id, form.xmlFormId, false, - (publish === true) ? undefined : Form.DraftVersion)) + .then(() => Forms.getByProjectAndXmlFormId(project.id, form.xmlFormId, false, Form.DraftVersion)) .then((option) => option.get()); -const createNew = (partial, project, publish = false) => async ({ run, Actees, Datasets, FormAttachments, Forms, Keys }) => { +const createNew = (partial, project) => async ({ Actees, Datasets, FormAttachments, Forms, Keys }) => { // Check encryption keys before proceeding - const defData = {}; - defData.keyId = await partial.aux.key.map(Keys.ensure).orElse(resolve(null)); + const keyId = await partial.aux.key.map(Keys.ensure).orElse(resolve(null)); // Parse form XML for fields and entity/dataset definitions - const [fields, parsedDataset] = await Promise.all([ - getFormFields(partial.xml), - (partial.aux.key.isDefined() ? resolve(Option.none()) : getDataset(partial.xml)) // Don't parse dataset schema if Form has encryption key - ]); + const [fields, parsedDataset] = await _parseFormXml(partial); // Check that meta field (group containing instanceId and name) exists await Forms.checkMeta(fields); @@ -126,60 +136,40 @@ const createNew = (partial, project, publish = false) => async ({ run, Actees, D await Forms.checkDeletedForms(partial.xmlFormId, project.id); await Forms.rejectIfWarnings(); - const formData = {}; - formData.acteeId = (await Actees.provision('form', project)).id; + // Provision Actee for form + const acteeId = (await Actees.provision('form', project)).id; // We will try to push to Enketo. If doing so fails or is too slow, then the // worker will try again later. - if (publish !== true) { - defData.draftToken = generateToken(); - defData.enketoId = await Forms.pushDraftToEnketo( - { projectId: project.id, xmlFormId: partial.xmlFormId, draftToken: defData.draftToken }, - 0.5 - ); - } else { - const enketoIds = await Forms.pushFormToEnketo( - { projectId: project.id, xmlFormId: partial.xmlFormId, acteeId: formData.acteeId }, - 0.5 - ); - Object.assign(formData, enketoIds); - } + const draftToken = generateToken(); + const enketoId = await Forms.pushDraftToEnketo( + { projectId: project.id, xmlFormId: partial.xmlFormId, draftToken }, + 0.5 + ); - // Save form + // Save draft form const savedForm = await Forms._createNew( - partial.with(formData), - partial.def.with(defData), - project, - publish + partial.with({ acteeId }), + partial.def.with({ keyId, draftToken, enketoId }), + project ); - // Prepare the form fields - const ids = { formId: savedForm.id, schemaId: savedForm.def.schemaId }; - - // Insert fields and setup form attachment slots - await Promise.all([ - run(insertMany(fields.map((field) => new Form.Field(Object.assign(field, ids))))), - FormAttachments.createNew(partial.xml, savedForm, partial.xls.itemsets) - ]); + // Insert form fields and attachments for new form def + await Forms._insertFormFields(fields, savedForm.id, savedForm.def.schemaId); + await FormAttachments.createNew(partial.xml, savedForm, partial.xls.itemsets); - // Update datasets and properties if defined + // Update datasets and properties, if defined if (parsedDataset.isDefined()) { await Datasets.createOrMerge(parsedDataset.get(), savedForm, fields); - - if (publish) { - await Datasets.publishIfExists(savedForm.def.id, savedForm.def.publishedAt.toISOString()); - } } - // Return the final saved form + // Return new draft Form frame with Form.Def return savedForm; }; // (if we are asked to publish right away, log that too:) -createNew.audit = (form, partial, _, publish) => (log) => - log('form.create', form).then(() => ((publish === true) - ? log('form.update.publish', form, { newDefId: form.currentDefId }) - : null)); +createNew.audit = (form) => (log) => + log('form.create', form); createNew.audit.withResult = true; //////////////////////////////////////////////////////////////////////////////// @@ -188,34 +178,15 @@ createNew.audit.withResult = true; // 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) => async ({ one, Forms }) => { - const insertWith = (moreData) => one(insert(partial.def.with({ +const _createNewDef = (partial, form, publish, data) => ({ one }) => + one(insert(partial.def.with({ formId: form.id, xlsBlobId: partial.xls.xlsBlobId, xml: partial.xml, ...data, - ...moreData + ...((publish === true) ? { publishedAt: new Date() } : {}) }))); - 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 and enketoId. - if (form.def.id == null || form.def.id !== form.draftDefId) { - const draftToken = generateToken(); - // Try to push the draft to Enketo. If doing so fails or is too slow, then - // the worker will try again later. - const enketoId = await Forms.pushDraftToEnketo( - { projectId: form.projectId, xmlFormId: form.xmlFormId, draftToken }, - 0.5 - ); - return insertWith({ draftToken, enketoId }); - } - - // 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) @@ -230,10 +201,10 @@ const _createNewDef = (partial, form, publish, data) => async ({ one, Forms }) = // =========== // 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 +// publish: set true if you want new version to be published (only used by setManagedKey, everywhere else calls publish() explicitly) // 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, duplicating = false) => async ({ run, Datasets, FormAttachments, Forms, Keys }) => { +const createVersion = (partial, form, publish, duplicating = false) => async ({ 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' })); @@ -242,10 +213,7 @@ const createVersion = (partial, form, publish, duplicating = false) => async ({ const keyId = await partial.aux.key.map(Keys.ensure).orElse(resolve(null)); // Parse form fields and dataset/entity definition from form XML - const [fields, parsedDataset] = await Promise.all([ - getFormFields(partial.xml), - (partial.aux.key.isDefined() ? resolve(Option.none()) : getDataset(partial.xml)) - ]); + const [fields, parsedDataset] = await _parseFormXml(partial); // Compute the intermediate schema ID let schemaId; @@ -279,32 +247,42 @@ const createVersion = (partial, form, publish, duplicating = false) => async ({ } } - const savedDef = await Forms._createNewDef(partial, form, publish, { schemaId, keyId }); + // 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; + if (!publish && (form.def.id == null || form.def.id !== form.draftDefId)) { + draftToken = generateToken(); + enketoId = await Forms.pushDraftToEnketo( + { projectId: form.projectId, xmlFormId: form.xmlFormId, draftToken }, + 0.5 + ); + } + // Note: If publish=true, we are in the enabling encryption special case, and we + // will just copy over whatever enketo ids do or don't exist already. - // Update corresponding form - await ((publish === true) - ? Forms._update(form, { currentDefId: savedDef.id }) - : Forms._update(form, { draftDefId: savedDef.id })); + // Save draft def + const savedDef = await Forms._createNewDef(partial, form, publish, { draftToken, enketoId, schemaId, keyId }); // Prepare the form fields - const ids = { formId: form.id, schemaId }; - const fieldsForInsert = new Array(fields.length); - for (let i = 0; i < fields.length; i += 1) - fieldsForInsert[i] = new Form.Field(Object.assign({}, fields[i], ids)); - - // Insert fields and setup form attachments - await Promise.all([ - (!match) ? run(insertMany(fieldsForInsert)) : noop, - FormAttachments.createVersion(partial.xml, form, savedDef, partial.xls.itemsets, publish) - ]); - - // Update datasets and properties if defined + if (!match) + await Forms._insertFormFields(fields, form.id, schemaId); + + // Insert attachments + await FormAttachments.createVersion(partial.xml, form, savedDef, partial.xls.itemsets, publish); + + // Update datasets and properties, if defined if (parsedDataset.isDefined()) { - await Datasets.createOrMerge(parsedDataset.get(), new Form(form, { def: savedDef }), fieldsForInsert); - if (publish) { - await Datasets.publishIfExists(savedDef.id, savedDef.publishedAt.toISOString()); - } + await Datasets.createOrMerge(parsedDataset.get(), new Form(form, { def: savedDef }), fields); + // Note: if publish=true, we are in the enabling encryption special case and won't + // do dataset operations. Dataset publishing will happen only through Forms.publish(). } + + // Note: It is rare that a form will be published here (and not in a separate step). + // publish=true is only used when encrypting a published form + await ((publish === true) + ? Forms._update(form, { currentDefId: savedDef.id }) + : Forms._update(form, { draftDefId: savedDef.id })); + return savedDef; }; @@ -319,18 +297,21 @@ createVersion.audit.withResult = true; // TODO: we need to make more explicit what .def actually represents throughout. // for now, enforce an extra check here just in case. -const publish = (form) => async ({ Forms, Datasets, FormAttachments }) => { +const publish = (form, concurrentWithCreate = false) => async ({ Forms, Datasets, FormAttachments }) => { if (form.draftDefId !== form.def.id) throw Problem.internal.unknown(); + // dont use publish concurrentWithCreate on already published forms + if (concurrentWithCreate && form.publishedAt != null) throw Problem.internal.unknown({ error: 'Attempting to immediately publish a form' }); + // Try to push the form to Enketo if it hasn't been pushed already. If doing // so fails or is too slow, then the worker will try again later. const enketoIds = form.enketoId == null || form.enketoOnceId == null ? await Forms.pushFormToEnketo(form, 0.5) : {}; - const publishedAt = (new Date()).toISOString(); - await Promise.all([ - Forms._update(form, { currentDefId: form.draftDefId, draftDefId: null, ...enketoIds }), + const publishedAt = concurrentWithCreate ? form.createdAt.toISOString() : (new Date()).toISOString(); + const [f, fd] = await Promise.all([ + Forms._update(form, { currentDefId: form.draftDefId, draftDefId: null, ...(concurrentWithCreate ? { updatedAt: null } : {}), ...enketoIds }), Forms._updateDef(form.def, { draftToken: null, enketoId: null, publishedAt }), ]) .catch(Problem.translate( @@ -350,9 +331,14 @@ const publish = (form) => async ({ Forms, Datasets, FormAttachments }) => { await FormAttachments.update(form, attachment.get(), null, dataset.id); } } + + return new Form(f, { def: new Form.Def(fd) }); }; -publish.audit = (form) => (log) => log('form.update.publish', form, + +// We don't need the new Form but we do need to wait for the result before logging. +publish.audit = (_, form) => (log) => log('form.update.publish', form, { oldDefId: form.currentDefId, newDefId: form.draftDefId }); +publish.audit.withResult = true; const clearDraft = (form) => ({ Forms }) => Forms._update(form, { draftDefId: null }); @@ -802,7 +788,9 @@ const _newSchema = () => ({ one }) => .then((s) => s.id); module.exports = { - fromXls, pushDraftToEnketo, pushFormToEnketo, _createNew, createNew, _createNewDef, createVersion, + fromXls, pushDraftToEnketo, pushFormToEnketo, + _insertFormFields, + _createNew, createNew, _createNewDef, createVersion, publish, clearDraft, _update, update, _updateDef, del, restore, purge, clearUnneededDrafts, diff --git a/lib/resources/forms.js b/lib/resources/forms.js index 9ac60a898..fc2c0f913 100644 --- a/lib/resources/forms.js +++ b/lib/resources/forms.js @@ -113,7 +113,10 @@ module.exports = (service, endpoint) => { .then(getOrNotFound) .then((project) => auth.canOrReject('form.create', project)) .then((project) => getPartial(Forms, request, project, Keys) - .then((partial) => Forms.createNew(partial, project, isTrue(query.publish)))))); + .then((partial) => Forms.createNew(partial, project)) + .then((newForm) => (isTrue(query.publish) + ? Forms.publish(newForm, true) + : newForm))))); // can POST empty body to copy the current def to draft. service.post('/projects/:projectId/forms/:id/draft', endpoint(({ Forms, Keys, Projects, Submissions }, { params, auth }, request) => diff --git a/lib/util/db.js b/lib/util/db.js index dcbf9e8c7..2bd29fb35 100644 --- a/lib/util/db.js +++ b/lib/util/db.js @@ -184,6 +184,7 @@ const insertMany = (objs) => { }; // generic update utility +// will set updatedAt if it exists, as long as it is not overwritten in the data const updater = (obj, data, whereKey = 'id') => { const keys = Object.keys(data); if (keys.length === 0) return sql`select true`; @@ -191,7 +192,7 @@ const updater = (obj, data, whereKey = 'id') => { return sql` update ${raw(obj.constructor.table)} set ${sql.join(keys.map((k) => sql`${sql.identifier([ k ])}=${assigner(k)}`), sql`,`)} -${obj.constructor.hasUpdatedAt ? sql`,"updatedAt"=clock_timestamp()` : nothing} +${(obj.constructor.hasUpdatedAt && !keys.includes('updatedAt')) ? sql`,"updatedAt"=clock_timestamp()` : nothing} where ${sql.identifier([ whereKey ])}=${obj[whereKey]} returning *`; }; diff --git a/lib/util/problem.js b/lib/util/problem.js index d3497d68f..76fde7b55 100644 --- a/lib/util/problem.js +++ b/lib/util/problem.js @@ -190,7 +190,7 @@ const problems = { // special version of uniquenessViolation to be more informative about the publish-version // conflict case. - versionUniquenessViolation: problem(409.6, ({ xmlFormId, version }) => `You tried to publish the form '${xmlFormId}' with version '${version}', but a published form has already existed in this project with those identifiers. Please choose a new name and try again. You can re-request with ?version=xyz to have the version changed in-place for you.`), + versionUniquenessViolation: problem(409.6, ({ xmlFormId, version }) => `You tried to publish the form '${xmlFormId}' with version '${version}', but a published form has already existed in this project with those identifiers.`), // tried to reuse an instanceId instanceIdConflict: problem(409.8, () => 'You tried to update a submission, but the instanceID you provided already exists on this server. Please try again with a unique instanceID.'), diff --git a/test/integration/api/audits.js b/test/integration/api/audits.js index ebeddb473..cfb4421d8 100644 --- a/test/integration/api/audits.js +++ b/test/integration/api/audits.js @@ -134,7 +134,7 @@ describe('/audits', () => { audits[1].action.should.equal('form.update.publish'); audits[1].acteeId.should.equal(form.acteeId); audits[1].actee.should.eql(plain(form.forApi())); - audits[1].details.should.eql({ newDefId: form.currentDefId }); + audits[1].details.should.eql({ newDefId: form.currentDefId, oldDefId: null }); audits[1].loggedAt.should.be.a.recentIsoDate(); audits[2].actorId.should.equal(alice.actor.id); @@ -637,8 +637,8 @@ describe('/audits', () => { body.length.should.equal(4); body.map(a => a.action).should.eql([ 'form.update.publish', - 'form.create', 'dataset.create', + 'form.create', 'user.session.create' ]); }); diff --git a/test/integration/api/datasets.js b/test/integration/api/datasets.js index 8d678359d..f336a509a 100644 --- a/test/integration/api/datasets.js +++ b/test/integration/api/datasets.js @@ -2214,10 +2214,7 @@ describe('datasets and entities', () => { `; - it('should NOT autolink on upload new form and simultaneously publish', testService(async (service) => { - // this path of directly publishing a form on upload isn't possible in central - // so it's not going to be supported. if it were, logic would go around line #170 in - // lib/model/query/forms.js in Forms.createNew after the dataset is published. + it('should autolink on upload new form and simultaneously publish', testService(async (service) => { const asAlice = await service.login('alice'); await asAlice.post('/v1/projects/1/forms?publish=true') @@ -2228,7 +2225,7 @@ describe('datasets and entities', () => { await asAlice.get('/v1/projects/1/forms/updateEntity/attachments') .then(({ body }) => { body[0].name.should.equal('people.csv'); - body[0].datasetExists.should.be.false(); + body[0].datasetExists.should.be.true(); }); })); @@ -3939,6 +3936,38 @@ describe('datasets and entities', () => { audit.should.equal(Option.none()); })); + it('should log appropriate sequence of form and dataset events', testService(async (service) => { + + const asAlice = await service.login('alice'); + + await asAlice.post('/v1/projects/1/forms?publish=true') + .send(testData.forms.simpleEntity) + .set('Content-Type', 'text/xml') + .expect(200); + + await asAlice.post('/v1/projects/1/forms/simpleEntity/draft') + .set('Content-Type', 'application/xml') + .send(testData.forms.simpleEntity.replace('orx:version="1.0"', 'orx:version="draft1"').replace(/first_name/g, 'nickname')) + .expect(200); + + await asAlice.post('/v1/projects/1/forms/simpleEntity/draft/publish'); + + await asAlice.get('/v1/audits?action=nonverbose') + .expect(200) + .then(({ body }) => { + body.length.should.equal(7); + body.map(a => a.action).should.eql([ + 'form.update.publish', + 'dataset.update', + 'form.update.draft.set', + 'form.update.publish', + 'dataset.create', + 'form.create', + 'user.session.create' + ]); + }); + })); + it('should not log dataset modification when no new property is added', testService(async (service, { Audits }) => { const asAlice = await service.login('alice'); @@ -3958,7 +3987,7 @@ describe('datasets and entities', () => { audit.should.equal(Option.none()); })); - it('should log dataset publishing in audit log', testService(async (service, { Audits }) => { + it('should log dataset publishing with properties in audit log', testService(async (service, { Audits }) => { const asAlice = await service.login('alice'); diff --git a/test/integration/api/forms/draft.js b/test/integration/api/forms/draft.js index a54c81558..47d4ae648 100644 --- a/test/integration/api/forms/draft.js +++ b/test/integration/api/forms/draft.js @@ -3,6 +3,8 @@ const appRoot = require('app-root-path'); const should = require('should'); const { testService } = require('../../setup'); const testData = require('../../../data/xml'); +const { Form } = require(appRoot + '/lib/model/frames'); + const { exhaust } = require(appRoot + '/lib/worker/worker'); const { sql } = require('slonik'); @@ -1263,9 +1265,9 @@ describe('api: /projects/:id/forms (drafts)', () => { .expect(200); should.not.exist(beforeWorker.enketoId); should.not.exist(beforeWorker.enketoOnceId); + global.enketo.callCount.should.equal(2); // Second request, from the 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`); @@ -1328,6 +1330,68 @@ describe('api: /projects/:id/forms (drafts)', () => { body[1].details.newDraftDefId.should.equal(form.currentDefId); }); }))))); + + it('should log the correct def ids in the form audit log', testService(async (service, { Forms }) => { + const asAlice = await service.login('alice'); + + await asAlice.post('/v1/projects/1/forms?publish=true') + .send(testData.forms.simple2) + .set('Content-Type', 'text/xml') + .expect(200); + + const formT1 = await Forms.getByProjectAndXmlFormId(1, 'simple2').then((o) => o.get()); + + await asAlice.post('/v1/projects/1/forms/simple2/draft') + .set('Content-Type', 'application/xml') + .send(testData.forms.simple2.replace('version="2.1"', 'version="2.2"')) + .expect(200); + + const formT2 = await Forms.getByProjectAndXmlFormId(1, 'simple2', false, Form.DraftVersion).then((o) => o.get()); + + await asAlice.post('/v1/projects/1/forms/simple2/draft') + .set('Content-Type', 'application/xml') + .send(testData.forms.simple2.replace('version="2.1"', 'version="2.3"')) + .expect(200); + + const formT3 = await Forms.getByProjectAndXmlFormId(1, 'simple2', false, Form.DraftVersion).then((o) => o.get()); + + await asAlice.post('/v1/projects/1/forms/simple2/draft/publish'); + + const formT4 = await Forms.getByProjectAndXmlFormId(1, 'simple2').then((o) => o.get()); + + await asAlice.get('/v1/audits?action=nonverbose') + .expect(200) + .then(({ body }) => { + // first form event: form create + body[4].action.should.equal('form.create'); + + // second form event: form publish + body[3].action.should.equal('form.update.publish'); + // new def id should match form def at time T1 + body[3].details.should.eql({ newDefId: formT1.def.id, oldDefId: null }); + + // third form event: draft + body[2].action.should.equal('form.update.draft.set'); + body[2].details.should.eql({ newDraftDefId: formT2.def.id, oldDraftDefId: null }); + + // forth form event: draft + body[1].action.should.equal('form.update.draft.set'); + body[1].details.should.eql({ newDraftDefId: formT3.def.id, oldDraftDefId: formT2.def.id }); + + // fifth form event: publish + body[0].action.should.equal('form.update.publish'); + body[0].details.should.eql({ newDefId: formT4.def.id, oldDefId: formT1.def.id }); + + body.map(a => a.action).should.eql([ + 'form.update.publish', + 'form.update.draft.set', + 'form.update.draft.set', + 'form.update.publish', + 'form.create', + 'user.session.create' + ]); + }); + })); }); describe('/fields GET', () => { diff --git a/test/integration/api/forms/forms.js b/test/integration/api/forms/forms.js index ef1f3fab2..185406442 100644 --- a/test/integration/api/forms/forms.js +++ b/test/integration/api/forms/forms.js @@ -74,8 +74,8 @@ describe('api: /projects/:id/forms (create, read, update)', () => { .set('Content-Type', 'application/xml') .expect(409) .then(({ body }) => { - body.details.fields.should.eql([ 'projectId', 'xmlFormId', 'version' ]); - body.details.values.should.eql([ '1', 'simple', '' ]); + body.message.should.eql("You tried to publish the form 'simple' with version '', but a published form has already existed in this project with those identifiers."); + body.details.should.eql({ xmlFormId: 'simple', version: '' }); }))))); it('should return the created form upon success', testService((service) => @@ -295,7 +295,35 @@ describe('api: /projects/:id/forms (create, read, update)', () => { .expect(200) .then(({ body }) => { body.publishedAt.should.be.a.recentIsoDate(); - }))))); + body.publishedAt.should.eql(body.createdAt); + (body.updatedAt == null).should.equal(true); + }) + .then(() => asAlice.get('/v1/audits?action=form') + .expect(200) + .then(({ body }) => { + body.map((a) => a.action).should.eql(['form.update.publish', 'form.create']); + })))))); + + it('should have published timestamp different from create timestamp if published separately', testService((service) => + service.login('alice', (asAlice) => + asAlice.post('/v1/projects/1/forms') + .send(testData.forms.simple2) + .set('Content-Type', 'application/xml') + .expect(200) + .then(() => asAlice.post('/v1/projects/1/forms/simple2/draft/publish') + .expect(200)) + .then(() => asAlice.get('/v1/projects/1/forms/simple2') + .expect(200) + .then(({ body }) => { + body.publishedAt.should.be.a.recentIsoDate(); + body.publishedAt.should.not.eql(body.createdAt); + body.updatedAt.should.be.a.recentIsoDate(); + }) + .then(() => asAlice.get('/v1/audits?action=form') + .expect(200) + .then(({ body }) => { + body.map((a) => a.action).should.eql(['form.update.publish', 'form.create']); + })))))); describe('Enketo ID for draft', () => { it('should request an enketoId', testService(async (service, { env }) => { @@ -380,7 +408,8 @@ describe('api: /projects/:id/forms (create, read, update)', () => { .set('Content-Type', 'application/xml') .send(testData.forms.simple2) .expect(200); - global.enketo.callCount.should.equal(1); + // This will make a published enketo token and a draft token even though the draft is not used + global.enketo.callCount.should.equal(2); without(['token'], global.enketo.createData).should.eql({ openRosaUrl: `${env.domain}/v1/projects/1`, xmlFormId: 'simple2' @@ -392,6 +421,7 @@ describe('api: /projects/:id/forms (create, read, update)', () => { it('should return with success even if request to Enketo fails', testService(async (service) => { const asAlice = await service.login('alice'); global.enketo.state = 'error'; + global.enketo.autoReset = false; const { body } = await asAlice.post('/v1/projects/1/forms?publish=true') .set('Content-Type', 'application/xml') .send(testData.forms.simple2) @@ -403,6 +433,7 @@ describe('api: /projects/:id/forms (create, read, update)', () => { it('should wait for Enketo only briefly @slow', testService(async (service) => { const asAlice = await service.login('alice'); global.enketo.wait = (done) => { setTimeout(done, 600); }; + global.enketo.autoReset = false; const { body } = await asAlice.post('/v1/projects/1/forms?publish=true') .set('Content-Type', 'application/xml') .send(testData.forms.simple2) @@ -411,20 +442,36 @@ describe('api: /projects/:id/forms (create, read, update)', () => { should.not.exist(body.enketoOnceId); })); + it('should wait for published Enketo only briefly @slow', testService(async (service) => { + const asAlice = await service.login('alice'); + await asAlice.post('/v1/projects/1/forms') + .set('Content-Type', 'application/xml') + .send(testData.forms.simple2) + .expect(200); + global.enketo.wait = (done) => { setTimeout(done, 600); }; + const { body } = await asAlice.post('/v1/projects/1/forms/simple2/draft/publish'); + should.not.exist(body.enketoId); + should.not.exist(body.enketoOnceId); + })); + it('should request Enketo IDs from worker if request from endpoint fails', testService(async (service, container) => { const asAlice = await service.login('alice'); // First request to Enketo, from the endpoint global.enketo.state = 'error'; + global.enketo.autoReset = false; await asAlice.post('/v1/projects/1/forms?publish=true') .set('Content-Type', 'application/xml') .send(testData.forms.simple2) .expect(200); + // reset enketo mock to its default behavior + // also resets callCount + global.enketo.reset(); + // Second request, from the worker - global.enketo.callCount.should.equal(1); await exhaust(container); - global.enketo.callCount.should.equal(2); + global.enketo.callCount.should.equal(1); const { body } = await asAlice.get('/v1/projects/1/forms/simple2') .expect(200); without(['token'], global.enketo.createData).should.eql({ @@ -441,9 +488,9 @@ describe('api: /projects/:id/forms (create, read, update)', () => { .set('Content-Type', 'application/xml') .send(testData.forms.simple2) .expect(200); - global.enketo.callCount.should.equal(1); + global.enketo.callCount.should.equal(2); await exhaust(container); - global.enketo.callCount.should.equal(1); + global.enketo.callCount.should.equal(2); })); }); diff --git a/test/integration/fixtures/02-forms.js b/test/integration/fixtures/02-forms.js index 1312c6c9c..a43d89d84 100644 --- a/test/integration/fixtures/02-forms.js +++ b/test/integration/fixtures/02-forms.js @@ -6,18 +6,25 @@ const forms = [ simple, withrepeat ]; module.exports = async ({ Assignments, Forms, Projects, Roles }) => { const project = (await Projects.getById(1)).get(); const { id: formview } = (await Roles.getBySystemName('formview')).get(); + + // Create the forms without Enketo IDs in order to maintain existing tests. + global.enketo.state = 'error'; + global.enketo.autoReset = false; + /* 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); + const form = await Forms.createNew(partial, project); + await Forms.publish(form, true); // Delete the assignment of the formview actor created by Forms.createNew() // in order to maintain existing tests. - const [{ actorId }] = await Assignments.getByActeeAndRoleId(acteeId, formview); + const [{ actorId }] = await Assignments.getByActeeAndRoleId(form.acteeId, formview); await Assignments.revokeByActorId(actorId); } /* eslint-enable no-await-in-loop */ + + // Reset enketo to not affect tests + global.enketo.reset(); }; diff --git a/test/integration/other/form-versioning.js b/test/integration/other/form-versioning.js index f045686d9..3bdf0317b 100644 --- a/test/integration/other/form-versioning.js +++ b/test/integration/other/form-versioning.js @@ -72,7 +72,8 @@ describe('form forward versioning', () => { Form.fromXml(testData.forms.withAttachments), Blob.fromFile(__filename).then(Blobs.ensure) ]) - .then(([ project, partial, blobId ]) => Forms.createNew(partial, project, true) + .then(([ project, partial, blobId ]) => Forms.createNew(partial, project) + .then((formDraft) => Forms.publish(formDraft, true)) .then((savedForm) => Promise.all([ 'goodone.csv', 'goodtwo.mp3' ] .map((name) => FormAttachments.getByFormDefIdAndName(savedForm.def.id, name) .then(force) @@ -106,7 +107,8 @@ describe('form forward versioning', () => { Form.fromXml(testData.forms.withAttachments), Blob.fromFile(__filename).then(Blobs.ensure) ]) - .then(([ project, partial, blobId ]) => Forms.createNew(partial, project, true) + .then(([ project, partial, blobId ]) => Forms.createNew(partial, project) + .then((formDraft) => Forms.publish(formDraft, true)) .then((savedForm) => Promise.all([ 'goodone.csv', 'goodtwo.mp3' ] .map((name) => FormAttachments.getByFormDefIdAndName(savedForm.def.id, name) .then(force) diff --git a/test/util/enketo.js b/test/util/enketo.js index 3b5f97b07..91d6a84ee 100644 --- a/test/util/enketo.js +++ b/test/util/enketo.js @@ -29,13 +29,17 @@ const defaults = { // The OpenRosa URL that was passed to the create() method receivedUrl: undefined, // An object with a property for each argument passed to the edit() method - editData: undefined + editData: undefined, + + // Control whether enketo resets after a request. Set to false in tests that + // need multiple error requests or timeouts in a row. + autoReset: true, }; let cancelToken = 0; const reset = () => { - if (global.enketo === undefined) global.enketo = {}; + if (global.enketo === undefined) global.enketo = { reset }; Object.assign(global.enketo, defaults); cancelToken += 1; }; @@ -44,7 +48,10 @@ const reset = () => { const request = () => { global.enketo.callCount += 1; const options = { ...global.enketo }; - Object.assign(global.enketo, without(['callCount'], defaults)); + + if (global.enketo.autoReset) + Object.assign(global.enketo, without(['callCount'], defaults)); + return new Promise((resolve, reject) => { const { wait } = options; const tokenBeforeWait = cancelToken;