From 01454836d5b4eda0633dc36c09a0ab0c9cb1feb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Wed, 5 Apr 2023 09:19:02 -0400 Subject: [PATCH 1/6] convert funnel property correlation table --- .../scenes/funnels/funnelCorrelationLogic.ts | 10 + .../funnels/funnelCorrelationUsageLogic.ts | 19 +- .../src/scenes/funnels/funnelLogic.test.ts | 168 +------ frontend/src/scenes/funnels/funnelLogic.ts | 260 +--------- .../funnelPropertyCorrelationLogic.test.ts | 446 ++++++++++++++++++ .../funnels/funnelPropertyCorrelationLogic.ts | 188 ++++++++ frontend/src/scenes/funnels/funnelUtils.ts | 17 + .../views/Funnels/CorrelationActionsCell.tsx | 10 +- .../FunnelPropertyCorrelationTable.tsx | 13 +- 9 files changed, 687 insertions(+), 444 deletions(-) create mode 100644 frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.test.ts create mode 100644 frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.ts diff --git a/frontend/src/scenes/funnels/funnelCorrelationLogic.ts b/frontend/src/scenes/funnels/funnelCorrelationLogic.ts index d59482e6aa57c..4d9d388817438 100644 --- a/frontend/src/scenes/funnels/funnelCorrelationLogic.ts +++ b/frontend/src/scenes/funnels/funnelCorrelationLogic.ts @@ -140,6 +140,7 @@ export const funnelCorrelationLogic = kea([ }, }), selectors({ + // apiParams for data exploration and legacy mode apiParams: [ (s) => [s.isUsingDataExploration, s.dataExplorationApiParams, s.legacyApiParams], (isUsingDataExploration, dataExplorationApiParams, legacyApiParams) => { @@ -162,6 +163,15 @@ export const funnelCorrelationLogic = kea([ return cleanedParams }, ], + // aggregationGroupTypeIndex for data exploration and legacy mode + aggregationGroupTypeIndex: [ + (s) => [s.isUsingDataExploration, s.querySource, s.filters], + (isUsingDataExploration, querySource, filters) => { + return isUsingDataExploration + ? querySource?.aggregation_group_type_index + : filters?.aggregation_group_type_index + }, + ], // event correlation correlationValues: [ diff --git a/frontend/src/scenes/funnels/funnelCorrelationUsageLogic.ts b/frontend/src/scenes/funnels/funnelCorrelationUsageLogic.ts index 02565ecbf2a78..dbf57ad57a65c 100644 --- a/frontend/src/scenes/funnels/funnelCorrelationUsageLogic.ts +++ b/frontend/src/scenes/funnels/funnelCorrelationUsageLogic.ts @@ -12,6 +12,7 @@ import { parseEventAndProperty } from './funnelUtils' import { funnelLogic } from './funnelLogic' import { funnelDataLogic } from './funnelDataLogic' import { funnelCorrelationLogic } from './funnelCorrelationLogic' +import { funnelPropertyCorrelationLogic } from './funnelPropertyCorrelationLogic' export const funnelCorrelationUsageLogic = kea([ props({} as InsightLogicProps), @@ -26,14 +27,12 @@ export const funnelCorrelationUsageLogic = kea( actions: [ insightLogic(props), ['loadResultsSuccess'], + insightDataLogic(props), + ['loadDataSuccess'], funnelLogic(props), - [ - 'hideSkewWarning as legacyHideSkewWarning', - 'setPropertyCorrelationTypes', - 'excludePropertyFromProject', - 'setPropertyNames', - 'openCorrelationPersonsModal', - ], + ['hideSkewWarning as legacyHideSkewWarning', 'openCorrelationPersonsModal'], + funnelDataLogic(props), + ['hideSkewWarning'], funnelCorrelationLogic(props), [ 'setCorrelationTypes', @@ -41,10 +40,8 @@ export const funnelCorrelationUsageLogic = kea( 'loadEventWithPropertyCorrelations', 'excludeEventPropertyFromProject', ], - funnelDataLogic(props), - ['hideSkewWarning'], - insightDataLogic(props), - ['loadDataSuccess'], + funnelPropertyCorrelationLogic(props), + ['setPropertyCorrelationTypes', 'excludePropertyFromProject', 'setPropertyNames'], eventUsageLogic, ['reportCorrelationViewed', 'reportCorrelationInteraction'], ], diff --git a/frontend/src/scenes/funnels/funnelLogic.test.ts b/frontend/src/scenes/funnels/funnelLogic.test.ts index 77234ecad6388..01f42ce027d87 100644 --- a/frontend/src/scenes/funnels/funnelLogic.test.ts +++ b/frontend/src/scenes/funnels/funnelLogic.test.ts @@ -1,20 +1,11 @@ -import { DEFAULT_EXCLUDED_PERSON_PROPERTIES, funnelLogic } from './funnelLogic' +import { funnelLogic } from './funnelLogic' import { MOCK_DEFAULT_TEAM, MOCK_TEAM_ID } from 'lib/api.mock' -import { expectLogic, partial } from 'kea-test-utils' +import { expectLogic } from 'kea-test-utils' import { initKeaTests } from '~/test/init' import { preflightLogic } from 'scenes/PreflightCheck/preflightLogic' import { eventUsageLogic } from 'lib/utils/eventUsageLogic' import { insightLogic } from 'scenes/insights/insightLogic' -import { - AvailableFeature, - CorrelationConfigType, - FunnelCorrelationResultsType, - FunnelsFilterType, - FunnelVizType, - InsightLogicProps, - InsightShortId, - InsightType, -} from '~/types' +import { AvailableFeature, CorrelationConfigType, InsightLogicProps, InsightShortId, InsightType } from '~/types' import { teamLogic } from 'scenes/teamLogic' import { userLogic } from 'scenes/userLogic' import { router } from 'kea-router' @@ -133,7 +124,7 @@ const funnelResults = [ describe('funnelLogic', () => { let logic: ReturnType - let correlationConfig: CorrelationConfigType = {} + const correlationConfig: CorrelationConfigType = {} beforeEach(() => { useAvailableFeatures([AvailableFeature.CORRELATION_ANALYSIS, AvailableFeature.GROUP_ANALYTICS]) @@ -619,157 +610,6 @@ describe('funnelLogic', () => { }) }) - describe('funnel correlation properties', () => { - const props = { dashboardItemId: Insight123, syncWithUrl: true } - - it('Selecting all properties returns expected result', async () => { - await initFunnelLogic(props) - await expectLogic(logic, () => logic.actions.setPropertyNames(logic.values.allProperties)) - .toFinishListeners() - .toMatchValues({ - propertyCorrelations: { - events: [ - { - event: { event: 'some property' }, - success_count: 1, - failure_count: 1, - odds_ratio: 1, - correlation_type: 'success', - result_type: FunnelCorrelationResultsType.Properties, - }, - { - event: { event: 'another property' }, - success_count: 1, - failure_count: 1, - odds_ratio: 1, - correlation_type: 'failure', - result_type: FunnelCorrelationResultsType.Properties, - }, - ], - }, - }) - }) - - it('Deselecting all returns empty result', async () => { - await initFunnelLogic(props) - await expectLogic(logic, () => logic.actions.setPropertyNames([])) - .toDispatchActions(logic, ['loadPropertyCorrelationsSuccess']) - .toMatchValues({ - propertyCorrelations: { - events: [], - }, - }) - }) - - it('are not triggered when results are loaded, when trends visualisation set', async () => { - await initFunnelLogic(props) - await expectLogic(logic, () => { - logic.actions.loadResultsSuccess({ - filters: { - insight: InsightType.FUNNELS, - funnel_viz_type: FunnelVizType.Trends, - } as FunnelsFilterType, - }) - }).toNotHaveDispatchedActions(['loadEventCorrelations', 'loadPropertyCorrelations']) - }) - - it('triggers update to correlation list when property excluded from project', async () => { - userLogic.mount() - await initFunnelLogic(props) - // - // // Make sure we have loaded the team already - // await expectLogic(teamLogic, () => teamLogic.actions.loadCurrentTeam()).toFinishAllListeners() - - await expectLogic(logic, () => { - logic.actions.setPropertyNames(logic.values.allProperties) - logic.actions.loadResultsSuccess({ filters: { insight: InsightType.FUNNELS } }) - logic.actions.excludePropertyFromProject('another property') - }) - .toFinishAllListeners() - .toMatchValues({ - propertyNames: ['some property', 'third property'], - excludedPropertyNames: DEFAULT_EXCLUDED_PERSON_PROPERTIES.concat(['another property']), - allProperties: ['some property', 'third property'], - }) - - expect(logic.values.propertyCorrelationValues).toEqual([ - { - event: { event: 'some property' }, - success_count: 1, - failure_count: 1, - odds_ratio: 1, - correlation_type: 'success', - result_type: FunnelCorrelationResultsType.Properties, - }, - ]) - }) - - it('isPropertyExcludedFromProject returns true initially, then false when excluded, and is persisted to team config', async () => { - await initFunnelLogic(props) - - expect(logic.values.isPropertyExcludedFromProject('some property')).toBe(false) - - await expectLogic(logic, () => - logic.actions.excludePropertyFromProject('some property') - ).toFinishAllListeners() - - expect(logic.values.isPropertyExcludedFromProject('some property')).toBe(true) - - await expectLogic(teamLogic).toMatchValues({ - currentTeam: partial({ - correlation_config: { - excluded_person_property_names: DEFAULT_EXCLUDED_PERSON_PROPERTIES.concat(['some property']), - }, - }), - }) - - // Also make sure that excluding the property again doesn't double - // up on the config list - await expectLogic(logic, () => - logic.actions.excludePropertyFromProject('some property') - ).toFinishAllListeners() - - await expectLogic(teamLogic).toMatchValues({ - currentTeam: partial({ - correlation_config: { - excluded_person_property_names: DEFAULT_EXCLUDED_PERSON_PROPERTIES.concat(['some property']), - }, - }), - }) - }) - - it('loads property exclude list from Project settings', async () => { - correlationConfig = { excluded_person_property_names: ['some property'] } - await initFunnelLogic(props) - - await expectLogic(teamLogic).toMatchValues({ - currentTeam: partial({ - correlation_config: { excluded_person_property_names: ['some property'] }, - }), - }) - - await expectLogic(logic, () => { - logic.actions.setPropertyNames(logic.values.allProperties) - logic.actions.loadResultsSuccess({ filters: { insight: InsightType.FUNNELS } }) - }) - .toFinishAllListeners() - .toMatchValues({ - propertyCorrelations: { - events: [ - { - event: { event: 'another property' }, - success_count: 1, - failure_count: 1, - odds_ratio: 1, - correlation_type: 'failure', - result_type: FunnelCorrelationResultsType.Properties, - }, - ], - }, - }) - }) - }) - describe('funnel simple vs. advanced mode', () => { beforeEach(async () => { await initFunnelLogic() diff --git a/frontend/src/scenes/funnels/funnelLogic.ts b/frontend/src/scenes/funnels/funnelLogic.ts index 6f9a7a0596851..bc21c97e20dcc 100644 --- a/frontend/src/scenes/funnels/funnelLogic.ts +++ b/frontend/src/scenes/funnels/funnelLogic.ts @@ -1,21 +1,16 @@ import { kea } from 'kea' import equal from 'fast-deep-equal' -import api from 'lib/api' import { insightLogic } from 'scenes/insights/insightLogic' import { average, percentage, sum } from 'lib/utils' import type { funnelLogicType } from './funnelLogicType' import { - AnyPropertyFilter, BinCountValue, - CorrelationConfigType, - ElementPropertyFilter, FilterType, FlattenedFunnelStepByBreakdown, FunnelResultType, FunnelConversionWindowTimeUnit, FunnelCorrelation, FunnelCorrelationResultsType, - FunnelCorrelationType, FunnelsFilterType, FunnelStep, FunnelStepRangeEntityFilter, @@ -28,8 +23,6 @@ import { HistogramGraphDatum, InsightLogicProps, InsightType, - PropertyFilterType, - PropertyOperator, StepOrderValue, TrendResult, } from '~/types' @@ -49,45 +42,18 @@ import { stepsWithConversionMetrics, flattenedStepsByBreakdown, generateBaselineConversionUrl, + parseBreakdownValue, + parseEventAndProperty, } from './funnelUtils' import { dashboardsModel } from '~/models/dashboardsModel' import { cleanFilters } from 'scenes/insights/utils/cleanFilters' import { isFunnelsFilter, keyForInsightLogicProps } from 'scenes/insights/sharedUtils' -import { teamLogic } from '../teamLogic' -import { personPropertiesModel } from '~/models/personPropertiesModel' -import { groupPropertiesModel } from '~/models/groupPropertiesModel' -import { elementsToAction } from 'scenes/events/createActionFromEvent' import { groupsModel, Noun } from '~/models/groupsModel' import { dayjs } from 'lib/dayjs' -import { lemonToast } from 'lib/lemon-ui/lemonToast' import { LemonSelectOptions } from 'lib/lemon-ui/LemonSelect' import { openPersonsModal } from 'scenes/trends/persons-modal/PersonsModal' import { funnelTitle } from 'scenes/trends/persons-modal/persons-modal-utils' -// List of events that should be excluded, if we don't have an explicit list of -// excluded properties. Copied from -// https://github.com/PostHog/posthog/issues/6474#issuecomment-952044722 -export const DEFAULT_EXCLUDED_PERSON_PROPERTIES = [ - '$initial_geoip_postal_code', - '$initial_geoip_latitude', - '$initial_geoip_longitude', - '$geoip_latitude', - '$geoip_longitude', - '$geoip_postal_code', - '$geoip_continent_code', - '$geoip_continent_name', - '$initial_geoip_continent_code', - '$initial_geoip_continent_name', - '$geoip_time_zone', - '$geoip_country_code', - '$geoip_subdivision_1_code', - '$initial_geoip_subdivision_1_code', - '$geoip_subdivision_2_code', - '$initial_geoip_subdivision_2_code', - '$geoip_subdivision_name', - '$initial_geoip_subdivision_name', -] - export type OpenPersonsModelProps = { step: FunnelStep stepIndex?: number @@ -103,14 +69,8 @@ export const funnelLogic = kea({ values: [ insightLogic(props), ['filters as inflightFilters', 'insight', 'isInDashboardContext', 'hiddenLegendKeys'], - teamLogic, - ['currentTeamId', 'currentTeam'], - personPropertiesModel, - ['personProperties'], groupsModel, ['aggregationLabel'], - groupPropertiesModel, - ['groupProperties'], ], actions: [insightLogic(props), ['loadResults', 'loadResultsSuccess', 'toggleVisibility']], logic: [dashboardsModel], @@ -177,53 +137,7 @@ export const funnelLogic = kea({ correlation, success, }), - setPropertyCorrelationTypes: (types: FunnelCorrelationType[]) => ({ types }), - setPropertyNames: (propertyNames: string[]) => ({ propertyNames }), - excludePropertyFromProject: (propertyName: string) => ({ propertyName }), - }), - defaults: { - // This is a hack to get `FunnelCorrelationResultsType` imported in `funnelLogicType.ts` - __ignore: null as FunnelCorrelationResultsType | null, - }, - loaders: ({ values }) => ({ - propertyCorrelations: [ - { events: [] } as Record<'events', FunnelCorrelation[]>, - { - loadPropertyCorrelations: async (_, breakpoint) => { - const targetProperties = - values.propertyNames.length >= values.allProperties.length ? ['$all'] : values.propertyNames - - if (targetProperties.length === 0) { - return { events: [] } - } - - await breakpoint(100) - - try { - const results: Omit[] = ( - await api.create(`api/projects/${values.currentTeamId}/insights/funnel/correlation`, { - ...values.apiParams, - funnel_correlation_type: 'properties', - funnel_correlation_names: targetProperties, - funnel_correlation_exclude_names: values.excludedPropertyNames, - }) - ).result?.events - - return { - events: results.map((result) => ({ - ...result, - result_type: FunnelCorrelationResultsType.Properties, - })), - } - } catch (error) { - lemonToast.error('Failed to load correlation results', { toastId: 'funnel-correlation-error' }) - return { events: [] } - } - }, - }, - ], }), - reducers: ({ props }) => ({ people: { clearFunnel: () => [], @@ -242,27 +156,12 @@ export const funnelLogic = kea({ [insightLogic(props).actionTypes.abortQuery]: (_: any, { exception }: any) => exception ?? null, }, ], - propertyCorrelationTypes: [ - [FunnelCorrelationType.Success, FunnelCorrelationType.Failure] as FunnelCorrelationType[], - { - setPropertyCorrelationTypes: (_, { types }) => types, - }, - ], skewWarningHidden: [ false, { hideSkewWarning: () => true, }, ], - propertyNames: [ - [] as string[], - { - setPropertyNames: (_, { propertyNames }) => propertyNames, - excludePropertyFromProject: (selectedProperties, { propertyName }) => { - return selectedProperties.filter((p) => p !== propertyName) - }, - }, - ], isTooltipShown: [ false, { @@ -282,12 +181,6 @@ export const funnelLogic = kea({ showTooltip: (_, { origin }) => origin, }, ], - loadedPropertyCorrelationsTableOnce: [ - false, - { - loadPropertyCorrelations: () => true, - }, - ], }), selectors: ({ selectors }) => ({ @@ -573,62 +466,10 @@ export const funnelLogic = kea({ return !(e?.status === 400 && e?.type === 'validation_error') }, ], - propertyCorrelationValues: [ - () => [selectors.propertyCorrelations, selectors.propertyCorrelationTypes, selectors.excludedPropertyNames], - (propertyCorrelations, propertyCorrelationTypes, excludedPropertyNames): FunnelCorrelation[] => { - return propertyCorrelations.events - .filter( - (correlation) => - propertyCorrelationTypes.includes(correlation.correlation_type) && - !excludedPropertyNames.includes(correlation.event.event.split('::')[0]) - ) - .map((value) => { - return { - ...value, - odds_ratio: - value.correlation_type === FunnelCorrelationType.Success - ? value.odds_ratio - : 1 / value.odds_ratio, - } - }) - .sort((first, second) => { - return second.odds_ratio - first.odds_ratio - }) - }, - ], disableFunnelBreakdownBaseline: [ () => [(_, props) => props], (props: InsightLogicProps): boolean => !!props.cachedInsight?.disable_baseline, ], - - isPropertyExcludedFromProject: [ - () => [selectors.excludedPropertyNames], - (excludedPropertyNames) => (propertyName: string) => - excludedPropertyNames.find((name) => name === propertyName) !== undefined, - ], - excludedPropertyNames: [ - () => [selectors.currentTeam], - (currentTeam): string[] => - currentTeam?.correlation_config?.excluded_person_property_names || DEFAULT_EXCLUDED_PERSON_PROPERTIES, - ], - inversePropertyNames: [ - (s) => [s.filters, s.personProperties, s.groupProperties], - (filters, personProperties, groupProperties) => (excludedPersonProperties: string[]) => { - const targetProperties = - filters.aggregation_group_type_index !== undefined - ? groupProperties(filters.aggregation_group_type_index) - : personProperties - return targetProperties - .map((property) => property.name) - .filter((property) => !excludedPersonProperties.includes(property)) - }, - ], - allProperties: [ - (s) => [s.inversePropertyNames, s.excludedPropertyNames], - (inversePropertyNames, excludedPropertyNames): string[] => { - return inversePropertyNames(excludedPropertyNames || []) - }, - ], aggregationTargetLabel: [ (s) => [s.filters, s.aggregationLabel], (filters, aggregationLabel): Noun => aggregationLabel(filters.aggregation_group_type_index), @@ -797,102 +638,5 @@ export const funnelLogic = kea({ toggleAdvancedMode: () => { actions.setFilters({ funnel_advanced: !values.filters.funnel_advanced }) }, - excludePropertyFromProject: ({ propertyName }) => { - appendToCorrelationConfig('excluded_person_property_names', values.excludedPropertyNames, propertyName) - }, - setPropertyNames: async () => { - actions.loadPropertyCorrelations({}) - }, }), }) - -const appendToCorrelationConfig = ( - configKey: keyof CorrelationConfigType, - currentValue: string[], - configValue: string -): void => { - // Helper to handle updating correlationConfig within the Team model. Only - // handles further appending to current values. - - // When we exclude a property, we want to update the config stored - // on the current Team/Project. - const oldCurrentTeam = teamLogic.values.currentTeam - - // If we haven't actually retrieved the current team, we can't - // update the config. - if (oldCurrentTeam === null || !currentValue) { - console.warn('Attempt to update correlation config without first retrieving existing config') - return - } - - const oldCorrelationConfig = oldCurrentTeam.correlation_config - - const configList = [...Array.from(new Set(currentValue.concat([configValue])))] - - const correlationConfig = { - ...oldCorrelationConfig, - [configKey]: configList, - } - - teamLogic.actions.updateCurrentTeam({ - correlation_config: correlationConfig, - }) -} - -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], - } - } -} - -const parseEventAndProperty = ( - event: FunnelCorrelation['event'] -): { - name: string - properties?: AnyPropertyFilter[] -} => { - const components = event.event.split('::') - /* - The `event` is either an event name, or event::property::property_value - */ - if (components.length === 1) { - return { name: components[0] } - } else if (components[0] === '$autocapture') { - // We use elementsToAction to generate the required property filters - const elementData = elementsToAction(event.elements) - return { - name: components[0], - properties: Object.entries(elementData) - .filter(([, propertyValue]) => !!propertyValue) - .map(([propertyKey, propertyValue]) => ({ - key: propertyKey as ElementPropertyFilter['key'], - operator: PropertyOperator.Exact, - type: PropertyFilterType.Element, - value: [propertyValue as string], - })), - } - } else { - return { - name: components[0], - properties: [ - { - key: components[1], - operator: PropertyOperator.Exact, - value: components[2], - type: PropertyFilterType.Event, - }, - ], - } - } -} diff --git a/frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.test.ts b/frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.test.ts new file mode 100644 index 0000000000000..46fec17cf9709 --- /dev/null +++ b/frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.test.ts @@ -0,0 +1,446 @@ +import { expectLogic, partial } from 'kea-test-utils' +import { MOCK_DEFAULT_TEAM } from 'lib/api.mock' +import { teamLogic } from 'scenes/teamLogic' +import { userLogic } from 'scenes/userLogic' +import { useAvailableFeatures } from '~/mocks/features' +import { useMocks } from '~/mocks/jest' +import { initKeaTests } from '~/test/init' +import { + AvailableFeature, + CorrelationConfigType, + FunnelCorrelationResultsType, + InsightLogicProps, + InsightShortId, + InsightType, +} from '~/types' +import { DEFAULT_EXCLUDED_PERSON_PROPERTIES, funnelPropertyCorrelationLogic } from './funnelPropertyCorrelationLogic' + +const Insight12 = '12' as InsightShortId +const Insight123 = '123' as InsightShortId + +export const mockInsight = { + id: Insight123, + short_id: 'SvoU2bMC', + name: null, + filters: { + breakdown: null, + breakdown_type: null, + display: 'FunnelViz', + events: [ + { + id: '$pageview', + type: 'events', + order: 0, + name: '$pageview', + custom_name: null, + math: null, + math_property: null, + properties: [], + }, + { + id: '$pageview', + type: 'events', + order: 1, + name: '$pageview', + custom_name: null, + math: null, + math_property: null, + properties: [], + }, + { + id: '$pageview', + type: 'events', + order: 2, + name: '$pageview', + custom_name: null, + math: null, + math_property: null, + properties: [], + }, + { + id: '$pageview', + type: 'events', + order: 3, + name: '$pageview', + custom_name: null, + math: null, + math_property: null, + properties: [], + }, + ], + funnel_from_step: 0, + funnel_to_step: 1, + funnel_viz_type: 'steps', + insight: 'FUNNELS', + layout: 'vertical', + }, + order: null, + deleted: false, + dashboard: null, + layouts: {}, + color: null, + last_refresh: null, + result: null, + created_at: '2021-09-22T18:22:20.036153Z', + description: null, + updated_at: '2021-09-22T19:03:49.322258Z', + tags: [], + favorited: false, + saved: false, + created_by: { + id: 1, + uuid: '017c0441-bcb2-0000-bccf-dfc24328c5f3', + distinct_id: 'fM7b6ZFi8MOssbkDI55ot8tMY2hkzrHdRy1qERa6rCK', + first_name: 'Alex', + email: 'alex@posthog.com', + }, +} + +const funnelResults = [ + { + action_id: '$pageview', + count: 19, + name: '$pageview', + order: 0, + type: 'events', + }, + { + action_id: '$pageview', + count: 7, + name: '$pageview', + order: 1, + type: 'events', + }, + { + action_id: '$pageview', + count: 4, + name: '$pageview', + order: 2, + type: 'events', + }, +] + +describe('funnelPropertyCorrelationLogic', () => { + const props = { dashboardItemId: Insight123, syncWithUrl: true } + let logic: ReturnType + let correlationConfig: CorrelationConfigType = {} + + beforeEach(() => { + useAvailableFeatures([AvailableFeature.CORRELATION_ANALYSIS, AvailableFeature.GROUP_ANALYTICS]) + useMocks({ + get: { + '/api/projects/@current': () => [ + 200, + { + ...MOCK_DEFAULT_TEAM, + correlation_config: correlationConfig, + }, + ], + '/api/projects/:team/insights/': (req) => { + if (req.url.searchParams.get('saved')) { + return [ + 200, + { + results: funnelResults, + }, + ] + } + const shortId = req.url.searchParams.get('short_id') || '' + if (shortId === '500') { + return [500, { status: 0, detail: 'error from the API' }] + } + return [ + 200, + { + results: [mockInsight], + }, + ] + }, + '/api/projects/:team/insights/trend/': { results: ['trends result from api'] }, + '/api/projects/:team/groups_types/': [], + '/some/people/url': { results: [{ people: [] }] }, + '/api/projects/:team/persons/funnel': { results: [], next: null }, + '/api/projects/:team/persons/properties': [ + { name: 'some property', count: 20 }, + { name: 'another property', count: 10 }, + { name: 'third property', count: 5 }, + ], + '/api/projects/:team/groups/property_definitions': { + '0': [ + { name: 'industry', count: 2 }, + { name: 'name', count: 1 }, + ], + '1': [{ name: 'name', count: 1 }], + }, + }, + patch: { + '/api/projects/:id': (req) => [ + 200, + { + ...MOCK_DEFAULT_TEAM, + correlation_config: { + ...correlationConfig, + excluded_person_property_names: (req.body as any)?.correlation_config + ?.excluded_person_property_names, + }, + }, + ], + }, + post: { + '/api/projects/:team/insights/': (req) => [ + 200, + { id: 12, short_id: Insight12, ...((req.body as any) || {}) }, + ], + '/api/projects/:team/insights/:id/viewed': [201], + '/api/projects/:team/insights/funnel/': { + is_cached: true, + last_refresh: '2021-09-16T13:41:41.297295Z', + result: funnelResults, + type: 'Funnel', + }, + '/api/projects/:team/insights/funnel/correlation': (req) => { + const data = req.body as any + if (data?.funnel_correlation_type === 'properties') { + const excludePropertyFromProjectNames = data?.funnel_correlation_exclude_names || [] + const includePropertyNames = data?.funnel_correlation_names || [] + return [ + 200, + { + is_cached: true, + last_refresh: '2021-09-16T13:41:41.297295Z', + result: { + events: [ + { + event: { event: 'some property' }, + success_count: 1, + failure_count: 1, + odds_ratio: 1, + correlation_type: 'success', + }, + { + event: { event: 'another property' }, + success_count: 1, + failure_count: 1, + odds_ratio: 1, + correlation_type: 'failure', + }, + ] + .filter( + (correlation) => + includePropertyNames.includes('$all') || + includePropertyNames.includes(correlation.event.event) + ) + .filter( + (correlation) => + !excludePropertyFromProjectNames.includes(correlation.event.event) + ), + }, + type: 'Funnel', + }, + ] + } else if (data?.funnel_correlation_type === 'events') { + return [ + 200, + { + is_cached: true, + last_refresh: '2021-09-16T13:41:41.297295Z', + result: { + events: [ + { + event: { event: 'some event' }, + success_count: 1, + failure_count: 1, + odds_ratio: 1, + correlation_type: 'success', + }, + { + event: { event: 'another event' }, + success_count: 1, + failure_count: 1, + odds_ratio: 1, + correlation_type: 'failure', + }, + ], + }, + type: 'Funnel', + }, + ] + } else if (data?.funnel_correlation_type === 'event_with_properties') { + const targetEvent = data?.funnel_correlation_event_names[0] + const excludedProperties = data?.funnel_correlation_event_exclude_property_names + return [ + 200, + { + result: { + events: [ + { + success_count: 1, + failure_count: 0, + odds_ratio: 29, + correlation_type: 'success', + event: { event: `some event::name::Hester` }, + }, + { + success_count: 1, + failure_count: 0, + odds_ratio: 29, + correlation_type: 'success', + event: { event: `some event::Another name::Alice` }, + }, + { + success_count: 1, + failure_count: 0, + odds_ratio: 25, + correlation_type: 'success', + event: { event: `another event::name::Aloha` }, + }, + { + success_count: 1, + failure_count: 0, + odds_ratio: 25, + correlation_type: 'success', + event: { event: `another event::Another name::Bob` }, + }, + ].filter( + (record) => + record.event.event.split('::')[0] === targetEvent && + !excludedProperties.includes(record.event.event.split('::')[1]) + ), + last_refresh: '2021-11-05T09:26:16.175923Z', + is_cached: false, + }, + }, + ] + } + }, + }, + }) + initKeaTests(false) + // window.POSTHOG_APP_CONTEXT = undefined // to force API request to /api/project/@current + }) + + const defaultProps: InsightLogicProps = { + dashboardItemId: undefined, + cachedInsight: { + short_id: undefined, + filters: { + insight: InsightType.FUNNELS, + actions: [ + { id: '$pageview', order: 0 }, + { id: '$pageview', order: 1 }, + ], + }, + result: null, + }, + } + + async function initPropertyFunnelCorrelationLogic(props: InsightLogicProps = defaultProps): Promise { + teamLogic.mount() + await expectLogic(teamLogic).toFinishAllListeners() + userLogic.mount() + await expectLogic(userLogic).toFinishAllListeners() + logic = funnelPropertyCorrelationLogic(props) + logic.mount() + await expectLogic(logic).toFinishAllListeners() + } + + it('Selecting all properties returns expected result', async () => { + await initPropertyFunnelCorrelationLogic(props) + await expectLogic(logic, () => logic.actions.setPropertyNames(logic.values.allProperties)) + .toFinishListeners() + .toMatchValues({ + propertyCorrelations: { + events: [ + { + event: { event: 'some property' }, + success_count: 1, + failure_count: 1, + odds_ratio: 1, + correlation_type: 'success', + result_type: FunnelCorrelationResultsType.Properties, + }, + { + event: { event: 'another property' }, + success_count: 1, + failure_count: 1, + odds_ratio: 1, + correlation_type: 'failure', + result_type: FunnelCorrelationResultsType.Properties, + }, + ], + }, + }) + }) + + it('Deselecting all returns empty result', async () => { + await initPropertyFunnelCorrelationLogic(props) + await expectLogic(logic, () => logic.actions.setPropertyNames([])) + .toDispatchActions(logic, ['loadPropertyCorrelationsSuccess']) + .toMatchValues({ + propertyCorrelations: { + events: [], + }, + }) + }) + + it('isPropertyExcludedFromProject returns true initially, then false when excluded, and is persisted to team config', async () => { + await initPropertyFunnelCorrelationLogic(props) + + expect(logic.values.isPropertyExcludedFromProject('some property')).toBe(false) + + await expectLogic(logic, () => logic.actions.excludePropertyFromProject('some property')).toFinishAllListeners() + + expect(logic.values.isPropertyExcludedFromProject('some property')).toBe(true) + + await expectLogic(teamLogic).toMatchValues({ + currentTeam: partial({ + correlation_config: { + excluded_person_property_names: DEFAULT_EXCLUDED_PERSON_PROPERTIES.concat(['some property']), + }, + }), + }) + + // Also make sure that excluding the property again doesn't double + // up on the config list + await expectLogic(logic, () => logic.actions.excludePropertyFromProject('some property')).toFinishAllListeners() + + await expectLogic(teamLogic).toMatchValues({ + currentTeam: partial({ + correlation_config: { + excluded_person_property_names: DEFAULT_EXCLUDED_PERSON_PROPERTIES.concat(['some property']), + }, + }), + }) + }) + + it('loads property exclude list from Project settings', async () => { + correlationConfig = { excluded_person_property_names: ['some property'] } + await initPropertyFunnelCorrelationLogic(props) + + await expectLogic(teamLogic).toMatchValues({ + currentTeam: partial({ + correlation_config: { excluded_person_property_names: ['some property'] }, + }), + }) + + await expectLogic(logic, () => { + logic.actions.setPropertyNames(logic.values.allProperties) + // logic.actions.loadResultsSuccess({ filters: { insight: InsightType.FUNNELS } }) + }) + .toFinishAllListeners() + .toMatchValues({ + propertyCorrelations: { + events: [ + { + event: { event: 'another property' }, + success_count: 1, + failure_count: 1, + odds_ratio: 1, + correlation_type: 'failure', + result_type: FunnelCorrelationResultsType.Properties, + }, + ], + }, + }) + }) +}) diff --git a/frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.ts b/frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.ts new file mode 100644 index 0000000000000..74f9c53fc5d6d --- /dev/null +++ b/frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.ts @@ -0,0 +1,188 @@ +import { kea, props, key, path, selectors, listeners, connect, reducers, actions, defaults } from 'kea' +import { loaders } from 'kea-loaders' + +import { teamLogic } from '../teamLogic' +import { personPropertiesModel } from '~/models/personPropertiesModel' +import { groupPropertiesModel } from '~/models/groupPropertiesModel' + +import { FunnelCorrelation, FunnelCorrelationResultsType, FunnelCorrelationType, InsightLogicProps } from '~/types' +import { keyForInsightLogicProps } from 'scenes/insights/sharedUtils' +import { appendToCorrelationConfig } from './funnelUtils' +import api from 'lib/api' +import { lemonToast } from '@posthog/lemon-ui' + +import type { funnelPropertyCorrelationLogicType } from './funnelPropertyCorrelationLogicType' +import { funnelCorrelationLogic } from './funnelCorrelationLogic' + +// List of events that should be excluded, if we don't have an explicit list of +// excluded properties. Copied from +// https://github.com/PostHog/posthog/issues/6474#issuecomment-952044722 +export const DEFAULT_EXCLUDED_PERSON_PROPERTIES = [ + '$initial_geoip_postal_code', + '$initial_geoip_latitude', + '$initial_geoip_longitude', + '$geoip_latitude', + '$geoip_longitude', + '$geoip_postal_code', + '$geoip_continent_code', + '$geoip_continent_name', + '$initial_geoip_continent_code', + '$initial_geoip_continent_name', + '$geoip_time_zone', + '$geoip_country_code', + '$geoip_subdivision_1_code', + '$initial_geoip_subdivision_1_code', + '$geoip_subdivision_2_code', + '$initial_geoip_subdivision_2_code', + '$geoip_subdivision_name', + '$initial_geoip_subdivision_name', +] + +export const funnelPropertyCorrelationLogic = kea([ + props({} as InsightLogicProps), + key(keyForInsightLogicProps('insight_funnel')), + path((key) => ['scenes', 'funnels', 'funnelCorrelationLogic', key]), + connect((props: InsightLogicProps) => ({ + values: [ + funnelCorrelationLogic(props), + ['apiParams', 'aggregationGroupTypeIndex'], + teamLogic, + ['currentTeamId', 'currentTeam'], + personPropertiesModel, + ['personProperties'], + groupPropertiesModel, + ['groupProperties'], + ], + })), + actions({ + setPropertyCorrelationTypes: (types: FunnelCorrelationType[]) => ({ types }), + setPropertyNames: (propertyNames: string[]) => ({ propertyNames }), + excludePropertyFromProject: (propertyName: string) => ({ propertyName }), + }), + defaults({ + // This is a hack to get `FunnelCorrelationResultsType` imported in `funnelCorrelationLogicType.ts` + __ignore: null as FunnelCorrelationResultsType | null, + }), + loaders(({ values }) => ({ + propertyCorrelations: [ + { events: [] } as Record<'events', FunnelCorrelation[]>, + { + loadPropertyCorrelations: async (_, breakpoint) => { + const targetProperties = + values.propertyNames.length >= values.allProperties.length ? ['$all'] : values.propertyNames + + if (targetProperties.length === 0) { + return { events: [] } + } + + await breakpoint(100) + + try { + const results: Omit[] = ( + await api.create(`api/projects/${values.currentTeamId}/insights/funnel/correlation`, { + ...values.apiParams, + funnel_correlation_type: 'properties', + funnel_correlation_names: targetProperties, + funnel_correlation_exclude_names: values.excludedPropertyNames, + }) + ).result?.events + + return { + events: results.map((result) => ({ + ...result, + result_type: FunnelCorrelationResultsType.Properties, + })), + } + } catch (error) { + lemonToast.error('Failed to load correlation results', { toastId: 'funnel-correlation-error' }) + return { events: [] } + } + }, + }, + ], + })), + reducers({ + propertyCorrelationTypes: [ + [FunnelCorrelationType.Success, FunnelCorrelationType.Failure] as FunnelCorrelationType[], + { + setPropertyCorrelationTypes: (_, { types }) => types, + }, + ], + propertyNames: [ + [] as string[], + { + setPropertyNames: (_, { propertyNames }) => propertyNames, + excludePropertyFromProject: (selectedProperties, { propertyName }) => { + return selectedProperties.filter((p) => p !== propertyName) + }, + }, + ], + loadedPropertyCorrelationsTableOnce: [ + false, + { + loadPropertyCorrelations: () => true, + }, + ], + }), + selectors({ + propertyCorrelationValues: [ + (s) => [s.propertyCorrelations, s.propertyCorrelationTypes, s.excludedPropertyNames], + (propertyCorrelations, propertyCorrelationTypes, excludedPropertyNames): FunnelCorrelation[] => { + return propertyCorrelations.events + .filter( + (correlation) => + propertyCorrelationTypes.includes(correlation.correlation_type) && + !excludedPropertyNames.includes(correlation.event.event.split('::')[0]) + ) + .map((value) => { + return { + ...value, + odds_ratio: + value.correlation_type === FunnelCorrelationType.Success + ? value.odds_ratio + : 1 / value.odds_ratio, + } + }) + .sort((first, second) => { + return second.odds_ratio - first.odds_ratio + }) + }, + ], + excludedPropertyNames: [ + (s) => [s.currentTeam], + (currentTeam): string[] => + currentTeam?.correlation_config?.excluded_person_property_names || DEFAULT_EXCLUDED_PERSON_PROPERTIES, + ], + isPropertyExcludedFromProject: [ + (s) => [s.excludedPropertyNames], + (excludedPropertyNames) => (propertyName: string) => + excludedPropertyNames.find((name) => name === propertyName) !== undefined, + ], + inversePropertyNames: [ + (s) => [s.aggregationGroupTypeIndex, s.personProperties, s.groupProperties], + (aggregationGroupTypeIndex, personProperties, groupProperties) => (excludedPersonProperties: string[]) => { + const targetProperties = + aggregationGroupTypeIndex !== undefined + ? groupProperties(aggregationGroupTypeIndex) + : personProperties + return targetProperties + .map((property) => property.name) + .filter((property) => !excludedPersonProperties.includes(property)) + }, + ], + allProperties: [ + (s) => [s.inversePropertyNames, s.excludedPropertyNames], + (inversePropertyNames, excludedPropertyNames): string[] => { + return inversePropertyNames(excludedPropertyNames || []) + }, + ], + }), + listeners(({ actions, values }) => ({ + excludePropertyFromProject: ({ propertyName }) => { + appendToCorrelationConfig('excluded_person_property_names', values.excludedPropertyNames, propertyName) + }, + setPropertyNames: async () => { + actions.loadPropertyCorrelations({}) + }, + })), +]) diff --git a/frontend/src/scenes/funnels/funnelUtils.ts b/frontend/src/scenes/funnels/funnelUtils.ts index 21df24db74ea8..312bd2e50ba3a 100644 --- a/frontend/src/scenes/funnels/funnelUtils.ts +++ b/frontend/src/scenes/funnels/funnelUtils.ts @@ -524,6 +524,23 @@ export const transformLegacyHiddenLegendKeys = ( return hiddenLegendKeys } +export 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], + } + } +} + export const parseEventAndProperty = ( event: FunnelCorrelation['event'] ): { diff --git a/frontend/src/scenes/insights/views/Funnels/CorrelationActionsCell.tsx b/frontend/src/scenes/insights/views/Funnels/CorrelationActionsCell.tsx index bd17085d432d7..bd4b4dcaba5ba 100644 --- a/frontend/src/scenes/insights/views/Funnels/CorrelationActionsCell.tsx +++ b/frontend/src/scenes/insights/views/Funnels/CorrelationActionsCell.tsx @@ -2,14 +2,14 @@ import { useState } from 'react' import { useActions, useValues } from 'kea' import { insightLogic } from 'scenes/insights/insightLogic' -import { funnelLogic } from 'scenes/funnels/funnelLogic' +import { funnelCorrelationLogic } from 'scenes/funnels/funnelCorrelationLogic' +import { funnelCorrelationDetailsLogic } from 'scenes/funnels/funnelCorrelationDetailsLogic' +import { funnelPropertyCorrelationLogic } from 'scenes/funnels/funnelPropertyCorrelationLogic' import { FunnelCorrelation, FunnelCorrelationResultsType } from '~/types' import { Popover } from 'lib/lemon-ui/Popover/Popover' import { LemonButton, LemonButtonProps } from 'lib/lemon-ui/LemonButton' import { IconEllipsis } from 'lib/lemon-ui/icons' -import { funnelCorrelationLogic } from 'scenes/funnels/funnelCorrelationLogic' -import { funnelCorrelationDetailsLogic } from 'scenes/funnels/funnelCorrelationDetailsLogic' export const EventCorrelationActionsCell = ({ record }: { record: FunnelCorrelation }): JSX.Element => { const { insightProps } = useValues(insightLogic) @@ -49,8 +49,8 @@ export const EventCorrelationActionsCell = ({ record }: { record: FunnelCorrelat export const PropertyCorrelationActionsCell = ({ record }: { record: FunnelCorrelation }): JSX.Element => { const { insightProps } = useValues(insightLogic) - const { excludePropertyFromProject } = useActions(funnelLogic(insightProps)) - const { isPropertyExcludedFromProject } = useValues(funnelLogic(insightProps)) + const { isPropertyExcludedFromProject } = useValues(funnelPropertyCorrelationLogic(insightProps)) + const { excludePropertyFromProject } = useActions(funnelPropertyCorrelationLogic(insightProps)) const { setFunnelCorrelationDetails } = useActions(funnelCorrelationDetailsLogic(insightProps)) const propertyName = (record.event.event || '').split('::')[0] diff --git a/frontend/src/scenes/insights/views/Funnels/FunnelPropertyCorrelationTable.tsx b/frontend/src/scenes/insights/views/Funnels/FunnelPropertyCorrelationTable.tsx index e9ad12a048724..373fa3b0db0bd 100644 --- a/frontend/src/scenes/insights/views/Funnels/FunnelPropertyCorrelationTable.tsx +++ b/frontend/src/scenes/insights/views/Funnels/FunnelPropertyCorrelationTable.tsx @@ -19,11 +19,13 @@ import { FunnelCorrelationTableEmptyState } from './FunnelCorrelationTableEmptyS import { PropertyCorrelationActionsCell } from './CorrelationActionsCell' import { funnelCorrelationUsageLogic } from 'scenes/funnels/funnelCorrelationUsageLogic' import { parseDisplayNameForCorrelation } from 'scenes/funnels/funnelUtils' +import { funnelPropertyCorrelationLogic } from 'scenes/funnels/funnelPropertyCorrelationLogic' export function FunnelPropertyCorrelationTable(): JSX.Element | null { const { insightProps } = useValues(insightLogic) + const { steps, filters, aggregationTargetLabel } = useValues(funnelLogic(insightProps)) + const { openCorrelationPersonsModal } = useActions(funnelLogic(insightProps)) const { - steps, propertyCorrelationValues, propertyCorrelationTypes, excludedPropertyNames, @@ -31,12 +33,11 @@ export function FunnelPropertyCorrelationTable(): JSX.Element | null { inversePropertyNames, propertyNames, allProperties, - filters, - aggregationTargetLabel, loadedPropertyCorrelationsTableOnce, - } = useValues(funnelLogic(insightProps)) - const { setPropertyCorrelationTypes, setPropertyNames, openCorrelationPersonsModal, loadPropertyCorrelations } = - useActions(funnelLogic(insightProps)) + } = useValues(funnelPropertyCorrelationLogic(insightProps)) + const { setPropertyCorrelationTypes, setPropertyNames, loadPropertyCorrelations } = useActions( + funnelPropertyCorrelationLogic(insightProps) + ) const { correlationPropKey } = useValues(funnelCorrelationUsageLogic(insightProps)) const { reportCorrelationInteraction } = useActions(funnelCorrelationUsageLogic(insightProps)) From 729b8e1c0a9b446225e90ad977b065fd926253b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Wed, 5 Apr 2023 09:32:50 -0400 Subject: [PATCH 2/6] fix connected logic --- frontend/src/scenes/funnels/funnelCorrelationUsageLogic.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/frontend/src/scenes/funnels/funnelCorrelationUsageLogic.ts b/frontend/src/scenes/funnels/funnelCorrelationUsageLogic.ts index dbf57ad57a65c..a603cfe51c75c 100644 --- a/frontend/src/scenes/funnels/funnelCorrelationUsageLogic.ts +++ b/frontend/src/scenes/funnels/funnelCorrelationUsageLogic.ts @@ -22,7 +22,12 @@ export const funnelCorrelationUsageLogic = kea( connect((props: InsightLogicProps) => ({ logic: [eventUsageLogic], - values: [insightLogic(props), ['filters', 'isInDashboardContext'], funnelLogic(props), ['allProperties']], + values: [ + insightLogic(props), + ['filters', 'isInDashboardContext'], + funnelPropertyCorrelationLogic(props), + ['allProperties'], + ], actions: [ insightLogic(props), From b12b59785da4d836dd2bd6e4f4d4bdf18abf7255 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Wed, 5 Apr 2023 11:21:45 -0400 Subject: [PATCH 3/6] tests --- .../src/scenes/funnels/funnelLogic.test.ts | 265 +--------------- .../funnelPropertyCorrelationLogic.test.ts | 287 +++--------------- 2 files changed, 43 insertions(+), 509 deletions(-) diff --git a/frontend/src/scenes/funnels/funnelLogic.test.ts b/frontend/src/scenes/funnels/funnelLogic.test.ts index 01f42ce027d87..5b0bf66d2aaf3 100644 --- a/frontend/src/scenes/funnels/funnelLogic.test.ts +++ b/frontend/src/scenes/funnels/funnelLogic.test.ts @@ -1,11 +1,11 @@ import { funnelLogic } from './funnelLogic' -import { MOCK_DEFAULT_TEAM, MOCK_TEAM_ID } from 'lib/api.mock' +import { MOCK_TEAM_ID } from 'lib/api.mock' import { expectLogic } from 'kea-test-utils' import { initKeaTests } from '~/test/init' import { preflightLogic } from 'scenes/PreflightCheck/preflightLogic' import { eventUsageLogic } from 'lib/utils/eventUsageLogic' import { insightLogic } from 'scenes/insights/insightLogic' -import { AvailableFeature, CorrelationConfigType, InsightLogicProps, InsightShortId, InsightType } from '~/types' +import { AvailableFeature, InsightLogicProps, InsightShortId, InsightType } from '~/types' import { teamLogic } from 'scenes/teamLogic' import { userLogic } from 'scenes/userLogic' import { router } from 'kea-router' @@ -17,87 +17,8 @@ import { openPersonsModal } from 'scenes/trends/persons-modal/PersonsModal' jest.mock('scenes/trends/persons-modal/PersonsModal') -const Insight12 = '12' as InsightShortId const Insight123 = '123' as InsightShortId -export const mockInsight = { - id: Insight123, - short_id: 'SvoU2bMC', - name: null, - filters: { - breakdown: null, - breakdown_type: null, - display: 'FunnelViz', - events: [ - { - id: '$pageview', - type: 'events', - order: 0, - name: '$pageview', - custom_name: null, - math: null, - math_property: null, - properties: [], - }, - { - id: '$pageview', - type: 'events', - order: 1, - name: '$pageview', - custom_name: null, - math: null, - math_property: null, - properties: [], - }, - { - id: '$pageview', - type: 'events', - order: 2, - name: '$pageview', - custom_name: null, - math: null, - math_property: null, - properties: [], - }, - { - id: '$pageview', - type: 'events', - order: 3, - name: '$pageview', - custom_name: null, - math: null, - math_property: null, - properties: [], - }, - ], - funnel_from_step: 0, - funnel_to_step: 1, - funnel_viz_type: 'steps', - insight: 'FUNNELS', - layout: 'vertical', - }, - order: null, - deleted: false, - dashboard: null, - layouts: {}, - color: null, - last_refresh: null, - result: null, - created_at: '2021-09-22T18:22:20.036153Z', - description: null, - updated_at: '2021-09-22T19:03:49.322258Z', - tags: [], - favorited: false, - saved: false, - created_by: { - id: 1, - uuid: '017c0441-bcb2-0000-bccf-dfc24328c5f3', - distinct_id: 'fM7b6ZFi8MOssbkDI55ot8tMY2hkzrHdRy1qERa6rCK', - first_name: 'Alex', - email: 'alex@posthog.com', - }, -} - const funnelResults = [ { action_id: '$pageview', @@ -124,200 +45,24 @@ const funnelResults = [ describe('funnelLogic', () => { let logic: ReturnType - const correlationConfig: CorrelationConfigType = {} beforeEach(() => { useAvailableFeatures([AvailableFeature.CORRELATION_ANALYSIS, AvailableFeature.GROUP_ANALYTICS]) useMocks({ get: { - '/api/projects/@current': () => [ - 200, - { - ...MOCK_DEFAULT_TEAM, - correlation_config: correlationConfig, - }, - ], - '/api/projects/:team/insights/': (req) => { - if (req.url.searchParams.get('saved')) { - return [ - 200, - { - results: funnelResults, - }, - ] - } - const shortId = req.url.searchParams.get('short_id') || '' - if (shortId === '500') { - return [500, { status: 0, detail: 'error from the API' }] - } - return [ - 200, - { - results: [mockInsight], - }, - ] + '/api/projects/:team/insights/': { + results: [{}], }, - '/api/projects/:team/insights/trend/': { results: ['trends result from api'] }, + '/api/projects/:team/insights/:id/': {}, '/api/projects/:team/groups_types/': [], - '/some/people/url': { results: [{ people: [] }] }, - '/api/projects/:team/persons/funnel': { results: [], next: null }, - '/api/projects/:team/persons/properties': [ - { name: 'some property', count: 20 }, - { name: 'another property', count: 10 }, - { name: 'third property', count: 5 }, - ], - '/api/projects/:team/groups/property_definitions': { - '0': [ - { name: 'industry', count: 2 }, - { name: 'name', count: 1 }, - ], - '1': [{ name: 'name', count: 1 }], - }, - }, - patch: { - '/api/projects/:id': (req) => [ - 200, - { - ...MOCK_DEFAULT_TEAM, - correlation_config: { - ...correlationConfig, - excluded_person_property_names: (req.body as any)?.correlation_config - ?.excluded_person_property_names, - }, - }, - ], }, post: { - '/api/projects/:team/insights/': (req) => [ - 200, - { id: 12, short_id: Insight12, ...((req.body as any) || {}) }, - ], - '/api/projects/:team/insights/:id/viewed': [201], '/api/projects/:team/insights/funnel/': { - is_cached: true, - last_refresh: '2021-09-16T13:41:41.297295Z', result: funnelResults, - type: 'Funnel', - }, - '/api/projects/:team/insights/funnel/correlation': (req) => { - const data = req.body as any - if (data?.funnel_correlation_type === 'properties') { - const excludePropertyFromProjectNames = data?.funnel_correlation_exclude_names || [] - const includePropertyNames = data?.funnel_correlation_names || [] - return [ - 200, - { - is_cached: true, - last_refresh: '2021-09-16T13:41:41.297295Z', - result: { - events: [ - { - event: { event: 'some property' }, - success_count: 1, - failure_count: 1, - odds_ratio: 1, - correlation_type: 'success', - }, - { - event: { event: 'another property' }, - success_count: 1, - failure_count: 1, - odds_ratio: 1, - correlation_type: 'failure', - }, - ] - .filter( - (correlation) => - includePropertyNames.includes('$all') || - includePropertyNames.includes(correlation.event.event) - ) - .filter( - (correlation) => - !excludePropertyFromProjectNames.includes(correlation.event.event) - ), - }, - type: 'Funnel', - }, - ] - } else if (data?.funnel_correlation_type === 'events') { - return [ - 200, - { - is_cached: true, - last_refresh: '2021-09-16T13:41:41.297295Z', - result: { - events: [ - { - event: { event: 'some event' }, - success_count: 1, - failure_count: 1, - odds_ratio: 1, - correlation_type: 'success', - }, - { - event: { event: 'another event' }, - success_count: 1, - failure_count: 1, - odds_ratio: 1, - correlation_type: 'failure', - }, - ], - }, - type: 'Funnel', - }, - ] - } else if (data?.funnel_correlation_type === 'event_with_properties') { - const targetEvent = data?.funnel_correlation_event_names[0] - const excludedProperties = data?.funnel_correlation_event_exclude_property_names - return [ - 200, - { - result: { - events: [ - { - success_count: 1, - failure_count: 0, - odds_ratio: 29, - correlation_type: 'success', - event: { event: `some event::name::Hester` }, - }, - { - success_count: 1, - failure_count: 0, - odds_ratio: 29, - correlation_type: 'success', - event: { event: `some event::Another name::Alice` }, - }, - { - success_count: 1, - failure_count: 0, - odds_ratio: 25, - correlation_type: 'success', - event: { event: `another event::name::Aloha` }, - }, - { - success_count: 1, - failure_count: 0, - odds_ratio: 25, - correlation_type: 'success', - event: { event: `another event::Another name::Bob` }, - }, - ].filter( - (record) => - record.event.event.split('::')[0] === targetEvent && - !excludedProperties.includes(record.event.event.split('::')[1]) - ), - last_refresh: '2021-11-05T09:26:16.175923Z', - is_cached: false, - }, - }, - ] - } }, }, }) initKeaTests(false) - window.POSTHOG_APP_CONTEXT = undefined // to force API request to /api/project/@current }) const defaultProps: InsightLogicProps = { diff --git a/frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.test.ts b/frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.test.ts index 46fec17cf9709..1b9e54a714659 100644 --- a/frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.test.ts +++ b/frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.test.ts @@ -15,111 +15,8 @@ import { } from '~/types' import { DEFAULT_EXCLUDED_PERSON_PROPERTIES, funnelPropertyCorrelationLogic } from './funnelPropertyCorrelationLogic' -const Insight12 = '12' as InsightShortId const Insight123 = '123' as InsightShortId -export const mockInsight = { - id: Insight123, - short_id: 'SvoU2bMC', - name: null, - filters: { - breakdown: null, - breakdown_type: null, - display: 'FunnelViz', - events: [ - { - id: '$pageview', - type: 'events', - order: 0, - name: '$pageview', - custom_name: null, - math: null, - math_property: null, - properties: [], - }, - { - id: '$pageview', - type: 'events', - order: 1, - name: '$pageview', - custom_name: null, - math: null, - math_property: null, - properties: [], - }, - { - id: '$pageview', - type: 'events', - order: 2, - name: '$pageview', - custom_name: null, - math: null, - math_property: null, - properties: [], - }, - { - id: '$pageview', - type: 'events', - order: 3, - name: '$pageview', - custom_name: null, - math: null, - math_property: null, - properties: [], - }, - ], - funnel_from_step: 0, - funnel_to_step: 1, - funnel_viz_type: 'steps', - insight: 'FUNNELS', - layout: 'vertical', - }, - order: null, - deleted: false, - dashboard: null, - layouts: {}, - color: null, - last_refresh: null, - result: null, - created_at: '2021-09-22T18:22:20.036153Z', - description: null, - updated_at: '2021-09-22T19:03:49.322258Z', - tags: [], - favorited: false, - saved: false, - created_by: { - id: 1, - uuid: '017c0441-bcb2-0000-bccf-dfc24328c5f3', - distinct_id: 'fM7b6ZFi8MOssbkDI55ot8tMY2hkzrHdRy1qERa6rCK', - first_name: 'Alex', - email: 'alex@posthog.com', - }, -} - -const funnelResults = [ - { - action_id: '$pageview', - count: 19, - name: '$pageview', - order: 0, - type: 'events', - }, - { - action_id: '$pageview', - count: 7, - name: '$pageview', - order: 1, - type: 'events', - }, - { - action_id: '$pageview', - count: 4, - name: '$pageview', - order: 2, - type: 'events', - }, -] - describe('funnelPropertyCorrelationLogic', () => { const props = { dashboardItemId: Insight123, syncWithUrl: true } let logic: ReturnType @@ -136,30 +33,9 @@ describe('funnelPropertyCorrelationLogic', () => { correlation_config: correlationConfig, }, ], - '/api/projects/:team/insights/': (req) => { - if (req.url.searchParams.get('saved')) { - return [ - 200, - { - results: funnelResults, - }, - ] - } - const shortId = req.url.searchParams.get('short_id') || '' - if (shortId === '500') { - return [500, { status: 0, detail: 'error from the API' }] - } - return [ - 200, - { - results: [mockInsight], - }, - ] - }, - '/api/projects/:team/insights/trend/': { results: ['trends result from api'] }, + '/api/projects/:team/insights/': { results: [{}] }, + '/api/projects/:team/insights/:id/': {}, '/api/projects/:team/groups_types/': [], - '/some/people/url': { results: [{ people: [] }] }, - '/api/projects/:team/persons/funnel': { results: [], next: null }, '/api/projects/:team/persons/properties': [ { name: 'some property', count: 20 }, { name: 'another property', count: 10 }, @@ -187,136 +63,50 @@ describe('funnelPropertyCorrelationLogic', () => { ], }, post: { - '/api/projects/:team/insights/': (req) => [ - 200, - { id: 12, short_id: Insight12, ...((req.body as any) || {}) }, - ], - '/api/projects/:team/insights/:id/viewed': [201], - '/api/projects/:team/insights/funnel/': { - is_cached: true, - last_refresh: '2021-09-16T13:41:41.297295Z', - result: funnelResults, - type: 'Funnel', - }, '/api/projects/:team/insights/funnel/correlation': (req) => { const data = req.body as any - if (data?.funnel_correlation_type === 'properties') { - const excludePropertyFromProjectNames = data?.funnel_correlation_exclude_names || [] - const includePropertyNames = data?.funnel_correlation_names || [] - return [ - 200, - { - is_cached: true, - last_refresh: '2021-09-16T13:41:41.297295Z', - result: { - events: [ - { - event: { event: 'some property' }, - success_count: 1, - failure_count: 1, - odds_ratio: 1, - correlation_type: 'success', - }, - { - event: { event: 'another property' }, - success_count: 1, - failure_count: 1, - odds_ratio: 1, - correlation_type: 'failure', - }, - ] - .filter( - (correlation) => - includePropertyNames.includes('$all') || - includePropertyNames.includes(correlation.event.event) - ) - .filter( - (correlation) => - !excludePropertyFromProjectNames.includes(correlation.event.event) - ), - }, - type: 'Funnel', - }, - ] - } else if (data?.funnel_correlation_type === 'events') { - return [ - 200, - { - is_cached: true, - last_refresh: '2021-09-16T13:41:41.297295Z', - result: { - events: [ - { - event: { event: 'some event' }, - success_count: 1, - failure_count: 1, - odds_ratio: 1, - correlation_type: 'success', - }, - { - event: { event: 'another event' }, - success_count: 1, - failure_count: 1, - odds_ratio: 1, - correlation_type: 'failure', - }, - ], - }, - type: 'Funnel', - }, - ] - } else if (data?.funnel_correlation_type === 'event_with_properties') { - const targetEvent = data?.funnel_correlation_event_names[0] - const excludedProperties = data?.funnel_correlation_event_exclude_property_names - return [ - 200, - { - result: { - events: [ - { - success_count: 1, - failure_count: 0, - odds_ratio: 29, - correlation_type: 'success', - event: { event: `some event::name::Hester` }, - }, - { - success_count: 1, - failure_count: 0, - odds_ratio: 29, - correlation_type: 'success', - event: { event: `some event::Another name::Alice` }, - }, - { - success_count: 1, - failure_count: 0, - odds_ratio: 25, - correlation_type: 'success', - event: { event: `another event::name::Aloha` }, - }, - { - success_count: 1, - failure_count: 0, - odds_ratio: 25, - correlation_type: 'success', - event: { event: `another event::Another name::Bob` }, - }, - ].filter( - (record) => - record.event.event.split('::')[0] === targetEvent && - !excludedProperties.includes(record.event.event.split('::')[1]) + const excludePropertyFromProjectNames = data?.funnel_correlation_exclude_names || [] + const includePropertyNames = data?.funnel_correlation_names || [] + return [ + 200, + { + is_cached: true, + last_refresh: '2021-09-16T13:41:41.297295Z', + result: { + events: [ + { + event: { event: 'some property' }, + success_count: 1, + failure_count: 1, + odds_ratio: 1, + correlation_type: 'success', + }, + { + event: { event: 'another property' }, + success_count: 1, + failure_count: 1, + odds_ratio: 1, + correlation_type: 'failure', + }, + ] + .filter( + (correlation) => + includePropertyNames.includes('$all') || + includePropertyNames.includes(correlation.event.event) + ) + .filter( + (correlation) => + !excludePropertyFromProjectNames.includes(correlation.event.event) ), - last_refresh: '2021-11-05T09:26:16.175923Z', - is_cached: false, - }, }, - ] - } + type: 'Funnel', + }, + ] }, }, }) initKeaTests(false) - // window.POSTHOG_APP_CONTEXT = undefined // to force API request to /api/project/@current + window.POSTHOG_APP_CONTEXT = undefined // to force API request to /api/project/@current }) const defaultProps: InsightLogicProps = { @@ -425,7 +215,6 @@ describe('funnelPropertyCorrelationLogic', () => { await expectLogic(logic, () => { logic.actions.setPropertyNames(logic.values.allProperties) - // logic.actions.loadResultsSuccess({ filters: { insight: InsightType.FUNNELS } }) }) .toFinishAllListeners() .toMatchValues({ From bd19ac5238addc2d3e1653651ab5fd6facf77162 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Wed, 5 Apr 2023 11:38:43 -0400 Subject: [PATCH 4/6] fix connected logic --- frontend/src/scenes/funnels/funnelCorrelationFeedbackLogic.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/scenes/funnels/funnelCorrelationFeedbackLogic.ts b/frontend/src/scenes/funnels/funnelCorrelationFeedbackLogic.ts index 7feb269f4880d..14d89ea0b8f42 100644 --- a/frontend/src/scenes/funnels/funnelCorrelationFeedbackLogic.ts +++ b/frontend/src/scenes/funnels/funnelCorrelationFeedbackLogic.ts @@ -4,9 +4,9 @@ import { eventUsageLogic } from 'lib/utils/eventUsageLogic' import { keyForInsightLogicProps } from 'scenes/insights/sharedUtils' import type { funnelCorrelationFeedbackLogicType } from './funnelCorrelationFeedbackLogicType' -import { funnelLogic } from './funnelLogic' import { InsightLogicProps } from '~/types' import { funnelCorrelationLogic } from './funnelCorrelationLogic' +import { funnelPropertyCorrelationLogic } from './funnelPropertyCorrelationLogic' export const funnelCorrelationFeedbackLogic = kea([ props({} as InsightLogicProps), @@ -17,7 +17,7 @@ export const funnelCorrelationFeedbackLogic = kea Date: Wed, 5 Apr 2023 13:15:53 -0400 Subject: [PATCH 5/6] fix key for property correlation logic --- frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.ts b/frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.ts index 74f9c53fc5d6d..463905a44c227 100644 --- a/frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.ts +++ b/frontend/src/scenes/funnels/funnelPropertyCorrelationLogic.ts @@ -41,7 +41,7 @@ export const DEFAULT_EXCLUDED_PERSON_PROPERTIES = [ export const funnelPropertyCorrelationLogic = kea([ props({} as InsightLogicProps), key(keyForInsightLogicProps('insight_funnel')), - path((key) => ['scenes', 'funnels', 'funnelCorrelationLogic', key]), + path((key) => ['scenes', 'funnels', 'funnelPropertyCorrelationLogic', key]), connect((props: InsightLogicProps) => ({ values: [ funnelCorrelationLogic(props), From 0adb43ea7b67338d25baf5eae4ef3f8ffc002043 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Wed, 5 Apr 2023 13:36:25 -0400 Subject: [PATCH 6/6] convert funnel property correlation table --- .../views/Funnels/FunnelCorrelation.tsx | 19 ++-- .../views/Funnels/FunnelCorrelationTable.tsx | 2 +- .../FunnelPropertyCorrelationTable.tsx | 96 +++++++++++++++---- 3 files changed, 89 insertions(+), 28 deletions(-) diff --git a/frontend/src/scenes/insights/views/Funnels/FunnelCorrelation.tsx b/frontend/src/scenes/insights/views/Funnels/FunnelCorrelation.tsx index 8c0dac9c41919..f74a93b3d7312 100644 --- a/frontend/src/scenes/insights/views/Funnels/FunnelCorrelation.tsx +++ b/frontend/src/scenes/insights/views/Funnels/FunnelCorrelation.tsx @@ -12,18 +12,21 @@ import { } from './FunnelCorrelationSkewWarning' import { FunnelCorrelationTable, FunnelCorrelationTableDataExploration } from './FunnelCorrelationTable' import { FunnelCorrelationFeedbackForm } from './FunnelCorrelationFeedbackForm' -import { FunnelPropertyCorrelationTable } from './FunnelPropertyCorrelationTable' +import { + FunnelPropertyCorrelationTable, + FunnelPropertyCorrelationTableDataExploration, +} from './FunnelPropertyCorrelationTable' import { AvailableFeature } from '~/types' import './FunnelCorrelation.scss' export const FunnelCorrelation = (): JSX.Element | null => { - const { insightProps, isUsingDataExploration } = useValues(insightLogic) + const { insightProps, isUsingDataExploration: dx } = useValues(insightLogic) const { steps: legacySteps } = useValues(funnelLogic(insightProps)) const { steps } = useValues(funnelDataLogic(insightProps)) useMountedLogic(funnelCorrelationUsageLogic(insightProps)) - if (isUsingDataExploration ? steps.length <= 1 : legacySteps.length <= 1) { + if (dx ? steps.length <= 1 : legacySteps.length <= 1) { return null } @@ -32,14 +35,10 @@ export const FunnelCorrelation = (): JSX.Element | null => {

Correlation analysis

- {isUsingDataExploration ? ( - - ) : ( - - )} - {isUsingDataExploration ? : } + {dx ? : } + {dx ? : } - {!isUsingDataExploration && } + {dx ? : }
diff --git a/frontend/src/scenes/insights/views/Funnels/FunnelCorrelationTable.tsx b/frontend/src/scenes/insights/views/Funnels/FunnelCorrelationTable.tsx index 318471f4b8061..4ae2f55d44c8d 100644 --- a/frontend/src/scenes/insights/views/Funnels/FunnelCorrelationTable.tsx +++ b/frontend/src/scenes/insights/views/Funnels/FunnelCorrelationTable.tsx @@ -37,7 +37,7 @@ export function FunnelCorrelationTableDataExploration(): JSX.Element | null { const { loadedEventCorrelationsTableOnce } = useValues(funnelCorrelationLogic(insightProps)) const { loadEventCorrelations } = useActions(funnelCorrelationLogic(insightProps)) - // Load correlations only if this component is mounted, and then reload if query changes + // Load correlations only if this component is mounted, and then reload if the query changes useEffect(() => { // We only automatically refresh results when the query changes after the user has manually asked for the first results to be loaded if (loadedEventCorrelationsTableOnce) { diff --git a/frontend/src/scenes/insights/views/Funnels/FunnelPropertyCorrelationTable.tsx b/frontend/src/scenes/insights/views/Funnels/FunnelPropertyCorrelationTable.tsx index 373fa3b0db0bd..4edb29c884b77 100644 --- a/frontend/src/scenes/insights/views/Funnels/FunnelPropertyCorrelationTable.tsx +++ b/frontend/src/scenes/insights/views/Funnels/FunnelPropertyCorrelationTable.tsx @@ -4,7 +4,12 @@ import Column from 'antd/lib/table/Column' import { useActions, useValues } from 'kea' import { RiseOutlined, FallOutlined, InfoCircleOutlined } from '@ant-design/icons' import { funnelLogic } from 'scenes/funnels/funnelLogic' -import { FunnelCorrelation, FunnelCorrelationResultsType, FunnelCorrelationType } from '~/types' +import { + FunnelCorrelation, + FunnelCorrelationResultsType, + FunnelCorrelationType, + FunnelStepWithNestedBreakdown, +} from '~/types' import Checkbox from 'antd/lib/checkbox/Checkbox' import { insightLogic } from 'scenes/insights/insightLogic' import { ValueInspectorButton } from 'scenes/funnels/ValueInspectorButton' @@ -20,10 +25,79 @@ import { PropertyCorrelationActionsCell } from './CorrelationActionsCell' import { funnelCorrelationUsageLogic } from 'scenes/funnels/funnelCorrelationUsageLogic' import { parseDisplayNameForCorrelation } from 'scenes/funnels/funnelUtils' import { funnelPropertyCorrelationLogic } from 'scenes/funnels/funnelPropertyCorrelationLogic' +import { Noun } from '~/models/groupsModel' +import { funnelDataLogic } from 'scenes/funnels/funnelDataLogic' + +export function FunnelPropertyCorrelationTableDataExploration(): JSX.Element | null { + const { insightProps } = useValues(insightLogic) + const { steps, querySource, aggregationTargetLabel } = useValues(funnelDataLogic(insightProps)) + const { loadedPropertyCorrelationsTableOnce, propertyNames, allProperties } = useValues( + funnelPropertyCorrelationLogic(insightProps) + ) + const { loadPropertyCorrelations, setPropertyNames } = useActions(funnelPropertyCorrelationLogic(insightProps)) + + // Load correlations only if this component is mounted, and then reload if the query change + useEffect(() => { + // We only automatically refresh results when the query changes after the user has manually asked for the first results to be loaded + if (loadedPropertyCorrelationsTableOnce) { + if (propertyNames.length === 0) { + setPropertyNames(allProperties) + } + + loadPropertyCorrelations({}) + } + }, [querySource]) + + return ( + + ) +} export function FunnelPropertyCorrelationTable(): JSX.Element | null { const { insightProps } = useValues(insightLogic) const { steps, filters, aggregationTargetLabel } = useValues(funnelLogic(insightProps)) + const { loadedPropertyCorrelationsTableOnce, propertyNames, allProperties } = useValues( + funnelPropertyCorrelationLogic(insightProps) + ) + const { loadPropertyCorrelations, setPropertyNames } = useActions(funnelPropertyCorrelationLogic(insightProps)) + + // Load correlations only if this component is mounted, and then reload if filters change + useEffect(() => { + // We only automatically refresh results when filters change after the user has manually asked for the first results to be loaded + if (loadedPropertyCorrelationsTableOnce) { + if (propertyNames.length === 0) { + setPropertyNames(allProperties) + } + + loadPropertyCorrelations({}) + } + }, [filters]) + + return ( + + ) +} + +type FunnelPropertyCorrelationTableComponentProps = { + steps: FunnelStepWithNestedBreakdown[] + aggregation_group_type_index?: number | undefined + aggregationTargetLabel: Noun +} + +export function FunnelPropertyCorrelationTableComponent({ + steps, + aggregation_group_type_index, + aggregationTargetLabel, +}: FunnelPropertyCorrelationTableComponentProps): JSX.Element | null { + const { insightProps } = useValues(insightLogic) const { openCorrelationPersonsModal } = useActions(funnelLogic(insightProps)) const { propertyCorrelationValues, @@ -41,18 +115,6 @@ export function FunnelPropertyCorrelationTable(): JSX.Element | null { const { correlationPropKey } = useValues(funnelCorrelationUsageLogic(insightProps)) const { reportCorrelationInteraction } = useActions(funnelCorrelationUsageLogic(insightProps)) - // Load correlations only if this component is mounted, and then reload if filters change - useEffect(() => { - // We only automatically refresh results when filters change after the user has manually asked for the first results to be loaded - if (loadedPropertyCorrelationsTableOnce) { - if (propertyNames.length === 0) { - setPropertyNames(allProperties) - } - - loadPropertyCorrelations({}) - } - }, [filters]) - const onClickCorrelationType = (correlationType: FunnelCorrelationType): void => { if (propertyCorrelationTypes) { if (propertyCorrelationTypes.includes(correlationType)) { @@ -120,7 +182,7 @@ export function FunnelPropertyCorrelationTable(): JSX.Element | null {
{capitalizeFirstLetter(aggregationTargetLabel.plural)}{' '} - {filters.aggregation_group_type_index != undefined ? 'that' : 'who'} converted were{' '} + {aggregation_group_type_index != undefined ? 'that' : 'who'} converted were{' '} {get_friendly_numeric_value(record.odds_ratio)}x {is_success ? 'more' : 'less'} likely @@ -243,7 +305,7 @@ export function FunnelPropertyCorrelationTable(): JSX.Element | null { Completed @@ -263,8 +325,8 @@ export function FunnelPropertyCorrelationTable(): JSX.Element | null { title={ <> {capitalizeFirstLetter(aggregationTargetLabel.plural)}{' '} - {filters.aggregation_group_type_index != undefined ? 'that' : 'who'}{' '} - have this property and did not complete the entire funnel. + {aggregation_group_type_index != undefined ? 'that' : 'who'} have this + property and did not complete the entire funnel. } >