Skip to content

Commit

Permalink
fix(insights): Clear "data warehouse not supported in funnels" error (#…
Browse files Browse the repository at this point in the history
…24809)

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
Twixes and github-actions[bot] authored Sep 5, 2024
1 parent c26b3dc commit 1bf80d4
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 7 deletions.
35 changes: 35 additions & 0 deletions posthog/api/test/test_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
from posthog.models.utils import UUIDT
from posthog.schema import (
CachedEventsQueryResponse,
DataWarehouseNode,
EventPropertyFilter,
EventsQuery,
FunnelsQuery,
HogQLPropertyFilter,
HogQLQuery,
PersonPropertyFilter,
Expand Down Expand Up @@ -731,6 +733,39 @@ def test_invalid_query_kind(self):
api_response.content,
)

def test_funnel_query_with_data_warehouse_node_temporarily_raises(self):
# As of September 2024, funnels don't support data warehouse tables YET, so we want a helpful error message
api_response = self.client.post(
f"/api/projects/{self.team.id}/query/",
{
"query": FunnelsQuery(
series=[
DataWarehouseNode(
id="xyz",
table_name="xyz",
id_field="id",
distinct_id_field="customer_email",
timestamp_field="created",
),
DataWarehouseNode(
id="abc",
table_name="abc",
id_field="id",
distinct_id_field="customer_email",
timestamp_field="timestamp",
),
],
).model_dump()
},
)
self.assertEqual(api_response.status_code, 400)
self.assertDictEqual(
api_response.json(),
self.validation_error_response(
"Data warehouse tables are not supported in funnels just yet. For now, please try this funnel without the data warehouse-based step."
),
)

def test_missing_query(self):
api_response = self.client.post(f"/api/projects/{self.team.id}/query/", {"query": {}})
self.assertEqual(api_response.status_code, 400)
Expand Down
8 changes: 6 additions & 2 deletions posthog/hogql_queries/insights/funnels/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,9 @@ def _serialize_step(
action_id = step.event
type = "events"
elif isinstance(step, DataWarehouseNode):
raise NotImplementedError("DataWarehouseNode is not supported in funnels")
raise ValidationError(
"Data warehouse tables are not supported in funnels just yet. For now, please try this funnel without the data warehouse-based step."
)
else:
action = Action.objects.get(pk=step.id)
name = action.name
Expand Down Expand Up @@ -584,7 +586,9 @@ def _build_step_query(
action = Action.objects.get(pk=int(entity.id), team=self.context.team)
event_expr = action_to_expr(action)
elif isinstance(entity, DataWarehouseNode):
raise NotImplementedError("DataWarehouseNode is not supported in funnels")
raise ValidationError(
"Data warehouse tables are not supported in funnels just yet. For now, please try this funnel without the data warehouse-based step."
)
elif entity.event is None:
# all events
event_expr = ast.Constant(value=1)
Expand Down
10 changes: 9 additions & 1 deletion posthog/hogql_queries/insights/funnels/funnel_event_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@
from posthog.hogql_queries.utils.query_date_range import QueryDateRange
from posthog.models.action.action import Action
from posthog.models.property.property import PropertyName
from posthog.schema import ActionsNode, EventsNode, FunnelExclusionActionsNode, FunnelExclusionEventsNode
from posthog.schema import (
ActionsNode,
DataWarehouseNode,
EventsNode,
FunnelExclusionActionsNode,
FunnelExclusionEventsNode,
)
from rest_framework.exceptions import ValidationError


Expand Down Expand Up @@ -143,6 +149,8 @@ def _entity_expr(self, skip_entity_filter: bool) -> ast.Expr | None:
events.update(action.get_step_events())
except Action.DoesNotExist:
raise ValidationError(f"Action ID {node.id} does not exist!")
elif isinstance(node, DataWarehouseNode):
continue # Data warehouse nodes aren't based on events
else:
raise ValidationError("Series and exclusions must be compose of action and event nodes")

Expand Down
17 changes: 13 additions & 4 deletions posthog/hogql_queries/insights/utils/entities.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from posthog.schema import (
ActionsNode,
CohortPropertyFilter,
DataWarehouseNode,
EmptyPropertyFilter,
EventsNode,
FunnelExclusionActionsNode,
Expand All @@ -9,16 +10,16 @@
)
from posthog.types import AnyPropertyFilter, EntityNode, ExclusionEntityNode
from collections import Counter
from rest_framework.exceptions import ValidationError


def is_equal_type(a: EntityNode, b: EntityNode | ExclusionEntityNode) -> bool:
if isinstance(a, EventsNode):
return isinstance(b, EventsNode) or isinstance(b, FunnelExclusionEventsNode)
elif isinstance(a, ActionsNode):
if isinstance(a, ActionsNode):
return isinstance(b, ActionsNode) or isinstance(b, FunnelExclusionActionsNode)
else:
raise ValidationError(detail=f"Type comparision for {type(a)} and {type(b)} not implemented.")
if isinstance(a, DataWarehouseNode):
return isinstance(b, DataWarehouseNode)
raise ValueError(detail=f"Type comparison for {type(a)} and {type(b)} not implemented.")


def is_equal(a: EntityNode, b: EntityNode | ExclusionEntityNode, compare_properties=True) -> bool:
Expand All @@ -44,6 +45,14 @@ def is_equal(a: EntityNode, b: EntityNode | ExclusionEntityNode, compare_propert
):
return False

# different data source
if (
isinstance(a, DataWarehouseNode)
and isinstance(b, DataWarehouseNode)
and (a.id != b.id or a.id_field != b.id_field)
):
return False

# different properties
if compare_properties and _sorted_property_reprs(a.properties) != _sorted_property_reprs(b.properties):
return False
Expand Down

0 comments on commit 1bf80d4

Please sign in to comment.