Skip to content

Commit 1ee3fdf

Browse files
committed
[AI-FSSDK] [FSSDK-12735] Fix decisionEventDispatched for holdout impression and add missing decision reason
1 parent 354d2bc commit 1ee3fdf

4 files changed

Lines changed: 22 additions & 4 deletions

File tree

lib/core/decision_service/index.spec.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
USER_HAS_NO_FORCED_VARIATION
2020
} from 'log_message';
2121
import { beforeEach, describe, expect, it, MockInstance, vi } from 'vitest';
22-
import { CMAB_DUMMY_ENTITY_ID, CMAB_FETCH_FAILED, DecisionService } from '.';
22+
import { CMAB_DUMMY_ENTITY_ID, CMAB_FETCH_FAILED, DecisionService, TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT } from '.';
2323
import OptimizelyUserContext from '../../optimizely_user_context';
2424
import { createProjectConfig, ProjectConfig } from '../../project_config/project_config';
2525
import { BucketerParams, Experiment, ExperimentBucketMap, Holdout, OptimizelyDecideOption, UserAttributes, UserProfile } from '../../shared_types';
@@ -2286,6 +2286,9 @@ describe('DecisionService', () => {
22862286
variation: config.variationIdMap['holdout_variation_running_id'],
22872287
},
22882288
});
2289+
2290+
const reasonStrings = variation.reasons.map((r: any) => typeof r === 'string' ? r : r[0]);
2291+
expect(reasonStrings).toContain(TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT);
22892292
});
22902293

22912294
it('should return null decision with holdout info when excludeTargetedDeliveries is true but no delivery matches', async () => {

lib/core/decision_service/index.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ export const USER_MEETS_CONDITIONS_FOR_HOLDOUT = 'User %s meets conditions for h
119119
export const USER_DOESNT_MEET_CONDITIONS_FOR_HOLDOUT = 'User %s does not meet conditions for holdout %s.';
120120
export const USER_BUCKETED_INTO_HOLDOUT_VARIATION = 'User %s is in variation %s of holdout %s.';
121121
export const USER_NOT_BUCKETED_INTO_HOLDOUT_VARIATION = 'User %s is in no holdout variation.';
122-
export const TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT = 'User %s is in holdout %s but targeted deliveries are excluded from holdout.';
122+
export const TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT = "Holdout '%s' has excludeTargetedDeliveries enabled, continuing to rollout evaluation.";
123123

124124
export interface DecisionObj {
125125
experiment: Experiment | Holdout | null;
@@ -984,8 +984,8 @@ export class DecisionService {
984984
if (holdoutDecision.result.variation) {
985985
if (holdout.excludeTargetedDeliveries) {
986986
const userId = user.getUserId();
987-
this.logger?.info(TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT, userId, holdout.key);
988-
decideReasons.push([TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT, userId, holdout.key]);
987+
this.logger?.info(TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT, holdout.key);
988+
decideReasons.push([TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT, holdout.key]);
989989
activeHoldoutDecision = holdoutDecision.result;
990990
activeHoldout = holdout;
991991
break;

lib/optimizely/index.spec.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -875,6 +875,11 @@ describe('Optimizely', () => {
875875

876876
it('should dispatch separate holdout impression when decisionObj.holdout is populated', async () => {
877877
const processSpy = vi.spyOn(eventProcessor, 'process');
878+
const notificationSpyLocal = vi.fn();
879+
optimizely.notificationCenter.addNotificationListener(
880+
NOTIFICATION_TYPES.DECISION,
881+
notificationSpyLocal
882+
);
878883

879884
const holdoutExperiment = projectConfig.holdouts[0];
880885
const holdoutVariation = projectConfig.holdouts[0].variations[0];
@@ -917,6 +922,15 @@ describe('Optimizely', () => {
917922
expect(holdoutImpression.ruleKey).toBe('holdout_test_key');
918923
expect(holdoutImpression.ruleType).toBe('holdout');
919924
expect(holdoutImpression.enabled).toBe(false);
925+
926+
expect(notificationSpyLocal).toHaveBeenCalledWith(
927+
expect.objectContaining({
928+
type: DECISION_NOTIFICATION_TYPES.FLAG,
929+
decisionInfo: expect.objectContaining({
930+
decisionEventDispatched: true,
931+
}),
932+
})
933+
);
920934
});
921935
});
922936

lib/optimizely/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1549,6 +1549,7 @@ export default class Optimizely extends BaseService implements Client {
15491549
decisionSource: DECISION_SOURCES.HOLDOUT,
15501550
};
15511551
this.sendImpressionEvent(holdoutDecisionObj, key, userId, false, attributes);
1552+
decisionEventDispatched = true;
15521553
}
15531554

15541555
const shouldIncludeReasons = options[OptimizelyDecideOption.INCLUDE_REASONS];

0 commit comments

Comments
 (0)