Просмотр исходного кода

Merge pull request #4639 from overleaf/ab-web-mono-analytics-id

Analytics ID support

GitOrigin-RevId: 820a6c0f4d19f046f6c791ce4dc64dbc80748924
Alexandre Bourdin 5 лет назад
Родитель
Сommit
6dd641e0e6
36 измененных файлов с 556 добавлено и 248 удалено
  1. 5 3
      services/web/app/src/Features/Analytics/AnalyticsController.js
  2. 142 33
      services/web/app/src/Features/Analytics/AnalyticsManager.js
  3. 4 4
      services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceHelper.js
  4. 26 0
      services/web/app/src/Features/Analytics/UserAnalyticsIdCache.js
  5. 11 4
      services/web/app/src/Features/Authentication/AuthenticationController.js
  6. 8 4
      services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js
  7. 1 1
      services/web/app/src/Features/Project/ProjectController.js
  8. 5 5
      services/web/app/src/Features/Project/ProjectCreationHandler.js
  9. 1 1
      services/web/app/src/Features/SplitTests/SplitTestHandler.js
  10. 124 29
      services/web/app/src/Features/SplitTests/SplitTestV2Handler.js
  11. 1 1
      services/web/app/src/Features/Subscription/FeaturesUpdater.js
  12. 80 32
      services/web/app/src/Features/Subscription/RecurlyEventHandler.js
  13. 1 1
      services/web/app/src/Features/Subscription/SubscriptionController.js
  14. 1 1
      services/web/app/src/Features/Subscription/SubscriptionUpdater.js
  15. 6 2
      services/web/app/src/Features/TokenAccess/TokenAccessHandler.js
  16. 10 2
      services/web/app/src/Features/User/UserCreator.js
  17. 1 1
      services/web/app/src/Features/User/UserPostRegistrationAnalyticsManager.js
  18. 1 2
      services/web/app/src/Features/User/UserRegistrationHandler.js
  19. 5 0
      services/web/app/src/infrastructure/Server.js
  20. 1 0
      services/web/app/src/models/User.js
  21. 1 1
      services/web/modules/server-ce-scripts/scripts/create-user.js
  22. 7 23
      services/web/test/unit/src/Analytics/AnalyticsControllerTests.js
  23. 15 6
      services/web/test/unit/src/Analytics/AnalyticsManagerTests.js
  24. 13 7
      services/web/test/unit/src/Authentication/AuthenticationControllerTests.js
  25. 1 1
      services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.js
  26. 1 1
      services/web/test/unit/src/Project/ProjectControllerTests.js
  27. 3 3
      services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js
  28. 26 26
      services/web/test/unit/src/Subscription/RecurlyEventHandlerTests.js
  29. 3 2
      services/web/test/unit/src/Subscription/SubscriptionControllerTests.js
  30. 1 1
      services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js
  31. 7 7
      services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js
  32. 3 4
      services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js
  33. 5 3
      services/web/test/unit/src/User/UserControllerTests.js
  34. 4 4
      services/web/test/unit/src/User/UserCreatorTests.js
  35. 8 4
      services/web/test/unit/src/User/UserPostRegistrationAnalyticsManagerTests.js
  36. 24 29
      services/web/test/unit/src/User/UserRegistrationHandlerTests.js

+ 5 - 3
services/web/app/src/Features/Analytics/AnalyticsController.js

@@ -30,10 +30,12 @@ module.exports = {
     if (!Features.hasFeature('analytics')) {
       return res.sendStatus(202)
     }
-    const userId =
-      SessionManager.getLoggedInUserId(req.session) || req.sessionID
     delete req.body._csrf
-    AnalyticsManager.recordEvent(userId, req.params.event, req.body)
+    AnalyticsManager.recordEventForSession(
+      req.session,
+      req.params.event,
+      req.body
+    )
     res.sendStatus(202)
   },
 }

+ 142 - 33
services/web/app/src/Features/Analytics/AnalyticsManager.js

@@ -1,21 +1,32 @@
+const SessionManager = require('../Authentication/SessionManager')
+const UserAnalyticsIdCache = require('./UserAnalyticsIdCache')
 const Settings = require('@overleaf/settings')
 const Metrics = require('../../infrastructure/Metrics')
 const Queues = require('../../infrastructure/Queues')
+const uuid = require('uuid')
+const _ = require('lodash')
+const { expressify } = require('../../util/promises')
 
 const analyticsEventsQueue = Queues.getAnalyticsEventsQueue()
 const analyticsEditingSessionsQueue = Queues.getAnalyticsEditingSessionsQueue()
 const analyticsUserPropertiesQueue = Queues.getAnalyticsUserPropertiesQueue()
 
-function identifyUser(userId, oldUserId) {
-  if (!userId || !oldUserId) {
+const ONE_MINUTE_MS = 60 * 1000
+
+function identifyUser(userId, analyticsId, isNewUser) {
+  if (!userId || !analyticsId) {
     return
   }
-  if (isAnalyticsDisabled() || isSmokeTestUser(userId)) {
+  if (_isAnalyticsDisabled() || _isSmokeTestUser(userId)) {
     return
   }
   Metrics.analyticsQueue.inc({ status: 'adding', event_type: 'identify' })
   analyticsEventsQueue
-    .add('identify', { userId, oldUserId })
+    .add(
+      'identify',
+      { userId, analyticsId, isNewUser },
+      { delay: ONE_MINUTE_MS }
+    )
     .then(() => {
       Metrics.analyticsQueue.inc({ status: 'added', event_type: 'identify' })
     })
@@ -24,29 +35,81 @@ function identifyUser(userId, oldUserId) {
     })
 }
 
-function recordEvent(userId, event, segmentation) {
+async function recordEventForUser(userId, event, segmentation) {
   if (!userId) {
     return
   }
-  if (isAnalyticsDisabled() || isSmokeTestUser(userId)) {
+  if (_isAnalyticsDisabled() || _isSmokeTestUser(userId)) {
     return
   }
-  Metrics.analyticsQueue.inc({ status: 'adding', event_type: 'event' })
-  analyticsEventsQueue
-    .add('event', { userId, event, segmentation })
-    .then(() => {
-      Metrics.analyticsQueue.inc({ status: 'added', event_type: 'event' })
-    })
-    .catch(() => {
-      Metrics.analyticsQueue.inc({ status: 'error', event_type: 'event' })
-    })
+  const analyticsId = await UserAnalyticsIdCache.get(userId)
+  if (analyticsId) {
+    _recordEvent({ analyticsId, userId, event, segmentation, isLoggedIn: true })
+  }
+}
+
+function recordEventForSession(session, event, segmentation) {
+  const { analyticsId, userId } = getIdsFromSession(session)
+  if (!analyticsId) {
+    return
+  }
+  if (_isAnalyticsDisabled() || _isSmokeTestUser(userId)) {
+    return
+  }
+  _recordEvent({
+    analyticsId,
+    userId,
+    event,
+    segmentation,
+    isLoggedIn: !!userId,
+  })
+}
+
+async function setUserPropertyForUser(userId, propertyName, propertyValue) {
+  if (_isAnalyticsDisabled() || _isSmokeTestUser(userId)) {
+    return
+  }
+
+  _checkPropertyValue(propertyValue)
+
+  const analyticsId = await UserAnalyticsIdCache.get(userId)
+  if (analyticsId) {
+    _setUserProperty({ analyticsId, propertyName, propertyValue })
+  }
+}
+
+async function setUserPropertyForAnalyticsId(
+  analyticsId,
+  propertyName,
+  propertyValue
+) {
+  if (_isAnalyticsDisabled()) {
+    return
+  }
+
+  _checkPropertyValue(propertyValue)
+
+  _setUserProperty({ analyticsId, propertyName, propertyValue })
+}
+
+async function setUserPropertyForSession(session, propertyName, propertyValue) {
+  const { analyticsId, userId } = getIdsFromSession(session)
+  if (_isAnalyticsDisabled() || _isSmokeTestUser(userId)) {
+    return
+  }
+
+  _checkPropertyValue(propertyValue)
+
+  if (analyticsId) {
+    _setUserProperty({ analyticsId, propertyName, propertyValue })
+  }
 }
 
 function updateEditingSession(userId, projectId, countryCode) {
   if (!userId) {
     return
   }
-  if (isAnalyticsDisabled() || isSmokeTestUser(userId)) {
+  if (_isAnalyticsDisabled() || _isSmokeTestUser(userId)) {
     return
   }
   Metrics.analyticsQueue.inc({
@@ -69,26 +132,36 @@ function updateEditingSession(userId, projectId, countryCode) {
     })
 }
 
-function setUserProperty(userId, propertyName, propertyValue) {
-  if (!userId) {
-    return
-  }
-  if (isAnalyticsDisabled() || isSmokeTestUser(userId)) {
-    return
-  }
-
-  if (propertyValue === undefined) {
-    throw new Error(
-      'propertyValue cannot be undefined, use null to unset a property'
+function _recordEvent(
+  { analyticsId, userId, event, segmentation, isLoggedIn },
+  { delay } = {}
+) {
+  Metrics.analyticsQueue.inc({ status: 'adding', event_type: 'event' })
+  analyticsEventsQueue
+    .add(
+      'event',
+      { analyticsId, userId, event, segmentation, isLoggedIn },
+      { delay }
     )
-  }
+    .then(() => {
+      Metrics.analyticsQueue.inc({ status: 'added', event_type: 'event' })
+    })
+    .catch(() => {
+      Metrics.analyticsQueue.inc({ status: 'error', event_type: 'event' })
+    })
+}
 
+function _setUserProperty({ analyticsId, propertyName, propertyValue }) {
   Metrics.analyticsQueue.inc({
     status: 'adding',
     event_type: 'user-property',
   })
   analyticsUserPropertiesQueue
-    .add({ userId, propertyName, propertyValue })
+    .add({
+      analyticsId,
+      propertyName,
+      propertyValue,
+    })
     .then(() => {
       Metrics.analyticsQueue.inc({
         status: 'added',
@@ -103,18 +176,54 @@ function setUserProperty(userId, propertyName, propertyValue) {
     })
 }
 
-function isSmokeTestUser(userId) {
+function _isSmokeTestUser(userId) {
   const smokeTestUserId = Settings.smokeTest && Settings.smokeTest.userId
   return smokeTestUserId != null && userId.toString() === smokeTestUserId
 }
 
-function isAnalyticsDisabled() {
+function _isAnalyticsDisabled() {
   return !(Settings.analytics && Settings.analytics.enabled)
 }
 
+function _checkPropertyValue(propertyValue) {
+  if (propertyValue === undefined) {
+    throw new Error(
+      'propertyValue cannot be undefined, use null to unset a property'
+    )
+  }
+}
+
+function getIdsFromSession(session) {
+  const analyticsId = _.get(session, ['analyticsId'])
+  const userId = SessionManager.getLoggedInUserId(session)
+  return { analyticsId, userId }
+}
+
+async function analyticsIdMiddleware(req, res, next) {
+  const session = req.session
+  const sessionUser = SessionManager.getSessionUser(session)
+  if (session.analyticsId) {
+    if (sessionUser && session.analyticsId !== sessionUser.analyticsId) {
+      session.analyticsId = sessionUser.analyticsId
+    }
+  } else {
+    if (sessionUser) {
+      session.analyticsId = sessionUser.analyticsId
+    } else {
+      session.analyticsId = uuid.v4()
+    }
+  }
+  next()
+}
+
 module.exports = {
   identifyUser,
-  recordEvent,
+  recordEventForSession,
+  recordEventForUser,
+  setUserPropertyForUser,
+  setUserPropertyForSession,
+  setUserPropertyForAnalyticsId,
   updateEditingSession,
-  setUserProperty,
+  getIdsFromSession,
+  analyticsIdMiddleware: expressify(analyticsIdMiddleware),
 }

+ 4 - 4
services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceHelper.js

@@ -79,7 +79,7 @@ function addUserProperties(userId, session) {
   }
 
   if (session.referal_id) {
-    AnalyticsManager.setUserProperty(
+    AnalyticsManager.setUserPropertyForUser(
       userId,
       `registered-from-bonus-scheme`,
       true
@@ -87,7 +87,7 @@ function addUserProperties(userId, session) {
   }
 
   if (session.required_login_for) {
-    AnalyticsManager.setUserProperty(
+    AnalyticsManager.setUserPropertyForUser(
       userId,
       `registered-from-${session.required_login_for}`,
       true
@@ -96,7 +96,7 @@ function addUserProperties(userId, session) {
 
   if (session.inbound) {
     if (session.inbound.referrer) {
-      AnalyticsManager.setUserProperty(
+      AnalyticsManager.setUserPropertyForUser(
         userId,
         `registered-from-referrer-${session.inbound.referrer.medium}`,
         session.inbound.referrer.detail || 'other'
@@ -106,7 +106,7 @@ function addUserProperties(userId, session) {
     if (session.inbound.utm) {
       for (const utmKey of UTM_KEYS) {
         if (session.inbound.utm[utmKey]) {
-          AnalyticsManager.setUserProperty(
+          AnalyticsManager.setUserPropertyForUser(
             userId,
             `registered-from-${utmKey.replace('_', '-')}`,
             session.inbound.utm[utmKey]

+ 26 - 0
services/web/app/src/Features/Analytics/UserAnalyticsIdCache.js

@@ -0,0 +1,26 @@
+const UserGetter = require('../User/UserGetter')
+const { CacheLoader } = require('cache-flow')
+
+class UserAnalyticsIdCache extends CacheLoader {
+  constructor() {
+    super('user-analytics-id', {
+      expirationTime: 60,
+      maxSize: 10000,
+    })
+  }
+
+  async load(userId) {
+    const user = await UserGetter.promises.getUser(userId, { analyticsId: 1 })
+    if (user) {
+      return user.analyticsId || user._id
+    }
+  }
+
+  keyToString(userId) {
+    if (userId) {
+      return userId.toString()
+    }
+  }
+}
+
+module.exports = new UserAnalyticsIdCache()

+ 11 - 4
services/web/app/src/Features/Authentication/AuthenticationController.js

@@ -48,6 +48,7 @@ const AuthenticationController = {
       ip_address: user._login_req_ip,
       must_reconfirm: user.must_reconfirm,
       v1_id: user.overleaf != null ? user.overleaf.id : undefined,
+      analyticsId: user.analyticsId || user._id,
     }
     callback(null, lightUser)
   },
@@ -82,6 +83,9 @@ const AuthenticationController = {
       return res.redirect('/login')
     } // OAuth2 'state' mismatch
 
+    const anonymousAnalyticsId = req.session.analyticsId
+    const isNewUser = req.session.justRegistered || false
+
     const Modules = require('../../infrastructure/Modules')
     Modules.hooks.fire(
       'preFinishLogin',
@@ -106,7 +110,7 @@ const AuthenticationController = {
 
         const redir =
           AuthenticationController._getRedirectFromSession(req) || '/project'
-        _loginAsyncHandlers(req, user)
+        _loginAsyncHandlers(req, user, anonymousAnalyticsId, isNewUser)
         const userId = user._id
         UserAuditLogHandler.addEntry(userId, 'login', userId, req.ip, err => {
           if (err) {
@@ -486,7 +490,8 @@ function _afterLoginSessionSetup(req, user, callback) {
     })
   })
 }
-function _loginAsyncHandlers(req, user) {
+
+function _loginAsyncHandlers(req, user, anonymousAnalyticsId, isNewUser) {
   UserHandler.setupLoginData(user, err => {
     if (err != null) {
       logger.warn({ err }, 'error setting up login data')
@@ -495,12 +500,14 @@ function _loginAsyncHandlers(req, user) {
   LoginRateLimiter.recordSuccessfulLogin(user.email)
   AuthenticationController._recordSuccessfulLogin(user._id)
   AuthenticationController.ipMatchCheck(req, user)
-  Analytics.recordEvent(user._id, 'user-logged-in')
-  Analytics.identifyUser(user._id, req.sessionID)
+  Analytics.recordEventForUser(user._id, 'user-logged-in')
+  Analytics.identifyUser(user._id, anonymousAnalyticsId, isNewUser)
+
   logger.log(
     { email: user.email, user_id: user._id.toString() },
     'successful log in'
   )
+
   req.session.justLoggedIn = true
   // capture the request ip for use when creating the session
   return (user._login_req_ip = req.ip)

+ 8 - 4
services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js

@@ -377,10 +377,14 @@ module.exports = CollaboratorsInviteController = {
           'project:membership:changed',
           { invites: true, members: true }
         )
-        AnalyticsManager.recordEvent(currentUser._id, 'project-invite-accept', {
-          projectId,
-          userId: currentUser._id,
-        })
+        AnalyticsManager.recordEventForUser(
+          currentUser._id,
+          'project-invite-accept',
+          {
+            projectId,
+            userId: currentUser._id,
+          }
+        )
         if (req.xhr) {
           return res.sendStatus(204) //  Done async via project page notification
         } else {

+ 1 - 1
services/web/app/src/Features/Project/ProjectController.js

@@ -780,7 +780,7 @@ const ProjectController = {
             metrics.inc(metricName)
 
             if (userId) {
-              AnalyticsManager.recordEvent(userId, 'project-opened', {
+              AnalyticsManager.recordEventForUser(userId, 'project-opened', {
                 projectId: project._id,
               })
             }

+ 5 - 5
services/web/app/src/Features/Project/ProjectCreationHandler.js

@@ -35,12 +35,12 @@ async function createBlankProject(ownerId, projectName, attributes = {}) {
   const isImport = attributes && attributes.overleaf
   const project = await _createBlankProject(ownerId, projectName, attributes)
   if (isImport) {
-    AnalyticsManager.recordEvent(ownerId, 'project-imported', {
+    AnalyticsManager.recordEventForUser(ownerId, 'project-imported', {
       projectId: project._id,
       attributes,
     })
   } else {
-    AnalyticsManager.recordEvent(ownerId, 'project-created', {
+    AnalyticsManager.recordEventForUser(ownerId, 'project-created', {
       projectId: project._id,
       attributes,
     })
@@ -50,7 +50,7 @@ async function createBlankProject(ownerId, projectName, attributes = {}) {
 
 async function createProjectFromSnippet(ownerId, projectName, docLines) {
   const project = await _createBlankProject(ownerId, projectName)
-  AnalyticsManager.recordEvent(ownerId, 'project-created', {
+  AnalyticsManager.recordEventForUser(ownerId, 'project-created', {
     projectId: project._id,
   })
   await _createRootDoc(project, ownerId, docLines)
@@ -63,7 +63,7 @@ async function createBasicProject(ownerId, projectName) {
   const docLines = await _buildTemplate('mainbasic.tex', ownerId, projectName)
   await _createRootDoc(project, ownerId, docLines)
 
-  AnalyticsManager.recordEvent(ownerId, 'project-created', {
+  AnalyticsManager.recordEventForUser(ownerId, 'project-created', {
     projectId: project._id,
   })
 
@@ -75,7 +75,7 @@ async function createExampleProject(ownerId, projectName) {
 
   await _addExampleProjectFiles(ownerId, projectName, project)
 
-  AnalyticsManager.recordEvent(ownerId, 'project-created', {
+  AnalyticsManager.recordEventForUser(ownerId, 'project-created', {
     projectId: project._id,
   })
 

+ 1 - 1
services/web/app/src/Features/SplitTests/SplitTestHandler.js

@@ -96,7 +96,7 @@ async function assignUserToVariant(userId, splitTest) {
       [`splitTests.${splitTest.id}`]: selectedVariant,
     },
   })
-  AnalyticsManager.setUserProperty(
+  AnalyticsManager.setUserPropertyForUser(
     userId,
     `split-test-${splitTest.id}`,
     selectedVariant

+ 124 - 29
services/web/app/src/Features/SplitTests/SplitTestV2Handler.js

@@ -1,6 +1,7 @@
 const UserGetter = require('../User/UserGetter')
 const UserUpdater = require('../User/UserUpdater')
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
+const UserAnalyticsIdCache = require('../Analytics/UserAnalyticsIdCache')
 const crypto = require('crypto')
 const _ = require('lodash')
 const { callbackify } = require('util')
@@ -24,7 +25,7 @@ const BETA_PHASE = 'beta'
  *   // execute the default behaviour (control group)
  * }
  * // then record an event
- * AnalyticsManager.recordEvent(userId, 'example-project-created', {
+ * AnalyticsManager.recordEventForUser(userId, 'example-project-created', {
  *   projectId: project._id,
  *   ...assignment.analytics.segmentation
  * })
@@ -35,8 +36,50 @@ const BETA_PHASE = 'beta'
  * @returns {Promise<{variant: string, analytics: {segmentation: {splitTest: string, variant: string, phase: string, versionNumber: number}|{}}}>}
  */
 async function getAssignment(userId, splitTestName, options) {
-  const splitTest = await splitTestCache.get(splitTestName)
+  if (!userId) {
+    return { variant: 'default', analytics: { segmentation: {} } }
+  }
+  const analyticsId = await UserAnalyticsIdCache.get(userId)
+  return _getAssignment(analyticsId, userId, undefined, splitTestName, options)
+}
+
+/**
+ * Get the assignment of a user to a split test by their session.
+ *
+ * @example
+ * // Assign user and record an event
+ *
+ * const assignment = await SplitTestV2Handler.getAssignment(req.session, 'example-project')
+ * if (assignment.variant === 'awesome-new-version') {
+ *   // execute my awesome change
+ * }
+ * else {
+ *   // execute the default behaviour (control group)
+ * }
+ * // then record an event
+ * AnalyticsManager.recordEventForSession(req.session, 'example-project-created', {
+ *   projectId: project._id,
+ *   ...assignment.analytics.segmentation
+ * })
+ *
+ * @param session the request session
+ * @param splitTestName the unique name of the split test
+ * @param options {Object<sync: boolean>} - for test purposes only, to force the synchronous update of the user's profile
+ * @returns {Promise<{variant: string, analytics: {segmentation: {splitTest: string, variant: string, phase: string, versionNumber: number}|{}}}>}
+ */
+async function getAssignmentForSession(session, splitTestName, options) {
+  const { userId, analyticsId } = AnalyticsManager.getIdsFromSession(session)
+  return _getAssignment(analyticsId, userId, session, splitTestName, options)
+}
 
+async function _getAssignment(
+  analyticsId,
+  userId,
+  session,
+  splitTestName,
+  options
+) {
+  const splitTest = await splitTestCache.get(splitTestName)
   if (splitTest) {
     const currentVersion = splitTest.getCurrentVersion()
     if (currentVersion.active) {
@@ -45,10 +88,12 @@ async function getAssignment(userId, splitTestName, options) {
         selectedVariantName,
         phase,
         versionNumber,
-      } = await _getAssignmentMetadata(userId, splitTest)
+      } = await _getAssignmentMetadata(analyticsId, userId, splitTest)
       if (activeForUser) {
         const assignmentConfig = {
           userId,
+          analyticsId,
+          session,
           splitTestName,
           variantName: selectedVariantName,
           phase,
@@ -89,21 +134,44 @@ async function assignInLocalsContext(res, userId, splitTestName, options) {
   res.locals.splitTestVariants[splitTestName] = assignment.variant
 }
 
-async function _getAssignmentMetadata(userId, splitTest) {
+async function assignInLocalsContextForSession(
+  res,
+  session,
+  splitTestName,
+  options
+) {
+  const assignment = await getAssignmentForSession(
+    session,
+    splitTestName,
+    options
+  )
+  if (!res.locals.splitTestVariants) {
+    res.locals.splitTestVariants = {}
+  }
+  res.locals.splitTestVariants[splitTestName] = assignment.variant
+}
+
+async function _getAssignmentMetadata(analyticsId, userId, splitTest) {
   const currentVersion = splitTest.getCurrentVersion()
   const phase = currentVersion.phase
   if ([ALPHA_PHASE, BETA_PHASE].includes(phase)) {
-    const user = await _getUser(userId)
-    if (
-      (phase === ALPHA_PHASE && !(user && user.alphaProgram)) ||
-      (phase === BETA_PHASE && !(user && user.betaProgram))
-    ) {
+    if (userId) {
+      const user = await _getUser(userId)
+      if (
+        (phase === ALPHA_PHASE && !(user && user.alphaProgram)) ||
+        (phase === BETA_PHASE && !(user && user.betaProgram))
+      ) {
+        return {
+          activeForUser: false,
+        }
+      }
+    } else {
       return {
         activeForUser: false,
       }
     }
   }
-  const percentile = _getPercentile(userId, splitTest.name, phase)
+  const percentile = _getPercentile(analyticsId, splitTest.name, phase)
   const selectedVariantName = _getVariantFromPercentile(
     currentVersion.variants,
     percentile
@@ -116,10 +184,10 @@ async function _getAssignmentMetadata(userId, splitTest) {
   }
 }
 
-function _getPercentile(userId, splitTestName, splitTestPhase) {
+function _getPercentile(analyticsId, splitTestName, splitTestPhase) {
   const hash = crypto
     .createHash('md5')
-    .update(userId + splitTestName + splitTestPhase)
+    .update(analyticsId + splitTestName + splitTestPhase)
     .digest('hex')
   const hashPrefix = hash.substr(0, 8)
   return Math.floor(
@@ -139,29 +207,52 @@ function _getVariantFromPercentile(variants, percentile) {
 
 async function _updateVariantAssignment({
   userId,
+  analyticsId,
+  session,
   splitTestName,
   phase,
   versionNumber,
   variantName,
 }) {
-  const user = await _getUser(userId)
-  if (user) {
-    const assignedSplitTests = user.splitTests || []
-    const assignmentLog = assignedSplitTests[splitTestName] || []
-    const existingAssignment = _.find(assignmentLog, { versionNumber })
-    if (!existingAssignment) {
-      await UserUpdater.promises.updateUser(userId, {
-        $addToSet: {
-          [`splitTests.${splitTestName}`]: {
-            variantName,
-            versionNumber,
-            phase,
-            assignedAt: new Date(),
+  const persistedAssignment = {
+    variantName,
+    versionNumber,
+    phase,
+    assignedAt: new Date(),
+  }
+  if (userId) {
+    const user = await _getUser(userId)
+    if (user) {
+      const assignedSplitTests = user.splitTests || []
+      const assignmentLog = assignedSplitTests[splitTestName] || []
+      const existingAssignment = _.find(assignmentLog, { versionNumber })
+      if (!existingAssignment) {
+        await UserUpdater.promises.updateUser(userId, {
+          $addToSet: {
+            [`splitTests.${splitTestName}`]: persistedAssignment,
           },
-        },
-      })
-      AnalyticsManager.setUserProperty(
-        userId,
+        })
+        AnalyticsManager.setUserPropertyForAnalyticsId(
+          analyticsId,
+          `split-test-${splitTestName}-${versionNumber}`,
+          variantName
+        )
+      }
+    }
+  } else if (session) {
+    if (!session.splitTests) {
+      session.splitTests = {}
+    }
+    if (!session.splitTests[splitTestName]) {
+      session.splitTests[splitTestName] = []
+    }
+    const existingAssignment = _.find(session.splitTests[splitTestName], {
+      versionNumber,
+    })
+    if (!existingAssignment) {
+      session.splitTests[splitTestName].push(persistedAssignment)
+      AnalyticsManager.setUserPropertyForAnalyticsId(
+        analyticsId,
         `split-test-${splitTestName}-${versionNumber}`,
         variantName
       )
@@ -179,9 +270,13 @@ async function _getUser(id) {
 
 module.exports = {
   getAssignment: callbackify(getAssignment),
+  getAssignmentForSession: callbackify(getAssignmentForSession),
   assignInLocalsContext: callbackify(assignInLocalsContext),
+  assignInLocalsContextForSession: callbackify(assignInLocalsContextForSession),
   promises: {
     getAssignment,
+    getAssignmentForSession,
     assignInLocalsContext,
+    assignInLocalsContextForSession,
   },
 }

+ 1 - 1
services/web/app/src/Features/Subscription/FeaturesUpdater.js

@@ -28,7 +28,7 @@ const FeaturesUpdater = {
         const matchedFeatureSet = FeaturesUpdater._getMatchedFeatureSet(
           features
         )
-        AnalyticsManager.setUserProperty(
+        AnalyticsManager.setUserPropertyForUser(
           userId,
           'feature-set',
           matchedFeatureSet

+ 80 - 32
services/web/app/src/Features/Subscription/RecurlyEventHandler.js

@@ -39,85 +39,133 @@ function sendRecurlyAnalyticsEvent(event, eventData) {
   }
 }
 
-function _sendSubscriptionStartedEvent(eventData) {
+async function _sendSubscriptionStartedEvent(eventData) {
   const userId = _getUserId(eventData)
   const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData)
-  AnalyticsManager.recordEvent(userId, 'subscription-started', {
+  AnalyticsManager.recordEventForUser(userId, 'subscription-started', {
     plan_code: planCode,
     quantity,
     is_trial: isTrial,
   })
-  AnalyticsManager.setUserProperty(userId, 'subscription-plan-code', planCode)
-  AnalyticsManager.setUserProperty(userId, 'subscription-state', state)
-  AnalyticsManager.setUserProperty(userId, 'subscription-is-trial', isTrial)
+  AnalyticsManager.setUserPropertyForUser(
+    userId,
+    'subscription-plan-code',
+    planCode
+  )
+  AnalyticsManager.setUserPropertyForUser(userId, 'subscription-state', state)
+  AnalyticsManager.setUserPropertyForUser(
+    userId,
+    'subscription-is-trial',
+    isTrial
+  )
 }
 
-function _sendSubscriptionUpdatedEvent(eventData) {
+async function _sendSubscriptionUpdatedEvent(eventData) {
   const userId = _getUserId(eventData)
   const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData)
-  AnalyticsManager.recordEvent(userId, 'subscription-updated', {
+  AnalyticsManager.recordEventForUser(userId, 'subscription-updated', {
     plan_code: planCode,
     quantity,
   })
-  AnalyticsManager.setUserProperty(userId, 'subscription-plan-code', planCode)
-  AnalyticsManager.setUserProperty(userId, 'subscription-state', state)
-  AnalyticsManager.setUserProperty(userId, 'subscription-is-trial', isTrial)
+  AnalyticsManager.setUserPropertyForUser(
+    userId,
+    'subscription-plan-code',
+    planCode
+  )
+  AnalyticsManager.setUserPropertyForUser(userId, 'subscription-state', state)
+  AnalyticsManager.setUserPropertyForUser(
+    userId,
+    'subscription-is-trial',
+    isTrial
+  )
 }
 
-function _sendSubscriptionCancelledEvent(eventData) {
+async function _sendSubscriptionCancelledEvent(eventData) {
   const userId = _getUserId(eventData)
   const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData)
-  AnalyticsManager.recordEvent(userId, 'subscription-cancelled', {
+  AnalyticsManager.recordEventForUser(userId, 'subscription-cancelled', {
     plan_code: planCode,
     quantity,
     is_trial: isTrial,
   })
-  AnalyticsManager.setUserProperty(userId, 'subscription-state', state)
-  AnalyticsManager.setUserProperty(userId, 'subscription-is-trial', isTrial)
+  AnalyticsManager.setUserPropertyForUser(userId, 'subscription-state', state)
+  AnalyticsManager.setUserPropertyForUser(
+    userId,
+    'subscription-is-trial',
+    isTrial
+  )
 }
 
-function _sendSubscriptionExpiredEvent(eventData) {
+async function _sendSubscriptionExpiredEvent(eventData) {
   const userId = _getUserId(eventData)
   const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData)
-  AnalyticsManager.recordEvent(userId, 'subscription-expired', {
+  AnalyticsManager.recordEventForUser(userId, 'subscription-expired', {
     plan_code: planCode,
     quantity,
     is_trial: isTrial,
   })
-  AnalyticsManager.setUserProperty(userId, 'subscription-plan-code', planCode)
-  AnalyticsManager.setUserProperty(userId, 'subscription-state', state)
-  AnalyticsManager.setUserProperty(userId, 'subscription-is-trial', isTrial)
+  AnalyticsManager.setUserPropertyForUser(
+    userId,
+    'subscription-plan-code',
+    planCode
+  )
+  AnalyticsManager.setUserPropertyForUser(userId, 'subscription-state', state)
+  AnalyticsManager.setUserPropertyForUser(
+    userId,
+    'subscription-is-trial',
+    isTrial
+  )
 }
 
-function _sendSubscriptionRenewedEvent(eventData) {
+async function _sendSubscriptionRenewedEvent(eventData) {
   const userId = _getUserId(eventData)
   const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData)
-  AnalyticsManager.recordEvent(userId, 'subscription-renewed', {
+  AnalyticsManager.recordEventForUser(userId, 'subscription-renewed', {
     plan_code: planCode,
     quantity,
     is_trial: isTrial,
   })
-  AnalyticsManager.setUserProperty(userId, 'subscription-plan-code', planCode)
-  AnalyticsManager.setUserProperty(userId, 'subscription-state', state)
-  AnalyticsManager.setUserProperty(userId, 'subscription-is-trial', isTrial)
+  AnalyticsManager.setUserPropertyForUser(
+    userId,
+    'subscription-plan-code',
+    planCode
+  )
+  AnalyticsManager.setUserPropertyForUser(userId, 'subscription-state', state)
+  AnalyticsManager.setUserPropertyForUser(
+    userId,
+    'subscription-is-trial',
+    isTrial
+  )
 }
 
-function _sendSubscriptionReactivatedEvent(eventData) {
+async function _sendSubscriptionReactivatedEvent(eventData) {
   const userId = _getUserId(eventData)
   const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData)
-  AnalyticsManager.recordEvent(userId, 'subscription-reactivated', {
+  AnalyticsManager.recordEventForUser(userId, 'subscription-reactivated', {
     plan_code: planCode,
     quantity,
   })
-  AnalyticsManager.setUserProperty(userId, 'subscription-plan-code', planCode)
-  AnalyticsManager.setUserProperty(userId, 'subscription-state', state)
-  AnalyticsManager.setUserProperty(userId, 'subscription-is-trial', isTrial)
+  AnalyticsManager.setUserPropertyForUser(
+    userId,
+    'subscription-plan-code',
+    planCode
+  )
+  AnalyticsManager.setUserPropertyForUser(userId, 'subscription-state', state)
+  AnalyticsManager.setUserPropertyForUser(
+    userId,
+    'subscription-is-trial',
+    isTrial
+  )
 }
 
-function _sendInvoicePaidEvent(eventData) {
+async function _sendInvoicePaidEvent(eventData) {
   const userId = _getUserId(eventData)
-  AnalyticsManager.recordEvent(userId, 'subscription-invoice-collected')
-  AnalyticsManager.setUserProperty(userId, 'subscription-is-trial', false)
+  AnalyticsManager.recordEventForUser(userId, 'subscription-invoice-collected')
+  AnalyticsManager.setUserPropertyForUser(
+    userId,
+    'subscription-is-trial',
+    false
+  )
 }
 
 function _getUserId(eventData) {

+ 1 - 1
services/web/app/src/Features/Subscription/SubscriptionController.js

@@ -116,7 +116,7 @@ async function userSubscriptionPage(req, res) {
     personalSubscription ? personalSubscription.plan : undefined
   )
 
-  AnalyticsManager.recordEvent(user._id, 'subscription-page-view')
+  AnalyticsManager.recordEventForSession(req.session, 'subscription-page-view')
 
   const data = {
     title: 'your_subscription',

+ 1 - 1
services/web/app/src/Features/Subscription/SubscriptionUpdater.js

@@ -360,7 +360,7 @@ async function _sendUserGroupPlanCodeUserProperty(userId) {
         bestFeatures = plan.features
       }
     }
-    AnalyticsManager.setUserProperty(
+    AnalyticsManager.setUserPropertyForUser(
       userId,
       'group-subscription-plan-code',
       bestPlanCode

+ 6 - 2
services/web/app/src/Features/TokenAccess/TokenAccessHandler.js

@@ -164,7 +164,9 @@ const TokenAccessHandler = {
   addReadOnlyUserToProject(userId, projectId, callback) {
     userId = ObjectId(userId.toString())
     projectId = ObjectId(projectId.toString())
-    Analytics.recordEvent(userId, 'project-joined', { mode: 'read-only' })
+    Analytics.recordEventForUser(userId, 'project-joined', {
+      mode: 'read-only',
+    })
     Project.updateOne(
       {
         _id: projectId,
@@ -179,7 +181,9 @@ const TokenAccessHandler = {
   addReadAndWriteUserToProject(userId, projectId, callback) {
     userId = ObjectId(userId.toString())
     projectId = ObjectId(projectId.toString())
-    Analytics.recordEvent(userId, 'project-joined', { mode: 'read-write' })
+    Analytics.recordEventForUser(userId, 'project-joined', {
+      mode: 'read-write',
+    })
     Project.updateOne(
       {
         _id: projectId,

+ 10 - 2
services/web/app/src/Features/User/UserCreator.js

@@ -84,8 +84,16 @@ async function createNewUser(attributes, options = {}) {
     }
   }
 
-  Analytics.recordEvent(user._id, 'user-registered')
-  Analytics.setUserProperty(user._id, 'created-at', new Date())
+  await Analytics.recordEventForUser(user._id, 'user-registered')
+  await Analytics.setUserPropertyForUser(user._id, 'created-at', new Date())
+  await Analytics.setUserPropertyForUser(user._id, 'user-id', user._id)
+  if (attributes.analyticsId) {
+    await Analytics.setUserPropertyForUser(
+      user._id,
+      'analytics-id',
+      attributes.analyticsId
+    )
+  }
 
   if (Features.hasFeature('saas')) {
     try {

+ 1 - 1
services/web/app/src/Features/User/UserPostRegistrationAnalyticsManager.js

@@ -32,7 +32,7 @@ async function checkAffiliations(userId) {
   )
 
   if (hasCommonsAccountAffiliation) {
-    await AnalyticsManager.setUserProperty(
+    await AnalyticsManager.setUserPropertyForUser(
       userId,
       'registered-from-commons-account',
       true

+ 1 - 2
services/web/app/src/Features/User/UserRegistrationHandler.js

@@ -8,7 +8,6 @@ const logger = require('logger-sharelatex')
 const crypto = require('crypto')
 const EmailHandler = require('../Email/EmailHandler')
 const OneTimeTokenHandler = require('../Security/OneTimeTokenHandler')
-const Analytics = require('../Analytics/AnalyticsManager')
 const settings = require('@overleaf/settings')
 const EmailHelper = require('../Helpers/EmailHelper')
 
@@ -31,6 +30,7 @@ const UserRegistrationHandler = {
           email: userDetails.email,
           first_name: userDetails.first_name,
           last_name: userDetails.last_name,
+          analyticsId: userDetails.analyticsId,
         },
         {},
         callback
@@ -87,7 +87,6 @@ const UserRegistrationHandler = {
             }, // this can be slow, just fire it off
           ],
           error => {
-            Analytics.recordEvent(user._id, 'user-registered')
             callback(error, user)
           }
         )

+ 5 - 0
services/web/app/src/infrastructure/Server.js

@@ -15,6 +15,7 @@ const sessionsRedisClient = UserSessionsRedis.client()
 
 const SessionAutostartMiddleware = require('./SessionAutostartMiddleware')
 const SessionStoreManager = require('./SessionStoreManager')
+const AnalyticsManager = require('../Features/Analytics/AnalyticsManager')
 const session = require('express-session')
 const RedisStore = require('connect-redis')(session)
 const bodyParser = require('./BodyParserWrapper')
@@ -32,6 +33,7 @@ const ProxyManager = require('./ProxyManager')
 const translations = require('./Translations')
 const Modules = require('./Modules')
 const Views = require('./Views')
+const Features = require('./Features')
 
 const ErrorController = require('../Features/Errors/ErrorController')
 const HttpErrorHandler = require('../Features/Errors/HttpErrorHandler')
@@ -125,6 +127,9 @@ webRouter.use(
     rolling: true,
   })
 )
+if (Features.hasFeature('saas')) {
+  webRouter.use(AnalyticsManager.analyticsIdMiddleware)
+}
 
 // patch the session store to generate a validation token for every new session
 SessionStoreManager.enableValidationToken(sessionStore)

+ 1 - 0
services/web/app/src/models/User.js

@@ -166,6 +166,7 @@ const UserSchema = new Schema({
   onboardingEmailSentAt: { type: Date },
   auditLog: [AuditLogEntrySchema],
   splitTests: Schema.Types.Mixed,
+  analyticsId: { type: String },
 })
 
 exports.User = mongoose.model('User', UserSchema)

+ 1 - 1
services/web/modules/server-ce-scripts/scripts/create-user.js

@@ -18,7 +18,7 @@ async function main() {
 
   await new Promise((resolve, reject) => {
     UserRegistrationHandler.registerNewUserAndSendActivationEmail(
-        email,
+      email,
       (error, user, setNewPasswordUrl) => {
         if (error) {
           return reject(error)

+ 7 - 23
services/web/test/unit/src/Analytics/AnalyticsControllerTests.js

@@ -12,7 +12,7 @@ describe('AnalyticsController', function () {
 
     this.AnalyticsManager = {
       updateEditingSession: sinon.stub(),
-      recordEvent: sinon.stub(),
+      recordEventForSession: sinon.stub(),
     }
 
     this.Features = {
@@ -42,6 +42,7 @@ describe('AnalyticsController', function () {
         params: {
           projectId: 'a project id',
         },
+        session: {},
       }
       this.GeoIpLookup.getDetails = sinon
         .stub()
@@ -78,35 +79,18 @@ describe('AnalyticsController', function () {
       delete this.expectedData._csrf
     })
 
-    it('should use the user_id', function (done) {
-      this.SessionManager.getLoggedInUserId.returns('1234')
-      this.controller.recordEvent(this.req, this.res)
-      this.AnalyticsManager.recordEvent
-        .calledWith('1234', this.req.params.event, this.expectedData)
-        .should.equal(true)
-      done()
-    })
-
-    it('should use the session id', function (done) {
+    it('should use the session', function (done) {
       this.controller.recordEvent(this.req, this.res)
-      this.AnalyticsManager.recordEvent
-        .calledWith(
-          this.req.sessionID,
-          this.req.params.event,
-          this.expectedData
-        )
+      this.AnalyticsManager.recordEventForSession
+        .calledWith(this.req.session, this.req.params.event, this.expectedData)
         .should.equal(true)
       done()
     })
 
     it('should remove the CSRF token before sending to the manager', function (done) {
       this.controller.recordEvent(this.req, this.res)
-      this.AnalyticsManager.recordEvent
-        .calledWith(
-          this.req.sessionID,
-          this.req.params.event,
-          this.expectedData
-        )
+      this.AnalyticsManager.recordEventForSession
+        .calledWith(this.req.session, this.req.params.event, this.expectedData)
         .should.equal(true)
       done()
     })

+ 15 - 6
services/web/test/unit/src/Analytics/AnalyticsManagerTests.js

@@ -10,6 +10,7 @@ const MODULE_PATH = path.join(
 describe('AnalyticsManager', function () {
   beforeEach(function () {
     this.fakeUserId = '123abc'
+    this.analyticsId = '123456'
     this.Settings = {
       analytics: { enabled: true },
     }
@@ -50,6 +51,9 @@ describe('AnalyticsManager', function () {
       requires: {
         '@overleaf/settings': this.Settings,
         '../../infrastructure/Queues': this.Queues,
+        './UserAnalyticsIdCache': (this.UserAnalyticsIdCache = {
+          get: sinon.stub().resolves(this.analyticsId),
+        }),
       },
     })
   })
@@ -70,21 +74,26 @@ describe('AnalyticsManager', function () {
 
   describe('queues the appropriate message for', function () {
     it('identifyUser', function () {
-      const oldUserId = '456def'
-      this.AnalyticsManager.identifyUser(this.fakeUserId, oldUserId)
+      const analyticsId = '456def'
+      this.AnalyticsManager.identifyUser(this.fakeUserId, analyticsId)
       sinon.assert.calledWithMatch(this.analyticsEventsQueue.add, 'identify', {
         userId: this.fakeUserId,
-        oldUserId,
+        analyticsId,
       })
     })
 
-    it('recordEvent', function () {
+    it('recordEventForUser', async function () {
       const event = 'fake-event'
-      this.AnalyticsManager.recordEvent(this.fakeUserId, event, null)
+      await this.AnalyticsManager.recordEventForUser(
+        this.fakeUserId,
+        event,
+        null
+      )
       sinon.assert.calledWithMatch(this.analyticsEventsQueue.add, 'event', {
+        analyticsId: this.analyticsId,
         event,
-        userId: this.fakeUserId,
         segmentation: null,
+        isLoggedIn: true,
       })
     })
 

+ 13 - 7
services/web/test/unit/src/Authentication/AuthenticationControllerTests.js

@@ -28,6 +28,7 @@ describe('AuthenticationController', function () {
     this.res = new MockResponse()
     this.callback = sinon.stub()
     this.next = sinon.stub()
+    this.req.session.analyticsId = 'abc-123'
 
     this.AuthenticationController = SandboxedModule.require(modulePath, {
       requires: {
@@ -53,8 +54,9 @@ describe('AuthenticationController', function () {
           setupLoginData: sinon.stub(),
         }),
         '../Analytics/AnalyticsManager': (this.AnalyticsManager = {
-          recordEvent: sinon.stub(),
+          recordEventForUser: sinon.stub(),
           identifyUser: sinon.stub(),
+          getIdsFromSession: sinon.stub().returns({ userId: this.user._id }),
         }),
         '../../infrastructure/SessionStoreManager': (this.SessionStoreManager = {}),
         '@overleaf/settings': (this.Settings = {
@@ -1236,9 +1238,11 @@ describe('AuthenticationController', function () {
       })
 
       it('should call identifyUser', function () {
-        this.AnalyticsManager.identifyUser
-          .calledWith(this.user._id, this.req.sessionID)
-          .should.equal(true)
+        sinon.assert.calledWith(
+          this.AnalyticsManager.identifyUser,
+          this.user._id,
+          this.req.session.analyticsId
+        )
       })
 
       it('should setup the user data in the background', function () {
@@ -1271,9 +1275,11 @@ describe('AuthenticationController', function () {
       })
 
       it('should track the login event', function () {
-        this.AnalyticsManager.recordEvent
-          .calledWith(this.user._id, 'user-logged-in')
-          .should.equal(true)
+        sinon.assert.calledWith(
+          this.AnalyticsManager.recordEventForUser,
+          this.user._id,
+          'user-logged-in'
+        )
       })
     })
   })

+ 1 - 1
services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.js

@@ -24,7 +24,7 @@ const { ObjectId } = require('mongodb')
 describe('CollaboratorsInviteController', function () {
   beforeEach(function () {
     this.user = { _id: 'id' }
-    this.AnalyticsManger = { recordEvent: sinon.stub() }
+    this.AnalyticsManger = { recordEventForUser: sinon.stub() }
     this.sendingUser = null
     this.AuthenticationController = {
       getSessionUser: req => {

+ 1 - 1
services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -173,7 +173,7 @@ describe('ProjectController', function () {
           .BrandVariationsHandler,
         '../ThirdPartyDataStore/TpdsProjectFlusher': this.TpdsProjectFlusher,
         '../../models/Project': {},
-        '../Analytics/AnalyticsManager': { recordEvent: () => {} },
+        '../Analytics/AnalyticsManager': { recordEventForUser: () => {} },
         '../../infrastructure/Modules': {
           hooks: { fire: sinon.stub().yields(null, []) },
         },

+ 3 - 3
services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js

@@ -40,7 +40,7 @@ describe('FeaturesUpdater', function () {
         '../Institutions/InstitutionsFeatures': (this.InstitutionsFeatures = {}),
         '../User/UserGetter': (this.UserGetter = {}),
         '../Analytics/AnalyticsManager': (this.AnalyticsManager = {
-          setUserProperty: sinon.stub(),
+          setUserPropertyForUser: sinon.stub(),
         }),
         '../../infrastructure/Modules': (this.Modules = {
           hooks: { fire: sinon.stub() },
@@ -182,7 +182,7 @@ describe('FeaturesUpdater', function () {
         )
 
         sinon.assert.calledWith(
-          this.AnalyticsManager.setUserProperty,
+          this.AnalyticsManager.setUserPropertyForUser,
           this.user_id,
           'feature-set',
           'personal'
@@ -201,7 +201,7 @@ describe('FeaturesUpdater', function () {
         )
 
         sinon.assert.calledWith(
-          this.AnalyticsManager.setUserProperty,
+          this.AnalyticsManager.setUserPropertyForUser,
           this.user_id,
           'feature-set',
           'mixed'

+ 26 - 26
services/web/test/unit/src/Subscription/RecurlyEventHandlerTests.js

@@ -27,8 +27,8 @@ describe('RecurlyEventHandler', function () {
     this.RecurlyEventHandler = SandboxedModule.require(modulePath, {
       requires: {
         '../Analytics/AnalyticsManager': (this.AnalyticsManager = {
-          recordEvent: sinon.stub(),
-          setUserProperty: sinon.stub(),
+          recordEventForUser: sinon.stub(),
+          setUserPropertyForUser: sinon.stub(),
         }),
       },
     })
@@ -40,7 +40,7 @@ describe('RecurlyEventHandler', function () {
       this.eventData
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.recordEvent,
+      this.AnalyticsManager.recordEventForUser,
       this.userId,
       'subscription-started',
       {
@@ -50,19 +50,19 @@ describe('RecurlyEventHandler', function () {
       }
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.setUserProperty,
+      this.AnalyticsManager.setUserPropertyForUser,
       this.userId,
       'subscription-plan-code',
       this.planCode
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.setUserProperty,
+      this.AnalyticsManager.setUserPropertyForUser,
       this.userId,
       'subscription-state',
       'active'
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.setUserProperty,
+      this.AnalyticsManager.setUserPropertyForUser,
       this.userId,
       'subscription-is-trial',
       true
@@ -83,7 +83,7 @@ describe('RecurlyEventHandler', function () {
       this.eventData
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.recordEvent,
+      this.AnalyticsManager.recordEventForUser,
       this.userId,
       'subscription-started',
       {
@@ -93,13 +93,13 @@ describe('RecurlyEventHandler', function () {
       }
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.setUserProperty,
+      this.AnalyticsManager.setUserPropertyForUser,
       this.userId,
       'subscription-state',
       'active'
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.setUserProperty,
+      this.AnalyticsManager.setUserPropertyForUser,
       this.userId,
       'subscription-is-trial',
       false
@@ -114,7 +114,7 @@ describe('RecurlyEventHandler', function () {
       this.eventData
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.recordEvent,
+      this.AnalyticsManager.recordEventForUser,
       this.userId,
       'subscription-updated',
       {
@@ -123,19 +123,19 @@ describe('RecurlyEventHandler', function () {
       }
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.setUserProperty,
+      this.AnalyticsManager.setUserPropertyForUser,
       this.userId,
       'subscription-plan-code',
       this.planCode
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.setUserProperty,
+      this.AnalyticsManager.setUserPropertyForUser,
       this.userId,
       'subscription-state',
       'active'
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.setUserProperty,
+      this.AnalyticsManager.setUserPropertyForUser,
       this.userId,
       'subscription-is-trial',
       true
@@ -149,7 +149,7 @@ describe('RecurlyEventHandler', function () {
       this.eventData
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.recordEvent,
+      this.AnalyticsManager.recordEventForUser,
       this.userId,
       'subscription-cancelled',
       {
@@ -159,13 +159,13 @@ describe('RecurlyEventHandler', function () {
       }
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.setUserProperty,
+      this.AnalyticsManager.setUserPropertyForUser,
       this.userId,
       'subscription-state',
       'cancelled'
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.setUserProperty,
+      this.AnalyticsManager.setUserPropertyForUser,
       this.userId,
       'subscription-is-trial',
       true
@@ -179,7 +179,7 @@ describe('RecurlyEventHandler', function () {
       this.eventData
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.recordEvent,
+      this.AnalyticsManager.recordEventForUser,
       this.userId,
       'subscription-expired',
       {
@@ -189,19 +189,19 @@ describe('RecurlyEventHandler', function () {
       }
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.setUserProperty,
+      this.AnalyticsManager.setUserPropertyForUser,
       this.userId,
       'subscription-plan-code',
       this.planCode
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.setUserProperty,
+      this.AnalyticsManager.setUserPropertyForUser,
       this.userId,
       'subscription-state',
       'expired'
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.setUserProperty,
+      this.AnalyticsManager.setUserPropertyForUser,
       this.userId,
       'subscription-is-trial',
       true
@@ -214,7 +214,7 @@ describe('RecurlyEventHandler', function () {
       this.eventData
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.recordEvent,
+      this.AnalyticsManager.recordEventForUser,
       this.userId,
       'subscription-renewed',
       {
@@ -231,7 +231,7 @@ describe('RecurlyEventHandler', function () {
       this.eventData
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.recordEvent,
+      this.AnalyticsManager.recordEventForUser,
       this.userId,
       'subscription-reactivated',
       {
@@ -255,7 +255,7 @@ describe('RecurlyEventHandler', function () {
       }
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.recordEvent,
+      this.AnalyticsManager.recordEventForUser,
       this.userId,
       'subscription-invoice-collected'
     )
@@ -274,7 +274,7 @@ describe('RecurlyEventHandler', function () {
         },
       }
     )
-    sinon.assert.notCalled(this.AnalyticsManager.recordEvent)
+    sinon.assert.notCalled(this.AnalyticsManager.recordEventForUser)
   })
 
   it('with closed_invoice_notification', function () {
@@ -291,7 +291,7 @@ describe('RecurlyEventHandler', function () {
       }
     )
     sinon.assert.calledWith(
-      this.AnalyticsManager.recordEvent,
+      this.AnalyticsManager.recordEventForUser,
       this.userId,
       'subscription-invoice-collected'
     )
@@ -310,6 +310,6 @@ describe('RecurlyEventHandler', function () {
         },
       }
     )
-    sinon.assert.notCalled(this.AnalyticsManager.recordEvent)
+    sinon.assert.notCalled(this.AnalyticsManager.recordEventForUser)
   })
 })

+ 3 - 2
services/web/test/unit/src/Subscription/SubscriptionControllerTests.js

@@ -141,8 +141,9 @@ describe('SubscriptionController', function () {
         }),
         './Errors': SubscriptionErrors,
         '../Analytics/AnalyticsManager': (this.AnalyticsManager = {
-          recordEvent: sinon.stub(),
-          setUserProperty: sinon.stub(),
+          recordEventForUser: sinon.stub(),
+          recordEventForSession: sinon.stub(),
+          setUserPropertyForUser: sinon.stub(),
         }),
         '../SplitTests/SplitTestHandler': (this.SplitTestHandler = {
           getTestSegmentation: () => {},

+ 1 - 1
services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js

@@ -108,7 +108,7 @@ describe('SubscriptionHandler', function () {
 
     this.EmailHandler = { sendEmail: sinon.stub() }
 
-    this.AnalyticsManager = { recordEvent: sinon.stub() }
+    this.AnalyticsManager = { recordEventForUser: sinon.stub() }
 
     this.PlansLocator = {
       findLocalPlanInSettings: sinon.stub().returns({ planCode: 'plan' }),

+ 7 - 7
services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js

@@ -105,7 +105,7 @@ describe('SubscriptionUpdater', function () {
       findOneAndUpdate: sinon.stub().yields(),
     }
     this.AnalyticsManager = {
-      setUserProperty: sinon.stub(),
+      setUserPropertyForUser: sinon.stub(),
     }
     this.SubscriptionUpdater = SandboxedModule.require(modulePath, {
       requires: {
@@ -527,7 +527,7 @@ describe('SubscriptionUpdater', function () {
         this.otherUserId,
         () => {
           sinon.assert.calledWith(
-            this.AnalyticsManager.setUserProperty,
+            this.AnalyticsManager.setUserPropertyForUser,
             this.otherUserId,
             'group-subscription-plan-code',
             'group_subscription'
@@ -547,7 +547,7 @@ describe('SubscriptionUpdater', function () {
         this.otherUserId
       )
       sinon.assert.calledWith(
-        this.AnalyticsManager.setUserProperty,
+        this.AnalyticsManager.setUserPropertyForUser,
         this.otherUserId,
         'group-subscription-plan-code',
         'group_subscription'
@@ -564,7 +564,7 @@ describe('SubscriptionUpdater', function () {
         this.otherUserId
       )
       sinon.assert.calledWith(
-        this.AnalyticsManager.setUserProperty,
+        this.AnalyticsManager.setUserPropertyForUser,
         this.otherUserId,
         'group-subscription-plan-code',
         'better_group_subscription'
@@ -581,7 +581,7 @@ describe('SubscriptionUpdater', function () {
         this.otherUserId
       )
       sinon.assert.calledWith(
-        this.AnalyticsManager.setUserProperty,
+        this.AnalyticsManager.setUserPropertyForUser,
         this.otherUserId,
         'group-subscription-plan-code',
         'better_group_subscription'
@@ -622,7 +622,7 @@ describe('SubscriptionUpdater', function () {
         this.otherUserId,
         () => {
           sinon.assert.calledWith(
-            this.AnalyticsManager.setUserProperty,
+            this.AnalyticsManager.setUserPropertyForUser,
             this.otherUserId,
             'group-subscription-plan-code',
             null
@@ -635,7 +635,7 @@ describe('SubscriptionUpdater', function () {
     it('should set the group plan code user property when removing user from all groups', function (done) {
       this.SubscriptionUpdater.removeUserFromAllGroups(this.otherUserId, () => {
         sinon.assert.calledWith(
-          this.AnalyticsManager.setUserProperty,
+          this.AnalyticsManager.setUserPropertyForUser,
           this.otherUserId,
           'group-subscription-plan-code',
           null

+ 3 - 4
services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js

@@ -12,7 +12,6 @@
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
 const SandboxedModule = require('sandboxed-module')
-const assert = require('assert')
 const path = require('path')
 const sinon = require('sinon')
 const modulePath = path.join(
@@ -43,7 +42,7 @@ describe('TokenAccessHandler', function () {
         }),
         crypto: (this.Crypto = require('crypto')),
         '../Analytics/AnalyticsManager': (this.Analytics = {
-          recordEvent: sinon.stub(),
+          recordEventForUser: sinon.stub(),
         }),
       },
     }))
@@ -138,7 +137,7 @@ describe('TokenAccessHandler', function () {
             this.Project.updateOne.lastCall.args[1].$addToSet
           ).to.have.keys('tokenAccessReadOnly_refs')
           sinon.assert.calledWith(
-            this.Analytics.recordEvent,
+            this.Analytics.recordEventForUser,
             this.userId,
             'project-joined',
             { mode: 'read-only' }
@@ -199,7 +198,7 @@ describe('TokenAccessHandler', function () {
             this.Project.updateOne.lastCall.args[1].$addToSet
           ).to.have.keys('tokenAccessReadAndWrite_refs')
           sinon.assert.calledWith(
-            this.Analytics.recordEvent,
+            this.Analytics.recordEventForUser,
             this.userId,
             'project-joined',
             { mode: 'read-write' }

+ 5 - 3
services/web/test/unit/src/User/UserControllerTests.js

@@ -24,6 +24,7 @@ describe('UserController', function () {
           _id: this.user_id,
           email: 'old@something.com',
         },
+        analyticsId: this.user_id,
       },
       sessionID: '123',
       body: {},
@@ -544,9 +545,10 @@ describe('UserController', function () {
     })
 
     it('should register the user and send them an email', function () {
-      this.UserRegistrationHandler.registerNewUserAndSendActivationEmail
-        .calledWith(this.email)
-        .should.equal(true)
+      sinon.assert.calledWith(
+        this.UserRegistrationHandler.registerNewUserAndSendActivationEmail,
+        this.email
+      )
     })
 
     it('should return the user and activation url', function () {

+ 4 - 4
services/web/test/unit/src/User/UserCreatorTests.js

@@ -42,8 +42,8 @@ describe('UserCreator', function () {
           },
         }),
         '../Analytics/AnalyticsManager': (this.Analytics = {
-          recordEvent: sinon.stub(),
-          setUserProperty: sinon.stub(),
+          recordEventForUser: sinon.stub(),
+          setUserPropertyForUser: sinon.stub(),
         }),
         './UserOnboardingEmailManager': (this.UserOnboardingEmailManager = {
           scheduleOnboardingEmail: sinon.stub(),
@@ -271,12 +271,12 @@ describe('UserCreator', function () {
         })
         assert.equal(user.email, this.email)
         sinon.assert.calledWith(
-          this.Analytics.recordEvent,
+          this.Analytics.recordEventForUser,
           user._id,
           'user-registered'
         )
         sinon.assert.calledWith(
-          this.Analytics.setUserProperty,
+          this.Analytics.setUserPropertyForUser,
           user._id,
           'created-at'
         )

+ 8 - 4
services/web/test/unit/src/User/UserPostRegistrationAnalyticsManagerTests.js

@@ -36,7 +36,7 @@ describe('UserPostRegistrationAnalyticsManager', function () {
       },
     }
     this.AnalyticsManager = {
-      setUserProperty: sinon.stub().resolves(),
+      setUserPropertyForUser: sinon.stub().resolves(),
     }
     this.UserPostRegistrationAnalyticsManager = SandboxedModule.require(
       MODULE_PATH,
@@ -72,7 +72,8 @@ describe('UserPostRegistrationAnalyticsManager', function () {
       )
       expect(this.InstitutionsAPI.promises.getUserAffiliations).not.to.have.been
         .called
-      expect(this.AnalyticsManager.setUserProperty).not.to.have.been.called
+      expect(this.AnalyticsManager.setUserPropertyForUser).not.to.have.been
+        .called
     })
 
     it('sets user property if user has commons account affiliationd', async function () {
@@ -92,7 +93,9 @@ describe('UserPostRegistrationAnalyticsManager', function () {
       await this.UserPostRegistrationAnalyticsManager.postRegistrationAnalytics(
         this.fakeUserId
       )
-      expect(this.AnalyticsManager.setUserProperty).to.have.been.calledWith(
+      expect(
+        this.AnalyticsManager.setUserPropertyForUser
+      ).to.have.been.calledWith(
         this.fakeUserId,
         'registered-from-commons-account',
         true
@@ -110,7 +113,8 @@ describe('UserPostRegistrationAnalyticsManager', function () {
       await this.UserPostRegistrationAnalyticsManager.postRegistrationAnalytics(
         this.fakeUserId
       )
-      expect(this.AnalyticsManager.setUserProperty).not.to.have.been.called
+      expect(this.AnalyticsManager.setUserPropertyForUser).not.to.have.been
+        .called
     })
   })
 })

+ 24 - 29
services/web/test/unit/src/User/UserRegistrationHandlerTests.js

@@ -24,7 +24,11 @@ const EmailHelper = require('../../../../app/src/Features/Helpers/EmailHelper')
 
 describe('UserRegistrationHandler', function () {
   beforeEach(function () {
-    this.user = { _id: (this.user_id = '31j2lk21kjl') }
+    this.analyticsId = '123456'
+    this.user = {
+      _id: (this.user_id = '31j2lk21kjl'),
+      analyticsId: this.analyticsId,
+    }
     this.User = { updateOne: sinon.stub().callsArgWith(2) }
     this.UserGetter = { getUserByAnyEmail: sinon.stub() }
     this.UserCreator = {
@@ -49,7 +53,9 @@ describe('UserRegistrationHandler', function () {
         '../Email/EmailHandler': this.EmailHandler,
         '../Security/OneTimeTokenHandler': this.OneTimeTokenHandler,
         '../Analytics/AnalyticsManager': (this.AnalyticsManager = {
-          recordEvent: sinon.stub(),
+          recordEventForUser: sinon.stub(),
+          setUserPropertyForUser: sinon.stub(),
+          identifyUser: sinon.stub(),
         }),
         '@overleaf/settings': (this.settings = {
           siteUrl: 'http://sl.example.com',
@@ -58,10 +64,11 @@ describe('UserRegistrationHandler', function () {
       },
     })
 
-    return (this.passingRequest = {
+    this.passingRequest = {
       email: 'something@email.com',
       password: '123',
-    })
+      analyticsId: this.analyticsId,
+    }
   })
 
   describe('validate Register Request', function () {
@@ -167,16 +174,15 @@ describe('UserRegistrationHandler', function () {
       })
 
       it('should create a new user', function (done) {
-        return this.handler.registerNewUser(this.passingRequest, err => {
-          this.UserCreator.createNewUser
-            .calledWith({
-              email: this.passingRequest.email,
-              holdingAccount: false,
-              first_name: this.passingRequest.first_name,
-              last_name: this.passingRequest.last_name,
-            })
-            .should.equal(true)
-          return done()
+        this.handler.registerNewUser(this.passingRequest, err => {
+          sinon.assert.calledWith(this.UserCreator.createNewUser, {
+            email: this.passingRequest.email,
+            holdingAccount: false,
+            first_name: this.passingRequest.first_name,
+            last_name: this.passingRequest.last_name,
+            analyticsId: this.user.analyticsId,
+          })
+          done()
         })
       })
 
@@ -227,15 +233,6 @@ describe('UserRegistrationHandler', function () {
           return done()
         })
       })
-
-      it('should track the registration event', function (done) {
-        return this.handler.registerNewUser(this.passingRequest, err => {
-          this.AnalyticsManager.recordEvent
-            .calledWith(this.user._id, 'user-registered')
-            .should.equal(true)
-          return done()
-        })
-      })
     })
 
     it('should call the ReferalAllocator', function (done) {
@@ -270,12 +267,10 @@ describe('UserRegistrationHandler', function () {
       })
 
       it('should ask the UserRegistrationHandler to register user', function () {
-        return this.handler.registerNewUser
-          .calledWith({
-            email: this.email,
-            password: this.password,
-          })
-          .should.equal(true)
+        sinon.assert.calledWith(this.handler.registerNewUser, {
+          email: this.email,
+          password: this.password,
+        })
       })
 
       it('should generate a new password reset token', function () {