Merge branch 'hogql/fix-dw-properties' into experiments/data-warehouse-support-v2

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

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<eventUsageLogicType>([
         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<eventUsageLogicType>([
                 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')
         },

frontend/src/scenes/experiments/ExperimentForm.tsx

@@ -224,14 +224,12 @@ const ExperimentFormFields = (): JSX.Element => {
                         </div>
                     </div>
                 </div>
-                {featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOLDOUTS] && (
-                    <div>
-                        <h3>Holdout group</h3>
-                        <div className="text-xs text-muted">Exclude a stable group of users from the experiment.</div>
-                        <LemonDivider />
-                        <HoldoutSelector />
-                    </div>
-                )}
+                <div>
+                    <h3>Holdout group</h3>
+                    <div className="text-xs text-muted">Exclude a stable group of users from the experiment.</div>
+                    <LemonDivider />
+                    <HoldoutSelector />
+                </div>
             </div>
             <LemonButton
                 className="mt-2"

frontend/src/scenes/experiments/ExperimentView/DistributionTable.tsx

@@ -20,11 +20,12 @@ import { Experiment, MultivariateFlagVariant } from '~/types'
 
 import { experimentLogic } from '../experimentLogic'
 import { VariantTag } from './components'
+import { HoldoutSelector } from './HoldoutSelector'
 import { VariantScreenshot } from './VariantScreenshot'
 
 export function DistributionModal({ experimentId }: { experimentId: Experiment['id'] }): JSX.Element {
     const { experiment, experimentLoading, isDistributionModalOpen } = useValues(experimentLogic({ experimentId }))
-    const { closeDistributionModal } = useActions(experimentLogic({ experimentId }))
+    const { closeDistributionModal, updateExperiment } = useActions(experimentLogic({ experimentId }))
 
     const _featureFlagLogic = featureFlagLogic({ id: experiment.feature_flag?.id ?? null } as FeatureFlagLogicProps)
     const { featureFlag, areVariantRolloutsValid, variantRolloutSum } = useValues(_featureFlagLogic)
@@ -63,6 +64,7 @@ export function DistributionModal({ experimentId }: { experimentId: Experiment['
                     <LemonButton
                         onClick={() => {
                             saveSidebarExperimentFeatureFlag(featureFlag)
+                            updateExperiment({ holdout_id: experiment.holdout_id })
                             closeDistributionModal()
                         }}
                         type="primary"
@@ -124,6 +126,7 @@ export function DistributionModal({ experimentId }: { experimentId: Experiment['
                         </p>
                     )}
                 </div>
+                <HoldoutSelector />
             </div>
         </LemonModal>
     )
@@ -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 <div className="h-16" />
+                }
                 return (
                     <div className="my-2">
                         <VariantScreenshot variantKey={item.key} rolloutPercentage={item.rollout_percentage} />
@@ -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 (
         <div>
             <div className="flex">
@@ -237,10 +260,17 @@ export function DistributionTable(): JSX.Element {
                     </div>
                 </div>
             </div>
+            {experiment.holdout && (
+                <LemonBanner type="info" className="mb-4">
+                    This experiment has a holdout group of {experiment.holdout.filters[0].rollout_percentage}%. The
+                    variants are modified to show their relative rollout percentage.
+                </LemonBanner>
+            )}
             <LemonTable
                 loading={false}
                 columns={columns}
-                dataSource={experiment.feature_flag?.filters.multivariate?.variants || []}
+                dataSource={tableData}
+                rowClassName={(item) => (item.key === `holdout-${experiment.holdout?.id}` ? 'bg-mid' : '')}
             />
         </div>
     )

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"
                 />

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 ? (
                 <Holdouts />
             ) : (
                 <>

frontend/src/scenes/experiments/experimentLogic.tsx

@@ -162,6 +162,7 @@ export const experimentLogic = kea<experimentLogicType>([
                 'reportExperimentVariantScreenshotUploaded',
                 'reportExperimentResultsLoadingTimeout',
                 'reportExperimentReleaseConditionsViewed',
+                'reportExperimentHoldoutAssigned',
             ],
         ],
     })),

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<holdoutsLogicType>([
         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<holdoutsLogicType>([
             },
         ],
     }),
-    loaders(({ values }) => ({
+    loaders(({ actions, values }) => ({
         holdouts: [
             [] as Holdout[],
             {
@@ -60,6 +64,7 @@ export const holdoutsLogic = kea<holdoutsLogicType>([
                 },
                 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 }) => {

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(".")

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")