Skip to content

feat(api): Feature variable APIs return default variable value when featureEnabled property is false. #162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ Metrics/AbcSize:
Metrics/BlockLength:
Enabled: false

Metrics/BlockNesting:
Enabled: false

Metrics/ClassLength:
Enabled: false

Expand Down
19 changes: 12 additions & 7 deletions lib/optimizely.rb
Original file line number Diff line number Diff line change
Expand Up @@ -499,15 +499,20 @@ def get_feature_variable_for_type(feature_flag_key, variable_key, variable_type,
source_string = Optimizely::DecisionService::DECISION_SOURCE_EXPERIMENT
end
feature_enabled = variation['featureEnabled']
variation_variable_usages = @config.variation_id_to_variable_usage_map[variation['id']]
variable_id = variable['id']
if variation_variable_usages&.key?(variable_id)
variable_value = variation_variable_usages[variable_id]['value']
@logger.log(Logger::INFO,
"Got variable value '#{variable_value}' for variable '#{variable_key}' of feature flag '#{feature_flag_key}'.")
if feature_enabled == true
variation_variable_usages = @config.variation_id_to_variable_usage_map[variation['id']]
variable_id = variable['id']
if variation_variable_usages&.key?(variable_id)
variable_value = variation_variable_usages[variable_id]['value']
@logger.log(Logger::INFO,
"Got variable value '#{variable_value}' for variable '#{variable_key}' of feature flag '#{feature_flag_key}'.")
else
@logger.log(Logger::DEBUG,
"Variable '#{variable_key}' is not used in variation '#{variation['key']}'. Returning the default variable value '#{variable_value}'.")
end
else
@logger.log(Logger::DEBUG,
"Variable '#{variable_key}' is not used in variation '#{variation['key']}'. Returning the default variable value '#{variable_value}'.")
"Feature '#{feature_flag_key}' for variation '#{variation['key']}' is not enabled. Returning the default variable value '#{variable_value}'.")
end
else
@logger.log(Logger::INFO,
Expand Down
24 changes: 17 additions & 7 deletions spec/project_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2035,7 +2035,7 @@ class InvalidErrorHandler; end
user_id = 'test_user'
user_attributes = {}

it 'should call decision listener with correct variable type and value, when user in experiment and feature is not enabled' do
it 'should call decision listener with default variable type and value, when user in experiment and feature is not enabled' do
integer_feature = project_instance.config.feature_flag_key_map['integer_single_variable_feature']
experiment_to_return = project_instance.config.experiment_id_map[integer_feature['experimentIds'][0]]
variation_to_return = experiment_to_return['variations'][0]
Expand All @@ -2056,7 +2056,7 @@ class InvalidErrorHandler; end
feature_enabled: false,
variable_key: 'integer_variable',
variable_type: 'integer',
variable_value: 42,
variable_value: 7,
source: 'EXPERIMENT',
source_experiment_key: 'test_experiment_integer_feature',
source_variation_key: 'control'
Expand All @@ -2069,7 +2069,12 @@ class InvalidErrorHandler; end
'integer',
user_id,
nil
)).to eq(42)
)).to eq(7)

expect(spy_logger).to have_received(:log).once.with(
Logger::DEBUG,
"Feature 'integer_single_variable_feature' for variation 'control' is not enabled. Returning the default variable value '7'."
)
end

it 'should call decision listener with correct variable type and value, when user in experiment and feature is enabled' do
Expand Down Expand Up @@ -2146,7 +2151,7 @@ class InvalidErrorHandler; end
)).to eq(true)
end

it 'should call listener with correct variable type and value, when user in rollout and feature is not enabled' do
it 'should call listener with default variable type and value, when user in rollout and feature is not enabled' do
experiment_to_return = config_body['rollouts'][0]['experiments'][1]
variation_to_return = experiment_to_return['variations'][0]
decision_to_return = Optimizely::DecisionService::Decision.new(
Expand All @@ -2165,7 +2170,7 @@ class InvalidErrorHandler; end
feature_enabled: false,
variable_key: 'boolean_variable',
variable_type: 'boolean',
variable_value: false,
variable_value: true,
source: 'ROLLOUT',
source_experiment_key: nil,
source_variation_key: nil
Expand All @@ -2178,10 +2183,15 @@ class InvalidErrorHandler; end
'boolean',
user_id,
user_attributes
)).to eq(false)
)).to eq(true)

expect(spy_logger).to have_received(:log).once.with(
Logger::DEBUG,
"Feature 'boolean_single_variable_feature' for variation '177773' is not enabled. Returning the default variable value 'true'."
)
end

it 'should call listener with correct variable type and value, when user neither in experiment nor in rollout' do
it 'should call listener with default variable type and value, when user neither in experiment nor in rollout' do
allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(nil)

expect(project_instance.notification_center).to receive(:send_notifications).once.with(
Expand Down