Sending impression event when feature is disabled for variation (#114)

Author: aliabbasrizvi

src/Optimizely/Optimizely.php

@@ -24,9 +24,7 @@
 use Optimizely\DecisionService\DecisionService;
 use Optimizely\DecisionService\FeatureDecision;
 use Optimizely\Entity\Experiment;
-use Optimizely\Entity\FeatureFlag;
 use Optimizely\Entity\FeatureVariable;
-use Optimizely\Entity\Rollout;
 use Optimizely\Logger\DefaultLogger;
 use Optimizely\ErrorHandler\ErrorHandlerInterface;
 use Optimizely\ErrorHandler\NoOpErrorHandler;
@@ -38,7 +36,6 @@
 use Optimizely\Notification\NotificationCenter;
 use Optimizely\Notification\NotificationType;
 use Optimizely\UserProfile\UserProfileServiceInterface;
-use Optimizely\Utils\EventTagUtils;
 use Optimizely\Utils\Validator;
 use Optimizely\Utils\VariableTypeUtils;
 
@@ -502,20 +499,19 @@ public function isFeatureEnabled($featureFlagKey, $userId, $attributes = null)
         $experiment = $decision->getExperiment();
         $variation = $decision->getVariation();
 
-        if (is_null($variation) || !$variation->getFeatureEnabled()) {
-            $this->_logger->log(Logger::INFO, "Feature Flag '{$featureFlagKey}' is not enabled for user '{$userId}'.");
-            return false;
-        }
-
         if ($decision->getSource() == FeatureDecision::DECISION_SOURCE_EXPERIMENT) {
             $this->sendImpressionEvent($experiment->getKey(), $variation->getKey(), $userId, $attributes);
         } else {
             $this->_logger->log(Logger::INFO, "The user '{$userId}' is not being experimented on Feature Flag '{$featureFlagKey}'.");
         }
 
-        $this->_logger->log(Logger::INFO, "Feature Flag '{$featureFlagKey}' is enabled for user '{$userId}'.");
+        if ($variation->getFeatureEnabled()) {
+            $this->_logger->log(Logger::INFO, "Feature Flag '{$featureFlagKey}' is enabled for user '{$userId}'.");
+            return true;
+        }
 
-        return true;
+        $this->_logger->log(Logger::INFO, "Feature Flag '{$featureFlagKey}' is not enabled for user '{$userId}'.");
+        return false;
     }
 
     /**

tests/OptimizelyTest.php

@@ -2087,8 +2087,9 @@ public function testIsFeatureEnabledGivenFeatureExperimentAndFeatureEnabledIsFal
             ->method('getVariationForFeature')
             ->will($this->returnValue($expected_decision));
 
-        $optimizelyMock->expects($this->never())
-            ->method('sendImpressionEvent');
+        $optimizelyMock->expects($this->exactly(1))
+            ->method('sendImpressionEvent')
+            ->with('test_experiment_double_feature', 'variation', 'user_id', []);
 
         $this->loggerMock->expects($this->at(0))
             ->method('log')
@@ -2188,7 +2189,12 @@ public function testIsFeatureEnabledGivenFeatureRolloutAndFeatureEnabledIsFalse(
         $optimizelyMock->expects($this->never())
             ->method('sendImpressionEvent');
 
+        // confirm log messages seen
         $this->loggerMock->expects($this->at(0))
+            ->method('log')
+            ->with(Logger::INFO, "The user 'user_id' is not being experimented on Feature Flag 'boolean_single_variable_feature'.");
+
+        $this->loggerMock->expects($this->at(1))
             ->method('log')
             ->with(Logger::INFO, "Feature Flag 'boolean_single_variable_feature' is not enabled for user 'user_id'.");