diff --git a/frontend/__snapshots__/components-activitylog--feature-flag-activity--dark.png b/frontend/__snapshots__/components-activitylog--feature-flag-activity--dark.png index 4e1c10ae84224..60b91a1db41ae 100644 Binary files a/frontend/__snapshots__/components-activitylog--feature-flag-activity--dark.png and b/frontend/__snapshots__/components-activitylog--feature-flag-activity--dark.png differ diff --git a/frontend/__snapshots__/components-activitylog--feature-flag-activity--light.png b/frontend/__snapshots__/components-activitylog--feature-flag-activity--light.png index 92e650848ddf5..08871b83227a1 100644 Binary files a/frontend/__snapshots__/components-activitylog--feature-flag-activity--light.png and b/frontend/__snapshots__/components-activitylog--feature-flag-activity--light.png differ diff --git a/frontend/__snapshots__/components-activitylog--with-caption--dark.png b/frontend/__snapshots__/components-activitylog--with-caption--dark.png index 2ed64b93d727e..0986471ed0ac5 100644 Binary files a/frontend/__snapshots__/components-activitylog--with-caption--dark.png and b/frontend/__snapshots__/components-activitylog--with-caption--dark.png differ diff --git a/frontend/__snapshots__/components-activitylog--with-caption--light.png b/frontend/__snapshots__/components-activitylog--with-caption--light.png index e29d7c742cb87..47150ea852cd9 100644 Binary files a/frontend/__snapshots__/components-activitylog--with-caption--light.png and b/frontend/__snapshots__/components-activitylog--with-caption--light.png differ diff --git a/frontend/__snapshots__/filters-property-filter-button--filter-types--dark.png b/frontend/__snapshots__/filters-property-filter-button--filter-types--dark.png index 0672903871c50..5058ccb17627f 100644 Binary files a/frontend/__snapshots__/filters-property-filter-button--filter-types--dark.png and b/frontend/__snapshots__/filters-property-filter-button--filter-types--dark.png differ diff --git a/frontend/__snapshots__/filters-property-filter-button--filter-types--light.png b/frontend/__snapshots__/filters-property-filter-button--filter-types--light.png index 4873b57513359..08358dc7bbe46 100644 Binary files a/frontend/__snapshots__/filters-property-filter-button--filter-types--light.png and b/frontend/__snapshots__/filters-property-filter-button--filter-types--light.png differ diff --git a/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--dark.png b/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--dark.png index 05f787dc5572a..5a6e9d130d4f4 100644 Binary files a/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--dark.png and b/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--light.png b/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--light.png index c9d1b05ba76e6..5181554e9dc89 100644 Binary files a/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--light.png and b/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--light.png differ diff --git a/frontend/__snapshots__/scenes-app-feature-flags--edit-multi-variate-feature-flag--light.png b/frontend/__snapshots__/scenes-app-feature-flags--edit-multi-variate-feature-flag--light.png index 841b3867bd4b3..8a38841b556f3 100644 Binary files a/frontend/__snapshots__/scenes-app-feature-flags--edit-multi-variate-feature-flag--light.png and b/frontend/__snapshots__/scenes-app-feature-flags--edit-multi-variate-feature-flag--light.png differ diff --git a/frontend/__snapshots__/scenes-app-feature-flags--feature-flags-list--dark.png b/frontend/__snapshots__/scenes-app-feature-flags--feature-flags-list--dark.png index 69a2ebf8a4015..a03810d6e8e54 100644 Binary files a/frontend/__snapshots__/scenes-app-feature-flags--feature-flags-list--dark.png and b/frontend/__snapshots__/scenes-app-feature-flags--feature-flags-list--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-feature-flags--feature-flags-list--light.png b/frontend/__snapshots__/scenes-app-feature-flags--feature-flags-list--light.png index dc8003bd35ebd..ca5b6259a6710 100644 Binary files a/frontend/__snapshots__/scenes-app-feature-flags--feature-flags-list--light.png and b/frontend/__snapshots__/scenes-app-feature-flags--feature-flags-list--light.png differ diff --git a/frontend/src/lib/components/ActivityLog/activityLogLogic.feature-flag.test.tsx b/frontend/src/lib/components/ActivityLog/activityLogLogic.feature-flag.test.tsx index 070bbc2fd0b47..a3541e78e3025 100644 --- a/frontend/src/lib/components/ActivityLog/activityLogLogic.feature-flag.test.tsx +++ b/frontend/src/lib/components/ActivityLog/activityLogLogic.feature-flag.test.tsx @@ -340,7 +340,7 @@ describe('the activity log logic', () => { key: 'id', type: 'cohort', value: 98, - operator: null, + operator: 'in', }, ], rollout_percentage: null, @@ -351,7 +351,7 @@ describe('the activity log logic', () => { key: 'id', type: 'cohort', value: 411, - operator: null, + operator: 'not_in', }, ], rollout_percentage: 100, @@ -364,7 +364,7 @@ describe('the activity log logic', () => { const actual = logic.values.humanizedActivity expect(render(<>{actual[0].description}).container).toHaveTextContent( - 'peter changed the filter conditions to apply to 100% of ID 98, and 100% of ID 411 on with cohort' + 'peter changed the filter conditions to apply to 100% of User in ID 98, and 100% of User not in ID 411 on with cohort' ) }) diff --git a/frontend/src/lib/components/ActivityLog/activityLogLogic.insight.test.tsx b/frontend/src/lib/components/ActivityLog/activityLogLogic.insight.test.tsx index c86e548e0b8ba..2f290332b7e19 100644 --- a/frontend/src/lib/components/ActivityLog/activityLogLogic.insight.test.tsx +++ b/frontend/src/lib/components/ActivityLog/activityLogLogic.insight.test.tsx @@ -134,6 +134,7 @@ describe('the activity log logic', () => { { type: 'cohort', key: 'id', + operator: 'in', value: 2, }, ], @@ -158,7 +159,7 @@ describe('the activity log logic', () => { let renderedExtendedDescription = render(<>{actual[0].extendedDescription}).container expect(renderedExtendedDescription).toHaveTextContent( - "Query summaryAShowing \"Views\"Pageviewcounted by total countwhere event'sBrowser= equals Chromeand person belongs to cohortID 2FiltersEvent'sCurrent URL= equals https://hedgebox.net/files/or event'sCountry Code= equals US or AUBreakdown byCountry Code" + "Query summaryAShowing \"Views\"Pageviewcounted by total countwhere event'sBrowser= equals Chromeand person belongs to cohortUser in ID 2FiltersEvent'sCurrent URL= equals https://hedgebox.net/files/or event'sCountry Code= equals US or AUBreakdown byCountry Code" ) ;(insightMock.after.breakdownFilter as BreakdownFilter) = { breakdowns: [ @@ -181,7 +182,7 @@ describe('the activity log logic', () => { renderedExtendedDescription = render(<>{actual[0].extendedDescription}).container expect(renderedExtendedDescription).toHaveTextContent( - "Query summaryAShowing \"Views\"Pageviewcounted by total countwhere event'sBrowser= equals Chromeand person belongs to cohortID 2FiltersEvent'sCurrent URL= equals https://hedgebox.net/files/or event'sCountry Code= equals US or AUBreakdown byCountry CodeSession duration" + "Query summaryAShowing \"Views\"Pageviewcounted by total countwhere event'sBrowser= equals Chromeand person belongs to cohortUser in ID 2FiltersEvent'sCurrent URL= equals https://hedgebox.net/files/or event'sCountry Code= equals US or AUBreakdown byCountry CodeSession duration" ) }) diff --git a/frontend/src/lib/components/PropertyFilters/components/OperatorValueSelect.tsx b/frontend/src/lib/components/PropertyFilters/components/OperatorValueSelect.tsx index 07fc7d9358cca..048c4534072d9 100644 --- a/frontend/src/lib/components/PropertyFilters/components/OperatorValueSelect.tsx +++ b/frontend/src/lib/components/PropertyFilters/components/OperatorValueSelect.tsx @@ -4,6 +4,7 @@ import { allOperatorsMapping, chooseOperatorMap, isMobile, + isOperatorCohort, isOperatorFlag, isOperatorMulti, isOperatorRange, @@ -72,21 +73,35 @@ export function OperatorValueSelect({ }: OperatorValueSelectProps): JSX.Element { const propertyDefinition = propertyDefinitions.find((pd) => pd.name === propertyKey) + const isCohortProperty = propertyKey === 'id' + // DateTime properties should not default to Exact const isDateTimeProperty = propertyDefinition?.property_type == PropertyType.DateTime - const startingOperator = - isDateTimeProperty && (!operator || operator == PropertyOperator.Exact) - ? PropertyOperator.IsDateExact - : operator || PropertyOperator.Exact + + const isInitialOperator = !operator || operator == PropertyOperator.Exact + + let startingOperator = operator || PropertyOperator.Exact + if (isInitialOperator) { + if (isDateTimeProperty) { + startingOperator = PropertyOperator.IsDateExact + } else if (isCohortProperty) { + startingOperator = PropertyOperator.In + } + } + const [currentOperator, setCurrentOperator] = useState(startingOperator) const [validationError, setValidationError] = useState(null) const [operators, setOperators] = useState([] as Array) useEffect(() => { - const limitedElementProperty = propertyKey === 'selector' || propertyKey === 'tag_name' - const operatorMapping: Record = chooseOperatorMap( - limitedElementProperty ? PropertyType.Selector : propertyDefinition?.property_type - ) + let propertyType = propertyDefinition?.property_type + if (propertyKey === 'selector' || propertyKey === 'tag_name') { + propertyType = PropertyType.Selector + } else if (propertyKey === 'id') { + propertyType = PropertyType.Cohort + } + const operatorMapping: Record = chooseOperatorMap(propertyType) + const operators = Object.keys(operatorMapping) as Array setOperators(operators) if ((currentOperator !== operator && operators.includes(startingOperator)) || !propertyDefinition) { @@ -94,7 +109,13 @@ export function OperatorValueSelect({ } else if (!operators.includes(currentOperator) && propertyDefinition) { // Whenever the property type changes such that the operator is not compatible, we need to reset the operator // But, only if the propertyDefinition is available - setCurrentOperator(isDateTimeProperty ? PropertyOperator.IsDateExact : PropertyOperator.Exact) + let defaultProperty = PropertyOperator.Exact + if (isDateTimeProperty) { + defaultProperty = PropertyOperator.IsDateExact + } else if (propertyType === PropertyType.Cohort) { + defaultProperty = PropertyOperator.In + } + setCurrentOperator(defaultProperty) } }, [propertyDefinition, propertyKey, operator]) @@ -114,7 +135,9 @@ export function OperatorValueSelect({ setValidationError(null) setCurrentOperator(newOperator) - if (isOperatorFlag(newOperator)) { + if (isOperatorCohort(newOperator)) { + onChange(newOperator, value || null) + } else if (isOperatorFlag(newOperator)) { onChange(newOperator, newOperator) } else if (isOperatorFlag(currentOperator || PropertyOperator.Exact)) { onChange(newOperator, null) diff --git a/frontend/src/lib/components/PropertyFilters/components/TaxonomicPropertyFilter.tsx b/frontend/src/lib/components/PropertyFilters/components/TaxonomicPropertyFilter.tsx index 68b4b84fdd569..efc30adccec35 100644 --- a/frontend/src/lib/components/PropertyFilters/components/TaxonomicPropertyFilter.tsx +++ b/frontend/src/lib/components/PropertyFilters/components/TaxonomicPropertyFilter.tsx @@ -64,10 +64,7 @@ export function TaxonomicPropertyFilter({ item ) => { selectItem(taxonomicGroup, value, item?.propertyFilterType) - if ( - taxonomicGroup.type === TaxonomicFilterGroupType.Cohorts || - taxonomicGroup.type === TaxonomicFilterGroupType.HogQLExpression - ) { + if (taxonomicGroup.type === TaxonomicFilterGroupType.HogQLExpression) { onComplete?.() } } @@ -87,14 +84,9 @@ export function TaxonomicPropertyFilter({ const valuePresent = filter?.type === 'cohort' || !!filter?.key const showInitialSearchInline = !disablePopover && - ((!filter?.type && (!filter || !(filter as any)?.key)) || - filter?.type === PropertyFilterType.Cohort || - filter?.type === PropertyFilterType.HogQL) - const showOperatorValueSelect = - filter?.type && - filter?.key && - filter?.type !== PropertyFilterType.Cohort && - filter?.type !== PropertyFilterType.HogQL + ((!filter?.type && (!filter || !(filter as any)?.key)) || filter?.type === PropertyFilterType.HogQL) + const showOperatorValueSelect = filter?.type && filter?.key && filter?.type !== PropertyFilterType.HogQL + const placeOperatorValueSelectOnLeft = filter?.type && filter?.key && filter?.type === PropertyFilterType.Cohort const { propertyDefinitionsByType } = useValues(propertyDefinitionsModel) @@ -117,6 +109,37 @@ export function TaxonomicPropertyFilter({ /> ) + const operatorValueSelect = ( + { + if (filter?.key && filter?.type) { + setFilter(index, { + key: filter?.key, + value: newValue || null, + operator: newOperator, + type: filter?.type, + ...(isGroupPropertyFilter(filter) ? { group_type_index: filter.group_type_index } : {}), + } as AnyPropertyFilter) + } + if (newOperator && newValue && !isOperatorMulti(newOperator) && !isOperatorRegex(newOperator)) { + onComplete() + } + }} + /> + ) + return (
)}
+ {showOperatorValueSelect && placeOperatorValueSelectOnLeft && operatorValueSelect} - {showOperatorValueSelect && ( - { - if (filter?.key && filter?.type) { - setFilter(index, { - key: filter?.key, - value: newValue || null, - operator: newOperator, - type: filter?.type, - ...(isGroupPropertyFilter(filter) - ? { group_type_index: filter.group_type_index } - : {}), - } as AnyPropertyFilter) - } - if ( - newOperator && - newValue && - !isOperatorMulti(newOperator) && - !isOperatorRegex(newOperator) - ) { - onComplete() - } - }} - /> - )} + {showOperatorValueSelect && !placeOperatorValueSelectOnLeft && operatorValueSelect}
)} diff --git a/frontend/src/lib/components/PropertyFilters/utils.test.ts b/frontend/src/lib/components/PropertyFilters/utils.test.ts index aa8e9bd093163..e5c06e8e93b6e 100644 --- a/frontend/src/lib/components/PropertyFilters/utils.test.ts +++ b/frontend/src/lib/components/PropertyFilters/utils.test.ts @@ -28,6 +28,7 @@ describe('isValidPropertyFilter()', () => { key: 'id', value: 33, type: PropertyFilterType.Cohort, + operator: PropertyOperator.NotIn, } expect(isValidPropertyFilter(emptyProperty)).toEqual(false) expect(isValidPropertyFilter(realProperty)).toEqual(true) @@ -50,7 +51,12 @@ describe('propertyFilterTypeToTaxonomicFilterType()', () => { it('returns values correctly', () => { expect(propertyFilterTypeToTaxonomicFilterType({} as EmptyPropertyFilter)).toEqual(undefined) expect( - propertyFilterTypeToTaxonomicFilterType({ type: PropertyFilterType.Cohort, key: 'id', value: 33 }) + propertyFilterTypeToTaxonomicFilterType({ + type: PropertyFilterType.Cohort, + operator: PropertyOperator.In, + key: 'id', + value: 33, + }) ).toEqual(TaxonomicFilterGroupType.Cohorts) expect( propertyFilterTypeToTaxonomicFilterType({ diff --git a/frontend/src/lib/components/PropertyFilters/utils.ts b/frontend/src/lib/components/PropertyFilters/utils.ts index 520f16cec3725..be77a88c4211c 100644 --- a/frontend/src/lib/components/PropertyFilters/utils.ts +++ b/frontend/src/lib/components/PropertyFilters/utils.ts @@ -1,6 +1,12 @@ import { TaxonomicFilterGroup, TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types' import { CORE_FILTER_DEFINITIONS_BY_GROUP } from 'lib/taxonomy' -import { allOperatorsMapping, isOperatorFlag } from 'lib/utils' +import { + allOperatorsMapping, + capitalizeFirstLetter, + cohortOperatorMap, + isOperatorCohort, + isOperatorFlag, +} from 'lib/utils' import { propertyDefinitionsModelType } from '~/models/propertyDefinitionsModelType' import { extractExpressionComment } from '~/queries/nodes/DataTable/utils' @@ -120,7 +126,8 @@ export function formatPropertyLabel( const taxonomicFilterGroupType = PROPERTY_FILTER_TYPE_TO_TAXONOMIC_FILTER_GROUP_TYPE[type] return type === 'cohort' - ? cohortsById[value]?.name || `ID ${value}` + ? `${capitalizeFirstLetter(cohortOperatorMap[operator || 'in'] || 'user in')} ` + + (cohortsById[value]?.name || `ID ${value}`) : (CORE_FILTER_DEFINITIONS_BY_GROUP[taxonomicFilterGroupType]?.[key]?.label || key) + (isOperatorFlag(operator) ? ` ${allOperatorsMapping[operator]}` @@ -256,6 +263,7 @@ export function isPropertyFilterWithOperator( isLogEntryPropertyFilter(filter) || isFeaturePropertyFilter(filter) || isGroupPropertyFilter(filter) || + isCohortPropertyFilter(filter) || isDataWarehousePropertyFilter(filter)) ) } @@ -378,13 +386,19 @@ export function createDefaultPropertyFilter( describeProperty: propertyDefinitionsModelType['values']['describeProperty'] ): AnyPropertyFilter { if (propertyType === PropertyFilterType.Cohort) { + const operator = + (isPropertyFilterWithOperator(filter) && isOperatorCohort(filter?.operator) && filter?.operator) || + PropertyOperator.In const cohortProperty: CohortPropertyFilter = { key: 'id', value: parseInt(String(propertyKey)), type: propertyType, + operator: operator, } return cohortProperty - } else if (propertyType === PropertyFilterType.HogQL) { + } + + if (propertyType === PropertyFilterType.HogQL) { const hogQLProperty: HogQLPropertyFilter = { type: propertyType, key: String(propertyKey), @@ -392,6 +406,7 @@ export function createDefaultPropertyFilter( } return hogQLProperty } + const apiType = propertyFilterTypeToPropertyDefinitionType(propertyType) ?? PropertyDefinitionType.Event const propertyValueType = describeProperty(propertyKey, apiType, taxonomicGroup.groupTypeIndex) @@ -405,7 +420,7 @@ export function createDefaultPropertyFilter( } const operator = property_name_to_default_operator_override[propertyKey] || - (isPropertyFilterWithOperator(filter) ? filter.operator : null) || + (isPropertyFilterWithOperator(filter) && !isOperatorCohort(filter.operator) ? filter.operator : null) || property_value_type_to_default_operator_override[propertyValueType ?? ''] || PropertyOperator.Exact diff --git a/frontend/src/lib/utils.tsx b/frontend/src/lib/utils.tsx index 5cddfb467dd22..c38a7bc5e709d 100644 --- a/frontend/src/lib/utils.tsx +++ b/frontend/src/lib/utils.tsx @@ -223,6 +223,11 @@ export const selectorOperatorMap: Record = { is_not: "≠ doesn't equal", } +export const cohortOperatorMap: Record = { + in: 'user in', + not_in: 'user not in', +} + export const allOperatorsMapping: Record = { ...dateTimeOperatorMap, ...stringOperatorMap, @@ -231,6 +236,7 @@ export const allOperatorsMapping: Record = { ...booleanOperatorMap, ...durationOperatorMap, ...selectorOperatorMap, + ...cohortOperatorMap, // slight overkill to spread all of these into the map // but gives freedom for them to diverge more over time } @@ -242,6 +248,7 @@ const operatorMappingChoice: Record { @@ -258,7 +265,14 @@ export function isOperatorMulti(operator: PropertyOperator): boolean { export function isOperatorFlag(operator: PropertyOperator): boolean { // these filter operators can only be just set, no additional parameter - return [PropertyOperator.IsSet, PropertyOperator.IsNotSet].includes(operator) + return [PropertyOperator.IsSet, PropertyOperator.IsNotSet, PropertyOperator.In, PropertyOperator.NotIn].includes( + operator + ) +} + +export function isOperatorCohort(operator: PropertyOperator): boolean { + // these filter operators use value different ( to represent the number of the cohort ) + return [PropertyOperator.In, PropertyOperator.NotIn].includes(operator) } export function isOperatorRegex(operator: PropertyOperator): boolean { diff --git a/frontend/src/queries/examples.ts b/frontend/src/queries/examples.ts index 60cd1c9f1c207..c7d0f5fdfa32c 100644 --- a/frontend/src/queries/examples.ts +++ b/frontend/src/queries/examples.ts @@ -152,6 +152,7 @@ const series: (EventsNode | ActionsNode)[] = [ { type: PropertyFilterType.Cohort, key: 'id', + operator: PropertyOperator.In, value: 2, }, ], diff --git a/frontend/src/queries/nodes/InsightQuery/utils/cleanProperties.test.ts b/frontend/src/queries/nodes/InsightQuery/utils/cleanProperties.test.ts index 2e39095876397..97c388dd7d9dc 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/cleanProperties.test.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/cleanProperties.test.ts @@ -39,7 +39,7 @@ describe('cleanGlobalProperties', () => { expect(result).toEqual({ type: 'AND', - values: [{ type: 'AND', values: [{ key: 'id', type: 'cohort', value: 636 }] }], + values: [{ type: 'AND', values: [{ key: 'id', type: 'cohort', value: 636, operator: null }] }], }) }) @@ -67,7 +67,7 @@ describe('cleanGlobalProperties', () => { values: [ { type: 'AND', - values: [{ key: 'id', type: 'cohort', value: 850 }], + values: [{ key: 'id', type: 'cohort', value: 850, operator: null }], }, ], }) diff --git a/frontend/src/queries/nodes/InsightQuery/utils/cleanProperties.ts b/frontend/src/queries/nodes/InsightQuery/utils/cleanProperties.ts index b623f6f15c73d..4474b6423dbf6 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/cleanProperties.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/cleanProperties.ts @@ -158,7 +158,7 @@ const cleanProperty = (property: Record): AnyPropertyFilter => { } const isPropertyWithOperator = (property: Record): boolean => { - return !['cohort', 'hogql'].includes(property['type']) + return !['hogql'].includes(property['type']) } // old style dict properties e.g. {"utm_medium__icontains": "email"} diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts index 902907830c7d1..c3126c6d36bc3 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts @@ -884,7 +884,7 @@ describe('filtersToQueryNode', () => { key: 'id', type: PropertyFilterType.Cohort, value: 6, - operator: null, + operator: 'in', }, ], }, @@ -909,6 +909,7 @@ describe('filtersToQueryNode', () => { { key: 'id', type: PropertyFilterType.Cohort, + operator: PropertyOperator.In, value: 6, }, ], diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index 93a5a0839e171..5cac82c05a9ec 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -2597,6 +2597,10 @@ "label": { "type": "string" }, + "operator": { + "$ref": "#/definitions/PropertyOperator", + "default": "in" + }, "type": { "const": "cohort", "type": "string" @@ -2605,7 +2609,7 @@ "type": "integer" } }, - "required": ["key", "type", "value"], + "required": ["key", "operator", "type", "value"], "type": "object" }, "CompareFilter": { @@ -7384,7 +7388,9 @@ "between", "not_between", "min", - "max" + "max", + "in", + "not_in" ], "type": "string" }, diff --git a/frontend/src/scenes/insights/insightDataLogic.test.ts b/frontend/src/scenes/insights/insightDataLogic.test.ts index c92b65798eea1..1d46ee553434d 100644 --- a/frontend/src/scenes/insights/insightDataLogic.test.ts +++ b/frontend/src/scenes/insights/insightDataLogic.test.ts @@ -96,6 +96,7 @@ describe('insightDataLogic', () => { { key: 'id', type: 'cohort', + operator: 'in', value: 2, }, ], diff --git a/frontend/src/scenes/session-recordings/playlist/playlistUtils.test.ts b/frontend/src/scenes/session-recordings/playlist/playlistUtils.test.ts index abd889a6ccaf7..41ba5673d2d60 100644 --- a/frontend/src/scenes/session-recordings/playlist/playlistUtils.test.ts +++ b/frontend/src/scenes/session-recordings/playlist/playlistUtils.test.ts @@ -99,6 +99,7 @@ describe('summarizePlaylistFilters()', () => { { key: 'id', type: PropertyFilterType.Cohort, + operator: PropertyOperator.In, value: 1, }, ], @@ -114,6 +115,7 @@ describe('summarizePlaylistFilters()', () => { { key: 'id', type: PropertyFilterType.Cohort, + operator: PropertyOperator.In, value: 1, }, ], @@ -156,6 +158,7 @@ describe('summarizePlaylistFilters()', () => { { key: 'id', type: PropertyFilterType.Cohort, + operator: PropertyOperator.In, value: 1, }, ], diff --git a/frontend/src/scenes/settings/project/TestAccountFiltersConfig.tsx b/frontend/src/scenes/settings/project/TestAccountFiltersConfig.tsx index dd2d580348037..b97ede88d8701 100644 --- a/frontend/src/scenes/settings/project/TestAccountFiltersConfig.tsx +++ b/frontend/src/scenes/settings/project/TestAccountFiltersConfig.tsx @@ -1,22 +1,65 @@ import { LemonSwitch, Link } from '@posthog/lemon-ui' import { useActions, useValues } from 'kea' import { PropertyFilters } from 'lib/components/PropertyFilters/PropertyFilters' +import { PROPERTY_FILTER_TYPE_TO_TAXONOMIC_FILTER_GROUP_TYPE } from 'lib/components/PropertyFilters/utils' import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types' import { LemonBanner } from 'lib/lemon-ui/LemonBanner' +import { getFilterLabel } from 'lib/taxonomy' import { eventUsageLogic } from 'lib/utils/eventUsageLogic' import { teamLogic } from 'scenes/teamLogic' +import { cohortsModel } from '~/models/cohortsModel' import { groupsModel } from '~/models/groupsModel' -import { AnyPropertyFilter } from '~/types' +import { AnyPropertyFilter, type CohortType, PropertyOperator, type TeamPublicType, type TeamType } from '~/types' import { filterTestAccountsDefaultsLogic } from './filterTestAccountDefaultsLogic' +function createTestAccountFilterWarningLabels( + currentTeam: TeamPublicType | TeamType | null, + cohortsById: Partial> +): string[] | null { + if (!currentTeam) { + return null + } + const positiveFilterOperators = [ + PropertyOperator.Exact, + PropertyOperator.IContains, + PropertyOperator.Regex, + PropertyOperator.IsSet, + PropertyOperator.In, + ] + const positiveFilters = [] + for (const filter of currentTeam.test_account_filters || []) { + if ('operator' in filter && !!filter.operator && positiveFilterOperators.includes(filter.operator)) { + positiveFilters.push(filter) + } + } + + return positiveFilters.map((filter) => { + if (!!filter.type && !!filter.key) { + // person properties can be checked for a label as if they were event properties + // so, we can check each acceptable type and see if it returns a value + if (filter.type === 'cohort') { + return `Cohort ${cohortsById[filter.value]?.name || filter.value}` + } + return ( + getFilterLabel(filter.key, PROPERTY_FILTER_TYPE_TO_TAXONOMIC_FILTER_GROUP_TYPE[filter.type]) || + filter.key + ) + } + return filter.key + }) +} + function TestAccountFiltersConfig(): JSX.Element { const { updateCurrentTeam } = useActions(teamLogic) const { setTeamDefault } = useActions(filterTestAccountsDefaultsLogic) const { reportTestAccountFiltersUpdated } = useActions(eventUsageLogic) - const { currentTeam, currentTeamLoading, testAccountFilterWarningLabels, testAccountFilterFrequentMistakes } = - useValues(teamLogic) + const { currentTeam, currentTeamLoading, testAccountFilterFrequentMistakes } = useValues(teamLogic) + const { cohortsById } = useValues(cohortsModel) + + const testAccountFilterWarningLabels = createTestAccountFilterWarningLabels(currentTeam, cohortsById) + const { groupsTaxonomicTypes } = useValues(groupsModel) const handleChange = (filters: AnyPropertyFilter[]): void => { diff --git a/frontend/src/scenes/teamLogic.tsx b/frontend/src/scenes/teamLogic.tsx index 5e22aa188f806..a215e5408c901 100644 --- a/frontend/src/scenes/teamLogic.tsx +++ b/frontend/src/scenes/teamLogic.tsx @@ -1,16 +1,14 @@ import { actions, afterMount, connect, kea, listeners, path, reducers, selectors } from 'kea' import { loaders } from 'kea-loaders' import api, { ApiConfig } from 'lib/api' -import { PROPERTY_FILTER_TYPE_TO_TAXONOMIC_FILTER_GROUP_TYPE } from 'lib/components/PropertyFilters/utils' import { OrganizationMembershipLevel } from 'lib/constants' import { IconSwapHoriz } from 'lib/lemon-ui/icons' import { lemonToast } from 'lib/lemon-ui/LemonToast/LemonToast' -import { getFilterLabel } from 'lib/taxonomy' import { identifierToHuman, isUserLoggedIn, resolveWebhookService } from 'lib/utils' import { eventUsageLogic } from 'lib/utils/eventUsageLogic' import { getAppContext } from 'lib/utils/getAppContext' -import { CorrelationConfigType, PropertyOperator, TeamPublicType, TeamType } from '~/types' +import { CorrelationConfigType, TeamPublicType, TeamType } from '~/types' import { organizationLogic } from './organizationLogic' import type { teamLogicType } from './teamLogicType' @@ -173,44 +171,6 @@ export const teamLogic = kea([ !!currentTeam?.effective_membership_level && currentTeam.effective_membership_level >= OrganizationMembershipLevel.Admin, ], - testAccountFilterWarningLabels: [ - (selectors) => [selectors.currentTeam], - (currentTeam) => { - if (!currentTeam) { - return null - } - const positiveFilterOperators = [ - PropertyOperator.Exact, - PropertyOperator.IContains, - PropertyOperator.Regex, - PropertyOperator.IsSet, - ] - const positiveFilters = [] - for (const filter of currentTeam.test_account_filters || []) { - if ( - 'operator' in filter && - !!filter.operator && - positiveFilterOperators.includes(filter.operator) - ) { - positiveFilters.push(filter) - } - } - - return positiveFilters.map((filter) => { - if (!!filter.type && !!filter.key) { - // person properties can be checked for a label as if they were event properties - // so, we can check each acceptable type and see if it returns a value - return ( - getFilterLabel( - filter.key, - PROPERTY_FILTER_TYPE_TO_TAXONOMIC_FILTER_GROUP_TYPE[filter.type] - ) || filter.key - ) - } - return filter.key - }) - }, - ], testAccountFilterFrequentMistakes: [ (selectors) => [selectors.currentTeam], (currentTeam): FrequentMistakeAdvice[] => { diff --git a/frontend/src/types.ts b/frontend/src/types.ts index cfdd444d99f1a..9d5a633dc2d42 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -640,6 +640,8 @@ export enum PropertyOperator { NotBetween = 'not_between', Minimum = 'min', Maximum = 'max', + In = 'in', + NotIn = 'not_in', } export enum SavedInsightsTabs { @@ -770,6 +772,8 @@ export interface CohortPropertyFilter extends BasePropertyFilter { key: 'id' /** @asType integer */ value: number + /** @default 'in' */ + operator: PropertyOperator } export interface GroupPropertyFilter extends BasePropertyFilter { @@ -3102,6 +3106,7 @@ export enum PropertyType { Boolean = 'Boolean', Duration = 'Duration', Selector = 'Selector', + Cohort = 'Cohort', } export enum PropertyDefinitionType { diff --git a/posthog/hogql/property.py b/posthog/hogql/property.py index 65db42700c958..9474e5d155885 100644 --- a/posthog/hogql/property.py +++ b/posthog/hogql/property.py @@ -494,7 +494,9 @@ def property_to_expr( cohort = Cohort.objects.get(team=team, id=property.value) return ast.CompareOperation( left=ast.Field(chain=["id" if scope == "person" else "person_id"]), - op=ast.CompareOperationOp.InCohort, + op=ast.CompareOperationOp.NotInCohort + if property.operator == PropertyOperator.NOT_IN.value + else ast.CompareOperationOp.InCohort, right=ast.Constant(value=cohort.pk), ) diff --git a/posthog/models/property/property.py b/posthog/models/property/property.py index 59fbec3a78626..22cd07cf3f862 100644 --- a/posthog/models/property/property.py +++ b/posthog/models/property/property.py @@ -67,6 +67,8 @@ class BehavioralPropertyType(StrEnum): "is_date_exact", "is_date_after", "is_date_before", + "in", + "not_in", ] OperatorInterval = Literal["day", "week", "month", "year"] diff --git a/posthog/schema.py b/posthog/schema.py index 4af2a7339913b..4ad9ba2d341e0 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -246,16 +246,6 @@ class ClickhouseQueryProgress(BaseModel): time_elapsed: int -class CohortPropertyFilter(BaseModel): - model_config = ConfigDict( - extra="forbid", - ) - key: Literal["id"] = "id" - label: Optional[str] = None - type: Literal["cohort"] = "cohort" - value: int - - class CompareFilter(BaseModel): model_config = ConfigDict( extra="forbid", @@ -977,6 +967,8 @@ class PropertyOperator(StrEnum): NOT_BETWEEN = "not_between" MIN = "min" MAX = "max" + IN_ = "in" + NOT_IN = "not_in" class QueryResponseAlternative5(BaseModel): @@ -2281,6 +2273,17 @@ class ChartSettings(BaseModel): ) +class CohortPropertyFilter(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + key: Literal["id"] = "id" + label: Optional[str] = None + operator: Optional[PropertyOperator] = PropertyOperator.IN_ + type: Literal["cohort"] = "cohort" + value: int + + class Response(BaseModel): model_config = ConfigDict( extra="forbid", diff --git a/posthog/test/test_migration_0459.py b/posthog/test/test_migration_0459.py index 444f548d46bff..35495ea7c5ff0 100644 --- a/posthog/test/test_migration_0459.py +++ b/posthog/test/test_migration_0459.py @@ -115,7 +115,7 @@ def test_migration(self) -> None: "kind": "DataTableNode", "source": { "kind": "ActorsQuery", - "properties": [{"key": "id", "type": "cohort", "value": 4669}], + "properties": [{"key": "id", "type": "cohort", "operator": "in", "value": 4669}], }, }, ) @@ -155,7 +155,7 @@ def test_migration(self) -> None: "kind": "ActorsQuery", "properties": [ {"key": "email", "type": "person", "value": "is_set", "operator": "is_set"}, - {"key": "id", "type": "cohort", "value": 3}, + {"key": "id", "type": "cohort", "operator": "in", "value": 3}, ], }, "propertiesViaUrl": True, @@ -185,7 +185,7 @@ def test_migration(self) -> None: "properties": [ {"key": "name", "type": "person", "value": "is_set", "operator": "is_set"}, {"key": "surname", "type": "person", "value": "is_set", "operator": "is_set"}, - {"key": "id", "type": "cohort", "value": 3}, + {"key": "id", "type": "cohort", "operator": "in", "value": 3}, ], "limit": 100, "offset": 100,