Skip to content

Commit

Permalink
feat(flags): restrict dynamic behavioral cohort targeting in the FF UI (
Browse files Browse the repository at this point in the history
#25529)

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
dmarticus and github-actions[bot] authored Oct 15, 2024
1 parent 6d7a20b commit 34f3eae
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 6 deletions.
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.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ interface PropertyFiltersProps {
allowRelativeDateOptions?: boolean
disabledReason?: string
exactMatchFeatureFlagCohortOperators?: boolean
hideBehavioralCohorts?: boolean
}

export function PropertyFilters({
Expand Down Expand Up @@ -67,6 +68,7 @@ export function PropertyFilters({
allowRelativeDateOptions,
disabledReason = undefined,
exactMatchFeatureFlagCohortOperators = false,
hideBehavioralCohorts,
}: PropertyFiltersProps): JSX.Element {
const logicProps = { propertyFilters, onChange, pageKey, sendAllKeyUpdates }
const { filters, filtersWithNew } = useValues(propertyFilterLogic(logicProps))
Expand All @@ -76,7 +78,7 @@ export function PropertyFilters({
// Update the logic's internal filters when the props change
useEffect(() => {
setFilters(propertyFilters ?? [])
}, [propertyFilters])
}, [propertyFilters, setFilters])

// do not open on initial render, only open if newly inserted
useEffect(() => {
Expand Down Expand Up @@ -131,6 +133,7 @@ export function PropertyFilters({
taxonomicFilterOptionsFromProp={taxonomicFilterOptionsFromProp}
allowRelativeDateOptions={allowRelativeDateOptions}
exactMatchFeatureFlagCohortOperators={exactMatchFeatureFlagCohortOperators}
hideBehavioralCohorts={hideBehavioralCohorts}
/>
)}
errorMessage={errorMessages && errorMessages[index]}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export function TaxonomicPropertyFilter({
taxonomicFilterOptionsFromProp,
allowRelativeDateOptions,
exactMatchFeatureFlagCohortOperators,
hideBehavioralCohorts,
}: PropertyFilterInternalProps): JSX.Element {
const pageKey = useMemo(() => pageKeyInput || `filter-${uniqueMemoizedIndex++}`, [pageKeyInput])
const groupTypes = taxonomicGroupTypes || [
Expand Down Expand Up @@ -114,6 +115,7 @@ export function TaxonomicPropertyFilter({
schemaColumns={schemaColumns}
propertyAllowList={propertyAllowList}
optionsFromProp={taxonomicFilterOptionsFromProp}
hideBehavioralCohorts={hideBehavioralCohorts}
/>
)

Expand Down
1 change: 1 addition & 0 deletions frontend/src/lib/components/PropertyFilters/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,5 @@ export interface PropertyFilterInternalProps {
propertyAllowList?: { [key in TaxonomicFilterGroupType]?: string[] }
allowRelativeDateOptions?: boolean
exactMatchFeatureFlagCohortOperators?: boolean
hideBehavioralCohorts?: boolean
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export function TaxonomicFilter({
popoverEnabled = true,
selectFirstItem = true,
propertyAllowList,
hideBehavioralCohorts,
}: TaxonomicFilterProps): JSX.Element {
// Generate a unique key for each unique TaxonomicFilter that's rendered
const taxonomicFilterLogicKey = useMemo(
Expand All @@ -60,6 +61,7 @@ export function TaxonomicFilter({
excludedProperties,
metadataSource,
propertyAllowList,
hideBehavioralCohorts,
}

const logic = taxonomicFilterLogic(taxonomicFilterLogicProps)
Expand All @@ -70,7 +72,7 @@ export function TaxonomicFilter({
if (groupType !== TaxonomicFilterGroupType.HogQLExpression) {
window.setTimeout(() => focusInput(), 1)
}
}, [])
}, [groupType])

const style = {
...(width ? { width } : {}),
Expand Down
43 changes: 43 additions & 0 deletions frontend/src/lib/components/TaxonomicFilter/cohortFilterUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { BehavioralFilterKey } from 'scenes/cohorts/CohortFilters/types'

import { AnyCohortCriteriaType, CohortCriteriaGroupFilter, CohortType } from '~/types'

function isCohortCriteriaGroupFilter(
value: AnyCohortCriteriaType | CohortCriteriaGroupFilter
): value is CohortCriteriaGroupFilter {
return (value as CohortCriteriaGroupFilter).type === 'AND' || (value as CohortCriteriaGroupFilter).type === 'OR'
}

const hasBehavioralFilter = (cohort: CohortType, allCohorts: CohortType[]): boolean => {
const checkCriteriaGroup = (group: CohortCriteriaGroupFilter): boolean => {
return group.values.some((value) => {
if (isCohortCriteriaGroupFilter(value)) {
return checkCriteriaGroup(value)
}
if (value.type === BehavioralFilterKey.Behavioral) {
return true
}
if (value.type === BehavioralFilterKey.Cohort) {
// the first time we load the page we haven't transformed the cohort data,
// so there's no value_property, and we need to use `value.value` instead.
const cohortId = value.value_property || value.value
const nestedCohort = allCohorts.find((item) => item.id === cohortId)
if (nestedCohort) {
return hasBehavioralFilter(nestedCohort, allCohorts)
}
return false
}
return false
})
}

return cohort.filters?.properties ? checkCriteriaGroup(cohort.filters.properties) : false
}

export const filterOutBehavioralCohorts = (items: CohortType[], hideBehavioralCohorts?: boolean): CohortType[] => {
if (!hideBehavioralCohorts) {
return items
}

return items.filter((item) => !hasBehavioralFilter(item, items))
}
15 changes: 13 additions & 2 deletions frontend/src/lib/components/TaxonomicFilter/infiniteListLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { CohortType, EventDefinition } from '~/types'

import { teamLogic } from '../../../scenes/teamLogic'
import { captureTimeToSeeData } from '../../internalMetrics'
import { filterOutBehavioralCohorts } from './cohortFilterUtils'
import type { infiniteListLogicType } from './infiniteListLogicType'

/*
Expand Down Expand Up @@ -240,11 +241,21 @@ export const infiniteListLogic = kea<infiniteListLogicType>([
hasRemoteDataSource: [(s) => [s.remoteEndpoint], (remoteEndpoint) => !!remoteEndpoint],
rawLocalItems: [
(selectors) => [
(state, props) => {
(state, props: InfiniteListLogicProps) => {
const taxonomicGroups = selectors.taxonomicGroups(state)
const group = taxonomicGroups.find((g) => g.type === props.listGroupType)

if (group?.logic && group?.value) {
return group.logic.selectors[group.value]?.(state) || null
const items = group.logic.selectors[group.value]?.(state)
// TRICKY: Feature flags don't support dynamic behavioral cohorts,
// so we don't want to show them as selectable options in the taxonomic filter
// in the feature flag UI.
// TODO: Once we support dynamic behavioral cohorts, we should show them in the taxonomic filter,
// and remove this kludge.
if (Array.isArray(items) && items.every((item) => 'filters' in item)) {
return filterOutBehavioralCohorts(items, props.hideBehavioralCohorts)
}
return items
}
if (group?.options) {
return group.options
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 @@ -37,6 +37,7 @@ export interface TaxonomicFilterProps {
excludedProperties?: { [key in TaxonomicFilterGroupType]?: TaxonomicFilterValue[] }
propertyAllowList?: { [key in TaxonomicFilterGroupType]?: string[] } // only return properties in this list, currently only working for EventProperties and PersonProperties
metadataSource?: AnyDataNode
hideBehavioralCohorts?: boolean
}

export interface TaxonomicFilterLogicProps extends TaxonomicFilterProps {
Expand Down
14 changes: 12 additions & 2 deletions frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ export function FeatureFlagReleaseConditions({
: null
}
exactMatchFeatureFlagCohortOperators={true}
hideBehavioralCohorts={true}
/>
</div>
)}
Expand Down Expand Up @@ -453,8 +454,17 @@ export function FeatureFlagReleaseConditions({
<>
<h3 className="l3">Release conditions</h3>
<div className="text-muted mb-4">
Specify the {aggregationTargetName} to which you want to release this flag. Note
that condition sets are rolled out independently of each other.
Specify {aggregationTargetName} for flag release. Condition sets roll out
independently.
{aggregationTargetName === 'users' && (
<>
{' '}
Cohort-based targeting{' '}
<Link to="https://posthog.com/docs/data/cohorts#can-you-use-a-dynamic-behavioral-cohort-as-a-feature-flag-target">
doesn't support dynamic behavioral cohorts.
</Link>{' '}
</>
)}
</div>
</>
)}
Expand Down

0 comments on commit 34f3eae

Please sign in to comment.