From 57cb342b479a34694b7619ecf83c237e1ab023ea Mon Sep 17 00:00:00 2001 From: meikel Date: Fri, 31 Jan 2025 15:09:28 +0100 Subject: [PATCH 01/13] feat(cdp): dont allow modification of hog code for now --- ee/settings.py | 3 ++ posthog/api/hog_function.py | 6 ++- posthog/api/test/test_hog_function.py | 57 +++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/ee/settings.py b/ee/settings.py index 4c62e2e5cf62d..6493faeb8c951 100644 --- a/ee/settings.py +++ b/ee/settings.py @@ -75,3 +75,6 @@ LANGFUSE_HOST = get_from_env("LANGFUSE_HOST", "https://us.cloud.langfuse.com", type_cast=str) ANTHROPIC_API_KEY = get_from_env("ANTHROPIC_API_KEY", "") + +# Whether to allow modification of hog function code +HOG_TRANSFORMATIONS_ENABLED = False diff --git a/posthog/api/hog_function.py b/posthog/api/hog_function.py index 7c39ef8cde34b..8fd757e662976 100644 --- a/posthog/api/hog_function.py +++ b/posthog/api/hog_function.py @@ -37,7 +37,7 @@ ) from posthog.models.plugin import TranspilerError from posthog.plugins.plugin_server_api import create_hog_invocation_test - +from django.conf import settings logger = structlog.get_logger(__name__) @@ -154,6 +154,10 @@ def validate(self, attrs): has_addon = team.organization.is_feature_available(AvailableFeature.DATA_PIPELINES) instance = cast(Optional[HogFunction], self.context.get("instance", self.instance)) + # Add check for HOG_TRANSFORMATIONS_ENABLED + if "hog" in attrs and not settings.HOG_TRANSFORMATIONS_ENABLED: + raise serializers.ValidationError({"hog": "Modifying hog function code is currently disabled."}) + hog_type = attrs.get("type", instance.type if instance else "destination") is_create = self.context.get("view") and self.context["view"].action == "create" diff --git a/posthog/api/test/test_hog_function.py b/posthog/api/test/test_hog_function.py index b38d103ac84b2..12cbcd7c237bf 100644 --- a/posthog/api/test/test_hog_function.py +++ b/posthog/api/test/test_hog_function.py @@ -6,6 +6,7 @@ from freezegun import freeze_time from inline_snapshot import snapshot from rest_framework import status +from django.test.utils import override_settings from common.hogvm.python.operation import HOGQL_BYTECODE_VERSION from posthog.api.test.test_hog_function_templates import MOCK_NODE_TEMPLATES @@ -1264,6 +1265,62 @@ def create(payload): } ) + @override_settings(HOG_TRANSFORMATIONS_ENABLED=False) + def test_cannot_modify_hog_when_transformations_disabled(self): + # First create a function + response = self.client.post( + f"/api/projects/{self.team.id}/hog_functions/", + data={ + "type": "destination", + "name": "Test Function", + "description": "Test description", + "hog": "", + "inputs": {}, + }, + ) + assert response.status_code == status.HTTP_201_CREATED + function_id = response.json()["id"] + + # Try to modify the hog code + response = self.client.patch( + f"/api/projects/{self.team.id}/hog_functions/{function_id}/", + data={ + "hog": "return event", + }, + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == { + "type": "validation_error", + "code": "invalid_input", + "detail": {"hog": "Modifying hog function code is currently disabled."}, + } + + @override_settings(HOG_TRANSFORMATIONS_ENABLED=True) + def test_can_modify_hog_when_transformations_enabled(self): + # First create a function + response = self.client.post( + f"/api/projects/{self.team.id}/hog_functions/", + data={ + "type": "destination", + "name": "Test Function", + "description": "Test description", + "hog": "fetch(inputs.url);", + "inputs": {}, + }, + ) + assert response.status_code == status.HTTP_201_CREATED + function_id = response.json()["id"] + + # Try to modify the hog code + response = self.client.patch( + f"/api/projects/{self.team.id}/hog_functions/{function_id}/", + data={ + "hog": "return event", + }, + ) + assert response.status_code == status.HTTP_200_OK + assert response.json()["hog"] == "modified_code();" + def test_list_hog_functions_ordered_by_execution_order_and_created_at(self): # Create functions with different execution orders and creation times with freeze_time("2024-01-01T00:00:00Z"): From b508deae3d572c2f14349924489c92bcac5d3d25 Mon Sep 17 00:00:00 2001 From: meikel Date: Fri, 31 Jan 2025 15:13:39 +0100 Subject: [PATCH 02/13] destinations should be modifiable --- posthog/api/hog_function.py | 10 ++++++---- posthog/api/test/test_hog_function.py | 22 +++++++++++----------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/posthog/api/hog_function.py b/posthog/api/hog_function.py index 8fd757e662976..6491145c604df 100644 --- a/posthog/api/hog_function.py +++ b/posthog/api/hog_function.py @@ -154,11 +154,13 @@ def validate(self, attrs): has_addon = team.organization.is_feature_available(AvailableFeature.DATA_PIPELINES) instance = cast(Optional[HogFunction], self.context.get("instance", self.instance)) - # Add check for HOG_TRANSFORMATIONS_ENABLED - if "hog" in attrs and not settings.HOG_TRANSFORMATIONS_ENABLED: - raise serializers.ValidationError({"hog": "Modifying hog function code is currently disabled."}) - + # Get the type, either from attrs or from instance hog_type = attrs.get("type", instance.type if instance else "destination") + + # Only check HOG_TRANSFORMATIONS_ENABLED for transformation type + if "hog" in attrs and hog_type == "transformation" and not settings.HOG_TRANSFORMATIONS_ENABLED: + raise serializers.ValidationError({"hog": "Modifying transformation code is currently disabled."}) + is_create = self.context.get("view") and self.context["view"].action == "create" template_id = attrs.get("template_id", instance.template_id if instance else None) diff --git a/posthog/api/test/test_hog_function.py b/posthog/api/test/test_hog_function.py index 12cbcd7c237bf..cf59df24358e7 100644 --- a/posthog/api/test/test_hog_function.py +++ b/posthog/api/test/test_hog_function.py @@ -1266,12 +1266,12 @@ def create(payload): ) @override_settings(HOG_TRANSFORMATIONS_ENABLED=False) - def test_cannot_modify_hog_when_transformations_disabled(self): - # First create a function + def test_cannot_modify_transformation_code_when_disabled(self): + # First create a transformation response = self.client.post( f"/api/projects/{self.team.id}/hog_functions/", data={ - "type": "destination", + "type": "transformation", "name": "Test Function", "description": "Test description", "hog": "", @@ -1281,7 +1281,7 @@ def test_cannot_modify_hog_when_transformations_disabled(self): assert response.status_code == status.HTTP_201_CREATED function_id = response.json()["id"] - # Try to modify the hog code + # Try to modify the transformation code response = self.client.patch( f"/api/projects/{self.team.id}/hog_functions/{function_id}/", data={ @@ -1292,26 +1292,26 @@ def test_cannot_modify_hog_when_transformations_disabled(self): assert response.json() == { "type": "validation_error", "code": "invalid_input", - "detail": {"hog": "Modifying hog function code is currently disabled."}, + "detail": {"hog": "Modifying transformation code is currently disabled."}, } - @override_settings(HOG_TRANSFORMATIONS_ENABLED=True) - def test_can_modify_hog_when_transformations_enabled(self): - # First create a function + @override_settings(HOG_TRANSFORMATIONS_ENABLED=False) + def test_can_modify_destination_code_when_transformations_disabled(self): + # Create a destination function response = self.client.post( f"/api/projects/{self.team.id}/hog_functions/", data={ "type": "destination", "name": "Test Function", "description": "Test description", - "hog": "fetch(inputs.url);", + "hog": "", "inputs": {}, }, ) assert response.status_code == status.HTTP_201_CREATED function_id = response.json()["id"] - # Try to modify the hog code + # Should be able to modify destination code even when transformations are disabled response = self.client.patch( f"/api/projects/{self.team.id}/hog_functions/{function_id}/", data={ @@ -1319,7 +1319,7 @@ def test_can_modify_hog_when_transformations_enabled(self): }, ) assert response.status_code == status.HTTP_200_OK - assert response.json()["hog"] == "modified_code();" + assert response.json()["hog"] == "return event" def test_list_hog_functions_ordered_by_execution_order_and_created_at(self): # Create functions with different execution orders and creation times From dd9b87a0cd36a0a9c2ec7e916203694b3e596d24 Mon Sep 17 00:00:00 2001 From: meikel Date: Fri, 31 Jan 2025 15:28:27 +0100 Subject: [PATCH 03/13] update logic --- ee/settings.py | 3 --- posthog/api/hog_function.py | 14 ++++++---- posthog/api/test/test_hog_function.py | 39 ++++++++++----------------- posthog/settings/web.py | 3 +++ 4 files changed, 26 insertions(+), 33 deletions(-) diff --git a/ee/settings.py b/ee/settings.py index 6493faeb8c951..4c62e2e5cf62d 100644 --- a/ee/settings.py +++ b/ee/settings.py @@ -75,6 +75,3 @@ LANGFUSE_HOST = get_from_env("LANGFUSE_HOST", "https://us.cloud.langfuse.com", type_cast=str) ANTHROPIC_API_KEY = get_from_env("ANTHROPIC_API_KEY", "") - -# Whether to allow modification of hog function code -HOG_TRANSFORMATIONS_ENABLED = False diff --git a/posthog/api/hog_function.py b/posthog/api/hog_function.py index 6491145c604df..fc2df29005189 100644 --- a/posthog/api/hog_function.py +++ b/posthog/api/hog_function.py @@ -156,11 +156,6 @@ def validate(self, attrs): # Get the type, either from attrs or from instance hog_type = attrs.get("type", instance.type if instance else "destination") - - # Only check HOG_TRANSFORMATIONS_ENABLED for transformation type - if "hog" in attrs and hog_type == "transformation" and not settings.HOG_TRANSFORMATIONS_ENABLED: - raise serializers.ValidationError({"hog": "Modifying transformation code is currently disabled."}) - is_create = self.context.get("view") and self.context["view"].action == "create" template_id = attrs.get("template_id", instance.template_id if instance else None) @@ -169,6 +164,15 @@ def validate(self, attrs): if template_id and template_id.startswith("plugin-"): template = create_legacy_plugin_template(template_id) + # If transformations are disabled, preserve the original hog code for existing transformations + if ( + not is_create + and "hog" in attrs + and hog_type == "transformation" + and not settings.HOG_TRANSFORMATIONS_ENABLED + ): + attrs["hog"] = instance.hog # Preserve the original code + if not has_addon: # In this case they are only allowed to create or update the function with free templates if not template: diff --git a/posthog/api/test/test_hog_function.py b/posthog/api/test/test_hog_function.py index cf59df24358e7..cf9e6816232a5 100644 --- a/posthog/api/test/test_hog_function.py +++ b/posthog/api/test/test_hog_function.py @@ -1266,60 +1266,49 @@ def create(payload): ) @override_settings(HOG_TRANSFORMATIONS_ENABLED=False) - def test_cannot_modify_transformation_code_when_disabled(self): - # First create a transformation + def test_can_create_transformation_when_transformations_disabled(self): + # Should be able to create a new transformation response = self.client.post( f"/api/projects/{self.team.id}/hog_functions/", data={ "type": "transformation", "name": "Test Function", "description": "Test description", - "hog": "", + "hog": "return event", "inputs": {}, }, ) assert response.status_code == status.HTTP_201_CREATED - function_id = response.json()["id"] - - # Try to modify the transformation code - response = self.client.patch( - f"/api/projects/{self.team.id}/hog_functions/{function_id}/", - data={ - "hog": "return event", - }, - ) - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert response.json() == { - "type": "validation_error", - "code": "invalid_input", - "detail": {"hog": "Modifying transformation code is currently disabled."}, - } + assert response.json()["hog"] == "return event" @override_settings(HOG_TRANSFORMATIONS_ENABLED=False) - def test_can_modify_destination_code_when_transformations_disabled(self): - # Create a destination function + def test_preserves_original_transformation_code_when_disabled(self): + # First create a transformation response = self.client.post( f"/api/projects/{self.team.id}/hog_functions/", data={ - "type": "destination", + "type": "transformation", "name": "Test Function", "description": "Test description", - "hog": "", + "hog": "return event", "inputs": {}, }, ) assert response.status_code == status.HTTP_201_CREATED function_id = response.json()["id"] + original_code = response.json()["hog"] - # Should be able to modify destination code even when transformations are disabled + # Try to modify the transformation code response = self.client.patch( f"/api/projects/{self.team.id}/hog_functions/{function_id}/", data={ - "hog": "return event", + "hog": "return modified_event", + "name": "Updated Name", # Should allow other fields to be updated }, ) assert response.status_code == status.HTTP_200_OK - assert response.json()["hog"] == "return event" + assert response.json()["hog"] == original_code # Code should remain unchanged + assert response.json()["name"] == "Updated Name" # Other fields should update def test_list_hog_functions_ordered_by_execution_order_and_created_at(self): # Create functions with different execution orders and creation times diff --git a/posthog/settings/web.py b/posthog/settings/web.py index 3098a8f45f80a..e3f0d6428071f 100644 --- a/posthog/settings/web.py +++ b/posthog/settings/web.py @@ -416,3 +416,6 @@ def static_varies_origin(headers, path, url): REMOTE_CONFIG_CDN_PURGE_ENDPOINT = get_from_env("REMOTE_CONFIG_CDN_PURGE_ENDPOINT", "") REMOTE_CONFIG_CDN_PURGE_TOKEN = get_from_env("REMOTE_CONFIG_CDN_PURGE_TOKEN", "") REMOTE_CONFIG_CDN_PURGE_DOMAINS = get_list(os.getenv("REMOTE_CONFIG_CDN_PURGE_DOMAINS", "")) + +# Whether to allow modification of transformation code +HOG_TRANSFORMATIONS_ENABLED = get_from_env("HOG_TRANSFORMATIONS_ENABLED", False, type_cast=str_to_bool) From df447a6c26b8aa609285a31d1ad9326be0dc73e3 Mon Sep 17 00:00:00 2001 From: meikel Date: Fri, 31 Jan 2025 15:29:32 +0100 Subject: [PATCH 04/13] kill comment --- posthog/api/hog_function.py | 1 - 1 file changed, 1 deletion(-) diff --git a/posthog/api/hog_function.py b/posthog/api/hog_function.py index fc2df29005189..3309e107d9e80 100644 --- a/posthog/api/hog_function.py +++ b/posthog/api/hog_function.py @@ -154,7 +154,6 @@ def validate(self, attrs): has_addon = team.organization.is_feature_available(AvailableFeature.DATA_PIPELINES) instance = cast(Optional[HogFunction], self.context.get("instance", self.instance)) - # Get the type, either from attrs or from instance hog_type = attrs.get("type", instance.type if instance else "destination") is_create = self.context.get("view") and self.context["view"].action == "create" From c7eaf836cb14efd6ec52d11b80f5b081a160bfb9 Mon Sep 17 00:00:00 2001 From: meikel Date: Fri, 31 Jan 2025 15:42:50 +0100 Subject: [PATCH 05/13] add instance null check --- posthog/api/hog_function.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/posthog/api/hog_function.py b/posthog/api/hog_function.py index 3309e107d9e80..5ed31abc0ba7b 100644 --- a/posthog/api/hog_function.py +++ b/posthog/api/hog_function.py @@ -169,8 +169,9 @@ def validate(self, attrs): and "hog" in attrs and hog_type == "transformation" and not settings.HOG_TRANSFORMATIONS_ENABLED + and instance is not None ): - attrs["hog"] = instance.hog # Preserve the original code + attrs["hog"] = instance.hog # Now we know instance exists if not has_addon: # In this case they are only allowed to create or update the function with free templates From 505bbac646d70b13ec5a3d30556935764c3fc1dc Mon Sep 17 00:00:00 2001 From: meikel Date: Fri, 31 Jan 2025 15:57:41 +0100 Subject: [PATCH 06/13] update logic and tests to use templates --- posthog/api/hog_function.py | 13 +++----- posthog/api/test/test_hog_function.py | 47 ++++++++++++++------------- 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/posthog/api/hog_function.py b/posthog/api/hog_function.py index 5ed31abc0ba7b..4eec97c7034ff 100644 --- a/posthog/api/hog_function.py +++ b/posthog/api/hog_function.py @@ -163,15 +163,10 @@ def validate(self, attrs): if template_id and template_id.startswith("plugin-"): template = create_legacy_plugin_template(template_id) - # If transformations are disabled, preserve the original hog code for existing transformations - if ( - not is_create - and "hog" in attrs - and hog_type == "transformation" - and not settings.HOG_TRANSFORMATIONS_ENABLED - and instance is not None - ): - attrs["hog"] = instance.hog # Now we know instance exists + # If transformations are disabled, use template code + # currently we always use template code for transformations + if not settings.HOG_TRANSFORMATIONS_ENABLED and template and hog_type == "transformation": + attrs["hog"] = template.hog # Always use template code when transformations are disabled if not has_addon: # In this case they are only allowed to create or update the function with free templates diff --git a/posthog/api/test/test_hog_function.py b/posthog/api/test/test_hog_function.py index cf9e6816232a5..0befec0561380 100644 --- a/posthog/api/test/test_hog_function.py +++ b/posthog/api/test/test_hog_function.py @@ -1266,50 +1266,53 @@ def create(payload): ) @override_settings(HOG_TRANSFORMATIONS_ENABLED=False) - def test_can_create_transformation_when_transformations_disabled(self): - # Should be able to create a new transformation + def test_transformation_always_uses_template_code(self): + # Create a transformation with a template response = self.client.post( f"/api/projects/{self.team.id}/hog_functions/", data={ "type": "transformation", "name": "Test Function", "description": "Test description", - "hog": "return event", + "template_id": template_slack.id, # Using an existing template + "hog": "return modified_event", # This should be ignored "inputs": {}, }, ) assert response.status_code == status.HTTP_201_CREATED - assert response.json()["hog"] == "return event" + assert response.json()["hog"] == template_slack.hog # Should use template code - @override_settings(HOG_TRANSFORMATIONS_ENABLED=False) - def test_preserves_original_transformation_code_when_disabled(self): - # First create a transformation - response = self.client.post( - f"/api/projects/{self.team.id}/hog_functions/", - data={ - "type": "transformation", - "name": "Test Function", - "description": "Test description", - "hog": "return event", - "inputs": {}, - }, - ) - assert response.status_code == status.HTTP_201_CREATED function_id = response.json()["id"] - original_code = response.json()["hog"] # Try to modify the transformation code response = self.client.patch( f"/api/projects/{self.team.id}/hog_functions/{function_id}/", data={ - "hog": "return modified_event", - "name": "Updated Name", # Should allow other fields to be updated + "hog": "return another_event", # This should be ignored + "name": "Updated Name", # Other fields should update }, ) assert response.status_code == status.HTTP_200_OK - assert response.json()["hog"] == original_code # Code should remain unchanged + assert response.json()["hog"] == template_slack.hog # Should still use template code assert response.json()["name"] == "Updated Name" # Other fields should update + @override_settings(HOG_TRANSFORMATIONS_ENABLED=True) + def test_transformation_uses_template_code_even_when_enabled(self): + # Even with transformations enabled, we should still use template code + response = self.client.post( + f"/api/projects/{self.team.id}/hog_functions/", + data={ + "type": "transformation", + "name": "Test Function", + "description": "Test description", + "template_id": template_slack.id, + "hog": "return custom_event", # This should be ignored + "inputs": {}, + }, + ) + assert response.status_code == status.HTTP_201_CREATED + assert response.json()["hog"] == template_slack.hog # Should always use template code + def test_list_hog_functions_ordered_by_execution_order_and_created_at(self): # Create functions with different execution orders and creation times with freeze_time("2024-01-01T00:00:00Z"): From 211c0b1287dc7850b2f46ec94af39945bb04dde0 Mon Sep 17 00:00:00 2001 From: meikel Date: Fri, 31 Jan 2025 16:07:22 +0100 Subject: [PATCH 07/13] update --- posthog/api/hog_function.py | 15 ++++++++++----- posthog/api/test/test_hog_function.py | 18 ++++++++---------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/posthog/api/hog_function.py b/posthog/api/hog_function.py index 4eec97c7034ff..24c375e1d10f9 100644 --- a/posthog/api/hog_function.py +++ b/posthog/api/hog_function.py @@ -157,17 +157,22 @@ def validate(self, attrs): hog_type = attrs.get("type", instance.type if instance else "destination") is_create = self.context.get("view") and self.context["view"].action == "create" + # If transformations are disabled and this is an update to a transformation, preserve original hog code + if ( + not is_create + and not settings.HOG_TRANSFORMATIONS_ENABLED + and "hog" in attrs + and hog_type == "transformation" + and instance is not None + ): + attrs["hog"] = instance.hog # Keep the original code + template_id = attrs.get("template_id", instance.template_id if instance else None) template = HogFunctionTemplates.template(template_id) if template_id else None if template_id and template_id.startswith("plugin-"): template = create_legacy_plugin_template(template_id) - # If transformations are disabled, use template code - # currently we always use template code for transformations - if not settings.HOG_TRANSFORMATIONS_ENABLED and template and hog_type == "transformation": - attrs["hog"] = template.hog # Always use template code when transformations are disabled - if not has_addon: # In this case they are only allowed to create or update the function with free templates if not template: diff --git a/posthog/api/test/test_hog_function.py b/posthog/api/test/test_hog_function.py index 0befec0561380..4938485e716a0 100644 --- a/posthog/api/test/test_hog_function.py +++ b/posthog/api/test/test_hog_function.py @@ -1266,34 +1266,32 @@ def create(payload): ) @override_settings(HOG_TRANSFORMATIONS_ENABLED=False) - def test_transformation_always_uses_template_code(self): - # Create a transformation with a template + def test_cannot_modify_transformation_code_when_disabled(self): + # First create a transformation response = self.client.post( f"/api/projects/{self.team.id}/hog_functions/", data={ "type": "transformation", "name": "Test Function", "description": "Test description", - "template_id": template_slack.id, # Using an existing template - "hog": "return modified_event", # This should be ignored + "hog": "return original_event", "inputs": {}, }, ) assert response.status_code == status.HTTP_201_CREATED - assert response.json()["hog"] == template_slack.hog # Should use template code - - function_id = response.json()["id"] + original_code = response.json()["hog"] # Try to modify the transformation code + function_id = response.json()["id"] response = self.client.patch( f"/api/projects/{self.team.id}/hog_functions/{function_id}/", data={ - "hog": "return another_event", # This should be ignored - "name": "Updated Name", # Other fields should update + "hog": "return modified_event", # Should be ignored + "name": "Updated Name", # Should be allowed to change }, ) assert response.status_code == status.HTTP_200_OK - assert response.json()["hog"] == template_slack.hog # Should still use template code + assert response.json()["hog"] == original_code # Code should not change assert response.json()["name"] == "Updated Name" # Other fields should update @override_settings(HOG_TRANSFORMATIONS_ENABLED=True) From 4e17b5390bbef3c5dd508d27aca1526a28d532a6 Mon Sep 17 00:00:00 2001 From: meikel Date: Fri, 31 Jan 2025 16:51:10 +0100 Subject: [PATCH 08/13] adjust logic to pr --- posthog/api/hog_function.py | 10 +++ posthog/api/test/test_hog_function.py | 89 +++++++++++++++++++-------- 2 files changed, 73 insertions(+), 26 deletions(-) diff --git a/posthog/api/hog_function.py b/posthog/api/hog_function.py index 24c375e1d10f9..9de7bd4cdf67c 100644 --- a/posthog/api/hog_function.py +++ b/posthog/api/hog_function.py @@ -202,6 +202,16 @@ def validate(self, attrs): attrs["inputs_schema"] = attrs.get("inputs_schema") or template.inputs_schema attrs["inputs"] = attrs.get("inputs") or {} + if hog_type == "transformation": + if not settings.HOG_TRANSFORMATIONS_ENABLED: + if not template: + raise serializers.ValidationError( + {"template_id": "Transformation functions must be created from a template."} + ) + # Currently we do not allow modifying the core transformation templates when transformations are disabled + attrs["hog"] = template.hog + attrs["inputs_schema"] = template.inputs_schema + # Used for both top level input validation, and mappings input validation def validate_input_and_filters(attrs: dict): if "inputs_schema" in attrs: diff --git a/posthog/api/test/test_hog_function.py b/posthog/api/test/test_hog_function.py index 4938485e716a0..c3709b3147b31 100644 --- a/posthog/api/test/test_hog_function.py +++ b/posthog/api/test/test_hog_function.py @@ -16,7 +16,7 @@ from posthog.models.hog_functions.hog_function import DEFAULT_STATE, HogFunction from posthog.test.base import APIBaseTest, ClickhouseTestMixin, QueryMatchingTest from posthog.cdp.templates.webhook.template_webhook import template as template_webhook -from posthog.cdp.templates.slack.template_slack import template as template_slack +from posthog.cdp.templates.slack.template_slack import template as slack_template EXAMPLE_FULL = { @@ -84,7 +84,7 @@ def setUp(self): def _create_slack_function(self, data: Optional[dict] = None): payload = { "name": "Slack", - "template_id": template_slack.id, + "template_id": slack_template.id, "type": "destination", "inputs": { "slack_workspace": {"value": 1}, @@ -103,8 +103,8 @@ def test_create_hog_function_works_for_free_template(self): response = self._create_slack_function() assert response.status_code == status.HTTP_201_CREATED, response.json() assert response.json()["created_by"]["id"] == self.user.id - assert response.json()["hog"] == template_slack.hog - assert response.json()["inputs_schema"] == template_slack.inputs_schema + assert response.json()["hog"] == slack_template.hog + assert response.json()["inputs_schema"] == slack_template.inputs_schema def test_free_users_cannot_override_hog_or_schema(self): response = self._create_slack_function( @@ -117,8 +117,8 @@ def test_free_users_cannot_override_hog_or_schema(self): ) new_response = response.json() # These did not change - assert new_response["hog"] == template_slack.hog - assert new_response["inputs_schema"] == template_slack.inputs_schema + assert new_response["hog"] == slack_template.hog + assert new_response["inputs_schema"] == slack_template.inputs_schema def test_free_users_cannot_use_without_template(self): response = self._create_slack_function({"template_id": None}) @@ -1266,33 +1266,70 @@ def create(payload): ) @override_settings(HOG_TRANSFORMATIONS_ENABLED=False) - def test_cannot_modify_transformation_code_when_disabled(self): - # First create a transformation + def test_transformation_functions_require_template_when_disabled(self): response = self.client.post( f"/api/projects/{self.team.id}/hog_functions/", data={ + "name": "Custom Transform", "type": "transformation", - "name": "Test Function", - "description": "Test description", - "hog": "return original_event", - "inputs": {}, + "hog": "return event", }, ) - assert response.status_code == status.HTTP_201_CREATED - original_code = response.json()["hog"] - # Try to modify the transformation code - function_id = response.json()["id"] - response = self.client.patch( - f"/api/projects/{self.team.id}/hog_functions/{function_id}/", - data={ - "hog": "return modified_event", # Should be ignored - "name": "Updated Name", # Should be allowed to change - }, - ) - assert response.status_code == status.HTTP_200_OK - assert response.json()["hog"] == original_code # Code should not change - assert response.json()["name"] == "Updated Name" # Other fields should update + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == { + "type": "validation_error", + "code": "invalid_input", + "detail": "Transformation functions must be created from a template.", + "attr": "template_id", + } + + @override_settings(HOG_TRANSFORMATIONS_ENABLED=False) + def test_transformation_functions_preserve_template_code_when_disabled(self): + # Create a proper mock template class + class MockTemplate: + id = "test-template" + name = "Test Template" + type = "transformation" + description = "Test template description" + status = "free" + inputs_schema = [] + hog = "return event" + icon_url = None + category = None + filters = None + masking = None + mappings = None + mapping_templates = None + sub_templates = None + + with patch("posthog.api.hog_function_template.HogFunctionTemplates.template") as mock_template: + mock_template.return_value = MockTemplate() + + # First create with transformations enabled + with override_settings(HOG_TRANSFORMATIONS_ENABLED=True): + response = self.client.post( + f"/api/projects/{self.team.id}/hog_functions/", + data={ + "name": "Template Transform", + "type": "transformation", + "template_id": "test-template", + "hog": "return event", + }, + ) + assert response.status_code == status.HTTP_201_CREATED, response.json() + function_id = response.json()["id"] + + # Try to update with transformations disabled + response = self.client.patch( + f"/api/projects/{self.team.id}/hog_functions/{function_id}/", + data={ + "hog": "return modified_event", + }, + ) + + assert response.status_code == status.HTTP_200_OK + assert response.json()["hog"] == "return event" # Original code preserved @override_settings(HOG_TRANSFORMATIONS_ENABLED=True) def test_transformation_uses_template_code_even_when_enabled(self): From b5bff5b0a2195e828ebe17782b96dcf7ea98c185 Mon Sep 17 00:00:00 2001 From: meikel Date: Fri, 31 Jan 2025 16:58:26 +0100 Subject: [PATCH 09/13] push --- posthog/api/hog_function.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/posthog/api/hog_function.py b/posthog/api/hog_function.py index 9de7bd4cdf67c..8a0b3830634e1 100644 --- a/posthog/api/hog_function.py +++ b/posthog/api/hog_function.py @@ -157,16 +157,6 @@ def validate(self, attrs): hog_type = attrs.get("type", instance.type if instance else "destination") is_create = self.context.get("view") and self.context["view"].action == "create" - # If transformations are disabled and this is an update to a transformation, preserve original hog code - if ( - not is_create - and not settings.HOG_TRANSFORMATIONS_ENABLED - and "hog" in attrs - and hog_type == "transformation" - and instance is not None - ): - attrs["hog"] = instance.hog # Keep the original code - template_id = attrs.get("template_id", instance.template_id if instance else None) template = HogFunctionTemplates.template(template_id) if template_id else None From f2fd0c6c0ab90f3a4f4c2378de08cdb99113cd4b Mon Sep 17 00:00:00 2001 From: meikel Date: Fri, 31 Jan 2025 17:00:00 +0100 Subject: [PATCH 10/13] rename back --- posthog/api/test/test_hog_function.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/posthog/api/test/test_hog_function.py b/posthog/api/test/test_hog_function.py index c3709b3147b31..9a7774ca2a056 100644 --- a/posthog/api/test/test_hog_function.py +++ b/posthog/api/test/test_hog_function.py @@ -16,7 +16,7 @@ from posthog.models.hog_functions.hog_function import DEFAULT_STATE, HogFunction from posthog.test.base import APIBaseTest, ClickhouseTestMixin, QueryMatchingTest from posthog.cdp.templates.webhook.template_webhook import template as template_webhook -from posthog.cdp.templates.slack.template_slack import template as slack_template +from posthog.cdp.templates.slack.template_slack import template as template_slack EXAMPLE_FULL = { @@ -84,7 +84,7 @@ def setUp(self): def _create_slack_function(self, data: Optional[dict] = None): payload = { "name": "Slack", - "template_id": slack_template.id, + "template_id": template_slack.id, "type": "destination", "inputs": { "slack_workspace": {"value": 1}, @@ -103,8 +103,8 @@ def test_create_hog_function_works_for_free_template(self): response = self._create_slack_function() assert response.status_code == status.HTTP_201_CREATED, response.json() assert response.json()["created_by"]["id"] == self.user.id - assert response.json()["hog"] == slack_template.hog - assert response.json()["inputs_schema"] == slack_template.inputs_schema + assert response.json()["hog"] == template_slack.hog + assert response.json()["inputs_schema"] == template_slack.inputs_schema def test_free_users_cannot_override_hog_or_schema(self): response = self._create_slack_function( @@ -117,8 +117,8 @@ def test_free_users_cannot_override_hog_or_schema(self): ) new_response = response.json() # These did not change - assert new_response["hog"] == slack_template.hog - assert new_response["inputs_schema"] == slack_template.inputs_schema + assert new_response["hog"] == template_slack.hog + assert new_response["inputs_schema"] == template_slack.inputs_schema def test_free_users_cannot_use_without_template(self): response = self._create_slack_function({"template_id": None}) From ec12e20176d3028144455401be88d5f2b2070ebe Mon Sep 17 00:00:00 2001 From: meikel Date: Fri, 31 Jan 2025 17:07:49 +0100 Subject: [PATCH 11/13] update tests --- posthog/api/test/test_hog_function.py | 31 ++++++++------------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/posthog/api/test/test_hog_function.py b/posthog/api/test/test_hog_function.py index 9a7774ca2a056..9dadbf42ab1ca 100644 --- a/posthog/api/test/test_hog_function.py +++ b/posthog/api/test/test_hog_function.py @@ -1286,25 +1286,8 @@ def test_transformation_functions_require_template_when_disabled(self): @override_settings(HOG_TRANSFORMATIONS_ENABLED=False) def test_transformation_functions_preserve_template_code_when_disabled(self): - # Create a proper mock template class - class MockTemplate: - id = "test-template" - name = "Test Template" - type = "transformation" - description = "Test template description" - status = "free" - inputs_schema = [] - hog = "return event" - icon_url = None - category = None - filters = None - masking = None - mappings = None - mapping_templates = None - sub_templates = None - with patch("posthog.api.hog_function_template.HogFunctionTemplates.template") as mock_template: - mock_template.return_value = MockTemplate() + mock_template.return_value = template_slack # Use existing template instead of creating mock # First create with transformations enabled with override_settings(HOG_TRANSFORMATIONS_ENABLED=True): @@ -1313,8 +1296,12 @@ class MockTemplate: data={ "name": "Template Transform", "type": "transformation", - "template_id": "test-template", - "hog": "return event", + "template_id": template_slack.id, + "hog": "return modified_event", + "inputs": { + "slack_workspace": {"value": 1}, + "channel": {"value": "#general"}, + }, }, ) assert response.status_code == status.HTTP_201_CREATED, response.json() @@ -1324,12 +1311,12 @@ class MockTemplate: response = self.client.patch( f"/api/projects/{self.team.id}/hog_functions/{function_id}/", data={ - "hog": "return modified_event", + "hog": "return another_event", }, ) assert response.status_code == status.HTTP_200_OK - assert response.json()["hog"] == "return event" # Original code preserved + assert response.json()["hog"] == template_slack.hog # Original template code preserved @override_settings(HOG_TRANSFORMATIONS_ENABLED=True) def test_transformation_uses_template_code_even_when_enabled(self): From 00b1597aea473cd4f3b4d54a015afbc664a19fc6 Mon Sep 17 00:00:00 2001 From: meikel Date: Fri, 31 Jan 2025 17:38:34 +0100 Subject: [PATCH 12/13] update --- posthog/api/hog_function.py | 2 +- posthog/api/test/test_hog_function.py | 6 +++--- posthog/settings/web.py | 4 +++- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/posthog/api/hog_function.py b/posthog/api/hog_function.py index 8a0b3830634e1..4870b5372ff12 100644 --- a/posthog/api/hog_function.py +++ b/posthog/api/hog_function.py @@ -193,7 +193,7 @@ def validate(self, attrs): attrs["inputs"] = attrs.get("inputs") or {} if hog_type == "transformation": - if not settings.HOG_TRANSFORMATIONS_ENABLED: + if not settings.HOG_TRANSFORMATIONS_CUSTOM_HOG_ENABLED: if not template: raise serializers.ValidationError( {"template_id": "Transformation functions must be created from a template."} diff --git a/posthog/api/test/test_hog_function.py b/posthog/api/test/test_hog_function.py index 9dadbf42ab1ca..82fbaea2bcd42 100644 --- a/posthog/api/test/test_hog_function.py +++ b/posthog/api/test/test_hog_function.py @@ -1265,7 +1265,7 @@ def create(payload): } ) - @override_settings(HOG_TRANSFORMATIONS_ENABLED=False) + @override_settings(HOG_TRANSFORMATIONS_CUSTOM_HOG_ENABLED=False) def test_transformation_functions_require_template_when_disabled(self): response = self.client.post( f"/api/projects/{self.team.id}/hog_functions/", @@ -1284,13 +1284,13 @@ def test_transformation_functions_require_template_when_disabled(self): "attr": "template_id", } - @override_settings(HOG_TRANSFORMATIONS_ENABLED=False) + @override_settings(HOG_TRANSFORMATIONS_CUSTOM_HOG_ENABLED=False) def test_transformation_functions_preserve_template_code_when_disabled(self): with patch("posthog.api.hog_function_template.HogFunctionTemplates.template") as mock_template: mock_template.return_value = template_slack # Use existing template instead of creating mock # First create with transformations enabled - with override_settings(HOG_TRANSFORMATIONS_ENABLED=True): + with override_settings(HOG_TRANSFORMATIONS_CUSTOM_HOG_ENABLED=True): response = self.client.post( f"/api/projects/{self.team.id}/hog_functions/", data={ diff --git a/posthog/settings/web.py b/posthog/settings/web.py index e3f0d6428071f..3a0be90a763fa 100644 --- a/posthog/settings/web.py +++ b/posthog/settings/web.py @@ -418,4 +418,6 @@ def static_varies_origin(headers, path, url): REMOTE_CONFIG_CDN_PURGE_DOMAINS = get_list(os.getenv("REMOTE_CONFIG_CDN_PURGE_DOMAINS", "")) # Whether to allow modification of transformation code -HOG_TRANSFORMATIONS_ENABLED = get_from_env("HOG_TRANSFORMATIONS_ENABLED", False, type_cast=str_to_bool) +HOG_TRANSFORMATIONS_CUSTOM_HOG_ENABLED = get_from_env( + "HOG_TRANSFORMATIONS_CUSTOM_HOG_ENABLED", False, type_cast=str_to_bool +) From 119f3148fde97eff7bd56b3011670766e2e63b72 Mon Sep 17 00:00:00 2001 From: meikel Date: Fri, 31 Jan 2025 18:59:16 +0100 Subject: [PATCH 13/13] fix tests --- posthog/api/test/test_hog_function.py | 90 ++++++++++++++++----------- 1 file changed, 52 insertions(+), 38 deletions(-) diff --git a/posthog/api/test/test_hog_function.py b/posthog/api/test/test_hog_function.py index 82fbaea2bcd42..fc7ba073c9438 100644 --- a/posthog/api/test/test_hog_function.py +++ b/posthog/api/test/test_hog_function.py @@ -1329,12 +1329,63 @@ def test_transformation_uses_template_code_even_when_enabled(self): "description": "Test description", "template_id": template_slack.id, "hog": "return custom_event", # This should be ignored - "inputs": {}, + "inputs": { + "slack_workspace": {"value": 1}, + "channel": {"value": "#general"}, + }, }, ) assert response.status_code == status.HTTP_201_CREATED assert response.json()["hog"] == template_slack.hog # Should always use template code + def test_transformation_type_gets_execution_order_automatically(self): + with patch("posthog.api.hog_function_template.HogFunctionTemplates.template") as mock_template: + mock_template.return_value = template_slack + + # Create first transformation function + response1 = self.client.post( + f"/api/projects/{self.team.id}/hog_functions/", + data={ + "type": "transformation", + "name": "First Transformation", + "template_id": template_slack.id, + "inputs": { + "slack_workspace": {"value": 1}, + "channel": {"value": "#general"}, + }, + }, + ) + assert response1.status_code == status.HTTP_201_CREATED + assert response1.json()["execution_order"] == 1 + + # Create second transformation function + response2 = self.client.post( + f"/api/projects/{self.team.id}/hog_functions/", + data={ + "type": "transformation", + "name": "Second Transformation", + "template_id": template_slack.id, + "inputs": { + "slack_workspace": {"value": 1}, + "channel": {"value": "#general"}, + }, + }, + ) + assert response2.status_code == status.HTTP_201_CREATED + assert response2.json()["execution_order"] == 2 + + # Create a non-transformation function - should not get execution_order + response3 = self.client.post( + f"/api/projects/{self.team.id}/hog_functions/", + data={ + **EXAMPLE_FULL, # This is fine for destination type + "type": "destination", + "name": "Destination Function", + }, + ) + assert response3.status_code == status.HTTP_201_CREATED + assert response3.json()["execution_order"] is None + def test_list_hog_functions_ordered_by_execution_order_and_created_at(self): # Create functions with different execution orders and creation times with freeze_time("2024-01-01T00:00:00Z"): @@ -1389,40 +1440,3 @@ def test_list_hog_functions_ordered_by_execution_order_and_created_at(self): "Function 3", # execution_order=2 "Function 4", # execution_order=null ] - - def test_transformation_type_gets_execution_order_automatically(self): - # Create first transformation function - response1 = self.client.post( - f"/api/projects/{self.team.id}/hog_functions/", - data={ - **EXAMPLE_FULL, - "type": "transformation", - "name": "First Transformation", - }, - ) - assert response1.status_code == status.HTTP_201_CREATED - assert response1.json()["execution_order"] == 1 - - # Create second transformation function - response2 = self.client.post( - f"/api/projects/{self.team.id}/hog_functions/", - data={ - **EXAMPLE_FULL, - "type": "transformation", - "name": "Second Transformation", - }, - ) - assert response2.status_code == status.HTTP_201_CREATED - assert response2.json()["execution_order"] == 2 - - # Create a non-transformation function - should not get execution_order - response3 = self.client.post( - f"/api/projects/{self.team.id}/hog_functions/", - data={ - **EXAMPLE_FULL, - "type": "destination", - "name": "Destination Function", - }, - ) - assert response3.status_code == status.HTTP_201_CREATED - assert response3.json()["execution_order"] is None