diff --git a/.changeset/quiet-wolves-fix.md b/.changeset/quiet-wolves-fix.md new file mode 100644 index 0000000..aec802f --- /dev/null +++ b/.changeset/quiet-wolves-fix.md @@ -0,0 +1,5 @@ +--- +"posthog-php": patch +--- + +Stop duplicating `distinct_id` inside `/flags` person properties. diff --git a/.github/workflows/sdk-compliance.yml b/.github/workflows/sdk-compliance.yml index df88af8..1dfcb90 100644 --- a/.github/workflows/sdk-compliance.yml +++ b/.github/workflows/sdk-compliance.yml @@ -14,8 +14,8 @@ on: jobs: compliance: name: PostHog SDK compliance tests - uses: PostHog/posthog-sdk-test-harness/.github/workflows/test-sdk-action.yml@68498bcf3ae95ac941476e6ca3d42d6086e1c7fd + uses: PostHog/posthog-sdk-test-harness/.github/workflows/test-sdk-action.yml@02c049e529001d02f37a534745678e057d371fb0 with: adapter-dockerfile: "sdk_compliance_adapter/Dockerfile" adapter-context: "." - test-harness-version: "0.9.0" + test-harness-version: "0.10.0" diff --git a/lib/Client.php b/lib/Client.php index 4e4b40c..6841573 100644 --- a/lib/Client.php +++ b/lib/Client.php @@ -624,7 +624,6 @@ private function doGetFeatureFlagResult( } [$personProperties, $groupProperties] = $this->addLocalPersonAndGroupProperties( - $distinctId, $groups, $personProperties, $groupProperties @@ -822,7 +821,6 @@ public function getAllFlags( } [$personProperties, $groupProperties] = $this->addLocalPersonAndGroupProperties( - $distinctId, $groups, $personProperties, $groupProperties @@ -906,7 +904,6 @@ public function evaluateFlags( } [$personProperties, $groupProperties] = $this->addLocalPersonAndGroupProperties( - $distinctId, $groups, $personProperties, $groupProperties @@ -1186,10 +1183,15 @@ private function computeFlagLocally( $this->groupTypeMapping ); } else { + $localPersonProperties = $personProperties; + if (!array_key_exists('distinct_id', $localPersonProperties)) { + $localPersonProperties['distinct_id'] = $distinctId; + } + return FeatureFlag::matchFeatureFlagProperties( $featureFlag, $distinctId, - $personProperties, + $localPersonProperties, $this->cohorts, $this->featureFlagsByKey, $evaluationCache, @@ -2086,16 +2088,10 @@ private function message($msg) } private function addLocalPersonAndGroupProperties( - string $distinctId, array $groups, array $personProperties, array $groupProperties ): array { - $allPersonProperties = array_merge( - ["distinct_id" => $distinctId], - $personProperties - ); - $allGroupProperties = []; if (count($groups) > 0) { foreach ($groups as $groupName => $groupValue) { @@ -2106,6 +2102,6 @@ private function addLocalPersonAndGroupProperties( } } - return [$allPersonProperties, $allGroupProperties]; + return [$personProperties, $allGroupProperties]; } } diff --git a/sdk_compliance_adapter/docker-compose.yml b/sdk_compliance_adapter/docker-compose.yml index 273b31f..9793a73 100644 --- a/sdk_compliance_adapter/docker-compose.yml +++ b/sdk_compliance_adapter/docker-compose.yml @@ -9,7 +9,7 @@ services: - test-network test-harness: - image: ghcr.io/posthog/sdk-test-harness:0.9.0 + image: ghcr.io/posthog/sdk-test-harness:0.10.0 command: ["run", "--adapter-url", "http://sdk-adapter:8080", "--mock-url", "http://test-harness:8081"] networks: - test-network diff --git a/test/FeatureFlagLocalEvaluationTest.php b/test/FeatureFlagLocalEvaluationTest.php index f5b5150..5f4d773 100644 --- a/test/FeatureFlagLocalEvaluationTest.php +++ b/test/FeatureFlagLocalEvaluationTest.php @@ -1135,6 +1135,47 @@ public function testFlagPersonProperties() $this->checkEmptyErrorLogs(); } + public function testFlagDistinctIdPropertyIsAvailableForLocalEvaluationOnly() + { + $localEvaluationResponse = MockedResponses::LOCAL_EVALUATION_SIMPLE_REQUEST; + $localEvaluationResponse['flags'][0]['key'] = 'distinct-id-flag'; + $localEvaluationResponse['flags'][0]['filters']['groups'][0]['properties'] = [ + [ + 'key' => 'distinct_id', + 'operator' => 'exact', + 'value' => 'some-distinct-id', + 'type' => 'person', + ], + ]; + + $this->http_client = new MockedHttpClient(host: "app.posthog.com", flagEndpointResponse: $localEvaluationResponse); + $this->client = new Client( + self::FAKE_API_KEY, + [ + "debug" => true, + ], + $this->http_client, + "test" + ); + PostHog::init(null, null, $this->client); + $this->http_client->calls = array(); + + $personProperties = ['region' => 'USA']; + $this->assertTrue(PostHog::getFeatureFlag( + 'distinct-id-flag', + 'some-distinct-id', + [], + $personProperties, + [], + true, + false + )); + $this->assertEquals(['region' => 'USA'], $personProperties); + $this->assertEquals(array(), $this->http_client->calls); + + $this->checkEmptyErrorLogs(); + } + public function testFlagPersonBooleanProperties() { $this->http_client = new MockedHttpClient(host: "app.posthog.com", flagEndpointResponse: MockedResponses::LOCAL_EVALUATION_BOOLEAN_REQUEST); @@ -1768,7 +1809,7 @@ public function testFeatureFlagsLocalEvaluationForNegatedCohorts() array( 0 => array( "path" => "/flags/?v=2", - 'payload' => '{"api_key":"random_key","distinct_id":"some-distinct-id","groups":{},"person_properties":{"distinct_id":"some-distinct-id","region":"USA","other":"thing"},"group_properties":{},"geoip_disable":false,"flag_keys_to_evaluate":["beta-feature"]}', + 'payload' => '{"api_key":"random_key","distinct_id":"some-distinct-id","groups":{},"person_properties":{"region":"USA","other":"thing"},"group_properties":{},"geoip_disable":false,"flag_keys_to_evaluate":["beta-feature"]}', "extraHeaders" => array(0 => 'User-Agent: posthog-php/' . PostHog::VERSION), "requestOptions" => array("timeout" => 3000, "shouldRetry" => false), ), diff --git a/test/FeatureFlagTest.php b/test/FeatureFlagTest.php index 9c699f3..d3ec7dc 100644 --- a/test/FeatureFlagTest.php +++ b/test/FeatureFlagTest.php @@ -251,7 +251,7 @@ public function testIsFeatureEnabled($response) ), 1 => array( "path" => "/flags/?v=2", - "payload" => sprintf('{"api_key":"%s","distinct_id":"user-id","groups":{},"person_properties":{"distinct_id":"user-id"},"group_properties":{},"geoip_disable":false,"flag_keys_to_evaluate":["having_fun"]}', self::FAKE_API_KEY), + "payload" => sprintf('{"api_key":"%s","distinct_id":"user-id","groups":{},"person_properties":{},"group_properties":{},"geoip_disable":false,"flag_keys_to_evaluate":["having_fun"]}', self::FAKE_API_KEY), "extraHeaders" => array(0 => 'User-Agent: posthog-php/' . PostHog::VERSION), "requestOptions" => array("timeout" => 3000, "shouldRetry" => false), ), @@ -271,7 +271,7 @@ public function testIsFeatureEnabledCapturesFeatureFlagCalledEventWithAdditional array( 0 => array( "path" => "/flags/?v=2", - "payload" => sprintf('{"api_key":"%s","distinct_id":"user-id","groups":{},"person_properties":{"distinct_id":"user-id"},"group_properties":{},"geoip_disable":false,"flag_keys_to_evaluate":["simple-test"]}', self::FAKE_API_KEY), + "payload" => sprintf('{"api_key":"%s","distinct_id":"user-id","groups":{},"person_properties":{},"group_properties":{},"geoip_disable":false,"flag_keys_to_evaluate":["simple-test"]}', self::FAKE_API_KEY), "extraHeaders" => array(0 => 'User-Agent: posthog-php/' . PostHog::VERSION), "requestOptions" => array("timeout" => 3000, "shouldRetry" => false), ), @@ -294,7 +294,7 @@ public function testWhitespacePersonalApiKeyFallsBackToFlagsEndpoint() [ [ "path" => "/flags/?v=2", - "payload" => sprintf('{"api_key":"%s","distinct_id":"user-id","groups":{},"person_properties":{"distinct_id":"user-id"},"group_properties":{},"geoip_disable":false,"flag_keys_to_evaluate":["simple-test"]}', self::FAKE_API_KEY), + "payload" => sprintf('{"api_key":"%s","distinct_id":"user-id","groups":{},"person_properties":{},"group_properties":{},"geoip_disable":false,"flag_keys_to_evaluate":["simple-test"]}', self::FAKE_API_KEY), "extraHeaders" => [0 => 'User-Agent: posthog-php/' . PostHog::VERSION], "requestOptions" => ["timeout" => 3000, "shouldRetry" => false], ], @@ -323,7 +323,7 @@ public function testIsFeatureEnabledGroups($response) 1 => array( "path" => "/flags/?v=2", "payload" => sprintf( - '{"api_key":"%s","distinct_id":"user-id","groups":{"company":"id:5"},"person_properties":{"distinct_id":"user-id"},"group_properties":{"company":{"$group_key":"id:5"}},"geoip_disable":false,"flag_keys_to_evaluate":["having_fun"]}', + '{"api_key":"%s","distinct_id":"user-id","groups":{"company":"id:5"},"person_properties":{},"group_properties":{"company":{"$group_key":"id:5"}},"geoip_disable":false,"flag_keys_to_evaluate":["having_fun"]}', self::FAKE_API_KEY ), "extraHeaders" => array(0 => 'User-Agent: posthog-php/' . PostHog::VERSION), @@ -351,7 +351,7 @@ public function testGetFeatureFlag($response) ), 1 => array( "path" => "/flags/?v=2", - "payload" => sprintf('{"api_key":"%s","distinct_id":"user-id","groups":{},"person_properties":{"distinct_id":"user-id"},"group_properties":{},"geoip_disable":false,"flag_keys_to_evaluate":["multivariate-test"]}', self::FAKE_API_KEY), + "payload" => sprintf('{"api_key":"%s","distinct_id":"user-id","groups":{},"person_properties":{},"group_properties":{},"geoip_disable":false,"flag_keys_to_evaluate":["multivariate-test"]}', self::FAKE_API_KEY), "extraHeaders" => array(0 => 'User-Agent: posthog-php/' . PostHog::VERSION), "requestOptions" => array("timeout" => 3000, "shouldRetry" => false), ), @@ -371,7 +371,7 @@ public function testGetFeatureFlagCapturesFeatureFlagCalledEventWithAdditionalMe array( 0 => array( "path" => "/flags/?v=2", - "payload" => sprintf('{"api_key":"%s","distinct_id":"user-id","groups":{},"person_properties":{"distinct_id":"user-id"},"group_properties":{},"geoip_disable":false,"flag_keys_to_evaluate":["multivariate-test"]}', self::FAKE_API_KEY), + "payload" => sprintf('{"api_key":"%s","distinct_id":"user-id","groups":{},"person_properties":{},"group_properties":{},"geoip_disable":false,"flag_keys_to_evaluate":["multivariate-test"]}', self::FAKE_API_KEY), "extraHeaders" => array(0 => 'User-Agent: posthog-php/' . PostHog::VERSION), "requestOptions" => array("timeout" => 3000, "shouldRetry" => false), ), @@ -420,7 +420,7 @@ public function testGetFeatureFlagGroups($response) 1 => array( "path" => "/flags/?v=2", "payload" => sprintf( - '{"api_key":"%s","distinct_id":"user-id","groups":{"company":"id:5"},"person_properties":{"distinct_id":"user-id"},"group_properties":{"company":{"$group_key":"id:5"}},"geoip_disable":false,"flag_keys_to_evaluate":["multivariate-test"]}', + '{"api_key":"%s","distinct_id":"user-id","groups":{"company":"id:5"},"person_properties":{},"group_properties":{"company":{"$group_key":"id:5"}},"geoip_disable":false,"flag_keys_to_evaluate":["multivariate-test"]}', self::FAKE_API_KEY ), "extraHeaders" => array(0 => 'User-Agent: posthog-php/' . PostHog::VERSION), diff --git a/test/PostHogTest.php b/test/PostHogTest.php index 07fcc08..fcb0e07 100644 --- a/test/PostHogTest.php +++ b/test/PostHogTest.php @@ -1298,7 +1298,7 @@ public function testDefaultPropertiesGetAddedProperly(): void ), 1 => array( "path" => "/flags/?v=2", - "payload" => sprintf('{"api_key":"%s","distinct_id":"some_id","groups":{"company":"id:5","instance":"app.posthog.com"},"person_properties":{"distinct_id":"some_id","x1":"y1"},"group_properties":{"company":{"$group_key":"id:5","x":"y"},"instance":{"$group_key":"app.posthog.com"}},"geoip_disable":false,"flag_keys_to_evaluate":["random_key"]}', self::FAKE_API_KEY), + "payload" => sprintf('{"api_key":"%s","distinct_id":"some_id","groups":{"company":"id:5","instance":"app.posthog.com"},"person_properties":{"x1":"y1"},"group_properties":{"company":{"$group_key":"id:5","x":"y"},"instance":{"$group_key":"app.posthog.com"}},"geoip_disable":false,"flag_keys_to_evaluate":["random_key"]}', self::FAKE_API_KEY), "extraHeaders" => array(0 => 'User-Agent: posthog-php/' . PostHog::VERSION), "requestOptions" => array("timeout" => 3000, "shouldRetry" => false), ), @@ -1336,7 +1336,7 @@ public function testDefaultPropertiesGetAddedProperly(): void array( 0 => array( "path" => "/flags/?v=2", - "payload" => sprintf('{"api_key":"%s","distinct_id":"some_id","groups":{"company":"id:5"},"person_properties":{"distinct_id":"some_id"},"group_properties":{"company":{"$group_key":"id:5"}},"geoip_disable":false,"flag_keys_to_evaluate":["random_key"]}', self::FAKE_API_KEY), + "payload" => sprintf('{"api_key":"%s","distinct_id":"some_id","groups":{"company":"id:5"},"person_properties":{},"group_properties":{"company":{"$group_key":"id:5"}},"geoip_disable":false,"flag_keys_to_evaluate":["random_key"]}', self::FAKE_API_KEY), "extraHeaders" => array(0 => 'User-Agent: posthog-php/' . PostHog::VERSION), "requestOptions" => array("timeout" => 3000, "shouldRetry" => false), ), @@ -1352,7 +1352,7 @@ public function testDefaultPropertiesGetAddedProperly(): void array( 0 => array( "path" => "/flags/?v=2", - "payload" => sprintf('{"api_key":"%s","distinct_id":"some_id","groups":{"company":"id:5","instance":"app.posthog.com"},"person_properties":{"distinct_id":"some_id","x1":"y1"},"group_properties":{"company":{"$group_key":"id:5","x":"y"},"instance":{"$group_key":"app.posthog.com"}},"geoip_disable":false,"flag_keys_to_evaluate":["random_key"]}', self::FAKE_API_KEY), + "payload" => sprintf('{"api_key":"%s","distinct_id":"some_id","groups":{"company":"id:5","instance":"app.posthog.com"},"person_properties":{"x1":"y1"},"group_properties":{"company":{"$group_key":"id:5","x":"y"},"instance":{"$group_key":"app.posthog.com"}},"geoip_disable":false,"flag_keys_to_evaluate":["random_key"]}', self::FAKE_API_KEY), "extraHeaders" => array(0 => 'User-Agent: posthog-php/' . PostHog::VERSION), "requestOptions" => array("timeout" => 3000, "shouldRetry" => false), ),