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

fix(insights): fix legacy transformation of properties #21165

Merged
merged 16 commits into from
Mar 27, 2024
Binary file modified frontend/__snapshots__/scenes-app-batchexports--exports--dark.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/scenes-app-batchexports--exports--light.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
135 changes: 135 additions & 0 deletions frontend/src/queries/nodes/InsightQuery/utils/cleanProperties.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
import { cleanEntityProperties, cleanGlobalProperties } from './cleanProperties'

describe('cleanGlobalProperties', () => {
it('handles empty properties', () => {
const properties = {}

const result = cleanGlobalProperties(properties)

expect(result).toEqual(undefined)
})

it('handles old style properties', () => {
const properties = { utm_medium__icontains: 'email' }

const result = cleanGlobalProperties(properties)

expect(result).toEqual({
type: 'AND',
values: [
{
type: 'AND',
values: [
{
key: 'utm_medium',
operator: 'icontains',
type: 'event',
value: 'email',
},
],
},
],
})
})

it('handles property filter lists', () => {
const properties = [{ key: 'id', type: 'cohort', value: 636, operator: null }]

const result = cleanGlobalProperties(properties)

expect(result).toEqual({
type: 'AND',
values: [{ type: 'AND', values: [{ key: 'id', type: 'cohort', value: 636 }] }],
})
})

it('handles property group filters', () => {
const properties = {
type: 'AND',
values: [{ type: 'AND', values: [{ key: 'id', type: 'cohort', value: 850, operator: null }] }],
}

const result = cleanGlobalProperties(properties)

expect(result).toEqual(properties)
})

it('handles property group filters values', () => {
const properties = {
type: 'AND',
values: [{ key: 'id', type: 'cohort', value: 850, operator: null }],
}

const result = cleanGlobalProperties(properties)

expect(result).toEqual({
type: 'AND',
values: [
{
type: 'AND',
values: [{ key: 'id', type: 'cohort', value: 850 }],
},
],
})
})
})

describe('cleanEntityProperties', () => {
it('handles empty properties', () => {
const properties = {}

const result = cleanEntityProperties(properties)

expect(result).toEqual(undefined)
})

it('handles old style properties', () => {
const properties = { utm_medium__icontains: 'email' }

const result = cleanEntityProperties(properties)

expect(result).toEqual([
{
key: 'utm_medium',
operator: 'icontains',
type: 'event',
value: 'email',
},
])
})

it('handles property filter lists', () => {
const properties = [
{ key: '$current_url', type: 'event', value: 'https://hedgebox.net/signup/', operator: 'exact' },
]

const result = cleanEntityProperties(properties)

expect(result).toEqual(properties)
})

it('handles property group values', () => {
const properties = {
type: 'AND',
values: [
{
key: '$current_url',
operator: 'exact',
type: 'event',
value: 'https://hedgebox.net/signup/',
},
],
}

const result = cleanEntityProperties(properties)

expect(result).toEqual([
{
key: '$current_url',
operator: 'exact',
type: 'event',
value: 'https://hedgebox.net/signup/',
},
])
})
})
180 changes: 180 additions & 0 deletions frontend/src/queries/nodes/InsightQuery/utils/cleanProperties.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
import {
AnyPropertyFilter,
EventPropertyFilter,
PropertyFilterType,
PropertyGroupFilter,
PropertyGroupFilterValue,
PropertyOperator,
} from '~/types'

/** Cleans properties of insights. These are either a simple list of property filters or a property group filter. The property group filter has
* a type (AND, OR) and a list of values that are property group filter values, which are either property group filter values or a simple list of
* property filters.
*/
export const cleanGlobalProperties = (
properties: Record<string, any> | Record<string, any>[] | undefined
): AnyPropertyFilter[] | PropertyGroupFilter | undefined => {
if (
properties == undefined ||
(Array.isArray(properties) && properties.length == 0) ||
Object.keys(properties).length == 0
) {
// empty properties
return undefined
} else if (isOldStyleProperties(properties)) {
// old style properties
properties = transformOldStyleProperties(properties)
properties = {
type: 'AND',
values: [{ type: 'AND', values: properties }],
}
return cleanPropertyGroupFilter(properties)
} else if (Array.isArray(properties)) {
// list of property filters
properties = {
type: 'AND',
values: [{ type: 'AND', values: properties }],
}
return cleanPropertyGroupFilter(properties)
} else if (
(properties['type'] === 'AND' || properties['type'] === 'OR') &&
!properties['values'].some((property: any) => property['type'] === 'AND' || property['type'] === 'OR')
) {
// property group filter value
properties = {
type: 'AND',
values: [properties],
}
return cleanPropertyGroupFilter(properties)
} else {
// property group filter
return cleanPropertyGroupFilter(properties)
}
}

/** Cleans properties of entities i.e. event and action nodes. These are a simple list of property filters. */
export const cleanEntityProperties = (
properties: Record<string, any> | Record<string, any>[] | undefined
): AnyPropertyFilter[] | undefined => {
if (
properties == undefined ||
(Array.isArray(properties) && properties.length == 0) ||
Object.keys(properties).length == 0
) {
// empty properties
return undefined
} else if (isOldStyleProperties(properties)) {
// old style properties
return transformOldStyleProperties(properties)
} else if (Array.isArray(properties)) {
// list of property filters
return properties.map(cleanProperty)
} else if (
(properties['type'] === 'AND' || properties['type'] === 'OR') &&
!properties['values'].some((property: any) => property['type'] === 'AND' || property['type'] === 'OR')
) {
// property group filter value
return properties.values.map(cleanProperty)
} else {
throw new Error('Unexpected format of entity properties.')
}
}

const cleanPropertyGroupFilter = (properties: Record<string, any>): PropertyGroupFilter => {
properties['values'] = cleanPropertyGroupFilterValues(properties.values)
return properties as PropertyGroupFilter
}

const cleanPropertyGroupFilterValues = (
properties: (AnyPropertyFilter | PropertyGroupFilterValue)[]
): (AnyPropertyFilter | PropertyGroupFilterValue)[] => {
return properties.map(cleanPropertyGroupFilterValue)
}

const cleanPropertyGroupFilterValue = (
property: AnyPropertyFilter | PropertyGroupFilterValue
): AnyPropertyFilter | PropertyGroupFilterValue => {
if (property['type'] == 'AND' || property['type'] == 'OR') {
// property group filter value
property['values'] = cleanPropertyGroupFilterValues(property['values'] as PropertyGroupFilterValue[])
return property
} else {
// property filter
return cleanProperty(property)
}
}

const cleanProperty = (property: Record<string, any>): AnyPropertyFilter => {
// fix type typo
if (property['type'] === 'events') {
property['type'] = 'event'
}

// fix value key typo
if (property['values'] !== undefined && property['value'] === undefined) {
property['value'] = property['values']
delete property['values']
}

// convert precalculated and static cohorts to cohorts
if (['precalculated-cohort', 'static-cohort'].includes(property['type'])) {
property['type'] = 'cohort'
}

// fix invalid property key for cohorts
if (property['type'] === 'cohort' && property['key'] !== 'id') {
property['key'] = 'id'
}

// set a default operator for properties that support it, but don't have an operator set
if (isPropertyWithOperator(property) && property['operator'] === undefined) {
property['operator'] = 'exact'
}

// remove the operator for properties that don't support it, but have it set
if (!isPropertyWithOperator(property) && property['operator'] !== undefined) {
delete property['operator']
}

// remove none from values
if (Array.isArray(property['value'])) {
property['value'] = property['value'].filter((x) => x !== null)
}

// remove keys without concrete value
Object.keys(property).forEach((key) => {
if (property[key] === undefined) {
delete property[key]
}
})

return property
}

const isPropertyWithOperator = (property: Record<string, any>): boolean => {
return !['cohort', 'hogql'].includes(property['type'])
}

// old style dict properties e.g. {"utm_medium__icontains": "email"}
const isOldStyleProperties = (properties: Record<string, any> | Record<string, any>[]): boolean => {
return (
!Array.isArray(properties) && Object.keys(properties).length === 1 && !['AND', 'OR'].includes(properties.type)
)
}

const transformOldStyleProperties = (
properties: Record<string, any> | Record<string, any>[]
): EventPropertyFilter[] => {
const key = Object.keys(properties)[0]
const value = Object.values(properties)[0]
const keySplit = key.split('__')

return [
{
key: keySplit[0],
value: value,
operator: keySplit.length > 1 ? (keySplit[1] as PropertyOperator) : PropertyOperator.Exact,
type: PropertyFilterType.Event,
},
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,12 @@ describe('filtersToQueryNode', () => {
kind: NodeKind.LifecycleQuery,
properties: {
type: FilterLogicalOperator.And,
values: [],
values: [
{
type: FilterLogicalOperator.And,
values: [],
},
],
},
filterTestAccounts: true,
dateRange: {
Expand Down Expand Up @@ -1394,7 +1399,12 @@ describe('filtersToQueryNode', () => {
},
properties: {
type: FilterLogicalOperator.And,
values: [],
values: [
{
type: FilterLogicalOperator.And,
values: [],
},
],
},
}
expect(result).toEqual(query)
Expand Down
Loading
Loading