Enable persons modal for properties correlation (#6611)

* Enable persons modal for properties correlation

* adjust header

* fix typings

ee/clickhouse/queries/funnels/base.py

@@ -365,18 +365,23 @@ def _build_filters(self, entity: Entity, index: int) -> str:
 
     def _get_funnel_person_step_condition(self):
         step_num = self._filter.funnel_step
+        custom_steps = self._filter.funnel_custom_steps
         max_steps = len(self._filter.entities)
 
-        if step_num is None:
-            raise ValueError("funnel_step should not be none")
-
         conditions = []
-        if step_num >= 0:
-            self.params.update({"step_num": [i for i in range(step_num, max_steps + 1)]})
-            conditions.append("steps IN %(step_num)s")
+
+        if custom_steps:
+            self.params.update({"custom_step_num": custom_steps})
+            conditions.append("steps IN %(custom_step_num)s")
+        elif step_num is not None:
+            if step_num >= 0:
+                self.params.update({"step_num": [i for i in range(step_num, max_steps + 1)]})
+                conditions.append("steps IN %(step_num)s")
+            else:
+                self.params.update({"step_num": abs(step_num) - 1})
+                conditions.append("steps = %(step_num)s")
         else:
-            self.params.update({"step_num": abs(step_num) - 1})
-            conditions.append("steps = %(step_num)s")
+            raise ValueError("Missing both funnel_step and funnel_custom_steps")
 
         if self._filter.funnel_step_breakdown is not None:
             prop_vals = self._parse_breakdown_prop_value()

ee/clickhouse/queries/funnels/test/test_funnel_persons.py

@@ -190,6 +190,92 @@ def test_basic_offset(self):
         results, _ = ClickhouseFunnelPersons(filter_offset, self.team).run()
         self.assertEqual(10, len(results))
 
+    def test_steps_with_custom_steps_parameter_are_equivalent_to_funnel_step(self):
+        self._create_sample_data_multiple_dropoffs()
+        data = {
+            "insight": INSIGHT_FUNNELS,
+            "interval": "day",
+            "date_from": "2021-05-01 00:00:00",
+            "date_to": "2021-05-07 00:00:00",
+            "funnel_window_days": 7,
+            "events": [
+                {"id": "step one", "order": 0},
+                {"id": "step two", "order": 1},
+                {"id": "step three", "order": 2},
+            ],
+        }
+        base_filter = Filter(data=data)
+
+        parameters = [
+            #  funnel_step,  custom_steps, expected_results
+            (1, [1, 2, 3], 35),
+            (2, [2, 3], 15),
+            (3, [3], 5),
+            (-2, [1], 20),
+            (-3, [2], 10),
+        ]
+
+        for funnel_step, custom_steps, expected_count in parameters:
+            filter = base_filter.with_data({"funnel_step": funnel_step})
+            results = ClickhouseFunnelPersons(filter, self.team)._exec_query()
+
+            new_filter = base_filter.with_data({"funnel_custom_steps": custom_steps})
+            new_results = ClickhouseFunnelPersons(new_filter, self.team)._exec_query()
+
+            self.assertEqual(new_results, results)
+            self.assertEqual(len(results), expected_count)
+
+    def test_steps_with_custom_steps_parameter_where_funnel_step_equivalence_isnt_possible(self):
+        self._create_sample_data_multiple_dropoffs()
+        data = {
+            "insight": INSIGHT_FUNNELS,
+            "interval": "day",
+            "date_from": "2021-05-01 00:00:00",
+            "date_to": "2021-05-07 00:00:00",
+            "funnel_window_days": 7,
+            "events": [
+                {"id": "step one", "order": 0},
+                {"id": "step two", "order": 1},
+                {"id": "step three", "order": 2},
+            ],
+        }
+        base_filter = Filter(data=data)
+
+        parameters = [
+            # custom_steps, expected_results
+            ([1, 2], 30),
+            ([1, 3], 25),
+            ([3, 1], 25),
+            ([1, 3, 3, 1], 25),
+        ]
+
+        for custom_steps, expected_count in parameters:
+            new_filter = base_filter.with_data({"funnel_custom_steps": custom_steps})
+            new_results = ClickhouseFunnelPersons(new_filter, self.team)._exec_query()
+
+            self.assertEqual(len(new_results), expected_count)
+
+    def test_steps_with_custom_steps_parameter_overrides_funnel_step(self):
+        self._create_sample_data_multiple_dropoffs()
+        data = {
+            "insight": INSIGHT_FUNNELS,
+            "interval": "day",
+            "date_from": "2021-05-01 00:00:00",
+            "date_to": "2021-05-07 00:00:00",
+            "funnel_window_days": 7,
+            "funnel_step": 1,  # means custom steps = [1,2,3]
+            "funnel_custom_steps": [3],
+            "events": [
+                {"id": "step one", "order": 0},
+                {"id": "step two", "order": 1},
+                {"id": "step three", "order": 2},
+            ],
+        }
+
+        results = ClickhouseFunnelPersons(Filter(data=data), self.team)._exec_query()
+
+        self.assertEqual(len(results), 5)
+
     @test_with_materialized_columns(["$browser"])
     def test_first_step_breakdowns(self):
         person1, person2 = self._create_browser_breakdown_events()
@@ -249,9 +335,21 @@ def test_first_step_breakdown_person(self):
         results = ClickhouseFunnelPersons(filter.with_data({"funnel_step_breakdown": "EE"}), self.team)._exec_query()
         self.assertCountEqual([val[0] for val in results], [person2.uuid])
 
+        # Check custom_steps give same answers for breakdowns
+        custom_step_results = ClickhouseFunnelPersons(
+            filter.with_data({"funnel_step_breakdown": "EE", "funnel_custom_steps": [1, 2, 3]}), self.team
+        )._exec_query()
+        self.assertEqual(results, custom_step_results)
+
         results = ClickhouseFunnelPersons(filter.with_data({"funnel_step_breakdown": "PL"}), self.team)._exec_query()
         self.assertCountEqual([val[0] for val in results], [person1.uuid])
 
+        # Check custom_steps give same answers for breakdowns
+        custom_step_results = ClickhouseFunnelPersons(
+            filter.with_data({"funnel_step_breakdown": "PL", "funnel_custom_steps": [1, 2, 3]}), self.team
+        )._exec_query()
+        self.assertEqual(results, custom_step_results)
+
     @test_with_materialized_columns(["$browser"], verify_no_jsonextract=False)
     def test_funnel_cohort_breakdown_persons(self):
         person = _create_person(distinct_ids=[f"person1"], team_id=self.team.pk, properties={"key": "value"})

frontend/src/scenes/funnels/funnelLogic.ts

@@ -28,6 +28,7 @@ import {
     FunnelStepReference,
     FunnelAPIResponse,
     TrendResult,
+    BreakdownType,
 } from '~/types'
 import { FunnelLayout, BinCountAuto, FEATURE_FLAGS } from 'lib/constants'
 import { preflightLogic } from 'scenes/PreflightCheck/logic'
@@ -81,11 +82,17 @@ export const funnelLogic = kea<funnelLogicType>({
         openPersonsModal: (
             step: FunnelStep | FunnelStepWithNestedBreakdown,
             stepNumber: number,
-            breakdown_value?: string | number
+            breakdown_value?: string | number,
+            breakdown?: string,
+            breakdown_type?: BreakdownType,
+            customSteps?: number[]
         ) => ({
             step,
             stepNumber,
             breakdown_value,
+            breakdown,
+            breakdown_type,
+            customSteps,
         }),
         openCorrelationPersonsModal: (entity: Record<string, any>, converted: boolean) => ({
             entity,
@@ -854,14 +861,14 @@ export const funnelLogic = kea<funnelLogicType>({
             })
             actions.loadFunnels()
         },
-        openPersonsModal: ({ step, stepNumber, breakdown_value }) => {
+        openPersonsModal: ({ step, stepNumber, breakdown_value, breakdown, breakdown_type, customSteps }) => {
             personsModalLogic.actions.loadPeople({
-                action: { id: step.action_id, name: step.name, properties: [], type: step.type },
+                action: 'session',
                 breakdown_value: breakdown_value !== undefined ? breakdown_value : undefined,
                 label: step.name,
                 date_from: '',
                 date_to: '',
-                filters: values.filters,
+                filters: { ...values.filters, breakdown, breakdown_type, funnel_custom_steps: customSteps },
                 saveOriginal: true,
                 funnelStep: stepNumber,
             })

frontend/src/scenes/insights/InsightTabs/FunnelTab/FunnelPropertyCorrelationTable.tsx

@@ -4,16 +4,17 @@ import Column from 'antd/lib/table/Column'
 import { useActions, useValues } from 'kea'
 import { RiseOutlined, FallOutlined } from '@ant-design/icons'
 import { funnelLogic } from 'scenes/funnels/funnelLogic'
-import { FunnelCorrelation, FunnelCorrelationType } from '~/types'
+import { FunnelCorrelation, FunnelCorrelationType, FunnelStep } from '~/types'
 import Checkbox from 'antd/lib/checkbox/Checkbox'
 import { insightLogic } from 'scenes/insights/insightLogic'
 import { PropertyNamesSelect } from 'lib/components/PropertyNamesSelect/PropertyNamesSelect'
+import { ValueInspectorButton } from 'scenes/funnels/FunnelBarGraph'
 
 export function FunnelPropertyCorrelationTable(): JSX.Element | null {
     const { insightProps } = useValues(insightLogic)
     const logic = funnelLogic(insightProps)
     const { stepsWithCount, propertyCorrelationValues, propertyCorrelationTypes } = useValues(logic)
-    const { setPropertyCorrelationTypes, loadPropertyCorrelations } = useActions(logic)
+    const { setPropertyCorrelationTypes, loadPropertyCorrelations, openPersonsModal } = useActions(logic)
     const onClickCorrelationType = (correlationType: FunnelCorrelationType): void => {
         if (propertyCorrelationTypes) {
             if (propertyCorrelationTypes.includes(correlationType)) {
@@ -25,6 +26,71 @@ export function FunnelPropertyCorrelationTable(): JSX.Element | null {
             setPropertyCorrelationTypes([correlationType])
         }
     }
+
+    const parseBreakdownValue = (item: string): { breakdown: string; breakdown_value: string } => {
+        const components = item.split('::')
+        if (components.length === 1) {
+            return { breakdown: components[0], breakdown_value: '' }
+        } else {
+            return {
+                breakdown: components[0],
+                breakdown_value: components[1],
+            }
+        }
+    }
+
+    // A sentinel node used to respect typings
+    const emptyFunnelStep: FunnelStep = {
+        action_id: '',
+        average_conversion_time: null,
+        count: 0,
+        name: '',
+        order: 0,
+        type: 'new_entity',
+    }
+
+    const renderSuccessCount = (record: FunnelCorrelation): JSX.Element => {
+        const { breakdown, breakdown_value } = parseBreakdownValue(record.event || '')
+
+        return (
+            <ValueInspectorButton
+                onClick={() => {
+                    openPersonsModal(
+                        { ...emptyFunnelStep, name: breakdown },
+                        stepsWithCount.length,
+                        breakdown_value,
+                        breakdown,
+                        'person',
+                        undefined
+                    )
+                }}
+            >
+                {record.success_count}
+            </ValueInspectorButton>
+        )
+    }
+
+    const renderFailureCount = (record: FunnelCorrelation): JSX.Element => {
+        const { breakdown, breakdown_value } = parseBreakdownValue(record.event || '')
+
+        return (
+            <ValueInspectorButton
+                onClick={() => {
+                    openPersonsModal(
+                        { ...emptyFunnelStep, name: breakdown },
+                        -2,
+                        breakdown_value,
+                        breakdown,
+                        'person',
+                        Array.from(Array(stepsWithCount.length).keys()).slice(1) // returns array like: [1,2,3,.... stepsWithCount.length - 1]
+                    )
+                }}
+            >
+                {record.failure_count}
+            </ValueInspectorButton>
+        )
+    }
+
     return stepsWithCount.length > 1 ? (
         <Table
             dataSource={propertyCorrelationValues}
@@ -109,8 +175,20 @@ export function FunnelPropertyCorrelationTable(): JSX.Element | null {
                 }}
                 align="left"
             />
-            <Column title="Completed" dataIndex="success_count" width={90} align="center" />
-            <Column title="Dropped off" dataIndex="failure_count" width={100} align="center" />
+            <Column
+                title="Completed"
+                key="success_count"
+                render={(_, record: FunnelCorrelation) => renderSuccessCount(record)}
+                width={90}
+                align="center"
+            />
+            <Column
+                title="Dropped off"
+                key="failure_count"
+                render={(_, record: FunnelCorrelation) => renderFailureCount(record)}
+                width={100}
+                align="center"
+            />
         </Table>
     ) : null
 }

frontend/src/scenes/insights/utils/cleanFilters.ts

@@ -74,6 +74,7 @@ export function cleanFilters(filters: Partial<FilterType>, oldFilters?: Partial<
             breakdown_type: breakdownEnabled ? filters.breakdown_type || undefined : undefined,
             funnel_correlation_person_entity: filters.funnel_correlation_person_entity || undefined,
             funnel_correlation_person_converted: filters.funnel_correlation_person_converted || undefined,
+            funnel_custom_steps: filters.funnel_custom_steps || undefined,
         }
 
         // if we came from an URL with just `#q={insight:TRENDS}` (no `events`/`actions`), add the default states `[]`

frontend/src/types.ts

@@ -813,6 +813,7 @@ export interface FilterType {
     max_edge_weight?: number | undefined // Paths
     funnel_correlation_person_entity?: Record<string, any> // Funnel Correlation Persons Filter
     funnel_correlation_person_converted?: 'true' | 'false' // Funnel Correlation Persons Converted - success or failure counts
+    funnel_custom_steps?: number[] // used to provide custom steps for which to get people in a funnel - primarily for correlation use
 }
 
 export interface SystemStatusSubrows {

posthog/constants.py

@@ -113,6 +113,7 @@ class AvailableFeature(str, Enum):
 FUNNEL_FROM_STEP = "funnel_from_step"
 FUNNEL_TO_STEP = "funnel_to_step"
 FUNNEL_STEP = "funnel_step"
+FUNNEL_CUSTOM_STEPS = "funnel_custom_steps"
 FUNNEL_STEP_BREAKDOWN = "funnel_step_breakdown"
 FUNNEL_LAYOUT = "layout"
 FUNNEL_ORDER_TYPE = "funnel_order_type"

posthog/models/filters/mixins/funnel.py

@@ -19,6 +19,7 @@
     FUNNEL_CORRELATION_PERSON_LIMIT,
     FUNNEL_CORRELATION_PERSON_OFFSET,
     FUNNEL_CORRELATION_TYPE,
+    FUNNEL_CUSTOM_STEPS,
     FUNNEL_FROM_STEP,
     FUNNEL_LAYOUT,
     FUNNEL_ORDER_TYPE,
@@ -141,14 +142,33 @@ def funnel_step(self) -> Optional[int]:
             return None
         return _step
 
+    @cached_property
+    def funnel_custom_steps(self) -> List[int]:
+        """
+        Custom step numbers to get persons for. This overrides FunnelPersonsStepMixin::funnel_step
+        """
+        raw_steps = self._data.get(FUNNEL_CUSTOM_STEPS, [])
+        if isinstance(raw_steps, str):
+            return json.loads(raw_steps)
+
+        return raw_steps
+
     @include_dict
     def funnel_step_to_dict(self):
-        return {FUNNEL_STEP: self.funnel_step} if self.funnel_step else {}
+        result: dict = {}
+        if self.funnel_step:
+            result[FUNNEL_STEP] = self.funnel_step
+        if self.funnel_custom_steps:
+            result[FUNNEL_CUSTOM_STEPS] = self.funnel_custom_steps
+        return result
 
 
 class FunnelPersonsStepBreakdownMixin(BaseParamMixin):
     @cached_property
     def funnel_step_breakdown(self) -> Optional[Union[str, int]]:
+        """
+        The breakdown value for which to get persons for.
+        """
         return self._data.get(FUNNEL_STEP_BREAKDOWN)
 
     @include_dict