Skip to content

Commit 939cabe

Browse files
committed
Revert "Preventing events from being sent when user is part of a rollout (#85)"
This reverts commit c2a8349.
1 parent 3d7e85f commit 939cabe

File tree

4 files changed

+50
-112
lines changed

4 files changed

+50
-112
lines changed

optimizely/decision_service.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,7 @@
2222
from .user_profile import UserProfile
2323

2424

25-
Decision = namedtuple('Decision', 'experiment variation source')
26-
DECISION_SOURCE_EXPERIMENT = 'experiment'
27-
DECISION_SOURCE_ROLLOUT = 'rollout'
25+
Decision = namedtuple('Decision', 'experiment variation')
2826
RESERVED_BUCKETING_ID_ATTRIBUTE = '$opt_bucketing_id'
2927

3028

@@ -212,7 +210,7 @@ def get_variation_for_rollout(self, rollout, user_id, attributes=None):
212210
if variation:
213211
self.logger.log(enums.LogLevels.DEBUG,
214212
'User "%s" is in variation %s of experiment %s.' % (user_id, variation.key, experiment.key))
215-
return Decision(experiment, variation, DECISION_SOURCE_ROLLOUT)
213+
return Decision(experiment, variation)
216214
else:
217215
# Evaluate no further rules
218216
self.logger.log(enums.LogLevels.DEBUG,
@@ -231,9 +229,9 @@ def get_variation_for_rollout(self, rollout, user_id, attributes=None):
231229
if variation:
232230
self.logger.log(enums.LogLevels.DEBUG,
233231
'User "%s" meets conditions for targeting rule "Everyone Else".' % user_id)
234-
return Decision(everyone_else_experiment, variation, DECISION_SOURCE_ROLLOUT)
232+
return Decision(everyone_else_experiment, variation)
235233

236-
return Decision(None, None, DECISION_SOURCE_ROLLOUT)
234+
return Decision(None, None)
237235

238236
def get_experiment_in_group(self, group, bucketing_id):
239237
""" Determine which experiment in the group the user is bucketed into.
@@ -307,4 +305,4 @@ def get_variation_for_feature(self, feature, user_id, attributes=None):
307305
rollout = self.config.get_rollout_from_id(feature.rolloutId)
308306
return self.get_variation_for_rollout(rollout, user_id, attributes)
309307

310-
return Decision(experiment, variation, DECISION_SOURCE_EXPERIMENT)
308+
return Decision(experiment, variation)

optimizely/optimizely.py

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -382,13 +382,10 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None):
382382
decision = self.decision_service.get_variation_for_feature(feature, user_id, attributes)
383383
if decision.variation:
384384
self.logger.log(enums.LogLevels.INFO, 'Feature "%s" is enabled for user "%s".' % (feature_key, user_id))
385-
# Send event if Decision came from an experiment.
386-
if decision.source == decision_service.DECISION_SOURCE_EXPERIMENT:
387-
self._send_impression_event(decision.experiment,
388-
decision.variation,
389-
user_id,
390-
attributes)
391-
385+
self._send_impression_event(decision.experiment,
386+
decision.variation,
387+
user_id,
388+
attributes)
392389
return True
393390

394391
self.logger.log(enums.LogLevels.INFO, 'Feature "%s" is not enabled for user "%s".' % (feature_key, user_id))

tests/test_decision_service.py

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ def test_get_variation_for_rollout__returns_none_if_no_experiments(self, mock_lo
377377
""" Test that get_variation_for_rollout returns None if there are no experiments (targeting rules). """
378378

379379
no_experiment_rollout = self.project_config.get_rollout_from_id('201111')
380-
self.assertEqual(decision_service.Decision(None, None, decision_service.DECISION_SOURCE_ROLLOUT),
380+
self.assertEqual(decision_service.Decision(None, None),
381381
self.decision_service.get_variation_for_rollout(no_experiment_rollout, 'test_user'))
382382

383383
# Assert no log messages were generated
@@ -393,8 +393,7 @@ def test_get_variation_for_rollout__returns_decision_if_user_in_rollout(self, mo
393393
mock.patch('optimizely.bucketer.Bucketer.bucket',
394394
return_value=self.project_config.get_variation_from_id('211127', '211129')) as mock_bucket:
395395
self.assertEqual(decision_service.Decision(self.project_config.get_experiment_from_id('211127'),
396-
self.project_config.get_variation_from_id('211127', '211129'),
397-
decision_service.DECISION_SOURCE_ROLLOUT),
396+
self.project_config.get_variation_from_id('211127', '211129')),
398397
self.decision_service.get_variation_for_rollout(rollout, 'test_user'))
399398

400399
# Check all log messages
@@ -415,8 +414,7 @@ def test_get_variation_for_rollout__calls_bucket_with_bucketing_id(self, mock_lo
415414
mock.patch('optimizely.bucketer.Bucketer.bucket',
416415
return_value=self.project_config.get_variation_from_id('211127', '211129')) as mock_bucket:
417416
self.assertEqual(decision_service.Decision(self.project_config.get_experiment_from_id('211127'),
418-
self.project_config.get_variation_from_id('211127', '211129'),
419-
decision_service.DECISION_SOURCE_ROLLOUT),
417+
self.project_config.get_variation_from_id('211127', '211129')),
420418
self.decision_service.get_variation_for_rollout(rollout,
421419
'test_user',
422420
{'$opt_bucketing_id': 'user_bucket_value'}))
@@ -440,7 +438,7 @@ def test_get_variation_for_rollout__skips_to_everyone_else_rule(self, mock_loggi
440438

441439
with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True) as mock_audience_check,\
442440
mock.patch('optimizely.bucketer.Bucketer.bucket', return_value=None):
443-
self.assertEqual(decision_service.Decision(None, None, decision_service.DECISION_SOURCE_ROLLOUT),
441+
self.assertEqual(decision_service.Decision(None, None),
444442
self.decision_service.get_variation_for_rollout(rollout, 'test_user'))
445443

446444
# Check that after first experiment, it skips to the last experiment to check
@@ -463,7 +461,7 @@ def test_get_variation_for_rollout__returns_none_for_user_not_in_rollout(self, m
463461
rollout = self.project_config.get_rollout_from_id('211111')
464462

465463
with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=False) as mock_audience_check:
466-
self.assertEqual(decision_service.Decision(None, None, decision_service.DECISION_SOURCE_ROLLOUT),
464+
self.assertEqual(decision_service.Decision(None, None),
467465
self.decision_service.get_variation_for_rollout(rollout, 'test_user'))
468466

469467
# Check that all experiments in rollout layer were checked
@@ -489,10 +487,8 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_experiment(
489487
expected_variation = self.project_config.get_variation_from_id('test_experiment', '111129')
490488
with mock.patch('optimizely.decision_service.DecisionService.get_variation',
491489
return_value=expected_variation) as mock_decision:
492-
self.assertEqual(decision_service.Decision(expected_experiment,
493-
expected_variation,
494-
decision_service.DECISION_SOURCE_EXPERIMENT),
495-
self.decision_service.get_variation_for_feature(feature, 'test_user'))
490+
self.assertEqual(decision_service.Decision(expected_experiment, expected_variation),
491+
self.decision_service.get_variation_for_feature(feature, 'user1'))
496492

497493
mock_decision.assert_called_once_with(
498494
self.project_config.get_experiment_from_key('test_experiment'), 'test_user', None
@@ -531,10 +527,8 @@ def test_get_variation_for_feature__returns_variation_if_user_not_in_experiment_
531527
'optimizely.helpers.audience.is_user_in_experiment',
532528
side_effect=[False, True]) as mock_audience_check, \
533529
mock.patch('optimizely.bucketer.Bucketer.bucket', return_value=expected_variation):
534-
self.assertEqual(decision_service.Decision(expected_experiment,
535-
expected_variation,
536-
decision_service.DECISION_SOURCE_ROLLOUT),
537-
self.decision_service.get_variation_for_feature(feature, 'test_user'))
530+
self.assertEqual(decision_service.Decision(expected_experiment, expected_variation),
531+
self.decision_service.get_variation_for_feature(feature, 'user1'))
538532

539533
self.assertEqual(2, mock_audience_check.call_count)
540534
mock_audience_check.assert_any_call(self.project_config,
@@ -555,10 +549,8 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_group(self,
555549
return_value=self.project_config.get_experiment_from_key('group_exp_1')) as mock_get_experiment_in_group, \
556550
mock.patch('optimizely.decision_service.DecisionService.get_variation',
557551
return_value=expected_variation) as mock_decision:
558-
self.assertEqual(decision_service.Decision(expected_experiment,
559-
expected_variation,
560-
decision_service.DECISION_SOURCE_EXPERIMENT),
561-
self.decision_service.get_variation_for_feature(feature, 'test_user'))
552+
self.assertEqual(decision_service.Decision(expected_experiment, expected_variation),
553+
self.decision_service.get_variation_for_feature(feature, 'user1'))
562554

563555
mock_get_experiment_in_group.assert_called_once_with(self.project_config.get_group('19228'), 'test_user')
564556
mock_decision.assert_called_once_with(self.project_config.get_experiment_from_key('group_exp_1'), 'test_user', None)
@@ -572,8 +564,8 @@ def test_get_variation_for_feature__returns_none_for_user_not_in_group(self, _):
572564
with mock.patch('optimizely.decision_service.DecisionService.get_experiment_in_group',
573565
return_value=None) as mock_get_experiment_in_group, \
574566
mock.patch('optimizely.decision_service.DecisionService.get_variation') as mock_decision:
575-
self.assertEqual(decision_service.Decision(None, None, decision_service.DECISION_SOURCE_EXPERIMENT),
576-
self.decision_service.get_variation_for_feature(feature, 'test_user'))
567+
self.assertEqual(decision_service.Decision(None, None),
568+
self.decision_service.get_variation_for_feature(feature, 'user1'))
577569

578570
mock_get_experiment_in_group.assert_called_once_with(self.project_config.get_group('19228'), 'test_user')
579571
self.assertFalse(mock_decision.called)
@@ -585,10 +577,8 @@ def test_get_variation_for_feature__returns_none_for_user_not_in_experiment(self
585577
expected_experiment = self.project_config.get_experiment_from_key('test_experiment')
586578

587579
with mock.patch('optimizely.decision_service.DecisionService.get_variation', return_value=None) as mock_decision:
588-
self.assertEqual(decision_service.Decision(expected_experiment,
589-
None,
590-
decision_service.DECISION_SOURCE_EXPERIMENT),
591-
self.decision_service.get_variation_for_feature(feature, 'test_user'))
580+
self.assertEqual(decision_service.Decision(expected_experiment, None),
581+
self.decision_service.get_variation_for_feature(feature, 'user1'))
592582

593583
mock_decision.assert_called_once_with(
594584
self.project_config.get_experiment_from_key('test_experiment'), 'test_user', None
@@ -603,10 +593,8 @@ def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_no
603593

604594
with mock.patch('optimizely.decision_service.DecisionService.get_experiment_in_group',
605595
return_value=self.project_config.get_experiment_from_key('group_exp_2')) as mock_decision:
606-
self.assertEqual(decision_service.Decision(expected_experiment,
607-
None,
608-
decision_service.DECISION_SOURCE_EXPERIMENT),
609-
self.decision_service.get_variation_for_feature(feature, 'test_user'))
596+
self.assertEqual(decision_service.Decision(expected_experiment, None),
597+
self.decision_service.get_variation_for_feature(feature, 'user_1'))
610598

611599
mock_decision.assert_called_once_with(self.project_config.get_group('19228'), 'test_user')
612600

0 commit comments

Comments
 (0)