chore(insights): Yeet the unbaked groups-on-events (#22764)

* chore(insights): Yeet the unbaked groups-on-events * Update TestAutoProjectMiddleware * Update verifiedDomainsLogic.test.ts.snap * Update funnel_correlation.py * Update test_property.py * Update query snapshots * Update UI snapshots for `chromium` (2) * Update query snapshots * Update UI snapshots for `chromium` (2) * Update query snapshots * Update query snapshots * Update query snapshots --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
PostHog · Jun 27, 2024 · 6a93be3 · 6a93be3
1 parent a3797ce
commit 6a93be3
Show file tree

Hide file tree

Showing 20 changed files with 818 additions and 1,023 deletions.
diff --git a/.github/actions/run-backend-tests/action.yml b/.github/actions/run-backend-tests/action.yml
@@ -148,7 +148,6 @@ runs:
           if: ${{ inputs.segment == 'Core' }}
           env:
               PERSON_ON_EVENTS_V2_ENABLED: ${{ inputs.person-on-events }}
-              GROUPS_ON_EVENTS_ENABLED: ${{ inputs.person-on-events }}
           shell: bash
           run: | # async_migrations covered in ci-async-migrations.yml
               pytest ${{

diff --git a/ee/clickhouse/models/test/test_property.py b/ee/clickhouse/models/test/test_property.py
@@ -14,7 +14,6 @@
 from posthog.models.filters import Filter
 from posthog.models.instance_setting import (
     get_instance_setting,
-    override_instance_config,
 )
 from posthog.models.organization import Organization
 from posthog.models.property import Property, TableWithProperties
@@ -1037,38 +1036,6 @@ def test_get_property_string_expr(self):
         )
         self.assertEqual(string_expr, ('"mat_pp_some_mat_prop2"', True))
 
-    def test_get_property_string_expr_groups(self):
-        if not get_instance_setting("GROUPS_ON_EVENTS_ENABLED"):
-            return
-
-        materialize("events", "some_mat_prop3", table_column="group2_properties")
-
-        string_expr = get_property_string_expr(
-            "events",
-            "some_mat_prop3",
-            "x",
-            "properties",
-            table_alias="e",
-            materialised_table_column="group2_properties",
-        )
-        self.assertEqual(string_expr, ('e."mat_gp2_some_mat_prop3"', True))
-
-        string_expr = get_property_string_expr(
-            "events",
-            "some_mat_prop3",
-            "'x'",
-            "gp_props_alias",
-            table_alias="e",
-            materialised_table_column="group1_properties",
-        )
-        self.assertEqual(
-            string_expr,
-            (
-                "replaceRegexpAll(JSONExtractRaw(e.gp_props_alias, 'x'), '^\"|\"$', '')",
-                False,
-            ),
-        )
-
 
 @pytest.mark.django_db
 def test_parse_prop_clauses_defaults(snapshot):
@@ -1255,22 +1222,7 @@ def test_breakdown_query_expression(
             {"breakdown_param_1": "$browser"},
         ),
         ('array("mat_pp_$browser") AS value', {"breakdown_param_1": "$browser"}),
-    ),
-    (
-        ["$browser", "$browser_version"],
-        "events",
-        "prop",
-        "properties",
-        "group2_properties",
-        (
-            "array(replaceRegexpAll(JSONExtractRaw(properties, %(breakdown_param_1)s), '^\"|\"$', ''),replaceRegexpAll(JSONExtractRaw(properties, %(breakdown_param_2)s), '^\"|\"$', '')) AS prop",
-            {"breakdown_param_1": "$browser", "breakdown_param_2": "$browser_version"},
-        ),
-        (
-            """array("mat_gp2_$browser",replaceRegexpAll(JSONExtractRaw(properties, %(breakdown_param_2)s), '^\"|\"$', '')) AS prop""",
-            {"breakdown_param_1": "$browser", "breakdown_param_2": "$browser_version"},
-        ),
-    ),
+    )
 ]
 
 
@@ -1289,31 +1241,30 @@ def test_breakdown_query_expression_materialised(
     expected_with: str,
     expected_without: str,
 ):
-    with override_instance_config("GROUPS_ON_EVENTS_ENABLED", True):
-        from posthog.models.team import util
+    from posthog.models.team import util
 
-        util.can_enable_actor_on_events = True
+    util.can_enable_actor_on_events = True
 
-        materialize(table, breakdown[0], table_column="properties")
-        actual = get_single_or_multi_property_string_expr(
-            breakdown,
-            table,
-            query_alias,
-            column,
-            materialised_table_column=materialise_column,
-        )
-        assert actual == expected_with
-
-        materialize(table, breakdown[0], table_column=materialise_column)  # type: ignore
-        actual = get_single_or_multi_property_string_expr(
-            breakdown,
-            table,
-            query_alias,
-            column,
-            materialised_table_column=materialise_column,
-        )
+    materialize(table, breakdown[0], table_column="properties")
+    actual = get_single_or_multi_property_string_expr(
+        breakdown,
+        table,
+        query_alias,
+        column,
+        materialised_table_column=materialise_column,
+    )
+    assert actual == expected_with
+
+    materialize(table, breakdown[0], table_column=materialise_column)  # type: ignore
+    actual = get_single_or_multi_property_string_expr(
+        breakdown,
+        table,
+        query_alias,
+        column,
+        materialised_table_column=materialise_column,
+    )
 
-        assert actual == expected_without
+    assert actual == expected_without
 
 
 @pytest.fixture

diff --git a/ee/clickhouse/queries/funnels/funnel_correlation.py b/ee/clickhouse/queries/funnels/funnel_correlation.py
@@ -24,7 +24,6 @@
 from posthog.models.filters import Filter
 from posthog.models.property.util import get_property_string_expr
 from posthog.models.team import Team
-from posthog.models.team.team import groups_on_events_querying_enabled
 from posthog.queries.funnels.utils import get_funnel_order_actor_class
 from posthog.queries.insight import insight_sync_execute
 from posthog.queries.person_distinct_id_query import get_team_distinct_ids_query
@@ -161,23 +160,7 @@ def properties_to_include(self) -> list[str]:
 
             for property_name in cast(list, self._filter.correlation_property_names):
                 if self._filter.aggregation_group_type_index is not None:
-                    if not groups_on_events_querying_enabled():
-                        continue
-
-                    if "$all" == property_name:
-                        return [f"group{self._filter.aggregation_group_type_index}_properties"]
-
-                    possible_mat_col = mat_event_cols.get(
-                        (
-                            property_name,
-                            f"group{self._filter.aggregation_group_type_index}_properties",
-                        )
-                    )
-                    if possible_mat_col is not None:
-                        props_to_include.append(possible_mat_col)
-                    else:
-                        props_to_include.append(f"group{self._filter.aggregation_group_type_index}_properties")
-
+                    continue  # We don't support group properties on events at this time
                 else:
                     if "$all" == property_name:
                         return [f"person_properties"]
@@ -499,12 +482,6 @@ def _get_events_join_query(self) -> str:
 
     def _get_aggregation_join_query(self):
         if self._filter.aggregation_group_type_index is None:
-            if (
-                alias_poe_mode_for_legacy(self._team.person_on_events_mode) != PersonsOnEventsMode.DISABLED
-                and groups_on_events_querying_enabled()
-            ):
-                return "", {}
-
             person_query, person_query_params = PersonQuery(
                 self._filter,
                 self._team.pk,
@@ -524,12 +501,9 @@ def _get_aggregation_join_query(self):
     def _get_properties_prop_clause(self):
         if (
             alias_poe_mode_for_legacy(self._team.person_on_events_mode) != PersonsOnEventsMode.DISABLED
-            and groups_on_events_querying_enabled()
+            and self._filter.aggregation_group_type_index is None
         ):
-            group_properties_field = f"group{self._filter.aggregation_group_type_index}_properties"
-            aggregation_properties_alias = (
-                "person_properties" if self._filter.aggregation_group_type_index is None else group_properties_field
-            )
+            aggregation_properties_alias = "person_properties"
         else:
             group_properties_field = f"groups_{self._filter.aggregation_group_type_index}.group_properties_{self._filter.aggregation_group_type_index}"
             aggregation_properties_alias = (

diff --git a/ee/clickhouse/queries/groups_join_query.py b/ee/clickhouse/queries/groups_join_query.py
@@ -7,7 +7,6 @@
 from posthog.models.filters.stickiness_filter import StickinessFilter
 from posthog.models.filters.utils import GroupTypeIndex
 from posthog.models.property.util import parse_prop_grouped_clauses
-from posthog.models.team.team import groups_on_events_querying_enabled
 from posthog.queries.util import PersonPropertiesMode, alias_poe_mode_for_legacy
 from posthog.schema import PersonsOnEventsMode
 
@@ -38,9 +37,6 @@ def __init__(
     def get_join_query(self) -> tuple[str, dict]:
         join_queries, params = [], {}
 
-        if self._person_on_events_mode != PersonsOnEventsMode.DISABLED and groups_on_events_querying_enabled():
-            return "", {}
-
         for group_type_index in self._column_optimizer.group_types_to_query:
             var = f"group_index_{group_type_index}"
             group_join_key = self._join_key or f'"$group_{group_type_index}"'

diff --git a/frontend/src/lib/api.mock.ts b/frontend/src/lib/api.mock.ts
@@ -86,7 +86,6 @@ export const MOCK_DEFAULT_TEAM: TeamType = {
     primary_dashboard: 1,
     live_events_columns: null,
     person_on_events_querying_enabled: true,
-    groups_on_events_querying_enabled: true,
     live_events_token: '123',
 }
 

diff --git a/frontend/src/scenes/instance/SystemStatus/systemStatusLogic.ts b/frontend/src/scenes/instance/SystemStatus/systemStatusLogic.ts
@@ -39,7 +39,6 @@ const EDITABLE_INSTANCE_SETTINGS = [
     'EMAIL_REPLY_TO',
     'AGGREGATE_BY_DISTINCT_IDS_TEAMS',
     'PERSON_ON_EVENTS_ENABLED',
-    'GROUPS_ON_EVENTS_ENABLED',
     'STRICT_CACHING_TEAMS',
     'SLACK_APP_CLIENT_ID',
     'SLACK_APP_CLIENT_SECRET',

diff --git a/...nes/settings/organization/VerifiedDomains/__snapshots__/verifiedDomainsLogic.test.ts.snap b/...nes/settings/organization/VerifiedDomains/__snapshots__/verifiedDomainsLogic.test.ts.snap
@@ -61,7 +61,6 @@ exports[`verifiedDomainsLogic values has proper defaults 1`] = `
           "data-attr",
         ],
         "effective_membership_level": 8,
-        "groups_on_events_querying_enabled": true,
         "has_group_types": true,
         "heatmaps_opt_in": true,
         "id": 997,

diff --git a/frontend/src/scenes/teamActivityDescriber.tsx b/frontend/src/scenes/teamActivityDescriber.tsx
@@ -222,7 +222,6 @@ const teamActionsMapping: Record<
     correlation_config: () => null,
     data_attributes: () => null,
     effective_membership_level: () => null,
-    groups_on_events_querying_enabled: () => null,
     has_group_types: () => null,
     ingested_event: () => null,
     is_demo: () => null,

diff --git a/frontend/src/types.ts b/frontend/src/types.ts
@@ -507,7 +507,6 @@ export interface TeamType extends TeamBasicType {
      */
     correlation_config: CorrelationConfigType | null
     person_on_events_querying_enabled: boolean
-    groups_on_events_querying_enabled: boolean
     extra_settings?: Record<string, string | number | boolean | undefined>
     modifiers?: HogQLQueryModifiers
     default_modifiers?: HogQLQueryModifiers

diff --git a/posthog/api/team.py b/posthog/api/team.py
@@ -30,10 +30,7 @@
 from posthog.models.organization import OrganizationMembership
 from posthog.models.personal_api_key import APIScopeObjectOrNotSupported
 from posthog.models.signals import mute_selected_signals
-from posthog.models.team.team import (
-    groups_on_events_querying_enabled,
-    set_team_in_cache,
-)
+from posthog.models.team.team import set_team_in_cache
 from posthog.models.team.util import delete_batch_exports, delete_bulky_postgres_data
 from posthog.models.utils import UUIDT, generate_random_token_project
 from posthog.permissions import (
@@ -117,7 +114,6 @@ class Meta:
 class TeamSerializer(serializers.ModelSerializer, UserPermissionsSerializerMixin):
     effective_membership_level = serializers.SerializerMethodField()
     has_group_types = serializers.SerializerMethodField()
-    groups_on_events_querying_enabled = serializers.SerializerMethodField()
     live_events_token = serializers.SerializerMethodField()
 
     class Meta:
@@ -162,7 +158,6 @@ class Meta:
             "live_events_columns",
             "recording_domains",
             "person_on_events_querying_enabled",
-            "groups_on_events_querying_enabled",
             "inject_web_apps",
             "extra_settings",
             "modifiers",
@@ -184,7 +179,6 @@ class Meta:
             "has_group_types",
             "default_modifiers",
             "person_on_events_querying_enabled",
-            "groups_on_events_querying_enabled",
             "live_events_token",
         )
 
@@ -194,9 +188,6 @@ def get_effective_membership_level(self, team: Team) -> Optional[OrganizationMem
     def get_has_group_types(self, team: Team) -> bool:
         return GroupTypeMapping.objects.filter(team=team).exists()
 
-    def get_groups_on_events_querying_enabled(self, team: Team) -> bool:
-        return groups_on_events_querying_enabled()
-
     def get_live_events_token(self, team: Team) -> Optional[str]:
         return encode_jwt(
             {"team_id": team.id, "api_token": team.api_token},