Move webformsEnabled to form_defs

sadiqkhoja · sadiqkhoja · commit 30effbb0be1b · 2026-06-01T17:20:28.000-04:00
diff --git a/lib/model/frames/form.js b/lib/model/frames/form.js
@@ -25,7 +25,7 @@
 // As with Users/Actors, this information is merged together upon return via the
 // API. This is partly for legacy reasons: Forms and FormsDef did not used to
 
-const { Frame, table, readable, writable, into, embedded, fieldTypes } = require('../frame');
+const { Frame, table, readable, writable, into, embedded, fieldTypes, from } = require('../frame');
 const { injectPublicKey, addVersionSuffix } = require('../../data/schema');
 const { Key } = require('./key');
 const { md5sum, shasum, sha256sum, digestWith, generateVersionSuffix } = require('../../util/crypto');
@@ -61,12 +61,12 @@ class Form extends Frame.define(
   'enketoOnceId', readable,             'acteeId',
   'createdAt',    readable,             'updatedAt',    readable,
   'deletedAt',
-  'webformsEnabled', readable, writable,
   embedded('createdBy'), embedded('publishedBy')
 ) {
   get def() { return this.aux.def; }
   get xls() { return this.aux.xls || {}; } // TODO/SL sort of a hack but let's see how it goes.
   get xml() { return this.aux.xml?.xml; }
+  get associated() { return this.aux.formAssociated; }
 
   isCurrentDraft() {
     return this.draftDefId && this.def?.id === this.draftDefId;
@@ -98,11 +98,15 @@ class Form extends Frame.define(
       ? this.enketoOnceId
       : null;
 
+    // Sometime we don't request def or there is no (draft) def, in those cases we read the latest
+    // `webformsEnabled`;
+    const webformsEnabled = this.def?.webformsEnabled ?? this.associated?.webformsEnabled;
+
     // Include deletedAt in response only if it is not null (most likely on resources about soft-deleted forms)
     // and also include the numeric form id (used to restore)
     const deletedAtIdFields = this.deletedAt ? { deletedAt: this.deletedAt, id: this.id } : null;
 
-    return Object.assign(Frame.prototype.forApi.call(this), { enketoId, enketoOnceId }, deletedAtIdFields);
+    return Object.assign(Frame.prototype.forApi.call(this), { enketoId, enketoOnceId, webformsEnabled }, deletedAtIdFields);
   }
 
   // Given an XML string, returns Promise[Object]. If the Promise rejects, the XML
@@ -155,6 +159,10 @@ class Form extends Frame.define(
 
 Form.Partial = class extends Form {};
 
+Form.Associated = Frame.define(
+  from('formAssociated'), into('formAssociated'),
+  'webformsEnabled', readable, writable
+);
 
 ////////////////////////////////////////////////////////////////////////////////
 // EXTENDED FORM
@@ -202,7 +210,8 @@ Form.Def = class extends Frame.define(
   'sha256',      readable,              'draftToken',   readable,
   'enketoId',    readable,              'createdAt',
   'publishedAt', readable,              'xlsBlobId',
-  'name',        readable,              'schemaId'
+  'name',        readable,              'schemaId',
+  'webformsEnabled', readable, writable
 ) {
   isDraft() {
     return this.publishedAt == null;
diff --git a/lib/model/migrations/20260521-01-move-webformsenabled-to-form-defs.js b/lib/model/migrations/20260521-01-move-webformsenabled-to-form-defs.js
@@ -0,0 +1,38 @@
+const up = async (db) => {
+  await db.raw(`
+    ALTER TABLE form_defs
+      ADD COLUMN "webformsEnabled" boolean;
+  `);
+
+  await db.raw(`
+    UPDATE form_defs
+    SET "webformsEnabled" = forms."webformsEnabled"
+    FROM forms
+    WHERE form_defs.id = forms."currentDefId"
+  `);
+
+  await db.raw(`
+    ALTER TABLE forms
+      DROP COLUMN "webformsEnabled";
+  `);
+};
+
+const down = async (db) => {
+  await db.raw(`
+    ALTER TABLE forms
+      ADD COLUMN "webformsEnabled" boolean NOT NULL DEFAULT FALSE;
+  `);
+
+  await db.raw(`
+    UPDATE forms
+    SET "webformsEnabled" = 
+      (SELECT fd."webformsEnabled" FROM form_defs fd WHERE fd.id = forms."currentDefId");
+  `);
+
+  await db.raw(`
+    ALTER TABLE form_defs
+      DROP COLUMN "webformsEnabled";
+  `);
+};
+
+module.exports = { up, down };
diff --git a/lib/model/query/analytics.js b/lib/model/query/analytics.js
@@ -323,7 +323,8 @@ group by (p."id", f."state")`);
 const countFormsWebformsEnabled = () => ({ all }) => all(sql`
   select f."projectId" as "projectId", count(f."id") as total
   from forms as f
-  where f."webformsEnabled" is true
+  join form_defs as fd on fd.id = f."currentDefId"
+  where fd."webformsEnabled" is true
   group by f."projectId"`);
 
 // This query checks the audit log for 'form.delete' actions
diff --git a/lib/model/query/forms.js b/lib/model/query/forms.js
@@ -110,13 +110,13 @@ with sch as
     values (default)
     returning *),
 def as
-  (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()
+  (insert into form_defs ("formId", "schemaId", xml, name, hash, sha, sha256, version, "keyId", "xlsBlobId", "draftToken", "enketoId", "createdAt", "webformsEnabled")
+  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(), ${def.webformsEnabled}
     from sch
   returning *),
 form as
-  (insert into forms (id, "xmlFormId", state, "projectId", "draftDefId", "acteeId", "enketoId", "enketoOnceId", "createdAt", "webformsEnabled")
-  select def."formId", ${form.xmlFormId}, ${form.state || 'open'}, ${project.id}, def.id, ${form.acteeId}, ${form.enketoId || null}, ${form.enketoOnceId || null}, def."createdAt", ${form.webformsEnabled || DEFAULT_WEBFORMS_ENABLED} from def
+  (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, Form.DraftVersion))
@@ -149,10 +149,12 @@ const createNew = (partial, project) => async ({ Actees, Datasets, FormAttachmen
     0.5
   );
 
+  const webformsEnabled = keyId ? false : DEFAULT_WEBFORMS_ENABLED;
+
   // Save draft form
   const savedForm = await Forms._createNew(
     partial.with({ acteeId }),
-    partial.def.with({ keyId, draftToken, enketoId }),
+    partial.def.with({ keyId, draftToken, enketoId, webformsEnabled }),
     project
   );
 
@@ -273,7 +275,9 @@ const createVersion = (partial, form, publish, duplicating = false) => async ({
   // will just copy over whatever enketo ids do or don't exist already.
 
   // Save draft def
-  const savedDef = await Forms._createNewDef(partial, form, publish, { draftToken, enketoId, schemaId, keyId });
+  // In production, form.associated should never be null but in testing code we call Form.createVersion directly
+  const webformsEnabled = keyId ? false : (form.associated?.webformsEnabled ?? DEFAULT_WEBFORMS_ENABLED);
+  const savedDef = await Forms._createNewDef(partial, form, publish, { draftToken, enketoId, schemaId, keyId, webformsEnabled });
 
   // Prepare the form fields
   if (!match)
@@ -655,6 +659,10 @@ const _getSql = ((fields, extend, options, defVersion, deleted = false, actorId)
       defVersion === Form.WithoutDef ? sql`${fields} FROM forms` :
       sql`${fields} FROM forms LEFT OUTER JOIN form_defs on ${versionJoinCondition(defVersion)}`
     }
+    JOIN (
+      SELECT forms.id, form_defs."webformsEnabled" FROM forms
+      JOIN form_defs ON form_defs.id=coalesce(forms."currentDefId", forms."draftDefId")
+    ) AS "formAssociated" ON "formAssociated".id = forms.id
 ${extend|| sql`
   left outer join
     (select "formId", count(id)::integer as "submissions", max("createdAt") as "lastSubmission",
@@ -707,7 +715,7 @@ const _get = (exec, options, defVersion, xml, deleted, actorId) => {
   if (xml !== Form.IncludeXml && xml !== Form.WithoutXml) throw new Error('Must use Form.WithoutXml or Form.IncludeXml; actually got: ' + xml.toString());
   if (defVersion === Form.WithoutDef && xml === Form.IncludeXml) throw new Error('Cannot request xml without a form def');
 
-  const frames = [ Form ];
+  const frames = [ Form, Form.Associated ];
   if (defVersion !== Form.WithoutDef) frames.push(Form.Def);
   if (xml === Form.IncludeXml) frames.push(Form.Xml);
   return extender(...frames)(Form.Extended, Actor.into('createdBy'))(_getSql)(exec, options, defVersion, deleted, actorId);
diff --git a/lib/resources/forms.js b/lib/resources/forms.js
@@ -246,6 +246,19 @@ module.exports = (service, endpoint, anonymousEndpoint) => {
           // already that there is no blob hash.
           FormAttachments.update(form, attachment, null, body.dataset ? dataset.id : null))))));
 
+  service.patch('/projects/:projectId/forms/:xmlFormId/draft', endpoint(async ({ Forms }, { auth, params, body }) => {
+    const form = await Forms.getByProjectAndXmlFormId(params.projectId, params.xmlFormId, Form.DraftVersion)
+      .then(getOrNotFound);
+
+    await auth.canOrReject('form.update', form);
+
+    const defFromRequest = Form.Def.fromApi(body);
+
+    await Forms._updateDef(form.def, defFromRequest);
+
+    const webformsEnabled = defFromRequest.webformsEnabled ?? form.def.webformsEnabled;
+    return { ...form.forApi(), webformsEnabled };
+  }));
 
   service.delete('/projects/:projectId/forms/:xmlFormId/draft', endpoint(({ Audits, Forms, Submissions }, { params, auth }) =>
     Forms.getByProjectAndXmlFormId(params.projectId, params.xmlFormId, Form.WithoutDef)
@@ -367,10 +380,13 @@ module.exports = (service, endpoint, anonymousEndpoint) => {
   // PRIMARY-FORM SPECIFIC ENDPOINTS
 
   service.patch('/projects/:projectId/forms/:xmlFormId', endpoint(({ Forms }, { auth, params, body }) =>
-    Forms.getByProjectAndXmlFormId(params.projectId, params.xmlFormId, Form.WithoutDef)
+    Forms.getByProjectAndXmlFormId(params.projectId, params.xmlFormId, Form.AnyVersion)
       .then(getOrNotFound)
       .then((form) => auth.canOrReject('form.update', form))
-      .then((form) => Forms.update(form, Form.fromApi(body)))
+      .then((form) => Promise.all([
+        Forms.update(form, Form.fromApi(body)),
+        Forms._updateDef(form.def, Form.Def.fromApi(body))
+      ]))
       // TODO: sucks to have to re-request but this shouldn't be a perf-critical path.
       .then(() => Forms.getByProjectAndXmlFormId(params.projectId, params.xmlFormId, Form.AnyVersion))
       .then(getOrNotFound)));
diff --git a/lib/util/constants.js b/lib/util/constants.js
@@ -15,7 +15,7 @@ const PURGE_DAY_RANGE = config.has('default.taskSchedule.purge')
 
 const DEFAULT_WEBFORMS_ENABLED = config.has('default.webformsEnabled')
   ? config.get('default.webformsEnabled')
-  : false;
+  : true;
 
 module.exports = {
   PURGE_DAY_RANGE,
diff --git a/test/integration/api/forms/forms.js b/test/integration/api/forms/forms.js
@@ -1772,6 +1772,59 @@ describe('api: /projects/:id/forms (create, read, update)', () => {
               body.webformsEnabled.should.equal(true);
             })))));
 
+    it('should update allowed fields', testService(async (service) => {
+      const asAlice = await service.login('alice');
+
+      await asAlice.post('/v1/projects/1/forms/simple/draft').expect(200);
+
+      // default is true now.
+      await asAlice.get('/v1/projects/1/forms/simple/draft').expect(200)
+        .then(({ body }) => {
+          body.webformsEnabled.should.be.true();
+        });
+
+      await asAlice.patch('/v1/projects/1/forms/simple/draft')
+        .send({ webformsEnabled: false })
+        .expect(200)
+        .then(({ body }) => {
+          body.webformsEnabled.should.be.false();
+        });
+
+      await asAlice.get('/v1/projects/1/forms/simple/draft').expect(200)
+        .then(({ body }) => {
+          body.webformsEnabled.should.be.false();
+        });
+    }));
+
+    it('should inherit webformsEnabled when creating new draft from published', testService(async (service) => {
+      const asAlice = await service.login('alice');
+
+      // First create a draft and enable webforms
+      await asAlice.post('/v1/projects/1/forms/simple/draft')
+        .expect(200);
+      await asAlice.patch('/v1/projects/1/forms/simple')
+        .send({ webformsEnabled: true })
+        .expect(200);
+      await asAlice.post('/v1/projects/1/forms/simple/draft/publish?version=v2')
+        .expect(200);
+
+      // Verify published version has webformsEnabled: true
+      await asAlice.get('/v1/projects/1/forms/simple')
+        .expect(200)
+        .then(({ body }) => {
+          body.webformsEnabled.should.equal(true);
+        });
+
+      // Create a new draft - should inherit webformsEnabled from published
+      await asAlice.post('/v1/projects/1/forms/simple/draft')
+        .expect(200);
+      await asAlice.get('/v1/projects/1/forms/simple/draft')
+        .expect(200)
+        .then(({ body }) => {
+          body.webformsEnabled.should.equal(true);
+        });
+    }));
+
     it('should reject if state is invalid', testService((service) =>
       service.login('alice', (asAlice) =>
         asAlice.patch('/v1/projects/1/forms/simple')
