Merge pull request #4068 from overleaf/ab-split-test-user-properties

Store assigned split tests as user properties GitOrigin-RevId: 1cc09d4d8f19badb73e87c46064bdeac131dd307
2024-09-16 02:52:31 -04:00 · 2021-05-26 14:37:15 +02:00 · 2021-05-26 14:37:15 +02:00 · a65c5dde01
commit a65c5dde01
parent 551e2bfb5c
6 changed files with 275 additions and 196 deletions
--- a/services/web/app/src/Features/Project/ProjectController.js
+++ b/services/web/app/src/Features/Project/ProjectController.js
@ -37,7 +37,7 @@ const BrandVariationsHandler = require('../BrandVariations/BrandVariationsHandle
 const UserController = require('../User/UserController')
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
 const Modules = require('../../infrastructure/Modules')
-const { getTestSegmentation } = require('../SplitTests/SplitTestHandler')
+const SplitTestHandler = require('../SplitTests/SplitTestHandler')
 const { getNewLogsUIVariantForUser } = require('../Helpers/NewLogsUI')

 const _ssoAvailable = (affiliation, session, linkedInstitutionIds) => {
@ -802,103 +802,137 @@ const ProjectController = {
              }
            }

-            const trackPdfDownload =
-              Settings.enablePdfCaching &&
-              (user.alphaProgram ||
-                (user.betaProgram &&
-                  getTestSegmentation(userId, 'track_pdf_download').variant ===
-                    'enabled'))
-            const enablePdfCaching =
-              Settings.enablePdfCaching &&
-              shouldDisplayFeature(
-                'enable_pdf_caching',
-                user.alphaProgram ||
-                  (user.betaProgram &&
-                    getTestSegmentation(userId, 'enable_pdf_caching')
-                      .variant === 'enabled')
-              )
+            let trackPdfDownload = false
+            let enablePdfCaching = false

-            res.render('project/editor', {
-              title: project.name,
-              priority_title: true,
-              bodyClasses: ['editor'],
-              project_id: project._id,
-              user: {
-                id: userId,
-                email: user.email,
-                first_name: user.first_name,
-                last_name: user.last_name,
-                referal_id: user.referal_id,
-                signUpDate: user.signUpDate,
-                allowedFreeTrial: allowedFreeTrial,
-                featureSwitches: user.featureSwitches,
-                features: user.features,
-                refProviders: _.mapValues(user.refProviders, Boolean),
-                alphaProgram: user.alphaProgram,
-                betaProgram: user.betaProgram,
-                isAdmin: user.isAdmin,
-              },
-              userSettings: {
-                mode: user.ace.mode,
-                editorTheme: user.ace.theme,
-                fontSize: user.ace.fontSize,
-                autoComplete: user.ace.autoComplete,
-                autoPairDelimiters: user.ace.autoPairDelimiters,
-                pdfViewer: user.ace.pdfViewer,
-                syntaxValidation: user.ace.syntaxValidation,
-                fontFamily: user.ace.fontFamily || 'lucida',
-                lineHeight: user.ace.lineHeight || 'normal',
-                overallTheme: user.ace.overallTheme,
-              },
-              privilegeLevel,
-              chatUrl: Settings.apis.chat.url,
-              anonymous,
-              anonymousAccessToken: anonymous ? anonRequestToken : null,
-              isTokenMember,
-              isRestrictedTokenMember: AuthorizationManager.isRestrictedUser(
-                userId,
+            const render = () => {
+              res.render('project/editor', {
+                title: project.name,
+                priority_title: true,
+                bodyClasses: ['editor'],
+                project_id: project._id,
+                user: {
+                  id: userId,
+                  email: user.email,
+                  first_name: user.first_name,
+                  last_name: user.last_name,
+                  referal_id: user.referal_id,
+                  signUpDate: user.signUpDate,
+                  allowedFreeTrial: allowedFreeTrial,
+                  featureSwitches: user.featureSwitches,
+                  features: user.features,
+                  refProviders: _.mapValues(user.refProviders, Boolean),
+                  alphaProgram: user.alphaProgram,
+                  betaProgram: user.betaProgram,
+                  isAdmin: user.isAdmin,
+                },
+                userSettings: {
+                  mode: user.ace.mode,
+                  editorTheme: user.ace.theme,
+                  fontSize: user.ace.fontSize,
+                  autoComplete: user.ace.autoComplete,
+                  autoPairDelimiters: user.ace.autoPairDelimiters,
+                  pdfViewer: user.ace.pdfViewer,
+                  syntaxValidation: user.ace.syntaxValidation,
+                  fontFamily: user.ace.fontFamily || 'lucida',
+                  lineHeight: user.ace.lineHeight || 'normal',
+                  overallTheme: user.ace.overallTheme,
+                },
                privilegeLevel,
-                isTokenMember
-              ),
-              languages: Settings.languages,
-              editorThemes: THEME_LIST,
-              maxDocLength: Settings.max_doc_length,
-              useV2History:
-                project.overleaf &&
-                project.overleaf.history &&
-                Boolean(project.overleaf.history.display),
-              brandVariation,
-              allowedImageNames,
-              gitBridgePublicBaseUrl: Settings.gitBridgePublicBaseUrl,
-              wsUrl,
-              showSupport: Features.hasFeature('support'),
-              showNewLogsUI: shouldDisplayFeature(
-                'new_logs_ui',
-                logsUIVariant.newLogsUI
-              ),
-              logsUISubvariant: logsUIVariant.subvariant,
-              showNewNavigationUI: shouldDisplayFeature(
-                'new_navigation_ui',
-                user.alphaProgram
-              ),
-              showReactShareModal: shouldDisplayFeature(
-                'new_share_modal_ui',
-                true
-              ),
-              showReactDropboxModal: shouldDisplayFeature(
-                'new_dropbox_modal_ui',
-                user.betaProgram
-              ),
-              showReactGithubSync: shouldDisplayFeature(
-                'new_github_sync_ui',
-                user.betaProgram || user.alphaProgram
-              ),
-              showNewBinaryFileUI: shouldDisplayFeature('new_binary_file'),
-              showSymbolPalette: shouldDisplayFeature('symbol_palette'),
-              trackPdfDownload,
-              enablePdfCaching,
-            })
-            timer.done()
+                chatUrl: Settings.apis.chat.url,
+                anonymous,
+                anonymousAccessToken: anonymous ? anonRequestToken : null,
+                isTokenMember,
+                isRestrictedTokenMember: AuthorizationManager.isRestrictedUser(
+                  userId,
+                  privilegeLevel,
+                  isTokenMember
+                ),
+                languages: Settings.languages,
+                editorThemes: THEME_LIST,
+                maxDocLength: Settings.max_doc_length,
+                useV2History:
+                  project.overleaf &&
+                  project.overleaf.history &&
+                  Boolean(project.overleaf.history.display),
+                brandVariation,
+                allowedImageNames,
+                gitBridgePublicBaseUrl: Settings.gitBridgePublicBaseUrl,
+                wsUrl,
+                showSupport: Features.hasFeature('support'),
+                showNewLogsUI: shouldDisplayFeature(
+                  'new_logs_ui',
+                  logsUIVariant.newLogsUI
+                ),
+                logsUISubvariant: logsUIVariant.subvariant,
+                showNewNavigationUI: shouldDisplayFeature(
+                  'new_navigation_ui',
+                  user.alphaProgram
+                ),
+                showReactShareModal: shouldDisplayFeature(
+                  'new_share_modal_ui',
+                  true
+                ),
+                showReactDropboxModal: shouldDisplayFeature(
+                  'new_dropbox_modal_ui',
+                  user.betaProgram
+                ),
+                showReactGithubSync: shouldDisplayFeature(
+                  'new_github_sync_ui',
+                  user.betaProgram || user.alphaProgram
+                ),
+                showNewBinaryFileUI: shouldDisplayFeature('new_binary_file'),
+                showSymbolPalette: shouldDisplayFeature('symbol_palette'),
+                trackPdfDownload,
+                enablePdfCaching,
+              })
+              timer.done()
+            }
+
+            Promise.all([
+              async () => {
+                if (Settings.enablePdfCaching) {
+                  if (user.alphaProgram) {
+                    trackPdfDownload = true
+                  } else if (user.betaProgram) {
+                    const testSegmentation = await SplitTestHandler.promises.getTestSegmentation(
+                      userId,
+                      'track_pdf_download'
+                    )
+                    trackPdfDownload =
+                      testSegmentation.enabled &&
+                      testSegmentation.variant === 'enabled'
+                  }
+                }
+              },
+              async () => {
+                if (Settings.enablePdfCaching) {
+                  if (user.alphaProgram) {
+                    enablePdfCaching = shouldDisplayFeature(
+                      'enable_pdf_caching',
+                      true
+                    )
+                  } else if (user.betaProgram) {
+                    const testSegmentation = await SplitTestHandler.promises.getTestSegmentation(
+                      userId,
+                      'enable_pdf_caching'
+                    )
+                    trackPdfDownload = shouldDisplayFeature(
+                      'enable_pdf_caching',
+                      testSegmentation.enabled &&
+                        testSegmentation.variant === 'enabled'
+                    )
+                  }
+                }
+              },
+            ])
+              .then(() => {
+                render()
+              })
+              .catch(error => {
+                logger.error({ err: error }, 'Failed to get test segmentation')
+                render()
+              })
          }
        )
      }
--- a/services/web/app/src/Features/Project/ProjectCreationHandler.js
+++ b/services/web/app/src/Features/Project/ProjectCreationHandler.js
@ -71,7 +71,7 @@ async function createBasicProject(ownerId, projectName) {
 async function createExampleProject(ownerId, projectName) {
  const project = await _createBlankProject(ownerId, projectName)

-  const testSegmentation = SplitTestHandler.getTestSegmentation(
+  const testSegmentation = await SplitTestHandler.promises.getTestSegmentation(
    ownerId,
    EXAMPLE_PROJECT_SPLITTEST_ID
  )
--- a/services/web/app/src/Features/SplitTests/SplitTestHandler.js
+++ b/services/web/app/src/Features/SplitTests/SplitTestHandler.js
@ -1,7 +1,23 @@
+const UserGetter = require('../User/UserGetter')
+const UserUpdater = require('../User/UserUpdater')
+const AnalyticsManager = require('../Analytics/AnalyticsManager')
 const Settings = require('settings-sharelatex')
 const _ = require('lodash')
 const crypto = require('crypto')
 const OError = require('@overleaf/o-error')
+const { callbackify } = require('util')
+
+const duplicateSplitTest = _.findKey(
+  _.groupBy(Settings.splitTests, 'id'),
+  group => {
+    return group.length > 1
+  }
+)
+if (duplicateSplitTest) {
+  throw new OError(
+    `Split test IDs must be unique: ${duplicateSplitTest} is defined at least twice`
+  )
+}

 const ACTIVE_SPLIT_TESTS = []
 for (const splitTest of Settings.splitTests) {
@ -31,23 +47,24 @@ for (const splitTest of Settings.splitTests) {
  }
 }

-function getTestSegmentation(userId, splitTestId) {
+async function getTestSegmentation(userId, splitTestId) {
  const splitTest = _.find(ACTIVE_SPLIT_TESTS, ['id', splitTestId])
  if (splitTest) {
-    let userIdAsPercentile = _getPercentile(userId, splitTestId)
-    for (const variant of splitTest.variants) {
-      if (userIdAsPercentile < variant.rolloutPercent) {
-        return {
-          enabled: true,
-          variant: variant.id,
-        }
-      } else {
-        userIdAsPercentile -= variant.rolloutPercent
+    const alreadyAssignedVariant = await getAlreadyAssignedVariant(
+      userId,
+      splitTestId
+    )
+    if (alreadyAssignedVariant) {
+      return {
+        enabled: true,
+        variant: alreadyAssignedVariant,
+      }
+    } else {
+      const variant = await assignUserToVariant(userId, splitTest)
+      return {
+        enabled: true,
+        variant,
      }
-    }
-    return {
-      enabled: true,
-      variant: 'default',
    }
  }
  return {
@ -55,6 +72,38 @@ function getTestSegmentation(userId, splitTestId) {
  }
 }

+async function getAlreadyAssignedVariant(userId, splitTestId) {
+  const user = await UserGetter.promises.getUser(userId, { splitTests: 1 })
+  if (user && user.splitTests) {
+    return user.splitTests[splitTestId]
+  }
+  return undefined
+}
+
+async function assignUserToVariant(userId, splitTest) {
+  let userIdAsPercentile = await _getPercentile(userId, splitTest.id)
+  let selectedVariant = 'default'
+  for (const variant of splitTest.variants) {
+    if (userIdAsPercentile < variant.rolloutPercent) {
+      selectedVariant = variant.id
+      break
+    } else {
+      userIdAsPercentile -= variant.rolloutPercent
+    }
+  }
+  await UserUpdater.promises.updateUser(userId, {
+    $set: {
+      [`splitTests.${splitTest.id}`]: selectedVariant,
+    },
+  })
+  AnalyticsManager.setUserProperty(
+    userId,
+    `split-test-${splitTest.id}`,
+    selectedVariant
+  )
+  return selectedVariant
+}
+
 function _getPercentile(userId, splitTestId) {
  const hash = crypto
    .createHash('md5')
@ -65,5 +114,8 @@ function _getPercentile(userId, splitTestId) {
 }

 module.exports = {
-  getTestSegmentation,
+  getTestSegmentation: callbackify(getTestSegmentation),
+  promises: {
+    getTestSegmentation,
+  },
 }
--- a/services/web/app/src/Features/Subscription/SubscriptionController.js
+++ b/services/web/app/src/Features/Subscription/SubscriptionController.js
@ -91,83 +91,81 @@ async function paymentPage(req, res) {
  }
 }

-function userSubscriptionPage(req, res, next) {
+async function userSubscriptionPage(req, res) {
  const user = AuthenticationController.getSessionUser(req)
-  SubscriptionViewModelBuilder.buildUsersSubscriptionViewModel(
-    user,
-    function (error, results) {
-      if (error) {
-        return next(error)
-      }
-      const {
-        personalSubscription,
-        memberGroupSubscriptions,
-        managedGroupSubscriptions,
-        confirmedMemberAffiliations,
-        managedInstitutions,
-        managedPublishers,
-        v1SubscriptionStatus,
-      } = results
-      LimitationsManager.userHasV1OrV2Subscription(
-        user,
-        function (err, hasSubscription) {
-          if (error) {
-            return next(error)
-          }
-          const fromPlansPage = req.query.hasSubscription
-          const plans = SubscriptionViewModelBuilder.buildPlansList(
-            personalSubscription ? personalSubscription.plan : undefined
-          )
-
-          let subscriptionCopy = 'default'
-          if (
-            personalSubscription ||
-            hasSubscription ||
-            (memberGroupSubscriptions && memberGroupSubscriptions.length > 0) ||
-            (confirmedMemberAffiliations &&
-              confirmedMemberAffiliations.length > 0 &&
-              _.find(confirmedMemberAffiliations, affiliation => {
-                return affiliation.licence && affiliation.licence !== 'free'
-              }))
-          ) {
-            AnalyticsManager.recordEvent(user._id, 'subscription-page-view')
-          } else {
-            const testSegmentation = SplitTestHandler.getTestSegmentation(
-              user._id,
-              SUBSCRIPTION_PAGE_SPLIT_TEST
-            )
-            if (testSegmentation.enabled) {
-              subscriptionCopy = testSegmentation.variant
-
-              AnalyticsManager.recordEvent(user._id, 'subscription-page-view', {
-                splitTestId: SUBSCRIPTION_PAGE_SPLIT_TEST,
-                splitTestVariantId: testSegmentation.variant,
-              })
-            } else {
-              AnalyticsManager.recordEvent(user._id, 'subscription-page-view')
-            }
-          }
-
-          const data = {
-            title: 'your_subscription',
-            plans,
-            user,
-            hasSubscription,
-            subscriptionCopy,
-            fromPlansPage,
-            personalSubscription,
-            memberGroupSubscriptions,
-            managedGroupSubscriptions,
-            confirmedMemberAffiliations,
-            managedInstitutions,
-            managedPublishers,
-            v1SubscriptionStatus,
-          }
-          res.render('subscriptions/dashboard', data)
-        }
-      )
-    }
+  const results = await SubscriptionViewModelBuilder.promises.buildUsersSubscriptionViewModel(
+    user
  )
+  const {
+    personalSubscription,
+    memberGroupSubscriptions,
+    managedGroupSubscriptions,
+    confirmedMemberAffiliations,
+    managedInstitutions,
+    managedPublishers,
+    v1SubscriptionStatus,
+  } = results
+  const hasSubscription = await LimitationsManager.promises.userHasV1OrV2Subscription(
+    user
+  )
+  const fromPlansPage = req.query.hasSubscription
+  const plans = SubscriptionViewModelBuilder.buildPlansList(
+    personalSubscription ? personalSubscription.plan : undefined
+  )
+
+  let subscriptionCopy = 'default'
+  if (
+    personalSubscription ||
+    hasSubscription ||
+    (memberGroupSubscriptions && memberGroupSubscriptions.length > 0) ||
+    (confirmedMemberAffiliations &&
+      confirmedMemberAffiliations.length > 0 &&
+      _.find(confirmedMemberAffiliations, affiliation => {
+        return affiliation.licence && affiliation.licence !== 'free'
+      }))
+  ) {
+    AnalyticsManager.recordEvent(user._id, 'subscription-page-view')
+  } else {
+    try {
+      const testSegmentation = await SplitTestHandler.promises.getTestSegmentation(
+        user._id,
+        SUBSCRIPTION_PAGE_SPLIT_TEST
+      )
+      if (testSegmentation.enabled) {
+        subscriptionCopy = testSegmentation.variant
+
+        AnalyticsManager.recordEvent(user._id, 'subscription-page-view', {
+          splitTestId: SUBSCRIPTION_PAGE_SPLIT_TEST,
+          splitTestVariantId: testSegmentation.variant,
+        })
+      } else {
+        AnalyticsManager.recordEvent(user._id, 'subscription-page-view')
+      }
+    } catch (error) {
+      logger.error(
+        { err: error },
+        `Failed to get segmentation for user '${user._id}' and split test '${SUBSCRIPTION_PAGE_SPLIT_TEST}'`
+      )
+      AnalyticsManager.recordEvent(user._id, 'subscription-page-view')
+    }
+  }
+
+  const data = {
+    title: 'your_subscription',
+    plans,
+    user,
+    hasSubscription,
+    subscriptionCopy,
+    fromPlansPage,
+    personalSubscription,
+    memberGroupSubscriptions,
+    managedGroupSubscriptions,
+    confirmedMemberAffiliations,
+    managedInstitutions,
+    managedPublishers,
+    v1SubscriptionStatus,
+  }
+  res.render('subscriptions/dashboard', data)
 }

 function createSubscription(req, res, next) {
@ -481,7 +479,7 @@ async function refreshUserFeatures(req, res) {
 module.exports = {
  plansPage: expressify(plansPage),
  paymentPage: expressify(paymentPage),
-  userSubscriptionPage,
+  userSubscriptionPage: expressify(userSubscriptionPage),
  createSubscription,
  successfulSubscription,
  cancelSubscription,
--- a/services/web/app/src/models/User.js
+++ b/services/web/app/src/models/User.js
@ -165,6 +165,7 @@ const UserSchema = new Schema({
  },
  onboardingEmailSentAt: { type: Date },
  auditLog: [AuditLogEntrySchema],
+  splitTests: Schema.Types.Mixed,
 })

 exports.User = mongoose.model('User', UserSchema)
--- a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js
+++ b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js
@ -300,9 +300,7 @@ describe('SubscriptionController', function () {

  describe('userSubscriptionPage', function () {
    beforeEach(function (done) {
-      this.SubscriptionViewModelBuilder.buildUsersSubscriptionViewModel.callsArgWith(
-        1,
-        null,
+      this.SubscriptionViewModelBuilder.promises.buildUsersSubscriptionViewModel.resolves(
        {
          personalSubscription: (this.personalSubscription = {
            'personal-subscription': 'mock',
@ -315,11 +313,7 @@ describe('SubscriptionController', function () {
      this.SubscriptionViewModelBuilder.buildPlansList.returns(
        (this.plans = { plans: 'mock' })
      )
-      this.LimitationsManager.userHasV1OrV2Subscription.callsArgWith(
-        1,
-        null,
-        false
-      )
+      this.LimitationsManager.promises.userHasV1OrV2Subscription.resolves(false)
      this.res.render = (view, data) => {
        this.data = data
        expect(view).to.equal('subscriptions/dashboard')