fix: Three clear options for persons on events (#21455)

* Revert "feat(hogql): Add feature flag for opting queries into v3 persons-on-events (#21150)"

This reverts commit 205376b.

* fix: Three clear options for persons on events

* Update UI snapshots for `chromium` (1)

* use UUID to target feature flag

* fix

* add missing option

* fix

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (2)

* Update posthog/models/team/team.py

Co-authored-by: ted kaemming <65315+tkaemming@users.noreply.github.com>

* Update posthog/models/team/team.py

Co-authored-by: ted kaemming <65315+tkaemming@users.noreply.github.com>

* fix schema

* fix

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (2)

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: ted kaemming <65315+tkaemming@users.noreply.github.com>

ee/clickhouse/queries/funnels/funnel_correlation.py

@@ -435,7 +435,7 @@ def get_properties_query(self) -> Tuple[str, Dict[str, Any]]:
         return query, params
 
     def _get_aggregation_target_join_query(self) -> str:
-        if self._team.person_on_events_mode == PersonOnEventsMode.V1_ENABLED:
+        if self._team.person_on_events_mode == PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS:
             aggregation_person_join = f"""
                 JOIN funnel_actors as actors
                     ON event.person_id = actors.actor_id

ee/session_recordings/queries/test/test_session_recording_list_from_session_replay.py

@@ -63,7 +63,7 @@ def create_event(
                 True,
                 False,
                 False,
-                PersonOnEventsMode.V1_ENABLED,
+                PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS,
                 {
                     "kperson_filter_pre__0": "rgInternal",
                     "kpersonquery_person_filter_fin__0": "rgInternal",
@@ -95,7 +95,7 @@ def create_event(
                 False,
                 True,
                 False,
-                PersonOnEventsMode.V2_ENABLED,
+                PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS,
                 {
                     "event_names": [],
                     "event_start_time": mock.ANY,
@@ -111,7 +111,7 @@ def create_event(
                 False,
                 True,
                 True,
-                PersonOnEventsMode.V2_ENABLED,
+                PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS,
                 {
                     "event_end_time": mock.ANY,
                     "event_names": [],

frontend/src/queries/schema.json

@@ -2620,7 +2620,14 @@
                     "type": "string"
                 },
                 "personsOnEventsMode": {
-                    "enum": ["disabled", "v1_enabled", "v1_mixed", "v2_enabled", "v3_enabled"],
+                    "enum": [
+                        "disabled",
+                        "v1_enabled",
+                        "v1_mixed",
+                        "v2_enabled",
+                        "v3_enabled",
+                        "person_id_override_properties_joined"
+                    ],
                     "type": "string"
                 }
             },

frontend/src/queries/schema.ts

@@ -179,7 +179,13 @@ export interface DataNode extends Node {
 
 /** HogQL Query Options are automatically set per team. However, they can be overriden in the query. */
 export interface HogQLQueryModifiers {
-    personsOnEventsMode?: 'disabled' | 'v1_enabled' | 'v1_mixed' | 'v2_enabled' | 'v3_enabled'
+    personsOnEventsMode?:
+        | 'disabled'
+        | 'v1_enabled'
+        | 'v1_mixed'
+        | 'v2_enabled'
+        | 'v3_enabled'
+        | 'person_id_override_properties_joined'
     personsArgMaxVersion?: 'auto' | 'v1' | 'v2'
     inCohortVia?: 'auto' | 'leftjoin' | 'subquery' | 'leftjoin_conjoined'
     materializationMode?: 'auto' | 'legacy_null_as_string' | 'legacy_null_as_null' | 'disabled'

frontend/src/scenes/settings/SettingsMap.tsx

@@ -31,7 +31,6 @@ import {
     ReplayCostControl,
     ReplayGeneral,
 } from './project/SessionRecordingSettings'
-import { SettingPersonsOnEvents } from './project/SettingPersonsOnEvents'
 import { SlackIntegration } from './project/SlackIntegration'
 import { SurveySettings } from './project/SurveySettings'
 import { ProjectAccountFiltersSetting } from './project/TestAccountFiltersConfig'
@@ -139,11 +138,6 @@ export const SettingsMap: SettingSection[] = [
                 title: 'Group analytics',
                 component: <GroupAnalyticsConfig />,
             },
-            {
-                id: 'persons-on-events',
-                title: 'Persons on events (beta)',
-                component: <SettingPersonsOnEvents />,
-            },
         ],
     },
 

posthog/api/test/__snapshots__/test_insight.ambr

@@ -1616,24 +1616,6 @@
   LIMIT 21 /*controller='project_insights-list',route='api/projects/%28%3FP%3Cparent_lookup_team_id%3E%5B%5E/.%5D%2B%29/insights/%3F%24'*/
   '''
 # ---
-# name: TestInsight.test_listing_insights_does_not_nplus1.30
-  '''
-  SELECT "posthog_taggeditem"."id",
-         "posthog_taggeditem"."tag_id",
-         "posthog_taggeditem"."dashboard_id",
-         "posthog_taggeditem"."insight_id",
-         "posthog_taggeditem"."event_definition_id",
-         "posthog_taggeditem"."property_definition_id",
-         "posthog_taggeditem"."action_id",
-         "posthog_taggeditem"."feature_flag_id"
-  FROM "posthog_taggeditem"
-  WHERE "posthog_taggeditem"."insight_id" IN (1,
-                                              2,
-                                              3,
-                                              4,
-                                              5 /* ... */) /*controller='project_insights-list',route='api/projects/%28%3FP%3Cparent_lookup_team_id%3E%5B%5E/.%5D%2B%29/insights/%3F%24'*/
-  '''
-# ---
 # name: TestInsight.test_listing_insights_does_not_nplus1.4
   '''
   SELECT "posthog_team"."id",

posthog/api/test/test_team.py

@@ -204,47 +204,6 @@ def test_update_project_timezone(self):
             ]
         )
 
-    @freeze_time("2022-02-08")
-    def test_activity_log_tracks_extra_settings(self):
-        self._assert_activity_log_is_empty()
-
-        response = self.client.patch("/api/projects/@current/", {"extra_settings": {"poe_v2_enabled": True}})
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-
-        response_data = response.json()
-        self.assertEqual(response_data["name"], self.team.name)
-        self.assertEqual(response_data["extra_settings"], {"poe_v2_enabled": True})
-
-        self._assert_activity_log(
-            [
-                {
-                    "activity": "updated",
-                    "created_at": "2022-02-08T00:00:00Z",
-                    "detail": {
-                        "changes": [
-                            {
-                                "action": "created",
-                                "after": {"poe_v2_enabled": True},
-                                "before": None,
-                                "field": "extra_settings",
-                                "type": "Team",
-                            },
-                        ],
-                        "name": "Default project",
-                        "short_id": None,
-                        "trigger": None,
-                        "type": None,
-                    },
-                    "item_id": str(self.team.pk),
-                    "scope": "Team",
-                    "user": {
-                        "email": "user1@posthog.com",
-                        "first_name": "",
-                    },
-                },
-            ]
-        )
-
     def test_update_test_filter_default_checked(self):
         response = self.client.patch("/api/projects/@current/", {"test_account_filters_default_checked": "true"})
         self.assertEqual(response.status_code, status.HTTP_200_OK)

posthog/hogql/modifiers.py

@@ -1,12 +1,7 @@
 from typing import Optional, TYPE_CHECKING
 
-from posthog.schema import (
-    HogQLQueryModifiers,
-    InCohortVia,
-    MaterializationMode,
-    PersonsArgMaxVersion,
-    PersonsOnEventsMode,
-)
+from posthog.schema import HogQLQueryModifiers, InCohortVia, MaterializationMode, PersonsArgMaxVersion
+from posthog.utils import PersonOnEventsMode
 
 if TYPE_CHECKING:
     from posthog.models import Team
@@ -21,10 +16,7 @@ def create_default_modifiers_for_team(
         modifiers = modifiers.model_copy()
 
     if modifiers.personsOnEventsMode is None:
-        if team.person_on_events_v3_querying_enabled:
-            modifiers.personsOnEventsMode = PersonsOnEventsMode.v3_enabled
-        else:
-            modifiers.personsOnEventsMode = team.person_on_events_mode
+        modifiers.personsOnEventsMode = team.person_on_events_mode or PersonOnEventsMode.DISABLED
 
     if modifiers.personsArgMaxVersion is None:
         modifiers.personsArgMaxVersion = PersonsArgMaxVersion.auto

posthog/hogql/test/test_modifiers.py

@@ -7,11 +7,7 @@
 
 
 class TestModifiers(BaseTest):
-    @override_settings(
-        PERSON_ON_EVENTS_OVERRIDE=False,
-        PERSON_ON_EVENTS_V2_OVERRIDE=False,
-        PERSON_ON_EVENTS_V3_OVERRIDE=False,
-    )
+    @override_settings(PERSON_ON_EVENTS_OVERRIDE=False, PERSON_ON_EVENTS_V2_OVERRIDE=False)
     def test_create_default_modifiers_for_team_init(self):
         assert self.team.person_on_events_mode == "disabled"
         modifiers = create_default_modifiers_for_team(self.team)
@@ -27,9 +23,6 @@ def test_create_default_modifiers_for_team_init(self):
         )
         assert modifiers.personsOnEventsMode == PersonsOnEventsMode.v2_enabled
 
-        with override_settings(PERSON_ON_EVENTS_V3_OVERRIDE=True):
-            assert create_default_modifiers_for_team(self.team).personsOnEventsMode == PersonsOnEventsMode.v3_enabled
-
     def test_modifiers_persons_on_events_mode_v1_enabled(self):
         query = "SELECT event, person_id FROM events"
 

posthog/hogql/test/test_printer.py

@@ -156,7 +156,9 @@ def test_fields_and_properties(self):
             context = HogQLContext(
                 team_id=self.team.pk,
                 within_non_hogql_query=True,
-                modifiers=HogQLQueryModifiers(personsOnEventsMode=PersonOnEventsMode.V1_ENABLED),
+                modifiers=HogQLQueryModifiers(
+                    personsOnEventsMode=PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS
+                ),
             )
             self.assertEqual(
                 self._expr("person.properties.bla", context),

posthog/models/team/team.py

@@ -57,7 +57,7 @@
 
 # keep in sync with posthog/frontend/src/scenes/project/Settings/ExtraTeamSettings.tsx
 class AvailableExtraSettings:
-    poe_v2_enabled = "poe_v2_enabled"
+    pass
 
 
 class TeamManager(models.Manager):
@@ -290,18 +290,24 @@ def aggregate_users_by_distinct_id(self) -> bool:
 
     @property
     def person_on_events_mode(self) -> PersonOnEventsMode:
-        # Persons on Events V2 always takes priority over Persons on Events V1
-        if self._person_on_events_v2_querying_enabled:
-            tag_queries(person_on_events_mode=PersonOnEventsMode.V2_ENABLED)
-            return PersonOnEventsMode.V2_ENABLED
+        if self._person_on_events_person_id_override_properties_on_events:
+            tag_queries(person_on_events_mode=PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS)
+            return PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS
 
-        if self._person_on_events_querying_enabled:
+        if self._person_on_events_person_id_no_override_properties_on_events:
             # also tag person_on_events_enabled for legacy compatibility
             tag_queries(
                 person_on_events_enabled=True,
-                person_on_events_mode=PersonOnEventsMode.V1_ENABLED,
+                person_on_events_mode=PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS,
             )
-            return PersonOnEventsMode.V1_ENABLED
+            return PersonOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS
+
+        if self._person_on_events_person_id_override_properties_joined:
+            tag_queries(
+                person_on_events_enabled=True,
+                person_on_events_mode=PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_JOINED,
+            )
+            return PersonOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_JOINED
 
         return PersonOnEventsMode.DISABLED
 
@@ -312,25 +318,17 @@ def person_on_events_querying_enabled(self) -> bool:
         return self.person_on_events_mode != PersonOnEventsMode.DISABLED
 
     @property
-    def _person_on_events_querying_enabled(self) -> bool:
+    def _person_on_events_person_id_no_override_properties_on_events(self) -> bool:
         if settings.PERSON_ON_EVENTS_OVERRIDE is not None:
             return settings.PERSON_ON_EVENTS_OVERRIDE
 
         # on PostHog Cloud, use the feature flag
         if is_cloud():
-            # users can override our feature flag via extra_settings
-            if self.extra_settings and AvailableExtraSettings.poe_v2_enabled in self.extra_settings:
-                return self.extra_settings["poe_v2_enabled"]
             return posthoganalytics.feature_enabled(
-                "person-on-events-enabled",
+                "persons-on-events-person-id-no-override-properties-on-events",
                 str(self.uuid),
-                groups={"organization": str(self.organization_id)},
-                group_properties={
-                    "organization": {
-                        "id": str(self.organization_id),
-                        "created_at": self.organization.created_at,
-                    }
-                },
+                groups={"team": str(self.id)},
+                group_properties={"team": {"id": str(self.id), "created_at": self.created_at, "uuid": self.uuid}},
                 only_evaluate_locally=True,
                 send_feature_flag_events=False,
             )
@@ -339,7 +337,7 @@ def _person_on_events_querying_enabled(self) -> bool:
         return get_instance_setting("PERSON_ON_EVENTS_ENABLED")
 
     @property
-    def _person_on_events_v2_querying_enabled(self) -> bool:
+    def _person_on_events_person_id_override_properties_on_events(self) -> bool:
         if settings.PERSON_ON_EVENTS_V2_OVERRIDE is not None:
             return settings.PERSON_ON_EVENTS_V2_OVERRIDE
 
@@ -362,23 +360,24 @@ def _person_on_events_v2_querying_enabled(self) -> bool:
         return get_instance_setting("PERSON_ON_EVENTS_V2_ENABLED")
 
     @property
-    def person_on_events_v3_querying_enabled(self) -> bool:
-        if settings.PERSON_ON_EVENTS_V3_OVERRIDE is not None:
-            return settings.PERSON_ON_EVENTS_V3_OVERRIDE
-
-        return posthoganalytics.feature_enabled(
-            "persons-on-events-v3-reads-enabled",
-            str(self.uuid),
-            groups={"organization": str(self.organization_id)},
-            group_properties={
-                "organization": {
-                    "id": str(self.organization_id),
-                    "created_at": self.organization.created_at,
-                }
-            },
-            only_evaluate_locally=True,
-            send_feature_flag_events=False,
-        )
+    def _person_on_events_person_id_override_properties_joined(self) -> bool:
+        # on PostHog Cloud, use the feature flag
+        if is_cloud():
+            return posthoganalytics.feature_enabled(
+                "persons-on-events-person-id-override-properties-joined",
+                str(self.uuid),
+                groups={"organization": str(self.organization_id)},
+                group_properties={
+                    "organization": {
+                        "id": str(self.organization_id),
+                        "created_at": self.organization.created_at,
+                    }
+                },
+                only_evaluate_locally=True,
+                send_feature_flag_events=False,
+            )
+
+        return False
 
     @property
     def strict_caching_enabled(self) -> bool: