diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom--dark.png b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom--dark.png index 52a44ae8eec..df215f085e9 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom--dark.png and b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark.png b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark.png index 64260538a2c..ee27b11bed5 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark.png and b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark.png differ diff --git a/frontend/src/lib/constants.tsx b/frontend/src/lib/constants.tsx index 29105a42624..d9d84f586c1 100644 --- a/frontend/src/lib/constants.tsx +++ b/frontend/src/lib/constants.tsx @@ -222,7 +222,6 @@ export const FEATURE_FLAGS = { REPLAY_TEMPLATES: 'replay-templates', // owner: @raquelmsmith #team-replay EXPERIMENTS_HOGQL: 'experiments-hogql', // owner: @jurajmajerik #team-experiments ROLE_BASED_ACCESS_CONTROL: 'role-based-access-control', // owner: @zach - EXPERIMENTS_HOLDOUTS: 'experiments-holdouts', // owner: @jurajmajerik #team-experiments MESSAGING: 'messaging', // owner @mariusandra #team-cdp SESSION_REPLAY_URL_BLOCKLIST: 'session-replay-url-blocklist', // owner: @richard-better #team-replay BILLING_TRIAL_FLOW: 'billing-trial-flow', // owner: @zach diff --git a/frontend/src/lib/utils/eventUsageLogic.ts b/frontend/src/lib/utils/eventUsageLogic.ts index 1264e4956a6..de0a7965820 100644 --- a/frontend/src/lib/utils/eventUsageLogic.ts +++ b/frontend/src/lib/utils/eventUsageLogic.ts @@ -14,6 +14,7 @@ import { TimeToSeeDataPayload } from 'lib/internalMetrics' import { isCoreFilter, PROPERTY_KEYS } from 'lib/taxonomy' import { objectClean } from 'lib/utils' import posthog from 'posthog-js' +import { Holdout } from 'scenes/experiments/holdoutsLogic' import { isFilterWithDisplay, isFunnelsFilter, isTrendsFilter } from 'scenes/insights/sharedUtils' import { preflightLogic } from 'scenes/PreflightCheck/preflightLogic' import { EventIndex } from 'scenes/session-recordings/player/eventIndex' @@ -486,6 +487,14 @@ export const eventUsageLogic = kea([ reportExperimentResultsLoadingTimeout: (experimentId: ExperimentIdType) => ({ experimentId }), reportExperimentReleaseConditionsViewed: (experimentId: ExperimentIdType) => ({ experimentId }), reportExperimentReleaseConditionsUpdated: (experimentId: ExperimentIdType) => ({ experimentId }), + reportExperimentHoldoutCreated: (holdout: Holdout) => ({ holdout }), + reportExperimentHoldoutAssigned: ({ + experimentId, + holdoutId, + }: { + experimentId: ExperimentIdType + holdoutId: Holdout['id'] + }) => ({ experimentId, holdoutId }), // Definition Popover reportDataManagementDefinitionHovered: (type: TaxonomicFilterGroupType) => ({ type }), @@ -1082,6 +1091,19 @@ export const eventUsageLogic = kea([ experiment_id: experimentId, }) }, + reportExperimentHoldoutCreated: ({ holdout }) => { + posthog.capture('experiment holdout created', { + name: holdout.name, + holdout_id: holdout.id, + filters: holdout.filters, + }) + }, + reportExperimentHoldoutAssigned: ({ experimentId, holdoutId }) => { + posthog.capture('experiment holdout assigned', { + experiment_id: experimentId, + holdout_id: holdoutId, + }) + }, reportPropertyGroupFilterAdded: () => { posthog.capture('property group filter added') }, diff --git a/frontend/src/scenes/experiments/ExperimentForm.tsx b/frontend/src/scenes/experiments/ExperimentForm.tsx index 1e0ad4f535b..f71d0bb3c0b 100644 --- a/frontend/src/scenes/experiments/ExperimentForm.tsx +++ b/frontend/src/scenes/experiments/ExperimentForm.tsx @@ -224,14 +224,12 @@ const ExperimentFormFields = (): JSX.Element => { - {featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOLDOUTS] && ( -
-

Holdout group

-
Exclude a stable group of users from the experiment.
- - -
- )} +
+

Holdout group

+
Exclude a stable group of users from the experiment.
+ + +
{ saveSidebarExperimentFeatureFlag(featureFlag) + updateExperiment({ holdout_id: experiment.holdout_id }) closeDistributionModal() }} type="primary" @@ -124,6 +126,7 @@ export function DistributionModal({ experimentId }: { experimentId: Experiment['

)} + ) @@ -179,6 +182,9 @@ export function DistributionTable(): JSX.Element { key: 'variant_screenshot', title: 'Screenshot', render: function Key(_, item): JSX.Element { + if (item.key === `holdout-${experiment.holdout?.id}`) { + return
+ } return (
@@ -213,6 +219,23 @@ export function DistributionTable(): JSX.Element { }) } + const holdoutData = experiment.holdout + ? [ + { + key: `holdout-${experiment.holdout.id}`, + rollout_percentage: experiment.holdout.filters[0].rollout_percentage, + } as MultivariateFlagVariant, + ] + : [] + + const variantData = (experiment.feature_flag?.filters.multivariate?.variants || []).map((variant) => ({ + ...variant, + rollout_percentage: + variant.rollout_percentage * ((100 - (experiment.holdout?.filters[0].rollout_percentage || 0)) / 100), + })) + + const tableData = [...variantData, ...holdoutData] + return (
@@ -237,10 +260,17 @@ export function DistributionTable(): JSX.Element {
+ {experiment.holdout && ( + + This experiment has a holdout group of {experiment.holdout.filters[0].rollout_percentage}%. The + variants are modified to show their relative rollout percentage. + + )} (item.key === `holdout-${experiment.holdout?.id}` ? 'bg-mid' : '')} />
) diff --git a/frontend/src/scenes/experiments/ExperimentView/HoldoutSelector.tsx b/frontend/src/scenes/experiments/ExperimentView/HoldoutSelector.tsx index f8982ad3d36..56129767d65 100644 --- a/frontend/src/scenes/experiments/ExperimentView/HoldoutSelector.tsx +++ b/frontend/src/scenes/experiments/ExperimentView/HoldoutSelector.tsx @@ -6,7 +6,7 @@ import { experimentLogic } from '../experimentLogic' export function HoldoutSelector(): JSX.Element { const { experiment, holdouts, isExperimentRunning } = useValues(experimentLogic) - const { setExperiment, updateExperiment } = useActions(experimentLogic) + const { setExperiment, reportExperimentHoldoutAssigned } = useActions(experimentLogic) const holdoutOptions = holdouts.map((holdout) => ({ value: holdout.id, @@ -37,7 +37,7 @@ export function HoldoutSelector(): JSX.Element { ...experiment, holdout_id: value, }) - updateExperiment({ holdout_id: value }) + reportExperimentHoldoutAssigned({ experimentId: experiment.id, holdoutId: value }) }} data-attr="experiment-holdout-selector" /> diff --git a/frontend/src/scenes/experiments/Experiments.tsx b/frontend/src/scenes/experiments/Experiments.tsx index a9e4779319b..6cd77e39439 100644 --- a/frontend/src/scenes/experiments/Experiments.tsx +++ b/frontend/src/scenes/experiments/Experiments.tsx @@ -249,13 +249,11 @@ export function Experiments(): JSX.Element { { key: ExperimentsTabs.All, label: 'All experiments' }, { key: ExperimentsTabs.Yours, label: 'Your experiments' }, { key: ExperimentsTabs.Archived, label: 'Archived experiments' }, - ...(featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOLDOUTS] - ? [{ key: ExperimentsTabs.Holdouts, label: 'Holdout groups' }] - : []), + { key: ExperimentsTabs.Holdouts, label: 'Holdout groups' }, ]} /> - {featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOLDOUTS] && tab === ExperimentsTabs.Holdouts ? ( + {tab === ExperimentsTabs.Holdouts ? ( ) : ( <> diff --git a/frontend/src/scenes/experiments/experimentLogic.tsx b/frontend/src/scenes/experiments/experimentLogic.tsx index 81388ea6455..647c4bdabf2 100644 --- a/frontend/src/scenes/experiments/experimentLogic.tsx +++ b/frontend/src/scenes/experiments/experimentLogic.tsx @@ -162,6 +162,7 @@ export const experimentLogic = kea([ 'reportExperimentVariantScreenshotUploaded', 'reportExperimentResultsLoadingTimeout', 'reportExperimentReleaseConditionsViewed', + 'reportExperimentHoldoutAssigned', ], ], })), diff --git a/frontend/src/scenes/experiments/holdoutsLogic.tsx b/frontend/src/scenes/experiments/holdoutsLogic.tsx index 3f70a30d612..f90b3e43789 100644 --- a/frontend/src/scenes/experiments/holdoutsLogic.tsx +++ b/frontend/src/scenes/experiments/holdoutsLogic.tsx @@ -1,7 +1,8 @@ -import { actions, events, kea, listeners, path, reducers } from 'kea' +import { actions, connect, events, kea, listeners, path, reducers } from 'kea' import { loaders } from 'kea-loaders' import api from 'lib/api' import { lemonToast } from 'lib/lemon-ui/LemonToast/LemonToast' +import { eventUsageLogic } from 'lib/utils/eventUsageLogic' import { UserBasicType } from '~/types' @@ -42,6 +43,9 @@ export const holdoutsLogic = kea([ deleteHoldout: (id: number | null) => ({ id }), loadHoldout: (id: number | null) => ({ id }), }), + connect({ + actions: [eventUsageLogic, ['reportExperimentHoldoutCreated']], + }), reducers({ holdout: [ NEW_HOLDOUT, @@ -50,7 +54,7 @@ export const holdoutsLogic = kea([ }, ], }), - loaders(({ values }) => ({ + loaders(({ actions, values }) => ({ holdouts: [ [] as Holdout[], { @@ -60,6 +64,7 @@ export const holdoutsLogic = kea([ }, createHoldout: async () => { const response = await api.create(`api/projects/@current/experiment_holdouts/`, values.holdout) + actions.reportExperimentHoldoutCreated(response) return [...values.holdouts, response] as Holdout[] }, updateHoldout: async ({ id, holdout }) => { diff --git a/posthog/hogql/property.py b/posthog/hogql/property.py index 10eb991ea8f..9536b70fa5f 100644 --- a/posthog/hogql/property.py +++ b/posthog/hogql/property.py @@ -346,6 +346,55 @@ def property_to_expr( chain = ["events", "properties"] elif property.type == "session" and scope == "replay_entity": chain = ["events", "session"] + elif property.type == "data_warehouse": + if isinstance(property.key, str): + split = property.key.split(".") + chain = split[:-1] + property_key = split[-1] + else: + raise QueryError("Data warehouse property filter value must be a string") + + # For data warehouse properties, we split the key on dots to get the table chain and property key + # e.g. "foobars.properties.$feature/flag" becomes chain=["foobars", "properties"] and property_key="$feature/flag" + # This allows us to properly reference nested fields in data warehouse tables while maintaining consistent + # property filtering behavior. The chain represents the path to traverse through nested table structures, + # while the property_key is the actual field name we want to filter on. + if isinstance(value, list): + if len(value) == 0: + return ast.Constant(value=1) + elif len(value) == 1: + value = value[0] + else: + field = ast.Field(chain=[*chain, property_key]) + exprs = [ + _expr_to_compare_op( + expr=field, + value=v, + operator=operator, + team=team, + property=property, + is_json_field=False, + ) + for v in value + ] + if ( + operator == PropertyOperator.NOT_ICONTAINS + or operator == PropertyOperator.NOT_REGEX + or operator == PropertyOperator.IS_NOT + ): + return ast.And(exprs=exprs) + return ast.Or(exprs=exprs) + + field = ast.Field(chain=[*chain, property_key]) + return _expr_to_compare_op( + expr=field, + value=value, + operator=operator, + team=team, + property=property, + is_json_field=False, + ) + elif property.type == "data_warehouse_person_property": if isinstance(property.key, str): table, key = property.key.split(".") diff --git a/posthog/hogql/test/test_property.py b/posthog/hogql/test/test_property.py index fb2f2f23097..56f342c3444 100644 --- a/posthog/hogql/test/test_property.py +++ b/posthog/hogql/test/test_property.py @@ -785,3 +785,45 @@ class TestProperty(BaseTest): ), self._parse_expr("person.extended_properties.bool_prop = true"), ) + + def test_data_warehouse_property_with_list_values(self): + credential = DataWarehouseCredential.objects.create( + team=self.team, access_key="_accesskey", access_secret="_secret" + ) + DataWarehouseTable.objects.create( + team=self.team, + name="foobars", + columns={ + "event": {"hogql": "StringDatabaseField", "clickhouse": "String"}, + "properties": {"hogql": "JSONField", "clickhouse": "String"}, + }, + credential=credential, + url_pattern="", + ) + + expr = property_to_expr( + Property( + type="data_warehouse", + key="foobars.properties.$feature/test", + value=["control", "test"], + operator="exact", + ), + self.team, + ) + + self.assertIsInstance(expr, ast.Or) + self.assertEqual(len(expr.exprs), 2) + + # First expression + compare_op_1 = expr.exprs[0] + self.assertIsInstance(compare_op_1, ast.CompareOperation) + self.assertIsInstance(compare_op_1.left, ast.Field) + self.assertEqual(compare_op_1.left.chain, ["foobars", "properties", "$feature/test"]) + self.assertEqual(compare_op_1.right.value, "control") + + # Second expression + compare_op_2 = expr.exprs[1] + self.assertIsInstance(compare_op_2, ast.CompareOperation) + self.assertIsInstance(compare_op_2.left, ast.Field) + self.assertEqual(compare_op_2.left.chain, ["foobars", "properties", "$feature/test"]) + self.assertEqual(compare_op_2.right.value, "test")