Skip to content

Commit 96ab364

Browse files
zashraf1985zashraf1985
committed
incorporated review feedback
1 parent c59e4cd commit 96ab364

File tree

5 files changed

+46
-45
lines changed

5 files changed

+46
-45
lines changed

Diff for: ‎src/Optimizely/Event/Builder/EventBuilder.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ private function getConversionParams($eventEntity, $eventTags)
229229
*
230230
* @return LogEvent Event object to be sent to dispatcher.
231231
*/
232-
public function createImpressionEvent($config, $experimentKey, $variationKey, $flagKey, $ruleKey, $ruleType, $userId, $attributes, $enabled)
232+
public function createImpressionEvent($config, $experimentKey, $variationKey, $flagKey, $ruleKey, $ruleType, $enabled, $userId, $attributes)
233233
{
234234
$eventParams = $this->getCommonParams($config, $userId, $attributes);
235235

Diff for: ‎src/Optimizely/Optimizely.php

+5-5
Original file line numberDiff line numberDiff line change
@@ -195,10 +195,10 @@ private function validateUserInputs($attributes, $eventTags = null)
195195
* @param array Associative array of user attributes
196196
* @param DatafileProjectConfig DatafileProjectConfig instance
197197
*/
198-
protected function sendImpressionEvent($config, $experimentKey, $variationKey, $flagKey, $ruleKey, $ruleType, $userId, $attributes, $enabled)
198+
protected function sendImpressionEvent($config, $experimentKey, $variationKey, $flagKey, $ruleKey, $ruleType, $enabled, $userId, $attributes)
199199
{
200200
$impressionEvent = $this->_eventBuilder
201-
->createImpressionEvent($config, $experimentKey, $variationKey, $flagKey, $ruleKey, $ruleType, $userId, $attributes, $enabled);
201+
->createImpressionEvent($config, $experimentKey, $variationKey, $flagKey, $ruleKey, $ruleType, $enabled, $userId, $attributes);
202202
$this->_logger->log(Logger::INFO, sprintf('Activating user "%s" in experiment "%s".', $userId, $experimentKey));
203203
$this->_logger->log(
204204
Logger::DEBUG,
@@ -274,7 +274,7 @@ public function activate($experimentKey, $userId, $attributes = null)
274274
return null;
275275
}
276276

277-
$this->sendImpressionEvent($config, $experimentKey, $variationKey, '', $experimentKey, FeatureDecision::DECITION_SOURCE_EXPERIMENT, $userId, $attributes, true);
277+
$this->sendImpressionEvent($config, $experimentKey, $variationKey, '', $experimentKey, FeatureDecision::DECITION_SOURCE_EXPERIMENT, true, $userId, $attributes);
278278

279279
return $variationKey;
280280
}
@@ -557,7 +557,7 @@ public function isFeatureEnabled($featureFlagKey, $userId, $attributes = null)
557557

558558
if ($config->getSendFlagDecisions() && ($decision->getSource() == FeatureDecision::DECISION_SOURCE_ROLLOUT || !$variation)) {
559559
$ruleKey = $decision->getExperiment() ? $decision->getExperiment()->getKey() : '';
560-
$this->sendImpressionEvent($config, $ruleKey, $variation ? $variation->getKey() : '', $featureFlagKey, $ruleKey, $decision->getSource(), $userId, $attributes, $featureEnabled);
560+
$this->sendImpressionEvent($config, $ruleKey, $variation ? $variation->getKey() : '', $featureFlagKey, $ruleKey, $decision->getSource(), $featureEnabled, $userId, $attributes);
561561
}
562562

563563
if ($variation) {
@@ -569,7 +569,7 @@ public function isFeatureEnabled($featureFlagKey, $userId, $attributes = null)
569569
'variationKey'=> $variation->getKey()
570570
);
571571

572-
$this->sendImpressionEvent($config, $experimentKey, $variation->getKey(), $featureFlagKey, $experimentKey, $decision->getSource(), $userId, $attributes, $featureEnabled);
572+
$this->sendImpressionEvent($config, $experimentKey, $variation->getKey(), $featureFlagKey, $experimentKey, $decision->getSource(), $featureEnabled, $userId, $attributes);
573573
} else {
574574
$this->_logger->log(Logger::INFO, "The user '{$userId}' is not being experimented on Feature Flag '{$featureFlagKey}'.");
575575
}

Diff for: ‎tests/EventTests/EventBuilderTest.php

+20-20
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,9 @@ public function testCreateImpressionEventNoAttributesNoValue()
147147
'test_experiment',
148148
'test_experiment',
149149
'experiment',
150+
true,
150151
$this->testUserId,
151-
null,
152-
true
152+
null
153153
);
154154

155155
$logEvent = $this->fakeParamsToReconcile($logEvent);
@@ -207,9 +207,9 @@ public function testCreateImpressionEventWithAttributesNoValue()
207207
'test_experiment',
208208
'test_experiment',
209209
'experiment',
210+
true,
210211
$this->testUserId,
211-
$userAttributes,
212-
true
212+
$userAttributes
213213
);
214214

215215
$logEvent = $this->fakeParamsToReconcile($logEvent);
@@ -248,9 +248,9 @@ public function testCreateImpressionEventWithFalseAttributesNoValue()
248248
'test_experiment',
249249
'test_experiment',
250250
'experiment',
251+
true,
251252
$this->testUserId,
252-
$userAttributes,
253-
true
253+
$userAttributes
254254
);
255255

256256
$logEvent = $this->fakeParamsToReconcile($logEvent);
@@ -290,9 +290,9 @@ public function testCreateImpressionEventWithZeroAttributesNoValue()
290290
'test_experiment',
291291
'test_experiment',
292292
'experiment',
293+
true,
293294
$this->testUserId,
294-
$userAttributes,
295-
true
295+
$userAttributes
296296
);
297297

298298
$logEvent = $this->fakeParamsToReconcile($logEvent);
@@ -322,9 +322,9 @@ public function testCreateImpressionEventWithInvalidAttributesNoValue()
322322
'test_experiment',
323323
'test_experiment',
324324
'experiment',
325+
true,
325326
$this->testUserId,
326-
$userAttributes,
327-
true
327+
$userAttributes
328328
);
329329

330330
$logEvent = $this->fakeParamsToReconcile($logEvent);
@@ -363,9 +363,9 @@ public function testCreateImpressionEventWithUserAgentWhenBotFilteringIsEnabled(
363363
'test_experiment',
364364
'test_experiment',
365365
'experiment',
366+
true,
366367
$this->testUserId,
367-
$userAttributes,
368-
true
368+
$userAttributes
369369
);
370370

371371
$logEvent = $this->fakeParamsToReconcile($logEvent);
@@ -411,9 +411,9 @@ public function testCreateImpressionEventWithInvalidAttributeTypes()
411411
'test_experiment',
412412
'test_experiment',
413413
'experiment',
414+
true,
414415
$this->testUserId,
415-
$userAttributes,
416-
true
416+
$userAttributes
417417
);
418418

419419
$logEvent = $this->fakeParamsToReconcile($logEvent);
@@ -462,9 +462,9 @@ public function testCreateImpressionEventWithUserAgentWhenBotFilteringIsDisabled
462462
'test_experiment',
463463
'test_experiment',
464464
'experiment',
465+
true,
465466
$this->testUserId,
466-
$userAttributes,
467-
true
467+
$userAttributes
468468
);
469469

470470
$logEvent = $this->fakeParamsToReconcile($logEvent);
@@ -509,9 +509,9 @@ public function testCreateImpressionEventWithUserAgentWhenBotFilteringIsNull()
509509
'test_experiment',
510510
'test_experiment',
511511
'experiment',
512+
true,
512513
$this->testUserId,
513-
$userAttributes,
514-
true
514+
$userAttributes
515515
);
516516

517517
$logEvent = $this->fakeParamsToReconcile($logEvent);
@@ -839,9 +839,9 @@ public function testCreateImpressionEventWithBucketingIDAttribute()
839839
'test_experiment',
840840
'test_experiment',
841841
'experiment',
842+
true,
842843
$this->testUserId,
843-
$userAttributes,
844-
true
844+
$userAttributes
845845
);
846846

847847

Diff for: ‎tests/OptimizelyTest.php

+18-17
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ public function testActivateWithEmptyUserID()
392392
// Verify that sendImpressionEvent is called with expected attributes
393393
$optimizelyMock->expects($this->exactly(1))
394394
->method('sendImpressionEvent')
395-
->with($this->projectConfig, 'test_experiment', 'variation', '', 'test_experiment', 'experiment', '', $userAttributes);
395+
->with($this->projectConfig, 'test_experiment', 'variation', '', 'test_experiment', 'experiment', true, '', $userAttributes);
396396

397397
// Call activate
398398
$this->assertEquals('variation', $optimizelyMock->activate('test_experiment', '', $userAttributes));
@@ -487,7 +487,7 @@ public function testActivateNoAudienceNoAttributes()
487487
// Verify that sendImpression is called with expected params
488488
$optimizelyMock->expects($this->exactly(1))
489489
->method('sendImpressionEvent')
490-
->with($this->projectConfig, 'group_experiment_1', 'group_exp_1_var_2', '', 'group_experiment_1', 'experiment', 'user_1', null);
490+
->with($this->projectConfig, 'group_experiment_1', 'group_exp_1_var_2', '', 'group_experiment_1', 'experiment', true, 'user_1', null);
491491

492492
// Call activate
493493
$this->assertSame('group_exp_1_var_2', $optimizelyMock->activate('group_experiment_1', 'user_1'));
@@ -520,7 +520,7 @@ public function testActivateNoAudienceNoAttributesAfterSetForcedVariation()
520520
// Verify that sendImpression is called with expected params
521521
$optimizelyMock->expects($this->exactly(1))
522522
->method('sendImpressionEvent')
523-
->with($this->projectConfig, 'group_experiment_1', 'group_exp_1_var_2', '', 'group_experiment_1', 'experiment', 'user_1', null);
523+
->with($this->projectConfig, 'group_experiment_1', 'group_exp_1_var_2', '', 'group_experiment_1', 'experiment', true, 'user_1', null);
524524

525525
// set forced variation
526526
$this->assertTrue($optimizelyMock->setForcedVariation($experimentKey, $userId, $variationKey), 'Set variation for paused experiment should have failed.');
@@ -581,7 +581,7 @@ public function testActivateWithAttributes()
581581
// Verify that sendImpressionEvent is called with expected attributes
582582
$optimizelyMock->expects($this->exactly(1))
583583
->method('sendImpressionEvent')
584-
->with($this->projectConfig, 'test_experiment', 'control', '', 'test_experiment', 'experiment', 'test_user', $userAttributes);
584+
->with($this->projectConfig, 'test_experiment', 'control', '', 'test_experiment', 'experiment', true, 'test_user', $userAttributes);
585585

586586
// Call activate
587587
$this->assertEquals('control', $optimizelyMock->activate('test_experiment', 'test_user', $userAttributes));
@@ -614,7 +614,7 @@ public function testActivateWithAttributesOfDifferentTypes()
614614
// Verify that sendImpressionEvent is called with expected attributes
615615
$optimizelyMock->expects($this->exactly(1))
616616
->method('sendImpressionEvent')
617-
->with($this->projectConfig, 'test_experiment', 'control', '', 'test_experiment', 'experiment', 'test_user', $userAttributes);
617+
->with($this->projectConfig, 'test_experiment', 'control', '', 'test_experiment', 'experiment', true, 'test_user', $userAttributes);
618618

619619
// Call activate
620620
$this->assertEquals('control', $optimizelyMock->activate('test_experiment', 'test_user', $userAttributes));
@@ -639,7 +639,7 @@ public function testActivateWithAttributesTypedAudienceMatch()
639639
// Verify that sendImpressionEvent is called with expected attributes
640640
$optimizelyMock->expects($this->at(0))
641641
->method('sendImpressionEvent')
642-
->with($this->projectConfigForTypedAudience, 'typed_audience_experiment', 'A', '', 'typed_audience_experiment', 'experiment', 'test_user', $userAttributes);
642+
->with($this->projectConfigForTypedAudience, 'typed_audience_experiment', 'A', '', 'typed_audience_experiment', 'experiment', true, 'test_user', $userAttributes);
643643

644644
// Should be included via exact match string audience with id '3468206642'
645645
$this->assertEquals('A', $optimizelyMock->activate('typed_audience_experiment', 'test_user', $userAttributes));
@@ -651,7 +651,7 @@ public function testActivateWithAttributesTypedAudienceMatch()
651651
// Verify that sendImpressionEvent is called with expected attributes
652652
$optimizelyMock->expects($this->at(0))
653653
->method('sendImpressionEvent')
654-
->with($this->projectConfigForTypedAudience, 'typed_audience_experiment', 'A', '', 'typed_audience_experiment', 'experiment', 'test_user', $userAttributes);
654+
->with($this->projectConfigForTypedAudience, 'typed_audience_experiment', 'A', '', 'typed_audience_experiment', 'experiment', true, 'test_user', $userAttributes);
655655

656656
//Should be included via exact match number audience with id '3468206646'
657657
$this->assertEquals('A', $optimizelyMock->activate('typed_audience_experiment', 'test_user', $userAttributes));
@@ -691,7 +691,7 @@ public function testActivateWithAttributesComplexAudienceMatch()
691691
// Verify that sendImpressionEvent is called once with expected attributes
692692
$optimizelyMock->expects($this->exactly(1))
693693
->method('sendImpressionEvent')
694-
->with($this->projectConfigForTypedAudience, 'audience_combinations_experiment', 'A', '', 'audience_combinations_experiment', 'experiment', 'test_user', $userAttributes);
694+
->with($this->projectConfigForTypedAudience, 'audience_combinations_experiment', 'A', '', 'audience_combinations_experiment', 'experiment', true, 'test_user', $userAttributes);
695695

696696
// Should be included via substring match string audience with id '3988293898', and
697697
// exact match number audience with id '3468206646'
@@ -2554,7 +2554,7 @@ public function testIsFeatureEnabledGivenFeatureExperimentAndFeatureEnabledIsTru
25542554
// assert that sendImpressionEvent is called with expected params
25552555
$optimizelyMock->expects($this->exactly(1))
25562556
->method('sendImpressionEvent')
2557-
->with($this->projectConfig, 'test_experiment_double_feature', 'control', 'double_single_variable_feature', 'test_experiment_double_feature', FeatureDecision::DECISION_SOURCE_FEATURE_TEST, 'user_id', []);
2557+
->with($this->projectConfig, 'test_experiment_double_feature', 'control', 'double_single_variable_feature', 'test_experiment_double_feature', FeatureDecision::DECISION_SOURCE_FEATURE_TEST, true, 'user_id', []);
25582558

25592559
$this->loggerMock->expects($this->at(0))
25602560
->method('log')
@@ -2659,7 +2659,7 @@ public function testIsFeatureEnabledGivenFeatureExperimentAndFeatureEnabledIsFal
26592659

26602660
$optimizelyMock->expects($this->exactly(1))
26612661
->method('sendImpressionEvent')
2662-
->with($this->projectConfig, 'test_experiment_double_feature', 'variation', 'double_single_variable_feature', 'test_experiment_double_feature', FeatureDecision::DECISION_SOURCE_FEATURE_TEST, 'user_id', []);
2662+
->with($this->projectConfig, 'test_experiment_double_feature', 'variation', 'double_single_variable_feature', 'test_experiment_double_feature', FeatureDecision::DECISION_SOURCE_FEATURE_TEST, false, 'user_id', []);
26632663

26642664
$this->loggerMock->expects($this->at(0))
26652665
->method('log')
@@ -3042,7 +3042,7 @@ public function testIsFeatureEnabledWithEmptyUserID()
30423042
// assert that sendImpressionEvent is called with expected params
30433043
$optimizelyMock->expects($this->exactly(1))
30443044
->method('sendImpressionEvent')
3045-
->with($this->projectConfig, 'test_experiment_double_feature', 'control', 'double_single_variable_feature', 'test_experiment_double_feature', FeatureDecision::DECISION_SOURCE_FEATURE_TEST, '', []);
3045+
->with($this->projectConfig, 'test_experiment_double_feature', 'control', 'double_single_variable_feature', 'test_experiment_double_feature', FeatureDecision::DECISION_SOURCE_FEATURE_TEST, true, '', []);
30463046

30473047
$this->loggerMock->expects($this->at(0))
30483048
->method('log')
@@ -4682,9 +4682,9 @@ public function testSendImpressionEventWithNoAttributes()
46824682
'group_experiment_1',
46834683
'group_experiment_1',
46844684
'experiment',
4685+
true,
46854686
'user_1',
4686-
null,
4687-
true
4687+
null
46884688
)
46894689
->willReturn(
46904690
new LogEvent(
@@ -4735,7 +4735,7 @@ public function testSendImpressionEventWithNoAttributes()
47354735
'Dispatching impression event to URL logx.optimizely.com/decision with params {"param1":"val1","param2":"val2"}.'
47364736
);
47374737

4738-
$optlyObject->sendImpressionEvent($this->projectConfig, 'group_experiment_1', 'group_exp_1_var_2', 'group_experiment_1', 'group_experiment_1', 'experiment', 'user_1', null, true);
4738+
$optlyObject->sendImpressionEvent($this->projectConfig, 'group_experiment_1', 'group_exp_1_var_2', 'group_experiment_1', 'group_experiment_1', 'experiment', true, 'user_1', null);
47394739
}
47404740

47414741
public function testSendImpressionEventDispatchFailure()
@@ -4758,7 +4758,7 @@ public function testSendImpressionEventDispatchFailure()
47584758
->method('log')
47594759
->with(Logger::ERROR, 'Unable to dispatch impression event. Error ');
47604760

4761-
$optlyObject->sendImpressionEvent($this->projectConfig, 'test_experiment', 'control', 'test_experiment', 'test_experiment', 'experiment', 'test_user', [], true);
4761+
$optlyObject->sendImpressionEvent($this->projectConfig, 'test_experiment', 'control', 'test_experiment', 'test_experiment', 'experiment', true, 'test_user', []);
47624762
}
47634763

47644764
public function testSendImpressionEventWithAttributes()
@@ -4781,6 +4781,7 @@ public function testSendImpressionEventWithAttributes()
47814781
'test_experiment',
47824782
'test_experiment',
47834783
'experiment',
4784+
true,
47844785
'test_user',
47854786
$userAttributes
47864787
)
@@ -4823,7 +4824,7 @@ public function testSendImpressionEventWithAttributes()
48234824

48244825
$optlyObject->notificationCenter = $this->notificationCenterMock;
48254826

4826-
$optlyObject->sendImpressionEvent($this->projectConfig, 'test_experiment', 'control', 'test_experiment', 'test_experiment', 'experiment', 'test_user', $userAttributes, true);
4827+
$optlyObject->sendImpressionEvent($this->projectConfig, 'test_experiment', 'control', 'test_experiment', 'test_experiment', 'experiment', true, 'test_user', $userAttributes);
48274828
}
48284829

48294830
/*
@@ -4999,7 +5000,7 @@ public function testRolloutSendImpressionWhenSendFlagDecisionFlagInDatafile()
49995000
// Verify that sendImpressionEvent is called with expected attributes
50005001
$optimizelyMock->expects($this->exactly(1))
50015002
->method('sendImpressionEvent')
5002-
->with($this->anything(), 'rollout_1_exp_1', '177771', 'boolean_single_variable_feature', 'rollout_1_exp_1', 'rollout', 'user_id', []);
5003+
->with($this->anything(), 'rollout_1_exp_1', '177771', 'boolean_single_variable_feature', 'rollout_1_exp_1', 'rollout', false, 'user_id', []);
50035004

50045005
$this->assertTrue($optimizelyMock->isFeatureEnabled('boolean_single_variable_feature', 'user_id', []));
50055006
}

Diff for: ‎tests/TestData.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -1694,9 +1694,9 @@ public function generateBucketValue($bucketingId)
16941694
*/
16951695
class OptimizelyTester extends Optimizely
16961696
{
1697-
public function sendImpressionEvent($config, $experimentKey, $variationKey, $flagKey, $ruleKey, $ruleType, $userId, $attributes, $enabled)
1697+
public function sendImpressionEvent($config, $experimentKey, $variationKey, $flagKey, $ruleKey, $ruleType, $enabled, $userId, $attributes)
16981698
{
1699-
parent::sendImpressionEvent($config, $experimentKey, $variationKey, $flagKey, $ruleKey, $ruleType, $userId, $attributes, $enabled);
1699+
parent::sendImpressionEvent($config, $experimentKey, $variationKey, $flagKey, $ruleKey, $ruleType, $enabled, $userId, $attributes);
17001700
}
17011701

17021702
public function validateInputs(array $values, $logLevel = Logger::ERROR)

0 commit comments

Comments
 (0)