diff --git a/lib/FeatureFlag.php b/lib/FeatureFlag.php index 17e95ec..ca6e3cd 100644 --- a/lib/FeatureFlag.php +++ b/lib/FeatureFlag.php @@ -337,57 +337,12 @@ private static function variantLookupTable($featureFlag) return $lookupTable; } - private static function compareFlagConditions($conditionA, $conditionB) - { - $AhasVariantOverride = isset($conditionA["variant"]); - $BhasVariantOverride = isset($conditionB["variant"]); - - if ($AhasVariantOverride && $BhasVariantOverride) { - return 0; - } elseif ($AhasVariantOverride) { - return -1; - } elseif ($BhasVariantOverride) { - return 1; - } else { - return 0; - } - } - public static function matchFeatureFlagProperties($flag, $distinctId, $properties, $cohorts = [], $flagsByKey = null, $evaluationCache = null) { $flagConditions = ($flag["filters"] ?? [])["groups"] ?? []; $isInconclusive = false; - // Add index to each condition to make stable sort possible - $flagConditionsWithIndexes = array(); - $i = 0; - foreach ($flagConditions as $key => $value) { - $flagConditionsWithIndexes[] = array($value, $i); - $i++; - } - // # Stable sort conditions with variant overrides to the top. - // # This ensures that if overrides are present, they are - // # evaluated first, and the variant override is applied to the first matching condition. - usort( - $flagConditionsWithIndexes, - function ($conditionA, $conditionB) { - $AhasVariantOverride = isset($conditionA[0]["variant"]); - $BhasVariantOverride = isset($conditionB[0]["variant"]); - - if ($AhasVariantOverride && $BhasVariantOverride) { - return $conditionA[1] - $conditionB[1]; - } elseif ($AhasVariantOverride) { - return -1; - } elseif ($BhasVariantOverride) { - return 1; - } else { - return $conditionA[1] - $conditionB[1]; - } - } - ); - - foreach ($flagConditionsWithIndexes as $conditionWithIndex) { - $condition = $conditionWithIndex[0]; + foreach ($flagConditions as $condition) { try { if (FeatureFlag::isConditionMatch($flag, $distinctId, $condition, $properties, $cohorts, $flagsByKey, $evaluationCache)) { $variantOverride = $condition["variant"] ?? null; diff --git a/test/FeatureFlagLocalEvaluationTest.php b/test/FeatureFlagLocalEvaluationTest.php index 8fafa0f..85c2316 100644 --- a/test/FeatureFlagLocalEvaluationTest.php +++ b/test/FeatureFlagLocalEvaluationTest.php @@ -1706,9 +1706,9 @@ public function testFlagWithInvalidVariantOverrides() $this->assertEquals(PostHog::getFeatureFlag('beta-feature', 'example_id'), "second-variant"); } - public function testFlagWithMultipleVariantOverrides() + public function testConditionsEvaluatedInOrder() { - $this->http_client = new MockedHttpClient(host: "app.posthog.com", flagEndpointResponse: MockedResponses::LOCAL_EVALUATION_MULTIPLE_VARIANT_OVERRIDES_REQUEST); + $this->http_client = new MockedHttpClient(host: "app.posthog.com", flagEndpointResponse: MockedResponses::LOCAL_EVALUATION_CONDITIONS_ORDER_REQUEST); $this->client = new Client( self::FAKE_API_KEY, [ @@ -1719,9 +1719,10 @@ public function testFlagWithMultipleVariantOverrides() ); PostHog::init(null, null, $this->client); - $this->assertEquals(PostHog::getFeatureFlag('beta-feature', 'test_id', [], ["email" => "test@posthog.com"]), "second-variant"); - $this->assertEquals(PostHog::getFeatureFlag('beta-feature', 'example_id'), "third-variant"); - $this->assertEquals(PostHog::getFeatureFlag('beta-feature', 'another_id'), "second-variant"); + // VIP users now match the first condition (100% rollout) instead of their specific variant override + // because conditions are evaluated in order + $result = PostHog::getFeatureFlag('test-flag', 'vip_user', [], ["email" => "user@vip.com"]); + $this->assertTrue(in_array($result, ['control', 'test'])); // Should get one of the regular variants, not vip-variant } public function testEventCalled() diff --git a/test/assests/MockedResponses.php b/test/assests/MockedResponses.php index 69307d5..b18ec06 100644 --- a/test/assests/MockedResponses.php +++ b/test/assests/MockedResponses.php @@ -1050,6 +1050,62 @@ class MockedResponses ], ]; + public const LOCAL_EVALUATION_CONDITIONS_ORDER_REQUEST = [ + 'count' => 1, + 'next' => null, + 'previous' => null, + 'flags' => [ + [ + "id" => 1, + "name" => "Test Flag", + "key" => "test-flag", + "active" => true, + "deleted" => false, + "filters" => [ + "groups" => [ + // First condition: 100% rollout for everyone + [ + "rollout_percentage" => 100, + ], + // Second condition: VIP users get a specific variant + // This used to be evaluated first due to sorting, but now it's evaluated second + [ + "properties" => [ + [ + "key" => "email", + "value" => "@vip.com", + "operator" => "icontains", + "type" => "person" + ] + ], + "rollout_percentage" => 100, + "variant" => "vip-variant" + ], + ], + "multivariate" => [ + "variants" => [ + [ + "key" => "control", + "name" => "Control", + "rollout_percentage" => 50 + ], + [ + "key" => "test", + "name" => "Test", + "rollout_percentage" => 50 + ], + [ + "key" => "vip-variant", + "name" => "VIP Variant", + "rollout_percentage" => 0 + ] + ] + ] + ], + ] + ], + ]; + public const EXPERIENCE_CONITNUITY_REQUEST = [ 'count' => 1,