From 27e2adecab0bbff13aadb52bccaede1b47d8fefa Mon Sep 17 00:00:00 2001 From: roo hutton Date: Thu, 6 Mar 2025 08:54:24 +0000 Subject: [PATCH] Merge pull request #23939 from overleaf/rh-cio-analytics-split-test Only send events to customer.io if in campaign split test GitOrigin-RevId: 572ad5efdfc1e86f525722c6a425fa1454f2cf3a --- .../src/Features/Project/ProjectController.js | 10 ++ .../Features/SplitTests/SplitTestHandler.js | 102 +++++++++++++++++- .../Subscription/RecurlyEventHandler.js | 9 +- .../web/app/views/project/editor/_meta.pug | 1 + .../js/infrastructure/event-tracking.ts | 4 + services/web/frontend/js/utils/meta.ts | 1 + .../src/Project/ProjectControllerTests.js | 1 + .../src/SplitTests/SplitTestHandlerTests.js | 60 ++++++++++- 8 files changed, 180 insertions(+), 8 deletions(-) diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index 82aa41c127..163794298a 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -768,6 +768,15 @@ const _ProjectController = { isPaywallChangeCompileTimeoutEnabled && (await ProjectController._getPaywallPlansPrices(req, res)) + const customerIoEnabled = + await SplitTestHandler.promises.hasUserBeenAssignedToVariant( + req, + userId, + 'customer-io-trial-conversion', + 'enabled', + true + ) + const addonPrices = isOverleafAssistBundleEnabled && (await ProjectController._getAddonPrices(req, res)) @@ -883,6 +892,7 @@ const _ProjectController = { isPaywallChangeCompileTimeoutEnabled, isOverleafAssistBundleEnabled, paywallPlans, + customerIoEnabled, addonPrices, }) timer.done() diff --git a/services/web/app/src/Features/SplitTests/SplitTestHandler.js b/services/web/app/src/Features/SplitTests/SplitTestHandler.js index 94fc036457..491c236a3c 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestHandler.js +++ b/services/web/app/src/Features/SplitTests/SplitTestHandler.js @@ -127,10 +127,97 @@ async function getAssignmentForUser( } } +/** + * Returns true if user has already been explicitly assigned to a variant. + * This will be false if the user **would** be assigned when calling getAssignment but hasn't yet. + * + * @param req express request + * @param {string} userId the user ID + * @param {string} splitTestName the unique name of the split test + * @param {string} variant variant name to check + * @param {boolean} ignoreVersion users explicitly assigned to a previous version should be treated as if assigned to latest version + */ +async function hasUserBeenAssignedToVariant( + req, + userId, + splitTestName, + variant, + ignoreVersion = false +) { + try { + const { session, query = {} } = req + + const splitTest = await _getSplitTest(splitTestName) + const currentVersion = SplitTestUtils.getCurrentVersion(splitTest) + + // Check the query string for an override, ignoring an invalid value + const queryVariant = query[splitTestName] + if (queryVariant === variant) { + const variants = await _getVariantNames(splitTestName) + if (variants.includes(queryVariant)) { + return true + } + } + + // Allow dev toolbar and session cache to override assignment from DB + if (Settings.devToolbar.enabled) { + const override = session?.splitTestOverrides?.[splitTestName] + if (override === variant) { + return true + } + } + + const canUseSessionCache = session && SessionManager.isUserLoggedIn(session) + if (canUseSessionCache) { + const cachedVariant = SplitTestSessionHandler.getCachedVariant( + session, + splitTestName, + currentVersion + ) + if (cachedVariant === variant) { + return true + } + } + + // get variant from db, including explicit assignments from previous versions if requested + const assignments = await getActiveAssignmentsForUser( + userId, + true, + ignoreVersion + ) + const testAssignment = assignments[splitTestName] + + if (!testAssignment || !testAssignment.assignedAt) { + return false + } + + // if variant matches and we can use cache, we should persist it in cache + if (testAssignment.variantName === variant && testAssignment.assignedAt) { + if (canUseSessionCache) { + SplitTestSessionHandler.setVariantInCache({ + session, + splitTestName, + currentVersion, + selectedVariantName: variant, + activeForUser: true, + }) + } + return true + } + } catch (error) { + logger.error({ err: error }, 'Failed to get split test assignment for user') + return false + } +} + /** * Get a mapping of the active split test assignments for the given user */ -async function getActiveAssignmentsForUser(userId, removeArchived = false) { +async function getActiveAssignmentsForUser( + userId, + removeArchived = false, + ignoreVersion = false +) { if (!Features.hasFeature('saas')) { return {} } @@ -156,9 +243,14 @@ async function getActiveAssignmentsForUser(userId, removeArchived = false) { } const userAssignments = user.splitTests?.[splitTest.name] if (Array.isArray(userAssignments)) { - const userAssignment = userAssignments.find( - x => x.versionNumber === versionNumber - ) + let userAssignment + if (!ignoreVersion) { + userAssignment = userAssignments.find( + x => x.versionNumber === versionNumber + ) + } else { + userAssignment = userAssignments[0] + } if (userAssignment) { assignment.assignedAt = userAssignment.assignedAt } @@ -578,6 +670,7 @@ module.exports = { getAssignmentForUser: callbackify(getAssignmentForUser), getOneTimeAssignment: callbackify(getOneTimeAssignment), getActiveAssignmentsForUser: callbackify(getActiveAssignmentsForUser), + hasUserBeenAssignedToVariant: callbackify(hasUserBeenAssignedToVariant), setOverrideInSession, clearOverridesInSession, promises: { @@ -585,5 +678,6 @@ module.exports = { getAssignmentForUser, getOneTimeAssignment, getActiveAssignmentsForUser, + hasUserBeenAssignedToVariant, }, } diff --git a/services/web/app/src/Features/Subscription/RecurlyEventHandler.js b/services/web/app/src/Features/Subscription/RecurlyEventHandler.js index 53cc47e9c0..906e1258ea 100644 --- a/services/web/app/src/Features/Subscription/RecurlyEventHandler.js +++ b/services/web/app/src/Features/Subscription/RecurlyEventHandler.js @@ -128,10 +128,17 @@ async function _sendSubscriptionStartedEvent(userId, eventData) { if (isTrial) { await SubscriptionEmailHandler.sendTrialOnboardingEmail(userId, planCode) - await SplitTestHandler.promises.getAssignmentForUser( + const cioAssignment = await SplitTestHandler.promises.getAssignmentForUser( userId, 'customer-io-trial-conversion' ) + if (cioAssignment.variant === 'enabled') { + AnalyticsManager.setUserPropertyForUserInBackground( + userId, + 'customer-io-integration', + true + ) + } } } diff --git a/services/web/app/views/project/editor/_meta.pug b/services/web/app/views/project/editor/_meta.pug index 8622f625d1..9659eeec50 100644 --- a/services/web/app/views/project/editor/_meta.pug +++ b/services/web/app/views/project/editor/_meta.pug @@ -42,6 +42,7 @@ meta(name="ol-shouldLoadHotjar" data-type="boolean" content=shouldLoadHotjar) meta(name="ol-isReviewerRoleEnabled" data-type="boolean" content=isReviewerRoleEnabled) meta(name="ol-odcRole" data-type="string" content=odcRole) meta(name="ol-isPaywallChangeCompileTimeoutEnabled" data-type="boolean" content=isPaywallChangeCompileTimeoutEnabled) +meta(name='ol-customerIoEnabled' data-type="boolean" content=customerIoEnabled) if(isPaywallChangeCompileTimeoutEnabled) //- expose plans info to show prices in paywall-change-compile-timeout test meta(name="ol-paywallPlans", data-type="json" content=paywallPlans) diff --git a/services/web/frontend/js/infrastructure/event-tracking.ts b/services/web/frontend/js/infrastructure/event-tracking.ts index 698f5df425..a2b65a1ce4 100644 --- a/services/web/frontend/js/infrastructure/event-tracking.ts +++ b/services/web/frontend/js/infrastructure/event-tracking.ts @@ -47,6 +47,10 @@ export function sendMB(key: string, segmentation: Segmentation = {}) { segmentation.page = window.location.pathname } + if (getMeta('ol-customerIoEnabled')) { + segmentation['customerio-integration'] = 'enabled' + } + sendBeacon(key, segmentation) if (typeof window.gtag !== 'function') return diff --git a/services/web/frontend/js/utils/meta.ts b/services/web/frontend/js/utils/meta.ts index e3e37b5a79..d8e0382999 100644 --- a/services/web/frontend/js/utils/meta.ts +++ b/services/web/frontend/js/utils/meta.ts @@ -84,6 +84,7 @@ export interface Meta { 'ol-currentInstitutionsWithLicence': Institution[] 'ol-currentManagedUserAdminEmail': string 'ol-currentUrl': string + 'ol-customerIoEnabled': boolean 'ol-debugPdfDetach': boolean 'ol-detachRole': 'detached' | 'detacher' | '' 'ol-dictionariesRoot': 'string' diff --git a/services/web/test/unit/src/Project/ProjectControllerTests.js b/services/web/test/unit/src/Project/ProjectControllerTests.js index 431cb22da6..46427171da 100644 --- a/services/web/test/unit/src/Project/ProjectControllerTests.js +++ b/services/web/test/unit/src/Project/ProjectControllerTests.js @@ -182,6 +182,7 @@ describe('ProjectController', function () { promises: { getAssignment: sinon.stub().resolves({ variant: 'default' }), getAssignmentForUser: sinon.stub().resolves({ variant: 'default' }), + hasUserBeenAssignedToVariant: sinon.stub().resolves(false), }, getAssignment: sinon.stub().yields(null, { variant: 'default' }), } diff --git a/services/web/test/unit/src/SplitTests/SplitTestHandlerTests.js b/services/web/test/unit/src/SplitTests/SplitTestHandlerTests.js index 5ff85c11ec..7fa6fd4003 100644 --- a/services/web/test/unit/src/SplitTests/SplitTestHandlerTests.js +++ b/services/web/test/unit/src/SplitTests/SplitTestHandlerTests.js @@ -14,7 +14,7 @@ const MODULE_PATH = Path.join( describe('SplitTestHandler', function () { beforeEach(function () { this.splitTests = [ - makeSplitTest('active-test'), + makeSplitTest('active-test', { versionNumber: 2 }), makeSplitTest('not-active-test', { active: false }), makeSplitTest('legacy-test'), makeSplitTest('no-analytics-test-1', { analyticsEnabled: false }), @@ -109,6 +109,27 @@ describe('SplitTestHandler', function () { await this.SplitTestHandler.promises.getActiveAssignmentsForUser( this.user._id ) + this.explicitAssignments = + await this.SplitTestHandler.promises.getActiveAssignmentsForUser( + this.user._id, + false, + true + ) + this.assignedToActiveTest = + await this.SplitTestHandler.promises.hasUserBeenAssignedToVariant( + this.req, + this.user._id, + 'active-test', + 'variant-1' + ) + this.assignedToActiveTestAnyVersion = + await this.SplitTestHandler.promises.hasUserBeenAssignedToVariant( + this.req, + this.user._id, + 'active-test', + 'variant-1', + true + ) }) it('handles the legacy assignment format', function () { @@ -123,7 +144,15 @@ describe('SplitTestHandler', function () { expect(this.assignments['active-test']).to.deep.equal({ variantName: 'variant-1', phase: 'release', - versionNumber: 1, + versionNumber: 2, + }) + }) + + it('returns the explicit assignment for each active test', function () { + expect(this.explicitAssignments['active-test']).to.deep.equal({ + variantName: 'variant-1', + phase: 'release', + versionNumber: 2, assignedAt: 'active-test-assigned-at', }) }) @@ -144,6 +173,14 @@ describe('SplitTestHandler', function () { }) }) + it('shows user has been assigned to previous version of variant', function () { + expect(this.assignedToActiveTestAnyVersion).to.be.true + }) + + it('shows user has not been explicitly assigned to current version of variant', function () { + expect(this.assignedToActiveTest).to.be.false + }) + it('does not return assignments for unknown tests', function () { expect(this.assignments).not.to.have.property('unknown-test') }) @@ -171,6 +208,19 @@ describe('SplitTestHandler', function () { await this.SplitTestHandler.promises.getActiveAssignmentsForUser( this.user._id ) + this.explicitAssignments = + await this.SplitTestHandler.promises.getActiveAssignmentsForUser( + this.user._id, + false, + true + ) + this.assignedToActiveTest = + await this.SplitTestHandler.promises.hasUserBeenAssignedToVariant( + this.req, + this.user._id, + 'active-test', + 'variant-1' + ) }) it('returns current assignments', function () { @@ -178,7 +228,7 @@ describe('SplitTestHandler', function () { 'active-test': { phase: 'release', variantName: 'variant-1', - versionNumber: 1, + versionNumber: 2, }, 'legacy-test': { phase: 'release', @@ -202,6 +252,10 @@ describe('SplitTestHandler', function () { }, }) }) + + it('shows user not assigned to variant', function () { + expect(this.assignedToActiveTest).to.be.false + }) }) describe('with settings overrides', function () {