From 54e918ddee1a8893a8bc49476a482bd7889b1910 Mon Sep 17 00:00:00 2001 From: rashidsp Date: Wed, 27 Mar 2019 21:48:28 +0500 Subject: [PATCH] feat(api): Feature variable APIs return default variable value when featureEnabled property is false. --- src/Optimizely/Optimizely.php | 26 ++++++++---- tests/OptimizelyTest.php | 78 +++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 9 deletions(-) diff --git a/src/Optimizely/Optimizely.php b/src/Optimizely/Optimizely.php index 58251f4b..4b14c703 100644 --- a/src/Optimizely/Optimizely.php +++ b/src/Optimizely/Optimizely.php @@ -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}'." ); } diff --git a/tests/OptimizelyTest.php b/tests/OptimizelyTest.php index b1371e96..abdab94c 100644 --- a/tests/OptimizelyTest.php +++ b/tests/OptimizelyTest.php @@ -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 @@ -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