From 44025f8342d9d2bf33c1a51474ebecfa34ddeba2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Oberm=C3=BCller?= Date: Mon, 9 Oct 2023 09:29:57 +0200 Subject: [PATCH] refactor(hogql): remove shown_as from queries (#17843) --- frontend/src/lib/constants.tsx | 2 +- frontend/src/queries/examples.ts | 4 ---- .../queries/nodes/InsightQuery/defaults.ts | 2 -- .../utils/filtersToQueryNode.test.ts | 6 ------ .../InsightQuery/utils/filtersToQueryNode.ts | 3 --- .../utils/queryNodeToFilter.test.ts | 14 +++++-------- .../InsightQuery/utils/queryNodeToFilter.ts | 1 - .../src/queries/nodes/InsightViz/utils.ts | 13 ------------ frontend/src/queries/schema.json | 13 ------------ frontend/src/queries/schema.ts | 6 +++--- .../scenes/insights/insightVizDataLogic.ts | 2 -- frontend/src/scenes/trends/trendsDataLogic.ts | 1 - .../scenes/trends/viz/ActionsLineGraph.tsx | 20 +++++++++---------- frontend/src/types.ts | 3 +++ .../legacy_compatibility/filter_to_query.py | 3 --- .../test/test_filter_to_query.py | 9 ++------- posthog/queries/trends/trends.py | 3 ++- posthog/schema.py | 9 --------- 18 files changed, 25 insertions(+), 89 deletions(-) diff --git a/frontend/src/lib/constants.tsx b/frontend/src/lib/constants.tsx index b750e3558b2fc..4089de5d9bb6d 100644 --- a/frontend/src/lib/constants.tsx +++ b/frontend/src/lib/constants.tsx @@ -98,7 +98,7 @@ export const ACTION_TYPE = 'action_type' export const EVENT_TYPE = 'event_type' export const STALE_EVENT_SECONDS = 30 * 24 * 60 * 60 // 30 days -// TODO: Deprecated; should be removed once backend is updated +/** @deprecated: should be removed once backend is updated */ export enum ShownAsValue { VOLUME = 'Volume', STICKINESS = 'Stickiness', diff --git a/frontend/src/queries/examples.ts b/frontend/src/queries/examples.ts index f14ff3f72a6b1..c6647cf8ca142 100644 --- a/frontend/src/queries/examples.ts +++ b/frontend/src/queries/examples.ts @@ -27,7 +27,6 @@ import { StepOrderValue, } from '~/types' import { defaultDataTableColumns } from '~/queries/nodes/DataTable/utils' -import { ShownAsValue } from '~/lib/constants' const Events: EventsQuery = { kind: NodeKind.EventsQuery, @@ -250,9 +249,6 @@ const InsightLifecycleQuery: LifecycleQuery = { date_from: '-7d', }, series, // TODO: Visualization only supports one event or action - lifecycleFilter: { - shown_as: ShownAsValue.LIFECYCLE, - }, } const TimeToSeeDataSessionsTable: DataTableNode = { diff --git a/frontend/src/queries/nodes/InsightQuery/defaults.ts b/frontend/src/queries/nodes/InsightQuery/defaults.ts index 876068329184f..8aced3682c278 100644 --- a/frontend/src/queries/nodes/InsightQuery/defaults.ts +++ b/frontend/src/queries/nodes/InsightQuery/defaults.ts @@ -10,7 +10,6 @@ import { TrendsQuery, } from '~/queries/schema' import { BaseMathType, FunnelVizType, InsightType, PathType, RetentionPeriod } from '~/types' -import { ShownAsValue } from 'lib/constants' const trendsQueryDefault: TrendsQuery = { kind: NodeKind.TrendsQuery, @@ -88,7 +87,6 @@ const lifecycleQueryDefault: LifecycleQuery = { math: BaseMathType.TotalCount, }, ], - lifecycleFilter: { shown_as: ShownAsValue.LIFECYCLE }, } export const nodeKindToDefaultQuery: Record = { diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts index 523179c5e4071..0cae5601db412 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.test.ts @@ -319,7 +319,6 @@ describe('filtersToQueryNode', () => { aggregation_axis_prefix: '£', aggregation_axis_postfix: '%', formula: 'A+B', - shown_as: ShownAsValue.VOLUME, display: ChartDisplayType.ActionsAreaGraph, }, breakdown: { @@ -486,7 +485,6 @@ describe('filtersToQueryNode', () => { compare: true, show_legend: true, hidden_legend_indexes: [0, 10], - shown_as: ShownAsValue.STICKINESS, display: ChartDisplayType.ActionsLineGraph, }, } @@ -507,7 +505,6 @@ describe('filtersToQueryNode', () => { const query: Partial = { kind: NodeKind.LifecycleQuery, lifecycleFilter: { - shown_as: ShownAsValue.LIFECYCLE, toggledLifecycles: ['new', 'dormant'], }, } @@ -643,9 +640,6 @@ describe('filtersToQueryNode', () => { }, ], interval: 'day', - lifecycleFilter: { - shown_as: ShownAsValue.LIFECYCLE, - }, } expect(result).toEqual(query) }) diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts index 5137fbf5b2116..f0462618aae0e 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts @@ -177,7 +177,6 @@ export const filtersToQueryNode = (filters: Partial): InsightQueryNo aggregation_axis_prefix: filters.aggregation_axis_prefix, aggregation_axis_postfix: filters.aggregation_axis_postfix, formula: filters.formula, - shown_as: filters.shown_as, display: filters.display, show_values_on_series: filters.show_values_on_series, show_percent_stack_view: filters.show_percent_stack_view, @@ -245,7 +244,6 @@ export const filtersToQueryNode = (filters: Partial): InsightQueryNo compare: filters.compare, show_legend: filters.show_legend, hidden_legend_indexes: cleanHiddenLegendIndexes(filters.hidden_legend_keys), - shown_as: filters.shown_as, show_values_on_series: filters.show_values_on_series, }) } @@ -253,7 +251,6 @@ export const filtersToQueryNode = (filters: Partial): InsightQueryNo // lifecycle filter if (isLifecycleFilter(filters) && isLifecycleQuery(query)) { query.lifecycleFilter = objectCleanWithEmpty({ - shown_as: filters.shown_as, toggledLifecycles: filters.toggledLifecycles, show_values_on_series: filters.show_values_on_series, }) diff --git a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.test.ts b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.test.ts index 7b60274cbb7b0..e6d9bb0eccdb5 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.test.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.test.ts @@ -1,21 +1,12 @@ import { hiddenLegendItemsToKeys, queryNodeToFilter } from '~/queries/nodes/InsightQuery/utils/queryNodeToFilter' import { InsightType, LifecycleFilterType } from '~/types' -import { ShownAsValue } from 'lib/constants' import { LifecycleQuery, NodeKind } from '~/queries/schema' describe('queryNodeToFilter', () => { test('converts a query node to a filter', () => { - const filters: Partial = { - entity_type: 'events', - insight: InsightType.LIFECYCLE, - shown_as: ShownAsValue.LIFECYCLE, - toggledLifecycles: ['new', 'dormant'], - } - const query: LifecycleQuery = { kind: NodeKind.LifecycleQuery, lifecycleFilter: { - shown_as: ShownAsValue.LIFECYCLE, toggledLifecycles: ['new', 'dormant'], }, series: [], @@ -23,6 +14,11 @@ describe('queryNodeToFilter', () => { const result = queryNodeToFilter(query) + const filters: Partial = { + entity_type: 'events', + insight: InsightType.LIFECYCLE, + toggledLifecycles: ['new', 'dormant'], + } expect(result).toEqual(filters) }) }) diff --git a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts index 0fa711f7a136b..53432d3c1009f 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts @@ -127,7 +127,6 @@ export const queryNodeToFilter = (query: InsightQueryNode): Partial if (isLifecycleQuery(query) && isLifecycleFilter(filters)) { filters.toggledLifecycles = query.lifecycleFilter?.toggledLifecycles - filters.shown_as = query.lifecycleFilter?.shown_as } // get node specific filter properties e.g. trendsFilter, funnelsFilter, ... diff --git a/frontend/src/queries/nodes/InsightViz/utils.ts b/frontend/src/queries/nodes/InsightViz/utils.ts index 46b551f5538b7..d9e169baacb1f 100644 --- a/frontend/src/queries/nodes/InsightViz/utils.ts +++ b/frontend/src/queries/nodes/InsightViz/utils.ts @@ -11,7 +11,6 @@ import { } from '~/queries/utils' import { filtersToQueryNode } from '../InsightQuery/utils/filtersToQueryNode' import equal from 'fast-deep-equal' -import { ShownAsValue } from 'lib/constants' export const getAllEventNames = (query: InsightQueryNode, allActions: ActionType[]): string[] => { const { actions, events } = seriesToActionsAndEvents((query as TrendsQuery).series || []) @@ -82,18 +81,6 @@ export const getBreakdown = (query: InsightQueryNode): BreakdownFilter | undefin } } -export const getShownAs = (query: InsightQueryNode): ShownAsValue | undefined => { - if (isLifecycleQuery(query)) { - return query.lifecycleFilter?.shown_as - } else if (isStickinessQuery(query)) { - return query.stickinessFilter?.shown_as - } else if (isTrendsQuery(query)) { - return query.trendsFilter?.shown_as - } else { - return undefined - } -} - export const getShowLegend = (query: InsightQueryNode): boolean | undefined => { if (isStickinessQuery(query)) { return query.stickinessFilter?.show_legend diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index c3fec7bc69a98..7be9f6aafca8e 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -1334,9 +1334,6 @@ "show_values_on_series": { "type": "boolean" }, - "shown_as": { - "$ref": "#/definitions/ShownAsValue" - }, "toggledLifecycles": { "items": { "$ref": "#/definitions/LifecycleToggle" @@ -2022,10 +2019,6 @@ "required": ["key", "operator", "type"], "type": "object" }, - "ShownAsValue": { - "enum": ["Volume", "Stickiness", "Lifecycle"], - "type": "string" - }, "StepOrderValue": { "enum": ["strict", "unordered", "ordered"], "type": "string" @@ -2051,9 +2044,6 @@ }, "show_values_on_series": { "type": "boolean" - }, - "shown_as": { - "$ref": "#/definitions/ShownAsValue" } }, "type": "object" @@ -2197,9 +2187,6 @@ "show_values_on_series": { "type": "boolean" }, - "shown_as": { - "$ref": "#/definitions/ShownAsValue" - }, "smoothing_intervals": { "type": "number" } diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index c671ff8183a4b..43564dbb10c02 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -398,7 +398,7 @@ export interface InsightsQueryBase extends Node { * `hidden_legend_keys` replaced by `hidden_legend_indexes` */ export type TrendsFilter = Omit< TrendsFilterType & { hidden_legend_indexes?: number[] }, - keyof FilterType | 'hidden_legend_keys' + keyof FilterType | 'hidden_legend_keys' | 'shown_as' > export interface TrendsQueryResponse extends QueryResponse { @@ -467,7 +467,7 @@ export interface PathsQuery extends InsightsQueryBase { * and `hidden_legend_keys` replaced by `hidden_legend_indexes` */ export type StickinessFilter = Omit< StickinessFilterType & { hidden_legend_indexes?: number[] }, - keyof FilterType | 'hidden_legend_keys' | 'stickiness_days' + keyof FilterType | 'hidden_legend_keys' | 'stickiness_days' | 'shown_as' > export interface StickinessQuery extends InsightsQueryBase { kind: NodeKind.StickinessQuery @@ -480,7 +480,7 @@ export interface StickinessQuery extends InsightsQueryBase { } /** `LifecycleFilterType` minus everything inherited from `FilterType` */ -export type LifecycleFilter = Omit & { +export type LifecycleFilter = Omit & { /** Lifecycles that have been removed from display are not included in this array */ toggledLifecycles?: LifecycleToggle[] } // using everything except what it inherits from FilterType diff --git a/frontend/src/scenes/insights/insightVizDataLogic.ts b/frontend/src/scenes/insights/insightVizDataLogic.ts index a5543905fe49a..9d451f8d12f4d 100644 --- a/frontend/src/scenes/insights/insightVizDataLogic.ts +++ b/frontend/src/scenes/insights/insightVizDataLogic.ts @@ -36,7 +36,6 @@ import { getFormula, getInterval, getSeries, - getShownAs, getShowLegend, getShowPercentStackView, getShowValueOnSeries, @@ -122,7 +121,6 @@ export const insightVizDataLogic = kea([ interval: [(s) => [s.querySource], (q) => (q ? getInterval(q) : null)], properties: [(s) => [s.querySource], (q) => (q ? q.properties : null)], samplingFactor: [(s) => [s.querySource], (q) => (q ? q.samplingFactor : null)], - shownAs: [(s) => [s.querySource], (q) => (q ? getShownAs(q) : null)], showLegend: [(s) => [s.querySource], (q) => (q ? getShowLegend(q) : null)], showValueOnSeries: [(s) => [s.querySource], (q) => (q ? getShowValueOnSeries(q) : null)], showPercentStackView: [(s) => [s.querySource], (q) => (q ? getShowPercentStackView(q) : null)], diff --git a/frontend/src/scenes/trends/trendsDataLogic.ts b/frontend/src/scenes/trends/trendsDataLogic.ts index 4c34e5296dcad..06e95485b8e80 100644 --- a/frontend/src/scenes/trends/trendsDataLogic.ts +++ b/frontend/src/scenes/trends/trendsDataLogic.ts @@ -25,7 +25,6 @@ export const trendsDataLogic = kea([ 'compare', 'interval', 'breakdown', - 'shownAs', 'showValueOnSeries', 'showPercentStackView', 'supportsPercentStackView', diff --git a/frontend/src/scenes/trends/viz/ActionsLineGraph.tsx b/frontend/src/scenes/trends/viz/ActionsLineGraph.tsx index c5e99ab164b11..4da7d2cbad9c6 100644 --- a/frontend/src/scenes/trends/viz/ActionsLineGraph.tsx +++ b/frontend/src/scenes/trends/viz/ActionsLineGraph.tsx @@ -24,7 +24,6 @@ export function ActionsLineGraph({ compare, display, interval, - shownAs, showValueOnSeries, showPercentStackView, supportsPercentStackView, @@ -84,19 +83,18 @@ export function ActionsLineGraph({ const urls = urlsForDatasets(crossDataset, index) if (urls?.length) { - const title = - shownAs === 'Stickiness' ? ( + const title = isStickiness ? ( + <> + stickiness on day {day} + + ) : ( + (label: string) => ( <> - stickiness on day {day} + {label} on{' '} + - ) : ( - (label: string) => ( - <> - {label} on{' '} - - - ) ) + ) openPersonsModal({ urls, diff --git a/frontend/src/types.ts b/frontend/src/types.ts index fef4d5ead7c64..0ca801012d6ec 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -1689,6 +1689,7 @@ export interface TrendsFilterType extends FilterType { smoothing_intervals?: number compare?: boolean formula?: string + /** @deprecated */ shown_as?: ShownAsValue display?: ChartDisplayType breakdown_histogram_bin_count?: number // trends breakdown histogram bin count @@ -1705,6 +1706,7 @@ export interface TrendsFilterType extends FilterType { export interface StickinessFilterType extends FilterType { compare?: boolean + /** @deprecated */ shown_as?: ShownAsValue display?: ChartDisplayType @@ -1775,6 +1777,7 @@ export interface RetentionFilterType extends FilterType { period?: RetentionPeriod } export interface LifecycleFilterType extends FilterType { + /** @deprecated */ shown_as?: ShownAsValue // frontend only diff --git a/posthog/hogql_queries/legacy_compatibility/filter_to_query.py b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py index 49ed19f24fd09..f8941c3899125 100644 --- a/posthog/hogql_queries/legacy_compatibility/filter_to_query.py +++ b/posthog/hogql_queries/legacy_compatibility/filter_to_query.py @@ -309,7 +309,6 @@ def _insight_filter(filter: Dict): aggregation_axis_prefix=filter.get("aggregation_axis_prefix"), aggregation_axis_postfix=filter.get("aggregation_axis_postfix"), formula=filter.get("formula"), - shown_as=filter.get("shown_as"), display=clean_display(filter.get("display")), show_values_on_series=filter.get("show_values_on_series"), show_percent_stack_view=filter.get("show_percent_stack_view"), @@ -379,7 +378,6 @@ def _insight_filter(filter: Dict): elif _insight_type(filter) == "LIFECYCLE": insight_filter = { "lifecycleFilter": LifecycleFilter( - shown_as=filter.get("shown_as"), # toggledLifecycles=filter.get('toggledLifecycles'), show_values_on_series=filter.get("show_values_on_series"), ) @@ -388,7 +386,6 @@ def _insight_filter(filter: Dict): insight_filter = { "stickinessFilter": StickinessFilter( compare=filter.get("compare"), - shown_as=filter.get("shown_as"), # show_legend=filter.get('show_legend'), # hidden_legend_indexes: cleanHiddenLegendIndexes(filter.get('hidden_legend_keys')), show_values_on_series=filter.get("show_values_on_series"), 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 bad08471ea241..f07405b248976 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 @@ -29,14 +29,12 @@ RetentionPeriod, RetentionType, SessionPropertyFilter, - ShownAsValue, StepOrderValue, TrendsFilter, FunnelsFilter, RetentionFilter, PathsFilter, StickinessFilter, - LifecycleFilter, ) from posthog.test.base import BaseTest @@ -994,7 +992,6 @@ def test_trends_filter(self): aggregation_axis_prefix="pre", aggregation_axis_postfix="post", formula="A + B", - shown_as=ShownAsValue.Volume, display=ChartDisplayType.ActionsAreaGraph, ), ) @@ -1173,7 +1170,7 @@ def test_stickiness_filter(self): self.assertEqual( query.stickinessFilter, - StickinessFilter(compare=True, shown_as=ShownAsValue.Stickiness), + StickinessFilter(compare=True), ) def test_lifecycle_filter(self): @@ -1186,7 +1183,5 @@ def test_lifecycle_filter(self): self.assertEqual( query.lifecycleFilter, - LifecycleFilter( - shown_as=ShownAsValue.Lifecycle, - ), + None, ) diff --git a/posthog/queries/trends/trends.py b/posthog/queries/trends/trends.py index 940abba59fab5..049417799bb8b 100644 --- a/posthog/queries/trends/trends.py +++ b/posthog/queries/trends/trends.py @@ -11,6 +11,7 @@ from posthog.clickhouse.query_tagging import get_query_tags, tag_queries from posthog.constants import ( + INSIGHT_LIFECYCLE, NON_BREAKDOWN_DISPLAY_TYPES, TREND_FILTER_TYPE_ACTIONS, TRENDS_CUMULATIVE, @@ -38,7 +39,7 @@ def _get_sql_for_entity(self, filter: Filter, team: Team, entity: Entity) -> Tup sql, params, parse_function = TrendsBreakdown( entity, filter, team, person_on_events_mode=team.person_on_events_mode ).get_query() - elif filter.shown_as == TRENDS_LIFECYCLE: + elif filter.insight == INSIGHT_LIFECYCLE or filter.shown_as == TRENDS_LIFECYCLE: query_type = "trends_lifecycle" sql, params, parse_function = self._format_lifecycle_query(entity, filter, team) else: diff --git a/posthog/schema.py b/posthog/schema.py index 705267fdf9cdc..906feaa2af7d0 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -407,12 +407,6 @@ class SessionPropertyFilter(BaseModel): value: Optional[Union[str, float, List[Union[str, float]]]] = None -class ShownAsValue(str, Enum): - Volume = "Volume" - Stickiness = "Stickiness" - Lifecycle = "Lifecycle" - - class StepOrderValue(str, Enum): strict = "strict" unordered = "unordered" @@ -428,7 +422,6 @@ class StickinessFilter(BaseModel): hidden_legend_indexes: Optional[List[float]] = None show_legend: Optional[bool] = None show_values_on_series: Optional[bool] = None - shown_as: Optional[ShownAsValue] = None class TimeToSeeDataSessionsQueryResponse(BaseModel): @@ -453,7 +446,6 @@ class TrendsFilter(BaseModel): show_legend: Optional[bool] = None show_percent_stack_view: Optional[bool] = None show_values_on_series: Optional[bool] = None - shown_as: Optional[ShownAsValue] = None smoothing_intervals: Optional[float] = None @@ -658,7 +650,6 @@ class LifecycleFilter(BaseModel): extra="forbid", ) show_values_on_series: Optional[bool] = None - shown_as: Optional[ShownAsValue] = None toggledLifecycles: Optional[List[LifecycleToggle]] = None