Просмотр исходного кода

Merge pull request #6442 from overleaf/em-split-tests-user-admin

Show split test assignments in user admin

GitOrigin-RevId: 4563a4899d5278a0ef84188ae25cb5dfd3d5cb57
Eric Mc Sween 4 лет назад
Родитель
Сommit
57228c5589

+ 35 - 2
services/web/app/src/Features/SplitTests/SplitTestHandler.js

@@ -5,7 +5,7 @@ const LocalsHelper = require('./LocalsHelper')
 const crypto = require('crypto')
 const _ = require('lodash')
 const { callbackify } = require('util')
-const splitTestCache = require('./SplitTestCache')
+const SplitTestCache = require('./SplitTestCache')
 
 const DEFAULT_VARIANT = 'default'
 const ALPHA_PHASE = 'alpha'
@@ -103,6 +103,37 @@ async function assignInLocalsContext(
   )
 }
 
+/**
+ * Get a mapping of the active split test assignments for the given user
+ */
+async function getActiveAssignmentsForUser(userId) {
+  const user = await UserGetter.promises.getUser(userId, { splitTests: 1 })
+  if (user == null || user.splitTests == null) {
+    return {}
+  }
+  const activeAssignments = {}
+  for (const [splitTestName, assignments] of Object.entries(user.splitTests)) {
+    const splitTest = await SplitTestCache.get(splitTestName)
+    if (splitTest == null) {
+      continue
+    }
+    const currentVersion = splitTest.getCurrentVersion()
+    if (!currentVersion || !currentVersion.active) {
+      continue
+    }
+
+    let assignment
+    if (Array.isArray(assignments)) {
+      assignment = _.maxBy(assignments, 'versionNumber')
+    } else {
+      // Older format is a single string rather than an array of objects
+      assignment = { variantName: assignments }
+    }
+    activeAssignments[splitTestName] = assignment
+  }
+  return activeAssignments
+}
+
 async function _getAssignment(
   splitTestName,
   { analyticsId, userId, session, sync }
@@ -111,7 +142,7 @@ async function _getAssignment(
     return DEFAULT_ASSIGNMENT
   }
 
-  const splitTest = await splitTestCache.get(splitTestName)
+  const splitTest = await SplitTestCache.get(splitTestName)
   const currentVersion = splitTest?.getCurrentVersion()
   if (!splitTest || !currentVersion?.active) {
     return DEFAULT_ASSIGNMENT
@@ -304,10 +335,12 @@ async function _getUser(id) {
 module.exports = {
   getAssignment: callbackify(getAssignment),
   getAssignmentForUser: callbackify(getAssignmentForUser),
+  getActiveAssignmentsForUser: callbackify(getActiveAssignmentsForUser),
   assignInLocalsContext: callbackify(assignInLocalsContext),
   promises: {
     getAssignment,
     getAssignmentForUser,
+    getActiveAssignmentsForUser,
     assignInLocalsContext,
   },
 }

+ 122 - 0
services/web/test/unit/src/SplitTests/SplitTestHandlerTests.js

@@ -0,0 +1,122 @@
+const Path = require('path')
+const SandboxedModule = require('sandboxed-module')
+const sinon = require('sinon')
+const { ObjectId } = require('mongodb')
+const { expect } = require('chai')
+
+const MODULE_PATH = Path.join(
+  __dirname,
+  '../../../../app/src/Features/SplitTests/SplitTestHandler'
+)
+
+describe('SplitTestHandler', function () {
+  beforeEach(function () {
+    this.splitTest = {
+      getCurrentVersion: sinon.stub().returns({ active: true }),
+    }
+    this.inactiveSplitTest = {
+      getCurrentVersion: sinon.stub().returns({ active: false }),
+    }
+
+    this.UserGetter = {
+      promises: {
+        getUser: sinon.stub().resolves(null),
+      },
+    }
+
+    this.SplitTestCache = {
+      get: sinon.stub().resolves(null),
+    }
+    this.SplitTestCache.get.withArgs('legacy-test').resolves(this.splitTest)
+    this.SplitTestCache.get.withArgs('other-test').resolves(this.splitTest)
+    this.SplitTestCache.get
+      .withArgs('inactive-test')
+      .resolves(this.inactiveSplitTest)
+
+    this.SplitTestHandler = SandboxedModule.require(MODULE_PATH, {
+      requires: {
+        '../User/UserGetter': this.UserGetter,
+        './SplitTestCache': this.SplitTestCache,
+        '../User/UserUpdater': {},
+        '../Analytics/AnalyticsManager': {},
+        './LocalsHelper': {},
+      },
+    })
+  })
+
+  describe('with an existing user', function () {
+    beforeEach(async function () {
+      this.user = {
+        _id: ObjectId(),
+        splitTests: {
+          'legacy-test': 'legacy-variant',
+          'other-test': [
+            { variantName: 'default', versionNumber: 1 },
+            { variantName: 'latest', versionNumber: 3 },
+            { variantName: 'experiment', versionNumber: 2 },
+          ],
+          'inactive-test': [{ variantName: 'trythis' }],
+          'unknown-test': [{ variantName: 'trythis' }],
+        },
+      }
+      this.UserGetter.promises.getUser
+        .withArgs(this.user._id)
+        .resolves(this.user)
+      this.assignments =
+        await this.SplitTestHandler.promises.getActiveAssignmentsForUser(
+          this.user._id
+        )
+    })
+
+    it('handles the legacy assignment format', function () {
+      expect(this.assignments).to.have.property('legacy-test')
+      expect(this.assignments['legacy-test'].variantName).to.equal(
+        'legacy-variant'
+      )
+    })
+
+    it('returns the last assignment for each active test', function () {
+      expect(this.assignments).to.have.property('other-test')
+      expect(this.assignments['other-test'].variantName).to.equal('latest')
+    })
+
+    it('does not return assignments for inactive tests', function () {
+      expect(this.assignments).not.to.have.property('inactive-test')
+    })
+
+    it('does not return assignments for unknown tests', function () {
+      expect(this.assignments).not.to.have.property('unknown-test')
+    })
+  })
+
+  describe('with an inexistent user', function () {
+    beforeEach(async function () {
+      const unknownUserId = ObjectId()
+      this.assignments =
+        await this.SplitTestHandler.promises.getActiveAssignmentsForUser(
+          unknownUserId
+        )
+    })
+
+    it('returns empty assignments', function () {
+      expect(this.assignments).to.deep.equal({})
+    })
+  })
+
+  describe('with a user without assignments', function () {
+    beforeEach(async function () {
+      this.user = { _id: ObjectId() }
+      this.UserGetter.promises.getUser
+        .withArgs(this.user._id)
+        .resolves(this.user)
+      this.assignments =
+        await this.SplitTestHandler.promises.getActiveAssignmentsForUser(
+          this.user._id
+        )
+    })
+
+    it('returns empty assignments', function () {
+      expect(this.assignments).to.deep.equal({})
+    })
+  })
+})