From db70128d03a646d983345c2876d78d6d10bb0c9a Mon Sep 17 00:00:00 2001
From: Miguel Serrano <mserranom@users.noreply.github.com>
Date: Wed, 31 May 2023 15:24:38 +0200
Subject: [PATCH] Merge pull request #13244 from
 overleaf/msm-fix-split-test-client

[web] Fix split test assignments not sent via LocalsHelper in Server Pro

GitOrigin-RevId: 3a69fda3a4a0e66ecde3772d918d9c6d46e89ecf
---
 .../Features/SplitTests/SplitTestHandler.js   | 50 +++++++++----------
 .../src/SplitTests/SplitTestHandlerTests.js   | 35 +++++++++++++
 2 files changed, 60 insertions(+), 25 deletions(-)

diff --git a/services/web/app/src/Features/SplitTests/SplitTestHandler.js b/services/web/app/src/Features/SplitTests/SplitTestHandler.js
index 8b1ec0132a..f1cd0a00dc 100644
--- a/services/web/app/src/Features/SplitTests/SplitTestHandler.js
+++ b/services/web/app/src/Features/SplitTests/SplitTestHandler.js
@@ -49,37 +49,37 @@ const DEFAULT_ASSIGNMENT = {
  * @returns {Promise<{variant: string, analytics: {segmentation: {splitTest: string, variant: string, phase: string, versionNumber: number}|{}}}>}
  */
 async function getAssignment(req, res, splitTestName, { sync = false } = {}) {
-  if (!Features.hasFeature('saas')) {
-    return _getNonSaasAssignment(splitTestName)
-  }
-
   const query = req.query || {}
   let assignment
 
-  // Check the query string for an override, ignoring an invalid value
-  const queryVariant = query[splitTestName]
-  if (queryVariant) {
-    const variants = await _getVariantNames(splitTestName)
-    if (variants.includes(queryVariant)) {
-      assignment = {
-        variant: queryVariant,
-        analytics: {
-          segmentation: {},
-        },
+  if (!Features.hasFeature('saas')) {
+    assignment = _getNonSaasAssignment(splitTestName)
+  } else {
+    // Check the query string for an override, ignoring an invalid value
+    const queryVariant = query[splitTestName]
+    if (queryVariant) {
+      const variants = await _getVariantNames(splitTestName)
+      if (variants.includes(queryVariant)) {
+        assignment = {
+          variant: queryVariant,
+          analytics: {
+            segmentation: {},
+          },
+        }
       }
     }
-  }
 
-  if (!assignment) {
-    const { userId, analyticsId } = AnalyticsManager.getIdsFromSession(
-      req.session
-    )
-    assignment = await _getAssignment(splitTestName, {
-      analyticsId,
-      userId,
-      session: req.session,
-      sync,
-    })
+    if (!assignment) {
+      const { userId, analyticsId } = AnalyticsManager.getIdsFromSession(
+        req.session
+      )
+      assignment = await _getAssignment(splitTestName, {
+        analyticsId,
+        userId,
+        session: req.session,
+        sync,
+      })
+    }
   }
 
   LocalsHelper.setSplitTestVariant(
diff --git a/services/web/test/unit/src/SplitTests/SplitTestHandlerTests.js b/services/web/test/unit/src/SplitTests/SplitTestHandlerTests.js
index 2aa370eb0a..308e7098ac 100644
--- a/services/web/test/unit/src/SplitTests/SplitTestHandlerTests.js
+++ b/services/web/test/unit/src/SplitTests/SplitTestHandlerTests.js
@@ -257,6 +257,41 @@ describe('SplitTestHandler', function () {
       assert.equal('default', assignment.variant)
     })
   })
+
+  describe('save assignments to res.locals', function () {
+    beforeEach(function () {
+      this.AnalyticsManager.getIdsFromSession.returns({
+        userId: 'abc123abc123',
+      })
+    })
+
+    it('when in SaaS mode it should set the variant', async function () {
+      await this.SplitTestHandler.promises.getAssignment(
+        this.req,
+        this.res,
+        'active-test'
+      )
+      expect(this.LocalsHelper.setSplitTestVariant).to.have.been.calledWith(
+        this.res.locals,
+        'active-test',
+        'variant-1'
+      )
+    })
+
+    it('when not in SaaS mode it should set the default variant', async function () {
+      this.Settings.overleaf = undefined
+      await this.SplitTestHandler.promises.getAssignment(
+        this.req,
+        this.res,
+        'active-test'
+      )
+      expect(this.LocalsHelper.setSplitTestVariant).to.have.been.calledWith(
+        this.res.locals,
+        'active-test',
+        'default'
+      )
+    })
+  })
 })
 
 function makeSplitTest(