From ee5c0d7a3672b94f3d94fc1d83d604499b101f91 Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Tue, 14 Dec 2021 18:51:02 +0500 Subject: [PATCH 1/4] fixes --- lib/optimizely.rb | 21 ++++++++++--------- .../config/datafile_project_config.rb | 4 ++-- lib/optimizely/decision_service.rb | 1 + lib/optimizely/optimizely_user_context.rb | 2 +- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 965cbdca..8a7936cd 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -214,8 +214,8 @@ def decide(user_context, key, decide_options = []) experiment = decision.experiment rule_key = experiment ? experiment['key'] : nil variation = decision['variation'] - variation_key = variation['key'] - feature_enabled = variation['featureEnabled'] + variation_key = variation ? variation['key'] : nil + feature_enabled = variation ? variation['featureEnabled'] : false decision_source = decision.source end @@ -298,8 +298,8 @@ def decide_for_keys(user_context, keys, decide_options = []) decisions end - def get_flag_variation_by_key(flag_key, variation_key) - project_config.get_variation_from_flag(flag_key, variation_key) + def get_flag_variation(flag_key, target_value, attribute) + project_config.get_variation_from_flag(flag_key, target_value, attribute) end # Buckets visitor and sends impression event to Optimizely. @@ -1098,12 +1098,13 @@ def send_impression(config, experiment, variation_key, flag_key, rule_key, enabl experiment_id = experiment['id'] experiment_key = experiment['key'] - if experiment_id != '' - variation_id = config.get_variation_id_from_key_by_experiment_id(experiment_id, variation_key) - else - varaition = get_flag_variation_by_key(flag_key, variation_key) - variation_id = varaition ? varaition['id'] : '' - end + variation = get_flag_variation(flag_key, variation_key, 'key') + + variation_id = if !variation + config.get_variation_id_from_key_by_experiment_id(experiment_id, variation_key) + else + variation ? variation['id'] : '' + end metadata = { flag_key: flag_key, diff --git a/lib/optimizely/config/datafile_project_config.rb b/lib/optimizely/config/datafile_project_config.rb index 9d942c6a..321512ae 100644 --- a/lib/optimizely/config/datafile_project_config.rb +++ b/lib/optimizely/config/datafile_project_config.rb @@ -302,9 +302,9 @@ def get_audience_from_id(audience_id) nil end - def get_variation_from_flag(flag_key, variation_key) + def get_variation_from_flag(flag_key, target_value, attribute) variations = @flag_variation_map[flag_key] - return variations.select { |variation| variation['key'] == variation_key }.first if variations + return variations.select { |variation| variation[attribute] == target_value }.first if variations nil end diff --git a/lib/optimizely/decision_service.rb b/lib/optimizely/decision_service.rb index 6fedf287..77fe0f0c 100644 --- a/lib/optimizely/decision_service.rb +++ b/lib/optimizely/decision_service.rb @@ -193,6 +193,7 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_cont next unless variation_id variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id) + variation = project_config.get_variation_from_flag(feature_flag['key'], variation_id, 'id') if variation.nil? return Decision.new(experiment, variation, DECISION_SOURCES['FEATURE_TEST']), decide_reasons end diff --git a/lib/optimizely/optimizely_user_context.rb b/lib/optimizely/optimizely_user_context.rb index d1715b7c..2d6f9500 100644 --- a/lib/optimizely/optimizely_user_context.rb +++ b/lib/optimizely/optimizely_user_context.rb @@ -164,7 +164,7 @@ def find_validated_forced_decision(context) reasons = [] target = rule_key ? "flag (#{flag_key}), rule (#{rule_key})" : "flag (#{flag_key})" if variation_key - variation = @optimizely_client.get_flag_variation_by_key(flag_key, variation_key) + variation = @optimizely_client.get_flag_variation(flag_key, variation_key, 'key') if variation reason = "Variation (#{variation_key}) is mapped to #{target} and user (#{@user_id}) in the forced decision map." reasons.push(reason) From cd080e0208ee712e6a4fd038de20b0b9fa4823ba Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Tue, 14 Dec 2021 19:05:51 +0500 Subject: [PATCH 2/4] unit test fix --- lib/optimizely.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 8a7936cd..b097b030 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -1101,7 +1101,7 @@ def send_impression(config, experiment, variation_key, flag_key, rule_key, enabl variation = get_flag_variation(flag_key, variation_key, 'key') variation_id = if !variation - config.get_variation_id_from_key_by_experiment_id(experiment_id, variation_key) + experiment_id != '' ? config.get_variation_id_from_key_by_experiment_id(experiment_id, variation_key) : '' else variation ? variation['id'] : '' end From 2a28eaef3fa9c3c90c06e2ee5700ed2f6d0a3c7e Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Wed, 15 Dec 2021 23:03:21 +0500 Subject: [PATCH 3/4] unit test added --- lib/optimizely.rb | 9 +++++++++ spec/optimizely_user_context_spec.rb | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index b097b030..5a4749f7 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -298,6 +298,15 @@ def decide_for_keys(user_context, keys, decide_options = []) decisions end + # Gets variation using variation key or id and flag key. + # + # @param flag_key - flag key from which the variation is required. + # @param target_value - variation value either id or key that will be matched. + # @param attribute - string representing variation attribute. + # + # @return [variation] + # @return [nil] if no variation found in flag_variation_map. + def get_flag_variation(flag_key, target_value, attribute) project_config.get_variation_from_flag(flag_key, target_value, attribute) end diff --git a/spec/optimizely_user_context_spec.rb b/spec/optimizely_user_context_spec.rb index 7a7d7bdf..035901ed 100644 --- a/spec/optimizely_user_context_spec.rb +++ b/spec/optimizely_user_context_spec.rb @@ -284,6 +284,29 @@ expect(decision.reasons).to eq(['Variation (b) is mapped to flag (feature_1), rule (exp_with_audience) and user (tester) in the forced decision map.']) end + it 'should return an expected decision object when forced decision is called and variation of different experiment but same flag key' do + user_id = 'tester' + feature_key = 'feature_1' + original_attributes = {} + user_context_obj = Optimizely::OptimizelyUserContext.new(forced_decision_project_instance, user_id, original_attributes) + context = Optimizely::OptimizelyUserContext::OptimizelyDecisionContext.new(feature_key, 'exp_with_audience') + forced_decision = Optimizely::OptimizelyUserContext::OptimizelyForcedDecision.new('3324490633') + user_context_obj.set_forced_decision(context, forced_decision) + expected = expect do + decision = user_context_obj.decide(feature_key, [Optimizely::Decide::OptimizelyDecideOption::INCLUDE_REASONS]) + expect(decision.variation_key).to eq('3324490633') + expect(decision.rule_key).to eq('exp_with_audience') + expect(decision.enabled).to be false + expect(decision.flag_key).to eq(feature_key) + expect(decision.user_context.user_id).to eq(user_id) + expect(decision.user_context.user_attributes.length).to eq(0) + expect(decision.user_context.forced_decisions.length).to eq(1) + expect(decision.user_context.forced_decisions).to eq(context => forced_decision) + expect(decision.reasons).to eq(['Variation (3324490633) is mapped to flag (feature_1), rule (exp_with_audience) and user (tester) in the forced decision map.']) + end + expected.to raise_error + end + it 'should return correct variation if rule in forced decision is deleted' do impression_log_url = 'https://logx.optimizely.com/v1/events' time_now = Time.now From 402763f529ec02e70ec25574be4a82a0edbb45a3 Mon Sep 17 00:00:00 2001 From: ozayr-zaviar Date: Thu, 16 Dec 2021 15:09:26 +0500 Subject: [PATCH 4/4] refactor --- lib/optimizely.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/optimizely.rb b/lib/optimizely.rb index 5a4749f7..de1315ce 100644 --- a/lib/optimizely.rb +++ b/lib/optimizely.rb @@ -1107,13 +1107,12 @@ def send_impression(config, experiment, variation_key, flag_key, rule_key, enabl experiment_id = experiment['id'] experiment_key = experiment['key'] - variation = get_flag_variation(flag_key, variation_key, 'key') + variation_id = config.get_variation_id_from_key_by_experiment_id(experiment_id, variation_key) unless experiment_id.empty? - variation_id = if !variation - experiment_id != '' ? config.get_variation_id_from_key_by_experiment_id(experiment_id, variation_key) : '' - else - variation ? variation['id'] : '' - end + unless variation_id + variation = !flag_key.empty? ? get_flag_variation(flag_key, variation_key, 'key') : nil + variation_id = variation ? variation['id'] : '' + end metadata = { flag_key: flag_key,