Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cohorts): Add default property tests #18817

Merged
merged 4 commits into from
Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 38 additions & 4 deletions posthog/api/cohort.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
get_feature_flag_hash_key_overrides,
)
from posthog.models.person.person import PersonDistinctId
from posthog.models.property.property import Property
from posthog.queries.insight import insight_sync_execute
import posthoganalytics
from posthog.metrics import LABEL_TEAM_ID
Expand Down Expand Up @@ -556,7 +557,7 @@ def insert_actors_into_cohort_by_query(cohort: Cohort, query: str, params: Dict[
capture_exception(err)


def get_cohort_actors_for_feature_flag(cohort_id: int, flag: str, team_id: int, batchsize: int = 5_000):
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
try:
feature_flag = FeatureFlag.objects.get(team_id=team_id, key=flag)
Expand All @@ -572,14 +573,16 @@ def get_cohort_actors_for_feature_flag(cohort_id: int, flag: str, team_id: int,
cohorts_cache = {}

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

default_person_properties = {}
for condition in feature_flag.conditions:
property_list = Filter(data=condition).property_groups.flat
for property in property_list:
if property.operator not in ("is_set", "is_not_set") and property.type == "person":
default_person_properties[property.key] = ""
default_person_properties.update(get_default_person_property(property, cohorts_cache))

try:
# QuerySet.Iterator() doesn't work with pgbouncer, it will load everything into memory and then stream
Expand All @@ -596,7 +599,7 @@ def get_cohort_actors_for_feature_flag(cohort_id: int, flag: str, team_id: int,
# "distinct_id", flat=True
# )[0]
distinct_id_subquery = Subquery(
PersonDistinctId.objects.filter(person_id=OuterRef("person_id")).values_list("id", flat=True)[:1]
PersonDistinctId.objects.filter(person_id=OuterRef("person_id")).values_list("id", flat=True)[:3]
)
prefetch_related_objects(
batch_of_persons,
Expand All @@ -612,6 +615,10 @@ def get_cohort_actors_for_feature_flag(cohort_id: int, flag: str, team_id: int,
break

for person in all_persons:
# ignore almost-deleted persons / persons with no distinct ids
if len(person.distinct_ids) == 0:
continue

distinct_id = person.distinct_ids[0]
person_overrides = {}
if feature_flag.ensure_experience_continuity:
Expand Down Expand Up @@ -660,3 +667,30 @@ def get_cohort_actors_for_feature_flag(cohort_id: int, flag: str, team_id: int,
if settings.DEBUG or settings.TEST:
raise err
capture_exception(err)


def get_default_person_property(prop: Property, cohorts_cache: Dict[int, Cohort]):
default_person_properties = {}

if prop.operator not in ("is_set", "is_not_set") and prop.type == "person":
default_person_properties[prop.key] = ""
elif prop.type == "cohort" and not isinstance(prop.value, list):
try:
parsed_cohort_id = int(prop.value)
except (ValueError, TypeError):
return None
cohort = cohorts_cache.get(parsed_cohort_id)
if cohort:
return get_default_person_properties_for_cohort(cohort, cohorts_cache)
return default_person_properties


def get_default_person_properties_for_cohort(cohort: Cohort, cohorts_cache: Dict[int, Cohort]) -> Dict[str, str]:
"""
Returns a dictionary of default person properties to use when evaluating a feature flag
"""
default_person_properties = {}
for property in cohort.properties.flat:
default_person_properties.update(get_default_person_property(property, cohorts_cache))

return default_person_properties
Loading
Loading