From 7678e5aeaedddf05f029c9bb552da712ea95db2a Mon Sep 17 00:00:00 2001 From: Alexandre Bourdin Date: Mon, 22 Nov 2021 11:35:10 +0100 Subject: [PATCH] Merge pull request #5769 from overleaf/ab-null-split-tests Setup null split tests GitOrigin-RevId: 4cba55e123d0a4add19cdace7434506e9d20c7a9 --- .../src/Features/Project/ProjectController.js | 39 ++++++++++++++++--- .../Features/SplitTests/SplitTestV2Handler.js | 18 +++++++++ .../components/share-modal-body.js | 5 +++ .../src/Project/ProjectControllerTests.js | 17 +++++--- 4 files changed, 68 insertions(+), 11 deletions(-) diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index e1d7e9fa9a..9ba9908c6c 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -611,6 +611,12 @@ const ProjectController = { : undefined } + // null test targeting logged in users + SplitTestV2Handler.promises.getAssignmentForSession( + req.session, + 'null-test-dashboard' + ) + res.render('project/list', viewModel) timer.done() }) @@ -724,12 +730,27 @@ const ProjectController = { TpdsProjectFlusher.flushProjectToTpdsIfNeeded(projectId, cb) }, sharingModalSplitTest(cb) { - SplitTestV2Handler.assignInLocalsContext( + SplitTestV2Handler.assignInLocalsContextForSession( res, - userId, + req.session, 'project-share-modal-paywall', - err => { - cb(err, null) + {}, + () => { + // do not fail editor load if assignment fails + cb() + } + ) + }, + sharingModalNullTest(cb) { + // null test targeting logged in users, for front-end side + SplitTestV2Handler.assignInLocalsContextForSession( + res, + req.session, + 'null-test-share-modal', + {}, + () => { + // do not fail editor load if assignment fails + cb() } ) }, @@ -737,8 +758,14 @@ const ProjectController = { SplitTestV2Handler.getAssignmentForSession( req.session, 'react-pdf-preview-rollout', - (err, assignment) => { - cb(err, assignment) + {}, + (error, assignment) => { + if (error) { + // do not fail editor load if assignment fails + cb(null, { variant: 'default' }) + } else { + cb(null, assignment) + } } ) }, diff --git a/services/web/app/src/Features/SplitTests/SplitTestV2Handler.js b/services/web/app/src/Features/SplitTests/SplitTestV2Handler.js index c04129ac89..095871753f 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestV2Handler.js +++ b/services/web/app/src/Features/SplitTests/SplitTestV2Handler.js @@ -78,6 +78,15 @@ async function getAssignmentForSession(session, splitTestName, options) { return _getAssignment(analyticsId, userId, session, splitTestName, options) } +/** + * Get the assignment of a user to a split test by their ID and stores it in the locals context. + * + * @param res the Express response object + * @param userId the user ID + * @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} + */ async function assignInLocalsContext(res, userId, splitTestName, options) { const assignment = await getAssignment(userId, splitTestName, options) if (!res.locals.splitTestVariants) { @@ -86,6 +95,15 @@ async function assignInLocalsContext(res, userId, splitTestName, options) { res.locals.splitTestVariants[splitTestName] = assignment.variant } +/** + * Get the assignment of a user to a split test by their session and stores it in the locals context. + * + * @param res the Express response object + * @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} + */ async function assignInLocalsContextForSession( res, session, diff --git a/services/web/frontend/js/features/share-project-modal/components/share-modal-body.js b/services/web/frontend/js/features/share-project-modal/components/share-modal-body.js index f4bd66df32..a3be594dbf 100644 --- a/services/web/frontend/js/features/share-project-modal/components/share-modal-body.js +++ b/services/web/frontend/js/features/share-project-modal/components/share-modal-body.js @@ -11,6 +11,7 @@ import { useSplitTestContext } from '../../../shared/context/split-test-context' import { Row } from 'react-bootstrap' import PropTypes from 'prop-types' import RecaptchaConditions from '../../../shared/components/recaptcha-conditions' +import * as eventTracking from '../../../infrastructure/event-tracking' export default function ShareModalBody() { const { isAdmin } = useShareProjectContext() @@ -18,6 +19,10 @@ export default function ShareModalBody() { splitTestVariants: PropTypes.object, }) + eventTracking.sendMB('share-modal-opened', { + splitTestVariant: splitTestVariants['null-test-share-modal'], + }) + const project = useProjectContext() switch (splitTestVariants['project-share-modal-paywall']) { diff --git a/services/web/test/unit/src/Project/ProjectControllerTests.js b/services/web/test/unit/src/Project/ProjectControllerTests.js index f0b4c57f7d..f274c711e2 100644 --- a/services/web/test/unit/src/Project/ProjectControllerTests.js +++ b/services/web/test/unit/src/Project/ProjectControllerTests.js @@ -136,14 +136,21 @@ describe('ProjectController', function () { } this.SplitTestV2Handler = { promises: { - getAssignment: sinon.stub().resolves({ active: false }), - assignInLocalsContext: sinon.stub().resolves(), + getAssignment: sinon.stub().resolves({ variant: 'default' }), + getAssignmentForSession: sinon.stub().resolves({ variant: 'default' }), + assignInLocalsContext: sinon.stub().resolves({ variant: 'default' }), + assignInLocalsContextForSession: sinon + .stub() + .resolves({ variant: 'default' }), }, + getAssignment: sinon.stub().yields(null, { variant: 'default' }), getAssignmentForSession: sinon .stub() - .yields(null, { variant: 'variant' }), - getAssignment: sinon.stub().yields(null, { active: false }), - assignInLocalsContext: sinon.stub().yields(null), + .yields(null, { variant: 'default' }), + assignInLocalsContext: sinon.stub().yields(null, { variant: 'default' }), + assignInLocalsContextForSession: sinon + .stub() + .yields(null, { variant: 'default' }), } this.ProjectController = SandboxedModule.require(MODULE_PATH, {