Quellcode durchsuchen

Merge pull request #19053 from overleaf/ab-split-tests-first-time-assignments

[web] Return isFirstTimeAssignment flag with split test assignments

GitOrigin-RevId: 70954470fbd9430749d83d8d1e08a3969d4a09e6
Jimmy Domagala-Tang vor 2 Jahren
Ursprung
Commit
04432478e1

+ 25 - 0
services/web/app/src/Features/Project/ProjectController.js

@@ -41,6 +41,7 @@ const ProjectAuditLogHandler = require('./ProjectAuditLogHandler')
 const PublicAccessLevels = require('../Authorization/PublicAccessLevels')
 const PublicAccessLevels = require('../Authorization/PublicAccessLevels')
 const TagsHandler = require('../Tags/TagsHandler')
 const TagsHandler = require('../Tags/TagsHandler')
 const TutorialHandler = require('../Tutorial/TutorialHandler')
 const TutorialHandler = require('../Tutorial/TutorialHandler')
+const UserUpdater = require('../User/UserUpdater')
 
 
 /**
 /**
  * @typedef {import("./types").GetProjectsRequest} GetProjectsRequest
  * @typedef {import("./types").GetProjectsRequest} GetProjectsRequest
@@ -423,6 +424,30 @@ const _ProjectController = {
         isInvitedMember,
         isInvitedMember,
       } = userValues
       } = userValues
 
 
+      // check if a user is not in the writefull-oauth-promotion, in which case they may be part of the auto trial group
+      if (
+        !anonymous &&
+        splitTestAssignments['writefull-oauth-promotion']?.variant === 'default'
+      ) {
+        // since we are auto-enrolling users into writefull if they are part of the group, we only want to
+        // auto enroll (set writefull to true) if its the first time they have entered the test
+        // this ensures that they can still turn writefull off (otherwise, we would be setting writefull on every time they access their projects)
+        const { variant, metadata } =
+          await SplitTestHandler.promises.getAssignment(
+            req,
+            res,
+            'writefull-auto-load'
+          )
+        if (variant === 'enabled' && metadata?.isFirstNonDefaultAssignment) {
+          await UserUpdater.promises.updateUser(userId, {
+            $set: {
+              writefull: { enabled: true },
+            },
+          })
+          user.writefull.enabled = true
+        }
+      }
+
       const brandVariation = project?.brandVariationId
       const brandVariation = project?.brandVariationId
         ? await BrandVariationsHandler.promises.getBrandVariationById(
         ? await BrandVariationsHandler.promises.getBrandVariationById(
             project.brandVariationId
             project.brandVariationId

+ 62 - 39
services/web/app/src/Features/SplitTests/SplitTestHandler.js

@@ -16,15 +16,17 @@ const logger = require('@overleaf/logger')
 const SplitTestSessionHandler = require('./SplitTestSessionHandler')
 const SplitTestSessionHandler = require('./SplitTestSessionHandler')
 const SplitTestUserGetter = require('./SplitTestUserGetter')
 const SplitTestUserGetter = require('./SplitTestUserGetter')
 
 
+/**
+ * @typedef {import("./types").Assignment} Assignment
+ */
+
 const DEFAULT_VARIANT = 'default'
 const DEFAULT_VARIANT = 'default'
 const ALPHA_PHASE = 'alpha'
 const ALPHA_PHASE = 'alpha'
 const BETA_PHASE = 'beta'
 const BETA_PHASE = 'beta'
 const RELEASE_PHASE = 'release'
 const RELEASE_PHASE = 'release'
 const DEFAULT_ASSIGNMENT = {
 const DEFAULT_ASSIGNMENT = {
   variant: DEFAULT_VARIANT,
   variant: DEFAULT_VARIANT,
-  analytics: {
-    segmentation: {},
-  },
+  metadata: {},
 }
 }
 
 
 /**
 /**
@@ -40,17 +42,12 @@ const DEFAULT_ASSIGNMENT = {
  * else {
  * else {
  *   // execute the default behaviour (control group)
  *   // execute the default behaviour (control group)
  * }
  * }
- * // then record an event
- * AnalyticsManager.recordEventForSession(req.session, 'example-project-created', {
- *   projectId: project._id,
- *   ...assignment.analytics.segmentation
- * })
  *
  *
  * @param req the request
  * @param req the request
  * @param res the Express response object
  * @param res the Express response object
  * @param splitTestName the unique name of the split test
  * @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
  * @param options {Object<sync: boolean>} - for test purposes only, to force the synchronous update of the user's profile
- * @returns {Promise<{variant: string, analytics: {segmentation: {splitTest: string, variant: string, phase: string, versionNumber: number}|{}}}>}
+ * @returns {Promise<Assignment>}
  */
  */
 async function getAssignment(req, res, splitTestName, { sync = false } = {}) {
 async function getAssignment(req, res, splitTestName, { sync = false } = {}) {
   const query = req.query || {}
   const query = req.query || {}
@@ -69,9 +66,7 @@ async function getAssignment(req, res, splitTestName, { sync = false } = {}) {
         if (variants.includes(queryVariant)) {
         if (variants.includes(queryVariant)) {
           assignment = {
           assignment = {
             variant: queryVariant,
             variant: queryVariant,
-            analytics: {
-              segmentation: {},
-            },
+            metadata: {},
           }
           }
         }
         }
       }
       }
@@ -112,7 +107,7 @@ async function getAssignment(req, res, splitTestName, { sync = false } = {}) {
  * @param userId the user ID
  * @param userId the user ID
  * @param splitTestName the unique name of the split test
  * @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
  * @param options {Object<sync: boolean>} - for test purposes only, to force the synchronous update of the user's profile
- * @returns {Promise<{variant: string, analytics: {segmentation: {splitTest: string, variant: string, phase: string, versionNumber: number}|{}}}>}
+ * @returns {Promise<Assignment>}
  */
  */
 async function getAssignmentForUser(
 async function getAssignmentForUser(
   userId,
   userId,
@@ -141,7 +136,7 @@ async function getAssignmentForUser(
  * @param user the user
  * @param user the user
  * @param splitTestName the unique name of the split test
  * @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
  * @param options {Object<sync: boolean>} - for test purposes only, to force the synchronous update of the user's profile
- * @returns {Promise<{variant: string, analytics: {segmentation: {splitTest: string, variant: string, phase: string, versionNumber: number}|{}}}>}
+ * @returns {Promise<Assignment>}
  */
  */
 async function getAssignmentForMongoUser(
 async function getAssignmentForMongoUser(
   user,
   user,
@@ -215,7 +210,7 @@ async function getActiveAssignmentsForUser(userId, removeArchived = false) {
  * To be used only in cases where we need random assignments that are independent of a user or session.
  * To be used only in cases where we need random assignments that are independent of a user or session.
  * If the test is in alpha or beta phase, always returns the default variant.
  * If the test is in alpha or beta phase, always returns the default variant.
  * @param splitTestName
  * @param splitTestName
- * @returns {Promise<{variant: string, analytics: {segmentation: {splitTest: string, variant: string, phase: string, versionNumber: number}|{}}}>}
+ * @returns {Promise<Assignment>}
  */
  */
 async function getOneTimeAssignment(splitTestName) {
 async function getOneTimeAssignment(splitTestName) {
   try {
   try {
@@ -239,7 +234,13 @@ async function getOneTimeAssignment(splitTestName) {
       undefined,
       undefined,
       splitTest
       splitTest
     )
     )
-    return _makeAssignment(splitTest, selectedVariantName, currentVersion)
+    return _makeAssignment({
+      variant: selectedVariantName,
+      currentVersion,
+      isFirstNonDefaultAssignment:
+        selectedVariantName !== DEFAULT_VARIANT &&
+        currentVersion.analyticsEnabled,
+    })
   } catch (error) {
   } catch (error) {
     logger.error({ err: error }, 'Failed to get one time split test assignment')
     logger.error({ err: error }, 'Failed to get one time split test assignment')
     return DEFAULT_ASSIGNMENT
     return DEFAULT_ASSIGNMENT
@@ -277,7 +278,7 @@ async function _getAssignment(
   if (Settings.devToolbar.enabled) {
   if (Settings.devToolbar.enabled) {
     const override = session?.splitTestOverrides?.[splitTestName]
     const override = session?.splitTestOverrides?.[splitTestName]
     if (override) {
     if (override) {
-      return _makeAssignment(splitTest, override, currentVersion)
+      return _makeAssignment({ variant: override, currentVersion })
     }
     }
   }
   }
 
 
@@ -308,7 +309,11 @@ async function _getAssignment(
       ) {
       ) {
         return DEFAULT_ASSIGNMENT
         return DEFAULT_ASSIGNMENT
       } else {
       } else {
-        return _makeAssignment(splitTest, cachedVariant, currentVersion)
+        return _makeAssignment({
+          variant: cachedVariant,
+          currentVersion,
+          isFirstNonDefaultAssignment: false,
+        })
       }
       }
     }
     }
   }
   }
@@ -325,8 +330,9 @@ async function _getAssignment(
     user ||
     user ||
     (userId &&
     (userId &&
       (await SplitTestUserGetter.promises.getUser(userId, splitTestName)))
       (await SplitTestUserGetter.promises.getUser(userId, splitTestName)))
-  const { activeForUser, selectedVariantName, phase, versionNumber } =
-    await _getAssignmentMetadata(analyticsId, user, splitTest)
+  const metadata = await _getAssignmentMetadata(analyticsId, user, splitTest)
+  const { activeForUser, selectedVariantName, phase, versionNumber } = metadata
+
   if (canUseSessionCache) {
   if (canUseSessionCache) {
     SplitTestSessionHandler.setVariantInCache({
     SplitTestSessionHandler.setVariantInCache({
       session,
       session,
@@ -336,6 +342,7 @@ async function _getAssignment(
       activeForUser,
       activeForUser,
     })
     })
   }
   }
+
   if (activeForUser) {
   if (activeForUser) {
     if (currentVersion.analyticsEnabled) {
     if (currentVersion.analyticsEnabled) {
       // if the user is logged in, persist the assignment
       // if the user is logged in, persist the assignment
@@ -396,7 +403,20 @@ async function _getAssignment(
         )
         )
       })
       })
     }
     }
-    return _makeAssignment(splitTest, selectedVariantName, currentVersion)
+    let isFirstNonDefaultAssignment
+    if (userId) {
+      isFirstNonDefaultAssignment = metadata.isFirstNonDefaultAssignment
+    } else {
+      const assignments =
+        await SplitTestSessionHandler.promises.getAssignments(session)
+      isFirstNonDefaultAssignment = !assignments?.[splitTestName]
+    }
+
+    return _makeAssignment({
+      variant: selectedVariantName,
+      currentVersion,
+      isFirstNonDefaultAssignment,
+    })
   }
   }
 
 
   return DEFAULT_ASSIGNMENT
   return DEFAULT_ASSIGNMENT
@@ -415,15 +435,21 @@ async function _getAssignmentMetadata(analyticsId, user, splitTest) {
   }
   }
   const userId = user?._id.toString()
   const userId = user?._id.toString()
   const percentile = getPercentile(analyticsId || userId, splitTest.name, phase)
   const percentile = getPercentile(analyticsId || userId, splitTest.name, phase)
-  const selectedVariantName = _getVariantFromPercentile(
-    currentVersion.variants,
-    percentile
-  )
+  const selectedVariantName =
+    _getVariantFromPercentile(currentVersion.variants, percentile) ||
+    DEFAULT_VARIANT
   return {
   return {
     activeForUser: true,
     activeForUser: true,
-    selectedVariantName: selectedVariantName || DEFAULT_VARIANT,
+    selectedVariantName,
     phase,
     phase,
     versionNumber: currentVersion.versionNumber,
     versionNumber: currentVersion.versionNumber,
+    isFirstNonDefaultAssignment:
+      selectedVariantName !== DEFAULT_VARIANT &&
+      currentVersion.analyticsEnabled &&
+      (!Array.isArray(user?.splitTests?.[splitTest.name]) ||
+        !user?.splitTests?.[splitTest.name]?.some(
+          assignment => assignment.variantName !== DEFAULT_VARIANT
+        )),
   }
   }
 }
 }
 
 
@@ -492,18 +518,17 @@ async function _recordAssignment({
   }
   }
 }
 }
 
 
-function _makeAssignment(splitTest, variant, currentVersion) {
+function _makeAssignment({
+  variant,
+  currentVersion,
+  isFirstNonDefaultAssignment,
+}) {
   return {
   return {
     variant,
     variant,
-    analytics: {
-      segmentation: splitTest
-        ? {
-            splitTest: splitTest.name,
-            variant,
-            phase: currentVersion.phase,
-            versionNumber: currentVersion.versionNumber,
-          }
-        : {},
+    metadata: {
+      phase: currentVersion.phase,
+      versionNumber: currentVersion.versionNumber,
+      isFirstNonDefaultAssignment,
     },
     },
   }
   }
 }
 }
@@ -543,9 +568,7 @@ function _getNonSaasAssignment(splitTestName) {
   if (Settings.splitTestOverrides?.[splitTestName]) {
   if (Settings.splitTestOverrides?.[splitTestName]) {
     return {
     return {
       variant: Settings.splitTestOverrides?.[splitTestName],
       variant: Settings.splitTestOverrides?.[splitTestName],
-      analytics: {
-        segmentation: {},
-      },
+      metadata: {},
     }
     }
   }
   }
   return DEFAULT_ASSIGNMENT
   return DEFAULT_ASSIGNMENT

+ 12 - 0
services/web/app/src/Features/SplitTests/types.d.ts

@@ -0,0 +1,12 @@
+type AssignmentMetadata = {
+  phase?: 'alpha' | 'beta' | 'release'
+  versionNumber?: boolean
+  // only returned when `analyticsEnabled` is set to `true` on the current version
+  // of the split test, and an assignment is queried for the user for the first time
+  isFirstNonDefaultAssignment?: boolean
+}
+
+export type Assignment = {
+  variant: string
+  metadata: AssignmentMetadata
+}

+ 5 - 0
services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -239,6 +239,11 @@ describe('ProjectController', function () {
         '../Survey/SurveyHandler': this.SurveyHandler,
         '../Survey/SurveyHandler': this.SurveyHandler,
         './ProjectAuditLogHandler': this.ProjectAuditLogHandler,
         './ProjectAuditLogHandler': this.ProjectAuditLogHandler,
         '../Tutorial/TutorialHandler': this.TutorialHandler,
         '../Tutorial/TutorialHandler': this.TutorialHandler,
+        '../User/UserUpdater': {
+          promises: {
+            updateUser: sinon.stub().resolves(),
+          },
+        },
       },
       },
     })
     })