diff --git a/services/web/app/src/Features/Analytics/AnalyticsController.js b/services/web/app/src/Features/Analytics/AnalyticsController.js index f18cee429d..f50470a2b3 100644 --- a/services/web/app/src/Features/Analytics/AnalyticsController.js +++ b/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) }, } diff --git a/services/web/app/src/Features/Analytics/AnalyticsManager.js b/services/web/app/src/Features/Analytics/AnalyticsManager.js index 278ea4691c..2437d001ed 100644 --- a/services/web/app/src/Features/Analytics/AnalyticsManager.js +++ b/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), } diff --git a/services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceHelper.js b/services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceHelper.js index c29f91caad..fa31d62234 100644 --- a/services/web/app/src/Features/Analytics/AnalyticsRegistrationSourceHelper.js +++ b/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] diff --git a/services/web/app/src/Features/Analytics/UserAnalyticsIdCache.js b/services/web/app/src/Features/Analytics/UserAnalyticsIdCache.js new file mode 100644 index 0000000000..c665f68bad --- /dev/null +++ b/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() diff --git a/services/web/app/src/Features/Authentication/AuthenticationController.js b/services/web/app/src/Features/Authentication/AuthenticationController.js index bbd0d4c543..6064a88b98 100644 --- a/services/web/app/src/Features/Authentication/AuthenticationController.js +++ b/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) diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js b/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js index 9a5383f5f7..4c5a2b8d22 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js +++ b/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 { diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index e3bca01b9c..93a08b9701 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/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, }) } diff --git a/services/web/app/src/Features/Project/ProjectCreationHandler.js b/services/web/app/src/Features/Project/ProjectCreationHandler.js index be8729cc0a..8b28d09923 100644 --- a/services/web/app/src/Features/Project/ProjectCreationHandler.js +++ b/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, }) diff --git a/services/web/app/src/Features/SplitTests/SplitTestHandler.js b/services/web/app/src/Features/SplitTests/SplitTestHandler.js index 0a08aad2de..989e21b14f 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestHandler.js +++ b/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 diff --git a/services/web/app/src/Features/SplitTests/SplitTestV2Handler.js b/services/web/app/src/Features/SplitTests/SplitTestV2Handler.js index 6d32618809..7516d6499b 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestV2Handler.js +++ b/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} - 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, }, } diff --git a/services/web/app/src/Features/Subscription/FeaturesUpdater.js b/services/web/app/src/Features/Subscription/FeaturesUpdater.js index 563ec45488..b9af77dbac 100644 --- a/services/web/app/src/Features/Subscription/FeaturesUpdater.js +++ b/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 diff --git a/services/web/app/src/Features/Subscription/RecurlyEventHandler.js b/services/web/app/src/Features/Subscription/RecurlyEventHandler.js index f21815b051..ea977a8a35 100644 --- a/services/web/app/src/Features/Subscription/RecurlyEventHandler.js +++ b/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) { diff --git a/services/web/app/src/Features/Subscription/SubscriptionController.js b/services/web/app/src/Features/Subscription/SubscriptionController.js index 3aa6f919e6..e1dabb55bc 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionController.js +++ b/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', diff --git a/services/web/app/src/Features/Subscription/SubscriptionUpdater.js b/services/web/app/src/Features/Subscription/SubscriptionUpdater.js index 050dd9a0d6..b15e496c2c 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionUpdater.js +++ b/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 diff --git a/services/web/app/src/Features/TokenAccess/TokenAccessHandler.js b/services/web/app/src/Features/TokenAccess/TokenAccessHandler.js index d97cb81c04..8f9a63ab42 100644 --- a/services/web/app/src/Features/TokenAccess/TokenAccessHandler.js +++ b/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, diff --git a/services/web/app/src/Features/User/UserCreator.js b/services/web/app/src/Features/User/UserCreator.js index 5bb5ba5574..828e0e7f83 100644 --- a/services/web/app/src/Features/User/UserCreator.js +++ b/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 { diff --git a/services/web/app/src/Features/User/UserPostRegistrationAnalyticsManager.js b/services/web/app/src/Features/User/UserPostRegistrationAnalyticsManager.js index c8dc5b9aed..389cac3198 100644 --- a/services/web/app/src/Features/User/UserPostRegistrationAnalyticsManager.js +++ b/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 diff --git a/services/web/app/src/Features/User/UserRegistrationHandler.js b/services/web/app/src/Features/User/UserRegistrationHandler.js index d935e4cd7b..a45b6dbcc8 100644 --- a/services/web/app/src/Features/User/UserRegistrationHandler.js +++ b/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) } ) diff --git a/services/web/app/src/infrastructure/Server.js b/services/web/app/src/infrastructure/Server.js index 7e30f77237..199c52a071 100644 --- a/services/web/app/src/infrastructure/Server.js +++ b/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) diff --git a/services/web/app/src/models/User.js b/services/web/app/src/models/User.js index 4ba8a810f2..5a4e396792 100644 --- a/services/web/app/src/models/User.js +++ b/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) diff --git a/services/web/modules/server-ce-scripts/scripts/create-user.js b/services/web/modules/server-ce-scripts/scripts/create-user.js index 994ad874ec..031c78fff2 100644 --- a/services/web/modules/server-ce-scripts/scripts/create-user.js +++ b/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) diff --git a/services/web/test/unit/src/Analytics/AnalyticsControllerTests.js b/services/web/test/unit/src/Analytics/AnalyticsControllerTests.js index 686400614f..9a29cffbe7 100644 --- a/services/web/test/unit/src/Analytics/AnalyticsControllerTests.js +++ b/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') + it('should use the session', function (done) { 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) { - 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() }) diff --git a/services/web/test/unit/src/Analytics/AnalyticsManagerTests.js b/services/web/test/unit/src/Analytics/AnalyticsManagerTests.js index ed08641a7b..d041c6e617 100644 --- a/services/web/test/unit/src/Analytics/AnalyticsManagerTests.js +++ b/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) - sinon.assert.calledWithMatch(this.analyticsEventsQueue.add, 'event', { + 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, }) }) diff --git a/services/web/test/unit/src/Authentication/AuthenticationControllerTests.js b/services/web/test/unit/src/Authentication/AuthenticationControllerTests.js index 834d0ba725..8ec78545a1 100644 --- a/services/web/test/unit/src/Authentication/AuthenticationControllerTests.js +++ b/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' + ) }) }) }) diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.js b/services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.js index c70a7d48fa..7f04d0ad03 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.js +++ b/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 => { diff --git a/services/web/test/unit/src/Project/ProjectControllerTests.js b/services/web/test/unit/src/Project/ProjectControllerTests.js index f111801131..fa0fdc158b 100644 --- a/services/web/test/unit/src/Project/ProjectControllerTests.js +++ b/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, []) }, }, diff --git a/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js b/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js index c4facd1b1d..40b27b5773 100644 --- a/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js +++ b/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' diff --git a/services/web/test/unit/src/Subscription/RecurlyEventHandlerTests.js b/services/web/test/unit/src/Subscription/RecurlyEventHandlerTests.js index 59492c0018..1c1cb18564 100644 --- a/services/web/test/unit/src/Subscription/RecurlyEventHandlerTests.js +++ b/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) }) }) diff --git a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js index 1f3e03388a..27d80343d3 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js +++ b/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: () => {}, diff --git a/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js b/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js index 2da7c0cfd7..a9671559f0 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js +++ b/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' }), diff --git a/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js b/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js index 5c6535b712..b4c41c0671 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js +++ b/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 diff --git a/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js b/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js index 6565d69bbb..15b88bc127 100644 --- a/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js +++ b/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' } diff --git a/services/web/test/unit/src/User/UserControllerTests.js b/services/web/test/unit/src/User/UserControllerTests.js index a3f7d6b51c..76eb385cff 100644 --- a/services/web/test/unit/src/User/UserControllerTests.js +++ b/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 () { diff --git a/services/web/test/unit/src/User/UserCreatorTests.js b/services/web/test/unit/src/User/UserCreatorTests.js index 82cbffd64c..b1c0b8c874 100644 --- a/services/web/test/unit/src/User/UserCreatorTests.js +++ b/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' ) diff --git a/services/web/test/unit/src/User/UserPostRegistrationAnalyticsManagerTests.js b/services/web/test/unit/src/User/UserPostRegistrationAnalyticsManagerTests.js index 1f95a75825..d14c9abf32 100644 --- a/services/web/test/unit/src/User/UserPostRegistrationAnalyticsManagerTests.js +++ b/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 }) }) }) diff --git a/services/web/test/unit/src/User/UserRegistrationHandlerTests.js b/services/web/test/unit/src/User/UserRegistrationHandlerTests.js index 15748e7b17..adcd01f14c 100644 --- a/services/web/test/unit/src/User/UserRegistrationHandlerTests.js +++ b/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 () {