Don't use PersonsOnEventsMode.disabled as fallback

PostHog · Jun 11, 2024 · f443626 · f443626
1 parent 8faf7cc
commit f443626
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 40 deletions.
diff --git a/posthog/hogql/test/test_modifiers.py b/posthog/hogql/test/test_modifiers.py
@@ -16,9 +16,10 @@
 class TestModifiers(BaseTest):
     @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"
+        assert self.team.person_on_events_mode == PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_JOINED
         modifiers = create_default_modifiers_for_team(self.team)
-        assert modifiers.personsOnEventsMode == PersonsOnEventsMode.DISABLED  # NB! not a None
+        # The default is not None! It's explicitly `PERSON_ID_OVERRIDE_PROPERTIES_JOINED`
+        assert modifiers.personsOnEventsMode == PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_JOINED
         modifiers = create_default_modifiers_for_team(
             self.team,
             HogQLQueryModifiers(personsOnEventsMode=PersonsOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS),
@@ -34,13 +35,19 @@ def test_team_modifiers_override(self):
         assert self.team.modifiers is None
         modifiers = create_default_modifiers_for_team(self.team)
         assert modifiers.personsOnEventsMode == self.team.default_modifiers["personsOnEventsMode"]
-        assert modifiers.personsOnEventsMode == PersonsOnEventsMode.DISABLED  # the default mode
+        assert (
+            modifiers.personsOnEventsMode
+            == PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_JOINED  # the default mode
+        )
 
         self.team.modifiers = {"personsOnEventsMode": PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS}
         self.team.save()
         modifiers = create_default_modifiers_for_team(self.team)
         assert modifiers.personsOnEventsMode == PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS
-        assert self.team.default_modifiers["personsOnEventsMode"] == PersonsOnEventsMode.DISABLED  # no change here
+        assert (
+            self.team.default_modifiers["personsOnEventsMode"]
+            == PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_JOINED  # no change here
+        )
 
         self.team.modifiers = {"personsOnEventsMode": PersonsOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS}
         self.team.save()
@@ -55,9 +62,9 @@ def test_team_modifiers_override(self):
     def test_modifiers_persons_on_events_default_is_based_on_team_property(self):
         assert self.team.modifiers is None
         modifiers = create_default_modifiers_for_team(self.team)
-        assert self.team.person_on_events_mode == PersonsOnEventsMode.person_id_override_properties_on_events
+        assert self.team.person_on_events_mode == PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS
         assert modifiers.personsOnEventsMode == self.team.default_modifiers["personsOnEventsMode"]
-        assert modifiers.personsOnEventsMode == PersonsOnEventsMode.person_id_override_properties_on_events
+        assert modifiers.personsOnEventsMode == PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS
 
     def test_modifiers_persons_on_events_mode_person_id_override_properties_on_events(self):
         query = "SELECT event, person_id FROM events"

diff --git a/posthog/models/team/team.py b/posthog/models/team/team.py
@@ -315,27 +315,17 @@ def person_on_events_mode_flag_based_default(self) -> PersonsOnEventsMode:
             return PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS
 
         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=PersonsOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS,
-            )
+            tag_queries(person_on_events_mode=PersonsOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS)
             return PersonsOnEventsMode.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=PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_JOINED,
-            )
-            return PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_JOINED
-
-        return PersonsOnEventsMode.DISABLED  # Fall back to no persons-on-events disabled
+        tag_queries(person_on_events_mode=PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_JOINED)
+        return PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_JOINED
 
     # KLUDGE: DO NOT REFERENCE IN THE BACKEND!
     # Keeping this property for now only to be used by the frontend in certain cases
     @property
     def person_on_events_querying_enabled(self) -> bool:
-        return self.person_on_events_mode in (
+        return self.person_on_events_mode in (  # Whether person properties on events are in use by default
             PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS,
             PersonsOnEventsMode.PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS,
         )
@@ -382,26 +372,6 @@ def _person_on_events_person_id_override_properties_on_events(self) -> bool:
 
         return get_instance_setting("PERSON_ON_EVENTS_V2_ENABLED")
 
-    @property
-    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:
         enabled_teams = get_list(get_instance_setting("STRICT_CACHING_TEAMS"))

diff --git a/posthog/schema.py b/posthog/schema.py
@@ -530,6 +530,8 @@ class PersonsJoinMode(str, Enum):
 
 
 class PersonsOnEventsMode(str, Enum):
+    # `disabled` is deprecated and set for removal
+    # `person_id_override_properties_joined` is the faster functional eqivalent
     DISABLED = "disabled"
     PERSON_ID_NO_OVERRIDE_PROPERTIES_ON_EVENTS = "person_id_no_override_properties_on_events"
     PERSON_ID_OVERRIDE_PROPERTIES_ON_EVENTS = "person_id_override_properties_on_events"