From f443626e8359fa7942073e8ea6b9badac7e52055 Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Fri, 7 Jun 2024 16:04:02 +0200 Subject: [PATCH] Don't use `PersonsOnEventsMode.disabled` as fallback --- posthog/hogql/test/test_modifiers.py | 19 +++++++++----- posthog/models/team/team.py | 38 +++------------------------- posthog/schema.py | 2 ++ 3 files changed, 19 insertions(+), 40 deletions(-) diff --git a/posthog/hogql/test/test_modifiers.py b/posthog/hogql/test/test_modifiers.py index ff83ba880d2a9..0f3e8c9799b6b 100644 --- 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 index fc73a8f977273..592f37ce13f60 100644 --- 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 index 91a96d1c96edd..7a6b480853833 100644 --- 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"