Skip to content

Commit

Permalink
feat: split funnel trends and steps flags (#24949)
Browse files Browse the repository at this point in the history
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
aspicer and github-actions[bot] authored Sep 13, 2024
1 parent 30dbf91 commit a6b229f
Show file tree
Hide file tree
Showing 10 changed files with 198 additions and 556 deletions.
1 change: 1 addition & 0 deletions frontend/src/lib/constants.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ export const FEATURE_FLAGS = {
WEB_ANALYTICS_LIVE_USER_COUNT: 'web-analytics-live-user-count', // owner: @robbie-c
SETTINGS_SESSION_TABLE_VERSION: 'settings-session-table-version', // owner: @robbie-c
INSIGHT_FUNNELS_USE_UDF: 'insight-funnels-use-udf', // owner: @aspicer #team-product-analytics
INSIGHT_FUNNELS_USE_UDF_TRENDS: 'insight-funnels-use-udf-trends', // owner: @aspicer #team-product-analytics
FIRST_TIME_FOR_USER_MATH: 'first-time-for-user-math', // owner: @skoob13 #team-product-analytics
MULTITAB_EDITOR: 'multitab-editor', // owner: @EDsCODE #team-data-warehouse
WEB_ANALYTICS_REPLAY: 'web-analytics-replay', // owner: @robbie-c
Expand Down
15 changes: 13 additions & 2 deletions posthog/hogql_queries/insights/funnels/funnels_query_runner.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from datetime import timedelta

from posthog.hogql.constants import HogQLGlobalSettings, MAX_BYTES_BEFORE_EXTERNAL_GROUP_BY
from math import ceil
from typing import Optional, Any
Expand All @@ -19,7 +20,10 @@
from posthog.hogql_queries.insights.funnels.funnel_trends import FunnelTrends
from posthog.hogql_queries.insights.funnels.funnel_trends_udf import FunnelTrendsUDF
from posthog.hogql_queries.insights.funnels.utils import get_funnel_actor_class, get_funnel_order_class
from posthog.hogql_queries.legacy_compatibility.feature_flag import insight_funnels_use_udf
from posthog.hogql_queries.legacy_compatibility.feature_flag import (
insight_funnels_use_udf,
insight_funnels_use_udf_trends,
)
from posthog.hogql_queries.query_runner import QueryRunner
from posthog.hogql_queries.utils.query_date_range import QueryDateRange
from posthog.models import Team
Expand Down Expand Up @@ -111,7 +115,14 @@ def calculate(self):

@cached_property
def _use_udf(self):
return self.context.funnelsFilter.useUdf or insight_funnels_use_udf(self.team)
if self.context.funnelsFilter.useUdf:
return True
funnelVizType = self.context.funnelsFilter.funnelVizType
if funnelVizType == FunnelVizType.TRENDS and insight_funnels_use_udf_trends(self.team):
return True
if funnelVizType == FunnelVizType.STEPS and insight_funnels_use_udf(self.team):
return True
return False

@cached_property
def funnel_order_class(self):
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@
WHERE equals(person_distinct_id2.team_id, 2)
GROUP BY person_distinct_id2.distinct_id
HAVING ifNull(equals(argMax(person_distinct_id2.is_deleted, person_distinct_id2.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS e__pdi ON equals(e.distinct_id, e__pdi.distinct_id)
INNER JOIN
LEFT JOIN
(SELECT person.id AS id,
replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'email'), ''), 'null'), '^"|"$', '') AS properties___email,
replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'age'), ''), 'null'), '^"|"$', '') AS properties___age
Expand Down Expand Up @@ -463,7 +463,7 @@
WHERE equals(person_distinct_id2.team_id, 2)
GROUP BY person_distinct_id2.distinct_id
HAVING ifNull(equals(argMax(person_distinct_id2.is_deleted, person_distinct_id2.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS e__pdi ON equals(e.distinct_id, e__pdi.distinct_id)
INNER JOIN
LEFT JOIN
(SELECT person.id AS id,
replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'email'), ''), 'null'), '^"|"$', '') AS properties___email,
replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'age'), ''), 'null'), '^"|"$', '') AS properties___age
Expand Down Expand Up @@ -582,7 +582,7 @@
WHERE equals(person_distinct_id2.team_id, 2)
GROUP BY person_distinct_id2.distinct_id
HAVING ifNull(equals(argMax(person_distinct_id2.is_deleted, person_distinct_id2.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS e__pdi ON equals(e.distinct_id, e__pdi.distinct_id)
INNER JOIN
LEFT JOIN
(SELECT person.id AS id,
replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'email'), ''), 'null'), '^"|"$', '') AS properties___email,
replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'age'), ''), 'null'), '^"|"$', '') AS properties___age
Expand Down Expand Up @@ -701,7 +701,7 @@
WHERE equals(person_distinct_id2.team_id, 2)
GROUP BY person_distinct_id2.distinct_id
HAVING ifNull(equals(argMax(person_distinct_id2.is_deleted, person_distinct_id2.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS e__pdi ON equals(e.distinct_id, e__pdi.distinct_id)
INNER JOIN
LEFT JOIN
(SELECT person.id AS id,
replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'email'), ''), 'null'), '^"|"$', '') AS properties___email,
replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'age'), ''), 'null'), '^"|"$', '') AS properties___age
Expand Down
25 changes: 23 additions & 2 deletions posthog/hogql_queries/insights/funnels/test/test_funnel_trends.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from posthog.models.cohort.cohort import Cohort
from posthog.models.filters import Filter
from posthog.queries.funnels.funnel_trends_persons import ClickhouseFunnelTrendsActors
from posthog.schema import FunnelsQuery
from posthog.schema import FunnelsQuery, FunnelsQueryResponse
from posthog.test.base import (
APIBaseTest,
ClickhouseTestMixin,
Expand All @@ -25,6 +25,7 @@


class BaseTestFunnelTrends(ClickhouseTestMixin, APIBaseTest):
__test__ = False
maxDiff = None

def _get_actors_at_step(self, filter, entrance_period_start, drop_off):
Expand Down Expand Up @@ -1616,4 +1617,24 @@ def test_parses_breakdown_correctly(self):

@patch("posthoganalytics.feature_enabled", new=Mock(return_value=False))
class TestFunnelTrends(BaseTestFunnelTrends):
pass
__test__ = True

def test_assert_flag_is_working(self):
filters = {
"insight": INSIGHT_FUNNELS,
"funnel_viz_type": "trends",
"display": TRENDS_LINEAR,
"interval": "hour",
"date_from": "2021-05-01 00:00:00",
"funnel_window_interval": 7,
"events": [
{"id": "step one", "order": 0},
{"id": "step two", "order": 1},
{"id": "step three", "order": 2},
],
}

query = cast(FunnelsQuery, filter_to_query(filters))
results = cast(FunnelsQueryResponse, FunnelsQueryRunner(query=query, team=self.team).calculate())

self.assertFalse(results.isUdf)
Original file line number Diff line number Diff line change
@@ -1,8 +1,55 @@
from typing import cast
from unittest.mock import patch, Mock

from posthog.constants import INSIGHT_FUNNELS, TRENDS_LINEAR
from posthog.hogql_queries.insights.funnels.funnels_query_runner import FunnelsQueryRunner
from posthog.hogql_queries.insights.funnels.test.test_funnel_trends import BaseTestFunnelTrends
from posthog.hogql_queries.legacy_compatibility.filter_to_query import filter_to_query
from posthog.schema import FunnelsQuery, FunnelsQueryResponse


@patch("posthoganalytics.feature_enabled", new=Mock(return_value=True))
@patch(
"posthoganalytics.feature_enabled",
new=Mock(side_effect=lambda key, *args, **kwargs: key == "insight-funnels-use-udf-trends"),
)
class TestFunnelTrendsUDF(BaseTestFunnelTrends):
pass
__test__ = True

def test_assert_trends_flag_is_on(self):
filters = {
"insight": INSIGHT_FUNNELS,
"funnel_viz_type": "trends",
"display": TRENDS_LINEAR,
"interval": "hour",
"date_from": "2021-05-01 00:00:00",
"funnel_window_interval": 7,
"events": [
{"id": "step one", "order": 0},
{"id": "step two", "order": 1},
{"id": "step three", "order": 2},
],
}

query = cast(FunnelsQuery, filter_to_query(filters))
results = cast(FunnelsQueryResponse, FunnelsQueryRunner(query=query, team=self.team).calculate())

self.assertTrue(results.isUdf)

def test_assert_steps_flag_is_off(self):
filters = {
"insight": INSIGHT_FUNNELS,
"funnel_viz_type": "steps",
"interval": "hour",
"date_from": "2021-05-01 00:00:00",
"funnel_window_interval": 7,
"events": [
{"id": "step one", "order": 0},
{"id": "step two", "order": 1},
{"id": "step three", "order": 2},
],
}

query = cast(FunnelsQuery, filter_to_query(filters))
results = cast(FunnelsQueryResponse, FunnelsQueryRunner(query=query, team=self.team).calculate())

self.assertFalse(results.isUdf)
55 changes: 50 additions & 5 deletions posthog/hogql_queries/insights/funnels/test/test_funnel_udf.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
from typing import cast
from unittest.mock import patch, Mock

from posthog.constants import FunnelOrderType
from posthog.constants import FunnelOrderType, INSIGHT_FUNNELS
from posthog.hogql_queries.insights.funnels import Funnel
from posthog.hogql_queries.insights.funnels.funnels_query_runner import FunnelsQueryRunner
from posthog.hogql_queries.insights.funnels.test.breakdown_cases import (
funnel_breakdown_test_factory,
funnel_breakdown_group_test_factory,
)
from posthog.hogql_queries.legacy_compatibility.filter_to_query import filter_to_query
from posthog.models import Action
from posthog.queries.funnels import ClickhouseFunnelActors
from posthog.schema import FunnelsQuery, FunnelsQueryResponse
from posthog.test.base import (
ClickhouseTestMixin,
_create_event,
Expand All @@ -27,7 +31,10 @@ def _create_action(**kwargs):
return action


@patch("posthoganalytics.feature_enabled", new=Mock(return_value=True))
funnel_flag_side_effect = lambda key, *args, **kwargs: key == "insight-funnels-use-udf"


@patch("posthoganalytics.feature_enabled", new=Mock(side_effect=funnel_flag_side_effect))
class TestFunnelBreakdownUDF(
ClickhouseTestMixin,
funnel_breakdown_test_factory( # type: ignore
Expand All @@ -41,7 +48,7 @@ class TestFunnelBreakdownUDF(
pass


@patch("posthoganalytics.feature_enabled", new=Mock(return_value=True))
@patch("posthoganalytics.feature_enabled", new=Mock(side_effect=funnel_flag_side_effect))
class TestFunnelGroupBreakdownUDF(
ClickhouseTestMixin,
funnel_breakdown_group_test_factory( # type: ignore
Expand All @@ -52,12 +59,50 @@ class TestFunnelGroupBreakdownUDF(
pass


@patch("posthoganalytics.feature_enabled", new=Mock(return_value=True))
@patch("posthoganalytics.feature_enabled", new=Mock(side_effect=funnel_flag_side_effect))
class TestFOSSFunnelUDF(funnel_test_factory(Funnel, _create_event, _create_person)): # type: ignore
def test_assert_flag_is_on(self):
filters = {
"insight": INSIGHT_FUNNELS,
"funnel_viz_type": "steps",
"interval": "hour",
"date_from": "2021-05-01 00:00:00",
"funnel_window_interval": 7,
"events": [
{"id": "step one", "order": 0},
{"id": "step two", "order": 1},
{"id": "step three", "order": 2},
],
}

query = cast(FunnelsQuery, filter_to_query(filters))
results = cast(FunnelsQueryResponse, FunnelsQueryRunner(query=query, team=self.team).calculate())

self.assertTrue(results.isUdf)

def test_assert_trends_flag_is_off(self):
filters = {
"insight": INSIGHT_FUNNELS,
"funnel_viz_type": "trends",
"interval": "hour",
"date_from": "2021-05-01 00:00:00",
"funnel_window_interval": 7,
"events": [
{"id": "step one", "order": 0},
{"id": "step two", "order": 1},
{"id": "step three", "order": 2},
],
}

query = cast(FunnelsQuery, filter_to_query(filters))
results = cast(FunnelsQueryResponse, FunnelsQueryRunner(query=query, team=self.team).calculate())

self.assertFalse(results.isUdf)

maxDiff = None


@patch("posthoganalytics.feature_enabled", new=Mock(return_value=True))
@patch("posthoganalytics.feature_enabled", new=Mock(side_effect=funnel_flag_side_effect))
class TestFunnelConversionTimeUDF(
ClickhouseTestMixin,
funnel_conversion_time_test_factory(FunnelOrderType.ORDERED, ClickhouseFunnelActors), # type: ignore
Expand Down
12 changes: 5 additions & 7 deletions posthog/hogql_queries/insights/funnels/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,11 @@ def get_funnel_actor_class(funnelsFilter: FunnelsFilter):

if funnelsFilter.funnelVizType == FunnelVizType.TRENDS:
return FunnelTrendsActors
else:
if funnelsFilter.funnelOrderType == StepOrderValue.UNORDERED:
return FunnelUnorderedActors
elif funnelsFilter.funnelOrderType == StepOrderValue.STRICT:
return FunnelStrictActors
else:
return FunnelActors
if funnelsFilter.funnelOrderType == StepOrderValue.UNORDERED:
return FunnelUnorderedActors
if funnelsFilter.funnelOrderType == StepOrderValue.STRICT:
return FunnelStrictActors
return FunnelActors


def funnel_window_interval_unit_to_sql(
Expand Down
Loading

0 comments on commit a6b229f

Please sign in to comment.