feat(property-filters): add not in cohort ability to filters (#24848)

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

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'
             )
         })
 

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"
             )
         })
 

frontend/src/lib/components/PropertyFilters/components/OperatorValueSelect.tsx

@@ -4,6 +4,7 @@ import {
     allOperatorsMapping,
     chooseOperatorMap,
     isMobile,
+    isOperatorCohort,
     isOperatorFlag,
     isOperatorMulti,
     isOperatorRange,
@@ -72,29 +73,49 @@ 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<string | null>(null)
 
     const [operators, setOperators] = useState([] as Array<PropertyOperator>)
     useEffect(() => {
-        const limitedElementProperty = propertyKey === 'selector' || propertyKey === 'tag_name'
-        const operatorMapping: Record<string, string> = 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<string, string> = chooseOperatorMap(propertyType)
+
         const operators = Object.keys(operatorMapping) as Array<PropertyOperator>
         setOperators(operators)
         if ((currentOperator !== operator && operators.includes(startingOperator)) || !propertyDefinition) {
             setCurrentOperator(startingOperator)
         } 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)

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 = (
+        <OperatorValueSelect
+            propertyDefinitions={propertyDefinitionsByType(
+                filter?.type || PropertyDefinitionType.Event,
+                isGroupPropertyFilter(filter) ? filter?.group_type_index : undefined
+            )}
+            type={filter?.type}
+            propertyKey={filter?.key}
+            operator={isPropertyFilterWithOperator(filter) ? filter.operator : null}
+            value={filter?.value}
+            placeholder="Enter value..."
+            endpoint={filter?.key && activeTaxonomicGroup?.valuesEndpoint?.(filter.key)}
+            eventNames={eventNames}
+            addRelativeDateTimeOptions={allowRelativeDateOptions}
+            onChange={(newOperator, newValue) => {
+                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 (
         <div
             className={clsx('TaxonomicPropertyFilter', {
@@ -157,6 +180,7 @@ export function TaxonomicPropertyFilter({
                         </div>
                     )}
                     <div className="TaxonomicPropertyFilter__row-items">
+                        {showOperatorValueSelect && placeOperatorValueSelectOnLeft && operatorValueSelect}
                         <LemonDropdown
                             overlay={taxonomicFilter}
                             placement="bottom-start"
@@ -184,43 +208,7 @@ export function TaxonomicPropertyFilter({
                                 )}
                             </LemonButton>
                         </LemonDropdown>
-                        {showOperatorValueSelect && (
-                            <OperatorValueSelect
-                                propertyDefinitions={propertyDefinitionsByType(
-                                    filter?.type || PropertyDefinitionType.Event,
-                                    isGroupPropertyFilter(filter) ? filter?.group_type_index : undefined
-                                )}
-                                type={filter?.type}
-                                propertyKey={filter?.key}
-                                operator={isPropertyFilterWithOperator(filter) ? filter.operator : null}
-                                value={filter?.value}
-                                placeholder="Enter value..."
-                                endpoint={filter?.key && activeTaxonomicGroup?.valuesEndpoint?.(filter.key)}
-                                eventNames={eventNames}
-                                addRelativeDateTimeOptions={allowRelativeDateOptions}
-                                onChange={(newOperator, newValue) => {
-                                    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}
                     </div>
                 </div>
             )}

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({

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,20 +386,27 @@ 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),
             value: null, // must specify something to be compatible with existing types
         }
         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
 

frontend/src/lib/utils.tsx

@@ -223,6 +223,11 @@ export const selectorOperatorMap: Record<string, string> = {
     is_not: "≠ doesn't equal",
 }
 
+export const cohortOperatorMap: Record<string, string> = {
+    in: 'user in',
+    not_in: 'user not in',
+}
+
 export const allOperatorsMapping: Record<string, string> = {
     ...dateTimeOperatorMap,
     ...stringOperatorMap,
@@ -231,6 +236,7 @@ export const allOperatorsMapping: Record<string, string> = {
     ...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<keyof typeof PropertyType, Record<string, st
     Boolean: booleanOperatorMap,
     Duration: durationOperatorMap,
     Selector: selectorOperatorMap,
+    Cohort: cohortOperatorMap,
 }
 
 export function chooseOperatorMap(propertyType: PropertyType | undefined): Record<string, string> {
@@ -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 {

frontend/src/queries/examples.ts

@@ -152,6 +152,7 @@ const series: (EventsNode | ActionsNode)[] = [
             {
                 type: PropertyFilterType.Cohort,
                 key: 'id',
+                operator: PropertyOperator.In,
                 value: 2,
             },
         ],