From 00fc0bcfbdb7b454bc2a7236fed758532a14de8e Mon Sep 17 00:00:00 2001 From: Tom Owers Date: Thu, 24 Oct 2024 00:21:50 +0100 Subject: [PATCH] feat(experiments): Added support for warehouse and person props in experiments goals (#25767) --- .../TaxonomicFilter/TaxonomicFilter.tsx | 2 + .../TaxonomicFilter/infiniteListLogic.test.ts | 19 +++++---- .../TaxonomicFilter/infiniteListLogic.ts | 39 ++++++++++++++----- .../lib/components/TaxonomicFilter/types.ts | 1 + .../TaxonomicPopover/TaxonomicPopover.tsx | 3 ++ .../InsightQuery/utils/filtersToQueryNode.ts | 1 + .../InsightQuery/utils/queryNodeToFilter.ts | 2 + frontend/src/queries/schema.json | 24 ++++++++++++ frontend/src/queries/schema.ts | 1 + .../src/scenes/experiments/MetricSelector.tsx | 3 ++ .../filters/ActionFilter/ActionFilter.tsx | 4 ++ .../ActionFilterRow/ActionFilterRow.tsx | 18 +++++++-- .../filters/ActionFilter/entityFilterLogic.ts | 1 + frontend/src/types.ts | 1 + .../insights/trends/aggregation_operations.py | 2 + posthog/schema.py | 8 ++++ 16 files changed, 109 insertions(+), 20 deletions(-) diff --git a/frontend/src/lib/components/TaxonomicFilter/TaxonomicFilter.tsx b/frontend/src/lib/components/TaxonomicFilter/TaxonomicFilter.tsx index c65ffd600898f..3d5973f55d75e 100644 --- a/frontend/src/lib/components/TaxonomicFilter/TaxonomicFilter.tsx +++ b/frontend/src/lib/components/TaxonomicFilter/TaxonomicFilter.tsx @@ -36,6 +36,7 @@ export function TaxonomicFilter({ selectFirstItem = true, propertyAllowList, hideBehavioralCohorts, + showNumericalPropsOnly, }: TaxonomicFilterProps): JSX.Element { // Generate a unique key for each unique TaxonomicFilter that's rendered const taxonomicFilterLogicKey = useMemo( @@ -62,6 +63,7 @@ export function TaxonomicFilter({ metadataSource, propertyAllowList, hideBehavioralCohorts, + showNumericalPropsOnly, } const logic = taxonomicFilterLogic(taxonomicFilterLogicProps) diff --git a/frontend/src/lib/components/TaxonomicFilter/infiniteListLogic.test.ts b/frontend/src/lib/components/TaxonomicFilter/infiniteListLogic.test.ts index 25e2866afc2c3..bf9b55115cb69 100644 --- a/frontend/src/lib/components/TaxonomicFilter/infiniteListLogic.test.ts +++ b/frontend/src/lib/components/TaxonomicFilter/infiniteListLogic.test.ts @@ -66,6 +66,7 @@ describe('infiniteListLogic', () => { taxonomicFilterLogicKey: 'testList', listGroupType: TaxonomicFilterGroupType.Events, taxonomicGroupTypes: [TaxonomicFilterGroupType.Events], + showNumericalPropsOnly: false, } const logicWithProps = infiniteListLogic({ ...defaultProps, ...props }) logicWithProps.mount() @@ -98,6 +99,7 @@ describe('infiniteListLogic', () => { taxonomicFilterLogicKey: 'testList', listGroupType: TaxonomicFilterGroupType.Events, taxonomicGroupTypes: [TaxonomicFilterGroupType.Events], + showNumericalPropsOnly: false, }) logic.mount() }) @@ -180,11 +182,11 @@ describe('infiniteListLogic', () => { index: 0, remoteItems: partial({ count: 156 }), localItems: partial({ count: 1 }), - items: partial({ count: 157 }), + items: partial({ count: 101 }), }) - expectLogic(logic, () => logic.actions.moveUp()).toMatchValues({ index: 156 }) - expectLogic(logic, () => logic.actions.moveUp()).toMatchValues({ index: 155 }) - expectLogic(logic, () => logic.actions.moveDown()).toMatchValues({ index: 156 }) + expectLogic(logic, () => logic.actions.moveUp()).toMatchValues({ index: 100 }) + expectLogic(logic, () => logic.actions.moveUp()).toMatchValues({ index: 99 }) + expectLogic(logic, () => logic.actions.moveDown()).toMatchValues({ index: 100 }) expectLogic(logic, () => logic.actions.moveDown()).toMatchValues({ index: 0 }) expectLogic(logic, () => logic.actions.moveDown()).toMatchValues({ index: 1 }) expectLogic(logic, () => logic.actions.moveUp()).toMatchValues({ index: 0 }) @@ -196,7 +198,7 @@ describe('infiniteListLogic', () => { index: 0, remoteItems: partial({ count: 156 }), localItems: partial({ count: 1 }), - items: partial({ count: 157 }), + items: partial({ count: 101 }), }) await expectLogic(logic, () => @@ -237,6 +239,7 @@ describe('infiniteListLogic', () => { taxonomicFilterLogicKey: 'testList', listGroupType: TaxonomicFilterGroupType.Wildcards, taxonomicGroupTypes: [TaxonomicFilterGroupType.Events, TaxonomicFilterGroupType.Actions], + showNumericalPropsOnly: false, optionsFromProp: { wildcard: [{ name: 'first' }, { name: 'second' }], }, @@ -260,6 +263,7 @@ describe('infiniteListLogic', () => { listGroupType: TaxonomicFilterGroupType.EventProperties, eventNames: ['$pageview'], taxonomicGroupTypes: [TaxonomicFilterGroupType.EventProperties], + showNumericalPropsOnly: false, }) logic.mount() }) @@ -368,9 +372,9 @@ describe('infiniteListLogic', () => { isExpandable: false, isExpanded: true, isExpandableButtonSelected: false, - totalResultCount: 2, + totalResultCount: 3, totalExtraCount: 0, - totalListCount: 2, + totalListCount: 3, expandedCount: 0, remoteItems: partial({ count: 2, @@ -389,6 +393,7 @@ describe('infiniteListLogic', () => { taxonomicFilterLogicKey: 'test-element-list', listGroupType: TaxonomicFilterGroupType.Elements, taxonomicGroupTypes: [TaxonomicFilterGroupType.Elements], + showNumericalPropsOnly: false, }) logicWithProps.mount() diff --git a/frontend/src/lib/components/TaxonomicFilter/infiniteListLogic.ts b/frontend/src/lib/components/TaxonomicFilter/infiniteListLogic.ts index eb8ca78c45ec4..05c5a387cad0f 100644 --- a/frontend/src/lib/components/TaxonomicFilter/infiniteListLogic.ts +++ b/frontend/src/lib/components/TaxonomicFilter/infiniteListLogic.ts @@ -69,7 +69,7 @@ async function fetchCachedListResponse(path: string, searchParams: Record([ - props({} as InfiniteListLogicProps), + props({ showNumericalPropsOnly: false } as InfiniteListLogicProps), key((props) => `${props.taxonomicFilterLogicKey}-${props.listGroupType}`), path((key) => ['lib', 'components', 'TaxonomicFilter', 'infiniteListLogic', key]), connect((props: InfiniteListLogicProps) => ({ @@ -310,15 +310,34 @@ export const infiniteListLogic = kea([ }, ], items: [ - (s) => [s.remoteItems, s.localItems], - (remoteItems, localItems) => ({ - results: [...localItems.results, ...remoteItems.results], - count: localItems.count + remoteItems.count, - searchQuery: localItems.searchQuery, - expandedCount: remoteItems.expandedCount, - queryChanged: remoteItems.queryChanged, - first: localItems.first && remoteItems.first, - }), + (s, p) => [s.remoteItems, s.localItems, p.showNumericalPropsOnly ?? (() => false)], + (remoteItems, localItems, showNumericalPropsOnly) => { + const results = [...localItems.results, ...remoteItems.results].filter((n) => { + if (!showNumericalPropsOnly) { + return true + } + + if ('is_numerical' in n) { + return !!n.is_numerical + } + + if ('property_type' in n) { + const property_type = n.property_type as string // Data warehouse props dont conformt to PropertyType for some reason + return property_type === 'Integer' || property_type === 'Float' + } + + return true + }) + + return { + results, + count: results.length, + searchQuery: localItems.searchQuery, + expandedCount: remoteItems.expandedCount, + queryChanged: remoteItems.queryChanged, + first: localItems.first && remoteItems.first, + } + }, ], totalResultCount: [(s) => [s.items], (items) => items.count || 0], totalExtraCount: [ diff --git a/frontend/src/lib/components/TaxonomicFilter/types.ts b/frontend/src/lib/components/TaxonomicFilter/types.ts index 40931c4ef93e3..8bbfdb247a607 100644 --- a/frontend/src/lib/components/TaxonomicFilter/types.ts +++ b/frontend/src/lib/components/TaxonomicFilter/types.ts @@ -38,6 +38,7 @@ export interface TaxonomicFilterProps { propertyAllowList?: { [key in TaxonomicFilterGroupType]?: string[] } // only return properties in this list, currently only working for EventProperties and PersonProperties metadataSource?: AnyDataNode hideBehavioralCohorts?: boolean + showNumericalPropsOnly?: boolean } export interface TaxonomicFilterLogicProps extends TaxonomicFilterProps { diff --git a/frontend/src/lib/components/TaxonomicPopover/TaxonomicPopover.tsx b/frontend/src/lib/components/TaxonomicPopover/TaxonomicPopover.tsx index c7f9adf4ac81a..2f80f363b674d 100644 --- a/frontend/src/lib/components/TaxonomicPopover/TaxonomicPopover.tsx +++ b/frontend/src/lib/components/TaxonomicPopover/TaxonomicPopover.tsx @@ -26,6 +26,7 @@ export interface TaxonomicPopoverProps): JSX.Element { const [localValue, setLocalValue] = useState(value || ('' as ValueType)) @@ -94,6 +96,7 @@ export function TaxonomicPopover } matchWidth={false} diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts index 0ac8e1510fd78..c7ccb75fb6c61 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts @@ -138,6 +138,7 @@ export const legacyEntityToNode = ( ...shared, math: entity.math || 'total', math_property: entity.math_property, + math_property_type: entity.math_property_type, math_hogql: entity.math_hogql, math_group_type_index: entity.math_group_type_index, } as any diff --git a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts index 9a8f36c6cb548..89e0a6f3d8e7b 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts @@ -1,3 +1,4 @@ +import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types' import { objectClean } from 'lib/utils' import { @@ -53,6 +54,7 @@ export const seriesNodeToFilter = ( // TODO: math is not supported by funnel and lifecycle queries math: node.math, math_property: node.math_property, + math_property_type: node.math_property_type as TaxonomicFilterGroupType, math_hogql: node.math_hogql, math_group_type_index: node.math_group_type_index, properties: node.properties as any, // TODO, diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index c243cf67ba628..cdb3c4449d002 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -31,6 +31,9 @@ "math_property": { "type": "string" }, + "math_property_type": { + "type": "string" + }, "name": { "type": "string" }, @@ -84,6 +87,9 @@ "math_property": { "type": "string" }, + "math_property_type": { + "type": "string" + }, "name": { "type": "string" }, @@ -172,6 +178,9 @@ "math_property": { "type": "string" }, + "math_property_type": { + "type": "string" + }, "name": { "type": "string" }, @@ -3986,6 +3995,9 @@ "math_property": { "type": "string" }, + "math_property_type": { + "type": "string" + }, "name": { "type": "string" }, @@ -4503,6 +4515,9 @@ "math_property": { "type": "string" }, + "math_property_type": { + "type": "string" + }, "name": { "type": "string" }, @@ -4935,6 +4950,9 @@ "math_property": { "type": "string" }, + "math_property_type": { + "type": "string" + }, "name": { "type": "string" }, @@ -5641,6 +5659,9 @@ "math_property": { "type": "string" }, + "math_property_type": { + "type": "string" + }, "name": { "type": "string" }, @@ -5701,6 +5722,9 @@ "math_property": { "type": "string" }, + "math_property_type": { + "type": "string" + }, "name": { "type": "string" }, diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index 92b6d52f251db..b288c4f49a364 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -480,6 +480,7 @@ export interface EntityNode extends Node { custom_name?: string math?: MathType math_property?: string + math_property_type?: string math_hogql?: string math_group_type_index?: 0 | 1 | 2 | 3 | 4 /** Properties configurable in the interface */ diff --git a/frontend/src/scenes/experiments/MetricSelector.tsx b/frontend/src/scenes/experiments/MetricSelector.tsx index 975ea070d11a4..4f4e7b6e1e262 100644 --- a/frontend/src/scenes/experiments/MetricSelector.tsx +++ b/frontend/src/scenes/experiments/MetricSelector.tsx @@ -132,6 +132,7 @@ export function ExperimentInsightCreator({ insightProps }: { insightProps: Insig seriesIndicatorType={isTrends ? undefined : 'numeric'} sortable={isTrends ? undefined : true} showNestedArrow={isTrends ? undefined : true} + showNumericalPropsOnly={isTrends} propertiesTaxonomicGroupTypes={[ TaxonomicFilterGroupType.EventProperties, TaxonomicFilterGroupType.PersonProperties, @@ -139,6 +140,8 @@ export function ExperimentInsightCreator({ insightProps }: { insightProps: Insig TaxonomicFilterGroupType.Cohorts, TaxonomicFilterGroupType.Elements, TaxonomicFilterGroupType.HogQLExpression, + TaxonomicFilterGroupType.DataWarehouseProperties, + TaxonomicFilterGroupType.DataWarehousePersonProperties, ]} />
diff --git a/frontend/src/scenes/insights/filters/ActionFilter/ActionFilter.tsx b/frontend/src/scenes/insights/filters/ActionFilter/ActionFilter.tsx index 8ebb237640060..91d3c1fcf260d 100644 --- a/frontend/src/scenes/insights/filters/ActionFilter/ActionFilter.tsx +++ b/frontend/src/scenes/insights/filters/ActionFilter/ActionFilter.tsx @@ -73,6 +73,8 @@ export interface ActionFilterProps { actionsTaxonomicGroupTypes?: TaxonomicFilterGroupType[] /** Which tabs to show for property filters */ propertiesTaxonomicGroupTypes?: TaxonomicFilterGroupType[] + /** Whether properties shown should be limited to just numerical types */ + showNumericalPropsOnly?: boolean hideDeleteBtn?: boolean readOnly?: boolean renderRow?: ({ @@ -108,6 +110,7 @@ export const ActionFilter = React.forwardRef( showNestedArrow = false, actionsTaxonomicGroupTypes, propertiesTaxonomicGroupTypes, + showNumericalPropsOnly, hideDeleteBtn, renderRow, buttonType = 'tertiary', @@ -170,6 +173,7 @@ export const ActionFilter = React.forwardRef( hideDuplicate, onRenameClick: showModal, sortable, + showNumericalPropsOnly, } const reachedLimit: boolean = Boolean(entitiesLimit && localFilters.length >= entitiesLimit) diff --git a/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx b/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx index 6340b62226ad7..b4d3fc1642172 100644 --- a/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx +++ b/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx @@ -123,6 +123,8 @@ export interface ActionFilterRowProps { deleteButton, }: Record) => JSX.Element // build your own row given these components trendsDisplayCategory: ChartDisplayCategory | null + /** Whether properties shown should be limited to just numerical types */ + showNumericalPropsOnly?: boolean } export function ActionFilterRow({ @@ -151,6 +153,7 @@ export function ActionFilterRow({ readOnly = false, renderRow, trendsDisplayCategory, + showNumericalPropsOnly, }: ActionFilterRowProps): JSX.Element { const { entityFilterVisible } = useValues(logic) const { @@ -177,6 +180,7 @@ export function ActionFilterRow({ const { math, math_property: mathProperty, + math_property_type: mathPropertyType, math_hogql: mathHogQL, math_group_type_index: mathGroupTypeIndex, } = filter @@ -196,9 +200,11 @@ export function ActionFilterRow({ mathDefinitions[selectedMath]?.category === MathCategory.HogQLExpression ? mathHogQL ?? 'count()' : undefined, + mathPropertyType, } : { math_property: undefined, + mathPropertyType: undefined, math_hogql: undefined, math_group_type_index: undefined, math: undefined, @@ -210,11 +216,12 @@ export function ActionFilterRow({ ...mathProperties, }) } - const onMathPropertySelect = (_: unknown, property: string): void => { + const onMathPropertySelect = (_: unknown, property: string, groupType: TaxonomicFilterGroupType): void => { updateFilterMath({ ...filter, math_hogql: undefined, math_property: property, + math_property_type: groupType, index, }) } @@ -223,6 +230,7 @@ export function ActionFilterRow({ updateFilterMath({ ...filter, math_property: undefined, + math_property_type: undefined, math_hogql: hogql, index, }) @@ -281,6 +289,7 @@ export function ActionFilterRow({ placeholder="All events" placeholderClass="" disabled={disabled || readOnly} + showNumericalPropsOnly={showNumericalPropsOnly} /> ) @@ -425,6 +434,8 @@ export function ActionFilterRow({ TaxonomicFilterGroupType.DataWarehouseProperties, TaxonomicFilterGroupType.NumericalEventProperties, TaxonomicFilterGroupType.SessionProperties, + TaxonomicFilterGroupType.PersonProperties, + TaxonomicFilterGroupType.DataWarehousePersonProperties, ]} schemaColumns={ filter.type == TaxonomicFilterGroupType.DataWarehouse && @@ -435,11 +446,12 @@ export function ActionFilterRow({ : [] } value={mathProperty} - onChange={(currentValue) => - onMathPropertySelect(index, currentValue) + onChange={(currentValue, groupType) => + onMathPropertySelect(index, currentValue, groupType) } eventNames={name ? [name] : []} data-attr="math-property-select" + showNumericalPropsOnly={showNumericalPropsOnly} renderValue={(currentValue) => ( ([ type: filter.type as EntityType, math: filter.math, math_property: filter.math_property, + math_property_type: filter.math_property_type, math_hogql: filter.math_hogql, index: filter.index, math_group_type_index: filter.math_group_type_index, diff --git a/frontend/src/types.ts b/frontend/src/types.ts index fd3c9e4f5405a..a07b4d883dbfc 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -1109,6 +1109,7 @@ export type EntityFilter = { export interface ActionFilter extends EntityFilter { math?: string math_property?: string + math_property_type?: TaxonomicFilterGroupType math_group_type_index?: integer | null math_hogql?: string properties?: AnyPropertyFilter[] diff --git a/posthog/hogql_queries/insights/trends/aggregation_operations.py b/posthog/hogql_queries/insights/trends/aggregation_operations.py index 39e5b8c2b2494..bea5230e45a1a 100644 --- a/posthog/hogql_queries/insights/trends/aggregation_operations.py +++ b/posthog/hogql_queries/insights/trends/aggregation_operations.py @@ -135,6 +135,8 @@ def _math_func(self, method: str, override_chain: Optional[list[str | int]]) -> chain = ["session_duration"] elif isinstance(self.series, DataWarehouseNode) and self.series.math_property: chain = [self.series.math_property] + elif self.series.math_property_type == "data_warehouse_person_properties" and self.series.math_property: + chain = ["person", *self.series.math_property.split(".")] else: chain = ["properties", self.series.math_property] diff --git a/posthog/schema.py b/posthog/schema.py index b77997f9f74f5..8188cf8a0217e 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -4391,6 +4391,7 @@ class DataWarehouseNode(BaseModel): math_group_type_index: Optional[MathGroupTypeIndex] = None math_hogql: Optional[str] = None math_property: Optional[str] = None + math_property_type: Optional[str] = None name: Optional[str] = None properties: Optional[ list[ @@ -4481,6 +4482,7 @@ class EntityNode(BaseModel): math_group_type_index: Optional[MathGroupTypeIndex] = None math_hogql: Optional[str] = None math_property: Optional[str] = None + math_property_type: Optional[str] = None name: Optional[str] = None properties: Optional[ list[ @@ -4559,6 +4561,7 @@ class EventsNode(BaseModel): math_group_type_index: Optional[MathGroupTypeIndex] = None math_hogql: Optional[str] = None math_property: Optional[str] = None + math_property_type: Optional[str] = None name: Optional[str] = None orderBy: Optional[list[str]] = Field(default=None, description="Columns to order by") properties: Optional[ @@ -4640,6 +4643,7 @@ class FunnelExclusionActionsNode(BaseModel): math_group_type_index: Optional[MathGroupTypeIndex] = None math_hogql: Optional[str] = None math_property: Optional[str] = None + math_property_type: Optional[str] = None name: Optional[str] = None properties: Optional[ list[ @@ -4708,6 +4712,7 @@ class FunnelExclusionEventsNode(BaseModel): math_group_type_index: Optional[MathGroupTypeIndex] = None math_hogql: Optional[str] = None math_property: Optional[str] = None + math_property_type: Optional[str] = None name: Optional[str] = None orderBy: Optional[list[str]] = Field(default=None, description="Columns to order by") properties: Optional[ @@ -4986,6 +4991,7 @@ class AIActionsNode(BaseModel): ] = None math_group_type_index: Optional[MathGroupTypeIndex] = None math_property: Optional[str] = None + math_property_type: Optional[str] = None name: Optional[str] = None orderBy: Optional[list[str]] = Field(default=None, description="Columns to order by") properties: Optional[ @@ -5034,6 +5040,7 @@ class AIEventsNode(BaseModel): ] = None math_group_type_index: Optional[MathGroupTypeIndex] = None math_property: Optional[str] = None + math_property_type: Optional[str] = None name: Optional[str] = None orderBy: Optional[list[str]] = Field(default=None, description="Columns to order by") properties: Optional[ @@ -5093,6 +5100,7 @@ class ActionsNode(BaseModel): math_group_type_index: Optional[MathGroupTypeIndex] = None math_hogql: Optional[str] = None math_property: Optional[str] = None + math_property_type: Optional[str] = None name: Optional[str] = None properties: Optional[ list[