fix(hogql): fix edge cases when converting real world lifecycle filters

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):

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,
 ]