Handle Action.DoesNotExist more

ee/clickhouse/models/action.py

@@ -90,11 +90,8 @@ def filter_event(
 
 def format_entity_filter(entity: Entity, prepend: str = "action", filter_by_team=True) -> Tuple[str, Dict]:
     if entity.type == TREND_FILTER_TYPE_ACTIONS:
-        try:
-            action = Action.objects.get(pk=entity.id)
-            entity_filter, params = format_action_filter(action, prepend=prepend, filter_by_team=filter_by_team)
-        except Action.DoesNotExist:
-            raise ValidationError("This action does not exist")
+        action = entity.get_action()
+        entity_filter, params = format_action_filter(action, prepend=prepend, filter_by_team=filter_by_team)
     else:
         key = f"{prepend}_event"
         entity_filter = f"event = %({key})s"

ee/clickhouse/queries/clickhouse_stickiness.py

@@ -45,7 +45,7 @@ def stickiness(self, entity: Entity, filter: StickinessFilter, team_id: int) ->
         params: Dict = {"team_id": team_id}
         params = {**params, **prop_filter_params, "num_intervals": filter.total_intervals}
         if entity.type == TREND_FILTER_TYPE_ACTIONS:
-            action = Action.objects.get(pk=entity.id)
+            action = entity.get_action()
             action_query, action_params = format_action_filter(action)
             if action_query == "":
                 return {}
@@ -82,13 +82,9 @@ def _retrieve_people(
 
 def _format_entity_filter(entity: Entity) -> Tuple[str, Dict]:
     if entity.type == TREND_FILTER_TYPE_ACTIONS:
-        try:
-            action = Action.objects.get(pk=entity.id)
-            action_query, params = format_action_filter(action)
-            entity_filter = "AND {}".format(action_query)
-
-        except Action.DoesNotExist:
-            raise ValidationError("This action does not exist")
+        action = entity.get_action()
+        action_query, params = format_action_filter(action)
+        entity_filter = "AND {}".format(action_query)
     else:
         entity_filter = "AND event = %(event)s"
         params = {"event": entity.id}

ee/clickhouse/queries/funnels/base.py

@@ -252,7 +252,7 @@ def _get_step_col(self, entity: Entity, index: int, entity_name: str) -> List[st
     def _build_step_query(self, entity: Entity, index: int, entity_name: str) -> str:
         filters = self._build_filters(entity, index)
         if entity.type == TREND_FILTER_TYPE_ACTIONS:
-            action = Action.objects.get(pk=entity.id)
+            action = entity.get_action()
             for action_step in action.steps.all():
                 self.params[entity_name].append(action_step.event)
             action_query, action_params = format_action_filter(action, "{}_step_{}".format(entity_name, index))

ee/clickhouse/queries/funnels/funnel_event_query.py

@@ -49,7 +49,7 @@ def _get_entity_query(self, entities=None, entity_name="events") -> Tuple[str, D
 
         for entity in entities_to_use:
             if entity.type == TREND_FILTER_TYPE_ACTIONS:
-                action = Action.objects.get(pk=entity.id)
+                action = entity.get_action()
                 for action_step in action.steps.all():
                     events.append(action_step.event)
             else:

ee/clickhouse/queries/sessions/util.py

@@ -26,7 +26,7 @@ def entity_query_conditions(filter: Filter, team: Team) -> Tuple[List[str], Dict
     params: Dict[str, Any] = {}
     for index, entity in enumerate(filter.entities):
         if entity.type == TREND_FILTER_TYPE_ACTIONS:
-            action = Action.objects.get(pk=entity.id)
+            action = entity.get_action()
             action_query, action_params = format_action_filter(action, prepend=f"action_{index}")
             entity_conditions.append(action_query)
             params = {**params, **action_params}

ee/clickhouse/queries/trends/breakdown.py

@@ -62,7 +62,7 @@ def _format_breakdown_query(self, entity: Entity, filter: Filter, team_id: int)
         action_query = ""
         action_params: Dict = {}
         if entity.type == TREND_FILTER_TYPE_ACTIONS:
-            action = Action.objects.get(pk=entity.id)
+            action = entity.get_action()
             action_query, action_params = format_action_filter(action, table_name="e")
 
         params = {

ee/clickhouse/queries/trends/lifecycle.py

@@ -57,7 +57,7 @@ def _format_lifecycle_query(self, entity: Entity, filter: Filter, team_id: int)
 
         if entity.type == TREND_FILTER_TYPE_ACTIONS:
             try:
-                action = Action.objects.get(pk=entity.id)
+                action = entity.get_action()
                 event_query, event_params = format_action_filter(action)
             except:
                 return "", {}, self._parse_result(filter, entity)
@@ -130,7 +130,7 @@ def get_people(
 
         if entity.type == TREND_FILTER_TYPE_ACTIONS:
             try:
-                action = Action.objects.get(pk=entity.id)
+                action = entity.get_action()
                 event_query, event_params = format_action_filter(action)
             except:
                 return []

ee/clickhouse/queries/trends/util.py

@@ -95,13 +95,10 @@ def get_active_user_params(filter: Filter, entity: Entity, team_id: int) -> Dict
 def populate_entity_params(entity: Entity) -> Tuple[Dict, Dict]:
     params, content_sql_params = {}, {}
     if entity.type == TREND_FILTER_TYPE_ACTIONS:
-        try:
-            action = Action.objects.get(pk=entity.id)
-            action_query, action_params = format_action_filter(action)
-            params = {**action_params}
-            content_sql_params = {"entity_query": "AND {action_query}".format(action_query=action_query)}
-        except:
-            raise ValidationError("Action does not exist")
+        action = entity.get_action()
+        action_query, action_params = format_action_filter(action)
+        params = {**action_params}
+        content_sql_params = {"entity_query": "AND {action_query}".format(action_query=action_query)}
     else:
         content_sql_params = {"entity_query": "AND event = %(event)s"}
         params = {"event": entity.id}

ee/clickhouse/views/test/test_clickhouse_insights.py

@@ -123,3 +123,9 @@ def test_funnel_time_to_convert_auto_bins(self):
         self.assertEqual(
             response.json(), {"result": [[2220.0, 2], [29080.0, 0], [55940.0, 0], [82800.0, 1],]},
         )
+
+    def test_funnel_invalid_action_handled(self):
+        response = self.client.post("/api/insight/funnel/", {"actions": [{"id": 666, "type": "actions", "order": 0},]},)
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.json(), self.validation_error_response("Action ID 666 does not exist!"))

posthog/models/action.py

@@ -21,6 +21,18 @@ class Meta:
             models.Index(fields=["team_id", "-updated_at"]),
         ]
 
+    name: models.CharField = models.CharField(max_length=400, null=True, blank=True)
+    team: models.ForeignKey = models.ForeignKey("Team", on_delete=models.CASCADE)
+    created_at: models.DateTimeField = models.DateTimeField(auto_now_add=True, blank=True)
+    created_by: models.ForeignKey = models.ForeignKey("User", on_delete=models.CASCADE, null=True, blank=True)
+    deleted: models.BooleanField = models.BooleanField(default=False)
+    events: models.ManyToManyField = models.ManyToManyField("Event", blank=True)
+    post_to_slack: models.BooleanField = models.BooleanField(default=False)
+    slack_message_format: models.CharField = models.CharField(default="", max_length=200, blank=True)
+    is_calculating: models.BooleanField = models.BooleanField(default=False)
+    updated_at: models.DateTimeField = models.DateTimeField(auto_now=True)
+    last_calculated_at: models.DateTimeField = models.DateTimeField(default=timezone.now, blank=True)
+
     def calculate_events(self, start=None, end=None):
         recalculate_all = False
         if start is None and end is None:
@@ -90,18 +102,6 @@ def on_perform(self, event):
             sender=None, event_name="action_performed", instance=self, payload=payload, user=event.team,
         )
 
-    name: models.CharField = models.CharField(max_length=400, null=True, blank=True)
-    team: models.ForeignKey = models.ForeignKey("Team", on_delete=models.CASCADE)
-    created_at: models.DateTimeField = models.DateTimeField(auto_now_add=True, blank=True)
-    created_by: models.ForeignKey = models.ForeignKey("User", on_delete=models.CASCADE, null=True, blank=True)
-    deleted: models.BooleanField = models.BooleanField(default=False)
-    events: models.ManyToManyField = models.ManyToManyField("Event", blank=True)
-    post_to_slack: models.BooleanField = models.BooleanField(default=False)
-    slack_message_format: models.CharField = models.CharField(default="", max_length=200, blank=True)
-    is_calculating: models.BooleanField = models.BooleanField(default=False)
-    updated_at: models.DateTimeField = models.DateTimeField(auto_now=True)
-    last_calculated_at: models.DateTimeField = models.DateTimeField(default=timezone.now, blank=True)
-
     def __str__(self):
         return self.name
 

posthog/models/entity.py

@@ -1,6 +1,9 @@
-from typing import Any, Dict, List, Optional, Union
+from typing import Any, Dict, Optional, Union
+
+from rest_framework.exceptions import ValidationError
 
 from posthog.constants import TREND_FILTER_TYPE_ACTIONS, TREND_FILTER_TYPE_EVENTS
+from posthog.models.action import Action
 from posthog.models.filters.mixins.property import PropertyMixin
 
 
@@ -63,3 +66,13 @@ def equals(self, other) -> bool:
             return False
 
         return True
+
+    def get_action(self) -> Action:
+        if self.type != TREND_FILTER_TYPE_ACTIONS:
+            raise ValueError(
+                f"Action can only be fetched for entities of type {TREND_FILTER_TYPE_ACTIONS}, not {self.type}!"
+            )
+        try:
+            return Action.objects.get(id=self.id)
+        except:
+            raise ValidationError(f"Action ID {self.id} does not exist!")

posthog/models/property.py

@@ -3,7 +3,6 @@
 
 from django.db.models import Exists, OuterRef, Q
 
-from posthog.models import cohort
 from posthog.utils import is_valid_regex
 
 ValueT = Union[str, int, List[str]]

posthog/test/base.py

@@ -67,7 +67,7 @@ def unauthenticated_response(
         }
 
     def validation_error_response(
-        self, message: str = "Malformed request", code: str = "invalid", attr: Optional[str] = None,
+        self, message: str = "Malformed request", code: str = "invalid_input", attr: Optional[str] = None,
     ) -> Dict[str, Optional[str]]:
         return {
             "type": "validation_error",