Skip to content

Commit

Permalink
chore(environments): Update filtering of product analytics resources
Browse files Browse the repository at this point in the history
  • Loading branch information
Twixes committed Dec 5, 2024
1 parent 9b79998 commit 3a18f77
Show file tree
Hide file tree
Showing 38 changed files with 237 additions and 74 deletions.
22 changes: 17 additions & 5 deletions posthog/api/activity_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,18 +131,30 @@ def important_changes(self, request: Request, *args: Any, **kwargs: Any) -> Resp
with timer("gather_query_parts"):
# first things this user created
my_insights = list(
Insight.objects.filter(created_by=user, team_id=self.team.pk).values_list("id", flat=True)
Insight.objects.filter(created_by=user, team__project_id=self.team.project_id).values_list(
"id", flat=True
)
)
my_feature_flags = list(
FeatureFlag.objects.filter(created_by=user, team_id=self.team.pk).values_list("id", flat=True)
FeatureFlag.objects.filter(created_by=user, team__project_id=self.team.project_id).values_list(
"id", flat=True
)
)
my_notebooks = list(
Notebook.objects.filter(created_by=user, team_id=self.team.pk).values_list("short_id", flat=True)
Notebook.objects.filter(created_by=user, team__project_id=self.team.project_id).values_list(
"short_id", flat=True
)
)
my_comments = list(
Comment.objects.filter(created_by=user, team_id=self.team.pk).values_list("id", flat=True)
Comment.objects.filter(created_by=user, team__project_id=self.team.project_id).values_list(
"id", flat=True
)
)
my_cohorts = list(
Cohort.objects.filter(created_by=user, team__project_id=self.team.project_id).values_list(
"id", flat=True
)
)
my_cohorts = list(Cohort.objects.filter(created_by=user, team_id=self.team.pk).values_list("id", flat=True))
my_hog_functions = list(
HogFunction.objects.filter(created_by=user, team_id=self.team.pk).values_list("id", flat=True)
)
Expand Down
22 changes: 13 additions & 9 deletions posthog/api/cohort.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
)
from posthog.models.person.person import PersonDistinctId
from posthog.models.property.property import Property, PropertyGroup
from posthog.models.team.team import Team
from posthog.queries.base import property_group_to_Q
from posthog.metrics import LABEL_TEAM_ID
from posthog.renderers import SafeJSONRenderer
Expand Down Expand Up @@ -195,7 +196,7 @@ def validate_filters(self, request_filters: dict):
instance = cast(Cohort, self.instance)
cohort_id = instance.pk
flags: QuerySet[FeatureFlag] = FeatureFlag.objects.filter(
team_id=self.context["team_id"], active=True, deleted=False
team__project_id=self.context["project_id"], active=True, deleted=False
)
cohort_used_in_flags = len([flag for flag in flags if cohort_id in flag.get_cohort_ids()]) > 0

Expand All @@ -208,7 +209,7 @@ def validate_filters(self, request_filters: dict):
)

if prop.type == "cohort":
nested_cohort = Cohort.objects.get(pk=prop.value, team_id=self.context["team_id"])
nested_cohort = Cohort.objects.get(pk=prop.value, team__project_id=self.context["project_id"])
dependent_cohorts = get_dependent_cohorts(nested_cohort)
for dependent_cohort in [nested_cohort, *dependent_cohorts]:
if (
Expand Down Expand Up @@ -425,7 +426,7 @@ def activity(self, request: request.Request, **kwargs):
page = int(request.query_params.get("page", "1"))

item_id = kwargs["pk"]
if not Cohort.objects.filter(id=item_id, team_id=self.team_id).exists():
if not Cohort.objects.filter(id=item_id, team__project_id=self.project_id).exists():
return Response("", status=status.HTTP_404_NOT_FOUND)

activity_page = load_activity(
Expand Down Expand Up @@ -480,7 +481,7 @@ class LegacyCohortViewSet(CohortViewSet):

def will_create_loops(cohort: Cohort) -> bool:
# Loops can only be formed when trying to update a Cohort, not when creating one
team_id = cohort.team_id
project_id = cohort.team.project_id

# We can model this as a directed graph, where each node is a Cohort and each edge is a reference to another Cohort
# There's a loop only if there's a cycle in the directed graph. The "directed" bit is important.
Expand All @@ -501,7 +502,7 @@ def dfs_loop_helper(current_cohort: Cohort, seen_cohorts, cohorts_on_path):
return True
elif property.value not in seen_cohorts:
try:
nested_cohort = Cohort.objects.get(pk=property.value, team_id=team_id)
nested_cohort = Cohort.objects.get(pk=property.value, team__project_id=project_id)
except Cohort.DoesNotExist:
raise ValidationError("Invalid Cohort ID in filter")

Expand Down Expand Up @@ -623,24 +624,27 @@ def insert_actors_into_cohort_by_query(cohort: Cohort, query: str, params: dict[

def get_cohort_actors_for_feature_flag(cohort_id: int, flag: str, team_id: int, batchsize: int = 1_000):
# :TODO: Find a way to incorporate this into the same code path as feature flag evaluation
project_id = Team.objects.only("project_id").get(pk=team_id).project_id
try:
feature_flag = FeatureFlag.objects.get(team_id=team_id, key=flag)
feature_flag = FeatureFlag.objects.get(team__project_id=project_id, key=flag)
except FeatureFlag.DoesNotExist:
return []

if not feature_flag.active or feature_flag.deleted or feature_flag.aggregation_group_type_index is not None:
return []

cohort = Cohort.objects.get(pk=cohort_id, team_id=team_id)
matcher_cache = FlagsMatcherCache(team_id)
cohort = Cohort.objects.get(pk=cohort_id, team__project_id=project_id)
matcher_cache = FlagsMatcherCache(team_id=team_id)
uuids_to_add_to_cohort = []
cohorts_cache: dict[int, CohortOrEmpty] = {}

if feature_flag.uses_cohorts:
# TODO: Consider disabling flags with cohorts for creating static cohorts
# because this is currently a lot more inefficient for flag matching,
# as we're required to go to the database for each person.
cohorts_cache = {cohort.pk: cohort for cohort in Cohort.objects.filter(team_id=team_id, deleted=False)}
cohorts_cache = {
cohort.pk: cohort for cohort in Cohort.objects.filter(team__project_id=project_id, deleted=False)
}

default_person_properties = {}
for condition in feature_flag.conditions:
Expand Down
4 changes: 3 additions & 1 deletion posthog/api/dashboards/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,9 @@ def create(self, validated_data: dict, *args: Any, **kwargs: Any) -> Dashboard:

elif use_dashboard:
try:
existing_dashboard = Dashboard.objects.get(id=use_dashboard, team_id=team_id)
existing_dashboard = Dashboard.objects.get(
id=use_dashboard, team__project_id=self.context["get_team"]().project_id
)
existing_tiles = (
DashboardTile.objects.filter(dashboard=existing_dashboard)
.exclude(deleted=True)
Expand Down
6 changes: 4 additions & 2 deletions posthog/api/feature_flag.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,9 @@ def properties_all_match(predicate):

if prop.type == "cohort":
try:
initial_cohort: Cohort = Cohort.objects.get(pk=prop.value, team_id=self.context["team_id"])
initial_cohort: Cohort = Cohort.objects.get(
pk=prop.value, team__project_id=self.context["project_id"]
)
dependent_cohorts = get_dependent_cohorts(initial_cohort)
for cohort in [initial_cohort, *dependent_cohorts]:
if [prop for prop in cohort.properties.flat if prop.type == "behavioral"]:
Expand Down Expand Up @@ -666,7 +668,7 @@ def local_evaluation(self, request: request.Request, **kwargs):
seen_cohorts_cache = {
cohort.pk: cohort
for cohort in Cohort.objects.db_manager(DATABASE_FOR_LOCAL_EVALUATION).filter(
team_id=self.team_id, deleted=False
team__project_id=self.project_id, deleted=False
)
}

Expand Down
2 changes: 1 addition & 1 deletion posthog/api/insight.py
Original file line number Diff line number Diff line change
Expand Up @@ -1184,7 +1184,7 @@ def activity(self, request: request.Request, **kwargs):
page = int(request.query_params.get("page", "1"))

item_id = kwargs["pk"]
if not Insight.objects.filter(id=item_id, team_id=self.team_id).exists():
if not Insight.objects.filter(id=item_id, team__project_id=self.project_id).exists():
return Response("", status=status.HTTP_404_NOT_FOUND)

activity_page = load_activity(
Expand Down
2 changes: 1 addition & 1 deletion posthog/api/organization_feature_flag.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def copy_flags(self, request, *args, **kwargs):

# search in destination project by name
destination_cohort = Cohort.objects.filter(
name=original_cohort.name, team_id=target_project_id, deleted=False
name=original_cohort.name, team__project_id=target_project_id, deleted=False
).first()

# create new cohort in the destination project
Expand Down
4 changes: 2 additions & 2 deletions posthog/api/sharing.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,12 @@ def get_serializer_context(

if dashboard_id:
try:
context["dashboard"] = Dashboard.objects.get(id=dashboard_id, team=self.team)
context["dashboard"] = Dashboard.objects.get(id=dashboard_id, team__project_id=self.team.project_id)
except Dashboard.DoesNotExist:
raise NotFound("Dashboard not found.")
if insight_id:
try:
context["insight"] = Insight.objects.get(id=insight_id, team=self.team)
context["insight"] = Insight.objects.get(id=insight_id, team__project_id=self.team.project_id)
except Insight.DoesNotExist:
raise NotFound("Insight not found.")
if recording_id:
Expand Down
4 changes: 2 additions & 2 deletions posthog/api/survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ def validate_conditions(self, value):
return value

action_ids = (value.get("id") for value in values)
project_actions = Action.objects.filter(team_id=self.context["team_id"], id__in=action_ids)
project_actions = Action.objects.filter(team__project_id=self.context["project_id"], id__in=action_ids)

for project_action in project_actions:
for step in project_action.steps:
Expand Down Expand Up @@ -562,7 +562,7 @@ def _associate_actions(self, instance: Survey, conditions):

action_ids = (value.get("id") for value in values)

instance.actions.set(Action.objects.filter(team_id=self.context["team_id"], id__in=action_ids))
instance.actions.set(Action.objects.filter(team__project_id=self.context["project_id"], id__in=action_ids))
instance.save()

def _add_user_survey_interacted_filters(self, instance: Survey, end_date=None):
Expand Down
6 changes: 4 additions & 2 deletions posthog/cdp/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def hog_function_filters_to_expr(filters: dict, team: Team, actions: dict[int, A
action_id = int(filter["id"])
action = actions.get(action_id, None)
if not action:
action = Action.objects.get(id=action_id, team=team)
action = Action.objects.get(id=action_id, team__project_id=team.project_id)
exprs.append(action_to_expr(action))
except KeyError:
# If an action doesn't exist, we want to return no events
Expand Down Expand Up @@ -80,7 +80,9 @@ def compile_filters_expr(filters: Optional[dict], team: Team, actions: Optional[
if actions is None:
# If not provided as an optimization we fetch all actions
actions_list = (
Action.objects.select_related("team").filter(team_id=team.id).filter(id__in=filter_action_ids(filters))
Action.objects.select_related("team")
.filter(team__project_id=team.project_id)
.filter(id__in=filter_action_ids(filters))
)
actions = {action.id: action for action in actions_list}

Expand Down
2 changes: 1 addition & 1 deletion posthog/demo/matrix/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def run_on_team(self, team: Team, user: User):
print(
f"Inferred {event_definition_count} event definitions, {property_definition_count} property definitions, and {event_properties_count} event-property pairs."
)
for cohort in Cohort.objects.filter(team=team):
for cohort in Cohort.objects.filter(team__project_id=team.project_id):
cohort.calculate_people_ch(pending_version=0)
team.project.save()
team.save()
Expand Down
2 changes: 1 addition & 1 deletion posthog/demo/products/hedgebox/matrix.py
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ def set_project_up(self, team, user):
)
),
)
for insight in Insight.objects.filter(team=team)
for insight in Insight.objects.filter(team__project_id=team.project_id)
),
)
except IntegrityError:
Expand Down
10 changes: 10 additions & 0 deletions posthog/hogql/context.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from dataclasses import dataclass, field
from functools import cached_property
from typing import TYPE_CHECKING, Any, Literal, Optional

from posthog.hogql.timings import HogQLTimings
Expand Down Expand Up @@ -94,3 +95,12 @@ def add_error(
):
if not any(n.start == start and n.end == end and n.message == message and n.fix == fix for n in self.errors):
self.errors.append(HogQLNotice(start=start, end=end, message=message, fix=fix))

@cached_property
def project_id(self) -> Optional[int]:
from posthog.models import Team

if not self.team and not self.team_id:
return None
team = self.team or Team.objects.only("project_id").get(id=self.team_id)
return team.project_id
6 changes: 3 additions & 3 deletions posthog/hogql/database/schema/cohort_people.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
}


def select_from_cohort_people_table(requested_fields: dict[str, list[str | int]], team_id: int):
def select_from_cohort_people_table(requested_fields: dict[str, list[str | int]], project_id: int):
from posthog.hogql import ast
from posthog.models import Cohort

cohort_tuples = list(
Cohort.objects.filter(is_static=False, team_id=team_id)
Cohort.objects.filter(is_static=False, team__project_id=project_id)
.exclude(version__isnull=True)
.values_list("id", "version")
)
Expand Down Expand Up @@ -76,7 +76,7 @@ class CohortPeople(LazyTable):
fields: dict[str, FieldOrTable] = COHORT_PEOPLE_FIELDS

def lazy_select(self, table_to_add: LazyTableToAdd, context, node):
return select_from_cohort_people_table(table_to_add.fields_accessed, context.team_id)
return select_from_cohort_people_table(table_to_add.fields_accessed, context.project_id)

def to_printed_clickhouse(self, context):
return "cohortpeople"
Expand Down
4 changes: 2 additions & 2 deletions posthog/hogql/functions/action.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def matches_action(node: ast.Expr, args: list[ast.Expr], context: HogQLContext)
from posthog.hogql.property import action_to_expr

if (isinstance(arg.value, int) or isinstance(arg.value, float)) and not isinstance(arg.value, bool):
actions = Action.objects.filter(id=int(arg.value), team_id=context.team_id).all()
actions = Action.objects.filter(id=int(arg.value), team__project_id=context.project_id).all()

Check failure on line 18 in posthog/hogql/functions/action.py

View workflow job for this annotation

GitHub Actions / Python code quality checks

Incompatible type for lookup 'team__project_id': (got "int | None", expected "str | int")
if len(actions) == 1:
context.add_notice(
start=arg.start,
Expand All @@ -27,7 +27,7 @@ def matches_action(node: ast.Expr, args: list[ast.Expr], context: HogQLContext)
raise QueryError(f"Could not find cohort with ID {arg.value}", node=arg)

if isinstance(arg.value, str):
actions = Action.objects.filter(name=arg.value, team_id=context.team_id).all()
actions = Action.objects.filter(name=arg.value, team__project_id=context.project_id).all()

Check failure on line 30 in posthog/hogql/functions/action.py

View workflow job for this annotation

GitHub Actions / Python code quality checks

Incompatible type for lookup 'team__project_id': (got "int | None", expected "str | int")
if len(actions) == 1:
context.add_notice(
start=arg.start,
Expand Down
4 changes: 2 additions & 2 deletions posthog/hogql/functions/cohort.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def cohort(node: ast.Expr, args: list[ast.Expr], context: HogQLContext) -> ast.E
from posthog.models import Cohort

if (isinstance(arg.value, int) or isinstance(arg.value, float)) and not isinstance(arg.value, bool):
cohorts1 = Cohort.objects.filter(id=int(arg.value), team_id=context.team_id).values_list(
cohorts1 = Cohort.objects.filter(id=int(arg.value), team__project_id=context.project_id).values_list(

Check failure on line 34 in posthog/hogql/functions/cohort.py

View workflow job for this annotation

GitHub Actions / Python code quality checks

Incompatible type for lookup 'team__project_id': (got "int | None", expected "str | int")
"id", "is_static", "version", "name"
)
if len(cohorts1) == 1:
Expand All @@ -45,7 +45,7 @@ def cohort(node: ast.Expr, args: list[ast.Expr], context: HogQLContext) -> ast.E
raise QueryError(f"Could not find cohort with ID {arg.value}", node=arg)

if isinstance(arg.value, str):
cohorts2 = Cohort.objects.filter(name=arg.value, team_id=context.team_id).values_list(
cohorts2 = Cohort.objects.filter(name=arg.value, team__project_id=context.project_id).values_list(

Check failure on line 48 in posthog/hogql/functions/cohort.py

View workflow job for this annotation

GitHub Actions / Python code quality checks

Incompatible type for lookup 'team__project_id': (got "int | None", expected "str | int")
"id", "is_static", "version"
)
if len(cohorts2) == 1:
Expand Down
28 changes: 28 additions & 0 deletions posthog/hogql/functions/test/__snapshots__/test_cohort.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,34 @@
LIMIT 100
'''
# ---
# name: TestCohort.test_in_cohort_dynamic_other_team_in_project
'''
-- ClickHouse

SELECT events.event AS event
FROM events
WHERE and(equals(events.team_id, 99999), in(events.person_id, (
SELECT cohortpeople.person_id AS person_id
FROM cohortpeople
WHERE and(equals(cohortpeople.team_id, 99999), equals(cohortpeople.cohort_id, XX))
GROUP BY cohortpeople.person_id, cohortpeople.cohort_id, cohortpeople.version
HAVING ifNull(greater(sum(cohortpeople.sign), 0), 0))), equals(events.event, %(hogql_val_0)s))
LIMIT 100
SETTINGS readonly=2, max_execution_time=60, allow_experimental_object_type=1, format_csv_allow_double_quotes=0, max_ast_elements=4000000, max_expanded_ast_elements=4000000, max_bytes_before_external_group_by=0

-- HogQL

SELECT event
FROM events
WHERE and(in(person_id, (
SELECT person_id
FROM raw_cohort_people
WHERE equals(cohort_id, XX)
GROUP BY person_id, cohort_id, version
HAVING greater(sum(sign), 0))), equals(event, 'RANDOM_TEST_ID::UUID'))
LIMIT 100
'''
# ---
# name: TestCohort.test_in_cohort_static
'''
-- ClickHouse
Expand Down
21 changes: 21 additions & 0 deletions posthog/hogql/functions/test/test_cohort.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from posthog.hogql.test.utils import pretty_print_response_in_tests
from posthog.models import Cohort
from posthog.models.cohort.util import recalculate_cohortpeople
from posthog.models.team.team import Team
from posthog.models.utils import UUIDT
from posthog.schema import HogQLQueryModifiers
from posthog.test.base import (
Expand Down Expand Up @@ -55,6 +56,26 @@ def test_in_cohort_dynamic(self):
self.assertEqual(len(response.results), 1)
self.assertEqual(response.results[0][0], random_uuid)

@pytest.mark.usefixtures("unittest_snapshot")
@override_settings(PERSON_ON_EVENTS_OVERRIDE=True, PERSON_ON_EVENTS_V2_OVERRIDE=False)
def test_in_cohort_dynamic_other_team_in_project(self):
random_uuid = self._create_random_events()
other_team_in_project = Team.objects.create(organization=self.organization, project=self.project)
cohort = Cohort.objects.create(
team=other_team_in_project, # Not self.team!
groups=[{"properties": [{"key": "$os", "value": "Chrome", "type": "person"}]}],
)
recalculate_cohortpeople(cohort, pending_version=0, initiating_user_id=None)
response = execute_hogql_query(
f"SELECT event FROM events WHERE person_id IN COHORT {cohort.pk} AND event='{random_uuid}'",
self.team,
modifiers=HogQLQueryModifiers(inCohortVia="subquery"),
pretty=False,
)
assert pretty_print_response_in_tests(response, self.team.pk) == self.snapshot
self.assertEqual(len(response.results), 1)
self.assertEqual(response.results[0][0], random_uuid)

@pytest.mark.usefixtures("unittest_snapshot")
@override_settings(PERSON_ON_EVENTS_OVERRIDE=True, PERSON_ON_EVENTS_V2_OVERRIDE=False)
def test_in_cohort_static(self):
Expand Down
Loading

0 comments on commit 3a18f77

Please sign in to comment.