diff --git a/posthog/hogql_queries/legacy_compatibility/filter_to_query.py b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py index 91e1cdaa75b4b..a12475943d799 100644 --- a/posthog/hogql_queries/legacy_compatibility/filter_to_query.py +++ b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py @@ -1,3 +1,4 @@ +from typing import List, Dict from posthog.models.entity.entity import Entity as BackendEntity from posthog.models.filters import AnyInsightFilter from posthog.models.filters.filter import Filter as LegacyFilter @@ -27,17 +28,72 @@ from posthog.types import InsightQueryNode -def entity_to_node(entity: BackendEntity) -> EventsNode | ActionsNode: +def is_property_with_operator(property: Dict): + return property.get("type") not in ("cohort", "hogql") + + +def clean_property(property: Dict): + cleaned_property = {**property} + + # convert precalculated cohorts to cohorts + if cleaned_property.get("type") == "precalculated-cohort": + cleaned_property["type"] = "cohort" + + # set a default operator for properties that support it, but don't have an operator set + if is_property_with_operator(cleaned_property) and cleaned_property.get("operator") is None: + cleaned_property["operator"] = "exact" + + # remove the operator for properties that don't support it, but have it set + if not is_property_with_operator(cleaned_property) and cleaned_property.get("operator") is not None: + del cleaned_property["operator"] + + # remove keys without concrete value + cleaned_property = {key: value for key, value in cleaned_property.items() if value is not None} + + return cleaned_property + + +def clean_entity_properties(properties: List[Dict] | None): + if properties is None: + return None + else: + return list(map(clean_property, properties)) + + +def clean_property_group_filter_value(value: Dict): + if value.get("type") in ("AND", "OR"): + value["values"] = map(clean_property_group_filter_value, value.get("values")) + return value + else: + return clean_property(value) + + +def clean_properties(properties: Dict): + properties["values"] = map(clean_property_group_filter_value, properties.get("values")) + return properties + + +def entity_to_node(entity: BackendEntity, include_properties: bool, include_math: bool) -> EventsNode | ActionsNode: shared = { "name": entity.name, "custom_name": entity.custom_name, - "properties": entity._data.get("properties", None), - "math": entity.math, - "math_property": entity.math_property, - "math_hogql": entity.math_hogql, - "math_group_type_index": entity.math_group_type_index, } + if include_properties: + shared = { + **shared, + "properties": clean_entity_properties(entity._data.get("properties", None)), + } + + if include_math: + shared = { + **shared, + "math": entity.math, + "math_property": entity.math_property, + "math_hogql": entity.math_hogql, + "math_group_type_index": entity.math_group_type_index, + } + if entity.type == "actions": return ActionsNode(id=entity.id, **shared) else: @@ -75,9 +131,13 @@ def _interval(filter: AnyInsightFilter): def _series(filter: AnyInsightFilter): + include_math = True + include_properties = True if filter.insight == "RETENTION" or filter.insight == "PATHS": return {} - return {"series": map(entity_to_node, filter.entities)} + elif filter.insight == "LIFECYCLE": + include_math = False + return {"series": map(lambda entity: entity_to_node(entity, include_properties, include_math), filter.entities)} def _sampling_factor(filter: AnyInsightFilter): @@ -94,9 +154,9 @@ def _properties(filter: AnyInsightFilter): return {} elif isinstance(raw_properties, list): raw_properties = {"type": "AND", "values": [{"type": "AND", "values": raw_properties}]} - return {"properties": PropertyGroupFilter(**raw_properties)} + return {"properties": PropertyGroupFilter(**clean_properties(raw_properties))} else: - return {"properties": PropertyGroupFilter(**raw_properties)} + return {"properties": PropertyGroupFilter(**clean_properties(raw_properties))} def _breakdown_filter(filter: AnyInsightFilter): diff --git a/posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py b/posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py index 6359b9e3e808d..ac7981576666e 100644 --- a/posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py +++ b/posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py @@ -300,6 +300,85 @@ "filter_test_accounts": True, } +# real world regression tests + +insight_18 = { + "actions": [ + { + "id": 2760, + "math": "total", + "name": "Pageviews", + "type": "actions", + "order": 0, + "properties": [{"key": "$browser", "type": "event", "value": "Chrome", "operator": None}], + "math_property": None, + } + ], + "display": "ActionsBar", + "insight": "LIFECYCLE", + "interval": "day", + "shown_as": "Lifecycle", +} + +insight_19 = { + "events": [ + { + "id": "created change", + "math": "total", + "name": "created change", + "type": "events", + "order": 0, + "properties": [{"key": "id", "type": "cohort", "value": 2208, "operator": None}], + "custom_name": None, + "math_property": None, + } + ], + "display": "ActionsLineGraph", + "insight": "LIFECYCLE", + "interval": "day", + "shown_as": "Lifecycle", +} + +insight_20 = { + "events": [ + { + "id": "$pageview", + "math": "total", + "name": "$pageview", + "type": "events", + "order": 0, + "properties": [], + "math_property": None, + } + ], + "display": "ActionsLineGraph", + "insight": "LIFECYCLE", + "interval": "day", + "shown_as": "Lifecycle", + "properties": [{"key": "id", "type": "cohort", "value": 929, "operator": "exact"}], +} + +insight_21 = { + "actions": [ + { + "id": 4317, + "math": "total", + "name": "Used newer version of the app", + "type": "actions", + "order": 0, + "properties": [], + "custom_name": None, + "math_property": None, + } + ], + "display": "ActionsLineGraph", + "insight": "LIFECYCLE", + "interval": "day", + "shown_as": "Lifecycle", + "properties": [{"key": "id", "type": "precalculated-cohort", "value": 760, "operator": None}], + "funnel_window_days": 14, +} + test_insights = [ insight_0, insight_1, @@ -319,6 +398,10 @@ insight_15, insight_16, insight_17, + insight_18, + insight_19, + insight_20, + insight_21, ] @@ -405,6 +488,7 @@ def test_base_insights(insight): {"type": "OR", "values": [{}]}, ], } +properties_10 = [{"key": "id", "type": "cohort", "value": 71, "operator": None}] test_properties = [ properties_0, @@ -417,6 +501,7 @@ def test_base_insights(insight): properties_7, properties_8, properties_9, + properties_10, ]