diff --git a/.eslintrc.js b/.eslintrc.js index 7aa24b651e2d9..b66539acec105 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -78,6 +78,7 @@ module.exports = { 'error', { ignoreRestSiblings: true, + destructuredArrayIgnorePattern: '^_$', }, ], '@typescript-eslint/prefer-ts-expect-error': 'error', diff --git a/ee/clickhouse/models/test/__snapshots__/test_cohort.ambr b/ee/clickhouse/models/test/__snapshots__/test_cohort.ambr index e45231b084eca..11c70ce750537 100644 --- a/ee/clickhouse/models/test/__snapshots__/test_cohort.ambr +++ b/ee/clickhouse/models/test/__snapshots__/test_cohort.ambr @@ -84,7 +84,8 @@ (SELECT pdi.person_id AS person_id, countIf(timestamp > now() - INTERVAL 2 year AND timestamp < now() - AND event = '$pageview') > 0 AS performed_event_condition_15_level_level_0_level_0_level_0_0 + AND event = '$pageview' + AND 1=1) > 0 AS performed_event_condition_15_level_level_0_level_0_level_0_0 FROM events e INNER JOIN (SELECT distinct_id, @@ -149,7 +150,8 @@ (SELECT pdi.person_id AS person_id, countIf(timestamp > now() - INTERVAL 2 year AND timestamp < now() - AND event = '$pageview') > 0 AS performed_event_condition_17_level_level_0_level_0_level_0_0 + AND event = '$pageview' + AND 1=1) > 0 AS performed_event_condition_17_level_level_0_level_0_level_0_0 FROM events e INNER JOIN (SELECT distinct_id, @@ -238,7 +240,8 @@ (SELECT pdi.person_id AS person_id, countIf(timestamp > now() - INTERVAL 2 year AND timestamp < now() - AND event = '$pageview') > 0 AS performed_event_condition_19_level_level_0_level_0_level_0_0, + AND event = '$pageview' + AND 1=1) > 0 AS performed_event_condition_19_level_level_0_level_0_level_0_0, minIf(timestamp, event = 'signup') >= now() - INTERVAL 15 day AND minIf(timestamp, event = 'signup') < now() as first_time_condition_19_level_level_0_level_1_level_0_level_0_level_0_0 FROM events e diff --git a/ee/clickhouse/queries/test/__snapshots__/test_cohort_query.ambr b/ee/clickhouse/queries/test/__snapshots__/test_cohort_query.ambr index ecb3645f37924..ac6833f1a0a51 100644 --- a/ee/clickhouse/queries/test/__snapshots__/test_cohort_query.ambr +++ b/ee/clickhouse/queries/test/__snapshots__/test_cohort_query.ambr @@ -7,10 +7,12 @@ (SELECT pdi.person_id AS person_id, countIf(timestamp > now() - INTERVAL 1 day AND timestamp < now() - AND event = '$pageview') > 0 AS performed_event_condition_None_level_level_0_level_0_level_0_0, + AND event = '$pageview' + AND 1=1) > 0 AS performed_event_condition_None_level_level_0_level_0_level_0_0, countIf(timestamp > now() - INTERVAL 2 week AND timestamp < now() - AND event = '$pageview') > 0 AS performed_event_condition_None_level_level_0_level_0_level_1_0, + AND event = '$pageview' + AND 1=1) > 0 AS performed_event_condition_None_level_level_0_level_0_level_1_0, minIf(timestamp, ((replaceRegexpAll(JSONExtractRaw(properties, '$current_url'), '^"|"$', '') = 'https://posthog.com/feedback/123' AND event = '$autocapture'))) >= now() - INTERVAL 2 week AND minIf(timestamp, ((replaceRegexpAll(JSONExtractRaw(properties, '$current_url'), '^"|"$', '') = 'https://posthog.com/feedback/123' @@ -126,7 +128,8 @@ (SELECT pdi.person_id AS person_id, countIf(timestamp > now() - INTERVAL 1 week AND timestamp < now() - AND event = '$pageview') > 0 AS performed_event_condition_None_level_level_0_level_0_0 + AND event = '$pageview' + AND 1=1) > 0 AS performed_event_condition_None_level_level_0_level_0_0 FROM events e INNER JOIN (SELECT distinct_id, @@ -167,7 +170,8 @@ (SELECT pdi.person_id AS person_id, countIf(timestamp > now() - INTERVAL 1 week AND timestamp < now() - AND event = '$pageview') > 0 AS performed_event_condition_None_level_level_0_level_1_level_0_0 + AND event = '$pageview' + AND 1=1) > 0 AS performed_event_condition_None_level_level_0_level_1_level_0_0 FROM events e INNER JOIN (SELECT distinct_id, @@ -250,7 +254,8 @@ AND event_0_latest_1 <= event_0_latest_0 + INTERVAL 3 day, 2, 1)) = 2 AS steps_0, countIf(timestamp > now() - INTERVAL 1 week AND timestamp < now() - AND event = '$new_view') >= 1 AS performed_event_multiple_condition_None_level_level_0_level_1_0 + AND event = '$new_view' + AND 1=1) >= 1 AS performed_event_multiple_condition_None_level_level_0_level_1_0 FROM (SELECT person_id, event, @@ -297,7 +302,8 @@ AND event_0_latest_1 <= event_0_latest_0 + INTERVAL 3 day, 2, 1)) = 2 AS steps_0, countIf(timestamp > now() - INTERVAL 1 week AND timestamp < now() - AND event = '$pageview') >= 1 AS performed_event_multiple_condition_None_level_level_0_level_1_0 + AND event = '$pageview' + AND 1=1) >= 1 AS performed_event_multiple_condition_None_level_level_0_level_1_0 FROM (SELECT person_id, event, @@ -348,6 +354,58 @@ AND (performed_event_multiple_condition_None_level_level_0_level_1_0))) ''' # --- +# name: TestCohortQuery.test_performed_event_with_event_filters + ''' + + SELECT behavior_query.person_id AS id + FROM + (SELECT pdi.person_id AS person_id, + countIf(timestamp > now() - INTERVAL 1 week + AND timestamp < now() + AND event = '$pageview' + AND (has(['something'], replaceRegexpAll(JSONExtractRaw(properties, '$filter_prop'), '^"|"$', '')))) > 0 AS performed_event_condition_None_level_level_0_level_0_0 + FROM events e + INNER JOIN + (SELECT distinct_id, + argMax(person_id, version) as person_id + FROM person_distinct_id2 + WHERE team_id = 2 + GROUP BY distinct_id + HAVING argMax(is_deleted, version) = 0) AS pdi ON e.distinct_id = pdi.distinct_id + WHERE team_id = 2 + AND event IN ['$pageview'] + AND timestamp <= now() + AND timestamp >= now() - INTERVAL 1 week + GROUP BY person_id) behavior_query + WHERE 1 = 1 + AND (((performed_event_condition_None_level_level_0_level_0_0))) + ''' +# --- +# name: TestCohortQuery.test_performed_event_with_event_filters_and_explicit_date + ''' + + SELECT behavior_query.person_id AS id + FROM + (SELECT pdi.person_id AS person_id, + countIf(timestamp > '2024-04-02 13:01:01' + AND timestamp < now() + AND event = '$pageview' + AND (has(['something'], replaceRegexpAll(JSONExtractRaw(properties, '$filter_prop'), '^"|"$', '')))) > 0 AS performed_event_condition_None_level_level_0_level_0_0 + FROM events e + INNER JOIN + (SELECT distinct_id, + argMax(person_id, version) as person_id + FROM person_distinct_id2 + WHERE team_id = 2 + GROUP BY distinct_id + HAVING argMax(is_deleted, version) = 0) AS pdi ON e.distinct_id = pdi.distinct_id + WHERE team_id = 2 + AND event IN ['$pageview'] + GROUP BY person_id) behavior_query + WHERE 1 = 1 + AND (((performed_event_condition_None_level_level_0_level_0_0))) + ''' +# --- # name: TestCohortQuery.test_person ''' @@ -356,7 +414,8 @@ (SELECT pdi.person_id AS person_id, countIf(timestamp > now() - INTERVAL 1 week AND timestamp < now() - AND event = '$pageview') > 0 AS performed_event_condition_None_level_level_0_level_0_0 + AND event = '$pageview' + AND 1=1) > 0 AS performed_event_condition_None_level_level_0_level_0_0 FROM events e INNER JOIN (SELECT distinct_id, @@ -393,7 +452,8 @@ (SELECT pdi.person_id AS person_id, countIf(timestamp > now() - INTERVAL 1 week AND timestamp < now() - AND event = '$pageview') > 0 AS performed_event_condition_None_level_level_0_level_0_0 + AND event = '$pageview' + AND 1=1) > 0 AS performed_event_condition_None_level_level_0_level_0_0 FROM events e INNER JOIN (SELECT distinct_id, @@ -430,10 +490,12 @@ (SELECT pdi.person_id AS person_id, countIf(timestamp > now() - INTERVAL 1 day AND timestamp < now() - AND event = '$pageview') > 0 AS performed_event_condition_None_level_level_0_level_0_level_0_0, + AND event = '$pageview' + AND 1=1) > 0 AS performed_event_condition_None_level_level_0_level_0_level_0_0, countIf(timestamp > now() - INTERVAL 2 week AND timestamp < now() - AND event = '$pageview') > 0 AS performed_event_condition_None_level_level_0_level_0_level_1_0, + AND event = '$pageview' + AND 1=1) > 0 AS performed_event_condition_None_level_level_0_level_0_level_1_0, minIf(timestamp, ((replaceRegexpAll(JSONExtractRaw(properties, '$current_url'), '^"|"$', '') = 'https://posthog.com/feedback/123' AND event = '$autocapture'))) >= now() - INTERVAL 2 week AND minIf(timestamp, ((replaceRegexpAll(JSONExtractRaw(properties, '$current_url'), '^"|"$', '') = 'https://posthog.com/feedback/123' @@ -579,7 +641,8 @@ (SELECT pdi.person_id AS person_id, countIf(timestamp > now() - INTERVAL 1 week AND timestamp < now() - AND event = '$pageview') > 0 AS performed_event_condition_None_level_level_0_level_1_0 + AND event = '$pageview' + AND 1=1) > 0 AS performed_event_condition_None_level_level_0_level_1_0 FROM events e INNER JOIN (SELECT distinct_id, @@ -619,7 +682,8 @@ (SELECT pdi.person_id AS person_id, countIf(timestamp > now() - INTERVAL 1 week AND timestamp < now() - AND event = '$pageview') > 0 AS performed_event_condition_None_level_level_0_level_1_level_0_0 + AND event = '$pageview' + AND 1=1) > 0 AS performed_event_condition_None_level_level_0_level_1_level_0_0 FROM events e INNER JOIN (SELECT distinct_id, @@ -668,10 +732,12 @@ (SELECT pdi.person_id AS person_id, countIf(timestamp > now() - INTERVAL 7 day AND timestamp < now() - AND event = '$new_view') > 0 AS performed_event_condition_None_level_level_0_level_0_level_0_0, + AND event = '$new_view' + AND 1=1) > 0 AS performed_event_condition_None_level_level_0_level_0_level_0_0, countIf(timestamp > now() - INTERVAL 1 week AND timestamp < now() - AND event = '$pageview') > 0 AS performed_event_condition_None_level_level_0_level_1_0 + AND event = '$pageview' + AND 1=1) > 0 AS performed_event_condition_None_level_level_0_level_1_0 FROM events e INNER JOIN (SELECT distinct_id, diff --git a/ee/clickhouse/queries/test/test_cohort_query.py b/ee/clickhouse/queries/test/test_cohort_query.py index 6b9d4cf7ce116..1c385499f68e5 100644 --- a/ee/clickhouse/queries/test/test_cohort_query.py +++ b/ee/clickhouse/queries/test/test_cohort_query.py @@ -1,5 +1,7 @@ from datetime import datetime, timedelta +from freezegun import freeze_time + from ee.clickhouse.queries.enterprise_cohort_query import check_negation_clause from posthog.client import sync_execute from posthog.constants import PropertyOperatorType @@ -216,10 +218,76 @@ def test_performed_event(self): { "key": "$pageview", "event_type": "events", + "explicit_datetime": "-1w", + "value": "performed_event", + "type": "behavioral", + } + ], + } + } + ) + + q, params = CohortQuery(filter=filter, team=self.team).get_query() + res = sync_execute(q, {**params, **filter.hogql_context.values}) + + self.assertEqual([p1.uuid], [r[0] for r in res]) + + @snapshot_clickhouse_queries + @freeze_time("2024-04-05 13:01:01") + def test_performed_event_with_event_filters_and_explicit_date(self): + p1 = _create_person( + team_id=self.team.pk, + distinct_ids=["p1"], + properties={"name": "test", "email": "test@posthog.com"}, + ) + _create_event( + team=self.team, + event="$pageview", + properties={"$filter_prop": "something"}, + distinct_id="p1", + timestamp=datetime.now() - timedelta(days=2), + ) + + _create_person( + team_id=self.team.pk, + distinct_ids=["p2"], + properties={"name": "test", "email": "test@posthog.com"}, + ) + _create_event( + team=self.team, + event="$pageview", + properties={}, + distinct_id="p2", + timestamp=datetime.now() - timedelta(days=2), + ) + _create_event( + team=self.team, + event="$pageview", + properties={"$filter_prop": "something"}, + distinct_id="p2", + # rejected because explicit datetime is set to 3 days ago + timestamp=datetime.now() - timedelta(days=5), + ) + flush_persons_and_events() + + filter = Filter( + data={ + "properties": { + "type": "AND", + "values": [ + { + "key": "$pageview", + "event_type": "events", + "explicit_datetime": str( + datetime.now() - timedelta(days=3) + ), # overrides time_value and time_interval "time_value": 1, "time_interval": "week", "value": "performed_event", "type": "behavioral", + "event_filters": [ + {"key": "$filter_prop", "value": "something", "operator": "exact", "type": "event"} + ], } ], } @@ -292,6 +360,78 @@ def test_performed_event_multiple(self): self.assertEqual([p1.uuid], [r[0] for r in res]) + def test_performed_event_multiple_with_event_filters(self): + p1 = _create_person( + team_id=self.team.pk, + distinct_ids=["p1"], + properties={"name": "test", "email": "test@posthog.com"}, + ) + _create_event( + team=self.team, + event="$pageview", + properties={"$filter_prop": "something"}, + distinct_id="p1", + timestamp=datetime.now() - timedelta(days=2), + ) + + _create_event( + team=self.team, + event="$pageview", + properties={"$filter_prop": "something"}, + distinct_id="p1", + timestamp=datetime.now() - timedelta(days=4), + ) + + _create_person( + team_id=self.team.pk, + distinct_ids=["p2"], + properties={"name": "test", "email": "test@posthog.com"}, + ) + _create_event( + team=self.team, + event="$pageview", + properties={}, + distinct_id="p2", + timestamp=datetime.now() - timedelta(days=2), + ) + _create_event( + team=self.team, + event="$pageview", + properties={}, + distinct_id="p2", + timestamp=datetime.now() - timedelta(days=4), + ) + flush_persons_and_events() + + filter = Filter( + data={ + "properties": { + "type": "AND", + "values": [ + { + "key": "$pageview", + "event_type": "events", + "operator": "gte", + "operator_value": 1, + "time_value": 1, + "time_interval": "week", + "value": "performed_event_multiple", + "type": "behavioral", + "event_filters": [ + {"key": "$filter_prop", "value": "something", "operator": "exact", "type": "event"}, + {"key": "$filter_prop", "value": "some", "operator": "icontains", "type": "event"}, + ], + } + ], + } + } + ) + + q, params = CohortQuery(filter=filter, team=self.team).get_query() + res = sync_execute(q, {**params, **filter.hogql_context.values}) + + self.assertEqual([p1.uuid], [r[0] for r in res]) + def test_performed_event_lte_1_times(self): _create_person( team_id=self.team.pk, diff --git a/ee/clickhouse/views/experiments.py b/ee/clickhouse/views/experiments.py index ff27ede958c66..c30ba6ee58092 100644 --- a/ee/clickhouse/views/experiments.py +++ b/ee/clickhouse/views/experiments.py @@ -28,7 +28,7 @@ from posthog.models.filters.filter import Filter from posthog.utils import generate_cache_key, get_safe_cache -EXPERIMENT_RESULTS_CACHE_DEFAULT_TTL = 60 * 30 # 30 minutes +EXPERIMENT_RESULTS_CACHE_DEFAULT_TTL = 60 * 60 # 1 hour def _calculate_experiment_results(experiment: Experiment, refresh: bool = False): diff --git a/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr b/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr index 3c6b8ef78c385..b365da44bb9d6 100644 --- a/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr +++ b/ee/clickhouse/views/test/__snapshots__/test_clickhouse_experiment_secondary_results.ambr @@ -1,7 +1,7 @@ # serializer version: 1 # name: ClickhouseTestExperimentSecondaryResults.test_basic_secondary_metric_results ''' - /* user_id:107 celery:posthog.tasks.tasks.sync_insight_caching_state */ + /* user_id:109 celery:posthog.tasks.tasks.sync_insight_caching_state */ SELECT team_id, date_diff('second', max(timestamp), now()) AS age FROM events diff --git a/frontend/__snapshots__/filters-cohort-filters-row-builder--cohort-criteria-row-builder--dark.png b/frontend/__snapshots__/filters-cohort-filters-row-builder--cohort-criteria-row-builder--dark.png index 2cf719cc12960..88b8b5fe7b092 100644 Binary files a/frontend/__snapshots__/filters-cohort-filters-row-builder--cohort-criteria-row-builder--dark.png and b/frontend/__snapshots__/filters-cohort-filters-row-builder--cohort-criteria-row-builder--dark.png differ diff --git a/frontend/__snapshots__/filters-cohort-filters-row-builder--cohort-criteria-row-builder--light.png b/frontend/__snapshots__/filters-cohort-filters-row-builder--cohort-criteria-row-builder--light.png index f12d2fdcc90a2..35a49f1af3f6e 100644 Binary files a/frontend/__snapshots__/filters-cohort-filters-row-builder--cohort-criteria-row-builder--light.png and b/frontend/__snapshots__/filters-cohort-filters-row-builder--cohort-criteria-row-builder--light.png differ diff --git a/frontend/src/lib/components/DateFilter/DateFilter.tsx b/frontend/src/lib/components/DateFilter/DateFilter.tsx index 8bb267b8b5486..808462941e01a 100644 --- a/frontend/src/lib/components/DateFilter/DateFilter.tsx +++ b/frontend/src/lib/components/DateFilter/DateFilter.tsx @@ -22,6 +22,7 @@ import { DateMappingOption, PropertyOperator } from '~/types' import { PropertyFilterDatePicker } from '../PropertyFilters/components/PropertyFilterDatePicker' import { dateFilterLogic } from './dateFilterLogic' import { RollingDateRangeFilter } from './RollingDateRangeFilter' +import { DateOption } from './rollingDateRangeFilterLogic' export interface DateFilterProps { showCustom?: boolean @@ -41,6 +42,7 @@ interface RawDateFilterProps extends DateFilterProps { dateFrom?: string | null | dayjs.Dayjs dateTo?: string | null | dayjs.Dayjs max?: number | null + allowedRollingDateOptions?: DateOption[] } export function DateFilter({ @@ -58,6 +60,7 @@ export function DateFilter({ dropdownPlacement = 'bottom-start', max, isFixedDateMode = false, + allowedRollingDateOptions, }: RawDateFilterProps): JSX.Element { const key = useRef(uuid()).current const logicProps: DateFilterLogicProps = { @@ -183,7 +186,11 @@ export function DateFilter({ ref: rollingDateRangeRef, }} max={max} - allowedDateOptions={isFixedDateMode ? ['hours', 'days', 'weeks', 'months', 'years'] : undefined} + allowedDateOptions={ + isFixedDateMode && !allowedRollingDateOptions + ? ['hours', 'days', 'weeks', 'months', 'years'] + : allowedRollingDateOptions + } fullWidth /> )} diff --git a/frontend/src/lib/components/HedgehogBuddy/HedgehogBuddy.tsx b/frontend/src/lib/components/HedgehogBuddy/HedgehogBuddy.tsx index 4dd79cceaed02..11c23499ac464 100644 --- a/frontend/src/lib/components/HedgehogBuddy/HedgehogBuddy.tsx +++ b/frontend/src/lib/components/HedgehogBuddy/HedgehogBuddy.tsx @@ -409,7 +409,6 @@ export function HedgehogBuddy({ return actor.setupKeyboardListeners() }, []) - // eslint-disable-next-line @typescript-eslint/no-unused-vars const [_, setTimerLoop] = useState(0) const [popoverVisible, setPopoverVisible] = useState(false) diff --git a/frontend/src/lib/components/PropertyFilters/utils.ts b/frontend/src/lib/components/PropertyFilters/utils.ts index 255c8428ba8ba..ad135c0525b0e 100644 --- a/frontend/src/lib/components/PropertyFilters/utils.ts +++ b/frontend/src/lib/components/PropertyFilters/utils.ts @@ -341,3 +341,11 @@ export function taxonomicFilterTypeToPropertyFilterType( | PropertyFilterType | undefined } + +export function isEmptyProperty(property: AnyPropertyFilter): boolean { + return ( + property.value === null || + property.value === undefined || + (Array.isArray(property.value) && property.value.length === 0) + ) +} diff --git a/frontend/src/models/cohortsModel.ts b/frontend/src/models/cohortsModel.ts index 2d2c106e4d728..295d3cd4b1fad 100644 --- a/frontend/src/models/cohortsModel.ts +++ b/frontend/src/models/cohortsModel.ts @@ -5,6 +5,7 @@ import api from 'lib/api' import { exportsLogic } from 'lib/components/ExportButton/exportsLogic' import { deleteWithUndo } from 'lib/utils/deleteWithUndo' import { permanentlyMount } from 'lib/utils/kea-logic-builders' +import { COHORT_EVENT_TYPES_WITH_EXPLICIT_DATETIME } from 'scenes/cohorts/CohortFilters/constants' import { BehavioralFilterKey } from 'scenes/cohorts/CohortFilters/types' import { personsLogic } from 'scenes/persons/personsLogic' import { isAuthenticatedTeam, teamLogic } from 'scenes/teamLogic' @@ -36,18 +37,7 @@ export function processCohort(cohort: CohortType): CohortType { ? { ...group, values: (group.values as AnyCohortCriteriaType[]).map((c) => - c.type && - [BehavioralFilterKey.Cohort, BehavioralFilterKey.Person].includes(c.type) && - !('value_property' in c) - ? { - ...c, - value_property: c.value, - value: - c.type === BehavioralFilterKey.Cohort - ? BehavioralCohortType.InCohort - : BehavioralEventType.HaveProperty, - } - : c + processCohortCriteria(c) ), } : group @@ -58,6 +48,45 @@ export function processCohort(cohort: CohortType): CohortType { } } +function convertTimeValueToRelativeTime(criteria: AnyCohortCriteriaType): string | undefined { + const timeValue = criteria?.time_value + const timeInterval = criteria?.time_interval + + if (timeValue && timeInterval) { + return `-${timeValue}${timeInterval[0]}` + } +} + +function processCohortCriteria(criteria: AnyCohortCriteriaType): AnyCohortCriteriaType { + if (!criteria.type) { + return criteria + } + + const processedCriteria = { ...criteria } + + if ( + [BehavioralFilterKey.Cohort, BehavioralFilterKey.Person].includes(criteria.type) && + !('value_property' in criteria) + ) { + processedCriteria.value_property = criteria.value + processedCriteria.value = + criteria.type === BehavioralFilterKey.Cohort + ? BehavioralCohortType.InCohort + : BehavioralEventType.HaveProperty + } + + if ( + [BehavioralFilterKey.Behavioral].includes(criteria.type) && + !('explicit_datetime' in criteria) && + criteria.value && + COHORT_EVENT_TYPES_WITH_EXPLICIT_DATETIME.includes(criteria.value) + ) { + processedCriteria.explicit_datetime = convertTimeValueToRelativeTime(criteria) + } + + return processedCriteria +} + export const cohortsModel = kea([ path(['models', 'cohortsModel']), connect({ diff --git a/frontend/src/scenes/cohorts/CohortFilters/CohortCriteriaRowBuilder.tsx b/frontend/src/scenes/cohorts/CohortFilters/CohortCriteriaRowBuilder.tsx index 14e58c0ddfd94..becc9a83ab995 100644 --- a/frontend/src/scenes/cohorts/CohortFilters/CohortCriteriaRowBuilder.tsx +++ b/frontend/src/scenes/cohorts/CohortFilters/CohortCriteriaRowBuilder.tsx @@ -47,6 +47,8 @@ export function CohortCriteriaRowBuilder({ ...(_field.type === FilterType.Text ? { value: _field.defaultValue } : {}), ...(_field.groupTypeFieldKey ? { groupTypeFieldKey: _field.groupTypeFieldKey } : {}), onChange: (newCriteria) => setCriteria(newCriteria, groupIndex, index), + groupIndex, + index, } as CohortFieldProps)} ) @@ -120,7 +122,7 @@ export function CohortCriteriaRowBuilder({ ↳
-
+
{rowShape.fields.map((field, i) => { return ( !field.hide && diff --git a/frontend/src/scenes/cohorts/CohortFilters/CohortField.tsx b/frontend/src/scenes/cohorts/CohortFilters/CohortField.tsx index eb30b33f72298..e0301d158606e 100644 --- a/frontend/src/scenes/cohorts/CohortFilters/CohortField.tsx +++ b/frontend/src/scenes/cohorts/CohortFilters/CohortField.tsx @@ -2,24 +2,30 @@ import './CohortField.scss' import clsx from 'clsx' import { useActions, useValues } from 'kea' +import { DateFilter } from 'lib/components/DateFilter/DateFilter' import { PropertyValue } from 'lib/components/PropertyFilters/components/PropertyValue' +import { PropertyFilters } from 'lib/components/PropertyFilters/PropertyFilters' import { TaxonomicFilterGroupType, TaxonomicFilterValue } from 'lib/components/TaxonomicFilter/types' import { TaxonomicPopover } from 'lib/components/TaxonomicPopover/TaxonomicPopover' +import { dayjs } from 'lib/dayjs' import { LemonButton, LemonButtonWithDropdown } from 'lib/lemon-ui/LemonButton' import { LemonDivider } from 'lib/lemon-ui/LemonDivider' import { LemonInput } from 'lib/lemon-ui/LemonInput/LemonInput' -import { useMemo } from 'react' +import { formatDate } from 'lib/utils' +import { useEffect, useMemo, useRef } from 'react' import { cohortFieldLogic } from 'scenes/cohorts/CohortFilters/cohortFieldLogic' import { + CohortEventFiltersFieldProps, CohortFieldBaseProps, CohortNumberFieldProps, CohortPersonPropertiesValuesFieldProps, + CohortRelativeAndExactTimeFieldProps, CohortSelectorFieldProps, CohortTaxonomicFieldProps, CohortTextFieldProps, } from 'scenes/cohorts/CohortFilters/types' -import { PropertyFilterType, PropertyFilterValue, PropertyOperator } from '~/types' +import { AnyPropertyFilter, PropertyFilterType, PropertyFilterValue, PropertyOperator } from '~/types' let uniqueMemoizedIndex = 0 @@ -167,6 +173,118 @@ export function CohortPersonPropertiesValuesField({ ) } +export function CohortEventFiltersField({ + fieldKey, + criteria, + cohortFilterLogicKey, + onChange: _onChange, + groupIndex, + index, +}: CohortEventFiltersFieldProps): JSX.Element { + const { logic } = useCohortFieldLogic({ + fieldKey, + criteria, + cohortFilterLogicKey, + onChange: _onChange, + }) + const { value } = useValues(logic) + const { onChange } = useActions(logic) + const componentRef = useRef(null) + + const valueExists = ((value as AnyPropertyFilter[]) || []).length > 0 + + useEffect(() => { + // :TRICKY: We check paremt has CohortCriteriaRow__Criteria__Field class and add basis-full class if value exists + // We need to do this because of how this list is generated, and we need to add a line-break programatically + // when the PropertyFilters take up too much space. + // Since the list of children is declared in the parent component, we can't add a class to the parent directly, without + // adding a lot of annoying complexity to the parent component. + // This is a hacky solution, but it works 🙈. + + // find parent with className CohortCriteriaRow__Criteria__Field and add basis-full class if value exists + const parent = componentRef.current?.closest('.CohortCriteriaRow__Criteria__Field') + if (parent) { + if (valueExists) { + parent.classList.add('basis-full') + } else { + parent.classList.remove('basis-full') + } + } + }, [componentRef, value]) + + return ( +
+ { + onChange({ [fieldKey]: newValue }) + }} + pageKey={`${fieldKey}-${groupIndex}-${index}`} + eventNames={criteria?.key ? [criteria?.key] : []} + disablePopover + hasRowOperator={valueExists ? true : false} + sendAllKeyUpdates + /> +
+ ) +} + +export function CohortRelativeAndExactTimeField({ + fieldKey, + criteria, + cohortFilterLogicKey, + onChange: _onChange, +}: CohortRelativeAndExactTimeFieldProps): JSX.Element { + const { logic } = useCohortFieldLogic({ + fieldKey, + criteria, + cohortFilterLogicKey, + onChange: _onChange, + }) + // This replaces the old TimeUnit and TimeInterval filters + // and combines them with a relative+exact time option. + // This is more inline with rest of analytics filters and make things much nicer here. + const { value } = useValues(logic) + const { onChange } = useActions(logic) + + return ( + { + onChange({ [fieldKey]: fromDate }) + }} + max={1000} + isFixedDateMode + allowedRollingDateOptions={['days', 'weeks', 'months', 'years']} + showCustom + dateOptions={[ + { + key: 'Last 7 days', + values: ['-7d'], + getFormattedDate: (date: dayjs.Dayjs): string => formatDate(date.subtract(7, 'd')), + defaultInterval: 'day', + }, + { + key: 'Last 30 days', + values: ['-30d'], + getFormattedDate: (date: dayjs.Dayjs): string => formatDate(date.subtract(14, 'd')), + defaultInterval: 'day', + }, + ]} + size="medium" + makeLabel={(_, startOfRange) => ( + Matches all values after {startOfRange} if evaluated today. + )} + /> + ) +} + export function CohortTextField({ value }: CohortTextFieldProps): JSX.Element { return {value} } diff --git a/frontend/src/scenes/cohorts/CohortFilters/CohortTaxonomicField.stories.tsx b/frontend/src/scenes/cohorts/CohortFilters/CohortTaxonomicField.stories.tsx index 69e9421c43aa1..8efe734e1b8b9 100644 --- a/frontend/src/scenes/cohorts/CohortFilters/CohortTaxonomicField.stories.tsx +++ b/frontend/src/scenes/cohorts/CohortFilters/CohortTaxonomicField.stories.tsx @@ -27,7 +27,7 @@ const Template: StoryFn = (props: CohortTaxonomicFi props.taxonomicGroupTypes[0] === TaxonomicFilterGroupType.Events && props.taxonomicGroupTypes[1] === TaxonomicFilterGroupType.Actions ? FilterType.EventsAndActions - : FilterType.EventProperties + : FilterType.PersonProperties return renderField[type]({ ...props, criteria: { diff --git a/frontend/src/scenes/cohorts/CohortFilters/constants.tsx b/frontend/src/scenes/cohorts/CohortFilters/constants.tsx index dc17d1b7883ea..e1f5c7b53bfbd 100644 --- a/frontend/src/scenes/cohorts/CohortFilters/constants.tsx +++ b/frontend/src/scenes/cohorts/CohortFilters/constants.tsx @@ -2,8 +2,10 @@ import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types' import { CohortTypeEnum, PROPERTY_MATCH_TYPE } from 'lib/constants' import { LemonSelectOptions } from 'lib/lemon-ui/LemonSelect' import { + CohortEventFiltersField, CohortNumberField, CohortPersonPropertiesValuesField, + CohortRelativeAndExactTimeField, CohortSelectorField, CohortTaxonomicField, CohortTextField, @@ -12,9 +14,11 @@ import { BehavioralFilterKey, BehavioralFilterType, CohortClientErrors, + CohortEventFiltersFieldProps, CohortFieldProps, CohortNumberFieldProps, CohortPersonPropertiesValuesFieldProps, + CohortRelativeAndExactTimeFieldProps, CohortTaxonomicFieldProps, CohortTextFieldProps, FieldOptionsType, @@ -342,18 +346,17 @@ export const ROWS: Record = { hide: true, }, { - type: FilterType.Text, - defaultValue: 'in the last', + fieldKey: 'event_filters', + type: FilterType.EventFilters, }, { - fieldKey: 'time_value', - type: FilterType.Number, - defaultValue: '30', + type: FilterType.Text, + defaultValue: 'after', }, { - fieldKey: 'time_interval', - type: FilterType.TimeUnit, - defaultValue: TimeUnitType.Day, + fieldKey: 'explicit_datetime', + type: FilterType.RelativeAndExactTime, + defaultValue: '-30d', }, ], }, @@ -374,18 +377,17 @@ export const ROWS: Record = { hide: true, }, { - type: FilterType.Text, - defaultValue: 'in the last', + fieldKey: 'event_filters', + type: FilterType.EventFilters, }, { - fieldKey: 'time_value', - type: FilterType.Number, - defaultValue: '30', + type: FilterType.Text, + defaultValue: 'after', }, { - fieldKey: 'time_interval', - type: FilterType.TimeUnit, - defaultValue: TimeUnitType.Day, + fieldKey: 'explicit_datetime', + type: FilterType.RelativeAndExactTime, + defaultValue: '-30d', }, ], }, @@ -405,6 +407,10 @@ export const ROWS: Record = { defaultValue: TaxonomicFilterGroupType.Events, hide: true, }, + { + fieldKey: 'event_filters', + type: FilterType.EventFilters, + }, { fieldKey: 'operator', type: FilterType.EventsAndActionsMathOperator, @@ -417,17 +423,12 @@ export const ROWS: Record = { }, { type: FilterType.Text, - defaultValue: 'times in the last', + defaultValue: 'times after', }, { - fieldKey: 'time_value', - type: FilterType.Number, - defaultValue: '30', - }, - { - fieldKey: 'time_interval', - type: FilterType.TimeUnit, - defaultValue: TimeUnitType.Day, + fieldKey: 'explicit_datetime', + type: FilterType.RelativeAndExactTime, + defaultValue: '-30d', }, ], }, @@ -568,7 +569,7 @@ export const ROWS: Record = { fields: [ { fieldKey: 'key', - type: FilterType.EventProperties, + type: FilterType.PersonProperties, }, { fieldKey: 'operator', @@ -588,7 +589,7 @@ export const ROWS: Record = { fields: [ { fieldKey: 'key', - type: FilterType.EventProperties, + type: FilterType.PersonProperties, }, { fieldKey: 'operator', @@ -823,6 +824,10 @@ export const ROWS: Record = { }, } +export const COHORT_EVENT_TYPES_WITH_EXPLICIT_DATETIME = Object.entries(ROWS) + .filter(([_, row]) => row.fields.some((field) => field.type === FilterType.RelativeAndExactTime)) + .map(([eventType, _]) => eventType) + // Building blocks of a row export const renderField: Record JSX.Element> = { [FilterType.Behavioral]: function _renderField(p) { @@ -877,7 +882,7 @@ export const renderField: Record JSX.El /> ) }, - [FilterType.EventProperties]: function _renderField(p) { + [FilterType.PersonProperties]: function _renderField(p) { return ( JSX.El /> ) }, + [FilterType.EventFilters]: function _renderField(p) { + return + }, [FilterType.PersonPropertyValues]: function _renderField(p) { return p.criteria['operator'] && [PropertyOperator.IsSet, PropertyOperator.IsNotSet].includes(p.criteria['operator']) ? ( @@ -913,6 +921,9 @@ export const renderField: Record JSX.El /> ) }, + [FilterType.RelativeAndExactTime]: function _renderField(p) { + return + }, [FilterType.EventType]: function _renderField() { return <> }, @@ -926,7 +937,8 @@ export const CRITERIA_VALIDATIONS: Record< (d: string | number | null | undefined) => CohortClientErrors | undefined > = { [FilterType.EventsAndActions]: () => CohortClientErrors.EmptyEventsAndActions, - [FilterType.EventProperties]: () => CohortClientErrors.EmptyEventProperties, + [FilterType.EventFilters]: () => CohortClientErrors.EmptyEventFilters, + [FilterType.PersonProperties]: () => CohortClientErrors.EmptyPersonProperties, [FilterType.PersonPropertyValues]: () => CohortClientErrors.EmptyPersonPropertyValues, [FilterType.EventType]: () => CohortClientErrors.EmptyEventType, [FilterType.Number]: (d) => (Number(d) > 1 ? undefined : CohortClientErrors.EmptyNumber), @@ -934,6 +946,7 @@ export const CRITERIA_VALIDATIONS: Record< [FilterType.TimeUnit]: () => CohortClientErrors.EmptyTimeUnit, [FilterType.MathOperator]: () => CohortClientErrors.EmptyMathOperator, [FilterType.EventsAndActionsMathOperator]: () => CohortClientErrors.EmptyMathOperator, + [FilterType.RelativeAndExactTime]: () => CohortClientErrors.EmptyRelativeAndExactTime, [FilterType.CohortId]: () => CohortClientErrors.EmptyCohortId, [FilterType.CohortValues]: () => CohortClientErrors.EmptyCohortValues, [FilterType.Value]: () => CohortClientErrors.EmptyValue, @@ -952,8 +965,7 @@ export const NEW_CRITERIA = { type: BehavioralFilterKey.Behavioral, value: BehavioralEventType.PerformEvent, event_type: TaxonomicFilterGroupType.Events, - time_value: '30', - time_interval: TimeUnitType.Day, + explicit_datetime: '-30d', } export const NEW_CRITERIA_GROUP: CohortCriteriaGroupFilter = { diff --git a/frontend/src/scenes/cohorts/CohortFilters/types.ts b/frontend/src/scenes/cohorts/CohortFilters/types.ts index f66717385bd6b..2b9f753b15558 100644 --- a/frontend/src/scenes/cohorts/CohortFilters/types.ts +++ b/frontend/src/scenes/cohorts/CohortFilters/types.ts @@ -21,7 +21,9 @@ export enum FilterType { Value = 'value', Text = 'text', EventsAndActions = 'eventsAndActions', - EventProperties = 'eventProperties', + RelativeAndExactTime = 'relativeAndExactTime', + EventFilters = 'eventFilters', + PersonProperties = 'personProperties', PersonPropertyValues = 'personPropertyValues', EventType = 'eventType', Number = 'number', @@ -86,6 +88,8 @@ export interface Row { export interface CohortFieldBaseProps extends Omit { cohortFilterLogicKey?: string + groupIndex?: number + index?: number } export interface CohortSelectorFieldProps extends CohortFieldBaseProps { @@ -105,6 +109,14 @@ export interface CohortPersonPropertiesValuesFieldProps extends Omit { + fieldOptionGroupTypes: never +} + +export interface CohortRelativeAndExactTimeFieldProps extends Omit { + fieldOptionGroupTypes: never +} + export interface CohortTextFieldProps extends CohortFieldBaseProps { value: string } @@ -119,6 +131,8 @@ export type CohortFieldProps = | CohortTaxonomicFieldProps | CohortTextFieldProps | CohortPersonPropertiesValuesFieldProps + | CohortEventFiltersFieldProps + | CohortRelativeAndExactTimeFieldProps export enum CohortClientErrors { NegationCriteriaMissingOther = 'Negation criteria can only be used when matching all criteria (AND), and must be accompanied by at least one positive matching criteria.', @@ -126,12 +140,14 @@ export enum CohortClientErrors { PeriodTimeMismatch = 'The lower bound period value must not be greater than the upper bound value.', SequentialTimeMismatch = 'The lower bound period sequential time value must not be greater than the upper bound time value.', EmptyEventsAndActions = 'Event or action cannot be empty.', - EmptyEventProperties = 'Event property cannot be empty.', + EmptyEventFilters = 'Event filters cannot be empty.', + EmptyPersonProperties = 'Person property name cannot be empty.', EmptyPersonPropertyValues = 'Person property value cannot be empty', EmptyEventType = 'Event type cannot be empty.', EmptyNumber = 'Period values must be at least 1 day and cannot be empty.', EmptyNumberTicker = 'Number cannot be empty.', EmptyTimeUnit = 'Time interval cannot be empty.', + EmptyRelativeAndExactTime = 'Time value cannot be empty.', EmptyMathOperator = 'Math operator cannot be empty.', EmptyCohortId = 'Cohort id cannot be empty.', EmptyCohortValues = 'Cohort value cannot be empty.', diff --git a/frontend/src/scenes/cohorts/cohortEditLogic.test.ts b/frontend/src/scenes/cohorts/cohortEditLogic.test.ts index 94f6910e1b99b..ec215fbb33bd5 100644 --- a/frontend/src/scenes/cohorts/cohortEditLogic.test.ts +++ b/frontend/src/scenes/cohorts/cohortEditLogic.test.ts @@ -17,6 +17,7 @@ import { BehavioralLifecycleType, CohortCriteriaGroupFilter, FilterLogicalOperator, + PropertyFilterType, PropertyOperator, TimeUnitType, } from '~/types' @@ -83,9 +84,6 @@ describe('cohortEditLogic', () => { }) .toFinishAllListeners() .toDispatchActions(['setCohort', 'deleteCohort', router.actionCreators.push(urls.cohorts())]) - .toMatchValues({ - cohort: mockCohort, - }) expect(api.update).toBeCalledTimes(1) }) @@ -431,6 +429,86 @@ describe('cohortEditLogic', () => { expect(api.update).toBeCalledTimes(0) }) + it('do not save on partial event filters', async () => { + await initCohortLogic({ id: 1 }) + await expectLogic(logic, async () => { + logic.actions.setCohort({ + ...mockCohort, + filters: { + properties: { + id: '39777', + type: FilterLogicalOperator.Or, + values: [ + { + id: '70427', + type: FilterLogicalOperator.And, + values: [ + { + type: BehavioralFilterKey.Behavioral, + value: BehavioralEventType.PerformEvent, + event_type: TaxonomicFilterGroupType.Events, + explicit_datetime: '-14d', + key: 'dashboard date range changed', + event_filters: [ + { + key: '$browser', + value: null, + type: PropertyFilterType.Event, + operator: PropertyOperator.Exact, + }, + ], + }, + { + type: BehavioralFilterKey.Behavioral, + value: BehavioralEventType.PerformEvent, + event_type: TaxonomicFilterGroupType.Events, + time_value: '1', + time_interval: TimeUnitType.Day, + key: '$rageclick', + negation: true, + event_filters: [ + { + key: '$browser', + value: null, + type: PropertyFilterType.Event, + operator: PropertyOperator.Exact, + }, + ], + }, + ], + }, + ], + }, + }, + }) + logic.actions.submitCohort() + }) + .toDispatchActions(['setCohort', 'submitCohort', 'submitCohortFailure']) + .toMatchValues({ + cohortErrors: partial({ + filters: { + properties: { + values: [ + { + values: [ + { + event_filters: 'Event filters cannot be empty.', + id: 'Event filters cannot be empty.', + }, + { + event_filters: 'Event filters cannot be empty.', + id: 'Event filters cannot be empty.', + }, + ], + }, + ], + }, + }, + }), + }) + expect(api.update).toBeCalledTimes(0) + }) + describe('empty input errors', () => { Object.entries(ROWS).forEach(([key, row]) => { it(`${key} row missing all required fields`, async () => { @@ -475,7 +553,10 @@ describe('cohortEditLogic', () => { partial( Object.fromEntries( row.fields - .filter(({ fieldKey }) => !!fieldKey) + .filter( + ({ fieldKey }) => + !!fieldKey && fieldKey !== 'event_filters' + ) // event_filters are optional .map(({ fieldKey, type }) => [ fieldKey, CRITERIA_VALIDATIONS[type](undefined), @@ -530,6 +611,16 @@ describe('cohortEditLogic', () => { }) it('duplicate group', async () => { + const expectedGroupValue = partial({ + ...mockCohort.filters.properties.values[0], + values: [ + { + ...(mockCohort.filters.properties.values[0] as CohortCriteriaGroupFilter).values[0], + explicit_datetime: '-30d', + }, + ], + }) // Backwards compatible processing adds explicit_datetime + await expectLogic(logic, () => { logic.actions.duplicateFilter(0) }) @@ -538,10 +629,7 @@ describe('cohortEditLogic', () => { cohort: partial({ filters: { properties: partial({ - values: [ - partial(mockCohort.filters.properties.values[0]), - partial(mockCohort.filters.properties.values[0]), - ], + values: [expectedGroupValue, expectedGroupValue], }), }, }), @@ -574,7 +662,17 @@ describe('cohortEditLogic', () => { filters: { properties: partial({ values: [ - partial(mockCohort.filters.properties.values[0]), + partial({ + ...mockCohort.filters.properties.values[0], + values: [ + { + ...( + mockCohort.filters.properties.values[0] as CohortCriteriaGroupFilter + ).values[0], + explicit_datetime: '-30d', + }, + ], + }), // Backwards compatible processing adds explicit_datetime partial({ type: FilterLogicalOperator.Or, values: [NEW_CRITERIA], diff --git a/frontend/src/scenes/cohorts/cohortUtils.tsx b/frontend/src/scenes/cohorts/cohortUtils.tsx index d8d701daeb13a..5d1efee633852 100644 --- a/frontend/src/scenes/cohorts/cohortUtils.tsx +++ b/frontend/src/scenes/cohorts/cohortUtils.tsx @@ -1,5 +1,6 @@ import equal from 'fast-deep-equal' import { DeepPartialMap, ValidationErrorType } from 'kea-forms' +import { isEmptyProperty } from 'lib/components/PropertyFilters/utils' import { ENTITY_MATCH_TYPE, PROPERTY_MATCH_TYPE } from 'lib/constants' import { areObjectValuesEmpty, calculateDays, isNumeric } from 'lib/utils' import { BEHAVIORAL_TYPE_TO_LABEL, CRITERIA_VALIDATIONS, ROWS } from 'scenes/cohorts/CohortFilters/constants' @@ -15,6 +16,7 @@ import { ActionType, AnyCohortCriteriaType, AnyCohortGroupType, + AnyPropertyFilter, BehavioralCohortType, BehavioralEventType, BehavioralLifecycleType, @@ -22,6 +24,7 @@ import { CohortCriteriaType, CohortType, FilterLogicalOperator, + PropertyFilterType, PropertyOperator, TimeUnitType, } from '~/types' @@ -272,6 +275,15 @@ export function validateGroup( requiredFields = requiredFields.filter((f) => f.fieldKey !== 'value_property') } + // Handle EventFilters separately, since these are optional + requiredFields = requiredFields.filter((f) => f.fieldKey !== 'event_filters') + const eventFilterError = + c?.event_filters && + c.event_filters.length > 0 && + c.event_filters.some((prop) => prop?.type !== PropertyFilterType.HogQL && isEmptyProperty(prop)) + ? CohortClientErrors.EmptyEventFilters + : undefined + const criteriaErrors = Object.fromEntries( requiredFields.map(({ fieldKey, type }) => [ fieldKey, @@ -284,13 +296,15 @@ export function validateGroup( : CRITERIA_VALIDATIONS?.[type](c[fieldKey]), ]) ) - const consolidatedErrors = Object.values(criteriaErrors) + + const allErrors = { ...criteriaErrors, event_filters: eventFilterError } + const consolidatedErrors = Object.values(allErrors) .filter((e) => !!e) .join(' ') return { - ...(areObjectValuesEmpty(criteriaErrors) ? {} : { id: consolidatedErrors }), - ...criteriaErrors, + ...(areObjectValuesEmpty(allErrors) ? {} : { id: consolidatedErrors }), + ...allErrors, } }), } @@ -361,7 +375,7 @@ export function determineFilterType( export function resolveCohortFieldValue( criteria: AnyCohortCriteriaType, fieldKey: string -): string | number | boolean | null | undefined { +): string | number | boolean | null | undefined | AnyPropertyFilter[] { // Resolve correct behavioral filter type if (fieldKey === 'value') { return criteriaToBehavioralFilterType(criteria) @@ -452,6 +466,7 @@ export function criteriaToHumanSentence( } data.fields.forEach(({ type, fieldKey, defaultValue, hide }) => { + // TODO: This needs to be much nicer for all cohort criteria options if (!hide) { if (type === FilterType.Text) { words.push(defaultValue) @@ -461,6 +476,8 @@ export function criteriaToHumanSentence( words.push(
{cohortsById?.[value]?.name ?? `Cohort ${value}`}
) } else if (type === FilterType.EventsAndActions && typeof value === 'number') { words.push(
{actionsById?.[value]?.name ?? `Action ${value}`}
) + } else if (type === FilterType.EventFilters && (criteria.event_filters?.length || 0) > 0) { + words.push(
with filters
) } else { words.push(
{value}
) } diff --git a/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsLogic.ts b/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsLogic.ts index dab0d6f408993..7aceb608d5b7d 100644 --- a/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsLogic.ts +++ b/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditionsLogic.ts @@ -1,6 +1,7 @@ import { actions, afterMount, connect, kea, key, listeners, path, props, propsChanged, reducers, selectors } from 'kea' import { subscriptions } from 'kea-subscriptions' import api from 'lib/api' +import { isEmptyProperty } from 'lib/components/PropertyFilters/utils' import { TaxonomicFilterGroupType, TaxonomicFilterProps } from 'lib/components/TaxonomicFilter/types' import { objectsEqual } from 'lib/utils' @@ -310,11 +311,3 @@ export const featureFlagReleaseConditionsLogic = kea([ }) .filter(Boolean) as AnyPropertyFilter[] } else { - // eslint-disable-next-line @typescript-eslint/no-unused-vars const [_, propertyName, propertyValue] = correlation.event.event.split('::') properties = [ diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 6ed01082581e0..80c14249936a2 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -1076,6 +1076,7 @@ export interface CohortCriteriaType { operator_value?: PropertyFilterValue time_value?: number | string | null time_interval?: TimeUnitType | null + explicit_datetime?: string | null total_periods?: number | null min_periods?: number | null seq_event_type?: TaxonomicFilterGroupType | null @@ -1084,6 +1085,7 @@ export interface CohortCriteriaType { seq_time_interval?: TimeUnitType | null negation?: boolean value_property?: string | null // Transformed into 'value' for api calls + event_filters?: AnyPropertyFilter[] | null } export type EmptyCohortGroupType = Partial diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 85a91e7999f3a..687a6926e2286 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1,4 +1,4 @@ -lockfileVersion: '6.0' +lockfileVersion: '6.1' settings: autoInstallPeers: true @@ -350,7 +350,7 @@ dependencies: optionalDependencies: fsevents: specifier: ^2.3.2 - version: 2.3.3 + version: 2.3.2 devDependencies: '@babel/core': @@ -12825,7 +12825,6 @@ packages: engines: {node: ^8.16.0 || ^10.6.0 || >=11.0.0} os: [darwin] requiresBuild: true - dev: true optional: true /fsevents@2.3.3: diff --git a/posthog/api/test/__snapshots__/test_cohort.ambr b/posthog/api/test/__snapshots__/test_cohort.ambr index adbe09a8b43b3..17cf49bcfa7ee 100644 --- a/posthog/api/test/__snapshots__/test_cohort.ambr +++ b/posthog/api/test/__snapshots__/test_cohort.ambr @@ -24,7 +24,8 @@ (SELECT pdi.person_id AS person_id, countIf(timestamp > now() - INTERVAL 1 day AND timestamp < now() - AND event = '$pageview') > 0 AS performed_event_condition_1_level_level_0_level_1_level_0_0 + AND event = '$pageview' + AND 1=1) > 0 AS performed_event_condition_1_level_level_0_level_1_level_0_0 FROM events e INNER JOIN (SELECT distinct_id, diff --git a/posthog/api/test/__snapshots__/test_feature_flag.ambr b/posthog/api/test/__snapshots__/test_feature_flag.ambr index 06c13c4df0866..0a554e99faae2 100644 --- a/posthog/api/test/__snapshots__/test_feature_flag.ambr +++ b/posthog/api/test/__snapshots__/test_feature_flag.ambr @@ -1739,7 +1739,7 @@ # --- # name: TestFeatureFlag.test_creating_static_cohort.14 ''' - /* user_id:198 celery:posthog.tasks.calculate_cohort.insert_cohort_from_feature_flag */ + /* user_id:199 celery:posthog.tasks.calculate_cohort.insert_cohort_from_feature_flag */ SELECT count(DISTINCT person_id) FROM person_static_cohort WHERE team_id = 2 diff --git a/posthog/api/test/__snapshots__/test_query.ambr b/posthog/api/test/__snapshots__/test_query.ambr index 628038e741728..80df09eb3d411 100644 --- a/posthog/api/test/__snapshots__/test_query.ambr +++ b/posthog/api/test/__snapshots__/test_query.ambr @@ -157,7 +157,7 @@ # --- # name: TestQuery.test_full_hogql_query_async ''' - /* user_id:464 celery:posthog.tasks.tasks.process_query_task */ + /* user_id:465 celery:posthog.tasks.tasks.process_query_task */ SELECT events.uuid AS uuid, events.event AS event, events.properties AS properties, diff --git a/posthog/api/test/test_cohort.py b/posthog/api/test/test_cohort.py index fa0c63be36bf1..282a68d4787a6 100644 --- a/posthog/api/test/test_cohort.py +++ b/posthog/api/test/test_cohort.py @@ -695,6 +695,84 @@ def test_creating_update_and_calculating_with_new_cohort_filters(self, patch_cap self.assertEqual(response.status_code, 200, response.content) self.assertEqual(2, len(response.json()["results"])) + @patch("posthog.api.cohort.report_user_action") + def test_calculating_with_new_cohort_event_filters(self, patch_capture): + _create_person( + distinct_ids=["p1"], + team_id=self.team.pk, + properties={"$some_prop": "something"}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id="p1", + properties={"$filter_prop": "something"}, + timestamp=datetime.now() - timedelta(hours=12), + ) + + _create_person( + distinct_ids=["p2"], + team_id=self.team.pk, + properties={"$some_prop": "not it"}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id="p2", + properties={"$filter_prop": "something2"}, + timestamp=datetime.now() - timedelta(hours=12), + ) + + _create_person( + distinct_ids=["p3"], + team_id=self.team.pk, + properties={"$some_prop": "something"}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id="p3", + properties={"$filter_prop": "something2"}, + timestamp=datetime.now() - timedelta(days=12), + ) + + flush_persons_and_events() + + response = self.client.post( + f"/api/projects/{self.team.id}/cohorts", + data={ + "name": "cohort A", + "filters": { + "properties": { + "type": "OR", + "values": [ + { + "key": "$pageview", + "event_type": "events", + "time_value": 1, + "time_interval": "day", + "value": "performed_event", + "type": "behavioral", + "event_filters": [ + {"key": "$filter_prop", "value": "something", "operator": "exact", "type": "event"} + ], + }, + ], + } + }, + }, + ) + self.assertEqual(response.status_code, 201, response.content) + + cohort_id = response.json()["id"] + + while response.json()["is_calculating"]: + response = self.client.get(f"/api/projects/{self.team.id}/cohorts/{cohort_id}") + + response = self.client.get(f"/api/projects/{self.team.id}/cohorts/{cohort_id}/persons/?cohort={cohort_id}") + self.assertEqual(response.status_code, 200, response.content) + self.assertEqual(1, len(response.json()["results"])) + @patch("posthog.api.cohort.report_user_action") def test_creating_update_and_calculating_with_new_cohort_query(self, patch_capture): _create_person( diff --git a/posthog/models/property/property.py b/posthog/models/property/property.py index cf5402f5afbdb..defd098cd7ef7 100644 --- a/posthog/models/property/property.py +++ b/posthog/models/property/property.py @@ -103,20 +103,28 @@ class BehavioralPropertyType(str, Enum): "hogql": ["key"], } +VALIDATE_CONDITIONAL_BEHAVIORAL_PROP_TYPES = { + BehavioralPropertyType.PERFORMED_EVENT: [ + {"time_value", "time_interval"}, + {"explicit_datetime"}, + ], + BehavioralPropertyType.PERFORMED_EVENT_MULTIPLE: [ + {"time_value", "time_interval"}, + {"explicit_datetime"}, + ], +} + + VALIDATE_BEHAVIORAL_PROP_TYPES = { BehavioralPropertyType.PERFORMED_EVENT: [ "key", "value", "event_type", - "time_value", - "time_interval", ], BehavioralPropertyType.PERFORMED_EVENT_MULTIPLE: [ "key", "value", "event_type", - "time_value", - "time_interval", "operator_value", ], BehavioralPropertyType.PERFORMED_EVENT_FIRST_TIME: [ @@ -175,8 +183,11 @@ class Property: type: PropertyType group_type_index: Optional[GroupTypeIndex] + # All these property keys are used in cohorts. # Type of `key` event_type: Optional[Literal["events", "actions"]] + # Any extra filters on the event + event_filters: Optional[List["Property"]] # Query people who did event '$pageview' 20 times in the last 30 days # translates into: # key = '$pageview', value = 'performed_event_multiple' @@ -185,6 +196,8 @@ class Property: operator_value: Optional[int] time_value: Optional[int] time_interval: Optional[OperatorInterval] + # Alternative to time_value & time_interval, for explicit date bound rather than relative + explicit_datetime: Optional[str] # Query people who did event '$pageview' in last week, but not in the previous 30 days # translates into: # key = '$pageview', value = 'restarted_performing_event' @@ -218,6 +231,7 @@ def __init__( operator_value: Optional[int] = None, time_value: Optional[int] = None, time_interval: Optional[OperatorInterval] = None, + explicit_datetime: Optional[str] = None, total_periods: Optional[int] = None, min_periods: Optional[int] = None, seq_event_type: Optional[str] = None, @@ -225,6 +239,7 @@ def __init__( seq_time_value: Optional[int] = None, seq_time_interval: Optional[OperatorInterval] = None, negation: Optional[bool] = None, + event_filters: Optional[List["Property"]] = None, **kwargs, ) -> None: self.key = key @@ -235,6 +250,7 @@ def __init__( self.operator_value = operator_value self.time_value = time_value self.time_interval = time_interval + self.explicit_datetime = explicit_datetime self.total_periods = total_periods self.min_periods = min_periods self.seq_event_type = seq_event_type @@ -242,6 +258,7 @@ def __init__( self.seq_time_value = seq_time_value self.seq_time_interval = seq_time_interval self.negation = None if negation is None else str_to_bool(negation) + self.event_filters = event_filters if value is None and self.operator in ["is_set", "is_not_set"]: self.value = self.operator @@ -264,6 +281,19 @@ def __init__( if getattr(self, attr, None) is None: raise ValueError(f"Missing required attr {attr} for property type {self.type}::{self.value}") + if cast(BehavioralPropertyType, self.value) in VALIDATE_CONDITIONAL_BEHAVIORAL_PROP_TYPES: + matches_attr_list = False + condition_list = VALIDATE_CONDITIONAL_BEHAVIORAL_PROP_TYPES[cast(BehavioralPropertyType, self.value)] + for attr_list in condition_list: + if all(getattr(self, attr, None) is not None for attr in attr_list): + matches_attr_list = True + break + + if not matches_attr_list: + raise ValueError( + f"Missing required parameters, atleast one of values ({'), ('.join([' & '.join(condition) for condition in condition_list])}) for property type {self.type}::{self.value}" + ) + def __repr__(self): params_repr = ", ".join(f"{key}={repr(value)}" for key, value in self.to_dict().items()) return f"Property({params_repr})" diff --git a/posthog/queries/foss_cohort_query.py b/posthog/queries/foss_cohort_query.py index 19d6e8282ee93..34300757f58d8 100644 --- a/posthog/queries/foss_cohort_query.py +++ b/posthog/queries/foss_cohort_query.py @@ -18,10 +18,10 @@ PropertyGroup, PropertyName, ) -from posthog.models.property.util import prop_filter_json_extract +from posthog.models.property.util import prop_filter_json_extract, parse_prop_grouped_clauses from posthog.queries.event_query import EventQuery from posthog.queries.util import PersonPropertiesMode -from posthog.utils import PersonOnEventsMode +from posthog.utils import PersonOnEventsMode, relative_date_parse Relative_Date = Tuple[int, OperatorInterval] Event = Tuple[str, Union[str, int]] @@ -458,24 +458,57 @@ def get_static_cohort_condition(self, prop: Property, prepend: str, idx: int) -> query, params = format_static_cohort_query(cohort, idx, prepend) return f"id {'NOT' if prop.negation else ''} IN ({query})", params + def _get_entity_event_filters(self, prop: Property, prepend: str, idx: int) -> Tuple[str, Dict[str, Any]]: + params: Dict[str, Any] = {} + + if prop.event_filters: + prop_query, prop_params = parse_prop_grouped_clauses( + team_id=self._team_id, + property_group=Filter(data={"properties": prop.event_filters}).property_groups, + prepend=f"{prepend}_{idx}_event_filters", + # should be no person properties in these filters, but if there are, use + # the inefficient person subquery default mode + person_properties_mode=PersonPropertiesMode.USING_SUBQUERY, + hogql_context=self._filter.hogql_context, + ) + params.update(prop_params) + return prop_query, params + else: + return "AND 1=1", {} + + def _get_entity_datetime_filters(self, prop: Property, prepend: str, idx: int) -> Tuple[str, Dict[str, Any]]: + if prop.explicit_datetime: + # Explicit datetime filter, can be a relative or absolute date, follows same convention + # as all analytics datetime filters + # TODO: Confirm if we need to validate the datetime + date_param = f"{prepend}_explicit_date_{idx}" + target_datetime = relative_date_parse(prop.explicit_datetime, self._team.timezone_info) + return f"timestamp > %({date_param})s", {f"{date_param}": target_datetime} + else: + date_value = parse_and_validate_positive_integer(prop.time_value, "time_value") + date_interval = validate_interval(prop.time_interval) + date_param = f"{prepend}_date_{idx}" + + self._check_earliest_date((date_value, date_interval)) + + return f"timestamp > now() - INTERVAL %({date_param})s {date_interval}", {f"{date_param}": date_value} + def get_performed_event_condition(self, prop: Property, prepend: str, idx: int) -> Tuple[str, Dict[str, Any]]: event = (prop.event_type, prop.key) column_name = f"performed_event_condition_{prepend}_{idx}" entity_query, entity_params = self._get_entity(event, prepend, idx) - date_value = parse_and_validate_positive_integer(prop.time_value, "time_value") - date_interval = validate_interval(prop.time_interval) - date_param = f"{prepend}_date_{idx}" - - self._check_earliest_date((date_value, date_interval)) + entity_filters, entity_filters_params = self._get_entity_event_filters(prop, prepend, idx) + date_filter, date_params = self._get_entity_datetime_filters(prop, prepend, idx) - field = f"countIf(timestamp > now() - INTERVAL %({date_param})s {date_interval} AND timestamp < now() AND {entity_query}) > 0 AS {column_name}" + field = f"countIf({date_filter} AND timestamp < now() AND {entity_query} {entity_filters}) > 0 AS {column_name}" self._fields.append(field) # Negation is handled in the where clause to ensure the right result if a full join occurs where the joined person did not perform the event return f"{'NOT' if prop.negation else ''} {column_name}", { - f"{date_param}": date_value, + **date_params, **entity_params, + **entity_filters_params, } def get_performed_event_multiple(self, prop: Property, prepend: str, idx: int) -> Tuple[str, Dict[str, Any]]: @@ -483,15 +516,13 @@ def get_performed_event_multiple(self, prop: Property, prepend: str, idx: int) - column_name = f"performed_event_multiple_condition_{prepend}_{idx}" entity_query, entity_params = self._get_entity(event, prepend, idx) + entity_filters, entity_filters_params = self._get_entity_event_filters(prop, prepend, idx) + date_filter, date_params = self._get_entity_datetime_filters(prop, prepend, idx) + count = parse_and_validate_positive_integer(prop.operator_value, "operator_value") - date_value = parse_and_validate_positive_integer(prop.time_value, "time_value") - date_interval = validate_interval(prop.time_interval) - date_param = f"{prepend}_date_{idx}" operator_value_param = f"{prepend}_operator_value_{idx}" - self._check_earliest_date((date_value, date_interval)) - - field = f"countIf(timestamp > now() - INTERVAL %({date_param})s {date_interval} AND timestamp < now() AND {entity_query}) {get_count_operator(prop.operator)} %({operator_value_param})s AS {column_name}" + field = f"countIf({date_filter} AND timestamp < now() AND {entity_query} {entity_filters}) {get_count_operator(prop.operator)} %({operator_value_param})s AS {column_name}" self._fields.append(field) # Negation is handled in the where clause to ensure the right result if a full join occurs where the joined person did not perform the event @@ -499,8 +530,9 @@ def get_performed_event_multiple(self, prop: Property, prepend: str, idx: int) - f"{'NOT' if prop.negation else ''} {column_name}", { f"{operator_value_param}": count, - f"{date_param}": date_value, + **date_params, **entity_params, + **entity_filters_params, }, )