From e0cd051d151384c2220514be4ba6edf779b815f0 Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Fri, 26 Jan 2024 12:11:51 +0000 Subject: [PATCH 1/3] fx(web-analytics): Create group property filters correctly --- posthog/hogql/property.py | 2 ++ posthog/models/property/property.py | 12 ++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/posthog/hogql/property.py b/posthog/hogql/property.py index ce4ea3bdfe14f..cd57b43d8f70d 100644 --- a/posthog/hogql/property.py +++ b/posthog/hogql/property.py @@ -151,6 +151,7 @@ def property_to_expr( type=property.type, key=property.key, operator=property.operator, + group_type_index=property.group_type_index, value=v, ), team, @@ -263,6 +264,7 @@ def property_to_expr( type=property.type, key=property.key, operator=property.operator, + group_type_index=property.group_type_index, value=v, ), team, diff --git a/posthog/models/property/property.py b/posthog/models/property/property.py index 2b7a8a558e186..0db92dcf7fc17 100644 --- a/posthog/models/property/property.py +++ b/posthog/models/property/property.py @@ -253,14 +253,14 @@ def __init__( if self.type not in VALIDATE_PROP_TYPES.keys(): raise ValueError(f"Invalid property type: {self.type}") - for key in VALIDATE_PROP_TYPES[self.type]: - if getattr(self, key, None) is None: - raise ValueError(f"Missing required key {key} for property type {self.type} with name {self.key}") + for attr in VALIDATE_PROP_TYPES[self.type]: + if getattr(self, attr, None) is None: + raise ValueError(f"Missing required attr {attr} for property type {self.type} with key {self.key}") if self.type == "behavioral": - for key in VALIDATE_BEHAVIORAL_PROP_TYPES[cast(BehavioralPropertyType, self.value)]: - if getattr(self, key, None) is None: - raise ValueError(f"Missing required key {key} for property type {self.type}::{self.value}") + for attr in VALIDATE_BEHAVIORAL_PROP_TYPES[cast(BehavioralPropertyType, self.value)]: + if getattr(self, attr, None) is None: + raise ValueError(f"Missing required attr {attr} for property type {self.type}::{self.value}") def __repr__(self): params_repr = ", ".join(f"{key}={repr(value)}" for key, value in self.to_dict().items()) From 43c77e48f4194b7e654d4f0c72ab1a3aeeed9a13 Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Fri, 26 Jan 2024 12:41:27 +0000 Subject: [PATCH 2/3] Fix error message in test --- posthog/hogql/test/test_property.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog/hogql/test/test_property.py b/posthog/hogql/test/test_property.py index 3b2e582149cc6..b927da4cf3796 100644 --- a/posthog/hogql/test/test_property.py +++ b/posthog/hogql/test/test_property.py @@ -87,7 +87,7 @@ def test_property_to_expr_group(self): self._property_to_expr({"type": "group", "key": "a", "value": "b"}) self.assertEqual( str(e.exception), - "Missing required key group_type_index for property type group with name a", + "Missing required attr group_type_index for property type group with key a", ) def test_property_to_expr_event(self): From eb48e5790e975750e72c65c48a306d17f2702cf0 Mon Sep 17 00:00:00 2001 From: Robbie Coomber Date: Fri, 26 Jan 2024 12:58:42 +0000 Subject: [PATCH 3/3] Add a test which failed but is fixed by this PR --- posthog/hogql/test/test_property.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/posthog/hogql/test/test_property.py b/posthog/hogql/test/test_property.py index b927da4cf3796..9e82e06dc5a2a 100644 --- a/posthog/hogql/test/test_property.py +++ b/posthog/hogql/test/test_property.py @@ -82,6 +82,10 @@ def test_property_to_expr_group(self): {"type": "group", "group_type_index": 0, "key": "a", "value": "b", "operator": "is_not_set"} ), ) + self.assertEqual( + self._property_to_expr(Property(type="group", group_type_index=0, key="a", value=["b", "c"])), + self._parse_expr("group_0.properties.a = 'b' OR group_0.properties.a = 'c'"), + ) with self.assertRaises(Exception) as e: self._property_to_expr({"type": "group", "key": "a", "value": "b"})