From 7d59ca3b467db46dc0766f7e05ffd2c9cbfa8604 Mon Sep 17 00:00:00 2001 From: esrakartalOpt Date: Tue, 21 Jul 2026 14:08:18 -0500 Subject: [PATCH 1/7] [AI-FSSDK] [FSSDK-12735] Add holdout exclusion logic for Targeted Delivery rules --- lib/optimizely/decision_service.rb | 93 ++++--- lib/optimizely/helpers/constants.rb | 8 +- spec/decision_service_holdout_spec.rb | 337 ++++++++++++++++++++++++++ 3 files changed, 385 insertions(+), 53 deletions(-) diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index ef4d0448..b514866f 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # -# Copyright 2017-2022, Optimizely and contributors +# Copyright 2017-2022, 2026, Optimizely and contributors # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -198,6 +198,7 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt # Check global holdouts first (flag level) — these apply to all rules across all flags holdouts = project_config.global_holdouts + exclude_td_holdout_decision = nil holdouts.each do |holdout| holdout_decision = get_variation_for_holdout(holdout, user_context, project_config) @@ -205,14 +206,21 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt next unless holdout_decision.decision - message = "The user '#{user_id}' is bucketed into holdout '#{holdout['key']}' for feature flag '#{feature_flag['key']}'." - @logger.log(Logger::INFO, message) - reasons.push(message) - return DecisionResult.new(holdout_decision.decision, false, reasons) + if holdout['excludeTargetedDeliveries'] == true + message = "The user '#{user_id}' is bucketed into holdout '#{holdout['key']}' for feature flag '#{feature_flag['key']}', but targeted deliveries are excluded." + @logger.log(Logger::INFO, message) + reasons.push(message) + exclude_td_holdout_decision = holdout_decision.decision + else + message = "The user '#{user_id}' is bucketed into holdout '#{holdout['key']}' for feature flag '#{feature_flag['key']}'." + @logger.log(Logger::INFO, message) + reasons.push(message) + return DecisionResult.new(holdout_decision.decision, false, reasons) + end end # Check if the feature flag has an experiment and the user is bucketed into that experiment - experiment_decision = get_variation_for_feature_experiment(project_config, feature_flag, user_context, user_profile_tracker, decide_options) + experiment_decision = get_variation_for_feature_experiment(project_config, feature_flag, user_context, user_profile_tracker, decide_options, exclude_td_holdout_decision) reasons.push(*experiment_decision.reasons) return DecisionResult.new(experiment_decision.decision, experiment_decision.error, reasons) if experiment_decision.decision @@ -221,7 +229,7 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt return DecisionResult.new(nil, experiment_decision.error, reasons) if experiment_decision.error # Check if the feature flag has a rollout and the user is bucketed into that rollout - rollout_decision = get_variation_for_feature_rollout(project_config, feature_flag, user_context) + rollout_decision = get_variation_for_feature_rollout(project_config, feature_flag, user_context, exclude_td_holdout_decision) reasons.push(*rollout_decision.reasons) if rollout_decision.decision @@ -237,6 +245,8 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt DecisionResult.new(rollout_decision.decision, rollout_decision.error, reasons) else + return DecisionResult.new(exclude_td_holdout_decision, false, reasons) if exclude_td_holdout_decision + message = "The user '#{user_id}' is not bucketed into a rollout for feature flag '#{feature_flag['key']}'." @logger.log(Logger::INFO, message) DecisionResult.new(nil, false, reasons) @@ -325,15 +335,7 @@ def get_variations_for_feature_list(project_config, feature_flags, user_context, decisions end - def get_variation_for_feature_experiment(project_config, feature_flag, user_context, user_profile_tracker, decide_options = []) - # Gets the variation the user is bucketed into for the feature flag's experiment. - # - # project_config - project_config - Instance of ProjectConfig - # feature_flag - The feature flag the user wants to access - # user_context - Optimizely user context instance - # - # Returns a DecisionResult containing the decision (or nil if not bucketed), - # an error flag, and an array of decision reasons. + def get_variation_for_feature_experiment(project_config, feature_flag, user_context, user_profile_tracker, decide_options = [], global_holdout_decision = nil) decide_reasons = [] user_id = user_context.user_id feature_flag_key = feature_flag['key'] @@ -355,7 +357,7 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_cont end experiment_id = experiment['id'] - variation_result = get_variation_from_experiment_rule(project_config, feature_flag_key, experiment, user_context, user_profile_tracker, decide_options) + variation_result = get_variation_from_experiment_rule(project_config, feature_flag_key, experiment, user_context, user_profile_tracker, decide_options, global_holdout_decision) error = variation_result.error reasons_received = variation_result.reasons variation_id = variation_result.variation_id @@ -384,16 +386,7 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_cont DecisionResult.new(nil, false, decide_reasons) end - def get_variation_for_feature_rollout(project_config, feature_flag, user_context) - # Determine which variation the user is in for a given rollout. - # Returns the variation of the first experiment the user qualifies for. - # - # project_config - project_config - Instance of ProjectConfig - # feature_flag - The feature flag the user wants to access - # user_context - Optimizely user context instance - # - # Returns a DecisionResult containing the decision (or nil if not bucketed), - # an error flag, and an array of decision reasons. + def get_variation_for_feature_rollout(project_config, feature_flag, user_context, global_holdout_decision = nil) decide_reasons = [] rollout_id = feature_flag['rolloutId'] @@ -418,7 +411,7 @@ def get_variation_for_feature_rollout(project_config, feature_flag, user_context index = 0 rollout_rules = rollout['experiments'] while index < rollout_rules.length - holdout_decision, variation, skip_to_everyone_else, reasons_received = get_variation_from_delivery_rule(project_config, feature_flag_key, rollout_rules, index, user_context) + holdout_decision, variation, skip_to_everyone_else, reasons_received = get_variation_from_delivery_rule(project_config, feature_flag_key, rollout_rules, index, user_context, global_holdout_decision) decide_reasons.push(*reasons_received) return DecisionResult.new(holdout_decision, false, decide_reasons) if holdout_decision @@ -435,16 +428,7 @@ def get_variation_for_feature_rollout(project_config, feature_flag, user_context DecisionResult.new(nil, false, decide_reasons) end - def get_variation_from_experiment_rule(project_config, flag_key, rule, user, user_profile_tracker, options = []) - # Determine which variation the user is in for a given rollout. - # Returns the variation from experiment rules. - # - # project_config - project_config - Instance of ProjectConfig - # flag_key - The feature flag the user wants to access - # rule - An experiment rule key - # user - Optimizely user context instance - # - # Returns variation_id and reasons + def get_variation_from_experiment_rule(project_config, flag_key, rule, user, user_profile_tracker, options = [], global_holdout_decision = nil) reasons = [] # Step 1: Forced decision check @@ -453,9 +437,16 @@ def get_variation_from_experiment_rule(project_config, flag_key, rule, user, use reasons.push(*forced_reasons) return VariationResult.new(nil, false, reasons, variation['id']) if variation - # Step 2: Local holdout check + # Step 2: Global holdout check (when excludeTargetedDeliveries is true, TD rules skip the holdout) + if global_holdout_decision && rule['type'] != Helpers::Constants::EXPERIMENT_TYPES['td'] + return VariationResult.new(nil, false, reasons, nil, global_holdout_decision) + end + + # Step 3: Local holdout check local_holdouts = project_config.get_holdouts_for_rule(rule['id']) local_holdouts.each do |holdout| + next if holdout['excludeTargetedDeliveries'] == true && rule['type'] == Helpers::Constants::EXPERIMENT_TYPES['td'] + holdout_decision = get_variation_for_holdout(holdout, user, project_config) reasons.push(*holdout_decision.reasons) next unless holdout_decision.decision @@ -464,22 +455,13 @@ def get_variation_from_experiment_rule(project_config, flag_key, rule, user, use return VariationResult.new(nil, false, reasons, holdout_variation['id'], holdout_decision.decision) end - # Step 3: Regular rule evaluation + # Step 4: Regular rule evaluation variation_result = get_variation(project_config, rule['id'], user, user_profile_tracker, options) variation_result.reasons = reasons + variation_result.reasons variation_result end - def get_variation_from_delivery_rule(project_config, flag_key, rules, rule_index, user_context) - # Determine which variation the user is in for a given rollout. - # Returns the variation from delivery rules. - # - # project_config - project_config - Instance of ProjectConfig - # flag_key - The feature flag the user wants to access - # rule - An experiment rule key - # user_context - Optimizely user context instance - # - # Returns [holdout_decision, variation, skip_to_everyone_else, reasons] + def get_variation_from_delivery_rule(project_config, flag_key, rules, rule_index, user_context, global_holdout_decision = nil) reasons = [] skip_to_everyone_else = false rule = rules[rule_index] @@ -490,15 +472,22 @@ def get_variation_from_delivery_rule(project_config, flag_key, rules, rule_index reasons.push(*forced_reasons) return [nil, variation, skip_to_everyone_else, reasons] if variation - # Step 2: Local holdout check + # Step 2: Global holdout check + if global_holdout_decision && rule['type'] != Helpers::Constants::EXPERIMENT_TYPES['td'] + return [global_holdout_decision, nil, skip_to_everyone_else, reasons] + end + + # Step 3: Local holdout check local_holdouts = project_config.get_holdouts_for_rule(rule['id']) local_holdouts.each do |holdout| + next if holdout['excludeTargetedDeliveries'] == true && rule['type'] == Helpers::Constants::EXPERIMENT_TYPES['td'] + holdout_decision = get_variation_for_holdout(holdout, user_context, project_config) reasons.push(*holdout_decision.reasons) return [holdout_decision.decision, nil, skip_to_everyone_else, reasons] if holdout_decision.decision end - # Step 3: Regular rule evaluation + # Step 4: Regular rule evaluation user_id = user_context.user_id attributes = user_context.user_attributes bucketing_id, bucketing_id_reasons = get_bucketing_id(user_id, attributes) diff --git a/lib/optimizely/helpers/constants.rb b/lib/optimizely/helpers/constants.rb index b23955b1..ecba408a 100644 --- a/lib/optimizely/helpers/constants.rb +++ b/lib/optimizely/helpers/constants.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # -# Copyright 2016-2020, 2022, Optimizely and contributors +# Copyright 2016-2020, 2022, 2026, Optimizely and contributors # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -349,6 +349,9 @@ module Constants }, 'includedRules' => { 'type' => %w[array null] + }, + 'excludeTargetedDeliveries' => { + 'type' => 'boolean' } } } @@ -369,6 +372,9 @@ module Constants }, 'includedRules' => { 'type' => %w[array null] + }, + 'excludeTargetedDeliveries' => { + 'type' => 'boolean' } } } diff --git a/spec/decision_service_holdout_spec.rb b/spec/decision_service_holdout_spec.rb index 370fd4b8..490eb1bf 100644 --- a/spec/decision_service_holdout_spec.rb +++ b/spec/decision_service_holdout_spec.rb @@ -1033,4 +1033,341 @@ end end end + + describe 'excludeTargetedDeliveries holdout behavior' do + let(:config_with_holdouts) do + Optimizely::DatafileProjectConfig.new( + OptimizelySpec::CONFIG_BODY_WITH_HOLDOUTS_JSON, + spy_logger, + error_handler + ) + end + + let(:project_with_holdouts) do + Optimizely::Project.new( + datafile: OptimizelySpec::CONFIG_BODY_WITH_HOLDOUTS_JSON, + logger: spy_logger, + error_handler: error_handler + ) + end + + let(:ds) do + Optimizely::DecisionService.new(spy_logger, spy_cmab_service) + end + + after(:example) { project_with_holdouts&.close } + + describe 'global holdout with excludeTargetedDeliveries' do + it 'applies holdout normally when excludeTargetedDeliveries is false' do + global_holdout = config_with_holdouts.get_holdout('holdout_1') + global_holdout['excludeTargetedDeliveries'] = false + + allow(ds).to receive(:get_variation_for_holdout) + .with(global_holdout, anything, anything) + .and_return( + Optimizely::DecisionService::DecisionResult.new( + Optimizely::DecisionService::Decision.new( + global_holdout, + global_holdout['variations'].first, + Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT'], + nil + ), + false, + ['User in global holdout'] + ) + ) + + # Stub other global holdouts to miss + config_with_holdouts.global_holdouts.reject { |h| h['id'] == 'holdout_1' }.each do |h| + allow(ds).to receive(:get_variation_for_holdout) + .with(h, anything, anything) + .and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) + end + + feature_flag = config_with_holdouts.feature_flag_key_map['boolean_feature'] + user_ctx = project_with_holdouts.create_user_context('test_user', {}) + + result = ds.get_decision_for_flag(feature_flag, user_ctx, config_with_holdouts) + + expect(result.decision).not_to be_nil + expect(result.decision.source).to eq(Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT']) + end + + it 'stores holdout decision but lets TD rules bypass when excludeTargetedDeliveries is true' do + global_holdout = config_with_holdouts.get_holdout('holdout_1') + + holdout_decision = Optimizely::DecisionService::Decision.new( + global_holdout, + global_holdout['variations'].first, + Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT'], + nil + ) + + # Set experiment 122227 as a TD rule + experiment = config_with_holdouts.experiment_id_map['122227'] + experiment['type'] = 'td' + + # Stub local holdouts to miss so only global holdout logic is tested + allow(ds).to receive(:get_variation_for_holdout).and_return( + Optimizely::DecisionService::DecisionResult.new(nil, false, []) + ) + + # Mock get_variation to return a variation (simulating TD rule match) + allow(ds).to receive(:get_variation).and_return( + Optimizely::DecisionService::VariationResult.new(nil, false, ['Bucketed into TD'], '122228') + ) + + user_ctx = project_with_holdouts.create_user_context('test_user', {}) + user_profile_tracker = Optimizely::UserProfileTracker.new('test_user', nil, spy_logger) + + # Pass holdout_decision as global_holdout_decision — TD rule should bypass it + result = ds.get_variation_from_experiment_rule( + config_with_holdouts, + 'boolean_feature', + experiment, + user_ctx, + user_profile_tracker, + [], + holdout_decision + ) + + # TD rule should NOT get holdout decision; should proceed to regular evaluation + expect(result.holdout_decision).to be_nil + expect(result.variation_id).to eq('122228') + end + + it 'applies holdout to non-TD (AB) rules even when excludeTargetedDeliveries is true' do + global_holdout = config_with_holdouts.get_holdout('holdout_1') + global_holdout['excludeTargetedDeliveries'] = true + + holdout_decision = Optimizely::DecisionService::Decision.new( + global_holdout, + global_holdout['variations'].first, + Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT'], + nil + ) + + # Experiment 122227 as AB type (non-TD) + experiment = config_with_holdouts.experiment_id_map['122227'] + experiment['type'] = 'ab' + + user_ctx = project_with_holdouts.create_user_context('test_user', {}) + user_profile_tracker = Optimizely::UserProfileTracker.new('test_user', nil, spy_logger) + + result = ds.get_variation_from_experiment_rule( + config_with_holdouts, + 'boolean_feature', + experiment, + user_ctx, + user_profile_tracker, + [], + holdout_decision + ) + + # AB rule should get holdout decision since it's not TD + expect(result.holdout_decision).to eq(holdout_decision) + end + end + + describe 'global holdout with missing excludeTargetedDeliveries (backward compat)' do + it 'defaults to applying holdout normally when field is absent' do + global_holdout = config_with_holdouts.get_holdout('holdout_1') + global_holdout.delete('excludeTargetedDeliveries') + + allow(ds).to receive(:get_variation_for_holdout) + .with(global_holdout, anything, anything) + .and_return( + Optimizely::DecisionService::DecisionResult.new( + Optimizely::DecisionService::Decision.new( + global_holdout, + global_holdout['variations'].first, + Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT'], + nil + ), + false, + ['User in global holdout'] + ) + ) + + config_with_holdouts.global_holdouts.reject { |h| h['id'] == 'holdout_1' }.each do |h| + allow(ds).to receive(:get_variation_for_holdout) + .with(h, anything, anything) + .and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) + end + + feature_flag = config_with_holdouts.feature_flag_key_map['boolean_feature'] + user_ctx = project_with_holdouts.create_user_context('test_user', {}) + + result = ds.get_decision_for_flag(feature_flag, user_ctx, config_with_holdouts) + + # Missing field means false => holdout applies immediately + expect(result.decision).not_to be_nil + expect(result.decision.source).to eq(Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT']) + end + end + + describe 'local holdout with excludeTargetedDeliveries' do + it 'skips local holdout for TD rules when excludeTargetedDeliveries is true' do + local_holdout = config_with_holdouts.get_holdout('holdout_local_1') + local_holdout['excludeTargetedDeliveries'] = true + + # Set experiment 122227 as TD type + experiment = config_with_holdouts.experiment_id_map['122227'] + experiment['type'] = 'td' + + # All global holdouts miss + config_with_holdouts.global_holdouts.each do |gh| + allow(ds).to receive(:get_variation_for_holdout) + .with(gh, anything, anything) + .and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) + end + + # Mock get_variation to simulate rule match + allow(ds).to receive(:get_variation).and_return( + Optimizely::DecisionService::VariationResult.new(nil, false, ['Bucketed into TD rule'], '122228') + ) + + user_ctx = project_with_holdouts.create_user_context('test_user', {}) + user_profile_tracker = Optimizely::UserProfileTracker.new('test_user', nil, spy_logger) + + result = ds.get_variation_from_experiment_rule( + config_with_holdouts, + 'boolean_feature', + experiment, + user_ctx, + user_profile_tracker + ) + + # Local holdout should be skipped for TD rule; regular evaluation proceeds + expect(result.variation_id).to eq('122228') + expect(result.holdout_decision).to be_nil + end + + it 'applies local holdout for AB rules even when excludeTargetedDeliveries is true' do + local_holdout = config_with_holdouts.get_holdout('holdout_local_1') + local_holdout['excludeTargetedDeliveries'] = true + + # Experiment 122227 as AB type + experiment = config_with_holdouts.experiment_id_map['122227'] + experiment['type'] = 'ab' + + # All global holdouts miss + config_with_holdouts.global_holdouts.each do |gh| + allow(ds).to receive(:get_variation_for_holdout) + .with(gh, anything, anything) + .and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) + end + + # Local holdout fires for AB rule + allow(ds).to receive(:get_variation_for_holdout) + .with(local_holdout, anything, anything) + .and_return( + Optimizely::DecisionService::DecisionResult.new( + Optimizely::DecisionService::Decision.new( + local_holdout, + local_holdout['variations'].first, + Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT'], + nil + ), + false, + ['User in local holdout'] + ) + ) + + user_ctx = project_with_holdouts.create_user_context('test_user', {}) + user_profile_tracker = Optimizely::UserProfileTracker.new('test_user', nil, spy_logger) + + result = ds.get_variation_from_experiment_rule( + config_with_holdouts, + 'boolean_feature', + experiment, + user_ctx, + user_profile_tracker + ) + + # Local holdout applies to AB rule + expect(result.holdout_decision).not_to be_nil + expect(result.holdout_decision.source).to eq(Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT']) + end + + it 'applies local holdout normally when excludeTargetedDeliveries is false' do + local_holdout = config_with_holdouts.get_holdout('holdout_local_1') + local_holdout['excludeTargetedDeliveries'] = false + + # Set experiment 122227 as TD type + experiment = config_with_holdouts.experiment_id_map['122227'] + experiment['type'] = 'td' + + # All global holdouts miss + config_with_holdouts.global_holdouts.each do |gh| + allow(ds).to receive(:get_variation_for_holdout) + .with(gh, anything, anything) + .and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) + end + + # Local holdout fires even for TD rule because excludeTargetedDeliveries is false + allow(ds).to receive(:get_variation_for_holdout) + .with(local_holdout, anything, anything) + .and_return( + Optimizely::DecisionService::DecisionResult.new( + Optimizely::DecisionService::Decision.new( + local_holdout, + local_holdout['variations'].first, + Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT'], + nil + ), + false, + ['User in local holdout'] + ) + ) + + user_ctx = project_with_holdouts.create_user_context('test_user', {}) + user_profile_tracker = Optimizely::UserProfileTracker.new('test_user', nil, spy_logger) + + result = ds.get_variation_from_experiment_rule( + config_with_holdouts, + 'boolean_feature', + experiment, + user_ctx, + user_profile_tracker + ) + + # Holdout applies to TD rule because excludeTargetedDeliveries is false + expect(result.holdout_decision).not_to be_nil + expect(result.holdout_decision.source).to eq(Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT']) + end + end + + describe 'forced decision beats holdout with excludeTargetedDeliveries' do + it 'returns forced decision even when excludeTargetedDeliveries is true and holdout covers the rule' do + local_holdout = config_with_holdouts.get_holdout('holdout_local_1') + local_holdout['excludeTargetedDeliveries'] = true + expect(local_holdout['trafficAllocation'].first['endOfRange']).to eq(10_000) + + experiment = config_with_holdouts.experiment_id_map['122227'] + experiment['type'] = 'ab' + + user_ctx = project_with_holdouts.create_user_context('test_user', {}) + context = Optimizely::OptimizelyUserContext::OptimizelyDecisionContext.new( + 'boolean_feature', + 'test_experiment_with_audience' + ) + forced = Optimizely::OptimizelyUserContext::OptimizelyForcedDecision.new('control_with_audience') + user_ctx.set_forced_decision(context, forced) + + user_profile_tracker = Optimizely::UserProfileTracker.new('test_user', nil, spy_logger) + + result = ds.get_variation_from_experiment_rule( + config_with_holdouts, + 'boolean_feature', + experiment, + user_ctx, + user_profile_tracker + ) + + expect(result.variation_id).to eq('122228') + expect(result.holdout_decision).to be_nil + end + end + end end From 10391448d231834a9d652dbc9945d88d21031b68 Mon Sep 17 00:00:00 2001 From: esrakartalOpt Date: Tue, 21 Jul 2026 16:09:08 -0500 Subject: [PATCH 2/7] Fix lint issues --- lib/optimizely/decision_service.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index b514866f..260aa55b 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -438,9 +438,7 @@ def get_variation_from_experiment_rule(project_config, flag_key, rule, user, use return VariationResult.new(nil, false, reasons, variation['id']) if variation # Step 2: Global holdout check (when excludeTargetedDeliveries is true, TD rules skip the holdout) - if global_holdout_decision && rule['type'] != Helpers::Constants::EXPERIMENT_TYPES['td'] - return VariationResult.new(nil, false, reasons, nil, global_holdout_decision) - end + return VariationResult.new(nil, false, reasons, nil, global_holdout_decision) if global_holdout_decision && rule['type'] != Helpers::Constants::EXPERIMENT_TYPES['td'] # Step 3: Local holdout check local_holdouts = project_config.get_holdouts_for_rule(rule['id']) @@ -473,9 +471,7 @@ def get_variation_from_delivery_rule(project_config, flag_key, rules, rule_index return [nil, variation, skip_to_everyone_else, reasons] if variation # Step 2: Global holdout check - if global_holdout_decision && rule['type'] != Helpers::Constants::EXPERIMENT_TYPES['td'] - return [global_holdout_decision, nil, skip_to_everyone_else, reasons] - end + return [global_holdout_decision, nil, skip_to_everyone_else, reasons] if global_holdout_decision && rule['type'] != Helpers::Constants::EXPERIMENT_TYPES['td'] # Step 3: Local holdout check local_holdouts = project_config.get_holdouts_for_rule(rule['id']) From 47d3e3d88fb419cbccfe6b4b061e0e7b5e134ae3 Mon Sep 17 00:00:00 2001 From: esrakartalOpt Date: Wed, 22 Jul 2026 14:30:20 -0500 Subject: [PATCH 3/7] [AI-FSSDK] [FSSDK-12735] Fix holdout event sending, local holdout exclusion, and TD null fallback --- lib/optimizely.rb | 24 ++++- lib/optimizely/decision_service.rb | 16 +--- spec/decision_service_holdout_spec.rb | 127 ++++++++++++++++++++++++-- 3 files changed, 146 insertions(+), 21 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index f524d822..056804bc 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -193,7 +193,7 @@ def create_user_context(user_id, attributes = nil) OptimizelyUserContext.new(self, user_id, attributes) end - def create_optimizely_decision(user_context, flag_key, decision, reasons, decide_options, config) + def create_optimizely_decision(user_context, flag_key, decision, reasons, decide_options, config, holdout_decision = nil) # Create Optimizely Decision Result. user_id = user_context.user_id attributes = user_context.user_attributes @@ -225,6 +225,22 @@ def create_optimizely_decision(user_context, flag_key, decision, reasons, decide decision_event_dispatched = true end + if holdout_decision && !decide_options.include?(OptimizelyDecideOption::DISABLE_DECISION_EVENT) && decision_source != Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT'] + holdout_experiment = holdout_decision.experiment + holdout_variation = holdout_decision.variation + send_impression( + config, + holdout_experiment, + holdout_variation ? holdout_variation['key'] : '', + flag_key, + holdout_experiment ? holdout_experiment['key'] : '', + holdout_variation ? holdout_variation['featureEnabled'] : false, + Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT'], + user_id, + attributes + ) + end + # Generate all variables map if decide options doesn't include excludeVariables unless decide_options.include? OptimizelyDecideOption::EXCLUDE_VARIABLES feature_flag['variables'].each do |variable| @@ -377,6 +393,7 @@ def decide_for_keys(user_context, keys, decide_options = [], ignore_default_opti end decision_list = @decision_service.get_variations_for_feature_list(config, flags_without_forced_decision, user_context, decide_options) + holdout_decisions = {} flags_without_forced_decision.each_with_index do |flag, i| decision = decision_list[i].decision reasons = decision_list[i].reasons @@ -390,6 +407,8 @@ def decide_for_keys(user_context, keys, decide_options = [], ignore_default_opti next end flag_decisions[flag_key] = decision + holdout_decision = decision_list[i].holdout_decision + holdout_decisions[flag_key] = holdout_decision if holdout_decision decision_reasons_dict[flag_key] ||= [] decision_reasons_dict[flag_key].push(*reasons) end @@ -402,7 +421,8 @@ def decide_for_keys(user_context, keys, decide_options = [], ignore_default_opti flag_decision, decision_reasons, decide_options, - config + config, + holdout_decisions[key] ) enabled_flags_only_missing = !decide_options.include?(OptimizelyDecideOption::ENABLED_FLAGS_ONLY) diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 260aa55b..dadbf42d 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -41,7 +41,7 @@ class DecisionService Decision = Struct.new(:experiment, :variation, :source, :cmab_uuid) CmabDecisionResult = Struct.new(:error, :result, :reasons) VariationResult = Struct.new(:cmab_uuid, :error, :reasons, :variation_id, :holdout_decision) - DecisionResult = Struct.new(:decision, :error, :reasons) + DecisionResult = Struct.new(:decision, :error, :reasons, :holdout_decision) DECISION_SOURCES = { 'EXPERIMENT' => 'experiment', @@ -223,10 +223,10 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt experiment_decision = get_variation_for_feature_experiment(project_config, feature_flag, user_context, user_profile_tracker, decide_options, exclude_td_holdout_decision) reasons.push(*experiment_decision.reasons) - return DecisionResult.new(experiment_decision.decision, experiment_decision.error, reasons) if experiment_decision.decision + return DecisionResult.new(experiment_decision.decision, experiment_decision.error, reasons, exclude_td_holdout_decision) if experiment_decision.decision # If there's an error (e.g., CMAB error), return immediately without falling back to rollout - return DecisionResult.new(nil, experiment_decision.error, reasons) if experiment_decision.error + return DecisionResult.new(nil, experiment_decision.error, reasons, exclude_td_holdout_decision) if experiment_decision.error # Check if the feature flag has a rollout and the user is bucketed into that rollout rollout_decision = get_variation_for_feature_rollout(project_config, feature_flag, user_context, exclude_td_holdout_decision) @@ -243,13 +243,11 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt reasons.push(message) end - DecisionResult.new(rollout_decision.decision, rollout_decision.error, reasons) + DecisionResult.new(rollout_decision.decision, rollout_decision.error, reasons, exclude_td_holdout_decision) else - return DecisionResult.new(exclude_td_holdout_decision, false, reasons) if exclude_td_holdout_decision - message = "The user '#{user_id}' is not bucketed into a rollout for feature flag '#{feature_flag['key']}'." @logger.log(Logger::INFO, message) - DecisionResult.new(nil, false, reasons) + DecisionResult.new(nil, false, reasons, exclude_td_holdout_decision) end end @@ -443,8 +441,6 @@ def get_variation_from_experiment_rule(project_config, flag_key, rule, user, use # Step 3: Local holdout check local_holdouts = project_config.get_holdouts_for_rule(rule['id']) local_holdouts.each do |holdout| - next if holdout['excludeTargetedDeliveries'] == true && rule['type'] == Helpers::Constants::EXPERIMENT_TYPES['td'] - holdout_decision = get_variation_for_holdout(holdout, user, project_config) reasons.push(*holdout_decision.reasons) next unless holdout_decision.decision @@ -476,8 +472,6 @@ def get_variation_from_delivery_rule(project_config, flag_key, rules, rule_index # Step 3: Local holdout check local_holdouts = project_config.get_holdouts_for_rule(rule['id']) local_holdouts.each do |holdout| - next if holdout['excludeTargetedDeliveries'] == true && rule['type'] == Helpers::Constants::EXPERIMENT_TYPES['td'] - holdout_decision = get_variation_for_holdout(holdout, user_context, project_config) reasons.push(*holdout_decision.reasons) return [holdout_decision.decision, nil, skip_to_everyone_else, reasons] if holdout_decision.decision diff --git a/spec/decision_service_holdout_spec.rb b/spec/decision_service_holdout_spec.rb index 490eb1bf..eca71232 100644 --- a/spec/decision_service_holdout_spec.rb +++ b/spec/decision_service_holdout_spec.rb @@ -1207,7 +1207,7 @@ end describe 'local holdout with excludeTargetedDeliveries' do - it 'skips local holdout for TD rules when excludeTargetedDeliveries is true' do + it 'local holdout still applies to TD rules even when excludeTargetedDeliveries is true' do local_holdout = config_with_holdouts.get_holdout('holdout_local_1') local_holdout['excludeTargetedDeliveries'] = true @@ -1222,10 +1222,21 @@ .and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) end - # Mock get_variation to simulate rule match - allow(ds).to receive(:get_variation).and_return( - Optimizely::DecisionService::VariationResult.new(nil, false, ['Bucketed into TD rule'], '122228') - ) + # Local holdout fires + allow(ds).to receive(:get_variation_for_holdout) + .with(local_holdout, anything, anything) + .and_return( + Optimizely::DecisionService::DecisionResult.new( + Optimizely::DecisionService::Decision.new( + local_holdout, + local_holdout['variations'].first, + Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT'], + nil + ), + false, + ['User in local holdout'] + ) + ) user_ctx = project_with_holdouts.create_user_context('test_user', {}) user_profile_tracker = Optimizely::UserProfileTracker.new('test_user', nil, spy_logger) @@ -1238,9 +1249,10 @@ user_profile_tracker ) - # Local holdout should be skipped for TD rule; regular evaluation proceeds - expect(result.variation_id).to eq('122228') - expect(result.holdout_decision).to be_nil + # Local holdout MUST still apply — excludeTargetedDeliveries is ignored for local holdouts + expect(result.holdout_decision).not_to be_nil + expect(result.holdout_decision.source).to eq(Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT']) + expect(result.variation_id).to eq(local_holdout['variations'].first['id']) end it 'applies local holdout for AB rules even when excludeTargetedDeliveries is true' do @@ -1338,6 +1350,105 @@ end end + describe 'global holdout excludeTargetedDeliveries integration with get_decision_for_flag' do + it 'returns TD decision with holdout_decision attached when excludeTargetedDeliveries=true and TD succeeds' do + global_holdout = config_with_holdouts.get_holdout('holdout_1') + global_holdout['excludeTargetedDeliveries'] = true + + holdout_variation = global_holdout['variations'].first + holdout_dec = Optimizely::DecisionService::Decision.new( + global_holdout, + holdout_variation, + Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT'], + nil + ) + + # Global holdout hits + allow(ds).to receive(:get_variation_for_holdout) + .with(global_holdout, anything, anything) + .and_return(Optimizely::DecisionService::DecisionResult.new(holdout_dec, false, ['User in holdout'])) + + # Other global holdouts miss + config_with_holdouts.global_holdouts.reject { |h| h['id'] == 'holdout_1' }.each do |h| + allow(ds).to receive(:get_variation_for_holdout) + .with(h, anything, anything) + .and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) + end + + # Set experiment 122227 as TD and mock a successful TD match + experiment = config_with_holdouts.experiment_id_map['122227'] + experiment['type'] = 'td' + + td_decision = Optimizely::DecisionService::Decision.new( + experiment, + experiment['variations'].first, + Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST'], + nil + ) + + allow(ds).to receive(:get_variation_for_feature_experiment).and_return( + Optimizely::DecisionService::DecisionResult.new(td_decision, false, ['TD matched']) + ) + + feature_flag = config_with_holdouts.feature_flag_key_map['boolean_feature'] + user_ctx = project_with_holdouts.create_user_context('test_user', {}) + + result = ds.get_decision_for_flag(feature_flag, user_ctx, config_with_holdouts) + + # TD decision is returned as the primary decision + expect(result.decision).not_to be_nil + expect(result.decision.source).to eq(Optimizely::DecisionService::DECISION_SOURCES['FEATURE_TEST']) + + # holdout_decision is attached separately for impression event + expect(result.holdout_decision).not_to be_nil + expect(result.holdout_decision.source).to eq(Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT']) + end + + it 'returns nil decision with holdout_decision attached when excludeTargetedDeliveries=true and TD returns nil' do + global_holdout = config_with_holdouts.get_holdout('holdout_1') + global_holdout['excludeTargetedDeliveries'] = true + + holdout_variation = global_holdout['variations'].first + holdout_dec = Optimizely::DecisionService::Decision.new( + global_holdout, + holdout_variation, + Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT'], + nil + ) + + # Global holdout hits + allow(ds).to receive(:get_variation_for_holdout) + .with(global_holdout, anything, anything) + .and_return(Optimizely::DecisionService::DecisionResult.new(holdout_dec, false, ['User in holdout'])) + + config_with_holdouts.global_holdouts.reject { |h| h['id'] == 'holdout_1' }.each do |h| + allow(ds).to receive(:get_variation_for_holdout) + .with(h, anything, anything) + .and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) + end + + # Experiment and rollout both return nil + allow(ds).to receive(:get_variation_for_feature_experiment).and_return( + Optimizely::DecisionService::DecisionResult.new(nil, false, ['No experiment match']) + ) + allow(ds).to receive(:get_variation_for_feature_rollout).and_return( + Optimizely::DecisionService::DecisionResult.new(nil, false, ['No rollout match']) + ) + + feature_flag = config_with_holdouts.feature_flag_key_map['boolean_feature'] + user_ctx = project_with_holdouts.create_user_context('test_user', {}) + + result = ds.get_decision_for_flag(feature_flag, user_ctx, config_with_holdouts) + + # Decision is nil — does NOT fall back to holdout as the primary decision + expect(result.decision).to be_nil + + # But holdout_decision is still attached for impression event + expect(result.holdout_decision).not_to be_nil + expect(result.holdout_decision.source).to eq(Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT']) + end + end + describe 'forced decision beats holdout with excludeTargetedDeliveries' do it 'returns forced decision even when excludeTargetedDeliveries is true and holdout covers the rule' do local_holdout = config_with_holdouts.get_holdout('holdout_local_1') From 9dd42dc659e96d6241b23a39f82eaf72a83d97a9 Mon Sep 17 00:00:00 2001 From: esrakartalOpt Date: Wed, 22 Jul 2026 16:38:59 -0500 Subject: [PATCH 4/7] [AI-FSSDK] [FSSDK-12735] Fix holdout impression event tracking and add bypass reason --- lib/optimizely.rb | 1 + lib/optimizely/decision_service.rb | 22 ++++++++++++++++++++-- spec/decision_service_holdout_spec.rb | 2 ++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 056804bc..b0c8035b 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -239,6 +239,7 @@ def create_optimizely_decision(user_context, flag_key, decision, reasons, decide user_id, attributes ) + decision_event_dispatched = true end # Generate all variables map if decide options doesn't include excludeVariables diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index dadbf42d..a4bf699c 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -436,7 +436,16 @@ def get_variation_from_experiment_rule(project_config, flag_key, rule, user, use return VariationResult.new(nil, false, reasons, variation['id']) if variation # Step 2: Global holdout check (when excludeTargetedDeliveries is true, TD rules skip the holdout) - return VariationResult.new(nil, false, reasons, nil, global_holdout_decision) if global_holdout_decision && rule['type'] != Helpers::Constants::EXPERIMENT_TYPES['td'] + if global_holdout_decision + if rule['type'] != Helpers::Constants::EXPERIMENT_TYPES['td'] + return VariationResult.new(nil, false, reasons, nil, global_holdout_decision) + else + holdout_key = global_holdout_decision.experiment ? global_holdout_decision.experiment['key'] : 'unknown' + message = "Holdout '#{holdout_key}' has excludeTargetedDeliveries enabled, continuing to rollout evaluation." + @logger.log(Logger::INFO, message) + reasons.push(message) + end + end # Step 3: Local holdout check local_holdouts = project_config.get_holdouts_for_rule(rule['id']) @@ -467,7 +476,16 @@ def get_variation_from_delivery_rule(project_config, flag_key, rules, rule_index return [nil, variation, skip_to_everyone_else, reasons] if variation # Step 2: Global holdout check - return [global_holdout_decision, nil, skip_to_everyone_else, reasons] if global_holdout_decision && rule['type'] != Helpers::Constants::EXPERIMENT_TYPES['td'] + if global_holdout_decision + if rule['type'] != Helpers::Constants::EXPERIMENT_TYPES['td'] + return [global_holdout_decision, nil, skip_to_everyone_else, reasons] + else + holdout_key = global_holdout_decision.experiment ? global_holdout_decision.experiment['key'] : 'unknown' + message = "Holdout '#{holdout_key}' has excludeTargetedDeliveries enabled, continuing to rollout evaluation." + @logger.log(Logger::INFO, message) + reasons.push(message) + end + end # Step 3: Local holdout check local_holdouts = project_config.get_holdouts_for_rule(rule['id']) diff --git a/spec/decision_service_holdout_spec.rb b/spec/decision_service_holdout_spec.rb index eca71232..39428875 100644 --- a/spec/decision_service_holdout_spec.rb +++ b/spec/decision_service_holdout_spec.rb @@ -745,6 +745,7 @@ expect(notification).to have_key(:variables), 'Notification should contain variables' expect(notification).to have_key(:reasons), 'Notification should contain reasons' expect(notification).to have_key(:decision_event_dispatched), 'Notification should contain decision_event_dispatched' + expect(notification[:decision_event_dispatched]).to eq(true), 'decision_event_dispatched should be true when holdout impression is sent' end end end @@ -1134,6 +1135,7 @@ # TD rule should NOT get holdout decision; should proceed to regular evaluation expect(result.holdout_decision).to be_nil expect(result.variation_id).to eq('122228') + expect(result.reasons).to include(a_string_matching(/Holdout 'global_holdout' has excludeTargetedDeliveries enabled, continuing to rollout evaluation/)) end it 'applies holdout to non-TD (AB) rules even when excludeTargetedDeliveries is true' do From e594b9257f25913c96d0589b2aede39f6ffda1ec Mon Sep 17 00:00:00 2001 From: esrakartalOpt Date: Wed, 22 Jul 2026 17:09:59 -0500 Subject: [PATCH 5/7] Fix lint --- lib/optimizely/decision_service.rb | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index a4bf699c..b1a0e3c8 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -437,14 +437,12 @@ def get_variation_from_experiment_rule(project_config, flag_key, rule, user, use # Step 2: Global holdout check (when excludeTargetedDeliveries is true, TD rules skip the holdout) if global_holdout_decision - if rule['type'] != Helpers::Constants::EXPERIMENT_TYPES['td'] - return VariationResult.new(nil, false, reasons, nil, global_holdout_decision) - else - holdout_key = global_holdout_decision.experiment ? global_holdout_decision.experiment['key'] : 'unknown' - message = "Holdout '#{holdout_key}' has excludeTargetedDeliveries enabled, continuing to rollout evaluation." - @logger.log(Logger::INFO, message) - reasons.push(message) - end + return VariationResult.new(nil, false, reasons, nil, global_holdout_decision) if rule['type'] != Helpers::Constants::EXPERIMENT_TYPES['td'] + + holdout_key = global_holdout_decision.experiment ? global_holdout_decision.experiment['key'] : 'unknown' + message = "Holdout '#{holdout_key}' has excludeTargetedDeliveries enabled, continuing to rollout evaluation." + @logger.log(Logger::INFO, message) + reasons.push(message) end # Step 3: Local holdout check @@ -477,14 +475,12 @@ def get_variation_from_delivery_rule(project_config, flag_key, rules, rule_index # Step 2: Global holdout check if global_holdout_decision - if rule['type'] != Helpers::Constants::EXPERIMENT_TYPES['td'] - return [global_holdout_decision, nil, skip_to_everyone_else, reasons] - else - holdout_key = global_holdout_decision.experiment ? global_holdout_decision.experiment['key'] : 'unknown' - message = "Holdout '#{holdout_key}' has excludeTargetedDeliveries enabled, continuing to rollout evaluation." - @logger.log(Logger::INFO, message) - reasons.push(message) - end + return [global_holdout_decision, nil, skip_to_everyone_else, reasons] if rule['type'] != Helpers::Constants::EXPERIMENT_TYPES['td'] + + holdout_key = global_holdout_decision.experiment ? global_holdout_decision.experiment['key'] : 'unknown' + message = "Holdout '#{holdout_key}' has excludeTargetedDeliveries enabled, continuing to rollout evaluation." + @logger.log(Logger::INFO, message) + reasons.push(message) end # Step 3: Local holdout check From 6e9e851331e662b1e63ac369524eb0329de53a3e Mon Sep 17 00:00:00 2001 From: esrakartalOpt Date: Thu, 23 Jul 2026 13:08:44 -0500 Subject: [PATCH 6/7] Correct the variable name --- lib/optimizely/decision_service.rb | 2 +- lib/optimizely/helpers/constants.rb | 4 +- spec/decision_service_holdout_spec.rb | 54 +++++++++++++-------------- 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index b1a0e3c8..0043e5a8 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -206,7 +206,7 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt next unless holdout_decision.decision - if holdout['excludeTargetedDeliveries'] == true + if holdout['exclude_targeted_deliveries'] == true message = "The user '#{user_id}' is bucketed into holdout '#{holdout['key']}' for feature flag '#{feature_flag['key']}', but targeted deliveries are excluded." @logger.log(Logger::INFO, message) reasons.push(message) diff --git a/lib/optimizely/helpers/constants.rb b/lib/optimizely/helpers/constants.rb index ecba408a..3d369a69 100644 --- a/lib/optimizely/helpers/constants.rb +++ b/lib/optimizely/helpers/constants.rb @@ -350,7 +350,7 @@ module Constants 'includedRules' => { 'type' => %w[array null] }, - 'excludeTargetedDeliveries' => { + 'exclude_targeted_deliveries' => { 'type' => 'boolean' } } @@ -373,7 +373,7 @@ module Constants 'includedRules' => { 'type' => %w[array null] }, - 'excludeTargetedDeliveries' => { + 'exclude_targeted_deliveries' => { 'type' => 'boolean' } } diff --git a/spec/decision_service_holdout_spec.rb b/spec/decision_service_holdout_spec.rb index 39428875..979e754c 100644 --- a/spec/decision_service_holdout_spec.rb +++ b/spec/decision_service_holdout_spec.rb @@ -1035,7 +1035,7 @@ end end - describe 'excludeTargetedDeliveries holdout behavior' do + describe 'exclude_targeted_deliveries holdout behavior' do let(:config_with_holdouts) do Optimizely::DatafileProjectConfig.new( OptimizelySpec::CONFIG_BODY_WITH_HOLDOUTS_JSON, @@ -1058,10 +1058,10 @@ after(:example) { project_with_holdouts&.close } - describe 'global holdout with excludeTargetedDeliveries' do - it 'applies holdout normally when excludeTargetedDeliveries is false' do + describe 'global holdout with exclude_targeted_deliveries' do + it 'applies holdout normally when exclude_targeted_deliveries is false' do global_holdout = config_with_holdouts.get_holdout('holdout_1') - global_holdout['excludeTargetedDeliveries'] = false + global_holdout['exclude_targeted_deliveries'] = false allow(ds).to receive(:get_variation_for_holdout) .with(global_holdout, anything, anything) @@ -1094,7 +1094,7 @@ expect(result.decision.source).to eq(Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT']) end - it 'stores holdout decision but lets TD rules bypass when excludeTargetedDeliveries is true' do + it 'stores holdout decision but lets TD rules bypass when exclude_targeted_deliveries is true' do global_holdout = config_with_holdouts.get_holdout('holdout_1') holdout_decision = Optimizely::DecisionService::Decision.new( @@ -1138,9 +1138,9 @@ expect(result.reasons).to include(a_string_matching(/Holdout 'global_holdout' has excludeTargetedDeliveries enabled, continuing to rollout evaluation/)) end - it 'applies holdout to non-TD (AB) rules even when excludeTargetedDeliveries is true' do + it 'applies holdout to non-TD (AB) rules even when exclude_targeted_deliveries is true' do global_holdout = config_with_holdouts.get_holdout('holdout_1') - global_holdout['excludeTargetedDeliveries'] = true + global_holdout['exclude_targeted_deliveries'] = true holdout_decision = Optimizely::DecisionService::Decision.new( global_holdout, @@ -1171,10 +1171,10 @@ end end - describe 'global holdout with missing excludeTargetedDeliveries (backward compat)' do + describe 'global holdout with missing exclude_targeted_deliveries (backward compat)' do it 'defaults to applying holdout normally when field is absent' do global_holdout = config_with_holdouts.get_holdout('holdout_1') - global_holdout.delete('excludeTargetedDeliveries') + global_holdout.delete('exclude_targeted_deliveries') allow(ds).to receive(:get_variation_for_holdout) .with(global_holdout, anything, anything) @@ -1208,10 +1208,10 @@ end end - describe 'local holdout with excludeTargetedDeliveries' do - it 'local holdout still applies to TD rules even when excludeTargetedDeliveries is true' do + describe 'local holdout with exclude_targeted_deliveries' do + it 'local holdout still applies to TD rules even when exclude_targeted_deliveries is true' do local_holdout = config_with_holdouts.get_holdout('holdout_local_1') - local_holdout['excludeTargetedDeliveries'] = true + local_holdout['exclude_targeted_deliveries'] = true # Set experiment 122227 as TD type experiment = config_with_holdouts.experiment_id_map['122227'] @@ -1251,15 +1251,15 @@ user_profile_tracker ) - # Local holdout MUST still apply — excludeTargetedDeliveries is ignored for local holdouts + # Local holdout MUST still apply — exclude_targeted_deliveries is ignored for local holdouts expect(result.holdout_decision).not_to be_nil expect(result.holdout_decision.source).to eq(Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT']) expect(result.variation_id).to eq(local_holdout['variations'].first['id']) end - it 'applies local holdout for AB rules even when excludeTargetedDeliveries is true' do + it 'applies local holdout for AB rules even when exclude_targeted_deliveries is true' do local_holdout = config_with_holdouts.get_holdout('holdout_local_1') - local_holdout['excludeTargetedDeliveries'] = true + local_holdout['exclude_targeted_deliveries'] = true # Experiment 122227 as AB type experiment = config_with_holdouts.experiment_id_map['122227'] @@ -1304,9 +1304,9 @@ expect(result.holdout_decision.source).to eq(Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT']) end - it 'applies local holdout normally when excludeTargetedDeliveries is false' do + it 'applies local holdout normally when exclude_targeted_deliveries is false' do local_holdout = config_with_holdouts.get_holdout('holdout_local_1') - local_holdout['excludeTargetedDeliveries'] = false + local_holdout['exclude_targeted_deliveries'] = false # Set experiment 122227 as TD type experiment = config_with_holdouts.experiment_id_map['122227'] @@ -1319,7 +1319,7 @@ .and_return(Optimizely::DecisionService::DecisionResult.new(nil, false, [])) end - # Local holdout fires even for TD rule because excludeTargetedDeliveries is false + # Local holdout fires even for TD rule because exclude_targeted_deliveries is false allow(ds).to receive(:get_variation_for_holdout) .with(local_holdout, anything, anything) .and_return( @@ -1346,16 +1346,16 @@ user_profile_tracker ) - # Holdout applies to TD rule because excludeTargetedDeliveries is false + # Holdout applies to TD rule because exclude_targeted_deliveries is false expect(result.holdout_decision).not_to be_nil expect(result.holdout_decision.source).to eq(Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT']) end end - describe 'global holdout excludeTargetedDeliveries integration with get_decision_for_flag' do - it 'returns TD decision with holdout_decision attached when excludeTargetedDeliveries=true and TD succeeds' do + describe 'global holdout exclude_targeted_deliveries integration with get_decision_for_flag' do + it 'returns TD decision with holdout_decision attached when exclude_targeted_deliveries=true and TD succeeds' do global_holdout = config_with_holdouts.get_holdout('holdout_1') - global_holdout['excludeTargetedDeliveries'] = true + global_holdout['exclude_targeted_deliveries'] = true holdout_variation = global_holdout['variations'].first holdout_dec = Optimizely::DecisionService::Decision.new( @@ -1406,9 +1406,9 @@ expect(result.holdout_decision.source).to eq(Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT']) end - it 'returns nil decision with holdout_decision attached when excludeTargetedDeliveries=true and TD returns nil' do + it 'returns nil decision with holdout_decision attached when exclude_targeted_deliveries=true and TD returns nil' do global_holdout = config_with_holdouts.get_holdout('holdout_1') - global_holdout['excludeTargetedDeliveries'] = true + global_holdout['exclude_targeted_deliveries'] = true holdout_variation = global_holdout['variations'].first holdout_dec = Optimizely::DecisionService::Decision.new( @@ -1451,10 +1451,10 @@ end end - describe 'forced decision beats holdout with excludeTargetedDeliveries' do - it 'returns forced decision even when excludeTargetedDeliveries is true and holdout covers the rule' do + describe 'forced decision beats holdout with exclude_targeted_deliveries' do + it 'returns forced decision even when exclude_targeted_deliveries is true and holdout covers the rule' do local_holdout = config_with_holdouts.get_holdout('holdout_local_1') - local_holdout['excludeTargetedDeliveries'] = true + local_holdout['exclude_targeted_deliveries'] = true expect(local_holdout['trafficAllocation'].first['endOfRange']).to eq(10_000) experiment = config_with_holdouts.experiment_id_map['122227'] From 58b69081d32d1b39f3666c126796f5baf6abad07 Mon Sep 17 00:00:00 2001 From: esrakartalOpt Date: Thu, 23 Jul 2026 16:03:25 -0500 Subject: [PATCH 7/7] Add method description back --- lib/optimizely/decision_service.rb | 44 ++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 0043e5a8..335f95b3 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -334,6 +334,17 @@ def get_variations_for_feature_list(project_config, feature_flags, user_context, end def get_variation_for_feature_experiment(project_config, feature_flag, user_context, user_profile_tracker, decide_options = [], global_holdout_decision = nil) + # Gets the variation the user is bucketed into for the feature flag's experiment. + # + # project_config - project_config - Instance of ProjectConfig + # feature_flag - The feature flag the user wants to access + # user_context - Optimizely user context instance + # user_profile_tracker - Tracker for reading and updating user profile of the user + # decide_options - Array of decide options + # global_holdout_decision - Decision from global holdout when excludeTargetedDeliveries is true (nil otherwise) + # + # Returns a DecisionResult containing the decision (or nil if not bucketed), + # an error flag, and an array of decision reasons. decide_reasons = [] user_id = user_context.user_id feature_flag_key = feature_flag['key'] @@ -385,6 +396,16 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_cont end def get_variation_for_feature_rollout(project_config, feature_flag, user_context, global_holdout_decision = nil) + # Determine which variation the user is in for a given rollout. + # Returns the variation of the first experiment the user qualifies for. + # + # project_config - project_config - Instance of ProjectConfig + # feature_flag - The feature flag the user wants to access + # user_context - Optimizely user context instance + # global_holdout_decision - Decision from global holdout when excludeTargetedDeliveries is true (nil otherwise) + # + # Returns a DecisionResult containing the decision (or nil if not bucketed), + # an error flag, and an array of decision reasons. decide_reasons = [] rollout_id = feature_flag['rolloutId'] @@ -427,6 +448,18 @@ def get_variation_for_feature_rollout(project_config, feature_flag, user_context end def get_variation_from_experiment_rule(project_config, flag_key, rule, user, user_profile_tracker, options = [], global_holdout_decision = nil) + # Determine which variation the user is in for a given experiment rule. + # Returns the variation from experiment rules. + # + # project_config - project_config - Instance of ProjectConfig + # flag_key - The feature flag the user wants to access + # rule - An experiment rule key + # user - Optimizely user context instance + # user_profile_tracker - Tracker for reading and updating user profile of the user + # options - Array of decide options + # global_holdout_decision - Decision from global holdout when excludeTargetedDeliveries is true (nil otherwise) + # + # Returns variation_id and reasons reasons = [] # Step 1: Forced decision check @@ -463,6 +496,17 @@ def get_variation_from_experiment_rule(project_config, flag_key, rule, user, use end def get_variation_from_delivery_rule(project_config, flag_key, rules, rule_index, user_context, global_holdout_decision = nil) + # Determine which variation the user is in for a given delivery rule. + # Returns the variation from delivery rules. + # + # project_config - project_config - Instance of ProjectConfig + # flag_key - The feature flag key + # rules - Array of delivery rules + # rule_index - Index of the current rule + # user_context - Optimizely user context instance + # global_holdout_decision - Decision from global holdout when excludeTargetedDeliveries is true (nil otherwise) + # + # Returns variation_id, reasons, and skip_to_everyone_else flag reasons = [] skip_to_everyone_else = false rule = rules[rule_index]