diff --git a/ee/clickhouse/models/property.py b/ee/clickhouse/models/property.py index bf4bb49a65f..c20d972baa1 100644 --- a/ee/clickhouse/models/property.py +++ b/ee/clickhouse/models/property.py @@ -78,13 +78,13 @@ def parse_prop_grouped_clauses( _top_level: bool = True, ) -> Tuple[str, Dict]: - if len(property_group.groups) == 0: + if len(property_group.values) == 0: return "", {} - if isinstance(property_group.groups[0], PropertyGroup): + if isinstance(property_group.values[0], PropertyGroup): group_clauses = [] final_params = {} - for idx, group in enumerate(property_group.groups): + for idx, group in enumerate(property_group.values): if isinstance(group, PropertyGroup): clause, params = parse_prop_grouped_clauses( team_id=team_id, @@ -106,7 +106,7 @@ def parse_prop_grouped_clauses( _final = f"{property_group.type} ".join(group_clauses) else: _final, final_params = parse_prop_clauses( - filters=cast(List[Property], property_group.groups), + filters=cast(List[Property], property_group.values), prepend=f"{prepend}", table_name=table_name, allow_denormalized_props=allow_denormalized_props, diff --git a/ee/clickhouse/models/test/test_property.py b/ee/clickhouse/models/test/test_property.py index 3f27049cf58..7c8bbbe2aed 100644 --- a/ee/clickhouse/models/test/test_property.py +++ b/ee/clickhouse/models/test/test_property.py @@ -378,12 +378,12 @@ class TestPropFormat(ClickhouseTestMixin, BaseTest): data={ "properties": { "type": "OR", - "groups": [ + "values": [ { "type": "AND", - "groups": [{"key": "attr_1", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}], + "values": [{"key": "attr_1", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}], }, - {"type": "OR", "groups": [{"key": "attr_1", "value": "val_2"}],}, + {"type": "OR", "values": [{"key": "attr_1", "value": "val_2"}],}, ], } } @@ -397,12 +397,12 @@ class TestPropFormat(ClickhouseTestMixin, BaseTest): data={ "properties": { "type": "OR", - "groups": [ + "values": [ { "type": "AND", - "groups": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}], + "values": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}], }, - {"type": "XOR", "groups": [{"key": "attr", "value": "val_2"}],}, + {"type": "XOR", "values": [{"key": "attr", "value": "val_2"}],}, ], } } @@ -435,9 +435,9 @@ class TestPropFormat(ClickhouseTestMixin, BaseTest): data={ "properties": { "type": "OR", - "groups": [ - {"type": "OR", "groups": [{"key": "email", "type": "person", "value": "1@posthog.com"}],}, - {"type": "OR", "groups": [{"key": "email", "type": "person", "value": "2@posthog.com"}],}, + "values": [ + {"type": "OR", "values": [{"key": "email", "type": "person", "value": "1@posthog.com"}],}, + {"type": "OR", "values": [{"key": "email", "type": "person", "value": "2@posthog.com"}],}, ], } } @@ -595,7 +595,7 @@ def test_parse_prop_clauses_defaults(snapshot): @pytest.mark.django_db def test_parse_groups_persons_edge_case_with_single_filter(snapshot): filter = Filter( - data={"properties": {"type": "OR", "groups": [{"key": "email", "type": "person", "value": "1@posthog.com"}],}} + data={"properties": {"type": "OR", "values": [{"key": "email", "type": "person", "value": "1@posthog.com"}],}} ) assert ( parse_prop_grouped_clauses( diff --git a/ee/clickhouse/queries/breakdown_props.py b/ee/clickhouse/queries/breakdown_props.py index cfb4be933e7..10132ddc4e1 100644 --- a/ee/clickhouse/queries/breakdown_props.py +++ b/ee/clickhouse/queries/breakdown_props.py @@ -52,7 +52,7 @@ def get_breakdown_prop_values( parsed_date_from, parsed_date_to, date_params = parse_timestamps(filter=filter, team_id=team_id) prop_filters, prop_filter_params = parse_prop_grouped_clauses( team_id=team_id, - property_group=PropertyGroup(type=PropertyOperatorType.AND, groups=filter.properties + entity.properties), + property_group=PropertyGroup(type=PropertyOperatorType.AND, values=filter.properties + entity.properties), table_name="e", prepend="e_brkdwn", person_properties_mode=PersonPropertiesMode.EXCLUDE, @@ -135,7 +135,7 @@ def _format_all_query(team_id: int, filter: Filter, **kwargs) -> Tuple[str, Dict prop_filters, prop_filter_params = parse_prop_grouped_clauses( team_id=team_id, - property_group=PropertyGroup(type=PropertyOperatorType.AND, groups=props_to_filter), + property_group=PropertyGroup(type=PropertyOperatorType.AND, values=props_to_filter), prepend="all_cohort_", table_name="all_events", ) diff --git a/ee/clickhouse/queries/event_query.py b/ee/clickhouse/queries/event_query.py index 6e0d7836c5b..60cd8f2d0db 100644 --- a/ee/clickhouse/queries/event_query.py +++ b/ee/clickhouse/queries/event_query.py @@ -188,7 +188,7 @@ class ClickhouseEventQuery(metaclass=ABCMeta): else: filter_query, filter_params = parse_prop_grouped_clauses( team_id=self._team_id, - property_group=PropertyGroup(type=PropertyOperatorType.AND, groups=[prop]), + property_group=PropertyGroup(type=PropertyOperatorType.AND, values=[prop]), prepend=f"global_{idx}", allow_denormalized_props=True, person_properties_mode=PersonPropertiesMode.EXCLUDE, diff --git a/ee/clickhouse/queries/test/test_column_optimizer.py b/ee/clickhouse/queries/test/test_column_optimizer.py index b826d4c1067..6541b5253fe 100644 --- a/ee/clickhouse/queries/test/test_column_optimizer.py +++ b/ee/clickhouse/queries/test/test_column_optimizer.py @@ -17,7 +17,7 @@ PROPERTIES_OF_ALL_TYPES = [ BASE_FILTER = Filter({"events": [{"id": "$pageview", "type": "events", "order": 0}]}) FILTER_WITH_PROPERTIES = BASE_FILTER.with_data({"properties": PROPERTIES_OF_ALL_TYPES}) -FILTER_WITH_GROUPS = BASE_FILTER.with_data({"properties": {"type": "AND", "groups": PROPERTIES_OF_ALL_TYPES}}) +FILTER_WITH_GROUPS = BASE_FILTER.with_data({"properties": {"type": "AND", "values": PROPERTIES_OF_ALL_TYPES}}) class TestColumnOptimizer(ClickhouseTestMixin, APIBaseTest): diff --git a/ee/clickhouse/queries/trends/breakdown.py b/ee/clickhouse/queries/trends/breakdown.py index 4fe590494e2..8694f677dd6 100644 --- a/ee/clickhouse/queries/trends/breakdown.py +++ b/ee/clickhouse/queries/trends/breakdown.py @@ -97,7 +97,7 @@ class ClickhouseTrendsBreakdown: "parsed_date_to": parsed_date_to, "actions_query": "AND {}".format(action_query) if action_query else "", "event_filter": "AND event = %(event)s" if not action_query else "", - "filters": prop_filters if props_to_filter.groups else "", + "filters": prop_filters if props_to_filter.values else "", } _params, _breakdown_filter_params = {}, {} diff --git a/posthog/models/filters/mixins/property.py b/posthog/models/filters/mixins/property.py index f7483dde8d6..a732afdf5c3 100644 --- a/posthog/models/filters/mixins/property.py +++ b/posthog/models/filters/mixins/property.py @@ -23,7 +23,7 @@ class PropertyMixin(BaseParamMixin): loaded_props = _props # if grouped properties - if isinstance(loaded_props, dict) and "type" in loaded_props and "groups" in loaded_props: + if isinstance(loaded_props, dict) and "type" in loaded_props and "values" in loaded_props: # property_groups is main function from now on # TODO: this function will go away at end of migration return [] @@ -44,7 +44,7 @@ class PropertyMixin(BaseParamMixin): loaded_props = _props # if grouped properties - if isinstance(loaded_props, dict) and "type" in loaded_props and "groups" in loaded_props: + if isinstance(loaded_props, dict) and "type" in loaded_props and "values" in loaded_props: try: return self._parse_property_group(loaded_props) except ValidationError as e: @@ -53,18 +53,18 @@ class PropertyMixin(BaseParamMixin): raise ValidationError(f"PropertyGroup is unparsable: {e}") # old properties - return PropertyGroup(type=PropertyOperatorType.AND, groups=self.properties) + return PropertyGroup(type=PropertyOperatorType.AND, values=self.properties) @cached_property def property_groups_flat(self) -> List[Property]: return list(self._property_groups_flat(self.property_groups)) def _property_groups_flat(self, prop_group: PropertyGroup): - for _group in prop_group.groups: - if isinstance(_group, PropertyGroup): - yield from self._property_groups_flat(_group) + for value in prop_group.values: + if isinstance(value, PropertyGroup): + yield from self._property_groups_flat(value) else: - yield _group + yield value def _parse_properties(self, properties: Optional[Any]) -> List[Property]: if isinstance(properties, list): @@ -94,9 +94,9 @@ class PropertyMixin(BaseParamMixin): return ret def _parse_property_group(self, group: Optional[Dict]) -> PropertyGroup: - if group and "type" in group and "groups" in group: + if group and "type" in group and "values" in group: return PropertyGroup( - PropertyOperatorType(group["type"].upper()), self._parse_property_group_list(group["groups"]) + PropertyOperatorType(group["type"].upper()), self._parse_property_group_list(group["values"]) ) return PropertyGroup(PropertyOperatorType.AND, cast(List[Property], [])) @@ -108,7 +108,7 @@ class PropertyMixin(BaseParamMixin): has_property_groups = False has_simple_properties = False for prop in prop_list: - if "type" in prop and "groups" in prop: + if "type" in prop and "values" in prop: has_property_groups = True else: has_simple_properties = True @@ -127,5 +127,5 @@ class PropertyMixin(BaseParamMixin): return {PROPERTIES: [prop.to_dict() for prop in self.properties]} return ( - {PROPERTIES: self.property_groups.to_dict()} if self.property_groups and self.property_groups.groups else {} + {PROPERTIES: self.property_groups.to_dict()} if self.property_groups and self.property_groups.values else {} ) diff --git a/posthog/models/filters/mixins/test/test_property.py b/posthog/models/filters/mixins/test/test_property.py index 84364a6ee2e..f3045b138df 100644 --- a/posthog/models/filters/mixins/test/test_property.py +++ b/posthog/models/filters/mixins/test/test_property.py @@ -12,32 +12,32 @@ def test_property_group_multi_level_parsing(): data={ "properties": { "type": "AND", - "groups": [ + "values": [ { "type": "AND", - "groups": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}], + "values": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}], }, - {"type": "OR", "groups": [{"key": "attr", "value": "val_2"}]}, + {"type": "OR", "values": [{"key": "attr", "value": "val_2"}]}, ], } } ) assert filter.property_groups.type == "AND" - assert isinstance(filter.property_groups.groups[0], PropertyGroup) - assert filter.property_groups.groups[0].type == "AND" - assert isinstance(filter.property_groups.groups[0].groups[0], Property) - assert filter.property_groups.groups[0].groups[0].key == "attr" - assert filter.property_groups.groups[0].groups[0].value == "val_1" - assert isinstance(filter.property_groups.groups[0].groups[1], Property) - assert filter.property_groups.groups[0].groups[1].key == "attr_2" - assert filter.property_groups.groups[0].groups[1].value == "val_2" + assert isinstance(filter.property_groups.values[0], PropertyGroup) + assert filter.property_groups.values[0].type == "AND" + assert isinstance(filter.property_groups.values[0].values[0], Property) + assert filter.property_groups.values[0].values[0].key == "attr" + assert filter.property_groups.values[0].values[0].value == "val_1" + assert isinstance(filter.property_groups.values[0].values[1], Property) + assert filter.property_groups.values[0].values[1].key == "attr_2" + assert filter.property_groups.values[0].values[1].value == "val_2" - assert isinstance(filter.property_groups.groups[1], PropertyGroup) - assert filter.property_groups.groups[1].type == "OR" - assert isinstance(filter.property_groups.groups[1].groups[0], Property) - assert filter.property_groups.groups[1].groups[0].key == "attr" - assert filter.property_groups.groups[1].groups[0].value == "val_2" + assert isinstance(filter.property_groups.values[1], PropertyGroup) + assert filter.property_groups.values[1].type == "OR" + assert isinstance(filter.property_groups.values[1].values[0], Property) + assert filter.property_groups.values[1].values[0].key == "attr" + assert filter.property_groups.values[1].values[0].value == "val_2" def test_property_group_simple_parsing(): @@ -45,25 +45,25 @@ def test_property_group_simple_parsing(): data={ "properties": { "type": "AND", - "groups": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}], + "values": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}], } } ) assert filter.property_groups.type == "AND" - assert isinstance(filter.property_groups.groups[0], Property) - assert filter.property_groups.groups[0].key == "attr" - assert filter.property_groups.groups[0].value == "val_1" - assert isinstance(filter.property_groups.groups[1], Property) - assert filter.property_groups.groups[1].key == "attr_2" - assert filter.property_groups.groups[1].value == "val_2" + assert isinstance(filter.property_groups.values[0], Property) + assert filter.property_groups.values[0].key == "attr" + assert filter.property_groups.values[0].value == "val_1" + assert isinstance(filter.property_groups.values[1], Property) + assert filter.property_groups.values[1].key == "attr_2" + assert filter.property_groups.values[1].value == "val_2" def test_property_group_empty_parsing(): filter = Filter(data={"properties": {}}) assert filter.property_groups.type == "AND" - assert filter.property_groups.groups == [] + assert filter.property_groups.values == [] def test_property_group_invalid_parsing(): @@ -72,7 +72,7 @@ def test_property_group_invalid_parsing(): data={ "properties": { "type": "XaND", - "groups": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"},], + "values": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"},], } } ) @@ -87,11 +87,11 @@ def test_property_group_includes_unhomogenous_groups(): data={ "properties": { "type": "AND", - "groups": [ - {"type": "or", "groups": [{"key": "attr", "value": "val_1"}]}, + "values": [ + {"type": "or", "values": [{"key": "attr", "value": "val_1"}]}, {"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}, - {"type": "OR", "groups": []}, + {"type": "OR", "values": []}, ], } } @@ -106,12 +106,12 @@ def test_property_multi_level_to_dict(): data={ "properties": { "type": "AND", - "groups": [ + "values": [ { "type": "AND", - "groups": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}], + "values": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}], }, - {"type": "OR", "groups": [{"key": "attr", "value": "val_2"}]}, + {"type": "OR", "values": [{"key": "attr", "value": "val_2"}]}, ], } } @@ -119,15 +119,15 @@ def test_property_multi_level_to_dict(): assert filter.property_groups.to_dict() == { "type": "AND", - "groups": [ + "values": [ { "type": "AND", - "groups": [ + "values": [ {"key": "attr", "value": "val_1", "operator": None, "type": "event"}, {"key": "attr_2", "value": "val_2", "operator": None, "type": "event"}, ], }, - {"type": "OR", "groups": [{"key": "attr", "value": "val_2", "operator": None, "type": "event"}]}, + {"type": "OR", "values": [{"key": "attr", "value": "val_2", "operator": None, "type": "event"}]}, ], } @@ -137,14 +137,14 @@ def test_property_group_simple_to_dict(): data={ "properties": { "type": "AND", - "groups": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}], + "values": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}], } } ) assert filter.property_groups.to_dict() == { "type": "AND", - "groups": [ + "values": [ {"key": "attr", "value": "val_1", "operator": None, "type": "event"}, {"key": "attr_2", "value": "val_2", "operator": None, "type": "event"}, ], @@ -155,19 +155,19 @@ def test_property_group_simple_json_parsing(): filter = Filter( data={ "properties": json.dumps( - {"type": "AND", "groups": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],} + {"type": "AND", "values": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}],} ) } ) assert filter.property_groups.type == "AND" - assert isinstance(filter.property_groups.groups[0], Property) - assert filter.property_groups.groups[0].key == "attr" - assert filter.property_groups.groups[0].value == "val_1" - assert isinstance(filter.property_groups.groups[1], Property) - assert filter.property_groups.groups[1].key == "attr_2" - assert filter.property_groups.groups[1].value == "val_2" + assert isinstance(filter.property_groups.values[0], Property) + assert filter.property_groups.values[0].key == "attr" + assert filter.property_groups.values[0].value == "val_1" + assert isinstance(filter.property_groups.values[1], Property) + assert filter.property_groups.values[1].key == "attr_2" + assert filter.property_groups.values[1].value == "val_2" def test_property_group_multi_level_json_parsing(): @@ -176,12 +176,12 @@ def test_property_group_multi_level_json_parsing(): "properties": json.dumps( { "type": "AND", - "groups": [ + "values": [ { "type": "AND", - "groups": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}], + "values": [{"key": "attr", "value": "val_1"}, {"key": "attr_2", "value": "val_2"}], }, - {"type": "OR", "groups": [{"key": "attr", "value": "val_2"}]}, + {"type": "OR", "values": [{"key": "attr", "value": "val_2"}]}, ], } ) @@ -189,18 +189,18 @@ def test_property_group_multi_level_json_parsing(): ) assert filter.property_groups.type == "AND" - assert isinstance(filter.property_groups.groups[0], PropertyGroup) - assert filter.property_groups.groups[0].type == "AND" + assert isinstance(filter.property_groups.values[0], PropertyGroup) + assert filter.property_groups.values[0].type == "AND" - assert isinstance(filter.property_groups.groups[0].groups[0], Property) - assert filter.property_groups.groups[0].groups[0].key == "attr" - assert filter.property_groups.groups[0].groups[0].value == "val_1" - assert isinstance(filter.property_groups.groups[0].groups[1], Property) - assert filter.property_groups.groups[0].groups[1].key == "attr_2" - assert filter.property_groups.groups[0].groups[1].value == "val_2" + assert isinstance(filter.property_groups.values[0].values[0], Property) + assert filter.property_groups.values[0].values[0].key == "attr" + assert filter.property_groups.values[0].values[0].value == "val_1" + assert isinstance(filter.property_groups.values[0].values[1], Property) + assert filter.property_groups.values[0].values[1].key == "attr_2" + assert filter.property_groups.values[0].values[1].value == "val_2" - assert isinstance(filter.property_groups.groups[1], PropertyGroup) - assert filter.property_groups.groups[1].type == "OR" - assert isinstance(filter.property_groups.groups[1].groups[0], Property) - assert filter.property_groups.groups[1].groups[0].key == "attr" - assert filter.property_groups.groups[1].groups[0].value == "val_2" + assert isinstance(filter.property_groups.values[1], PropertyGroup) + assert filter.property_groups.values[1].type == "OR" + assert isinstance(filter.property_groups.values[1].values[0], Property) + assert filter.property_groups.values[1].values[0].key == "attr" + assert filter.property_groups.values[1].values[0].value == "val_2" diff --git a/posthog/models/property.py b/posthog/models/property.py index 360e573ecc4..7203d1b9ac6 100644 --- a/posthog/models/property.py +++ b/posthog/models/property.py @@ -146,28 +146,28 @@ def lookup_q(key: str, value: Any) -> Q: class PropertyGroup: type: PropertyOperatorType - groups: Union[List[Property], List["PropertyGroup"]] + values: Union[List[Property], List["PropertyGroup"]] - def __init__(self, type: PropertyOperatorType, groups: Union[List[Property], List["PropertyGroup"]]) -> None: + def __init__(self, type: PropertyOperatorType, values: Union[List[Property], List["PropertyGroup"]]) -> None: self.type = type - self.groups = groups + self.values = values def combine_properties(self, operator: PropertyOperatorType, properties: List[Property]) -> "PropertyGroup": if not properties: return self - if len(self.groups) == 0: + if len(self.values) == 0: return PropertyGroup(PropertyOperatorType.AND, properties) return PropertyGroup(operator, [self, PropertyGroup(PropertyOperatorType.AND, properties)]) def to_dict(self): result: Dict = {} - if not self.groups: + if not self.values: return result - return {"type": self.type, "groups": [prop.to_dict() for prop in self.groups]} + return {"type": self.type, "values": [prop.to_dict() for prop in self.values]} def __repr__(self): - params_repr = ", ".join(f"{repr(prop)}" for prop in self.groups) + params_repr = ", ".join(f"{repr(prop)}" for prop in self.values) return f"PropertyGroup(type={self.type}-{params_repr})"