remove enrollmentComplete experiments from pools unless already enrolled (#2670)

* remove enrollmentComplete experiments from pools unless already enrolled

* Update backend/packages/Upgrade/test/unit/services/ExperimentAssignmentService.test.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* no multiple assignment per decision point

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: bcb37

backend/packages/Upgrade/src/api/services/ExperimentAssignmentService.ts

@@ -388,7 +388,10 @@ export class ExperimentAssignmentService {
         filteredExperiments,
         mergedIndividualAssignment,
         groupEnrollments,
-        experimentUserDoc
+        individualExclusions,
+        groupExclusions,
+        experimentUserDoc,
+        previewUser
       );
 
       // return empty if no experiments
@@ -636,50 +639,59 @@ export class ExperimentAssignmentService {
     experiments: Experiment[],
     individualEnrollments: IndividualEnrollment[],
     groupEnrollments: GroupEnrollment[],
-    experimentUser: ExperimentUser
+    individualExclusions: IndividualExclusion[],
+    groupExclusions: GroupExclusion[],
+    experimentUser: ExperimentUser,
+    previewUser: PreviewUser | null
   ): Experiment[] {
     // Create experiment pool
     const experimentPools = this.createExperimentPool(experiments);
-
-    // Filter pools which are not assigned
-    const unassignedPools = experimentPools.filter((pool) => {
-      return !pool.some((experiment) => {
-        const hasIndividualEnrollment = individualEnrollments.some((enrollment) => {
-          return enrollment.experimentId === experiment.id;
-        });
-        const hasGroupEnrollment = groupEnrollments.some((enrollment) => {
-          return (
-            enrollment.experimentId === experiment.id &&
-            enrollment.groupId === experimentUser.workingGroup[experiment.group]
-          );
-        });
-        return !!(hasIndividualEnrollment || hasGroupEnrollment);
-      });
-    });
-
-    // Select experiments inside the pools
     const random = seedrandom(experimentUser.id)();
-    const newSelectedExperiments = unassignedPools.map((pool) => {
-      return pool[Math.floor(random * pool.length)];
-    });
 
-    // Create new filtered experiment list
-    const priorSelectedExperiments = experimentPools.map((pool) => {
-      return pool.filter((experiment) => {
-        const individualEnrollment = individualEnrollments.some((enrollment) => {
-          return enrollment.experimentId === experiment.id;
-        });
-        const groupEnrollment = groupEnrollments.some((enrollment) => {
-          return (
-            enrollment.experimentId === experiment.id &&
-            enrollment.groupId === experimentUser.workingGroup[experiment.group]
-          );
+    // Choose one experiment from each pool
+    const experimentsToAssign = experimentPools
+      .map((pool) => {
+        const assignedExperiments = pool.filter((experiment) => {
+          const hasIndividualEnrollment = individualEnrollments.some((enrollment) => {
+            return enrollment.experimentId === experiment.id;
+          });
+          const hasGroupEnrollment = groupEnrollments.some((enrollment) => {
+            return (
+              enrollment.experimentId === experiment.id &&
+              enrollment.groupId === experimentUser.workingGroup[experiment.group]
+            );
+          });
+          return hasIndividualEnrollment || hasGroupEnrollment;
         });
-        return !!(individualEnrollment || groupEnrollment);
-      });
-    });
+        // If there is an experiment already assigned to the user, choose that experiment
+        if (assignedExperiments.length === 1) {
+          return assignedExperiments[0];
+        }
+        // If there are multiple experiments assigned to the user, choose the ENROLLING experiment
+        if (assignedExperiments.length > 1) {
+          return assignedExperiments.find((exp) => exp.state === EXPERIMENT_STATE.ENROLLING);
+        }
 
-    return priorSelectedExperiments.flat().concat(newSelectedExperiments);
+        // Otherwise choose a random ENROLLING experiment from which the user is not excluded
+        const filteredUnassignedPool = pool.filter((experiment) => {
+          const hasIndividualExclusion = individualExclusions.some((exclusion) => {
+            return exclusion.experimentId === experiment.id;
+          });
+          const hasGroupExclusion = groupExclusions.some((exclusion) => {
+            return (
+              exclusion.experimentId === experiment.id &&
+              exclusion.groupId === experimentUser.workingGroup[experiment.group]
+            );
+          });
+          const includable =
+            experiment.state === EXPERIMENT_STATE.ENROLLING ||
+            (previewUser && experiment.state === EXPERIMENT_STATE.PREVIEW);
+          return !(hasIndividualExclusion || hasGroupExclusion || !includable);
+        });
+        return filteredUnassignedPool[Math.floor(random * filteredUnassignedPool.length)];
+      })
+      .filter((exp) => exp !== undefined);
+    return experimentsToAssign;
   }
 
   private mapDecisionPoints(

backend/packages/Upgrade/test/integration/ExperimentAssignment/RevertToCondition.ts

@@ -181,21 +181,9 @@ export default async function testCase(): Promise<void> {
   );
   checkMarkExperimentPointForUser(markedExperimentPoint, experimentUsers[2].id, experimentName, experimentPoint);
 
-  // get all experiment condition for user 4
+  // user 4 should not be assigned to any condition as he has not marked any experiment point
   experimentConditionAssignments = await getAllExperimentCondition(experimentUsers[3].id, new UpgradeLogger());
-  checkExperimentAssignedIsNotDefault(experimentConditionAssignments, experimentName, experimentPoint);
-  checkConditionAssigned(experimentConditionAssignments, experimentName, experimentPoint, experimentObject.revertTo);
-
-  // mark experiment point for user 4
-  markedExperimentPoint = await markExperimentPoint(
-    experimentUsers[3].id,
-    experimentName,
-    experimentPoint,
-    condition,
-    experimentId,
-    new UpgradeLogger()
-  );
-  checkMarkExperimentPointForUser(markedExperimentPoint, experimentUsers[3].id, experimentName, experimentPoint);
+  expect(experimentConditionAssignments).toHaveLength(0);
 }
 
 function checkConditionAssigned(

backend/packages/Upgrade/test/integration/ExperimentAssignment/Scenario2.ts

@@ -178,20 +178,9 @@ export default async function testCase(): Promise<void> {
   );
   checkMarkExperimentPointForUser(markedExperimentPoint, experimentUsers[2].id, experimentName, experimentPoint);
 
-  // get all experiment condition for user 4
+  // user 4 should not be assigned to any condition as he has not marked any experiment point
   experimentConditionAssignments = await getAllExperimentCondition(experimentUsers[3].id, new UpgradeLogger());
-  checkExperimentAssignedIsNotDefault(experimentConditionAssignments, experimentName, experimentPoint);
-
-  // mark experiment point for user 4
-  markedExperimentPoint = await markExperimentPoint(
-    experimentUsers[3].id,
-    experimentName,
-    experimentPoint,
-    condition,
-    experimentId,
-    new UpgradeLogger()
-  );
-  checkMarkExperimentPointForUser(markedExperimentPoint, experimentUsers[3].id, experimentName, experimentPoint);
+  expect(experimentConditionAssignments).toHaveLength(0);
 
   await checkDeletedExperiment(experimentId, user);
 }

backend/packages/Upgrade/test/integration/ExperimentStats/WithinSubjectEnrollment.ts

@@ -2,12 +2,7 @@ import { Container } from 'typedi';
 import { ExperimentService } from '../../../src/api/services/ExperimentService';
 import { UserService } from '../../../src/api/services/UserService';
 import { systemUser } from '../mockData/user/index';
-import {
-  checkExperimentAssignedIsNull,
-  getAllExperimentCondition,
-  markExperimentPoint,
-  updateExcludeIfReachedFlag,
-} from '../utils';
+import { checkExperimentAssignedIsNull, getAllExperimentCondition, markExperimentPoint } from '../utils';
 import { experimentUsers } from '../mockData/experimentUsers/index';
 import { AnalyticsService } from '../../../src/api/services/AnalyticsService';
 import { withinSubjectExperiment } from '../mockData/experiment/index';
@@ -30,7 +25,6 @@ export default async function testCase(): Promise<void> {
   const user = await userService.upsertUser(systemUser as any, new UpgradeLogger());
   // experiment object
   const experimentObject = withinSubjectExperiment;
-  experimentObject.partitions = updateExcludeIfReachedFlag(experimentObject.partitions);
 
   // create experiment
   await experimentService.create(