fix(flags): Clean up backend for relative date ops (#19982)

posthog/api/feature_flag.py

@@ -54,6 +54,9 @@
     ProjectMembershipNecessaryPermissions,
     TeamMemberAccessPermission,
 )
+from posthog.queries.base import (
+    determine_parsed_date_for_property_matching,
+)
 from posthog.rate_limit import BurstRateThrottle
 from loginas.utils import is_impersonated_session
 
@@ -233,6 +236,14 @@ def properties_all_match(predicate):
                             code="cohort_does_not_exist",
                         )
 
+                if prop.operator in ("is_date_before", "is_date_after"):
+                    parsed_date = determine_parsed_date_for_property_matching(prop.value)
+
+                    if not parsed_date:
+                        raise serializers.ValidationError(
+                            detail=f"Invalid date value: {prop.value}", code="invalid_date"
+                        )
+
         payloads = filters.get("payloads", {})
 
         if not isinstance(payloads, dict):
@@ -625,6 +636,7 @@ def user_blast_radius(self, request: request.Request, **kwargs):
         condition = request.data.get("condition") or {}
         group_type_index = request.data.get("group_type_index", None)
 
+        # TODO: Handle distinct_id and $group_key properties, which are not currently supported
         users_affected, total_users = get_user_blast_radius(self.team, condition, group_type_index)
 
         return Response(

posthog/api/test/__snapshots__/test_feature_flag.ambr

@@ -276,12 +276,12 @@
          (SELECT id
           FROM person
           WHERE team_id = 2
-            AND ((has(['none'], replaceRegexpAll(JSONExtractRaw(properties, 'group'), '^"|"$', ''))
-                  OR has(['1', '2', '3'], replaceRegexpAll(JSONExtractRaw(properties, 'group'), '^"|"$', '')))) )
+            AND (((has(['none'], replaceRegexpAll(JSONExtractRaw(properties, 'group'), '^"|"$', '')))
+                  OR (has(['1', '2', '3'], replaceRegexpAll(JSONExtractRaw(properties, 'group'), '^"|"$', ''))))) )
      GROUP BY id
      HAVING max(is_deleted) = 0
-     AND ((has(['none'], replaceRegexpAll(JSONExtractRaw(argMax(person.properties, version), 'group'), '^"|"$', ''))
-           OR has(['1', '2', '3'], replaceRegexpAll(JSONExtractRaw(argMax(person.properties, version), 'group'), '^"|"$', '')))) SETTINGS optimize_aggregation_in_order = 1)
+     AND (((has(['none'], replaceRegexpAll(JSONExtractRaw(argMax(person.properties, version), 'group'), '^"|"$', '')))
+           OR (has(['1', '2', '3'], replaceRegexpAll(JSONExtractRaw(argMax(person.properties, version), 'group'), '^"|"$', ''))))) SETTINGS optimize_aggregation_in_order = 1)
   '''
 # ---
 # name: TestBlastRadius.test_user_blast_radius_with_single_cohort.1

posthog/api/test/test_feature_flag.py

@@ -2863,6 +2863,53 @@ def test_validation_person_properties(self):
             },
         )
 
+    def test_create_flag_with_invalid_date(self):
+        resp = self._create_flag_with_properties(
+            "date-flag",
+            [
+                {
+                    "key": "created_for",
+                    "type": "person",
+                    "value": "6hed",
+                    "operator": "is_date_before",
+                }
+            ],
+            expected_status=status.HTTP_400_BAD_REQUEST,
+        )
+
+        self.assertDictContainsSubset(
+            {
+                "type": "validation_error",
+                "code": "invalid_date",
+                "detail": "Invalid date value: 6hed",
+                "attr": "filters",
+            },
+            resp.json(),
+        )
+
+        resp = self._create_flag_with_properties(
+            "date-flag",
+            [
+                {
+                    "key": "created_for",
+                    "type": "person",
+                    "value": "1234-02-993284",
+                    "operator": "is_date_after",
+                }
+            ],
+            expected_status=status.HTTP_400_BAD_REQUEST,
+        )
+
+        self.assertDictContainsSubset(
+            {
+                "type": "validation_error",
+                "code": "invalid_date",
+                "detail": "Invalid date value: 1234-02-993284",
+                "attr": "filters",
+            },
+            resp.json(),
+        )
+
     def test_creating_feature_flag_with_non_existant_cohort(self):
         cohort_request = self._create_flag_with_properties(
             "cohort-flag",
@@ -4150,6 +4197,37 @@ def test_user_blast_radius(self):
         response_json = response.json()
         self.assertDictContainsSubset({"users_affected": 4, "total_users": 10}, response_json)
 
+    @freeze_time("2024-01-11")
+    def test_user_blast_radius_with_relative_date_filters(self):
+        for i in range(8):
+            _create_person(
+                team_id=self.team.pk,
+                distinct_ids=[f"person{i}"],
+                properties={"group": f"{i}", "created_at": f"2023-0{i+1}-04"},
+            )
+
+        response = self.client.post(
+            f"/api/projects/{self.team.id}/feature_flags/user_blast_radius",
+            {
+                "condition": {
+                    "properties": [
+                        {
+                            "key": "created_at",
+                            "type": "person",
+                            "value": "-10m",
+                            "operator": "is_date_before",
+                        }
+                    ],
+                    "rollout_percentage": 100,
+                }
+            },
+        )
+
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+        response_json = response.json()
+        self.assertDictContainsSubset({"users_affected": 3, "total_users": 8}, response_json)
+
     def test_user_blast_radius_with_zero_users(self):
         response = self.client.post(
             f"/api/projects/{self.team.id}/feature_flags/user_blast_radius",

posthog/models/feature_flag/flag_matching.py

@@ -682,6 +682,9 @@ def get_all_feature_flags(
     property_value_overrides: Dict[str, Union[str, int]] = {},
     group_property_value_overrides: Dict[str, Dict[str, Union[str, int]]] = {},
 ) -> Tuple[Dict[str, Union[str, bool]], Dict[str, dict], Dict[str, object], bool]:
+    property_value_overrides, group_property_value_overrides = add_local_person_and_group_properties(
+        distinct_id, groups, property_value_overrides, group_property_value_overrides
+    )
     all_feature_flags = get_feature_flags_for_team_in_cache(team_id)
     cache_hit = True
     if all_feature_flags is None:
@@ -954,3 +957,17 @@ def get_all_properties_with_math_operators(
             all_keys_and_fields.append(key_and_field_for_property(prop))
 
     return all_keys_and_fields
+
+
+def add_local_person_and_group_properties(distinct_id, groups, person_properties, group_properties):
+    all_person_properties = {"distinct_id": distinct_id, **(person_properties or {})}
+
+    all_group_properties = {}
+    if groups:
+        for group_name in groups:
+            all_group_properties[group_name] = {
+                "$group_key": groups[group_name],
+                **(group_properties.get(group_name) or {}),
+            }
+
+    return all_person_properties, all_group_properties

posthog/models/feature_flag/user_blast_radius.py

@@ -7,6 +7,19 @@
 from posthog.models.filters import Filter
 from posthog.models.property import GroupTypeIndex
 from posthog.models.team.team import Team
+from posthog.queries.base import relative_date_parse_for_feature_flag_matching
+
+
+def replace_proxy_properties(team: Team, feature_flag_condition: dict):
+    prop_groups = Filter(data=feature_flag_condition, team=team).property_groups
+
+    for prop in prop_groups.flat:
+        if prop.operator in ("is_date_before", "is_date_after"):
+            relative_date = relative_date_parse_for_feature_flag_matching(str(prop.value))
+            if relative_date:
+                prop.value = relative_date.strftime("%Y-%m-%d %H:%M:%S")
+
+    return Filter(data={"properties": prop_groups.to_dict()}, team=team)
 
 
 def get_user_blast_radius(
@@ -19,14 +32,16 @@ def get_user_blast_radius(
     # No rollout % calculations here, since it makes more sense to compute that on the frontend
     properties = feature_flag_condition.get("properties") or []
 
+    cleaned_filter = replace_proxy_properties(team, feature_flag_condition)
+
     if group_type_index is not None:
         try:
             from ee.clickhouse.queries.groups_join_query import GroupsJoinQuery
         except Exception:
             return 0, 0
 
         if len(properties) > 0:
-            filter = Filter(data=feature_flag_condition, team=team)
+            filter = cleaned_filter
 
             for property in filter.property_groups.flat:
                 if property.group_type_index is None or (property.group_type_index != group_type_index):
@@ -50,7 +65,7 @@ def get_user_blast_radius(
         return total_affected_count, team.groups_seen_so_far(group_type_index)
 
     if len(properties) > 0:
-        filter = Filter(data=feature_flag_condition, team=team)
+        filter = cleaned_filter
         cohort_filters = []
         for property in filter.property_groups.flat:
             if property.type in ["cohort", "precalculated-cohort", "static-cohort"]:

posthog/models/filters/test/__snapshots__/test_filter.ambr

@@ -359,7 +359,7 @@
   INNER JOIN "posthog_persondistinctid" ON ("posthog_person"."id" = "posthog_persondistinctid"."person_id")
   WHERE ("posthog_persondistinctid"."distinct_id" = 'example_id'
          AND "posthog_person"."team_id" = 2
-         AND "posthog_person"."id" = -1)
+         AND ("posthog_person"."properties" -> 'created_at') > '["2m", "3d"]')
   LIMIT 1
   '''
 # ---
@@ -370,7 +370,7 @@
   INNER JOIN "posthog_persondistinctid" ON ("posthog_person"."id" = "posthog_persondistinctid"."person_id")
   WHERE ("posthog_persondistinctid"."distinct_id" = 'example_id'
          AND "posthog_person"."team_id" = 2
-         AND "posthog_person"."id" = -1)
+         AND ("posthog_person"."properties" -> 'created_at') > '"bazinga"')
   LIMIT 1
   '''
 # ---

posthog/models/filters/test/test_filter.py

@@ -781,11 +781,7 @@ def test_person_relative_date_parsing(self):
             properties={"created_at": "2021-04-04T12:00:00Z"},
         )
         filter = Filter(
-            data={
-                "properties": [
-                    {"key": "created_at", "value": "2d", "type": "person", "operator": "is_relative_date_after"}
-                ]
-            }
+            data={"properties": [{"key": "created_at", "value": "2d", "type": "person", "operator": "is_date_after"}]}
         )
 
         with self.assertNumQueries(1), freeze_time("2021-04-06T10:00:00"):
@@ -807,11 +803,7 @@ def test_person_relative_date_parsing_with_override_property(self):
             properties={"created_at": "2021-04-04T12:00:00Z"},
         )
         filter = Filter(
-            data={
-                "properties": [
-                    {"key": "created_at", "value": "2m", "type": "person", "operator": "is_relative_date_after"}
-                ]
-            }
+            data={"properties": [{"key": "created_at", "value": "2m", "type": "person", "operator": "is_date_after"}]}
         )
 
         with self.assertNumQueries(1):
@@ -840,7 +832,7 @@ def test_person_relative_date_parsing_with_invalid_date(self):
         filter = Filter(
             data={
                 "properties": [
-                    {"key": "created_at", "value": ["2m", "3d"], "type": "person", "operator": "is_relative_date_after"}
+                    {"key": "created_at", "value": ["2m", "3d"], "type": "person", "operator": "is_date_after"}
                 ]
             }
         )
@@ -859,9 +851,7 @@ def test_person_relative_date_parsing_with_invalid_date(self):
 
         filter = Filter(
             data={
-                "properties": [
-                    {"key": "created_at", "value": "bazinga", "type": "person", "operator": "is_relative_date_after"}
-                ]
+                "properties": [{"key": "created_at", "value": "bazinga", "type": "person", "operator": "is_date_after"}]
             }
         )
 

posthog/models/property/property.py

@@ -71,8 +71,6 @@ class BehavioralPropertyType(str, Enum):
     "is_date_exact",
     "is_date_after",
     "is_date_before",
-    "is_relative_date_after",
-    "is_relative_date_before",
 ]
 
 OperatorInterval = Literal["day", "week", "month", "year"]

posthog/models/property/util.py

@@ -397,8 +397,6 @@ def negate_operator(operator: OperatorType) -> OperatorType:
         "is_not_set": "is_set",
         "is_date_before": "is_date_after",
         "is_date_after": "is_date_before",
-        "is_relative_date_before": "is_relative_date_after",
-        "is_relative_date_after": "is_relative_date_before",
         # is_date_exact not yet supported
     }.get(operator, operator)  # type: ignore
 

posthog/queries/base.py

@@ -172,35 +172,28 @@ def compare(lhs, rhs, operator):
         else:
             return compare(str(override_value), str(value), operator)
 
-    if operator in ["is_date_before", "is_date_after", "is_relative_date_before", "is_relative_date_after"]:
-        try:
-            if operator in ["is_relative_date_before", "is_relative_date_after"]:
-                parsed_date = relative_date_parse_for_feature_flag_matching(str(value))
-            else:
-                parsed_date = parser.parse(str(value))
-                parsed_date = convert_to_datetime_aware(parsed_date)
-        except Exception:
-            return False
+    if operator in ["is_date_before", "is_date_after"]:
+        parsed_date = determine_parsed_date_for_property_matching(value)
 
         if not parsed_date:
             return False
 
         if isinstance(override_value, datetime.datetime):
             override_date = convert_to_datetime_aware(override_value)
-            if operator in ("is_date_before", "is_relative_date_before"):
+            if operator == "is_date_before":
                 return override_date < parsed_date
             else:
                 return override_date > parsed_date
         elif isinstance(override_value, datetime.date):
-            if operator in ("is_date_before", "is_relative_date_before"):
+            if operator == "is_date_before":
                 return override_value < parsed_date.date()
             else:
                 return override_value > parsed_date.date()
         elif isinstance(override_value, str):
             try:
                 override_date = parser.parse(override_value)
                 override_date = convert_to_datetime_aware(override_date)
-                if operator in ("is_date_before", "is_relative_date_before"):
+                if operator == "is_date_before":
                     return override_date < parsed_date
                 else:
                     return override_date > parsed_date
@@ -210,6 +203,20 @@ def compare(lhs, rhs, operator):
     return False
 
 
+def determine_parsed_date_for_property_matching(value: ValueT):
+    parsed_date = None
+    try:
+        parsed_date = relative_date_parse_for_feature_flag_matching(str(value))
+
+        if not parsed_date:
+            parsed_date = parser.parse(str(value))
+            parsed_date = convert_to_datetime_aware(parsed_date)
+    except Exception:
+        return None
+
+    return parsed_date
+
+
 def empty_or_null_with_value_q(
     column: str,
     key: str,
@@ -349,16 +356,13 @@ def property_to_Q(
             negated=True,
         )
 
-    if property.operator in ("is_date_after", "is_date_before", "is_relative_date_before", "is_relative_date_after"):
-        effective_operator = "gt" if property.operator in ("is_date_after", "is_relative_date_after") else "lt"
+    if property.operator in ("is_date_after", "is_date_before"):
+        effective_operator = "gt" if property.operator == "is_date_after" else "lt"
         effective_value = value
-        if property.operator in ("is_relative_date_before", "is_relative_date_after"):
-            relative_date = relative_date_parse_for_feature_flag_matching(str(value))
-            if relative_date:
-                effective_value = relative_date.isoformat()
-            else:
-                # Return no data for invalid relative dates
-                return Q(pk=-1)
+
+        relative_date = relative_date_parse_for_feature_flag_matching(str(value))
+        if relative_date:
+            effective_value = relative_date.isoformat()
 
         return Q(**{f"{column}__{property.key}__{effective_operator}": effective_value})
 
@@ -444,7 +448,7 @@ def is_truthy_or_falsy_property_value(value: Any) -> bool:
 
 
 def relative_date_parse_for_feature_flag_matching(value: str) -> Optional[datetime.datetime]:
-    regex = r"^(?P<number>[0-9]+)(?P<interval>[a-z])$"
+    regex = r"^-?(?P<number>[0-9]+)(?P<interval>[a-z])$"
     match = re.search(regex, value)
     parsed_dt = datetime.datetime.now(tz=ZoneInfo("UTC"))
     if match: