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

Merge pull request #22252 from overleaf/ab-gradual-rollout-continuity

[web] Ensure continuity for gradual rollouts

GitOrigin-RevId: c5bada71ae476862c782dc669024944f12d77097
Alexandre Bourdin 1 год назад
Родитель
Сommit
356212ecde

+ 31 - 43
services/web/app/src/Features/SplitTests/SplitTestHandler.js

@@ -127,42 +127,6 @@ async function getAssignmentForUser(
   }
 }
 
-/**
- * Get the assignment of a user to a split test by their pre-fetched mongo doc.
- *
- * Warning: this does not support query parameters override, nor makes the assignment and split test info available to
- * the frontend through locals. Wherever possible, `getAssignment` should be used instead.
- *
- * @param user the user
- * @param splitTestName the unique name of the split test
- * @param options {Object<sync: boolean>} - for test purposes only, to force the synchronous update of the user's profile
- * @returns {Promise<Assignment>}
- */
-async function getAssignmentForMongoUser(
-  user,
-  splitTestName,
-  { sync = false } = {}
-) {
-  try {
-    if (!Features.hasFeature('saas')) {
-      return _getNonSaasAssignment(splitTestName)
-    }
-
-    return _getAssignment(splitTestName, {
-      analyticsId: await UserAnalyticsIdCache.get(user._id),
-      sync,
-      user,
-      userId: user._id.toString(),
-    })
-  } catch (error) {
-    logger.error(
-      { err: error },
-      'Failed to get split test assignment for mongo user'
-    )
-    return DEFAULT_ASSIGNMENT
-  }
-}
-
 /**
  * Get a mapping of the active split test assignments for the given user
  */
@@ -238,8 +202,7 @@ async function getOneTimeAssignment(splitTestName) {
       variant: selectedVariantName,
       currentVersion,
       isFirstNonDefaultAssignment:
-        selectedVariantName !== DEFAULT_VARIANT &&
-        currentVersion.analyticsEnabled,
+        selectedVariantName !== DEFAULT_VARIANT && _isSplitTest(splitTest),
     })
   } catch (error) {
     logger.error({ err: error }, 'Failed to get one time split test assignment')
@@ -344,7 +307,7 @@ async function _getAssignment(
   }
 
   if (activeForUser) {
-    if (currentVersion.analyticsEnabled) {
+    if (_isSplitTest(splitTest)) {
       // if the user is logged in, persist the assignment
       if (userId) {
         const assignmentData = {
@@ -424,7 +387,25 @@ async function _getAssignment(
 
 async function _getAssignmentMetadata(analyticsId, user, splitTest) {
   const currentVersion = SplitTestUtils.getCurrentVersion(splitTest)
+  const versionNumber = currentVersion.versionNumber
   const phase = currentVersion.phase
+
+  // For continuity on phase rollout for gradual rollouts, we keep all users from the previous phase enrolled to the variant.
+  // In beta, all alpha users are cohorted to the variant, and the same in release phase all alpha & beta users.
+  if (
+    _isGradualRollout(splitTest) &&
+    ((phase === BETA_PHASE && user?.alphaProgram) ||
+      (phase === RELEASE_PHASE && (user?.alphaProgram || user?.betaProgram)))
+  ) {
+    return {
+      activeForUser: true,
+      selectedVariantName: currentVersion.variants[0].name,
+      phase,
+      versionNumber,
+      isFirstNonDefaultAssignment: false,
+    }
+  }
+
   if (
     (phase === ALPHA_PHASE && !user?.alphaProgram) ||
     (phase === BETA_PHASE && !user?.betaProgram)
@@ -433,6 +414,7 @@ async function _getAssignmentMetadata(analyticsId, user, splitTest) {
       activeForUser: false,
     }
   }
+
   const userId = user?._id.toString()
   const percentile = getPercentile(analyticsId || userId, splitTest.name, phase)
   const selectedVariantName =
@@ -442,10 +424,10 @@ async function _getAssignmentMetadata(analyticsId, user, splitTest) {
     activeForUser: true,
     selectedVariantName,
     phase,
-    versionNumber: currentVersion.versionNumber,
+    versionNumber,
     isFirstNonDefaultAssignment:
       selectedVariantName !== DEFAULT_VARIANT &&
-      currentVersion.analyticsEnabled &&
+      _isSplitTest(splitTest) &&
       (!Array.isArray(user?.splitTests?.[splitTest.name]) ||
         !user?.splitTests?.[splitTest.name]?.some(
           assignment => assignment.variantName !== DEFAULT_VARIANT
@@ -582,10 +564,17 @@ async function _getSplitTest(name) {
   }
 }
 
+function _isSplitTest(featureFlag) {
+  return SplitTestUtils.getCurrentVersion(featureFlag).analyticsEnabled
+}
+
+function _isGradualRollout(featureFlag) {
+  return !SplitTestUtils.getCurrentVersion(featureFlag).analyticsEnabled
+}
+
 module.exports = {
   getPercentile,
   getAssignment: callbackify(getAssignment),
-  getAssignmentForMongoUser: callbackify(getAssignmentForMongoUser),
   getAssignmentForUser: callbackify(getAssignmentForUser),
   getOneTimeAssignment: callbackify(getOneTimeAssignment),
   getActiveAssignmentsForUser: callbackify(getActiveAssignmentsForUser),
@@ -593,7 +582,6 @@ module.exports = {
   clearOverridesInSession,
   promises: {
     getAssignment,
-    getAssignmentForMongoUser,
     getAssignmentForUser,
     getOneTimeAssignment,
     getActiveAssignmentsForUser,

+ 20 - 3
services/web/app/src/Features/SplitTests/SplitTestManager.js

@@ -83,7 +83,12 @@ async function createSplitTest(
 ) {
   const stripedVariants = []
   let stripeStart = 0
-  _checkNewVariantsConfiguration([], configuration.variants)
+
+  _checkNewVariantsConfiguration(
+    [],
+    configuration.variants,
+    configuration.analyticsEnabled
+  )
   for (const variant of configuration.variants) {
     stripedVariants.push({
       name: (variant.name || '').trim(),
@@ -139,7 +144,11 @@ async function updateSplitTestConfig({ name, configuration, comment }, userId) {
       `Cannot update with different phase - use /switch-to-next-phase endpoint instead`
     )
   }
-  _checkNewVariantsConfiguration(lastVersion.variants, configuration.variants)
+  _checkNewVariantsConfiguration(
+    lastVersion.variants,
+    configuration.variants,
+    configuration.analyticsEnabled
+  )
   const updatedVariants = _updateVariantsWithNewConfiguration(
     lastVersion.variants,
     configuration.variants
@@ -320,7 +329,15 @@ async function clearCache() {
   await CacheFlow.reset('split-test')
 }
 
-function _checkNewVariantsConfiguration(variants, newVariantsConfiguration) {
+function _checkNewVariantsConfiguration(
+  variants,
+  newVariantsConfiguration,
+  analyticsEnabled
+) {
+  if (newVariantsConfiguration?.length > 1 && !analyticsEnabled) {
+    throw new OError(`Gradual rollouts can only have a single variant`)
+  }
+
   const totalRolloutPercentage = _getTotalRolloutPercentage(
     newVariantsConfiguration
   )

+ 0 - 3
services/web/test/unit/src/Editor/EditorHttpControllerTests.js

@@ -132,9 +132,6 @@ describe('EditorHttpController', function () {
     this.SplitTestHandler = {
       promises: {
         getAssignmentForUser: sinon.stub().resolves({ variant: 'default' }),
-        getAssignmentForMongoUser: sinon
-          .stub()
-          .resolves({ variant: 'default' }),
       },
     }
     this.UserGetter = { promises: { getUser: sinon.stub().resolves(null, {}) } }