From bbfebce24972acb39166beeb24512eee8bdfb2a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Oberm=C3=BCller?= Date: Fri, 19 Jan 2024 15:03:32 +0100 Subject: [PATCH] chore(query-nodes): camel case retention filter (#19800) --- frontend/src/queries/examples.ts | 4 +- .../queries/nodes/InsightQuery/defaults.ts | 8 +- .../utils/filtersToQueryNode.test.ts | 18 ++-- .../InsightQuery/utils/filtersToQueryNode.ts | 10 +-- .../InsightQuery/utils/queryNodeToFilter.ts | 14 ++++ frontend/src/queries/schema.json | 25 ++++++ frontend/src/queries/schema.ts | 11 ++- .../EditorFilters/RetentionSummary.tsx | 30 +++---- .../InsightNav/insightNavLogic.test.ts | 4 +- .../insights/InsightNav/insightNavLogic.tsx | 24 +++--- .../filters/RetentionReferencePicker.tsx | 8 +- .../scenes/insights/summarizeInsight.test.ts | 12 +-- .../src/scenes/insights/summarizeInsight.ts | 10 +-- .../notebooks/Notebook/SlashCommands.tsx | 8 +- frontend/src/scenes/retention/queries.ts | 2 +- .../retention/retentionLineGraphLogic.ts | 6 +- .../scenes/web-analytics/webAnalyticsLogic.ts | 6 +- .../insights/retention_query_runner.py | 8 +- .../test/test_retention_query_runner.py | 82 +++++++++---------- .../legacy_compatibility/filter_to_query.py | 10 +-- .../test/test_filter_to_query.py | 8 +- posthog/schema.py | 12 +++ 22 files changed, 190 insertions(+), 130 deletions(-) diff --git a/frontend/src/queries/examples.ts b/frontend/src/queries/examples.ts index 429b038e862a7..33f0a612b31e8 100644 --- a/frontend/src/queries/examples.ts +++ b/frontend/src/queries/examples.ts @@ -218,8 +218,8 @@ const InsightRetentionQuery: RetentionQuery = { filterTestAccounts, retentionFilter: { // TODO: this should be typed as (EventsNode | ActionsNode)[] without math and properties - target_entity: { type: 'events', id: '$pageview', name: '$pageview' }, - returning_entity: { type: 'events', id: '$pageview', name: '$pageview' }, + targetEntity: { type: 'events', id: '$pageview', name: '$pageview' }, + returningEntity: { type: 'events', id: '$pageview', name: '$pageview' }, }, } diff --git a/frontend/src/queries/nodes/InsightQuery/defaults.ts b/frontend/src/queries/nodes/InsightQuery/defaults.ts index 369362dfc517a..c8742c3f35b58 100644 --- a/frontend/src/queries/nodes/InsightQuery/defaults.ts +++ b/frontend/src/queries/nodes/InsightQuery/defaults.ts @@ -42,18 +42,18 @@ const retentionQueryDefault: RetentionQuery = { kind: NodeKind.RetentionQuery, retentionFilter: { period: RetentionPeriod.Day, - total_intervals: 11, - target_entity: { + totalIntervals: 11, + targetEntity: { id: '$pageview', name: '$pageview', type: 'events', }, - returning_entity: { + returningEntity: { id: '$pageview', name: '$pageview', type: 'events', }, - retention_type: 'retention_first_time', + retentionType: 'retention_first_time', }, } diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts index 19dca7010808e..e96f51797b85a 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts @@ -423,11 +423,11 @@ describe('filtersToQueryNode', () => { const query: RetentionQuery = { kind: NodeKind.RetentionQuery, retentionFilter: { - retention_type: 'retention_first_time', - retention_reference: 'total', - total_intervals: 2, - returning_entity: { id: '1' }, - target_entity: { id: '1' }, + retentionType: 'retention_first_time', + retentionReference: 'total', + totalIntervals: 2, + returningEntity: { id: '1' }, + targetEntity: { id: '1' }, period: RetentionPeriod.Day, }, } @@ -674,15 +674,15 @@ describe('filtersToQueryNode', () => { }, retentionFilter: { period: RetentionPeriod.Week, - target_entity: { + targetEntity: { id: 'signed_up', name: 'signed_up', type: 'events', order: 0, }, - retention_type: 'retention_first_time', - total_intervals: 9, - returning_entity: { + retentionType: 'retention_first_time', + totalIntervals: 9, + returningEntity: { id: 1, name: 'Interacted with file', type: 'actions', diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts index a29007e6a2134..f826849125c51 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts @@ -297,11 +297,11 @@ export const filtersToQueryNode = (filters: Partial): InsightQueryNo // retention filter if (isRetentionFilter(filters) && isRetentionQuery(query)) { query.retentionFilter = objectCleanWithEmpty({ - retention_type: filters.retention_type, - retention_reference: filters.retention_reference, - total_intervals: filters.total_intervals, - returning_entity: filters.returning_entity, - target_entity: filters.target_entity, + retentionType: filters.retention_type, + retentionReference: filters.retention_reference, + totalIntervals: filters.total_intervals, + returningEntity: filters.returning_entity, + targetEntity: filters.target_entity, period: filters.period, }) // TODO: query.aggregation_group_type_index diff --git a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts index b23cc1e16b524..fab44bffaf3c9 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts @@ -10,6 +10,7 @@ import { LifecycleFilterLegacy, NodeKind, PathsFilterLegacy, + RetentionFilterLegacy, StickinessFilterLegacy, TrendsFilterLegacy, } from '~/queries/schema' @@ -154,6 +155,7 @@ export const queryNodeToFilter = (query: InsightQueryNode): Partial // replace camel cased props with the snake cased variant const camelCasedTrendsProps: TrendsFilterLegacy = {} + const camelCasedRetentionProps: RetentionFilterLegacy = {} const camelCasedPathsProps: PathsFilterLegacy = {} const camelCasedStickinessProps: StickinessFilterLegacy = {} const camelCasedLifecycleProps: LifecycleFilterLegacy = {} @@ -176,6 +178,17 @@ export const queryNodeToFilter = (query: InsightQueryNode): Partial delete queryCopy.trendsFilter?.showPercentStackView delete queryCopy.trendsFilter?.showLegend delete queryCopy.trendsFilter?.showValuesOnSeries + } else if (isRetentionQuery(queryCopy)) { + camelCasedRetentionProps.retention_reference = queryCopy.retentionFilter?.retentionReference + camelCasedRetentionProps.retention_type = queryCopy.retentionFilter?.retentionType + camelCasedRetentionProps.returning_entity = queryCopy.retentionFilter?.returningEntity + camelCasedRetentionProps.target_entity = queryCopy.retentionFilter?.targetEntity + camelCasedRetentionProps.total_intervals = queryCopy.retentionFilter?.totalIntervals + delete queryCopy.retentionFilter?.retentionReference + delete queryCopy.retentionFilter?.retentionType + delete queryCopy.retentionFilter?.returningEntity + delete queryCopy.retentionFilter?.targetEntity + delete queryCopy.retentionFilter?.totalIntervals } else if (isPathsQuery(queryCopy)) { camelCasedPathsProps.edge_limit = queryCopy.pathsFilter?.edgeLimit camelCasedPathsProps.paths_hogql_expression = queryCopy.pathsFilter?.pathsHogQLExpression @@ -215,6 +228,7 @@ export const queryNodeToFilter = (query: InsightQueryNode): Partial delete queryCopy.lifecycleFilter?.showValuesOnSeries } Object.assign(filters, camelCasedTrendsProps) + Object.assign(filters, camelCasedRetentionProps) Object.assign(filters, camelCasedPathsProps) Object.assign(filters, camelCasedStickinessProps) Object.assign(filters, camelCasedLifecycleProps) diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index f492314e17a37..80c7780105d38 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -3227,6 +3227,31 @@ "type": "object" }, "RetentionFilter": { + "additionalProperties": false, + "properties": { + "period": { + "$ref": "#/definitions/RetentionPeriod" + }, + "retentionReference": { + "enum": ["total", "previous"], + "type": "string" + }, + "retentionType": { + "$ref": "#/definitions/RetentionType" + }, + "returningEntity": { + "$ref": "#/definitions/RetentionEntity" + }, + "targetEntity": { + "$ref": "#/definitions/RetentionEntity" + }, + "totalIntervals": { + "type": "integer" + } + }, + "type": "object" + }, + "RetentionFilterLegacy": { "additionalProperties": false, "description": "`RetentionFilterType` minus everything inherited from `FilterType`", "properties": { diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index ff86ff420e8fd..e5afeab3ba479 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -574,7 +574,16 @@ export interface FunnelsQuery extends InsightsQueryBase { } /** `RetentionFilterType` minus everything inherited from `FilterType` */ -export type RetentionFilter = Omit +export type RetentionFilterLegacy = Omit + +export type RetentionFilter = { + retentionType?: RetentionFilterLegacy['retention_type'] + retentionReference?: RetentionFilterLegacy['retention_reference'] + totalIntervals?: RetentionFilterLegacy['total_intervals'] + returningEntity?: RetentionFilterLegacy['returning_entity'] + targetEntity?: RetentionFilterLegacy['target_entity'] + period?: RetentionFilterLegacy['period'] +} export interface RetentionValue { /** @asType integer */ diff --git a/frontend/src/scenes/insights/EditorFilters/RetentionSummary.tsx b/frontend/src/scenes/insights/EditorFilters/RetentionSummary.tsx index 8859b225ef419..51ec482717b1b 100644 --- a/frontend/src/scenes/insights/EditorFilters/RetentionSummary.tsx +++ b/frontend/src/scenes/insights/EditorFilters/RetentionSummary.tsx @@ -23,7 +23,7 @@ export function RetentionSummary({ insightProps }: EditorFilterProps): JSX.Eleme const { showGroupsOptions } = useValues(groupsModel) const { retentionFilter } = useValues(insightVizDataLogic(insightProps)) const { updateInsightFilter } = useActions(insightVizDataLogic(insightProps)) - const { target_entity, returning_entity, retention_type, total_intervals, period } = retentionFilter || {} + const { targetEntity, returningEntity, retentionType, totalIntervals, period } = retentionFilter || {} return (
@@ -45,17 +45,17 @@ export function RetentionSummary({ insightProps }: EditorFilterProps): JSX.Eleme hideFilter hideRename buttonCopy="Add graph series" - filters={{ events: [target_entity] } as FilterType} // retention filters use target and returning entity instead of events + filters={{ events: [targetEntity] } as FilterType} // retention filters use target and returning entity instead of events setFilters={(newFilters: FilterType) => { if (newFilters.events && newFilters.events.length > 0) { - updateInsightFilter({ target_entity: newFilters.events[0] }) + updateInsightFilter({ targetEntity: newFilters.events[0] }) } else if (newFilters.actions && newFilters.actions.length > 0) { - updateInsightFilter({ target_entity: newFilters.actions[0] }) + updateInsightFilter({ targetEntity: newFilters.actions[0] }) } else { - updateInsightFilter({ target_entity: undefined }) + updateInsightFilter({ targetEntity: undefined }) } }} - typeKey={`${keyForInsightLogicProps('new')(insightProps)}-target_entity`} + typeKey={`${keyForInsightLogicProps('new')(insightProps)}-targetEntity`} /> ), }))} - value={retention_type ? retentionOptions[retention_type] : undefined} - onChange={(value): void => updateInsightFilter({ retention_type: value as RetentionType })} + value={retentionType ? retentionOptions[retentionType] : undefined} + onChange={(value): void => updateInsightFilter({ retentionType: value as RetentionType })} dropdownMatchSelectWidth={false} />
@@ -81,8 +81,8 @@ export function RetentionSummary({ insightProps }: EditorFilterProps): JSX.Eleme updateInsightFilter({ total_intervals: (value || 0) + 1 })} + value={(totalIntervals ?? 11) - 1} + onChange={(value) => updateInsightFilter({ totalIntervals: (value || 0) + 1 })} /> { if (newFilters.events && newFilters.events.length > 0) { - updateInsightFilter({ returning_entity: newFilters.events[0] }) + updateInsightFilter({ returningEntity: newFilters.events[0] }) } else if (newFilters.actions && newFilters.actions.length > 0) { - updateInsightFilter({ returning_entity: newFilters.actions[0] }) + updateInsightFilter({ returningEntity: newFilters.actions[0] }) } else { - updateInsightFilter({ returning_entity: undefined }) + updateInsightFilter({ returningEntity: undefined }) } }} - typeKey={`${keyForInsightLogicProps('new')(insightProps)}-returning_entity`} + typeKey={`${keyForInsightLogicProps('new')(insightProps)}-returningEntity`} /> on any of the next {dateOptionPlurals[period ?? 'Day']}. diff --git a/frontend/src/scenes/insights/InsightNav/insightNavLogic.test.ts b/frontend/src/scenes/insights/InsightNav/insightNavLogic.test.ts index 872148e08a713..b9e4b0cd990fd 100644 --- a/frontend/src/scenes/insights/InsightNav/insightNavLogic.test.ts +++ b/frontend/src/scenes/insights/InsightNav/insightNavLogic.test.ts @@ -165,12 +165,12 @@ describe('insightNavLogic', () => { // source: { // kind: NodeKind.RetentionQuery, // retentionFilter: { - // returning_entity: { + // returningEntity: { // id: 'returning', // name: 'returning', // type: 'events', // }, - // target_entity: { + // targetEntity: { // id: 'target', // name: 'target', // type: 'events', diff --git a/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx b/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx index f5e6de98cebb0..a857419ad25af 100644 --- a/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx +++ b/frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx @@ -258,15 +258,15 @@ const cachePropertiesFromQuery = (query: InsightQueryNode, cache: QueryPropertyC // // set series (first two entries) from retention target and returning entity // if (isRetentionQuery(query)) { - // const { target_entity, returning_entity } = query.retentionFilter || {} + // const { targetEntity, returningEntity } = query.retentionFilter || {} // const series = actionsAndEventsToSeries({ // events: [ - // ...(target_entity?.type === 'events' ? [target_entity as ActionFilter] : []), - // ...(returning_entity?.type === 'events' ? [returning_entity as ActionFilter] : []), + // ...(targetEntity?.type === 'events' ? [targetEntity as ActionFilter] : []), + // ...(returningEntity?.type === 'events' ? [returningEntity as ActionFilter] : []), // ], // actions: [ - // ...(target_entity?.type === 'actions' ? [target_entity as ActionFilter] : []), - // ...(returning_entity?.type === 'actions' ? [returning_entity as ActionFilter] : []), + // ...(targetEntity?.type === 'actions' ? [targetEntity as ActionFilter] : []), + // ...(returningEntity?.type === 'actions' ? [returningEntity as ActionFilter] : []), // ], // }) // if (series.length > 0) { @@ -301,21 +301,21 @@ const mergeCachedProperties = (query: InsightQueryNode, cache: QueryPropertyCach } else { mergedQuery.series = cache.series } - } else if (cache.retentionFilter?.target_entity || cache.retentionFilter?.returning_entity) { + } else if (cache.retentionFilter?.targetEntity || cache.retentionFilter?.returningEntity) { mergedQuery.series = [ - ...(cache.retentionFilter.target_entity - ? [cache.retentionFilter.target_entity as EventsNode | ActionsNode] + ...(cache.retentionFilter.targetEntity + ? [cache.retentionFilter.targetEntity as EventsNode | ActionsNode] : []), - ...(cache.retentionFilter.returning_entity - ? [cache.retentionFilter.returning_entity as EventsNode | ActionsNode] + ...(cache.retentionFilter.returningEntity + ? [cache.retentionFilter.returningEntity as EventsNode | ActionsNode] : []), ] } } else if (isRetentionQuery(mergedQuery) && cache.series) { mergedQuery.retentionFilter = { ...mergedQuery.retentionFilter, - ...(cache.series.length > 0 ? { target_entity: cache.series[0] } : {}), - ...(cache.series.length > 1 ? { returning_entity: cache.series[1] } : {}), + ...(cache.series.length > 0 ? { targetEntity: cache.series[0] } : {}), + ...(cache.series.length > 1 ? { returningEntity: cache.series[1] } : {}), } } diff --git a/frontend/src/scenes/insights/filters/RetentionReferencePicker.tsx b/frontend/src/scenes/insights/filters/RetentionReferencePicker.tsx index 5ccac5b5c4883..f96c9e5ee462b 100644 --- a/frontend/src/scenes/insights/filters/RetentionReferencePicker.tsx +++ b/frontend/src/scenes/insights/filters/RetentionReferencePicker.tsx @@ -8,15 +8,15 @@ export function RetentionReferencePicker(): JSX.Element { const { retentionFilter } = useValues(insightVizDataLogic(insightProps)) const { updateInsightFilter } = useActions(insightVizDataLogic(insightProps)) - const { retention_reference } = retentionFilter || {} + const { retentionReference } = retentionFilter || {} return ( { - updateInsightFilter({ retention_reference }) + value={retentionReference || 'total'} + onChange={(retentionReference) => { + updateInsightFilter({ retentionReference }) }} options={[ { diff --git a/frontend/src/scenes/insights/summarizeInsight.test.ts b/frontend/src/scenes/insights/summarizeInsight.test.ts index 68c5330a1f97c..ad8d8cf548241 100644 --- a/frontend/src/scenes/insights/summarizeInsight.test.ts +++ b/frontend/src/scenes/insights/summarizeInsight.test.ts @@ -728,17 +728,17 @@ describe('summarizing insights', () => { const query: RetentionQuery = { kind: NodeKind.RetentionQuery, retentionFilter: { - target_entity: { + targetEntity: { id: '$autocapture', name: '$autocapture', type: 'events', }, - returning_entity: { + returningEntity: { id: '$autocapture', name: '$autocapture', type: 'events', }, - retention_type: RETENTION_FIRST_TIME, + retentionType: RETENTION_FIRST_TIME, }, } @@ -757,17 +757,17 @@ describe('summarizing insights', () => { const query: RetentionQuery = { kind: NodeKind.RetentionQuery, retentionFilter: { - target_entity: { + targetEntity: { id: 'purchase', name: 'purchase', type: 'events', }, - returning_entity: { + returningEntity: { id: '$pageview', name: '$pageview', type: 'events', }, - retention_type: RETENTION_RECURRING, + retentionType: RETENTION_RECURRING, }, aggregation_group_type_index: 0, } diff --git a/frontend/src/scenes/insights/summarizeInsight.ts b/frontend/src/scenes/insights/summarizeInsight.ts index 2b6e28644a221..d99cbcf763ba6 100644 --- a/frontend/src/scenes/insights/summarizeInsight.ts +++ b/frontend/src/scenes/insights/summarizeInsight.ts @@ -250,17 +250,17 @@ export function summarizeInsightQuery(query: InsightQueryNode, context: SummaryC return summary } else if (isRetentionQuery(query)) { const areTargetAndReturningIdentical = - query.retentionFilter?.returning_entity?.id === query.retentionFilter?.target_entity?.id && - query.retentionFilter?.returning_entity?.type === query.retentionFilter?.target_entity?.type + query.retentionFilter?.returningEntity?.id === query.retentionFilter?.targetEntity?.id && + query.retentionFilter?.returningEntity?.type === query.retentionFilter?.targetEntity?.type return ( `Retention of ${context.aggregationLabel(query.aggregation_group_type_index, true).plural}` + ` based on doing ${getDisplayNameFromEntityFilter( - (query.retentionFilter?.target_entity || {}) as EntityFilter + (query.retentionFilter?.targetEntity || {}) as EntityFilter )}` + - ` ${retentionOptions[query.retentionFilter?.retention_type || RETENTION_FIRST_TIME]} and returning with ` + + ` ${retentionOptions[query.retentionFilter?.retentionType || RETENTION_FIRST_TIME]} and returning with ` + (areTargetAndReturningIdentical ? 'the same event' - : getDisplayNameFromEntityFilter((query.retentionFilter?.returning_entity || {}) as EntityFilter)) + : getDisplayNameFromEntityFilter((query.retentionFilter?.returningEntity || {}) as EntityFilter)) ) } else if (isPathsQuery(query)) { // Sync format with PathsSummary in InsightDetails diff --git a/frontend/src/scenes/notebooks/Notebook/SlashCommands.tsx b/frontend/src/scenes/notebooks/Notebook/SlashCommands.tsx index 647645e6af1b6..3791046e658d5 100644 --- a/frontend/src/scenes/notebooks/Notebook/SlashCommands.tsx +++ b/frontend/src/scenes/notebooks/Notebook/SlashCommands.tsx @@ -160,18 +160,18 @@ const SLASH_COMMANDS: SlashCommandsItem[] = [ kind: NodeKind.RetentionQuery, retentionFilter: { period: RetentionPeriod.Day, - total_intervals: 11, - target_entity: { + totalIntervals: 11, + targetEntity: { id: '$pageview', name: '$pageview', type: 'events', }, - returning_entity: { + returningEntity: { id: '$pageview', name: '$pageview', type: 'events', }, - retention_type: 'retention_first_time', + retentionType: 'retention_first_time', }, }) ), diff --git a/frontend/src/scenes/retention/queries.ts b/frontend/src/scenes/retention/queries.ts index 97fb39b2614d0..bc840e0dcacb5 100644 --- a/frontend/src/scenes/retention/queries.ts +++ b/frontend/src/scenes/retention/queries.ts @@ -6,7 +6,7 @@ import { ActorsQuery, NodeKind, RetentionQuery } from '~/queries/schema' export function retentionToActorsQuery(query: RetentionQuery, selectedInterval: number, offset = 0): ActorsQuery { const group = query.aggregation_group_type_index !== undefined const selectActor = group ? 'group' : 'person' - const totalIntervals = (query.retentionFilter.total_intervals || 11) - selectedInterval + const totalIntervals = (query.retentionFilter.totalIntervals || 11) - selectedInterval const periodName = query.retentionFilter.period?.toLowerCase() ?? 'day' const selects = Array.from({ length: totalIntervals }, (_, intervalNumber) => `${periodName}_${intervalNumber}`) return { diff --git a/frontend/src/scenes/retention/retentionLineGraphLogic.ts b/frontend/src/scenes/retention/retentionLineGraphLogic.ts index ba460e3a88ea7..ec90368adf15a 100644 --- a/frontend/src/scenes/retention/retentionLineGraphLogic.ts +++ b/frontend/src/scenes/retention/retentionLineGraphLogic.ts @@ -29,7 +29,7 @@ export const retentionLineGraphLogic = kea([ trendSeries: [ (s) => [s.results, s.retentionFilter], (results, retentionFilter): RetentionTrendPayload[] => { - const { period, retention_reference } = retentionFilter || {} + const { period, retentionReference } = retentionFilter || {} // If the retention reference option is specified as previous, // then translate retention rates to relative to previous, // otherwise, just use what the result was originally. @@ -39,7 +39,7 @@ export const retentionLineGraphLogic = kea([ // Cohort 1 | 1000 | 120 | 190 | 170 | 140 // Cohort 2 | 6003 | 300 | 100 | 120 | 50 // - // If `retentionFilter.retention_reference` is not "previous" + // If `retentionFilter.retentionReference` is not "previous" // we want to calculate the percentages of the sizes compared // to the first value. If we have "previous" we want to go // further and translate these numbers into percentage of the @@ -74,7 +74,7 @@ export const retentionLineGraphLogic = kea([ : dayjs(cohortRetention.date).format('MMM D') : cohortRetention.label, data: - retention_reference === 'previous' + retentionReference === 'previous' ? retentionPercentages // Zip together the current a previous values, filling // in with 100 for the first index diff --git a/frontend/src/scenes/web-analytics/webAnalyticsLogic.ts b/frontend/src/scenes/web-analytics/webAnalyticsLogic.ts index 98df4dd8c5fc0..ec0c11f7bdc81 100644 --- a/frontend/src/scenes/web-analytics/webAnalyticsLogic.ts +++ b/frontend/src/scenes/web-analytics/webAnalyticsLogic.ts @@ -773,9 +773,9 @@ export const webAnalyticsLogic = kea([ dateRange, filterTestAccounts: true, retentionFilter: { - retention_type: RETENTION_FIRST_TIME, - retention_reference: 'total', - total_intervals: isGreaterThanMd ? 8 : 5, + retentionType: RETENTION_FIRST_TIME, + retentionReference: 'total', + totalIntervals: isGreaterThanMd ? 8 : 5, period: RetentionPeriod.Week, }, }, diff --git a/posthog/hogql_queries/insights/retention_query_runner.py b/posthog/hogql_queries/insights/retention_query_runner.py index f9e4cfd71f31d..b0ff701460395 100644 --- a/posthog/hogql_queries/insights/retention_query_runner.py +++ b/posthog/hogql_queries/insights/retention_query_runner.py @@ -58,11 +58,11 @@ def get_applicable_entity(self, event_query_type): "type": TREND_FILTER_TYPE_EVENTS, } ) - target_entity = self.query.retentionFilter.target_entity or default_entity + target_entity = self.query.retentionFilter.targetEntity or default_entity if event_query_type in [RetentionQueryType.TARGET, RetentionQueryType.TARGET_FIRST_TIME]: return target_entity - return self.query.retentionFilter.returning_entity or target_entity + return self.query.retentionFilter.returningEntity or target_entity def retention_events_query(self, event_query_type) -> ast.SelectQuery: start_of_interval_sql = self.query_date_range.get_start_of_interval_hogql( @@ -186,7 +186,7 @@ def date_filter_expr(self, event_query_type) -> ast.Expr: def build_target_event_query(self) -> ast.SelectQuery: event_query_type = ( RetentionQueryType.TARGET_FIRST_TIME - if self.query.retentionFilter.retention_type == RetentionType.retention_first_time + if self.query.retentionFilter.retentionType == RetentionType.retention_first_time else RetentionQueryType.TARGET ) return self.retention_events_query(event_query_type=event_query_type) @@ -266,7 +266,7 @@ def to_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: @cached_property def query_date_range(self) -> QueryDateRangeWithIntervals: - total_intervals = self.query.retentionFilter.total_intervals or DEFAULT_TOTAL_INTERVALS + total_intervals = self.query.retentionFilter.totalIntervals or DEFAULT_TOTAL_INTERVALS interval = ( IntervalType(self.query.retentionFilter.period.lower()) if self.query.retentionFilter.period diff --git a/posthog/hogql_queries/insights/test/test_retention_query_runner.py b/posthog/hogql_queries/insights/test/test_retention_query_runner.py index 93ade3ac992b8..bdb1151662454 100644 --- a/posthog/hogql_queries/insights/test/test_retention_query_runner.py +++ b/posthog/hogql_queries/insights/test/test_retention_query_runner.py @@ -223,7 +223,7 @@ def test_month_interval(self): "dateRange": {"date_to": _date(0, month=5, hour=0)}, "retentionFilter": { "period": "Month", - "total_intervals": 11, + "totalIntervals": 11, }, } ) @@ -397,7 +397,7 @@ def test_month_interval_with_person_on_events_v2(self): "dateRange": {"date_to": _date(0, month=5, hour=0)}, "retentionFilter": { "period": "Month", - "total_intervals": 11, + "totalIntervals": 11, }, } ) @@ -490,7 +490,7 @@ def test_week_interval(self): "dateRange": {"date_to": _date(10, month=1, hour=0)}, "retentionFilter": { "period": "Week", - "total_intervals": 7, + "totalIntervals": 7, }, } result_sunday = self.run_query(query=query) @@ -598,7 +598,7 @@ def test_hour_interval(self): "dateRange": {"date_to": _date(0, hour=16, minute=13)}, "retentionFilter": { "period": "Hour", - "total_intervals": 11, + "totalIntervals": 11, }, } ) @@ -690,7 +690,7 @@ def test_interval_rounding(self): "dateRange": {"date_to": _date(14, month=1, hour=0)}, "retentionFilter": { "period": "Week", - "total_intervals": 7, + "totalIntervals": 7, }, } ) @@ -780,9 +780,9 @@ def test_retention_people_first_time(self): query={ "dateRange": {"date_to": _date(10, hour=6)}, "retentionFilter": { - "target_entity": {"id": "$user_signed_up", "type": TREND_FILTER_TYPE_EVENTS}, - "returning_entity": {"id": "$pageview", "type": "events"}, - "retention_type": RETENTION_FIRST_TIME, + "targetEntity": {"id": "$user_signed_up", "type": TREND_FILTER_TYPE_EVENTS}, + "returningEntity": {"id": "$pageview", "type": "events"}, + "retentionType": RETENTION_FIRST_TIME, }, }, ) @@ -795,9 +795,9 @@ def test_retention_people_first_time(self): query={ "dateRange": {"date_to": _date(14, hour=6)}, "retentionFilter": { - "target_entity": {"id": "$user_signed_up", "type": TREND_FILTER_TYPE_EVENTS}, - "returning_entity": {"id": "$pageview", "type": "events"}, - "retention_type": RETENTION_FIRST_TIME, + "targetEntity": {"id": "$user_signed_up", "type": TREND_FILTER_TYPE_EVENTS}, + "returningEntity": {"id": "$pageview", "type": "events"}, + "retentionType": RETENTION_FIRST_TIME, }, }, ) @@ -882,9 +882,9 @@ def test_retention_people_in_period_first_time(self): query={ "dateRange": {"date_to": _date(10, hour=6)}, "retentionFilter": { - "target_entity": {"id": "$user_signed_up", "type": TREND_FILTER_TYPE_EVENTS}, - "returning_entity": {"id": "$pageview", "type": "events"}, - "retention_type": RETENTION_FIRST_TIME, + "targetEntity": {"id": "$user_signed_up", "type": TREND_FILTER_TYPE_EVENTS}, + "returningEntity": {"id": "$pageview", "type": "events"}, + "retentionType": RETENTION_FIRST_TIME, }, }, ) @@ -926,9 +926,9 @@ def test_retention_multiple_events(self): "dateRange": {"date_to": _date(6, hour=6)}, "retentionFilter": { "period": "Day", - "total_intervals": 7, - "target_entity": {"id": first_event, "name": first_event, "type": TREND_FILTER_TYPE_EVENTS}, - "returning_entity": {"id": "$pageview", "name": "$pageview", "type": "events"}, + "totalIntervals": 7, + "targetEntity": {"id": first_event, "name": first_event, "type": TREND_FILTER_TYPE_EVENTS}, + "returningEntity": {"id": "$pageview", "name": "$pageview", "type": "events"}, }, } ) @@ -984,9 +984,9 @@ def test_retention_any_event(self): "dateRange": {"date_to": _date(6, hour=6)}, "retentionFilter": { "period": "Day", - "total_intervals": 7, - "target_entity": {"id": None, "type": "events"}, - "returning_entity": {"id": None, "type": "events"}, + "totalIntervals": 7, + "targetEntity": {"id": None, "type": "events"}, + "returningEntity": {"id": None, "type": "events"}, }, } ) @@ -1035,13 +1035,13 @@ def test_retention_event_action(self): query={ "dateRange": {"date_to": _date(6, hour=0)}, "retentionFilter": { - "total_intervals": 7, - "target_entity": { + "totalIntervals": 7, + "targetEntity": { "id": action.pk, "name": action.name, "type": TREND_FILTER_TYPE_ACTIONS, }, - "returning_entity": { + "returningEntity": { "id": some_event, "name": some_event, "type": TREND_FILTER_TYPE_EVENTS, @@ -1078,14 +1078,14 @@ def test_first_time_retention(self): "dateRange": {"date_to": _date(5, hour=6)}, "retentionFilter": { "period": "Day", - "total_intervals": 7, - "retention_type": RETENTION_FIRST_TIME, - "target_entity": { + "totalIntervals": 7, + "retentionType": RETENTION_FIRST_TIME, + "targetEntity": { "id": "$user_signed_up", "name": "$user_signed_up", "type": TREND_FILTER_TYPE_EVENTS, }, - "returning_entity": {"id": "$pageview", "name": "$pageview", "type": "events"}, + "returningEntity": {"id": "$pageview", "name": "$pageview", "type": "events"}, }, } ) @@ -1233,7 +1233,7 @@ def test_retention_with_user_properties(self): ], }, "retentionFilter": { - "total_intervals": 7, + "totalIntervals": 7, }, } ) @@ -1297,9 +1297,9 @@ def test_retention_with_user_properties_via_action(self): query={ "dateRange": {"date_to": _date(6, hour=0)}, "retentionFilter": { - "total_intervals": 7, - "target_entity": {"id": action.pk, "name": action.name, "type": TREND_FILTER_TYPE_ACTIONS}, - "returning_entity": {"id": "$pageview", "name": "$pageview", "type": "events"}, + "totalIntervals": 7, + "targetEntity": {"id": action.pk, "name": action.name, "type": TREND_FILTER_TYPE_ACTIONS}, + "returningEntity": {"id": "$pageview", "name": "$pageview", "type": "events"}, }, } ) @@ -1348,9 +1348,9 @@ def test_retention_action_start_point(self): "dateRange": {"date_to": _date(6, hour=0)}, "retentionFilter": { "period": "Day", - "total_intervals": 7, - "target_entity": {"id": action.pk, "name": action.name, "type": TREND_FILTER_TYPE_ACTIONS}, - "returning_entity": {"id": action.pk, "name": action.name, "type": TREND_FILTER_TYPE_ACTIONS}, + "totalIntervals": 7, + "targetEntity": {"id": action.pk, "name": action.name, "type": TREND_FILTER_TYPE_ACTIONS}, + "returningEntity": {"id": action.pk, "name": action.name, "type": TREND_FILTER_TYPE_ACTIONS}, }, } ) @@ -1741,7 +1741,7 @@ def test_groups_aggregating(self): "aggregation_group_type_index": 0, "retentionFilter": { "period": "Week", - "total_intervals": 7, + "totalIntervals": 7, }, } ) @@ -1765,7 +1765,7 @@ def test_groups_aggregating(self): "aggregation_group_type_index": 0, "retentionFilter": { "period": "Week", - "total_intervals": 7, + "totalIntervals": 7, }, }, actor="group", @@ -1778,7 +1778,7 @@ def test_groups_aggregating(self): "aggregation_group_type_index": 1, "retentionFilter": { "period": "Week", - "total_intervals": 7, + "totalIntervals": 7, }, } ) @@ -1805,7 +1805,7 @@ def test_groups_in_period(self): "aggregation_group_type_index": 0, "retentionFilter": { "period": "Week", - "total_intervals": 7, + "totalIntervals": 7, }, }, actor="group", @@ -1827,7 +1827,7 @@ def test_groups_aggregating_person_on_events(self): "aggregation_group_type_index": 0, "retentionFilter": { "period": "Week", - "total_intervals": 7, + "totalIntervals": 7, }, } ) @@ -1851,7 +1851,7 @@ def test_groups_aggregating_person_on_events(self): "aggregation_group_type_index": 0, "retentionFilter": { "period": "Week", - "total_intervals": 7, + "totalIntervals": 7, }, }, actor="group", @@ -1865,7 +1865,7 @@ def test_groups_aggregating_person_on_events(self): "aggregation_group_type_index": 1, "retentionFilter": { "period": "Week", - "total_intervals": 7, + "totalIntervals": 7, }, } ) diff --git a/posthog/hogql_queries/legacy_compatibility/filter_to_query.py b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py index aea8dcaa6515b..25d4c7afb1b77 100644 --- a/posthog/hogql_queries/legacy_compatibility/filter_to_query.py +++ b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py @@ -358,13 +358,13 @@ def _insight_filter(filter: Dict): elif _insight_type(filter) == "RETENTION": insight_filter = { "retentionFilter": RetentionFilter( - retention_type=filter.get("retention_type"), - retention_reference=filter.get("retention_reference"), - total_intervals=filter.get("total_intervals"), - returning_entity=to_base_entity_dict(filter.get("returning_entity")) + retentionType=filter.get("retention_type"), + retentionReference=filter.get("retention_reference"), + totalIntervals=filter.get("total_intervals"), + returningEntity=to_base_entity_dict(filter.get("returning_entity")) if filter.get("returning_entity") is not None else None, - target_entity=to_base_entity_dict(filter.get("target_entity")) + targetEntity=to_base_entity_dict(filter.get("target_entity")) if filter.get("target_entity") is not None else None, period=filter.get("period"), diff --git a/posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py b/posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py index fad32d9d98205..d2df1af3fefb2 100644 --- a/posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py +++ b/posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py @@ -1398,17 +1398,17 @@ def test_retention_filter(self): self.assertEqual( query.retentionFilter, RetentionFilter( - retention_type=RetentionType.retention_first_time, - total_intervals=12, + retentionType=RetentionType.retention_first_time, + totalIntervals=12, period=RetentionPeriod.Week, - returning_entity={ + returningEntity={ "id": "$pageview", "name": "$pageview", "type": "events", "custom_name": None, "order": None, }, - target_entity={ + targetEntity={ "id": "$pageview", "name": "$pageview", "type": "events", diff --git a/posthog/schema.py b/posthog/schema.py index eb15e31980330..b71ed8cb06cb7 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -1168,6 +1168,18 @@ class QueryResponseAlternative11(BaseModel): class RetentionFilter(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + period: Optional[RetentionPeriod] = None + retentionReference: Optional[RetentionReference] = None + retentionType: Optional[RetentionType] = None + returningEntity: Optional[RetentionEntity] = None + targetEntity: Optional[RetentionEntity] = None + totalIntervals: Optional[int] = None + + +class RetentionFilterLegacy(BaseModel): model_config = ConfigDict( extra="forbid", )