fix: stop duplicating distinct_id in flags person properties

.changeset/quiet-wolves-fix.md

@@ -0,0 +1,6 @@
+---
+"posthog-ruby": patch
+"posthog-rails": patch
+---
+
+Stop duplicating `distinct_id` inside `/flags` person properties.

.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"

lib/posthog/client.rb

@@ -558,7 +558,7 @@ def evaluate_flags(
       return FeatureFlagEvaluations.new(host: host, distinct_id: distinct_id, flags: {}, groups: groups) if @disabled
 
       person_properties, group_properties = add_local_person_and_group_properties(
-        distinct_id, groups, person_properties, group_properties
+        groups, person_properties, group_properties
       )
 
       records = {}
@@ -672,8 +672,8 @@ def get_all_flags(
     )
       return {} if @disabled
 
-      person_properties, group_properties = add_local_person_and_group_properties(distinct_id, groups,
-                                                                                  person_properties, group_properties)
+      person_properties, group_properties = add_local_person_and_group_properties(groups, person_properties,
+                                                                                  group_properties)
       @feature_flags_poller.get_all_flags(distinct_id, groups, person_properties, group_properties,
                                           only_evaluate_locally)
     end
@@ -712,8 +712,8 @@ def get_feature_flag_payload(
       return nil if @disabled
 
       key = key.to_s
-      person_properties, group_properties = add_local_person_and_group_properties(distinct_id, groups,
-                                                                                  person_properties, group_properties)
+      person_properties, group_properties = add_local_person_and_group_properties(groups, person_properties,
+                                                                                  group_properties)
       @feature_flags_poller.get_feature_flag_payload(key, distinct_id, match_value, groups, person_properties,
                                                      group_properties, only_evaluate_locally)
     end
@@ -740,7 +740,7 @@ def get_all_flags_and_payloads(
       return { featureFlags: {}, featureFlagPayloads: {} } if @disabled
 
       person_properties, group_properties = add_local_person_and_group_properties(
-        distinct_id, groups, person_properties, group_properties
+        groups, person_properties, group_properties
       )
       response = @feature_flags_poller.get_all_flags_and_payloads(
         distinct_id, groups, person_properties, group_properties, only_evaluate_locally
@@ -902,7 +902,7 @@ def _get_feature_flag_result(
 
       key = key.to_s
       person_properties, group_properties = add_local_person_and_group_properties(
-        distinct_id, groups, person_properties, group_properties
+        groups, person_properties, group_properties
       )
       feature_flag_response, flag_was_locally_evaluated, request_id, evaluated_at, feature_flag_error, payload =
         @feature_flags_poller.get_feature_flag(
@@ -1048,7 +1048,7 @@ def shutdown?
       @shutdown_mutex.synchronize { @shutdown }
     end
 
-    def add_local_person_and_group_properties(distinct_id, groups, person_properties, group_properties)
+    def add_local_person_and_group_properties(groups, person_properties, group_properties)
       groups ||= {}
       person_properties ||= {}
       group_properties ||= {}
@@ -1061,16 +1061,14 @@ def add_local_person_and_group_properties(distinct_id, groups, person_properties
         symbolize_keys! value
       end
 
-      all_person_properties = { distinct_id: distinct_id }.merge(person_properties)
-
       all_group_properties = {}
       groups&.each do |group_name, group_key|
         all_group_properties[group_name] = {
           '$group_key': group_key
         }.merge((group_properties && group_properties[group_name]) || {})
       end
 
-      [all_person_properties, all_group_properties]
+      [person_properties, all_group_properties]
     end
   end
 end

lib/posthog/feature_flags.rb

@@ -921,7 +921,12 @@ def _compute_flag_locally(flag, distinct_id, groups = {}, person_properties = {}
 
       aggregation_group_type_index = flag_filters[:aggregation_group_type_index]
       if aggregation_group_type_index.nil?
-        return match_feature_flag_properties(flag, distinct_id, person_properties, evaluation_cache, @cohorts,
+        local_person_properties = person_properties || {}
+        unless local_person_properties.key?(:distinct_id) || local_person_properties.key?('distinct_id')
+          local_person_properties = local_person_properties.merge(distinct_id: distinct_id)
+        end
+
+        return match_feature_flag_properties(flag, distinct_id, local_person_properties, evaluation_cache, @cohorts,
                                              groups: groups, group_properties: group_properties)
       end
 

sdk_compliance_adapter/docker-compose.yml

@@ -7,7 +7,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

spec/posthog/client_spec.rb

@@ -1700,7 +1700,7 @@ def run
               'company' => { '$group_key' => 'id:5', 'x' => 'y' },
               'instance' => { '$group_key' => 'app.posthog.com' }
             },
-            'person_properties' => { 'distinct_id' => 'some_id', 'x1' => 'y1' }, 'token' => 'testsecret'
+            'person_properties' => { 'x1' => 'y1' }, 'token' => 'testsecret'
           }
         )
 
@@ -1734,7 +1734,7 @@ def run
         assert_requested :post, flags_endpoint, times: 1
         expect(WebMock).to have_requested(:post, flags_endpoint).with(
           body: { 'distinct_id' => 'some_id', 'groups' => {}, 'group_properties' => {},
-                  'person_properties' => { 'distinct_id' => 'some_id' }, 'token' => 'testsecret' }
+                  'person_properties' => {}, 'token' => 'testsecret' }
         )
         WebMock.reset_executed_requests!
 
@@ -1745,7 +1745,7 @@ def run
             'distinct_id' => 'some_id',
             'groups' => { 'company' => 'id:5' },
             'group_properties' => { 'company' => { '$group_key' => 'id:5' } },
-            'person_properties' => { 'distinct_id' => 'some_id' },
+            'person_properties' => {},
             'token' => 'testsecret'
           }
         )
@@ -1761,15 +1761,15 @@ def run
         assert_requested :post, flags_endpoint, times: 1
         expect(WebMock).to have_requested(:post, flags_endpoint).with(
           body: { 'distinct_id' => 'some_id', 'groups' => {}, 'group_properties' => {},
-                  'person_properties' => { 'distinct_id' => 'some_id' }, 'token' => 'testsecret' }
+                  'person_properties' => {}, 'token' => 'testsecret' }
         )
         WebMock.reset_executed_requests!
 
         client.is_feature_enabled('random_key', 'some_id', groups: {}, person_properties: nil, group_properties: nil)
         assert_requested :post, flags_endpoint, times: 1
         expect(WebMock).to have_requested(:post, flags_endpoint).with(
           body: { 'distinct_id' => 'some_id', 'groups' => {}, 'group_properties' => {},
-                  'person_properties' => { 'distinct_id' => 'some_id' }, 'token' => 'testsecret' }
+                  'person_properties' => {}, 'token' => 'testsecret' }
         )
         WebMock.reset_executed_requests!
       end

spec/posthog/feature_flag_spec.rb

@@ -57,6 +57,50 @@ module PostHog
                                 person_properties: { 'region' => 'Canada' })).to eq(false)
     end
 
+    it 'evaluates distinct_id locally without sending it in person_properties' do
+      api_feature_flag_res = {
+        'flags' => [
+          {
+            'id' => 1,
+            'name' => 'Distinct ID Feature',
+            'key' => 'distinct-id-flag',
+            'is_simple_flag' => true,
+            'active' => true,
+            'filters' => {
+              'groups' => [
+                {
+                  'properties' => [
+                    {
+                      'key' => 'distinct_id',
+                      'operator' => 'exact',
+                      'value' => 'some-distinct-id',
+                      'type' => 'person'
+                    }
+                  ],
+                  'rollout_percentage' => 100
+                }
+              ]
+            }
+          }
+        ]
+      }
+      stub_request(
+        :get,
+        'https://us.i.posthog.com/flags/definitions?token=testsecret&send_cohorts=true'
+      ).to_return(status: 200, body: api_feature_flag_res.to_json)
+      stub_request(:post, flags_endpoint).to_return(status: 400)
+
+      c = Client.new(api_key: API_KEY, personal_api_key: API_KEY, test_mode: true)
+      person_properties = { region: 'USA' }
+
+      expect(c.get_feature_flag('distinct-id-flag', 'some-distinct-id',
+                                person_properties: person_properties,
+                                only_evaluate_locally: true,
+                                send_feature_flag_events: false)).to eq(true)
+      expect(person_properties).not_to have_key(:distinct_id)
+      expect(a_request(:post, flags_endpoint)).not_to have_been_made
+    end
+
     it 'accepts symbol flag keys when evaluating locally' do
       api_feature_flag_res = {
         'flags' => [
@@ -320,7 +364,7 @@ module PostHog
           'distinct_id' => 'some-distinct-id_outside_rollout?',
           'groups' => {},
           'group_properties' => {},
-          'person_properties' => { 'distinct_id' => 'some-distinct-id_outside_rollout?', 'region' => 'USA',
+          'person_properties' => { 'region' => 'USA',
                                    'email' => 'a@b.com' },
           'token' => 'testsecret'
         }
@@ -338,7 +382,7 @@ module PostHog
           'distinct_id' => 'some-distinct-id',
           'groups' => {},
           'group_properties' => {},
-          'person_properties' => { 'distinct_id' => 'some-distinct-id', 'doesnt_matter' => '1' },
+          'person_properties' => { 'doesnt_matter' => '1' },
           'token' => 'testsecret'
         }
       )
@@ -4815,7 +4859,7 @@ module PostHog
 
       # Add the exact stub for the `/flags` endpoint as recommended in the error
       stub_request(:post, 'https://us.i.posthog.com/flags/?v=2').with(
-        body: '{"distinct_id":"distinct_id","groups":{},"person_properties":{"distinct_id":"distinct_id","region":"USA"},"group_properties":{},"token":"testsecret"}', # rubocop:disable Layout/LineLength
+        body: '{"distinct_id":"distinct_id","groups":{},"person_properties":{"region":"USA"},"group_properties":{},"token":"testsecret"}', # rubocop:disable Layout/LineLength
         headers: {
           'Accept' => '*/*',
           'Accept-Encoding' => 'gzip;q=1.0,deflate;q=0.6,identity;q=0.3',