Fix prop filtering when action has props (#2711)

* add prepend clause to prop filtering

* add defaults and parametrize an event field

* add test and make the fix generalized

ee/clickhouse/models/action.py

@@ -9,7 +9,7 @@ from posthog.models.action_step import ActionStep
 from posthog.models.event import Selector
 
 
-def format_action_filter(action: Action, prepend: str = "", index=0, use_loop: bool = False) -> Tuple[str, Dict]:
+def format_action_filter(action: Action, prepend: str = "action", index=0, use_loop: bool = False) -> Tuple[str, Dict]:
     # get action steps
     params = {"team_id": action.team.pk}
     steps = action.steps.all()
@@ -35,7 +35,9 @@ def format_action_filter(action: Action, prepend: str = "", index=0, use_loop: b
             from ee.clickhouse.models.property import parse_prop_clauses
 
             prop_query, prop_params = parse_prop_clauses(
-                Filter(data={"properties": step.properties}).properties, action.team.pk
+                Filter(data={"properties": step.properties}).properties,
+                action.team.pk,
+                prepend="action_props_{}".format(index),
             )
             conditions.append(prop_query.replace("AND", "", 1))
             params = {**params, **prop_params}
@@ -51,8 +53,8 @@ def format_action_filter(action: Action, prepend: str = "", index=0, use_loop: b
     return formatted_query, params
 
 
-def filter_event(step: ActionStep, prepend: str = "", index: int = 0) -> Tuple[List[str], Dict]:
-    params = {}
+def filter_event(step: ActionStep, prepend: str = "event", index: int = 0) -> Tuple[List[str], Dict]:
+    params = {"{}_{}".format(prepend, index): step.event}
     conditions = []
 
     if step.url:
@@ -72,7 +74,7 @@ def filter_event(step: ActionStep, prepend: str = "", index: int = 0) -> Tuple[L
             )
             params.update({"{}_prop_val_{}".format(prepend, index): "%" + step.url + "%"})
 
-    conditions.append("event = '{}'".format(step.event))
+    conditions.append("event = %({}_{})s".format(prepend, index))
 
     return conditions, params
 

ee/clickhouse/models/cohort.py

@@ -13,7 +13,7 @@ def format_person_query(cohort: Cohort) -> Tuple[str, Dict[str, Any]]:
     for group_idx, group in enumerate(cohort.groups):
         if group.get("action_id"):
             action = Action.objects.get(pk=group["action_id"], team_id=cohort.team.pk)
-            action_filter_query, action_params = format_action_filter(action)
+            action_filter_query, action_params = format_action_filter(action, index=group_idx)
             extract_person = "SELECT distinct_id FROM events WHERE team_id = %(team_id)s AND {query}".format(
                 query=action_filter_query
             )

ee/clickhouse/models/property.py

@@ -11,7 +11,7 @@ from posthog.models.team import Team
 
 
 def parse_prop_clauses(
-    filters: List[Property], team_id: int, prepend: str = "", table_name: str = ""
+    filters: List[Property], team_id: int, prepend: str = "global", table_name: str = ""
 ) -> Tuple[str, Dict]:
     final = []
     params: Dict[str, Any] = {"team_id": team_id}

ee/clickhouse/queries/test/test_trends.py

@@ -226,3 +226,35 @@ class TestClickhouseTrends(ClickhouseTestMixin, trend_test_factory(ClickhouseTre
         self.assertEqual(event_response[1]["data"][5], 1)  # property not defined
 
         self.assertTrue(self._compare_entity_response(action_response, event_response))
+
+    # this ensures that the properties don't conflict when formatting params
+    def test_action_with_prop(self):
+        person = Person.objects.create(
+            team_id=self.team.pk, distinct_ids=["blabla", "anonymous_id"], properties={"$some_prop": "some_val"}
+        )
+        sign_up_action = Action.objects.create(team=self.team, name="sign up")
+        ActionStep.objects.create(
+            action=sign_up_action, event="sign up", properties={"$current_url": "https://posthog.com/feedback/1234"}
+        )
+
+        with freeze_time("2020-01-02T13:01:01Z"):
+            _create_event(
+                team=self.team,
+                event="sign up",
+                distinct_id="blabla",
+                properties={"$current_url": "https://posthog.com/feedback/1234"},
+            )
+
+        with freeze_time("2020-01-04T13:01:01Z"):
+            action_response = ClickhouseTrends().run(
+                Filter(
+                    data={
+                        "actions": [{"id": sign_up_action.id, "math": "dau"}],
+                        "properties": [{"key": "$current_url", "value": "fake"}],
+                    }
+                ),
+                self.team,
+            )
+
+        # if the params were shared it would be 1 because action would take precedence
+        self.assertEqual(action_response[0]["count"], 0)