Skip to content

Commit

Permalink
feat(prop-defs): add hidden option to property definitions (#29085)
Browse files Browse the repository at this point in the history
  • Loading branch information
raquelmsmith authored Mar 1, 2025
1 parent e6cdfed commit a936eb9
Show file tree
Hide file tree
Showing 19 changed files with 580 additions and 151 deletions.
33 changes: 26 additions & 7 deletions ee/api/ee_event_definition.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
from typing import cast

import posthoganalytics
from django.utils import timezone
from loginas.utils import is_impersonated_session
from rest_framework import serializers

from ee.models.event_definition import EnterpriseEventDefinition
from posthog.api.shared import UserBasicSerializer
from posthog.api.tagged_item import TaggedItemSerializerMixin
from posthog.event_usage import groups
from posthog.models import User
from posthog.models.activity_logging.activity_log import (
Detail,
dict_changes_between,
log_activity,
Detail,
)

from loginas.utils import is_impersonated_session
from typing import cast
import posthoganalytics
from posthog.event_usage import groups
from posthog.models import User


class EnterpriseEventDefinitionSerializer(TaggedItemSerializerMixin, serializers.ModelSerializer):
updated_by = UserBasicSerializer(read_only=True)
Expand Down Expand Up @@ -44,6 +44,7 @@ class Meta:
"verified",
"verified_at",
"verified_by",
"hidden",
# Action fields
"is_action",
"action_id",
Expand All @@ -69,9 +70,27 @@ class Meta:
"created_by",
]

def validate(self, data):
validated_data = super().validate(data)

if "hidden" in validated_data and "verified" in validated_data:
if validated_data["hidden"] and validated_data["verified"]:
raise serializers.ValidationError("An event cannot be both hidden and verified")

return validated_data

def update(self, event_definition: EnterpriseEventDefinition, validated_data):
validated_data["updated_by"] = self.context["request"].user

# If setting hidden=True, ensure verified becomes false
if validated_data.get("hidden", False):
validated_data["verified"] = False
validated_data["verified_by"] = None
validated_data["verified_at"] = None
# If setting verified=True, ensure hidden becomes false
elif validated_data.get("verified", False):
validated_data["hidden"] = False

if "verified" in validated_data:
if validated_data["verified"] and not event_definition.verified:
# Verify event only if previously unverified
Expand Down
35 changes: 31 additions & 4 deletions ee/api/ee_property_definition.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
from rest_framework import serializers
from django.utils import timezone
from loginas.utils import is_impersonated_session
from rest_framework import serializers

from ee.models.property_definition import EnterprisePropertyDefinition
from posthog.api.shared import UserBasicSerializer
from posthog.api.tagged_item import TaggedItemSerializerMixin
from posthog.models import PropertyDefinition
from posthog.models.activity_logging.activity_log import (
Detail,
dict_changes_between,
log_activity,
Detail,
)
from loginas.utils import is_impersonated_session


class EnterprisePropertyDefinitionSerializer(TaggedItemSerializerMixin, serializers.ModelSerializer):
Expand All @@ -31,6 +32,7 @@ class Meta:
"verified",
"verified_at",
"verified_by",
"hidden",
)
read_only_fields = [
"id",
Expand All @@ -41,7 +43,32 @@ class Meta:
"verified_by",
]

def update(self, property_definition: EnterprisePropertyDefinition, validated_data):
def validate(self, data):
validated_data = super().validate(data)

if "hidden" in validated_data and "verified" in validated_data:
if validated_data["hidden"] and validated_data["verified"]:
raise serializers.ValidationError("A property cannot be both hidden and verified")

# If setting hidden=True, ensure verified becomes false
if validated_data.get("hidden", False):
validated_data["verified"] = False
# If setting verified=True, ensure hidden becomes false
elif validated_data.get("verified", False):
validated_data["hidden"] = False

return validated_data

def update(self, property_definition: EnterprisePropertyDefinition, validated_data: dict):
# If setting hidden=True, ensure verified becomes false
if validated_data.get("hidden", False):
validated_data["verified"] = False
validated_data["verified_by"] = None
validated_data["verified_at"] = None
# If setting verified=True, ensure hidden becomes false
elif validated_data.get("verified", False):
validated_data["hidden"] = False

validated_data["updated_by"] = self.context["request"].user
if "property_type" in validated_data:
if validated_data["property_type"] == "Numeric":
Expand Down
51 changes: 46 additions & 5 deletions ee/api/test/test_event_definition.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
from datetime import datetime
from typing import cast, Optional, Any
from typing import Any, Optional, cast

import dateutil.parser
from django.utils import timezone
from freezegun import freeze_time
from rest_framework import status

from ee.models.event_definition import EnterpriseEventDefinition
from ee.models.license import License, LicenseManager
from posthog.api.test.test_event_definition import capture_event, EventData
from ee.models.license import AvailableFeature, License, LicenseManager
from posthog.api.test.test_event_definition import EventData, capture_event
from posthog.api.test.test_organization import create_organization
from posthog.api.test.test_team import create_team
from posthog.api.test.test_user import create_user
from posthog.models import Tag, ActivityLog, Team, User
from posthog.models import ActivityLog, Tag, Team, User
from posthog.models.event_definition import EventDefinition
from posthog.test.base import APIBaseTest
from posthog.api.test.test_organization import create_organization


@freeze_time("2020-01-02")
Expand Down Expand Up @@ -368,6 +368,47 @@ def test_event_definition_no_duplicate_tags(self):

self.assertListEqual(sorted(response.json()["tags"]), ["a", "b"])

def test_exclude_hidden_events(self):
super(LicenseManager, cast(LicenseManager, License.objects)).create(
plan="enterprise", valid_until=timezone.datetime(2500, 1, 19, 3, 14, 7)
)
# Create some events with hidden flag
EnterpriseEventDefinition.objects.create(team=self.demo_team, name="visible_event")
EnterpriseEventDefinition.objects.create(team=self.demo_team, name="hidden_event1", hidden=True)
EnterpriseEventDefinition.objects.create(team=self.demo_team, name="hidden_event2", hidden=True)

# Test without enterprise taxonomy - hidden events should still be shown even with exclude_hidden=true
self.organization.available_features = []
self.organization.save()

response = self.client.get(f"/api/projects/{self.demo_team.pk}/event_definitions/?exclude_hidden=true")
assert response.status_code == status.HTTP_200_OK
event_names = {p["name"] for p in response.json()["results"]}
assert "visible_event" in event_names
assert "hidden_event1" in event_names
assert "hidden_event2" in event_names

# Test with enterprise taxonomy enabled - hidden events should be excluded when exclude_hidden=true
self.demo_team.organization.available_product_features = [
{"key": AvailableFeature.INGESTION_TAXONOMY, "name": "ingestion-taxonomy"}
]
self.demo_team.organization.save()

response = self.client.get(f"/api/projects/{self.demo_team.pk}/event_definitions/?exclude_hidden=true")
assert response.status_code == status.HTTP_200_OK
event_names = {p["name"] for p in response.json()["results"]}
assert "visible_event" in event_names
assert "hidden_event1" not in event_names
assert "hidden_event2" not in event_names

# Test with exclude_hidden=false (should be same as not setting it)
response = self.client.get(f"/api/projects/{self.demo_team.pk}/event_definitions/?exclude_hidden=false")
assert response.status_code == status.HTTP_200_OK
event_names = {p["name"] for p in response.json()["results"]}
assert "visible_event" in event_names
assert "hidden_event1" in event_names
assert "hidden_event2" in event_names

def test_event_type_event(self):
super(LicenseManager, cast(LicenseManager, License.objects)).create(
plan="enterprise", valid_until=timezone.datetime(2500, 1, 19, 3, 14, 7)
Expand Down
122 changes: 117 additions & 5 deletions ee/api/test/test_property_definition.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
from typing import cast, Optional
from freezegun import freeze_time
from typing import Optional, cast

import pytest
from django.db.utils import IntegrityError
from django.utils import timezone
from freezegun import freeze_time
from rest_framework import status

from ee.models.license import License, LicenseManager
from ee.models.property_definition import EnterprisePropertyDefinition
from posthog.models import EventProperty, Tag, ActivityLog
from posthog.constants import AvailableFeature
from posthog.models import ActivityLog, EventProperty, Tag
from posthog.models.property_definition import PropertyDefinition
from posthog.test.base import APIBaseTest

Expand Down Expand Up @@ -378,6 +380,72 @@ def test_verify_then_unverify(self):
assert response.json()["verified_by"] is None
assert response.json()["verified_at"] is None

@freeze_time("2021-08-25T22:09:14.252Z")
def test_hidden_property_behavior(self):
super(LicenseManager, cast(LicenseManager, License.objects)).create(
plan="enterprise", valid_until=timezone.datetime(2500, 1, 19, 3, 14, 7)
)
property = EnterprisePropertyDefinition.objects.create(team=self.team, name="hidden test property")
response = self.client.get(f"/api/projects/@current/property_definitions/{property.id}")
self.assertEqual(response.status_code, status.HTTP_200_OK)

assert response.json()["hidden"] is False
assert response.json()["verified"] is False

# Hide the property
self.client.patch(
f"/api/projects/@current/property_definitions/{property.id}",
{"hidden": True},
)
response = self.client.get(f"/api/projects/@current/property_definitions/{property.id}")
self.assertEqual(response.status_code, status.HTTP_200_OK)

assert response.json()["hidden"] is True
assert response.json()["verified"] is False # Hiding should ensure verified is False

# Try to verify and hide a property at the same time (should fail)
response = self.client.patch(
f"/api/projects/@current/property_definitions/{property.id}",
{"verified": True, "hidden": True},
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn("cannot be both hidden and verified", response.json()["detail"].lower())

# Unhide the property
self.client.patch(
f"/api/projects/@current/property_definitions/{property.id}",
{"hidden": False},
)
response = self.client.get(f"/api/projects/@current/property_definitions/{property.id}")
self.assertEqual(response.status_code, status.HTTP_200_OK)

assert response.json()["hidden"] is False
assert response.json()["verified"] is False

@freeze_time("2021-08-25T22:09:14.252Z")
def test_marking_hidden_removes_verified_status(self):
super(LicenseManager, cast(LicenseManager, License.objects)).create(
plan="enterprise", valid_until=timezone.datetime(2500, 1, 19, 3, 14, 7)
)
property = EnterprisePropertyDefinition.objects.create(
team=self.team, name="verified test property", verified=True
)
response = self.client.get(f"/api/projects/@current/property_definitions/{property.id}")
self.assertEqual(response.status_code, status.HTTP_200_OK)

assert response.json()["hidden"] is False
assert response.json()["verified"] is True

response = self.client.patch(
f"/api/projects/@current/property_definitions/{property.id}",
{"hidden": True},
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
property.refresh_from_db()
assert property.hidden is True
assert property.verified is False

@freeze_time("2021-08-25T22:09:14.252Z")
def test_verify_then_verify_again_no_change(self):
super(LicenseManager, cast(LicenseManager, License.objects)).create(
plan="enterprise", valid_until=timezone.datetime(2500, 1, 19, 3, 14, 7)
Expand Down Expand Up @@ -445,7 +513,7 @@ def test_cannot_update_verified_meta_properties_directly(self):
assert response.json()["verified_by"] is None
assert response.json()["verified_at"] is None

def test_list_property_definitions(self):
def test_list_property_definitions_verified_ordering(self):
super(LicenseManager, cast(LicenseManager, License.objects)).create(
plan="enterprise", valid_until=timezone.datetime(2500, 1, 19, 3, 14, 7)
)
Expand Down Expand Up @@ -494,7 +562,6 @@ def test_list_property_definitions(self):
]

# We should prefer properties that have been seen on an event if that is available

EventProperty.objects.get_or_create(team=self.team, event="$pageview", property="3_when_verified")
EventProperty.objects.get_or_create(team=self.team, event="$pageview", property="4_when_verified")

Expand All @@ -508,3 +575,48 @@ def test_list_property_definitions(self):
("5_when_verified", False, False),
("6_when_verified", False, False),
]

def test_exclude_hidden_properties(self):
super(LicenseManager, cast(LicenseManager, License.objects)).create(
plan="enterprise", valid_until=timezone.datetime(2500, 1, 19, 3, 14, 7)
)
# Create some properties with hidden flag
EnterprisePropertyDefinition.objects.create(team=self.team, name="visible_property", property_type="String")
EnterprisePropertyDefinition.objects.create(
team=self.team, name="hidden_property1", property_type="String", hidden=True
)
EnterprisePropertyDefinition.objects.create(
team=self.team, name="hidden_property2", property_type="String", hidden=True
)

# Test without enterprise taxonomy - hidden properties should still be shown even with exclude_hidden=true
self.organization.available_features = []
self.organization.save()

response = self.client.get(f"/api/projects/{self.team.pk}/property_definitions/?exclude_hidden=true")
self.assertEqual(response.status_code, status.HTTP_200_OK)
property_names = {p["name"] for p in response.json()["results"]}
self.assertIn("visible_property", property_names)
self.assertIn("hidden_property1", property_names)
self.assertIn("hidden_property2", property_names)

# Test with enterprise taxonomy enabled - hidden properties should be excluded when exclude_hidden=true
self.team.organization.available_product_features = [
{"key": AvailableFeature.INGESTION_TAXONOMY, "name": "ingestion-taxonomy"}
]
self.team.organization.save()

response = self.client.get(f"/api/projects/{self.team.pk}/property_definitions/?exclude_hidden=true")
self.assertEqual(response.status_code, status.HTTP_200_OK)
property_names = {p["name"] for p in response.json()["results"]}
self.assertIn("visible_property", property_names)
self.assertNotIn("hidden_property1", property_names)
self.assertNotIn("hidden_property2", property_names)

# Test with exclude_hidden=false (should be same as not setting it)
response = self.client.get(f"/api/projects/{self.team.pk}/property_definitions/?exclude_hidden=false")
self.assertEqual(response.status_code, status.HTTP_200_OK)
property_names = {p["name"] for p in response.json()["results"]}
self.assertIn("visible_property", property_names)
self.assertIn("hidden_property1", property_names)
self.assertIn("hidden_property2", property_names)
22 changes: 22 additions & 0 deletions ee/migrations/0022_enterpriseeventdefinition_hidden_and_more.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Generated by Django 4.2.18 on 2025-02-27 19:54

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("ee", "0021_conversation_status"),
]

operations = [
migrations.AddField(
model_name="enterpriseeventdefinition",
name="hidden",
field=models.BooleanField(blank=True, default=False, null=True),
),
migrations.AddField(
model_name="enterprisepropertydefinition",
name="hidden",
field=models.BooleanField(blank=True, default=False, null=True),
),
]
2 changes: 1 addition & 1 deletion ee/migrations/max_migration.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0021_conversation_status
0022_enterpriseeventdefinition_hidden_and_more
1 change: 1 addition & 0 deletions ee/models/event_definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class EnterpriseEventDefinition(EventDefinition):
blank=True,
related_name="verifying_user",
)
hidden = models.BooleanField(null=True, blank=True, default=False)

# Deprecated in favour of app-wide tagging model. See EnterpriseTaggedItem
deprecated_tags: ArrayField = ArrayField(models.CharField(max_length=32), null=True, blank=True, default=list)
Expand Down
Loading

0 comments on commit a936eb9

Please sign in to comment.