diff --git a/lib/optimizely/config/datafile_project_config.rb b/lib/optimizely/config/datafile_project_config.rb index d6f1d27b..b9e93450 100644 --- a/lib/optimizely/config/datafile_project_config.rb +++ b/lib/optimizely/config/datafile_project_config.rb @@ -196,16 +196,23 @@ def initialize(datafile, logger, error_handler) end flag_id = feature_flag['id'] - applicable_holdouts = [] - applicable_holdouts.concat(@included_holdouts[flag_id]) if @included_holdouts[flag_id] - - @global_holdouts.each_value do |holdout| - excluded_flag_ids = holdout['excludedFlags'] || [] - applicable_holdouts << holdout unless excluded_flag_ids.include?(flag_id) + # Prefer explicit holdouts (includedFlags) over global holdouts + # If multiple explicit holdouts exist, use the first one (deterministic based on datafile order) + applicable_holdout = @included_holdouts[flag_id]&.first + + # Use first global holdout that doesn't exclude this flag if no explicit holdout + unless applicable_holdout + @global_holdouts.each_value do |holdout| + excluded_flag_ids = holdout['excludedFlags'] || [] + unless excluded_flag_ids.include?(flag_id) + applicable_holdout = holdout + break + end + end end - @flag_holdouts_map[key] = applicable_holdouts unless applicable_holdouts.empty? + @flag_holdouts_map[key] = [applicable_holdout] if applicable_holdout end # Adding Holdout variations in variation id and key maps diff --git a/spec/config/datafile_project_config_spec.rb b/spec/config/datafile_project_config_spec.rb index 84dd4509..30ce340f 100644 --- a/spec/config/datafile_project_config_spec.rb +++ b/spec/config/datafile_project_config_spec.rb @@ -1249,17 +1249,18 @@ expect(holdouts).to eq([]) end - it 'should return global holdouts that do not exclude the flag' do + it 'should return only the most specific holdout for the flag' do holdouts = config_with_holdouts.get_holdouts_for_flag('multi_variate_feature') - expect(holdouts.length).to eq(3) - - global_holdout = holdouts.find { |h| h['key'] == 'global_holdout' } - expect(global_holdout).not_to be_nil - expect(global_holdout['id']).to eq('holdout_1') + expect(holdouts.length).to eq(1) + # Should only return the explicit holdout (specific_holdout), not the global ones specific_holdout = holdouts.find { |h| h['key'] == 'specific_holdout' } expect(specific_holdout).not_to be_nil expect(specific_holdout['id']).to eq('holdout_2') + + # Global holdout should NOT be included when explicit holdout exists + global_holdout = holdouts.find { |h| h['key'] == 'global_holdout' } + expect(global_holdout).to be_nil end it 'should not return global holdouts that exclude the flag' do @@ -1274,14 +1275,14 @@ holdouts1 = config_with_holdouts.get_holdouts_for_flag('multi_variate_feature') holdouts2 = config_with_holdouts.get_holdouts_for_flag('multi_variate_feature') expect(holdouts1).to equal(holdouts2) - expect(holdouts1.length).to eq(3) + expect(holdouts1.length).to eq(1) end - it 'should return only global holdouts for flags not specifically targeted' do + it 'should return only one global holdout for flags not specifically targeted' do holdouts = config_with_holdouts.get_holdouts_for_flag('string_single_variable_feature') - # Should only include global holdout (not excluded and no specific targeting) - expect(holdouts.length).to eq(2) + # Should only include one global holdout (not excluded and no specific targeting) + expect(holdouts.length).to eq(1) expect(holdouts.first['key']).to eq('global_holdout') end end