From 67c6f68c6511ebee7b5bbbcf39daa9ae563551cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Obermu=CC=88ller?= Date: Wed, 17 Jan 2024 00:25:59 +0100 Subject: [PATCH] camel case retention filter --- .../queries/nodes/InsightQuery/defaults.ts | 8 ++--- .../utils/filtersToQueryNode.test.ts | 18 +++++------ .../InsightQuery/utils/filtersToQueryNode.ts | 10 +++---- .../InsightQuery/utils/queryNodeToFilter.ts | 12 ++++++++ frontend/src/queries/schema.json | 25 ++++++++++++++++ frontend/src/queries/schema.ts | 11 ++++++- .../EditorFilters/RetentionSummary.tsx | 30 +++++++++---------- .../scenes/insights/summarizeInsight.test.ts | 12 ++++---- .../src/scenes/insights/summarizeInsight.ts | 10 +++---- .../notebooks/Notebook/SlashCommands.tsx | 8 ++--- .../scenes/web-analytics/webAnalyticsLogic.ts | 6 ++-- .../legacy_compatibility/filter_to_query.py | 10 +++---- posthog/schema.py | 12 ++++++++ 13 files changed, 115 insertions(+), 57 deletions(-) diff --git a/frontend/src/queries/nodes/InsightQuery/defaults.ts b/frontend/src/queries/nodes/InsightQuery/defaults.ts index 369362dfc517a3..c8742c3f35b583 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 f9d73ca5c64e60..2ea1df58c7ba27 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, }, } @@ -627,15 +627,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 73afb72e1e3a41..355bba30ee8d83 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts @@ -289,11 +289,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 9f41bcb7eefd2e..450bba98b097f9 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts @@ -16,6 +16,7 @@ import { LifecycleFilterLegacy, NodeKind, PathsFilterLegacy, + RetentionFilterLegacy, StickinessFilterLegacy, TrendsFilterLegacy, } from '~/queries/schema' @@ -214,6 +215,17 @@ export const queryNodeToFilter = (query: InsightQueryNode): Partial delete insightFilter.maxEdgeWeight delete insightFilter.funnelPaths delete insightFilter.funnelFilter + } else if (isRetentionQuery(query)) { + ;(legacyProps as RetentionFilterLegacy).retention_reference = insightFilter.retentionReference + ;(legacyProps as RetentionFilterLegacy).retention_type = insightFilter.retentionType + ;(legacyProps as RetentionFilterLegacy).returning_entity = insightFilter.returningEntity + ;(legacyProps as RetentionFilterLegacy).target_entity = insightFilter.targetEntity + ;(legacyProps as RetentionFilterLegacy).total_intervals = insightFilter.totalIntervals + delete insightFilter.retentionReference + delete insightFilter.retentionType + delete insightFilter.returningEntity + delete insightFilter.targetEntity + delete insightFilter.totalIntervals } Object.assign(filters, insightFilter) Object.assign(filters, legacyProps) diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index 9e8f64a8e12893..12f3dcd6189f8d 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 579b4def7cfc8f..4f7e73f4301d9c 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -569,7 +569,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 8859b225ef4199..51ec482717b1be 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/summarizeInsight.test.ts b/frontend/src/scenes/insights/summarizeInsight.test.ts index 68c5330a1f97c1..ad8d8cf5482418 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 2b6e28644a2214..d99cbcf763ba6e 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 647645e6af1b6b..3791046e658d5a 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/web-analytics/webAnalyticsLogic.ts b/frontend/src/scenes/web-analytics/webAnalyticsLogic.ts index 98df4dd8c5fc0b..ec0c11f7bdc814 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/legacy_compatibility/filter_to_query.py b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py index aea8dcaa6515be..25d4c7afb1b779 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/schema.py b/posthog/schema.py index 6c463e0a4ae24f..67c8059baf6151 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -1167,6 +1167,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", )