Merge pull request #36320 from overleaf/rh-cio-split-tests

[web] require admin opt-in for split test assignments sent to customer.io

GitOrigin-RevId: 5a6f5c3726f3b65c2f432ab20720e6f8f5c53eb1

services/web/app/src/Features/Project/ProjectListController.mjs

@@ -578,6 +578,21 @@ async function projectListPage(req, res, next) {
     }
   }
 
+  let splitTestUserProperties
+  if (isSaas) {
+    try {
+      ;[splitTestUserProperties] = await Modules.promises.hooks.fire(
+        'getSplitTestUserProperties',
+        userId
+      )
+    } catch (err) {
+      logger.error(
+        { err, userId },
+        'Failed to build split test user properties for customer.io'
+      )
+    }
+  }
+
   Modules.promises.hooks
     .fire('setUserProperties', userId, {
       overleaf_id: userId,
@@ -602,6 +617,7 @@ async function projectListPage(req, res, next) {
       ...(groupRole && { group_role: groupRole }),
       is_managed_user: Boolean(user.enrollment?.managedBy),
       ...(user.email && { email: user.email }),
+      ...splitTestUserProperties,
     })
     .catch(err => {
       logger.error({ err }, 'Failed to set user properties for customer.io')

services/web/app/src/Features/SplitTests/SplitTestManager.mjs

@@ -5,6 +5,11 @@ import OError from '@overleaf/o-error'
 import _ from 'lodash'
 import { CacheFlow } from 'cache-flow'
 
+// customer.io silently drops attribute values over 1000 bytes, so cap the
+// number of split tests whose assignments are sent in the
+// `split_test_assignments` attribute
+const MAX_CUSTOMER_IO_SPLIT_TESTS = 25
+
 const ALPHA_PHASE = 'alpha'
 const LABS_PHASE = 'labs'
 const BETA_PHASE = 'beta'
@@ -158,11 +163,15 @@ async function updateSplitTestInfo(name, info, labsInfo) {
       `Cannot update split test '${name}': not found`
     )
   }
+  if (info.customerIoEnabled && !splitTest.customerIoEnabled) {
+    await _checkCustomerIoSplitTestLimit(splitTest._id)
+  }
   splitTest.description = info.description
   splitTest.expectedEndDate = info.expectedEndDate
   splitTest.ticketUrl = info.ticketUrl
   splitTest.reportsUrls = info.reportsUrls
   splitTest.winningVariant = info.winningVariant
+  splitTest.customerIoEnabled = Boolean(info.customerIoEnabled)
   if (labsInfo) {
     splitTest.labsTitle = labsInfo.title
     splitTest.labsDescription = labsInfo.description
@@ -538,6 +547,19 @@ function _updateVariantsWithNewConfiguration(
   return variantsCopy
 }
 
+async function _checkCustomerIoSplitTestLimit(splitTestId) {
+  const count = await SplitTest.countDocuments({
+    _id: { $ne: splitTestId },
+    customerIoEnabled: true,
+    archived: { $ne: true },
+  })
+  if (count >= MAX_CUSTOMER_IO_SPLIT_TESTS) {
+    throw new Errors.InvalidError(
+      `Cannot send more than ${MAX_CUSTOMER_IO_SPLIT_TESTS} split tests to customer.io: disable another split test first`
+    )
+  }
+}
+
 function _getTotalRolloutPercentage(variants) {
   return _.sumBy(variants, 'rolloutPercent')
 }

services/web/app/src/Features/SplitTests/SplitTestUserGetter.mjs

@@ -45,7 +45,8 @@ function getProjection(splitTestName) {
 
 /**
  * @param id
- * @param {string} splitTestName
+ * @param {string} [splitTestName] restrict the `splitTests` sub-document to a
+ *   single test; omit to fetch all assignments.
  * @param {string} path
  * @return {Promise<SplitTestUser>}
  */

services/web/app/src/models/SplitTest.mjs

@@ -180,6 +180,10 @@ export const SplitTestSchema = new Schema(
       type: String,
       required: false,
     },
+    customerIoEnabled: {
+      type: Boolean,
+      required: false,
+    },
     archived: {
       type: Boolean,
       required: false,

services/web/test/unit/src/Project/ProjectListController.test.mjs

@@ -568,6 +568,53 @@ describe('ProjectListController', function () {
       )
     })
 
+    it('should send packed split test assignments to customer.io', async function (ctx) {
+      ctx.Features.hasFeature.withArgs('saas').returns(true)
+      ctx.Modules.promises.hooks.fire
+        .withArgs('getSplitTestUserProperties', ctx.user._id)
+        .resolves([{ split_test_assignments: { 'test-a': 'variant-1' } }])
+      ctx.res.render = () => {}
+
+      await ctx.ProjectListController.projectListPage(ctx.req, ctx.res)
+
+      expect(ctx.Modules.promises.hooks.fire).to.have.been.calledWith(
+        'setUserProperties',
+        ctx.user._id,
+        sinon.match({
+          split_test_assignments: { 'test-a': 'variant-1' },
+        })
+      )
+    })
+
+    it('should not request split test user properties in a non-saas environment', async function (ctx) {
+      ctx.Features.hasFeature.withArgs('saas').returns(false)
+      ctx.res.render = () => {}
+
+      await ctx.ProjectListController.projectListPage(ctx.req, ctx.res)
+
+      expect(ctx.Modules.promises.hooks.fire).to.not.have.been.calledWith(
+        'getSplitTestUserProperties',
+        sinon.match.any
+      )
+    })
+
+    it('should still send other user properties when building split test assignments fails', async function (ctx) {
+      ctx.Features.hasFeature.withArgs('saas').returns(true)
+      ctx.Modules.promises.hooks.fire
+        .withArgs('getSplitTestUserProperties', ctx.user._id)
+        .rejects(new Error('boom'))
+      ctx.res.render = () => {}
+
+      await ctx.ProjectListController.projectListPage(ctx.req, ctx.res)
+
+      const call = ctx.Modules.promises.hooks.fire
+        .getCalls()
+        .find(call => call.args[0] === 'setUserProperties')
+      expect(call).to.exist
+      expect(call.args[2]).to.not.have.property('split_test_assignments')
+      expect(call.args[2]).to.have.property('overleaf_id', ctx.user._id)
+    })
+
     it('should send enterprise_commons=true when user has commons from an enterprise_commons institution', async function (ctx) {
       ctx.Features.hasFeature.withArgs('saas').returns(true)
       ctx.UserGetter.promises.getUserFullEmails.resolves([