Skip to content

Commit

Permalink
Merge branch 'master' into haacked/fix-remote-config-instructions
Browse files Browse the repository at this point in the history
  • Loading branch information
haacked authored Feb 14, 2025
2 parents 40c9b3c + ebbdb8a commit 4325426
Show file tree
Hide file tree
Showing 145 changed files with 3,945 additions and 1,384 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/storybook-chromatic.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ on:
- 'package.json'
- '.github/workflows/storybook-chromatic.yml'
- 'playwright.config.ts'
- 'playwright/e2e/**'
- 'playwright/utils/**'

concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,5 @@ venv
yalc.lock
# Flox.dev lockfile (will differ slightly per local dev env, even with consistent pinning in TOML file)
.flox/env/manifest.lock

playwright/.auth/
2 changes: 2 additions & 0 deletions CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ posthog/tasks/update_survey_adaptive_sampling @PostHog/team-surveys
posthog/tasks/update_survey_iteration @PostHog/team-surveys
posthog/tasks/test/test_stop_surveys_reached_target.py @PostHog/team-surveys
plugin-server/src/ @PostHog/team-cdp
posthog/clickhouse/migrations/** @PostHog/clickhouse
posthog/clickhouse/migrations2/** @PostHog/clickhouse
54 changes: 47 additions & 7 deletions ee/api/hooks.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
from typing import cast
from urllib.parse import urlparse

from django.http import Http404
from rest_framework import exceptions, serializers, mixins, viewsets, status
from rest_framework.response import Response
from django.core.exceptions import ValidationError

from ee.models.hook import Hook, HOOK_EVENTS
from django.conf import settings
Expand All @@ -19,12 +21,17 @@ def hog_functions_enabled(team: Team) -> bool:
return "*" in enabled_teams or str(team.id) in enabled_teams


def create_zapier_hog_function(hook: Hook, serializer_context: dict) -> HogFunction:
def create_zapier_hog_function(hook: Hook, serializer_context: dict, from_migration: bool = False) -> HogFunction:
description = template_zapier.description
if from_migration:
description = f"{description} Migrated from legacy hook {hook.id}."

serializer = HogFunctionSerializer(
data={
"template_id": template_zapier.id,
"type": "destination",
"name": f"Zapier webhook for action {hook.resource_id}",
"description": description,
"filters": {"actions": [{"id": str(hook.resource_id), "name": "", "type": "actions", "order": 0}]},
"inputs": {
"hook": {
Expand Down Expand Up @@ -125,12 +132,45 @@ def perform_create(self, serializer):
serializer.save(user=user, team_id=self.team_id)

def destroy(self, request, *args, **kwargs):
if not hog_functions_enabled(self.team):
return super().destroy(request, *args, **kwargs)

HogFunction.objects.filter(team_id=self.team_id, id=kwargs["pk"]).delete()

return Response(status=status.HTTP_204_NO_CONTENT)
found = False
try:
instance = self.get_object()
found = True

# We do this by finding one where the description contains the hook id
fns = HogFunction.objects.filter(
team_id=self.team_id,
template_id=template_zapier.id,
description__icontains=f"{instance.id}",
)

for fn in fns:
fn.enabled = False
fn.deleted = True
fn.save()

self.perform_destroy(instance)

except (Hook.DoesNotExist, Http404):
pass

if not found:
# Otherwise we try and delete the hog function by id
try:
hog_function = HogFunction.objects.get(
team_id=self.team_id, template_id=template_zapier.id, id=kwargs["pk"]
)
hog_function.enabled = False
hog_function.deleted = True
hog_function.save()
found = True
except (HogFunction.DoesNotExist, ValidationError):
pass

if found:
return Response(status=status.HTTP_204_NO_CONTENT)
else:
return Response(status=status.HTTP_404_NOT_FOUND)


def valid_domain(url) -> bool:
Expand Down
45 changes: 39 additions & 6 deletions ee/api/test/test_hooks.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
from typing import cast
import uuid

from inline_snapshot import snapshot

from ee.api.hooks import valid_domain
from ee.api.hooks import valid_domain, create_zapier_hog_function
from ee.api.test.base import APILicensedTest
from ee.models.hook import Hook
from common.hogvm.python.operation import HOGQL_BYTECODE_VERSION
from posthog.models.action.action import Action
from posthog.models.hog_functions.hog_function import HogFunction
from posthog.test.base import ClickhouseTestMixin
from posthog.cdp.templates.zapier.template_zapier import template as template_zapier


class TestHooksAPI(ClickhouseTestMixin, APILicensedTest):
Expand Down Expand Up @@ -113,6 +115,8 @@ def test_create_hog_function_via_hook(self):
"resource_id": self.action.id,
}

assert hog_function.description == template_zapier.description

assert hog_function.filters == {
"actions": [{"id": str(self.action.id), "name": "", "type": "actions", "order": 0}],
"bytecode": ["_H", HOGQL_BYTECODE_VERSION, 32, "$pageview", 32, "event", 1, 1, 11, 3, 1, 4, 1],
Expand Down Expand Up @@ -206,13 +210,42 @@ def test_delete_hog_function_via_hook(self):

hook_id = res.json()["id"]

assert HogFunction.objects.count() == 1
assert HogFunction.objects.filter(enabled=True, deleted=False).count() == 1

with self.settings(HOOK_HOG_FUNCTION_TEAMS="*"):
res = self.client.delete(f"/api/projects/{self.team.id}/hooks/{hook_id}")
assert res.status_code == 204
res = self.client.delete(f"/api/projects/{self.team.id}/hooks/{hook_id}")
assert res.status_code == 204

assert HogFunction.objects.filter(enabled=True, deleted=False).count() == 0

def test_delete_migrated_hog_function_via_hook(self):
hooks = []
hog_functions = []
for hook_id in [uuid.uuid4(), uuid.uuid4()]:
hook = Hook.objects.create(
id=hook_id,
user=self.user,
team=self.team,
resource_id=20,
target=f"https://hooks.zapier.com/hooks/standard/{hook_id}",
)

hog_function = create_zapier_hog_function(
hook, {"user": hook.user, "get_team": lambda hook=hook: hook.team}, from_migration=True
)
hog_function.save()
hooks.append(hook)
hog_functions.append(hog_function)

res = self.client.delete(f"/api/projects/{self.team.id}/hooks/{hooks[0].id}")
assert res.status_code == 204

assert not HogFunction.objects.exists()
# Ensure the right hook and hog function were deleted
loaded_hooks = Hook.objects.all()
assert len(loaded_hooks) == 1
assert str(loaded_hooks[0].id) == str(hooks[1].id)
loaded_hog_functions = HogFunction.objects.filter(enabled=True, deleted=False)
assert len(loaded_hog_functions) == 1
assert str(loaded_hog_functions[0].id) == str(hog_functions[1].id)


def test_valid_domain() -> None:
Expand Down
24 changes: 24 additions & 0 deletions ee/clickhouse/views/experiments.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,30 @@ class Meta:
"saved_metrics",
]

def to_representation(self, instance):
data = super().to_representation(instance)
# Normalize query date ranges to the experiment's current range
# Cribbed from ExperimentTrendsQuery
new_date_range = {
"date_from": data["start_date"] if data["start_date"] else "",
"date_to": data["end_date"] if data["end_date"] else "",
"explicitDate": True,
}
for metrics_list in [data.get("metrics", []), data.get("metrics_secondary", [])]:
for metric in metrics_list:
if metric.get("count_query", {}).get("dateRange"):
metric["count_query"]["dateRange"] = new_date_range
if metric.get("funnels_query", {}).get("dateRange"):
metric["funnels_query"]["dateRange"] = new_date_range

for saved_metric in data.get("saved_metrics", []):
if saved_metric.get("query", {}).get("count_query", {}).get("dateRange"):
saved_metric["query"]["count_query"]["dateRange"] = new_date_range
if saved_metric.get("query", {}).get("funnels_query", {}).get("dateRange"):
saved_metric["query"]["funnels_query"]["dateRange"] = new_date_range

return data

def validate_saved_metrics_ids(self, value):
if value is None:
return value
Expand Down
121 changes: 120 additions & 1 deletion ee/clickhouse/views/test/test_clickhouse_experiments.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
from datetime import datetime, timedelta, UTC
from django.core.cache import cache
from flaky import flaky
from freezegun import freeze_time
from rest_framework import status

from ee.api.test.base import APILicensedTest
from dateutil import parser

from ee.clickhouse.views.experiment_saved_metrics import ExperimentToSavedMetricSerializer
from posthog.models import WebExperiment
from posthog.models.action.action import Action
from posthog.models.cohort.cohort import Cohort
from posthog.models.experiment import Experiment
from posthog.models.experiment import Experiment, ExperimentSavedMetric
from posthog.models.feature_flag import FeatureFlag, get_feature_flags_for_team_in_cache
from posthog.schema import ExperimentSignificanceCode
from posthog.test.base import (
Expand Down Expand Up @@ -635,6 +637,123 @@ def test_validate_saved_metrics_payload(self):
self.assertEqual(response.json()["type"], "validation_error")
self.assertEqual(response.json()["detail"], "Metadata must be an object")

@freeze_time("2025-02-10T13:00:00Z")
def test_fetching_experiment_with_stale_metric_dates_applies_experiment_date_range(self):
test_feature_flag = FeatureFlag.objects.create(
name=f"Test experiment flag",
key="test-flag",
team=self.team,
filters={
"groups": [{"properties": [], "rollout_percentage": None}],
"multivariate": {
"variants": [
{
"key": "control",
"name": "Control",
"rollout_percentage": 50,
},
{
"key": "test",
"name": "Test",
"rollout_percentage": 50,
},
]
},
},
created_by=self.user,
)
funnel_query = {
"kind": "ExperimentFunnelsQuery",
"funnels_query": {
"kind": "FunnelsQuery",
"series": [
{"kind": "EventsNode", "name": "[jan-16-running] seen", "event": "[jan-16-running] seen"},
{"kind": "EventsNode", "name": "[jan-16-running] payment", "event": "[jan-16-running] payment"},
],
"dateRange": {"date_to": "2025-02-13T23:59", "date_from": "2025-01-30T12:16", "explicitDate": True},
"funnelsFilter": {
"layout": "horizontal",
"funnelVizType": "steps",
"funnelWindowInterval": 14,
"funnelWindowIntervalUnit": "day",
},
"filterTestAccounts": True,
},
}
trends_query = {
"kind": "ExperimentTrendsQuery",
"count_query": {
"kind": "TrendsQuery",
"series": [
{
"kind": "EventsNode",
"math": "total",
"name": "[jan-16-running] event one",
"event": "[jan-16-running] event one",
}
],
"interval": "day",
"dateRange": {"date_to": "2025-01-16T23:59", "date_from": "2025-01-02T13:54", "explicitDate": True},
"trendsFilter": {"display": "ActionsLineGraph"},
"filterTestAccounts": True,
},
}
saved_trends_metric = ExperimentSavedMetric.objects.create(
name="Test saved metric",
description="Test description",
query=trends_query,
team=self.team,
created_by=self.user,
)
saved_funnel_metric = ExperimentSavedMetric.objects.create(
name="Test saved metric",
description="Test description",
query=funnel_query,
team=self.team,
created_by=self.user,
)
experiment = Experiment.objects.create(
name="Test Experiment with stale dates",
team=self.team,
feature_flag=test_feature_flag,
start_date=datetime(2025, 2, 1),
end_date=None,
metrics=[funnel_query],
metrics_secondary=[trends_query],
)

for saved_metric_data in [saved_funnel_metric, saved_trends_metric]:
saved_metric_serializer = ExperimentToSavedMetricSerializer(
data={
"experiment": experiment.id,
"saved_metric": saved_metric_data.id,
"metadata": {"type": "secondary"},
},
)
saved_metric_serializer.is_valid(raise_exception=True)
saved_metric_serializer.save()

response = self.client.get(f"/api/projects/{self.team.id}/experiments/{experiment.id}")
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(
response.json()["metrics"][0]["funnels_query"]["dateRange"]["date_from"], "2025-02-01T00:00:00Z"
)
self.assertEqual(response.json()["metrics"][0]["funnels_query"]["dateRange"]["date_to"], "")
self.assertEqual(
response.json()["metrics_secondary"][0]["count_query"]["dateRange"]["date_from"], "2025-02-01T00:00:00Z"
)
self.assertEqual(response.json()["metrics_secondary"][0]["count_query"]["dateRange"]["date_to"], "")
self.assertEqual(
response.json()["saved_metrics"][0]["query"]["funnels_query"]["dateRange"]["date_from"],
"2025-02-01T00:00:00Z",
)
self.assertEqual(response.json()["saved_metrics"][0]["query"]["funnels_query"]["dateRange"]["date_to"], "")
self.assertEqual(
response.json()["saved_metrics"][1]["query"]["count_query"]["dateRange"]["date_from"],
"2025-02-01T00:00:00Z",
)
self.assertEqual(response.json()["saved_metrics"][1]["query"]["count_query"]["dateRange"]["date_to"], "")

def test_adding_behavioral_cohort_filter_to_experiment_fails(self):
cohort = Cohort.objects.create(
team=self.team,
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 0 additions & 1 deletion frontend/src/lib/constants.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ export const FEATURE_FLAGS = {
REMOTE_CONFIG: 'remote-config', // owner: @benjackwhite
SITE_DESTINATIONS: 'site-destinations', // owner: @mariusandra #team-cdp
SITE_APP_FUNCTIONS: 'site-app-functions', // owner: @mariusandra #team-cdp
HOG_TRANSFORMATIONS: 'hog-transformations', // owner: #team-cdp
HOG_TRANSFORMATIONS_CUSTOM_HOG_ENABLED: 'hog-transformation-custom-hog-code', // owner: #team-cdp
REPLAY_HOGQL_FILTERS: 'replay-hogql-filters', // owner: @pauldambra #team-replay
SUPPORT_MESSAGE_OVERRIDE: 'support-message-override', // owner: @abigail
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lib/lemon-ui/LemonModal/LemonModal.scss
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
backdrop-filter: blur(0);
}

@each $i in (1061, 1062, 1066, 1067, 1068, 1069) {
@each $i in (1161, 1162, 1166, 1167, 1168, 1169) {
&.LemonModal__overlay--z-#{$i} {
z-index: #{$i};
}
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lib/lemon-ui/LemonModal/LemonModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export interface LemonModalProps {
* some components need more fine control of the z-index
* they can push a specific value to control their position in the stacking order
*/
zIndex?: '1061' | '1062' | '1066' | '1067' | '1068' | '1069'
zIndex?: '1161' | '1162' | '1166' | '1167' | '1168' | '1169'
}

export const LemonModalHeader = ({ children, className }: LemonModalInnerProps): JSX.Element => {
Expand Down
Loading

0 comments on commit 4325426

Please sign in to comment.