Przeglądaj źródła

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
Miguel Serrano 3 lat temu
rodzic
commit
db70128d03

+ 25 - 25
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(

+ 35 - 0
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(