Skip to content

Commit

Permalink
Merge branch 'master' into dn-chore/timestamp-button
Browse files Browse the repository at this point in the history
  • Loading branch information
daibhin committed Apr 16, 2024
2 parents adaa455 + 4fd7a76 commit 14ef0b3
Show file tree
Hide file tree
Showing 61 changed files with 2,137 additions and 1,872 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/automerge.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:
steps:
- name: Automerge
if: env.IS_POSTHOG_BOT_AVAILABLE == 'true'
uses: pascalgn/automerge-action@v0.15.5
uses: pascalgn/automerge-action@v0.16.3
env:
GITHUB_TOKEN: ${{ secrets.POSTHOG_BOT_GITHUB_TOKEN }}
- run: echo
30 changes: 15 additions & 15 deletions ee/api/test/__snapshots__/test_organization_resource_access.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"posthog_user"."events_column_config"
FROM "posthog_user"
WHERE "posthog_user"."id" = 2
LIMIT 21 /**/
LIMIT 21
'''
# ---
# name: TestOrganizationResourceAccessAPI.test_list_organization_resource_access_is_not_nplus1.1
Expand All @@ -53,7 +53,7 @@
"posthog_organization"."available_features"
FROM "posthog_organization"
WHERE "posthog_organization"."id" = '00000000-0000-0000-0000-000000000000'::uuid
LIMIT 21 /*controller='organization_resource_access-list',route='api/organizations/%28%3FP%3Cparent_lookup_organization_id%3E%5B%5E/.%5D%2B%29/resource_access/%3F%24'*/
LIMIT 21
'''
# ---
# name: TestOrganizationResourceAccessAPI.test_list_organization_resource_access_is_not_nplus1.10
Expand All @@ -79,7 +79,7 @@
"posthog_organization"."available_features"
FROM "posthog_organization"
WHERE "posthog_organization"."id" = '00000000-0000-0000-0000-000000000000'::uuid
LIMIT 21 /*controller='organization_resource_access-list',route='api/organizations/%28%3FP%3Cparent_lookup_organization_id%3E%5B%5E/.%5D%2B%29/resource_access/%3F%24'*/
LIMIT 21
'''
# ---
# name: TestOrganizationResourceAccessAPI.test_list_organization_resource_access_is_not_nplus1.11
Expand All @@ -88,7 +88,7 @@
FROM "posthog_organizationmembership"
WHERE ("posthog_organizationmembership"."organization_id" = '00000000-0000-0000-0000-000000000000'::uuid
AND "posthog_organizationmembership"."user_id" = 2)
LIMIT 1 /*controller='organization_resource_access-list',route='api/organizations/%28%3FP%3Cparent_lookup_organization_id%3E%5B%5E/.%5D%2B%29/resource_access/%3F%24'*/
LIMIT 1
'''
# ---
# name: TestOrganizationResourceAccessAPI.test_list_organization_resource_access_is_not_nplus1.12
Expand All @@ -102,14 +102,14 @@
FROM "posthog_organizationmembership"
WHERE ("posthog_organizationmembership"."organization_id" = '00000000-0000-0000-0000-000000000000'::uuid
AND "posthog_organizationmembership"."user_id" = 2)
LIMIT 21 /*controller='organization_resource_access-list',route='api/organizations/%28%3FP%3Cparent_lookup_organization_id%3E%5B%5E/.%5D%2B%29/resource_access/%3F%24'*/
LIMIT 21
'''
# ---
# name: TestOrganizationResourceAccessAPI.test_list_organization_resource_access_is_not_nplus1.13
'''
SELECT COUNT(*) AS "__count"
FROM "ee_organizationresourceaccess"
WHERE "ee_organizationresourceaccess"."organization_id" = '00000000-0000-0000-0000-000000000000'::uuid /*controller='organization_resource_access-list',route='api/organizations/%28%3FP%3Cparent_lookup_organization_id%3E%5B%5E/.%5D%2B%29/resource_access/%3F%24'*/
WHERE "ee_organizationresourceaccess"."organization_id" = '00000000-0000-0000-0000-000000000000'::uuid
'''
# ---
# name: TestOrganizationResourceAccessAPI.test_list_organization_resource_access_is_not_nplus1.14
Expand All @@ -123,7 +123,7 @@
"ee_organizationresourceaccess"."updated_at"
FROM "ee_organizationresourceaccess"
WHERE "ee_organizationresourceaccess"."organization_id" = '00000000-0000-0000-0000-000000000000'::uuid
LIMIT 100 /*controller='organization_resource_access-list',route='api/organizations/%28%3FP%3Cparent_lookup_organization_id%3E%5B%5E/.%5D%2B%29/resource_access/%3F%24'*/
LIMIT 100
'''
# ---
# name: TestOrganizationResourceAccessAPI.test_list_organization_resource_access_is_not_nplus1.2
Expand All @@ -149,7 +149,7 @@
"posthog_organization"."available_features"
FROM "posthog_organization"
WHERE "posthog_organization"."id" = '00000000-0000-0000-0000-000000000000'::uuid
LIMIT 21 /*controller='organization_resource_access-list',route='api/organizations/%28%3FP%3Cparent_lookup_organization_id%3E%5B%5E/.%5D%2B%29/resource_access/%3F%24'*/
LIMIT 21
'''
# ---
# name: TestOrganizationResourceAccessAPI.test_list_organization_resource_access_is_not_nplus1.3
Expand All @@ -158,7 +158,7 @@
FROM "posthog_organizationmembership"
WHERE ("posthog_organizationmembership"."organization_id" = '00000000-0000-0000-0000-000000000000'::uuid
AND "posthog_organizationmembership"."user_id" = 2)
LIMIT 1 /*controller='organization_resource_access-list',route='api/organizations/%28%3FP%3Cparent_lookup_organization_id%3E%5B%5E/.%5D%2B%29/resource_access/%3F%24'*/
LIMIT 1
'''
# ---
# name: TestOrganizationResourceAccessAPI.test_list_organization_resource_access_is_not_nplus1.4
Expand All @@ -172,7 +172,7 @@
FROM "posthog_organizationmembership"
WHERE ("posthog_organizationmembership"."organization_id" = '00000000-0000-0000-0000-000000000000'::uuid
AND "posthog_organizationmembership"."user_id" = 2)
LIMIT 21 /*controller='organization_resource_access-list',route='api/organizations/%28%3FP%3Cparent_lookup_organization_id%3E%5B%5E/.%5D%2B%29/resource_access/%3F%24'*/
LIMIT 21
'''
# ---
# name: TestOrganizationResourceAccessAPI.test_list_organization_resource_access_is_not_nplus1.5
Expand All @@ -183,14 +183,14 @@
FROM "posthog_instancesetting"
WHERE "posthog_instancesetting"."key" = 'constance:posthog:RATE_LIMIT_ENABLED'
ORDER BY "posthog_instancesetting"."id" ASC
LIMIT 1 /*controller='organization_resource_access-list',route='api/organizations/%28%3FP%3Cparent_lookup_organization_id%3E%5B%5E/.%5D%2B%29/resource_access/%3F%24'*/
LIMIT 1
'''
# ---
# name: TestOrganizationResourceAccessAPI.test_list_organization_resource_access_is_not_nplus1.6
'''
SELECT COUNT(*) AS "__count"
FROM "ee_organizationresourceaccess"
WHERE "ee_organizationresourceaccess"."organization_id" = '00000000-0000-0000-0000-000000000000'::uuid /*controller='organization_resource_access-list',route='api/organizations/%28%3FP%3Cparent_lookup_organization_id%3E%5B%5E/.%5D%2B%29/resource_access/%3F%24'*/
WHERE "ee_organizationresourceaccess"."organization_id" = '00000000-0000-0000-0000-000000000000'::uuid
'''
# ---
# name: TestOrganizationResourceAccessAPI.test_list_organization_resource_access_is_not_nplus1.7
Expand All @@ -204,7 +204,7 @@
"ee_organizationresourceaccess"."updated_at"
FROM "ee_organizationresourceaccess"
WHERE "ee_organizationresourceaccess"."organization_id" = '00000000-0000-0000-0000-000000000000'::uuid
LIMIT 100 /*controller='organization_resource_access-list',route='api/organizations/%28%3FP%3Cparent_lookup_organization_id%3E%5B%5E/.%5D%2B%29/resource_access/%3F%24'*/
LIMIT 100
'''
# ---
# name: TestOrganizationResourceAccessAPI.test_list_organization_resource_access_is_not_nplus1.8
Expand Down Expand Up @@ -235,7 +235,7 @@
"posthog_user"."events_column_config"
FROM "posthog_user"
WHERE "posthog_user"."id" = 2
LIMIT 21 /**/
LIMIT 21
'''
# ---
# name: TestOrganizationResourceAccessAPI.test_list_organization_resource_access_is_not_nplus1.9
Expand All @@ -261,6 +261,6 @@
"posthog_organization"."available_features"
FROM "posthog_organization"
WHERE "posthog_organization"."id" = '00000000-0000-0000-0000-000000000000'::uuid
LIMIT 21 /*controller='organization_resource_access-list',route='api/organizations/%28%3FP%3Cparent_lookup_organization_id%3E%5B%5E/.%5D%2B%29/resource_access/%3F%24'*/
LIMIT 21
'''
# ---
40 changes: 22 additions & 18 deletions ee/clickhouse/queries/experiments/trend_experiment_result.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import json
from dataclasses import asdict, dataclass
from datetime import datetime
from functools import lru_cache
Expand All @@ -20,6 +21,7 @@
TRENDS_LINEAR,
UNIQUE_USERS,
ExperimentSignificanceCode,
ExperimentNoResultsErrorKeys,
)
from posthog.models.feature_flag import FeatureFlag
from posthog.models.filters.filter import Filter
Expand Down Expand Up @@ -467,34 +469,36 @@ def calculate_p_value(control_variant: Variant, test_variants: List[Variant]) ->
)


def validate_event_variants(insight_results, variants):
if not insight_results or not insight_results[0]:
raise ValidationError("No experiment events have been ingested yet.", code="no-events")
def validate_event_variants(trend_results, variants):
errors = {
ExperimentNoResultsErrorKeys.NO_EVENTS: True,
ExperimentNoResultsErrorKeys.NO_FLAG_INFO: True,
ExperimentNoResultsErrorKeys.NO_CONTROL_VARIANT: True,
ExperimentNoResultsErrorKeys.NO_TEST_VARIANT: True,
}

missing_variants = []
if not trend_results or not trend_results[0]:
raise ValidationError(code="no-results", detail=json.dumps(errors))

errors[ExperimentNoResultsErrorKeys.NO_EVENTS] = False

# Check if "control" is present
control_found = False
for event in insight_results:
for event in trend_results:
event_variant = event.get("breakdown_value")
if event_variant == "control":
control_found = True
errors[ExperimentNoResultsErrorKeys.NO_CONTROL_VARIANT] = False
errors[ExperimentNoResultsErrorKeys.NO_FLAG_INFO] = False
break
if not control_found:
missing_variants.append("control")

# Check if at least one of the test variants is present
test_variants = [variant for variant in variants if variant != "control"]
test_variant_found = False
for event in insight_results:
for event in trend_results:
event_variant = event.get("breakdown_value")
if event_variant in test_variants:
test_variant_found = True
errors[ExperimentNoResultsErrorKeys.NO_TEST_VARIANT] = False
errors[ExperimentNoResultsErrorKeys.NO_FLAG_INFO] = False
break
if not test_variant_found:
missing_variants.extend(test_variants)

if not len(missing_variants) == 0:
missing_variants_str = ", ".join(missing_variants)
message = f"No experiment events have been ingested yet for the following variants: {missing_variants_str}"
raise ValidationError(message, code=f"missing-flag-variants::{missing_variants_str}")
has_errors = any(errors.values())
if has_errors:
raise ValidationError(detail=json.dumps(errors))
107 changes: 65 additions & 42 deletions ee/clickhouse/queries/test/test_experiments.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,79 +134,102 @@ def test_validate_event_variants_no_flag_info(self):

class TestTrendExperiments(unittest.TestCase):
def test_validate_event_variants_no_events(self):
expected_code = "no-events"
with self.assertRaises(ValidationError) as context:
validate_trend_event_variants([], ["test", "control"])

self.assertEqual(expected_code, context.exception.detail[0].code)
trend_results = []

def test_validate_event_variants_missing_variants(self):
insight_results = [
expected_errors = json.dumps(
{
"action": {
"id": "step-b-0",
"type": "events",
"order": 0,
"name": "step-b-0",
},
"label": "test",
"breakdown_value": "test",
ExperimentNoResultsErrorKeys.NO_EVENTS: True,
ExperimentNoResultsErrorKeys.NO_FLAG_INFO: True,
ExperimentNoResultsErrorKeys.NO_CONTROL_VARIANT: True,
ExperimentNoResultsErrorKeys.NO_TEST_VARIANT: True,
}
]
)

expected_code = "missing-flag-variants::control"
with self.assertRaises(ValidationError) as context:
validate_trend_event_variants(insight_results, ["test", "control"])
validate_trend_event_variants(trend_results, ["test", "control"])

self.assertEqual(expected_code, context.exception.detail[0].code)
self.assertEqual(context.exception.detail[0], expected_errors)

def test_validate_event_variants_missing_control(self):
insight_results = [
def test_validate_event_variants_no_control(self):
trend_results = [
{
"action": {
"id": "step-b-0",
"id": "trend-event",
"type": "events",
"order": 0,
"name": "step-b-0",
"name": "trend-event",
},
"label": "test_1",
"breakdown_value": "test_1",
}
]

# Only 1 test variant is required to return results
expected_code = "missing-flag-variants::control"
expected_errors = json.dumps(
{
ExperimentNoResultsErrorKeys.NO_EVENTS: False,
ExperimentNoResultsErrorKeys.NO_FLAG_INFO: False,
ExperimentNoResultsErrorKeys.NO_CONTROL_VARIANT: True,
ExperimentNoResultsErrorKeys.NO_TEST_VARIANT: False,
}
)

with self.assertRaises(ValidationError) as context:
validate_trend_event_variants(insight_results, ["control", "test_1", "test_2"])
validate_trend_event_variants(trend_results, ["control", "test_1", "test_2"])

self.assertEqual(expected_code, context.exception.detail[0].code)
self.assertEqual(context.exception.detail[0], expected_errors)

def test_validate_event_variants_ignore_old_variant(self):
insight_results = [
def test_validate_event_variants_no_test(self):
trend_results = [
{
"action": {
"id": "step-b-0",
"id": "trend-event",
"type": "events",
"order": 0,
"name": "step-b-0",
"name": "trend-event",
},
"label": "test",
"breakdown_value": "test",
},
"label": "control",
"breakdown_value": "control",
}
]

expected_errors = json.dumps(
{
ExperimentNoResultsErrorKeys.NO_EVENTS: False,
ExperimentNoResultsErrorKeys.NO_FLAG_INFO: False,
ExperimentNoResultsErrorKeys.NO_CONTROL_VARIANT: False,
ExperimentNoResultsErrorKeys.NO_TEST_VARIANT: True,
}
)

with self.assertRaises(ValidationError) as context:
validate_trend_event_variants(trend_results, ["control", "test_1", "test_2"])

self.assertEqual(context.exception.detail[0], expected_errors)

def test_validate_event_variants_no_flag_info(self):
trend_results = [
{
"action": {
"id": "step-b-0",
"id": "trend-event",
"type": "events",
"order": 0,
"name": "step-b-0",
"name": "trend-event",
},
"label": "test",
"breakdown_value": "old-variant",
},
"label": "",
"breakdown_value": "",
}
]

expected_code = "missing-flag-variants::control"
expected_errors = json.dumps(
{
ExperimentNoResultsErrorKeys.NO_EVENTS: False,
ExperimentNoResultsErrorKeys.NO_FLAG_INFO: True,
ExperimentNoResultsErrorKeys.NO_CONTROL_VARIANT: True,
ExperimentNoResultsErrorKeys.NO_TEST_VARIANT: True,
}
)

with self.assertRaises(ValidationError) as context:
validate_trend_event_variants(insight_results, ["test", "control"])
validate_trend_event_variants(trend_results, ["control", "test_1", "test_2"])

self.assertEqual(expected_code, context.exception.detail[0].code)
self.assertEqual(context.exception.detail[0], expected_errors)
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export type LemonInputSelectProps = Pick<
onBlur?: () => void
onInputChange?: (newValue: string) => void
'data-attr'?: string
popoverClassName?: string
}

export function LemonInputSelect({
Expand All @@ -50,6 +51,7 @@ export function LemonInputSelect({
disableFiltering = false,
allowCustomValues = false,
autoFocus = false,
popoverClassName,
...props
}: LemonInputSelectProps): JSX.Element {
const [showPopover, setShowPopover] = useState(false)
Expand Down Expand Up @@ -269,6 +271,7 @@ export function LemonInputSelect({
popoverFocusRef.current = true
e.stopPropagation()
}}
className={popoverClassName}
overlay={
<div className="space-y-px overflow-y-auto">
{visibleOptions.length ? (
Expand Down
3 changes: 2 additions & 1 deletion frontend/src/scenes/experiments/ExperimentView/Info.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { ProgressStatus } from '~/types'
import { StatusTag } from '../Experiment'
import { experimentLogic } from '../experimentLogic'
import { getExperimentStatus } from '../experimentsLogic'
import { ResultsTag } from './components'
import { ActionBanner, ResultsTag } from './components'

export function Info(): JSX.Element {
const { experiment } = useValues(experimentLogic)
Expand Down Expand Up @@ -102,6 +102,7 @@ export function Info(): JSX.Element {
compactButtons
/>
</div>
<ActionBanner />
</div>
)
}
Loading

0 comments on commit 14ef0b3

Please sign in to comment.