Skip to content

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

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.

Sign up for GitHub

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 2 commits into from
Apr 1, 2019
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
54e918d
feat(api): Feature variable APIs return default variable value when f…
rashidsp Mar 27, 2019
052ec8c
Merge branch 'master' into rashid/feature-enable-checking
mjc1283 Apr 1, 2019
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
26 changes: 17 additions & 9 deletions src/Optimizely/Optimizely.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,18 +620,26 @@ public function getFeatureVariableValueForType(
);
} else {
$variation = $decision->getVariation();
$variable_usage = $variation->getVariableUsageById($variable->getId());
if ($variable_usage) {
$variableValue = $variable_usage->getValue();
$this->_logger->log(
Logger::INFO,
"Returning variable value '{$variableValue}' for variation '{$variation->getKey()}' ".
"of feature flag '{$featureFlagKey}'"
);
if ($variation->getFeatureEnabled()) {
$variableUsage = $variation->getVariableUsageById($variable->getId());
if ($variableUsage) {
$variableValue = $variableUsage->getValue();
$this->_logger->log(
Logger::INFO,
"Returning variable value '{$variableValue}' for variation '{$variation->getKey()}' ".
"of feature flag '{$featureFlagKey}'"
);
} else {
$this->_logger->log(
Logger::INFO,
"Variable '{$variableKey}' is not used in variation '{$variation->getKey()}', ".
"returning default value '{$variableValue}'."
);
}
} else {
$this->_logger->log(
Logger::INFO,
"Variable '{$variableKey}' is not used in variation '{$variation->getKey()}', ".
"Feature '{$featureFlagKey}' for variation '{$variation->getKey()}' is not enabled, ".
"returning default value '{$variableValue}'."
);
}
Expand Down
78 changes: 78 additions & 0 deletions tests/OptimizelyTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2784,6 +2784,45 @@ public function testGetFeatureVariableValueForTypeGivenFeatureFlagIsEnabledForUs
);
}

public function testGetFeatureVariableValueForTypeReturnDefaultVariableValueGivenUserInExperimentAndFeatureFlagIsNotEnabled()
{
// should return specific value
$decisionServiceMock = $this->getMockBuilder(DecisionService::class)
->setConstructorArgs(array($this->loggerMock, $this->projectConfig))
->setMethods(array('getVariationForFeature'))
->getMock();

$decisionService = new \ReflectionProperty(Optimizely::class, '_decisionService');
$decisionService->setAccessible(true);
$decisionService->setValue($this->optimizelyObject, $decisionServiceMock);

$experiment = $this->projectConfig->getExperimentFromKey('test_experiment_double_feature');
$variation = $this->projectConfig->getVariationFromKey('test_experiment_double_feature', 'control');
$variation->setFeatureEnabled(false);
$expectedDecision = new FeatureDecision(
$experiment,
$variation,
FeatureDecision::DECISION_SOURCE_EXPERIMENT
);

$decisionServiceMock->expects($this->exactly(1))
->method('getVariationForFeature')
->will($this->returnValue($expectedDecision));

$this->loggerMock->expects($this->exactly(1))
->method('log')
->with(
Logger::INFO,
"Feature 'double_single_variable_feature' for variation 'control' is not enabled, ".
"returning default value '14.99'."
);

$this->assertSame(
$this->optimizelyObject->getFeatureVariableValueForType('double_single_variable_feature', 'double_variable', 'user_id', [], 'double'),
'14.99'
);
}

public function testGetFeatureVariableValueForTypeWithRolloutRule()
{
// should return specific value
Expand Down Expand Up @@ -2822,6 +2861,45 @@ public function testGetFeatureVariableValueForTypeWithRolloutRule()
$this->assertTrue($this->optimizelyObject->getFeatureVariableBoolean('boolean_single_variable_feature', 'boolean_variable', 'user_id', []));
}

public function testGetFeatureVariableValueReturnDefaultVariableValueGivenUserInRolloutAndFeatureFlagIsNotEnabled()
{
// should return specific value
$decisionServiceMock = $this->getMockBuilder(DecisionService::class)
->setConstructorArgs(array($this->loggerMock, $this->projectConfig))
->setMethods(array('getVariationForFeature'))
->getMock();

$decisionService = new \ReflectionProperty(Optimizely::class, '_decisionService');
$decisionService->setAccessible(true);
$decisionService->setValue($this->optimizelyObject, $decisionServiceMock);

$featureFlag = $this->projectConfig->getFeatureFlagFromKey('boolean_single_variable_feature');
$rolloutId = $featureFlag->getRolloutId();
$rollout = $this->projectConfig->getRolloutFromId($rolloutId);
$experiment = $rollout->getExperiments()[0];
$expectedVariation = $experiment->getVariations()[0];
$expectedVariation->setFeatureEnabled(false);
$expectedDecision = new FeatureDecision(
$experiment,
$expectedVariation,
FeatureDecision::DECISION_SOURCE_ROLLOUT
);

$decisionServiceMock->expects($this->exactly(1))
->method('getVariationForFeature')
->will($this->returnValue($expectedDecision));

$this->loggerMock->expects($this->exactly(1))
->method('log')
->with(
Logger::INFO,
"Feature 'boolean_single_variable_feature' for variation '177771' is not enabled, ".
"returning default value 'true'."
);

$this->assertTrue($this->optimizelyObject->getFeatureVariableBoolean('boolean_single_variable_feature', 'boolean_variable', 'user_id', []));
}

public function testGetFeatureVariableValueForTypeGivenFeatureFlagIsEnabledForUserAndVariableNotInVariation()
{
// should return default value
Expand Down