Merge pull request #28013 from overleaf/rh-editor-hotjar

Support user limits on split test variants, add Hotjar editor support

GitOrigin-RevId: c5df831436c2b7d7e242cf4d3ff00899aba77886

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

@@ -395,6 +395,7 @@ const _ProjectController = {
       'track-pdf-download',
       !anonymous && 'writefull-oauth-promotion',
       'hotjar',
+      'hotjar-editor-onboarding',
       'editor-redesign',
       'overleaf-assist-bundle',
       'word-count-client',
@@ -453,17 +454,10 @@ const _ProjectController = {
             ),
         })
       )
-    const splitTestAssignments = {}
 
     try {
       const responses = await pProps({
         userValues: userId ? getUserValues(userId) : defaultUserValues(),
-        splitTestAssignments: Promise.all(
-          splitTests.map(async splitTest => {
-            splitTestAssignments[splitTest] =
-              await SplitTestHandler.promises.getAssignment(req, res, splitTest)
-          })
-        ),
         project: ProjectGetter.promises.getProject(projectId, {
           name: 1,
           lastUpdated: 1,
@@ -503,8 +497,57 @@ const _ProjectController = {
         subscription,
         isTokenMember,
         isInvitedMember,
+        affiliations,
       } = userValues
 
+      let inEnterpriseCommons = false
+      for (const affiliation of affiliations || []) {
+        inEnterpriseCommons =
+          inEnterpriseCommons || affiliation.institution?.enterpriseCommons
+      }
+
+      const getSplitTestAssignment = async splitTest => {
+        if (splitTest === 'hotjar-editor-onboarding') {
+          const sevenDaysAgo = new Date(Date.now() - 7 * 24 * 60 * 60 * 1000)
+          const userRegisteredMoreThan7DaysAgo =
+            user.signUpDate && user.signUpDate < sevenDaysAgo
+
+          const isExcluded =
+            user.betaProgram ||
+            inEnterpriseCommons ||
+            userIsMemberOfGroupSubscription ||
+            userRegisteredMoreThan7DaysAgo
+
+          if (!isExcluded) {
+            return await SplitTestHandler.promises.getAssignment(
+              req,
+              res,
+              splitTest
+            )
+          } else {
+            return {
+              variant: 'default',
+              analytics: {
+                segmentation: {},
+              },
+            }
+          }
+        } else {
+          return await SplitTestHandler.promises.getAssignment(
+            req,
+            res,
+            splitTest
+          )
+        }
+      }
+      const splitTestAssignments = {}
+      await Promise.all(
+        splitTests.map(async splitTest => {
+          splitTestAssignments[splitTest] =
+            await getSplitTestAssignment(splitTest)
+        })
+      )
+
       const brandVariation = project?.brandVariationId
         ? await BrandVariationsHandler.promises.getBrandVariationById(
             project.brandVariationId
@@ -853,7 +896,9 @@ const _ProjectController = {
         otMigrationStage: project.overleaf?.history?.otMigrationStage ?? 0,
         projectTags,
         isSaas: Features.hasFeature('saas'),
-        shouldLoadHotjar: splitTestAssignments.hotjar?.variant === 'enabled',
+        shouldLoadHotjar:
+          splitTestAssignments['hotjar-editor-onboarding']?.variant ===
+          'enabled',
         isOverleafAssistBundleEnabled,
         customerIoEnabled,
         addonPrices,
@@ -1195,6 +1240,7 @@ const defaultUserValues = () => ({
   learnedWords: [],
   projectTags: [],
   userHasInstitutionLicence: false,
+  affiliations: [],
   subscription: undefined,
   isTokenMember: false,
   isInvitedMember: false,

services/web/app/src/Features/SplitTests/SplitTestHandler.js

@@ -15,6 +15,7 @@ const SessionManager = require('../Authentication/SessionManager')
 const logger = require('@overleaf/logger')
 const SplitTestSessionHandler = require('./SplitTestSessionHandler')
 const SplitTestUserGetter = require('./SplitTestUserGetter')
+const SplitTestManager = require('./SplitTestManager')
 
 /**
  * @import { Assignment } from "./types"
@@ -513,9 +514,37 @@ async function _getAssignmentMetadata(analyticsId, user, splitTest) {
 
   const userId = user?._id.toString()
   const percentile = getPercentile(analyticsId || userId, splitTest.name, phase)
-  const selectedVariantName =
+  let selectedVariantName =
     _getVariantFromPercentile(currentVersion.variants, percentile) ||
     DEFAULT_VARIANT
+
+  // Some variants may have a limit on the number of users that can be assigned
+  if (selectedVariantName !== DEFAULT_VARIANT) {
+    const selectedVariant = currentVersion.variants.find(
+      variant => variant.name === selectedVariantName
+    )
+    const userLimit = selectedVariant?.userLimit
+
+    if (userLimit && typeof userLimit === 'number') {
+      const userAssignments = user?.splitTests?.[splitTest.name]
+      const existingAssignment = Array.isArray(userAssignments)
+        ? userAssignments.find(
+            assignment =>
+              assignment.phase === phase &&
+              assignment.variantName === selectedVariantName
+          )
+        : null
+
+      if (!existingAssignment) {
+        const currentCount =
+          selectedVariant.userCount ?? Number.MAX_SAFE_INTEGER
+
+        if (currentCount >= userLimit) {
+          selectedVariantName = DEFAULT_VARIANT
+        }
+      }
+    }
+  }
   return {
     activeForUser: true,
     selectedVariantName,
@@ -587,15 +616,117 @@ async function _recordAssignment({
     const assignmentLog = assignedSplitTests[splitTestName] || []
     const existingAssignment = _.find(assignmentLog, { versionNumber })
     if (!existingAssignment) {
-      await UserUpdater.promises.updateUser(userId, {
-        $addToSet: {
-          [`splitTests.${splitTestName}`]: persistedAssignment,
-        },
-      })
+      const shouldIncrementCounter = await _shouldIncrementVariantCounter(
+        splitTestName,
+        variantName,
+        phase,
+        user
+      )
+
+      const updatePromises = [
+        UserUpdater.promises.updateUser(userId, {
+          $addToSet: {
+            [`splitTests.${splitTestName}`]: persistedAssignment,
+          },
+        }),
+      ]
+
+      if (shouldIncrementCounter) {
+        updatePromises.push(
+          _incrementVariantCounter(splitTestName, variantName, versionNumber)
+        )
+      }
+
+      await Promise.all(updatePromises)
     }
   }
 }
 
+/**
+ * Check if the variant counter should be incremented for this assignment
+ * Only increment for tests with user limits and when user hasn't been assigned to this variant in this phase before
+ * @param {string} splitTestName - The name of the split test
+ * @param {string} variantName - The name of the variant
+ * @param {string} phase - The phase of the split test
+ * @param {Object} user - The user object
+ * @returns {Promise<boolean>} Whether the counter should be incremented
+ */
+async function _shouldIncrementVariantCounter(
+  splitTestName,
+  variantName,
+  phase,
+  user
+) {
+  if (variantName === DEFAULT_VARIANT) {
+    return false
+  }
+
+  const splitTest = await _getSplitTest(splitTestName)
+  if (!splitTest) {
+    return false
+  }
+
+  const currentVersion = SplitTestUtils.getCurrentVersion(splitTest)
+  if (!currentVersion) {
+    return false
+  }
+
+  const variant = currentVersion.variants.find(v => v.name === variantName)
+  const hasUserLimit =
+    variant?.userLimit && typeof variant.userLimit === 'number'
+
+  if (!hasUserLimit) {
+    return false
+  }
+
+  const userAssignments = user?.splitTests?.[splitTest.name]
+  const existingPhaseAssignment = Array.isArray(userAssignments)
+    ? userAssignments.find(
+        assignment =>
+          assignment.phase === phase && assignment.variantName === variantName
+      )
+    : null
+
+  // Only increment if user hasn't been assigned to this variant in this phase before
+  return !existingPhaseAssignment
+}
+
+/**
+ * Increment the user counter for a specific variant
+ * @param {string} splitTestName - The name of the split test
+ * @param {string} variantName - The name of the variant
+ * @param {number} versionNumber - The version to update
+ */
+async function _incrementVariantCounter(
+  splitTestName,
+  variantName,
+  versionNumber
+) {
+  try {
+    await SplitTest.updateOne(
+      {
+        name: splitTestName,
+        'versions.versionNumber': versionNumber,
+        'versions.variants.name': variantName,
+      },
+      {
+        $inc: {
+          'versions.$.variants.$[variant].userCount': 1,
+        },
+      },
+      {
+        arrayFilters: [{ 'variant.name': variantName }],
+      }
+    ).exec()
+    await SplitTestManager.clearCache()
+  } catch (error) {
+    logger.error(
+      { err: error, splitTestName, variantName, versionNumber },
+      'Failed to increment variant counter'
+    )
+  }
+}
+
 function _makeAssignment({
   variant,
   currentVersion,

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

@@ -90,9 +90,10 @@ async function createSplitTest(
     configuration.analyticsEnabled
   )
   for (const variant of configuration.variants) {
-    stripedVariants.push({
+    const variantData = {
       name: (variant.name || '').trim(),
       rolloutPercent: variant.rolloutPercent,
+      userLimit: variant.userLimit,
       rolloutStripes:
         variant.rolloutPercent > 0
           ? [
@@ -102,7 +103,11 @@ async function createSplitTest(
               },
             ]
           : [],
-    })
+    }
+    if (variant.userLimit && typeof variant.userLimit === 'number') {
+      variantData.userCount = 0
+    }
+    stripedVariants.push(variantData)
     stripeStart += variant.rolloutPercent
   }
   const splitTest = new SplitTest({
@@ -258,6 +263,9 @@ async function switchToNextPhase({ name, comment }, userId) {
   for (const variant of lastVersionCopy.variants) {
     variant.rolloutPercent = 0
     variant.rolloutStripes = []
+    if (variant.userCount) {
+      variant.userCount = 0
+    }
   }
   lastVersionCopy.author = userId
   lastVersionCopy.comment = comment
@@ -307,6 +315,29 @@ async function revertToPreviousVersion(
   previousVersionCopy.createdAt = new Date()
   previousVersionCopy.author = userId
   previousVersionCopy.comment = comment
+
+  // restore user count from most recent version of this phase
+  const mostRecentVersionOfTargetPhase = splitTest.versions.findLast(
+    v => v.phase === previousVersion.phase
+  )
+
+  if (mostRecentVersionOfTargetPhase) {
+    for (const variant of previousVersionCopy.variants) {
+      const correspondingVariant = mostRecentVersionOfTargetPhase.variants.find(
+        v => v.name === variant.name
+      )
+      if (correspondingVariant?.userCount) {
+        variant.userCount = correspondingVariant.userCount
+      }
+    }
+  } else {
+    for (const variant of previousVersionCopy.variants) {
+      if (variant.userCount) {
+        variant.userCount = 0
+      }
+    }
+  }
+
   splitTest.versions.push(previousVersionCopy)
   return _saveSplitTest(splitTest)
 }
@@ -362,6 +393,24 @@ function _checkNewVariantsConfiguration(
         )} cannot be decreased: revert to a previous configuration instead`
       )
     }
+    if (variant.userLimit !== undefined) {
+      // Existing variant has a user limit - can only increase it
+      if (
+        newVariantConfiguration.userLimit !== undefined &&
+        newVariantConfiguration.userLimit < variant.userLimit
+      ) {
+        throw new OError(
+          `User limit for variant '${variant.name}' cannot be decreased: revert to a previous configuration instead`
+        )
+      }
+    } else {
+      // Existing variant has no user limit - cannot add one
+      if (newVariantConfiguration.userLimit !== undefined) {
+        throw new OError(
+          `User limit cannot be added to variant '${variant.name}' after creation: user limits can only be set when the split test is created`
+        )
+      }
+    }
   }
 }
 
@@ -377,7 +426,7 @@ function _updateVariantsWithNewConfiguration(
     }
     const variant = _.find(variantsCopy, { name: newVariantConfig.name })
     if (!variant) {
-      variantsCopy.push({
+      const newVariant = {
         name: newVariantConfig.name,
         rolloutPercent: newVariantConfig.rolloutPercent,
         rolloutStripes: [
@@ -386,7 +435,15 @@ function _updateVariantsWithNewConfiguration(
             end: totalRolloutPercentage + newVariantConfig.rolloutPercent,
           },
         ],
-      })
+      }
+      if (
+        newVariantConfig.userLimit &&
+        typeof newVariantConfig.userLimit === 'number'
+      ) {
+        newVariant.userLimit = newVariantConfig.userLimit
+        newVariant.userCount = 0
+      }
+      variantsCopy.push(newVariant)
       totalRolloutPercentage += newVariantConfig.rolloutPercent
     } else if (variant.rolloutPercent < newVariantConfig.rolloutPercent) {
       const newStripeSize =
@@ -398,6 +455,12 @@ function _updateVariantsWithNewConfiguration(
       })
       totalRolloutPercentage += newStripeSize
     }
+    if (newVariantConfig.userLimit >= variant?.userLimit) {
+      variant.userLimit = newVariantConfig.userLimit
+    }
+    if (variant?.userLimit && !variant.userCount) {
+      variant.userCount = 0
+    }
   }
   return variantsCopy
 }

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

@@ -60,6 +60,16 @@ const VariantSchema = new Schema(
         end: RolloutPercentType,
       },
     ],
+    userLimit: {
+      type: Number,
+      min: [0, 'User limit must be 0 or greater, got {VALUE}'],
+      required: false,
+    },
+    userCount: {
+      type: Number,
+      min: [0, 'User count must be 0 or greater, got {VALUE}'],
+      required: false,
+    },
   },
   { _id: false }
 )

services/web/frontend/js/infrastructure/hotjar.ts

@@ -2,17 +2,34 @@ import getMeta from '@/utils/meta'
 import { debugConsole } from '@/utils/debugging'
 import { initializeHotjar } from '@/infrastructure/hotjar-snippet'
 import { createTrackingLoader } from '@/infrastructure/tracking-loader'
+import grammarlyExtensionPresent from '@/shared/utils/grammarly'
 
 const { hotjarId, hotjarVersion } = getMeta('ol-ExposedSettings')
 const shouldLoadHotjar = getMeta('ol-shouldLoadHotjar')
 
-if (hotjarId && hotjarVersion && shouldLoadHotjar) {
+function attemptHotjarLoad() {
+  if (!hotjarId || !hotjarVersion) {
+    return
+  }
+
   if (!/^\d+$/.test(hotjarId) || !/^\d+$/.test(hotjarVersion)) {
     debugConsole.error('Invalid Hotjar id or version')
-  } else {
-    createTrackingLoader(
-      () => initializeHotjar(hotjarId, hotjarVersion),
-      'Hotjar'
-    )
+    return
   }
+
+  if (grammarlyExtensionPresent()) {
+    return
+  }
+
+  createTrackingLoader(
+    () => initializeHotjar(hotjarId, hotjarVersion),
+    'Hotjar'
+  )
+}
+
+if (hotjarId && hotjarVersion && shouldLoadHotjar) {
+  document.addEventListener('DOMContentLoaded', () => {
+    // add delay to allow extensions to inject shadow DOM
+    setTimeout(attemptHotjarLoad, 1000)
+  })
 }

services/web/test/unit/src/SplitTests/SplitTestHandlerTests.js

@@ -372,6 +372,101 @@ describe('SplitTestHandler', function () {
       )
     })
   })
+
+  describe('variant user limits', function () {
+    beforeEach(function () {
+      this.AnalyticsManager.getIdsFromSession.returns({
+        userId: 'abc123abc123',
+      })
+      this.SplitTestUserGetter.promises.getUser.resolves({
+        _id: new ObjectId('abc123abc123abc123abc123'),
+        splitTests: {},
+      })
+    })
+
+    it('should assign to variant when under limit', async function () {
+      this.cachedSplitTests.set(
+        'active-test',
+        makeSplitTest('active-test', { userLimit: 100, userCount: 50 })
+      )
+
+      const assignment = await this.SplitTestHandler.promises.getAssignment(
+        this.req,
+        this.res,
+        'active-test'
+      )
+
+      expect(assignment.variant).to.equal('variant-1')
+    })
+
+    it('should assign to default when limit reached', async function () {
+      this.cachedSplitTests.set(
+        'active-test',
+        makeSplitTest('active-test', { userLimit: 100, userCount: 100 })
+      )
+
+      const assignment = await this.SplitTestHandler.promises.getAssignment(
+        this.req,
+        this.res,
+        'active-test'
+      )
+
+      expect(assignment.variant).to.equal('default')
+    })
+
+    it('should not apply limits when no limit configured', async function () {
+      const assignment = await this.SplitTestHandler.promises.getAssignment(
+        this.req,
+        this.res,
+        'active-test'
+      )
+
+      expect(assignment.variant).to.equal('variant-1')
+    })
+
+    it('should allow already assigned users even when limit reached', async function () {
+      this.cachedSplitTests.set(
+        'active-test',
+        makeSplitTest('active-test', { userLimit: 100, userCount: 100 })
+      )
+      this.SplitTestUserGetter.promises.getUser.resolves({
+        _id: new ObjectId('abc123abc123abc123abc123'),
+        splitTests: {
+          'active-test': [
+            {
+              variantName: 'variant-1',
+              versionNumber: 1,
+              assignedAt: new Date(),
+              phase: 'release',
+            },
+          ],
+        },
+      })
+
+      const assignment = await this.SplitTestHandler.promises.getAssignment(
+        this.req,
+        this.res,
+        'active-test'
+      )
+
+      expect(assignment.variant).to.equal('variant-1')
+    })
+
+    it('should assign to default if userCount is undefined', async function () {
+      this.cachedSplitTests.set(
+        'active-test',
+        makeSplitTest('active-test', { userLimit: 100, userCount: undefined })
+      )
+
+      const assignment = await this.SplitTestHandler.promises.getAssignment(
+        this.req,
+        this.res,
+        'active-test'
+      )
+
+      expect(assignment.variant).to.equal('default')
+    })
+  })
 })
 
 function makeSplitTest(
@@ -381,8 +476,24 @@ function makeSplitTest(
     analyticsEnabled = active,
     phase = 'release',
     versionNumber = 1,
+    userLimit = undefined,
+    userCount = undefined,
   } = {}
 ) {
+  const variant = {
+    name: 'variant-1',
+    rolloutPercent: 100,
+    rolloutStripes: [{ start: 0, end: 100 }],
+  }
+
+  if (userLimit !== undefined) {
+    variant.userLimit = userLimit
+  }
+
+  if (userCount !== undefined) {
+    variant.userCount = userCount
+  }
+
   return {
     name,
     versions: [
@@ -391,13 +502,7 @@ function makeSplitTest(
         analyticsEnabled,
         phase,
         versionNumber,
-        variants: [
-          {
-            name: 'variant-1',
-            rolloutPercent: 100,
-            rolloutStripes: [{ start: 0, end: 100 }],
-          },
-        ],
+        variants: [variant],
       },
     ],
   }