Skip to content

Commit 4c513ba

Browse files
authored
Fix isFeatureEnabled to always return a boolean value. (#79)
1 parent 34fbd4d commit 4c513ba

File tree

2 files changed

+22
-19
lines changed

2 files changed

+22
-19
lines changed

lib/optimizely.rb

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ def set_forced_variation(experiment_key, user_id, variation_key)
160160
#
161161
# experiment_key - String - key identifying the experiment.
162162
# user_id - String - The user ID to be used for bucketing.
163-
# variation_key - The variation key specifies the variation which the user will
163+
# variation_key - The variation key specifies the variation which the user will
164164
# be forced into. If nil, then clear the existing experiment-to-variation mapping.
165165
#
166166
# Returns - Boolean - indicates if the set completed successfully.
@@ -234,10 +234,12 @@ def track(event_key, user_id, attributes = nil, event_tags = nil)
234234
rescue => e
235235
@logger.log(Logger::ERROR, "Unable to dispatch conversion event. Error: #{e}")
236236
end
237+
237238
@notification_center.send_notifications(
238239
NotificationCenter::NOTIFICATION_TYPES[:TRACK],
239240
event_key, user_id, attributes, event_tags, conversion_event
240241
)
242+
return nil
241243
end
242244

243245
def is_feature_enabled(feature_flag_key, user_id, attributes = nil)
@@ -255,7 +257,7 @@ def is_feature_enabled(feature_flag_key, user_id, attributes = nil)
255257
unless @is_valid
256258
logger = SimpleLogger.new
257259
logger.log(Logger::ERROR, InvalidDatafileError.new('is_feature_enabled').message)
258-
return nil
260+
return false
259261
end
260262

261263
feature_flag = @config.get_feature_flag_from_key(feature_flag_key)
@@ -408,7 +410,7 @@ def get_feature_variable_for_type(feature_flag_key, variable_key, variable_type,
408410
@logger.log(Logger::INFO, "No feature flag was found for key '#{feature_flag_key}'.")
409411
return nil
410412
end
411-
413+
412414
variable = @config.get_feature_variable(feature_flag, variable_key)
413415

414416
# Error message logged in ProjectConfig- get_feature_flag_from_key

spec/project_spec.rb

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ class InvalidErrorHandler; end
121121
before(:example) do
122122
allow(Time).to receive(:now).and_return(time_now)
123123
allow(SecureRandom).to receive(:uuid).and_return('a68cf1ad-0393-4e18-af87-efe8f01a7c9c');
124+
124125
@expected_activate_params = {
125126
account_id: '12001',
126127
project_id: '111001',
@@ -271,7 +272,7 @@ class InvalidErrorHandler; end
271272
instance_of(Optimizely::Event)
272273
)
273274
project_instance.activate('test_experiment', 'test_user')
274-
275+
275276
expect(spy_logger).to have_received(:log).once.with(Logger::INFO, include("Dispatching impression event to" \
276277
" URL #{impression_log_url} with params #{params}"))
277278
end
@@ -288,7 +289,7 @@ class InvalidErrorHandler; end
288289
expect { project_instance.activate('test_experiment', 'test_user', 'invalid') }
289290
.to raise_error(Optimizely::InvalidAttributeFormatError)
290291
end
291-
292+
292293
it 'should override the audience check if the user is whitelisted to a specific variation' do
293294
params = @expected_activate_params
294295
params[:visitors][0][:visitor_id] = 'forced_audience_user'
@@ -330,7 +331,7 @@ class InvalidErrorHandler; end
330331
before(:example) do
331332
allow(Time).to receive(:now).and_return(time_now)
332333
allow(SecureRandom).to receive(:uuid).and_return('a68cf1ad-0393-4e18-af87-efe8f01a7c9c');
333-
334+
334335
@expected_track_event_params = {
335336
account_id: '12001',
336337
project_id: '111001',
@@ -365,7 +366,7 @@ class InvalidErrorHandler; end
365366
project_instance.track('test_event', 'test_user')
366367
expect(project_instance.event_dispatcher).to have_received(:dispatch_event).with(Optimizely::Event.new(:post, conversion_log_url, params, post_headers)).once
367368
end
368-
369+
369370
it 'should properly track an event by calling dispatch_event with right params after forced variation' do
370371
params = @expected_track_event_params
371372
params[:visitors][0][:snapshots][0][:decisions][0][:variation_id] = '111129'
@@ -548,15 +549,15 @@ class InvalidErrorHandler; end
548549
expect(project_instance.get_variation('test_experiment_with_audience', 'test_user', user_attributes))
549550
.to eq('control_with_audience')
550551
end
551-
552+
552553
it 'should have get_variation return expected variation with bucketing id attribute when audience conditions match' do
553554
user_attributes = {
554555
'browser_type' => 'firefox',
555556
OptimizelySpec::RESERVED_ATTRIBUTE_KEY_BUCKETING_ID => 'pid'
556-
}
557+
}
557558
expect(project_instance.get_variation('test_experiment_with_audience', 'test_user', user_attributes))
558559
.to eq('control_with_audience')
559-
end
560+
end
560561

561562
it 'should have get_variation return nil when attributes are invalid' do
562563
allow(project_instance).to receive(:attributes_valid?).and_return(false)
@@ -586,7 +587,7 @@ class InvalidErrorHandler; end
586587
user_attributes = {
587588
'browser_type' => 'firefox',
588589
OptimizelySpec::RESERVED_ATTRIBUTE_KEY_BUCKETING_ID => 'pid'
589-
}
590+
}
590591
expect(project_instance.get_variation('test_experiment_not_started', 'test_user',user_attributes)).to eq(nil)
591592
end
592593

@@ -647,12 +648,12 @@ class InvalidErrorHandler; end
647648
}
648649
end
649650

650-
it 'should return nil when called with invalid project config' do
651+
it 'should return false when called with invalid project config' do
651652
invalid_project = Optimizely::Project.new('invalid',nil,spy_logger)
652-
expect(invalid_project.is_feature_enabled('totally_invalid_feature_key', 'test_user')).to be nil
653-
653+
expect(invalid_project.is_feature_enabled('totally_invalid_feature_key', 'test_user')).to be false
654+
654655
end
655-
656+
656657
it 'should return false when the feature flag key is invalid' do
657658
expect(project_instance.is_feature_enabled('totally_invalid_feature_key', 'test_user')).to be false
658659
expect(spy_logger).to have_received(:log).once.with(Logger::ERROR, "Feature flag key 'totally_invalid_feature_key' is not in datafile.")
@@ -691,14 +692,14 @@ class InvalidErrorHandler; end
691692
variation_to_return,
692693
Optimizely::DecisionService::DECISION_SOURCE_EXPERIMENT
693694
)
694-
695+
695696
expect(project_instance.notification_center).to receive(:send_notifications)
696697
.with(
697698
Optimizely::NotificationCenter::NOTIFICATION_TYPES[:ACTIVATE],
698699
experiment_to_return, 'test_user', nil, variation_to_return,
699700
instance_of(Optimizely::Event)
700701
)
701-
702+
702703
allow(project_instance.decision_service).to receive(:get_variation_for_feature).and_return(decision_to_return)
703704

704705
expected_params = @expected_bucketed_params
@@ -976,7 +977,7 @@ class InvalidErrorHandler; end
976977
expect(project_instance.get_variation('test_experiment','test_user')). to eq('variation')
977978
end
978979

979-
# setForcedVariation on a running experiment with audience enabled and then call getVariation on that same experiment with invalid attributes.
980+
# setForcedVariation on a running experiment with audience enabled and then call getVariation on that same experiment with invalid attributes.
980981
it 'should return nil when getVariation called on audience enabled running experiment with invalid attributes' do
981982
project_instance.set_forced_variation('test_experiment_with_audience','test_user','control_with_audience')
982983
expect { project_instance.get_variation('test_experiment_with_audience', 'test_user', 'invalid') }
@@ -986,7 +987,7 @@ class InvalidErrorHandler; end
986987
# Adding this test case to cover this in code coverage. All test cases for getForceVariation are present in
987988
# project_config_spec.rb which test the get_force_variation method in project_config. The one in optimizely.rb
988989
# only calls the other one
989-
990+
990991
# getForceVariation on a running experiment after setforcevariation
991992
it 'should return expected variation id when get_forced_variation is called on a running experiment after setForcedVariation' do
992993
project_instance.set_forced_variation('test_experiment','test_user','variation')

0 commit comments

Comments
 (0)