From ab68afe3f85611eab6be21ecb32828d21a035a2c Mon Sep 17 00:00:00 2001 From: Ridham Shah <49234788+RidhamShah@users.noreply.github.com> Date: Fri, 15 Sep 2023 00:36:08 +0530 Subject: [PATCH] Fix/issue-1000 Updated mark call related changes (#1011) * Solve mark and assignment issue * add v4 validator changes as well * remove unused import * remove payload from mark validator v5 also --------- Co-authored-by: RidhamShah Co-authored-by: danoswaltCL <97542869+danoswaltCL@users.noreply.github.com> --- .../ExperimentClientController.v4.ts | 4 +- .../ExperimentClientController.v5.ts | 2 - .../validators/MarkExperimentValidator.v4.ts | 45 +++++++++---------- .../validators/MarkExperimentValidator.v5.ts | 45 +++++++++---------- clientlibs/js/src/common/index.ts | 28 +++++++++--- .../functions/getDecisionPointAssignment.ts | 18 +++----- .../js/src/functions/markDecisionPoint.ts | 16 ++++--- 7 files changed, 80 insertions(+), 78 deletions(-) diff --git a/backend/packages/Upgrade/src/api/controllers/ExperimentClientController.v4.ts b/backend/packages/Upgrade/src/api/controllers/ExperimentClientController.v4.ts index 0d6002f6cb..d294ead1e5 100644 --- a/backend/packages/Upgrade/src/api/controllers/ExperimentClientController.v4.ts +++ b/backend/packages/Upgrade/src/api/controllers/ExperimentClientController.v4.ts @@ -23,7 +23,6 @@ import { IUserAliases, IWorkingGroup, PAYLOAD_TYPE, - EXPERIMENT_TYPE, IPayload, } from 'upgrade_types'; import { FeatureFlag } from '../models/FeatureFlag'; @@ -528,7 +527,6 @@ export class ExperimentClientController { } return { ...rest, - experimentType: assignedFactor ? EXPERIMENT_TYPE.FACTORIAL : EXPERIMENT_TYPE.SIMPLE, assignedCondition: { id: assignedCondition[0].id, conditionCode: assignedCondition[0].conditionCode, @@ -663,7 +661,7 @@ export class ExperimentClientController { @Req() request: AppRequest ): Promise { - let result = envelope.data.map(async log => { + const result = envelope.data.map(async (log) => { // getOriginalUserDoc call for alias const experimentUserDoc = await this.getUserDoc(log.object.assignee.id, request.logger); if (experimentUserDoc) { diff --git a/backend/packages/Upgrade/src/api/controllers/ExperimentClientController.v5.ts b/backend/packages/Upgrade/src/api/controllers/ExperimentClientController.v5.ts index b05476d33f..3427578d50 100644 --- a/backend/packages/Upgrade/src/api/controllers/ExperimentClientController.v5.ts +++ b/backend/packages/Upgrade/src/api/controllers/ExperimentClientController.v5.ts @@ -22,7 +22,6 @@ import { IGroupMembership, IUserAliases, IWorkingGroup, - EXPERIMENT_TYPE, PAYLOAD_TYPE, IPayload, } from 'upgrade_types'; @@ -540,7 +539,6 @@ export class ExperimentClientController { }); return { ...rest, - experimentType: assignedFactor ? EXPERIMENT_TYPE.FACTORIAL : EXPERIMENT_TYPE.SIMPLE, assignedCondition: finalConditionData, assignedFactor: assignedFactor ? finalFactorData : undefined, }; diff --git a/backend/packages/Upgrade/src/api/controllers/validators/MarkExperimentValidator.v4.ts b/backend/packages/Upgrade/src/api/controllers/validators/MarkExperimentValidator.v4.ts index d3b558a816..1b60cc731d 100644 --- a/backend/packages/Upgrade/src/api/controllers/validators/MarkExperimentValidator.v4.ts +++ b/backend/packages/Upgrade/src/api/controllers/validators/MarkExperimentValidator.v4.ts @@ -1,6 +1,17 @@ import { Type } from 'class-transformer'; -import { IsNotEmpty, IsDefined, IsString, IsOptional, IsEnum, IsObject, ValidateIf, ValidateNested, ValidationOptions, registerDecorator } from 'class-validator'; -import { EXPERIMENT_TYPE, MARKED_DECISION_POINT_STATUS, PAYLOAD_TYPE } from 'upgrade_types'; +import { + IsNotEmpty, + IsDefined, + IsString, + IsOptional, + IsEnum, + IsObject, + ValidateIf, + ValidateNested, + ValidationOptions, + registerDecorator, +} from 'class-validator'; +import { MARKED_DECISION_POINT_STATUS, PAYLOAD_TYPE } from 'upgrade_types'; const IsAssignedFactorRecord = (validationOptions?: ValidationOptions) => { return function (object: unknown, propertyName: string) { @@ -15,16 +26,14 @@ const IsAssignedFactorRecord = (validationOptions?: ValidationOptions) => { }, validator: { validate(value: any) { - return validateAssignedFactorData(value) + return validateAssignedFactorData(value); }, }, }); }; }; -function validateAssignedFactorData( - data: any -): boolean { +function validateAssignedFactorData(data: any): boolean { const keys = Object.keys(data); for (const key of keys) { const factor = data[key]; @@ -45,9 +54,7 @@ function isValidAssignedFactor(value: any): value is AssignedFactor { function isValidPayload(value: any): value is Payload { return ( - typeof value === 'object' && - Object.values(PAYLOAD_TYPE).includes(value.type) && - typeof value.value === 'string' + typeof value === 'object' && Object.values(PAYLOAD_TYPE).includes(value.type) && typeof value.value === 'string' ); } class Payload { @@ -71,20 +78,13 @@ class AssignedFactor { } class AssignedCondition { - @IsNotEmpty() + @IsOptional() @IsString() - id: string; + id?: string; - @IsDefined() - @IsNotEmpty() + @IsOptional() @IsString() - conditionCode: string; - - @IsObject() - @ValidateNested() - @ValidateIf((object, value) => value !== null) - @Type(() => Payload) - payload: Payload | null; + conditionCode?: string; @IsOptional() @IsString() @@ -100,11 +100,6 @@ class Data { @IsString() target: string; - @IsEnum(EXPERIMENT_TYPE) - @IsDefined() - @IsNotEmpty() - experimentType: EXPERIMENT_TYPE; - @IsOptional() @ValidateNested() @Type(() => AssignedCondition) diff --git a/backend/packages/Upgrade/src/api/controllers/validators/MarkExperimentValidator.v5.ts b/backend/packages/Upgrade/src/api/controllers/validators/MarkExperimentValidator.v5.ts index 8b7dc5c013..2cb81e9a99 100644 --- a/backend/packages/Upgrade/src/api/controllers/validators/MarkExperimentValidator.v5.ts +++ b/backend/packages/Upgrade/src/api/controllers/validators/MarkExperimentValidator.v5.ts @@ -1,6 +1,17 @@ import { Type } from 'class-transformer'; -import { IsNotEmpty, IsDefined, IsString, IsOptional, IsEnum, IsObject, ValidateNested, ValidateIf, ValidationOptions, registerDecorator } from 'class-validator'; -import { EXPERIMENT_TYPE, MARKED_DECISION_POINT_STATUS, PAYLOAD_TYPE } from 'upgrade_types'; +import { + IsNotEmpty, + IsDefined, + IsString, + IsOptional, + IsEnum, + IsObject, + ValidateNested, + ValidateIf, + ValidationOptions, + registerDecorator, +} from 'class-validator'; +import { MARKED_DECISION_POINT_STATUS, PAYLOAD_TYPE } from 'upgrade_types'; const IsAssignedFactorRecord = (validationOptions?: ValidationOptions) => { return function (object: unknown, propertyName: string) { @@ -15,16 +26,14 @@ const IsAssignedFactorRecord = (validationOptions?: ValidationOptions) => { }, validator: { validate(value: any) { - return validateAssignedFactorData(value) + return validateAssignedFactorData(value); }, }, }); }; }; -function validateAssignedFactorData( - data: any -): boolean { +function validateAssignedFactorData(data: any): boolean { const keys = Object.keys(data); for (const key of keys) { const factor = data[key]; @@ -45,9 +54,7 @@ function isValidAssignedFactor(value: any): value is AssignedFactor { function isValidPayload(value: any): value is Payload { return ( - typeof value === 'object' && - Object.values(PAYLOAD_TYPE).includes(value.type) && - typeof value.value === 'string' + typeof value === 'object' && Object.values(PAYLOAD_TYPE).includes(value.type) && typeof value.value === 'string' ); } @@ -72,20 +79,13 @@ class AssignedFactor { } class AssignedCondition { - @IsNotEmpty() + @IsOptional() @IsString() - id: string; + id?: string; - @IsDefined() - @IsNotEmpty() + @IsOptional() @IsString() - conditionCode: string; - - @IsObject() - @ValidateNested() - @ValidateIf((object, value) => value !== null) - @Type(() => Payload) - payload: Payload | null; + conditionCode?: string; @IsOptional() @IsString() @@ -101,11 +101,6 @@ class Data { @IsString() target: string; - @IsEnum(EXPERIMENT_TYPE) - @IsDefined() - @IsNotEmpty() - experimentType: EXPERIMENT_TYPE; - @IsOptional() @ValidateNested() @Type(() => AssignedCondition) diff --git a/clientlibs/js/src/common/index.ts b/clientlibs/js/src/common/index.ts index d97600ca3d..55d12bcfc8 100644 --- a/clientlibs/js/src/common/index.ts +++ b/clientlibs/js/src/common/index.ts @@ -1,4 +1,4 @@ -import { IExperimentAssignmentv5 } from "../../../../types/src"; +import { IExperimentAssignmentv5, PAYLOAD_TYPE } from 'upgrade_types'; export function rotateAssignmentList(assignment: IExperimentAssignmentv5) { if (assignment.assignedCondition.length > 1) { @@ -10,7 +10,25 @@ export function rotateAssignmentList(assignment: IExperimentAssignmentv5) { return assignment; } -export function findExperimentAssignmentBySiteAndTarget(site: string, target: string, experimentAssignmentData: IExperimentAssignmentv5[]): IExperimentAssignmentv5 { - const assignment = experimentAssignmentData.find((assignment) => assignment.site === site && assignment.target === target); - return assignment; -}; \ No newline at end of file +export function findExperimentAssignmentBySiteAndTarget( + site: string, + target: string, + experimentAssignmentData: IExperimentAssignmentv5[] +): IExperimentAssignmentv5 { + const assignment = experimentAssignmentData.find( + (assignment) => assignment.site === site && assignment.target === target + ); + return ( + assignment || { + site: site, + target: target, + assignedCondition: [ + { + conditionCode: null, + payload: null, + id: null, + }, + ], + } + ); +} diff --git a/clientlibs/js/src/functions/getDecisionPointAssignment.ts b/clientlibs/js/src/functions/getDecisionPointAssignment.ts index b590348ae1..454fa18495 100644 --- a/clientlibs/js/src/functions/getDecisionPointAssignment.ts +++ b/clientlibs/js/src/functions/getDecisionPointAssignment.ts @@ -8,18 +8,14 @@ export default function getDecisionPointAssignment( clientState: UpGradeClientInterfaces.IClientState ): Assignment { if (clientState?.allExperimentAssignmentData) { - const experimentAssignment = findExperimentAssignmentBySiteAndTarget(site, target, clientState.allExperimentAssignmentData) + const experimentAssignment = findExperimentAssignmentBySiteAndTarget( + site, + target, + clientState.allExperimentAssignmentData + ); - if (experimentAssignment) { - const assignment = new Assignment( - experimentAssignment, - clientState, - ); - - return assignment; - } else { - return null; - } + const assignment = new Assignment(experimentAssignment, clientState); + return assignment; } else { return null; } diff --git a/clientlibs/js/src/functions/markDecisionPoint.ts b/clientlibs/js/src/functions/markDecisionPoint.ts index 0b8f1097a0..a26c06c146 100644 --- a/clientlibs/js/src/functions/markDecisionPoint.ts +++ b/clientlibs/js/src/functions/markDecisionPoint.ts @@ -16,15 +16,11 @@ export default async function markDecisionPoint( uniquifier?: string, clientError?: string ): Promise { - const assignment = findExperimentAssignmentBySiteAndTarget(site, target, experimentAssignmentData) - - if (!assignment) { - throw new Error('No assignment found'); - } + const assignment = findExperimentAssignmentBySiteAndTarget(site, target, experimentAssignmentData); rotateAssignmentList(assignment); - const data = { ...assignment, assignedCondition: { ...assignment.assignedCondition[0], conditionCode : condition} }; + const data = { ...assignment, assignedCondition: { ...assignment.assignedCondition[0], conditionCode: condition } }; let requestBody: UpGradeClientInterfaces.IMarkDecisionPointRequestBody = { userId, @@ -44,7 +40,13 @@ export default async function markDecisionPoint( clientError, }; } - const response = await fetchDataService(url, token, clientSessionId, requestBody, UpGradeClientEnums.REQUEST_TYPES.POST); + const response = await fetchDataService( + url, + token, + clientSessionId, + requestBody, + UpGradeClientEnums.REQUEST_TYPES.POST + ); if (response.status) { return response.data; } else {