diff --git a/backend/package-lock.json b/backend/package-lock.json index 0f144d5eb1..534c817b40 100644 --- a/backend/package-lock.json +++ b/backend/package-lock.json @@ -1,12 +1,12 @@ { "name": "ab_testing_backend", - "version": "5.1.2", + "version": "5.1.3", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "ab_testing_backend", - "version": "5.1.2", + "version": "5.1.3", "license": "ISC", "dependencies": { "dayjs": "^1.11.10", diff --git a/backend/package.json b/backend/package.json index cc5f8e656b..a20811e625 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "ab_testing_backend", - "version": "5.1.2", + "version": "5.1.3", "description": "Backend for A/B Testing Project", "scripts": { "install:all": "npm ci && cd packages/Scheduler && npm ci && cd ../Upgrade && npm ci", diff --git a/backend/packages/Scheduler/package-lock.json b/backend/packages/Scheduler/package-lock.json index e5ae00c25f..25fb0c69de 100644 --- a/backend/packages/Scheduler/package-lock.json +++ b/backend/packages/Scheduler/package-lock.json @@ -1,12 +1,12 @@ { "name": "ppl-upgrade-serverless", - "version": "5.1.2", + "version": "5.1.3", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "ppl-upgrade-serverless", - "version": "5.1.2", + "version": "5.1.3", "license": "MIT", "dependencies": { "jsonwebtoken": "^9.0.0", diff --git a/backend/packages/Scheduler/package.json b/backend/packages/Scheduler/package.json index 5a2fac8b42..54befe6068 100644 --- a/backend/packages/Scheduler/package.json +++ b/backend/packages/Scheduler/package.json @@ -1,6 +1,6 @@ { "name": "ppl-upgrade-serverless", - "version": "5.1.2", + "version": "5.1.3", "description": "Serverless webpack example using Typescript", "main": "handler.js", "scripts": { diff --git a/backend/packages/Upgrade/package-lock.json b/backend/packages/Upgrade/package-lock.json index 6735eb0306..406f49c383 100644 --- a/backend/packages/Upgrade/package-lock.json +++ b/backend/packages/Upgrade/package-lock.json @@ -1,12 +1,12 @@ { "name": "ab_testing_backend", - "version": "5.1.2", + "version": "5.1.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "ab_testing_backend", - "version": "5.1.2", + "version": "5.1.3", "license": "ISC", "dependencies": { "@aws-sdk/client-s3": "^3.485.0", diff --git a/backend/packages/Upgrade/package.json b/backend/packages/Upgrade/package.json index 168084a30e..574f29158a 100644 --- a/backend/packages/Upgrade/package.json +++ b/backend/packages/Upgrade/package.json @@ -1,6 +1,6 @@ { "name": "ab_testing_backend", - "version": "5.1.2", + "version": "5.1.3", "description": "Backend for A/B Testing Project", "main": "index.js", "scripts": { diff --git a/backend/packages/Upgrade/src/api/repositories/GroupEnrollmentRepository.ts b/backend/packages/Upgrade/src/api/repositories/GroupEnrollmentRepository.ts index 8f08b8fff5..98dd67113e 100644 --- a/backend/packages/Upgrade/src/api/repositories/GroupEnrollmentRepository.ts +++ b/backend/packages/Upgrade/src/api/repositories/GroupEnrollmentRepository.ts @@ -1,5 +1,7 @@ import { EntityRepository, In, Repository } from 'typeorm'; import { GroupEnrollment } from '../models/GroupEnrollment'; +import repositoryError from './utils/repositoryError'; +import { UpgradeLogger } from 'src/lib/logger/UpgradeLogger'; @EntityRepository(GroupEnrollment) export class GroupEnrollmentRepository extends Repository { @@ -9,4 +11,19 @@ export class GroupEnrollmentRepository extends Repository { relations: ['experiment', 'condition'], }); } + + public async deleteGroupEnrollment(id: string, logger: UpgradeLogger): Promise { + const result = await this.createQueryBuilder() + .delete() + .from(GroupEnrollment) + .where('groupId=:id', { id }) + .execute() + .catch((errorMsg: any) => { + const errorMsgString = repositoryError('GroupEnrollmentRepository', 'deleteGroupEnrollment', { id }, errorMsg); + logger.error(errorMsg); + throw errorMsgString; + }); + + return result.raw; + } } diff --git a/backend/packages/Upgrade/src/api/services/CheckService.ts b/backend/packages/Upgrade/src/api/services/CheckService.ts index 6f5a489111..67000fc62f 100644 --- a/backend/packages/Upgrade/src/api/services/CheckService.ts +++ b/backend/packages/Upgrade/src/api/services/CheckService.ts @@ -18,7 +18,8 @@ export class CheckService { private groupEnrollmentRepository: GroupEnrollmentRepository, @InjectRepository() private individualEnrollmentRepository: IndividualEnrollmentRepository, - @InjectRepository() private groupExclusionRepository: GroupExclusionRepository, + @InjectRepository() + private groupExclusionRepository: GroupExclusionRepository, @InjectRepository() private individualExclusionRepository: IndividualExclusionRepository, @InjectRepository() diff --git a/backend/packages/Upgrade/src/api/services/ExperimentAssignmentService.ts b/backend/packages/Upgrade/src/api/services/ExperimentAssignmentService.ts index 26aca8cc81..0572c0d22e 100644 --- a/backend/packages/Upgrade/src/api/services/ExperimentAssignmentService.ts +++ b/backend/packages/Upgrade/src/api/services/ExperimentAssignmentService.ts @@ -71,16 +71,18 @@ import { UserStratificationFactor } from '../models/UserStratificationFactor'; @Service() export class ExperimentAssignmentService { constructor( - @InjectRepository() private experimentRepository: ExperimentRepository, + @InjectRepository() + private experimentRepository: ExperimentRepository, @InjectRepository() private decisionPointRepository: DecisionPointRepository, @InjectRepository() private individualExclusionRepository: IndividualExclusionRepository, - @InjectRepository() private groupExclusionRepository: GroupExclusionRepository, + @InjectRepository() + private groupExclusionRepository: GroupExclusionRepository, // @InjectRepository() // private groupAssignmentRepository: GroupAssignmentRepository, - @InjectRepository() private groupEnrollmentRepository: GroupEnrollmentRepository, @InjectRepository() + private groupEnrollmentRepository: GroupEnrollmentRepository, // @InjectRepository() // private individualAssignmentRepository: IndividualAssignmentRepository, @InjectRepository() @@ -143,7 +145,7 @@ export class ExperimentAssignmentService { const previewUser: PreviewUser = await this.previewUserService.findOne(userId, logger); - // search decision points in experiments cahce + // search decision points in experiments cache const cacheKey = CACHE_PREFIX.MARK_KEY_PREFIX + '-' + site + '-' + target; const dpExperiments = await this.cacheService.wrap(cacheKey, () => this.decisionPointRepository.find({ @@ -155,6 +157,7 @@ export class ExperimentAssignmentService { 'experiment', 'experiment.conditions', 'experiment.conditions.conditionPayloads', + 'experiment.partitions', 'experiment.experimentSegmentInclusion', 'experiment.experimentSegmentExclusion', 'experiment.experimentSegmentInclusion.segment', @@ -193,46 +196,8 @@ export class ExperimentAssignmentService { } } - // Experiment has assignment type as GROUP_ASSIGNMENT - const groupExperiments = experiments.filter(({ assignmentUnit }) => assignmentUnit === ASSIGNMENT_UNIT.GROUP); - // check for group and working group - const experimentUser = userDoc; - if (groupExperiments.length > 0) { - /** - * Check already assigned group experiment or exclude group experiment - * @param filteredGroupExperiments - * @param addError - */ - - if ( - !experimentUser.group || - !experimentUser.workingGroup || - Object.keys(experimentUser.workingGroup).length === 0 - ) { - const invalidGroupExperiment = await this.groupExperimentWithoutEnrollments( - groupExperiments, - experimentUser, - logger - ); - const invalidGroupExperimentIds = invalidGroupExperiment.map((experiment) => experiment.id); - experiments = experiments.filter(({ id }) => !invalidGroupExperimentIds.includes(id)); - } else { - const experimentWithInvalidGroupOrWorkingGroup = this.experimentsWithInvalidGroupAndWorkingGroup( - experimentUser, - groupExperiments - ); - const invalidGroupExperiment = await this.groupExperimentWithoutEnrollments( - experimentWithInvalidGroupOrWorkingGroup, - experimentUser, - logger - ); - const invalidGroupExperimentIds = invalidGroupExperiment.map((experiment) => experiment.id); - experiments = experiments.filter(({ id }) => !invalidGroupExperimentIds.includes(id)); - } - } - // ============= check if user or group is excluded - const [userExcluded, groupExcluded] = await this.checkUserOrGroupIsGloballyExcluded(experimentUser); + const [userExcluded, groupExcluded] = await this.checkUserOrGroupIsGloballyExcluded(userDoc, experiments[0]?.group); if (userExcluded || groupExcluded.length > 0) { // no experiments if the user or group is excluded from the experiment @@ -248,13 +213,10 @@ export class ExperimentAssignmentService { } // experiment level inclusion and exclusion - const [filteredExperiments, exclusionReason] = await this.experimentLevelExclusionInclusion( - globalFilteredExperiments, - experimentUser - ); - - if (filteredExperiments.length) { - let monitoredDocument: MonitoredDecisionPoint = await this.monitoredDecisionPointRepository.findOne({ + const [, exclusionReason] = await this.experimentLevelExclusionInclusion(globalFilteredExperiments, userDoc); + let monitoredDocument: MonitoredDecisionPoint; + if (experimentId && experiments.length) { + monitoredDocument = await this.monitoredDecisionPointRepository.findOne({ where: { site: site, target: target, @@ -262,11 +224,26 @@ export class ExperimentAssignmentService { }, relations: ['user'], }); + const selectedExperimentDP = dpExperiments.find((dp) => dp.experiment.id === experimentId); + const experiment = experiments[0]; + const { conditions } = experiment; + const payloadCondition = conditions.flatMap((condition) => condition.conditionPayloads); + + const matchedCondition = conditions.filter((dbCondition) => dbCondition.conditionCode === condition); + const matchedPayloadCondition = payloadCondition.filter((con) => con.payloadValue === condition); + if (matchedCondition.length === 0 && matchedPayloadCondition.length === 0 && condition !== null) { + const error = new Error(`Condition not found: ${condition}`); + (error as any).type = SERVER_ERROR.CONDITION_NOT_FOUND; + logger.error(error); + throw error; + } - if (experimentId && experiments.length > 0) { - const selectedExperimentDP = dpExperiments.find((dp) => dp.experiment.id === experimentId); + if ( + (experiment.state === EXPERIMENT_STATE.ENROLLING || + experiment.state === EXPERIMENT_STATE.ENROLLMENT_COMPLETE) && + !previewUser + ) { const decisionPointId = selectedExperimentDP.id; - const experiment = experiments[0]; let individualEnrollments: IndividualEnrollment; let individualExclusions: IndividualExclusion; let groupEnrollments: GroupEnrollment | undefined; @@ -307,96 +284,53 @@ export class ExperimentAssignmentService { logger.error(err); throw err; } - - const { conditions } = experiment; - const payloadCondition = conditions.flatMap((condition) => condition.conditionPayloads); - - const matchedCondition = conditions.filter((dbCondition) => dbCondition.conditionCode === condition); - const matchedPayloadCondition = payloadCondition.filter((con) => con.payloadValue === condition); - if (matchedCondition.length === 0 && matchedPayloadCondition.length === 0 && condition !== null) { - const error = new Error(`Condition not found: ${condition}`); - (error as any).type = SERVER_ERROR.CONDITION_NOT_FOUND; - logger.error(error); - throw error; - } - - if ( - (experiment.state === EXPERIMENT_STATE.ENROLLING || - experiment.state === EXPERIMENT_STATE.ENROLLMENT_COMPLETE) && - !previewUser - ) { - const experiment = await this.experimentService.findOne(experimentId); - await this.updateEnrollmentExclusion( - userDoc, - experiment, - selectedExperimentDP, - { - individualEnrollment: individualEnrollments, - individualExclusion: individualExclusions, - groupEnrollment: groupEnrollments, - groupExclusion: groupExclusions, - }, - { - user: userExcluded, - group: groupExcluded, - }, - exclusionReason, - status, - condition - ); - if (experiment.enrollmentCompleteCondition) { - await this.checkEnrollmentEndingCriteriaForCount(experiment, logger); - } + await this.updateEnrollmentExclusion( + userDoc, + experiment, + selectedExperimentDP, + { + individualEnrollment: individualEnrollments, + individualExclusion: individualExclusions, + groupEnrollment: groupEnrollments, + groupExclusion: groupExclusions, + }, + { + user: userExcluded, + group: groupExcluded, + }, + exclusionReason, + status, + condition + ); + if (experiment.enrollmentCompleteCondition) { + await this.checkEnrollmentEndingCriteriaForCount(experiment, logger); } } - // adding in monitored experiment point table - - const assignmentUnit = experiments - ? experiments.find((exp) => exp.id === experimentId)?.assignmentUnit || experiments[0]?.assignmentUnit - : null; - monitoredDocument = await this.monitoredDecisionPointRepository.saveRawJson({ - id: monitoredDocument?.id || uuid(), - experimentId: experimentId, - condition: assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS ? null : condition, - user: userDoc, - site: site, - target: target, - }); - - // save monitored log document - const logDocument = await this.monitoredDecisionPointLogRepository.save({ - monitoredDecisionPoint: monitoredDocument, - condition: condition, - uniquifier: uniquifier, - }); - return { - ...monitoredDocument, - condition: monitoredDocument.condition || logDocument.condition, - }; - } else { - const assignmentUnit = experiments - ? experiments.find((exp) => exp.id === experimentId)?.assignmentUnit || experiments[0]?.assignmentUnit - : null; - const monitoredDocument = await this.monitoredDecisionPointRepository.saveRawJson({ - id: uuid(), - experimentId: experimentId, - condition: assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS ? null : condition, - user: userDoc, - site: site, - target: target, - }); - - // save monitored log document - const logDocument = await this.monitoredDecisionPointLogRepository.save({ - monitoredDecisionPoint: monitoredDocument, - condition: assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS ? condition : null, - uniquifier: assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS ? uniquifier : null, - }); - return { - ...monitoredDocument, - condition: monitoredDocument.condition || logDocument.condition, - }; } + // adding in monitored experiment point table + + const assignmentUnit = experiments + ? experiments.find((exp) => exp.id === experimentId)?.assignmentUnit || experiments[0]?.assignmentUnit + : null; + monitoredDocument = await this.monitoredDecisionPointRepository.saveRawJson({ + id: monitoredDocument?.id || uuid(), + experimentId: experimentId, + condition: assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS ? null : condition, + user: userDoc, + site: site, + target: target, + }); + + // save monitored log document + const logDocument = await this.monitoredDecisionPointLogRepository.save({ + monitoredDecisionPoint: monitoredDocument, + condition: assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS ? condition : null, + uniquifier: assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS ? uniquifier : null, + }); + return { + ...monitoredDocument, + condition: monitoredDocument.condition || logDocument.condition, + }; } public async getAllExperimentConditions( @@ -438,13 +372,6 @@ export class ExperimentAssignmentService { // check if user or group is excluded let experiments: Experiment[] = []; - const [userExcluded, groupExcluded] = await this.checkUserOrGroupIsGloballyExcluded(experimentUser); - - if (userExcluded || groupExcluded.length > 0) { - // return null if the user or group is excluded from the experiment - return []; - } - if (previewUser) { experiments = await this.experimentRepository.getValidExperimentsWithPreview(context); } else { @@ -452,41 +379,37 @@ export class ExperimentAssignmentService { } experiments = experiments.map((exp) => this.experimentService.formatingConditionPayload(exp)); + const [userExcluded, groupExcluded] = await this.checkUserOrGroupIsGloballyExcluded(experimentUser, experiments[0]?.group); + + if (userExcluded || groupExcluded.length > 0) { + // return null if the user or group is excluded from the experiment + return []; + } + // Experiment has assignment type as GROUP_ASSIGNMENT const groupExperiments = experiments.filter(({ assignmentUnit }) => assignmentUnit === ASSIGNMENT_UNIT.GROUP); // check for group and working group if (groupExperiments.length > 0) { - /** - * Check already assigned group experiment or exclude group experiment - * @param filteredGroupExperiments - * @param addError - */ - - if ( - !experimentUser.group || - !experimentUser.workingGroup || - Object.keys(experimentUser.workingGroup).length === 0 - ) { - const invalidGroupExperiment = await this.groupExperimentWithoutEnrollments( - groupExperiments, - experimentUser, - logger - ); - const invalidGroupExperimentIds = invalidGroupExperiment.map((experiment) => experiment.id); - experiments = experiments.filter(({ id }) => !invalidGroupExperimentIds.includes(id)); - } else { - const experimentWithInvalidGroupOrWorkingGroup = this.experimentsWithInvalidGroupAndWorkingGroup( + // if empty group/workingGroup data: + let invalidGroupExperiment: Experiment[] = []; + let experimentWithInvalidGroupOrWorkingGroup: Experiment[] = []; + let isGroupWorkingGroupMissing = !experimentUser.group || !experimentUser.workingGroup || + (experimentUser.group && Object.keys(experimentUser.group).length === 0) || (experimentUser.workingGroup && Object.keys(experimentUser.workingGroup).length === 0); + + if (!isGroupWorkingGroupMissing) { + // if group/workingGroup data present, check for invalid group/workingGroup: + experimentWithInvalidGroupOrWorkingGroup = this.experimentsWithInvalidGroupAndWorkingGroup( experimentUser, groupExperiments ); - const invalidGroupExperiment = await this.groupExperimentWithoutEnrollments( - experimentWithInvalidGroupOrWorkingGroup, - experimentUser, - logger - ); - const invalidGroupExperimentIds = invalidGroupExperiment.map((experiment) => experiment.id); - experiments = experiments.filter(({ id }) => !invalidGroupExperimentIds.includes(id)); } + invalidGroupExperiment = await this.groupExperimentWithoutEnrollments( + isGroupWorkingGroupMissing ? groupExperiments : experimentWithInvalidGroupOrWorkingGroup, + experimentUser, + logger + ); + const invalidGroupExperimentIds = invalidGroupExperiment.map((experiment) => experiment.id); + experiments = experiments.filter(({ id }) => !invalidGroupExperimentIds.includes(id)); } // try catch block for experiment assignment error @@ -769,7 +692,7 @@ export class ExperimentAssignmentService { return pool; } - private async checkUserOrGroupIsGloballyExcluded(experimentUser: ExperimentUser): Promise<[string, string[]]> { + private async checkUserOrGroupIsGloballyExcluded(experimentUser: ExperimentUser, experimentGroup: string): Promise<[string, string[]]> { let userGroup = []; userGroup = Object.keys(experimentUser.workingGroup || {}).map((type: string) => { return `${type}_${experimentUser.workingGroup[type]}`; @@ -797,7 +720,7 @@ export class ExperimentAssignmentService { updatedGlobalExcludeSegmentObj[globalExcludeSegment.id].allExcludedSegmentIds.forEach((segmentId) => { const foundSegment: Segment = updatedSegmentDetails.find((segment) => segment.id === segmentId); excludedUsers.push(...foundSegment.individualForSegment.map((individual) => individual.userId)); - excludedGroups.push(...foundSegment.groupForSegment.map((group) => `${group.type}_${group.groupId}`)); + excludedGroups.push(...foundSegment.groupForSegment.filter((group) => group.type === experimentGroup ? `${group.type}_${group.groupId}` : false)); }); //users and groups excluded from GlobalExclude segment @@ -1355,15 +1278,15 @@ export class ExperimentAssignmentService { groupEnrollment: GroupEnrollment; groupExclusion: GroupExclusion; }, - gloabllyExcluded: { user: string; group: string[] }, - experimentLevelExclused: { experiment: Experiment; reason: string }[], + globallyExcluded: { user: string; group: string[] }, + experimentLevelExcluded: { experiment: Experiment; reason: string, matchedGroup: boolean }[], status: MARKED_DECISION_POINT_STATUS, condition: string ): Promise { const { assignmentUnit, state, consistencyRule } = experiment; // experiment level exclusion let experimentExcluded = false; - if (experimentLevelExclused.length > 0) { + if (experimentLevelExcluded.length > 0) { experimentExcluded = true; } @@ -1379,46 +1302,45 @@ export class ExperimentAssignmentService { // Don't mark the experiment if user or group are in exclusion list // TODO update this with segment implementation - if (gloabllyExcluded.user) { - const excludeUserDoc: Pick = { - user, - experiment, - exclusionCode: EXCLUSION_CODE.PARTICIPANT_ON_EXCLUSION_LIST, - }; - await this.individualExclusionRepository.saveRawJson([excludeUserDoc]); - return; - } else if (gloabllyExcluded.group.length > 0) { - const excludeGroupDoc: Pick = { - groupId: user?.workingGroup[experiment.group], - experiment, - exclusionCode: EXCLUSION_CODE.GROUP_ON_EXCLUSION_LIST, - }; - await this.groupExclusionRepository.saveRawJson([excludeGroupDoc]); - return; - } else if (experimentExcluded) { - // TODO: testing this - if (experimentLevelExclused[0].reason === 'user') { - const excludeUserDoc: Pick = { - user, - experiment, - exclusionCode: EXCLUSION_CODE.PARTICIPANT_ON_EXCLUSION_LIST, - }; - await this.individualExclusionRepository.saveRawJson([excludeUserDoc]); - } else if (experimentLevelExclused[0].reason === 'group') { + // Create the excludeUserDoc outside of the conditional statements to avoid repetition + const excludeUserDoc: Pick = { + user, + experiment, + exclusionCode: EXCLUSION_CODE.PARTICIPANT_ON_EXCLUSION_LIST, + }; + if (globallyExcluded.user || globallyExcluded.group.length) { + // store Individual exclusion document for the Group Exclusion as well: + const promiseArray = []; + promiseArray.push(this.individualExclusionRepository.saveRawJson([excludeUserDoc])); + if (globallyExcluded.group.length) { + // store Group exclusion document: const excludeGroupDoc: Pick = { groupId: user?.workingGroup[experiment.group], experiment, exclusionCode: EXCLUSION_CODE.GROUP_ON_EXCLUSION_LIST, }; - await this.groupExclusionRepository.saveRawJson([excludeGroupDoc]); - } else { - const excludeUserDoc: Pick = { - user, + promiseArray.push(this.groupExclusionRepository.saveRawJson([excludeGroupDoc])); + // check if excluded group was earlier included, if yes - remove them: + promiseArray.push(this.groupEnrollmentRepository.deleteGroupEnrollment(user?.workingGroup[experiment.group], new UpgradeLogger())); + } + await Promise.all(promiseArray); + return; + } else if (experimentExcluded) { + // store Individual exclusion document with the Group Exclusion document: + const promiseArray = []; + promiseArray.push(this.individualExclusionRepository.saveRawJson([excludeUserDoc])); + if (experimentLevelExcluded[0].reason === 'group' && experimentLevelExcluded[0].matchedGroup) { + // store Group exclusion document: + const excludeGroupDoc: Pick = { + groupId: user?.workingGroup[experiment.group], experiment, - exclusionCode: EXCLUSION_CODE.PARTICIPANT_ON_EXCLUSION_LIST, + exclusionCode: EXCLUSION_CODE.GROUP_ON_EXCLUSION_LIST, }; - await this.individualExclusionRepository.saveRawJson([excludeUserDoc]); + promiseArray.push(this.groupExclusionRepository.saveRawJson([excludeGroupDoc])); + // check if excluded group was earlier included, if yes - remove them: + promiseArray.push(this.groupEnrollmentRepository.deleteGroupEnrollment(user?.workingGroup[experiment.group], new UpgradeLogger())); } + await Promise.all(promiseArray); return; } @@ -1426,7 +1348,13 @@ export class ExperimentAssignmentService { let noGroupSpecified = false; let invalidGroup = false; if (assignmentUnit === ASSIGNMENT_UNIT.GROUP) { - if (!user.group || !user.workingGroup || Object.keys(user.workingGroup).length === 0) { + // if group or workingGroup is not provided or if group or workingGroup is an empty array: + if ( + !user.group || + !user.workingGroup || + (user.group && Object.keys(user.group).length === 0) || + (user.workingGroup && Object.keys(user.workingGroup).length === 0) + ) { noGroupSpecified = true; } else { const experimentWithInvalidGroupOrWorkingGroup = this.experimentsWithInvalidGroupAndWorkingGroup(user, [ @@ -1549,11 +1477,11 @@ export class ExperimentAssignmentService { promiseArray.push(this.individualExclusionRepository.saveRawJson([individualExclusionDocument])); } await Promise.all(promiseArray); - } else if (experiment.assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS) { + } else if (experiment.assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS && !individualEnrollment) { const individualEnrollmentDocument: Omit = { id: uuid(), experiment, - partition: decisionPoint, + partition: decisionPoint as DecisionPoint, user, condition: null, enrollmentCode: ENROLLMENT_CODE.ALGORITHMIC, @@ -1573,7 +1501,7 @@ export class ExperimentAssignmentService { { id: uuid(), experiment, - partition: decisionPoint, + partition: decisionPoint as DecisionPoint, user, condition: conditionAssigned, enrollmentCode: ENROLLMENT_CODE.ALGORITHMIC, @@ -1756,7 +1684,7 @@ export class ExperimentAssignmentService { private async experimentLevelExclusionInclusion( experiments: Experiment[], experimentUser: ExperimentUser - ): Promise<[Experiment[], { experiment: Experiment; reason: string }[]]> { + ): Promise<[Experiment[], { experiment: Experiment; reason: string, matchedGroup: boolean }[]]> { let segmentDetails: Segment[] = []; let segmentObj = {}; @@ -1836,7 +1764,7 @@ export class ExperimentAssignmentService { }); } - // psuedocode for experiment level inclusion and exclusion + // pseudocode for experiment level inclusion and exclusion // // If the user or the user's group is on the global exclude list, exclude the user. // @@ -1859,6 +1787,7 @@ export class ExperimentAssignmentService { experiments.forEach((experiment) => { let inclusionFlag = false; let exclusionFlag = false; + let sameGroupExclusionFlag = false; if (experiment.filterMode === FILTER_MODE.INCLUDE_ALL) { if (explicitExperimentIndividualExclusionFilteredData.some((e) => e.experimentId === experiment.id)) { @@ -1867,18 +1796,27 @@ export class ExperimentAssignmentService { includedExperiments.push(experiment); } else { for (const userGroup of userGroups) { - if ( - explicitExperimentGroupExclusionFilteredData.some( - (e) => e.groupId === userGroup.groupId && e.type === userGroup.type && e.experimentId === experiment.id - ) - ) { + const matchingExclusionData = explicitExperimentGroupExclusionFilteredData.find( + (e) => e.groupId === userGroup.groupId && e.type === userGroup.type && e.experimentId === experiment.id + ); + + if (matchingExclusionData) { inclusionFlag = true; + + if (matchingExclusionData.type === experiment.group) { + sameGroupExclusionFlag = true; + } } } if (!inclusionFlag) { includedExperiments.push(experiment); } else { - excludedExperiments.push({ experiment: experiment, reason: 'group' }); + const matchedGroup = sameGroupExclusionFlag; + excludedExperiments.push({ + experiment, + reason: 'group', + matchedGroup, + }); } } } else { diff --git a/backend/packages/Upgrade/src/api/services/ExperimentService.ts b/backend/packages/Upgrade/src/api/services/ExperimentService.ts index 7c95ac73e4..894a1684ac 100644 --- a/backend/packages/Upgrade/src/api/services/ExperimentService.ts +++ b/backend/packages/Upgrade/src/api/services/ExperimentService.ts @@ -28,6 +28,7 @@ import { SEGMENT_TYPE, EXPERIMENT_TYPE, CACHE_PREFIX, + ASSIGNMENT_UNIT, } from 'upgrade_types'; import { IndividualExclusionRepository } from '../repositories/IndividualExclusionRepository'; import { GroupExclusionRepository } from '../repositories/GroupExclusionRepository'; @@ -681,7 +682,11 @@ export class ExperimentService { await this.groupExclusionRepository.saveRawJson(groupExclusionDocs); } - if (consistencyRule === CONSISTENCY_RULE.INDIVIDUAL || consistencyRule === CONSISTENCY_RULE.GROUP) { + if ( + consistencyRule === CONSISTENCY_RULE.INDIVIDUAL || + consistencyRule === CONSISTENCY_RULE.GROUP || + experiment.assignmentUnit === ASSIGNMENT_UNIT.WITHIN_SUBJECTS + ) { // individual exclusion document const individualExclusionDocs: Array< Omit diff --git a/backend/packages/Upgrade/test/unit/services/ExperimentAssignmentService.test.ts b/backend/packages/Upgrade/test/unit/services/ExperimentAssignmentService.test.ts index ef133ec9e8..d0409ea461 100644 --- a/backend/packages/Upgrade/test/unit/services/ExperimentAssignmentService.test.ts +++ b/backend/packages/Upgrade/test/unit/services/ExperimentAssignmentService.test.ts @@ -440,7 +440,7 @@ describe('Experiment Assignment Service Test', () => { const condition = 'testCondition'; const clientError = 'clientError'; const loggerMock = { info: sandbox.stub(), error: sandbox.stub() }; - const decisionPointRespositoryMock = { find: sandbox.stub().resolves([]) }; + const decisionPointRepositoryMock = { find: sandbox.stub().resolves([]) }; const individualEnrollmentRepositoryMock = { findEnrollments: sandbox.stub().resolves([]) }; const individualExclusionRepositoryMock = { findExcluded: sandbox.stub().resolves([]) }; const groupEnrollmentRepositoryMock = { findEnrollments: sandbox.stub().resolves([]) }; @@ -451,7 +451,7 @@ describe('Experiment Assignment Service Test', () => { }), }; - testedModule.decisionPointRepository = decisionPointRespositoryMock; + testedModule.decisionPointRepository = decisionPointRepositoryMock; testedModule.experimentService = experimentServiceMock; testedModule.experimentService.getCachedValidExperiments = sandbox.stub().resolves([]); testedModule.individualEnrollmentRepository = individualEnrollmentRepositoryMock; @@ -485,7 +485,7 @@ describe('Experiment Assignment Service Test', () => { const target = 'testTarget'; const condition = 'testCondition'; const loggerMock = { info: sandbox.stub(), error: sandbox.stub() }; - const decisionPointRespositoryMock = { find: sandbox.stub().resolves([]) }; + const decisionPointRepositoryMock = { find: sandbox.stub().resolves([]) }; const individualEnrollmentRepositoryMock = { findEnrollments: sandbox.stub().resolves([]) }; const individualExclusionRepositoryMock = { findExcluded: sandbox.stub().resolves([]) }; const groupEnrollmentRepositoryMock = { findEnrollments: sandbox.stub().resolves([]) }; @@ -496,7 +496,7 @@ describe('Experiment Assignment Service Test', () => { }), }; - testedModule.decisionPointRepository = decisionPointRespositoryMock; + testedModule.decisionPointRepository = decisionPointRepositoryMock; testedModule.experimentService = experimentServiceMock; testedModule.cacheService.wrap = sandbox.stub().resolves([]); testedModule.individualEnrollmentRepository = individualEnrollmentRepositoryMock; @@ -528,7 +528,7 @@ describe('Experiment Assignment Service Test', () => { const target = 'testTarget'; const condition = 'testCondition'; const loggerMock = { info: sandbox.stub(), error: sandbox.stub() }; - const decisionPointRespositoryMock = { find: sandbox.stub().resolves([simpleDPExperiment]) }; + const decisionPointRepositoryMock = { find: sandbox.stub().resolves([simpleDPExperiment]) }; const individualEnrollmentRepositoryMock = { findEnrollments: sandbox.stub().resolves([]) }; const individualExclusionRepositoryMock = { findExcluded: sandbox.stub().resolves([]) }; const groupEnrollmentRepositoryMock = { findEnrollments: sandbox.stub().resolves([]) }; @@ -548,7 +548,7 @@ describe('Experiment Assignment Service Test', () => { findOne: sandbox.stub().resolves(monitoredDocument), }; - testedModule.decisionPointRepository = decisionPointRespositoryMock; + testedModule.decisionPointRepository = decisionPointRepositoryMock; testedModule.experimentService = experimentServiceMock; testedModule.experimentService.getCachedValidExperiments = sandbox .stub() @@ -578,7 +578,7 @@ describe('Experiment Assignment Service Test', () => { const target = 'testTarget'; const condition = 'testCondition'; const loggerMock = { info: sandbox.stub(), error: sandbox.stub() }; - const decisionPointRespositoryMock = { find: sandbox.stub().resolves([simpleDPExperiment]) }; + const decisionPointRepositoryMock = { find: sandbox.stub().resolves([simpleDPExperiment]) }; const individualEnrollmentRepositoryMock = { findEnrollments: sandbox.stub().resolves([]) }; const individualExclusionRepositoryMock = { findExcluded: sandbox.stub().resolves([]) }; const groupEnrollmentRepositoryMock = { findEnrollments: sandbox.stub().resolves([]) }; @@ -598,7 +598,7 @@ describe('Experiment Assignment Service Test', () => { findOne: sandbox.stub().resolves(monitoredDocument), }; - testedModule.decisionPointRepository = decisionPointRespositoryMock; + testedModule.decisionPointRepository = decisionPointRepositoryMock; testedModule.experimentService = experimentServiceMock; testedModule.experimentService.getCachedValidExperiments = sandbox .stub() @@ -628,7 +628,7 @@ describe('Experiment Assignment Service Test', () => { const target = 'testTarget'; const condition = 'testCondition'; const loggerMock = { info: sandbox.stub(), error: sandbox.stub() }; - const decisionPointRespositoryMock = { find: sandbox.stub().resolves([withinSubjectDPExperiment]) }; + const decisionPointRepositoryMock = { find: sandbox.stub().resolves([withinSubjectDPExperiment]) }; const individualEnrollmentRepositoryMock = { findEnrollments: sandbox.stub().resolves([]) }; const individualExclusionRepositoryMock = { findExcluded: sandbox.stub().resolves([]) }; const groupEnrollmentRepositoryMock = { findEnrollments: sandbox.stub().resolves([]) }; @@ -648,7 +648,7 @@ describe('Experiment Assignment Service Test', () => { findOne: sandbox.stub().resolves(monitoredDocument), }; - testedModule.decisionPointRepository = decisionPointRespositoryMock; + testedModule.decisionPointRepository = decisionPointRepositoryMock; testedModule.experimentService = experimentServiceMock; testedModule.experimentService.getCachedValidExperiments = sandbox .stub() diff --git a/clientlibs/java/pom.xml b/clientlibs/java/pom.xml index 791596795d..9ee6a4add5 100644 --- a/clientlibs/java/pom.xml +++ b/clientlibs/java/pom.xml @@ -9,7 +9,7 @@ at the same time that happen to rev to the same new version will be caught by a merge conflict. --> - 5.1.2 + 5.1.3 diff --git a/clientlibs/js/package-lock.json b/clientlibs/js/package-lock.json index c15ce10db1..21c0bec54a 100644 --- a/clientlibs/js/package-lock.json +++ b/clientlibs/js/package-lock.json @@ -1,12 +1,12 @@ { "name": "upgrade_client_lib", - "version": "5.1.2", + "version": "5.1.3", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "upgrade_client_lib", - "version": "5.1.2", + "version": "5.1.3", "license": "ISC", "dependencies": { "axios": "^1.4.0", diff --git a/clientlibs/js/package.json b/clientlibs/js/package.json index 70fcfb8c56..9062b70e1c 100644 --- a/clientlibs/js/package.json +++ b/clientlibs/js/package.json @@ -1,6 +1,6 @@ { "name": "upgrade_client_lib", - "version": "5.1.2", + "version": "5.1.3", "description": "Client library to communicate with the Upgrade server", "files": [ "dist/*" diff --git a/frontend/package-lock.json b/frontend/package-lock.json index ed05f47a79..ee1fc64a89 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -1,12 +1,12 @@ { "name": "ab-testing", - "version": "5.1.2", + "version": "5.1.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "ab-testing", - "version": "5.1.2", + "version": "5.1.3", "license": "MIT", "dependencies": { "@angular/animations": "^17.1.2", diff --git a/frontend/package.json b/frontend/package.json index fbec666e54..0f195568c4 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "ab-testing", - "version": "5.1.2", + "version": "5.1.3", "license": "MIT", "scripts": { "ng": "ng", diff --git a/package-lock.json b/package-lock.json index 9f26286e3b..6db347ff3a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "UpGrade", - "version": "5.1.2", + "version": "5.1.3", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "UpGrade", - "version": "5.1.2", + "version": "5.1.3", "license": "ISC", "devDependencies": { "@angular-eslint/eslint-plugin": "^14.1.2", diff --git a/package.json b/package.json index 0217d48ccf..878fbc4db6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "UpGrade", - "version": "5.1.2", + "version": "5.1.3", "description": "This is a combined repository for UpGrade, an open-source platform to support large-scale A/B testing in educational applications. Learn more at www.upgradeplatform.org", "main": "index.js", "devDependencies": { diff --git a/types/package-lock.json b/types/package-lock.json index 431ca679e7..6fb904626b 100644 --- a/types/package-lock.json +++ b/types/package-lock.json @@ -1,12 +1,12 @@ { "name": "upgrade_types", - "version": "5.1.2", + "version": "5.1.3", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "upgrade_types", - "version": "5.1.2", + "version": "5.1.3", "license": "ISC", "devDependencies": { "eslint": "^8.27.0", diff --git a/types/package.json b/types/package.json index 61bf263793..a22692f1a0 100644 --- a/types/package.json +++ b/types/package.json @@ -1,6 +1,6 @@ { "name": "upgrade_types", - "version": "5.1.2", + "version": "5.1.3", "description": "", "main": "src/index.ts", "types": "src/index.ts", diff --git a/types/src/Experiment/enums.ts b/types/src/Experiment/enums.ts index 169611b65a..aac835cac6 100644 --- a/types/src/Experiment/enums.ts +++ b/types/src/Experiment/enums.ts @@ -72,15 +72,20 @@ export enum ENROLLMENT_CODE { } export enum EXCLUSION_CODE { - ERROR = 'participant excluded due to unspecified error', + // individual level: REACHED_PRIOR = 'participant reached experiment prior to experiment enrolling', REACHED_AFTER = 'participant reached experiment during enrollment complete', + // experiment level: PARTICIPANT_ON_EXCLUSION_LIST = 'participant was on the exclusion list', GROUP_ON_EXCLUSION_LIST = 'participant’s group was on the exclusion list', + // group level: EXCLUDED_DUE_TO_GROUP_LOGIC = 'participant excluded due to group assignment logic', NO_GROUP_SPECIFIED = 'participant excluded due to incomplete group information', INVALID_GROUP_OR_WORKING_GROUP = "participant's group or working group is incorrect", + // triggered by client SDK: EXCLUDED_BY_CLIENT = 'participant is excluded by client', + // generic error (for future use): + ERROR = 'participant excluded due to unspecified error', } export enum EXPERIMENT_LOG_TYPE {