Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(data-warehouse): make separate property type in taxonomic filter for data warehouse person properties #21169

Merged
merged 11 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions frontend/src/lib/components/PropertyFilters/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ export const PROPERTY_FILTER_TYPE_TO_TAXONOMIC_FILTER_GROUP_TYPE: Omit<
[PropertyFilterType.HogQL]: TaxonomicFilterGroupType.HogQLExpression,
[PropertyFilterType.Group]: TaxonomicFilterGroupType.GroupsPrefix,
[PropertyFilterType.DataWarehouse]: TaxonomicFilterGroupType.DataWarehouse,
[PropertyFilterType.DataWarehousePersonProperty]: TaxonomicFilterGroupType.DataWarehousePersonProperties,
}

export function formatPropertyLabel(
Expand Down Expand Up @@ -332,6 +333,10 @@ export function taxonomicFilterTypeToPropertyFilterType(
return PropertyFilterType.DataWarehouse
}

if (filterType == TaxonomicFilterGroupType.DataWarehousePersonProperties) {
return PropertyFilterType.DataWarehousePersonProperty
}

return Object.entries(propertyFilterMapping).find(([, v]) => v === filterType)?.[0] as
| PropertyFilterType
| undefined
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/lib/components/TaxonomicFilter/InfiniteList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,9 @@ export function InfiniteList({ popupAnchorElement }: InfiniteListProps): JSX.Ele
<div
{...commonDivProps}
data-attr={`prop-filter-${listGroupType}-${rowIndex}`}
onClick={() => canSelectItem(listGroupType) && selectItem(group, itemValue ?? null, item)}
onClick={() => {
return canSelectItem(listGroupType) && selectItem(group, itemValue ?? null, item)
}}
>
{renderItemContents({
item,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { IconCohort } from 'lib/lemon-ui/icons'
import { CORE_FILTER_DEFINITIONS_BY_GROUP } from 'lib/taxonomy'
import { capitalizeFirstLetter, pluralize, toParams } from 'lib/utils'
import { getEventDefinitionIcon, getPropertyDefinitionIcon } from 'scenes/data-management/events/DefinitionHeader'
import { dataWarehouseJoinsLogic } from 'scenes/data-warehouse/external/dataWarehouseJoinsLogic'
import { dataWarehouseSceneLogic } from 'scenes/data-warehouse/external/dataWarehouseSceneLogic'
import { DataWarehouseTableType } from 'scenes/data-warehouse/types'
import { experimentsLogic } from 'scenes/experiments/experimentsLogic'
Expand Down Expand Up @@ -86,6 +87,8 @@ export const taxonomicFilterLogic = kea<taxonomicFilterLogicType>([
['allGroupProperties'],
dataWarehouseSceneLogic,
['externalTables'],
dataWarehouseJoinsLogic,
['columnsJoinedToPersons'],
],
}),
actions(() => ({
Expand Down Expand Up @@ -225,6 +228,16 @@ export const taxonomicFilterLogic = kea<taxonomicFilterLogicType>([
getPopoverHeader: () => 'Data Warehouse Column',
getIcon: () => <IconServer />,
},
{
name: 'Data Warehouse Person Properties',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a long name 😅

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

External Person Properties?

searchPlaceholder: 'person properties from data warehouse tables',
type: TaxonomicFilterGroupType.DataWarehousePersonProperties,
logic: dataWarehouseJoinsLogic,
value: 'columnsJoinedToPersons',
getName: (personProperty: PersonProperty) => personProperty.name,
getValue: (personProperty: PersonProperty) => personProperty.id,
getPopoverHeader: () => 'Data Warehouse Person Property',
},
{
name: 'Autocapture elements',
searchPlaceholder: 'autocapture elements',
Expand Down
1 change: 1 addition & 0 deletions frontend/src/lib/components/TaxonomicFilter/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ export enum TaxonomicFilterGroupType {
CohortsWithAllUsers = 'cohorts_with_all',
DataWarehouse = 'data_warehouse',
DataWarehouseProperties = 'data_warehouse_properties',
DataWarehousePersonProperties = 'data_warehouse_person_properties',
Elements = 'elements',
Events = 'events',
EventProperties = 'event_properties',
Expand Down
6 changes: 6 additions & 0 deletions frontend/src/queries/nodes/InsightViz/GlobalAndOrFilters.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { useActions, useValues } from 'kea'
import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types'
import { FEATURE_FLAGS } from 'lib/constants'
import { featureFlagLogic } from 'lib/logic/featureFlagLogic'
import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic'
import { keyForInsightLogicProps } from 'scenes/insights/sharedUtils'

Expand All @@ -16,6 +18,7 @@ export function GlobalAndOrFilters({ insightProps }: EditorFilterProps): JSX.Ele
const { groupsTaxonomicTypes } = useValues(groupsModel)
const { isTrends, querySource, isDataWarehouseSeries } = useValues(insightVizDataLogic(insightProps))
const { updateQuerySource } = useActions(insightVizDataLogic(insightProps))
const { featureFlags } = useValues(featureFlagLogic)

const taxonomicGroupTypes = [
TaxonomicFilterGroupType.EventProperties,
Expand All @@ -26,6 +29,9 @@ export function GlobalAndOrFilters({ insightProps }: EditorFilterProps): JSX.Ele
TaxonomicFilterGroupType.Elements,
...(isTrends ? [TaxonomicFilterGroupType.Sessions] : []),
TaxonomicFilterGroupType.HogQLExpression,
...(featureFlags[FEATURE_FLAGS.DATA_WAREHOUSE] && featureFlags[FEATURE_FLAGS.HOGQL_INSIGHTS]
? [TaxonomicFilterGroupType.DataWarehousePersonProperties]
: []),
]

return (
Expand Down
3 changes: 3 additions & 0 deletions frontend/src/queries/nodes/InsightViz/TrendsSeries.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ export function TrendsSeries(): JSX.Element | null {
...(isTrends ? [TaxonomicFilterGroupType.Sessions] : []),
TaxonomicFilterGroupType.HogQLExpression,
TaxonomicFilterGroupType.DataWarehouseProperties,
...(featureFlags[FEATURE_FLAGS.DATA_WAREHOUSE] && featureFlags[FEATURE_FLAGS.HOGQL_INSIGHTS]
? [TaxonomicFilterGroupType.DataWarehousePersonProperties]
: []),
]

if (!isInsightQueryNode(querySource)) {
Expand Down
29 changes: 28 additions & 1 deletion frontend/src/queries/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,9 @@
},
{
"$ref": "#/definitions/DataWarehousePropertyFilter"
},
{
"$ref": "#/definitions/DataWarehousePersonPropertyFilter"
}
]
},
Expand Down Expand Up @@ -862,6 +865,29 @@
"required": ["distinct_id_field", "id", "id_field", "kind", "table_name", "timestamp_field"],
"type": "object"
},
"DataWarehousePersonPropertyFilter": {
"additionalProperties": false,
"properties": {
"key": {
"type": "string"
},
"label": {
"type": "string"
},
"operator": {
"$ref": "#/definitions/PropertyOperator"
},
"type": {
"const": "data_warehouse_person_property",
"type": "string"
},
"value": {
"$ref": "#/definitions/PropertyFilterValue"
}
},
"required": ["key", "operator", "type"],
"type": "object"
},
"DataWarehousePropertyFilter": {
"additionalProperties": false,
"properties": {
Expand Down Expand Up @@ -3528,7 +3554,8 @@
"recording",
"group",
"hogql",
"data_warehouse"
"data_warehouse",
"data_warehouse_person_property"
],
"type": "string"
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import { afterMount, kea, path } from 'kea'
import { afterMount, connect, kea, path, selectors } from 'kea'
import { loaders } from 'kea-loaders'
import api from 'lib/api'
import { capitalizeFirstLetter } from 'lib/utils'

import { DataWarehouseViewLink } from '~/types'
import { DatabaseSchemaQueryResponseField } from '~/queries/schema'
import { DataWarehouseViewLink, PropertyDefinition, PropertyType } from '~/types'

import type { dataWarehouseJoinsLogicType } from './dataWarehouseJoinsLogicType'
import { dataWarehouseSceneLogic } from './dataWarehouseSceneLogic'

export const dataWarehouseJoinsLogic = kea<dataWarehouseJoinsLogicType>([
path(['scenes', 'data-warehouse', 'external', 'dataWarehouseJoinsLogic']),
Expand All @@ -19,6 +22,43 @@ export const dataWarehouseJoinsLogic = kea<dataWarehouseJoinsLogicType>([
},
],
}),
connect(() => ({
values: [dataWarehouseSceneLogic, ['externalTablesMap']],
})),
selectors({
personTableJoins: [(s) => [s.joins], (joins) => joins.filter((join) => join.source_table_name === 'persons')],
tablesJoinedToPersons: [
(s) => [s.externalTablesMap, s.personTableJoins],
(externalTablesMap, personTableJoins) => {
return personTableJoins.map((join: DataWarehouseViewLink) => {
// valid join should have a joining table name
const table = externalTablesMap[join.joining_table_name as string]
return {
table,
join,
}
})
},
],
columnsJoinedToPersons: [
(s) => [s.tablesJoinedToPersons],
(tablesJoinedToPersons) => {
return tablesJoinedToPersons.reduce((acc, { table, join }) => {
if (table) {
acc.push(
...table.columns.map((column: DatabaseSchemaQueryResponseField) => ({
id: join.field_name + ': ' + column.key,
name: join.field_name + ': ' + column.key,
table: join.field_name,
property_type: capitalizeFirstLetter(column.type) as PropertyType,
}))
)
}
return acc
}, [] as PropertyDefinition[])
},
],
}),
afterMount(({ actions }) => {
actions.loadJoins()
}),
Expand Down
7 changes: 7 additions & 0 deletions frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,7 @@ export enum PropertyFilterType {
Group = 'group',
HogQL = 'hogql',
DataWarehouse = 'data_warehouse',
DataWarehousePersonProperty = 'data_warehouse_person_property',
}

/** Sync with plugin-server/src/types.ts */
Expand Down Expand Up @@ -679,6 +680,11 @@ export interface DataWarehousePropertyFilter extends BasePropertyFilter {
operator: PropertyOperator
}

export interface DataWarehousePersonPropertyFilter extends BasePropertyFilter {
type: PropertyFilterType.DataWarehousePersonProperty
operator: PropertyOperator
}

/** Sync with plugin-server/src/types.ts */
export interface ElementPropertyFilter extends BasePropertyFilter {
type: PropertyFilterType.Element
Expand Down Expand Up @@ -735,6 +741,7 @@ export type AnyPropertyFilter =
| HogQLPropertyFilter
| EmptyPropertyFilter
| DataWarehousePropertyFilter
| DataWarehousePersonPropertyFilter

export type AnyFilterLike = AnyPropertyFilter | PropertyGroupFilter | PropertyGroupFilterValue

Expand Down
16 changes: 15 additions & 1 deletion plugin-server/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,14 @@ export interface PersonPropertyFilter extends PropertyFilterWithOperator {
type: 'person'
}

export interface DataWarehousePropertyFilter extends PropertyFilterWithOperator {
type: 'data_warehouse'
}

export interface DataWarehousePersonPropertyFilter extends PropertyFilterWithOperator {
type: 'data_warehouse_person_property'
}

/** Sync with posthog/frontend/src/types.ts */
export interface ElementPropertyFilter extends PropertyFilterWithOperator {
type: 'element'
Expand All @@ -886,7 +894,13 @@ export interface CohortPropertyFilter extends PropertyFilterBase {
}

/** Sync with posthog/frontend/src/types.ts */
export type PropertyFilter = EventPropertyFilter | PersonPropertyFilter | ElementPropertyFilter | CohortPropertyFilter
export type PropertyFilter =
| EventPropertyFilter
| PersonPropertyFilter
| ElementPropertyFilter
| CohortPropertyFilter
| DataWarehousePropertyFilter
| DataWarehousePersonPropertyFilter

/** Sync with posthog/frontend/src/types.ts */
export enum StringMatching {
Expand Down
8 changes: 7 additions & 1 deletion posthog/hogql/property.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ def property_to_expr(
or property.type == "person"
or property.type == "group"
or property.type == "data_warehouse"
or property.type == "data_warehouse_person_property"
or property.type == "session"
):
if scope == "person" and property.type != "person":
Expand All @@ -146,9 +147,14 @@ def property_to_expr(
)
operator = cast(Optional[PropertyOperator], property.operator) or PropertyOperator.exact
value = property.value

if property.type == "person" and scope != "person":
chain = ["person", "properties"]
elif property.type == "data_warehouse_person_property":
if isinstance(property.value, str):
table, value = property.value.split(": ")
chain = ["person", table]
else:
raise NotImplementedException("Data warehouse person property filter value must be a string")
elif property.type == "group":
chain = [f"group_{property.group_type_index}", "properties"]
elif property.type == "data_warehouse":
Expand Down
1 change: 1 addition & 0 deletions posthog/models/property/property.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class BehavioralPropertyType(str, Enum):
"session",
"hogql",
"data_warehouse",
"data_warehouse_person_property",
]

PropertyName = str
Expand Down
Loading
Loading