fix for ch22995 - include prereq value in event even if prereq is off

Author: eli-darkly

src/LaunchDarkly/FeatureFlag.php

@@ -109,12 +109,9 @@ public function isOn()
      */
     public function evaluate($user, $featureRequester, $includeReasonsInEvents = false)
     {
-        if ($this->isOn()) {
-            $prereqEvents = array();
-            $detail = $this->evaluateInternal($user, $featureRequester, $prereqEvents, $includeReasonsInEvents);
-            return new EvalResult($detail, $prereqEvents);
-        }
-        return new EvalResult($this->getOffValue(EvaluationReason::off()), array());
+        $prereqEvents = array();
+        $detail = $this->evaluateInternal($user, $featureRequester, $prereqEvents, $includeReasonsInEvents);
+        return new EvalResult($detail, $prereqEvents);
     }
 
     /**
@@ -126,6 +123,10 @@ public function evaluate($user, $featureRequester, $includeReasonsInEvents = fal
      */
     private function evaluateInternal($user, $featureRequester, &$events, $includeReasonsInEvents)
     {
+        if (!$this->isOn()) {
+            return $this->getOffValue(EvaluationReason::off());
+        }
+
         $prereqFailureReason = $this->checkPrerequisites($user, $featureRequester, $events, $includeReasonsInEvents);
         if ($prereqFailureReason !== null) {
             return $this->getOffValue($prereqFailureReason);
@@ -170,19 +171,14 @@ private function checkPrerequisites($user, $featureRequester, &$events, $include
                     $prereqFeatureFlag = $featureRequester->getFeature($prereq->getKey());
                     if ($prereqFeatureFlag == null) {
                         $prereqOk = false;
-                    } elseif ($prereqFeatureFlag->isOn()) {
+                    } else {
                         $prereqEvalResult = $prereqFeatureFlag->evaluateInternal($user, $featureRequester, $events, $includeReasonsInEvents);
                         $variation = $prereq->getVariation();
-                        if ($prereqEvalResult->getVariationIndex() !== $variation) {
+                        if (!$prereqFeatureFlag->isOn() || $prereqEvalResult->getVariationIndex() !== $variation) {
                             $prereqOk = false;
                         }
-                    } else {
-                        $prereqOk = false;
-                    }
-                    if ($prereqFeatureFlag) {
                         array_push($events, Util::newFeatureRequestEvent($prereq->getKey(), $user,
-                            $prereqEvalResult ? $prereqEvalResult->getVariationIndex() : null,
-                            $prereqEvalResult ? $prereqEvalResult->getValue() : null,
+                            $prereqEvalResult->getVariationIndex(), $prereqEvalResult->getValue(),
                             null, $prereqFeatureFlag->getVersion(), $this->_key,
                             ($includeReasonsInEvents && $prereqEvalResult) ? $prereqEvalResult->getReason() : null
                         ));

tests/FeatureFlagTest.php

@@ -314,6 +314,59 @@ public function testFlagReturnsOffVariationIfPrerequisiteIsNotFound()
         self::assertEquals(array(), $result->getPrerequisiteEvents());
     }
 
+    public function testFlagReturnsOffVariationAndEventIfPrerequisiteIsOff()
+    {
+        $flag0Json = array(
+            'key' => 'feature0',
+            'version' => 1,
+            'deleted' => false,
+            'on' => true,
+            'targets' => array(),
+            'prerequisites' => array(
+                array('key' => 'feature1', 'variation' => 1)
+            ),
+            'rules' => array(),
+            'offVariation' => 1,
+            'fallthrough' => array('variation' => 0),
+            'variations' => array('fall', 'off', 'on'),
+            'salt' => ''
+        );
+        $flag1Json = array(
+            'key' => 'feature1',
+            'version' => 2,
+            'deleted' => false,
+            'on' => false,
+            'targets' => array(),
+            'prerequisites' => array(),
+            'rules' => array(),
+            'offVariation' => 1,
+            // note that even though it returns the desired variation, it is still off and therefore not a match
+            'fallthrough' => array('variation' => 0),
+            'variations' => array('nogo', 'go'),
+            'salt' => ''
+        );
+        $flag0 = FeatureFlag::decode($flag0Json);
+        $flag1 = FeatureFlag::decode($flag1Json);
+        $ub = new LDUserBuilder('x');
+        $user = $ub->build();
+        $requester = new MockFeatureRequesterForFeature();
+        $requester->key = $flag1->getKey();
+        $requester->val = $flag1;
+
+        $result = $flag0->evaluate($user, $requester);
+        $detail = new EvaluationDetail('off', 1, EvaluationReason::prerequisiteFailed('feature1'));
+        self::assertEquals($detail, $result->getDetail());
+
+        $events = $result->getPrerequisiteEvents();
+        self::assertEquals(1, count($events));
+        $event = $events[0];
+        self::assertEquals('feature', $event['kind']);
+        self::assertEquals($flag1->getKey(), $event['key']);
+        self::assertEquals('go', $event['value']);
+        self::assertEquals($flag1->getVersion(), $event['version']);
+        self::assertEquals($flag0->getKey(), $event['prereqOf']);
+    }
+
     public function testFlagReturnsOffVariationAndEventIfPrerequisiteIsNotMet()
     {
         $flag0Json = array(