feat(experiments): Properly translate event and action filters #28662

danielbachhuber · 2025-02-13T13:20:11Z

Merges into #28347

Changes

Properly translates event and action filters into their corresponding metric config objects.

Event

CleanShot.2025-02-13.at.05.12.49.mp4

Action

CleanShot.2025-02-13.at.05.14.44.mp4

Shared metric

CleanShot.2025-02-13.at.05.19.25.mp4

How did you test this code?

I added an event and an action to an experiment and verified they saved as expected.

I also created a shared metric for an action and a shared metric for a filter and verified they saved as expected.

greptile-apps

PR Summary

This PR adds functionality to properly translate event and action filters into metric configuration objects for experiments in PostHog. Here are the key changes:

Added new ExperimentActionMetricConfig type and interface in schema-general.ts to support action-based metrics in experiments
Introduced filterToMetricConfig utility function to convert event/action filters into metric configurations, preserving properties like math operations and HogQL expressions
Added metricConfigToFilter utility function for the reverse conversion from metric configs back to filter format
Updated ExperimentMetricForm and SharedExperimentMetricForm components to use the new conversion utilities instead of direct metric construction
Added comprehensive test coverage for the new filter translation functions, including property preservation and type safety checks

The changes improve type safety and maintainability by centralizing the conversion logic in dedicated utility functions.

_{5 file(s) reviewed, 7 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2025-02-13T13:21:28Z

frontend/src/scenes/experiments/Metrics/ExperimentMetricForm.tsx

                    const entity = events?.[0] || actions?.[0]
-                    if (entity) {
+                    const metricConfig = filterToMetricConfig(entity as EventsNode | ActionsNode)


logic: Type assertion to EventsNode | ActionsNode could fail if entity is undefined - consider adding null check

Suggested change

const entity = events?.[0] || actions?.[0]

if (entity) {

const metricConfig = filterToMetricConfig(entity as EventsNode | ActionsNode)

const entity = events?.[0] || actions?.[0]

if (!entity) return

const metricConfig = filterToMetricConfig(entity as EventsNode | ActionsNode)

greptile-apps · 2025-02-13T13:22:00Z

frontend/src/scenes/experiments/utils.test.ts

+                    math_property: undefined,
+                    math_hogql: undefined,
+                    properties: [{ key: '$lib', type: 'event', value: ['python'], operator: 'exact' }],
+                    kind: NodeKind.EventsNode,


logic: kind is set to EventsNode for an action type filter - this seems incorrect and inconsistent with the type declaration

greptile-apps · 2025-02-13T13:22:01Z

frontend/src/scenes/experiments/utils.test.ts

+        const action = {
+            id: '8',
+            name: 'jan-16-running payment action',
+            kind: 'EventsNode',


style: kind is set to 'EventsNode' as string instead of using NodeKind.EventsNode enum

Suggested change

kind: 'EventsNode',

kind: NodeKind.EventsNode,

greptile-apps · 2025-02-13T13:22:08Z

frontend/src/scenes/experiments/SharedMetrics/SharedExperimentMetricForm.tsx

                    const entity = events?.[0] || actions?.[0]
-                    if (entity) {
+                    const metricConfig = filterToMetricConfig(entity as EventsNode | ActionsNode)


logic: Type assertion on line 49 could fail if entity is undefined since line 48 doesn't check for null/undefined

Suggested change

const entity = events?.[0] || actions?.[0]

if (entity) {

const metricConfig = filterToMetricConfig(entity as EventsNode | ActionsNode)

const entity = events?.[0] || actions?.[0]

if (!entity) return

const metricConfig = filterToMetricConfig(entity as EventsNode | ActionsNode)

greptile-apps · 2025-02-13T13:22:24Z

frontend/src/scenes/experiments/utils.ts

-                },
+                    id: metric_config.action,
+                    name: metric_config.name,
+                    kind: NodeKind.EventsNode,


logic: incorrect node kind for actions - should be NodeKind.ActionsNode instead of NodeKind.EventsNode

Suggested change

kind: NodeKind.EventsNode,

kind: NodeKind.ActionsNode,

greptile-apps · 2025-02-13T13:22:25Z

frontend/src/scenes/experiments/utils.ts

+export function metricConfigToFilter(
+    metric_config: ExperimentEventMetricConfig | ExperimentActionMetricConfig | ExperimentDataWarehouseMetricConfig
+): FilterType {
+    if (metric_config.kind === NodeKind.ExperimentEventMetricConfig) {
        return {
            events: [
                {
-                    id: '$pageview',
-                    name: '$pageview',
+                    id: metric_config.event,
+                    name: metric_config.event,
+                    kind: NodeKind.EventsNode,
                    type: 'events',
-                    math: 'total',
-                    math_property: null,
-                    math_hogql: null,
-                },
+                    math: metric_config.math,
+                    math_property: metric_config.math_property,
+                    math_hogql: metric_config.math_hogql,
+                    properties: metric_config.properties,
+                } as EventsNode,
            ],
            actions: [],
        }
-    }
-
-    const config = metric.metric_config
-    if (config.kind === NodeKind.ExperimentEventMetricConfig) {
+    } else if (metric_config.kind === NodeKind.ExperimentActionMetricConfig) {
        return {
-            events: [
+            events: [],
+            actions: [
                {
-                    id: config.event,
-                    name: config.event,
-                    type: 'events',
-                    math: config.math,
-                    math_property: config.math_property,
-                    math_hogql: config.math_hogql,
-                },
+                    id: metric_config.action,
+                    name: metric_config.name,
+                    kind: NodeKind.EventsNode,
+                    type: 'actions',
+                    math: metric_config.math,
+                    math_property: metric_config.math_property,
+                    math_hogql: metric_config.math_hogql,
+                    properties: metric_config.properties,
+                } as EventsNode,
            ],
-            actions: [],
        }
    }

    return {}
 }


logic: missing handling for ExperimentDataWarehouseMetricConfig case despite being in the function signature

greptile-apps · 2025-02-13T13:22:26Z

frontend/src/scenes/experiments/utils.ts

+        return undefined
+    }
+
+    if (entity.kind === NodeKind.EventsNode) {


style: consider adding a type guard here instead of using Record<string, any>

andehen

Awesome! Tested locally and stores metrics as expected 👍

andehen · 2025-02-13T16:45:43Z

frontend/src/queries/schema/schema-general.ts

+    properties?: AnyPropertyFilter[]
+}
+
+export interface ExperimentActionMetricConfig {


One thought: We could remove event / action from these two schemas, and add id and type: 'events' | 'action. It will match what we get from the form then. We will get away with one schema and simplify metricConfigToFilter logic. Not a strong opinion, but it will be less code.

@andehen Potentially! I think I'd like to keep them separate for now, as there's some value in having them explicitly defined as such.

danielbachhuber added 2 commits February 13, 2025 05:09

Properly translate event and action filters

05de06a

Regenerate schema

41f47b7

danielbachhuber requested a review from a team February 13, 2025 13:20

greptile-apps bot reviewed Feb 13, 2025

View reviewed changes

Don't type cast unnecessarily

9ef08a2

andehen approved these changes Feb 13, 2025

View reviewed changes

danielbachhuber merged commit 382fb93 into init-new-experiment-query-runner Feb 13, 2025
98 of 99 checks passed

danielbachhuber deleted the assign-events-actions branch February 13, 2025 18:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(experiments): Properly translate event and action filters #28662

feat(experiments): Properly translate event and action filters #28662

danielbachhuber commented Feb 13, 2025

greptile-apps bot left a comment

greptile-apps bot Feb 13, 2025

greptile-apps bot Feb 13, 2025

greptile-apps bot Feb 13, 2025

greptile-apps bot Feb 13, 2025

greptile-apps bot Feb 13, 2025

greptile-apps bot Feb 13, 2025

greptile-apps bot Feb 13, 2025

andehen left a comment

andehen Feb 13, 2025

danielbachhuber Feb 13, 2025

feat(experiments): Properly translate event and action filters #28662

feat(experiments): Properly translate event and action filters #28662

Conversation

danielbachhuber commented Feb 13, 2025

Changes

How did you test this code?

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Feb 13, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 13, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 13, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 13, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 13, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 13, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 13, 2025

Choose a reason for hiding this comment

andehen left a comment

Choose a reason for hiding this comment

andehen Feb 13, 2025

Choose a reason for hiding this comment

danielbachhuber Feb 13, 2025

Choose a reason for hiding this comment