From 8d6ebe76664d92f930f7823d8d71f205d836b2f6 Mon Sep 17 00:00:00 2001 From: Robbie Date: Tue, 6 Feb 2024 16:04:48 +0000 Subject: [PATCH] feat(web-analytics): Add limit and hasMore to stats_table (#20120) * Add limit and hasMore to stats_table * Add test * Use paginator helper class * Fix * Add asType * Update posthog/hogql_queries/web_analytics/stats_table.py Co-authored-by: Julian Bez * Add limit and offset * Formatting * Set limit in modal * Add more asType --------- Co-authored-by: Julian Bez --- frontend/src/queries/schema.json | 21 ++++++++++ frontend/src/queries/schema.ts | 7 ++++ .../scenes/web-analytics/webAnalyticsLogic.ts | 15 ++++++- .../web_analytics/stats_table.py | 40 +++++++++++-------- .../test/test_web_stats_table.py | 34 +++++++++++++++- posthog/schema.py | 24 +++++++++++ 6 files changed, 122 insertions(+), 19 deletions(-) diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index 3f541bb4d9788..56f8d86a3ac85 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -3487,6 +3487,9 @@ "items": {}, "type": "array" }, + "hasMore": { + "type": "boolean" + }, "hogql": { "type": "string" }, @@ -3496,9 +3499,15 @@ "last_refresh": { "type": "string" }, + "limit": { + "type": "integer" + }, "next_allowed_client_refresh": { "type": "string" }, + "offset": { + "type": "integer" + }, "results": { "items": {}, "type": "array" @@ -4939,6 +4948,9 @@ "const": "WebStatsTableQuery", "type": "string" }, + "limit": { + "type": "integer" + }, "properties": { "$ref": "#/definitions/WebAnalyticsPropertyFilters" }, @@ -4968,6 +4980,9 @@ "items": {}, "type": "array" }, + "hasMore": { + "type": "boolean" + }, "hogql": { "type": "string" }, @@ -4977,9 +4992,15 @@ "last_refresh": { "type": "string" }, + "limit": { + "type": "integer" + }, "next_allowed_client_refresh": { "type": "string" }, + "offset": { + "type": "integer" + }, "results": { "items": {}, "type": "array" diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index 1907b59e769dd..e2be8786be27d 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -935,6 +935,8 @@ export interface WebStatsTableQuery extends WebAnalyticsQueryBase { response?: WebStatsTableQueryResponse includeScrollDepth?: boolean // automatically sets includeBounceRate to true includeBounceRate?: boolean + /** @asType integer */ + limit?: number } export interface WebStatsTableQueryResponse extends QueryResponse { results: unknown[] @@ -942,6 +944,11 @@ export interface WebStatsTableQueryResponse extends QueryResponse { columns?: unknown[] hogql?: string samplingRate?: SamplingRate + hasMore?: boolean + /** @asType integer */ + limit?: number + /** @asType integer */ + offset?: number } export type InsightQueryNode = diff --git a/frontend/src/scenes/web-analytics/webAnalyticsLogic.ts b/frontend/src/scenes/web-analytics/webAnalyticsLogic.ts index 76624758725db..a2f55dfbd91bc 100644 --- a/frontend/src/scenes/web-analytics/webAnalyticsLogic.ts +++ b/frontend/src/scenes/web-analytics/webAnalyticsLogic.ts @@ -552,6 +552,7 @@ export const webAnalyticsLogic = kea([ includeScrollDepth: statusCheck?.isSendingPageLeavesScroll, includeBounceRate: true, sampling, + limit: 10, }, embedded: false, }, @@ -572,6 +573,7 @@ export const webAnalyticsLogic = kea([ dateRange, includeScrollDepth: statusCheck?.isSendingPageLeavesScroll, sampling, + limit: 10, }, embedded: false, }, @@ -602,6 +604,7 @@ export const webAnalyticsLogic = kea([ breakdownBy: WebStatsBreakdown.InitialReferringDomain, dateRange, sampling, + limit: 10, }, }, insightProps: createInsightProps(TileId.SOURCES, SourceTab.REFERRING_DOMAIN), @@ -620,6 +623,7 @@ export const webAnalyticsLogic = kea([ breakdownBy: WebStatsBreakdown.InitialChannelType, dateRange, sampling, + limit: 10, }, }, insightProps: createInsightProps(TileId.SOURCES, SourceTab.CHANNEL), @@ -638,6 +642,7 @@ export const webAnalyticsLogic = kea([ breakdownBy: WebStatsBreakdown.InitialUTMSource, dateRange, sampling, + limit: 10, }, }, insightProps: createInsightProps(TileId.SOURCES, SourceTab.UTM_SOURCE), @@ -656,6 +661,7 @@ export const webAnalyticsLogic = kea([ breakdownBy: WebStatsBreakdown.InitialUTMMedium, dateRange, sampling, + limit: 10, }, }, insightProps: createInsightProps(TileId.SOURCES, SourceTab.UTM_MEDIUM), @@ -674,6 +680,7 @@ export const webAnalyticsLogic = kea([ breakdownBy: WebStatsBreakdown.InitialUTMCampaign, dateRange, sampling, + limit: 10, }, }, insightProps: createInsightProps(TileId.SOURCES, SourceTab.UTM_CAMPAIGN), @@ -692,6 +699,7 @@ export const webAnalyticsLogic = kea([ breakdownBy: WebStatsBreakdown.InitialUTMContent, dateRange, sampling, + limit: 10, }, }, insightProps: createInsightProps(TileId.SOURCES, SourceTab.UTM_CONTENT), @@ -710,6 +718,7 @@ export const webAnalyticsLogic = kea([ breakdownBy: WebStatsBreakdown.InitialUTMTerm, dateRange, sampling, + limit: 10, }, }, insightProps: createInsightProps(TileId.SOURCES, SourceTab.UTM_TERM), @@ -794,6 +803,7 @@ export const webAnalyticsLogic = kea([ breakdownBy: WebStatsBreakdown.OS, dateRange, sampling, + limit: 10, }, embedded: false, }, @@ -857,6 +867,7 @@ export const webAnalyticsLogic = kea([ breakdownBy: WebStatsBreakdown.Country, dateRange, sampling, + limit: 10, }, }, insightProps: createInsightProps(TileId.GEOGRAPHY, GeographyTab.COUNTRIES), @@ -875,6 +886,7 @@ export const webAnalyticsLogic = kea([ breakdownBy: WebStatsBreakdown.Region, dateRange, sampling, + limit: 10, }, }, insightProps: createInsightProps(TileId.GEOGRAPHY, GeographyTab.REGIONS), @@ -893,6 +905,7 @@ export const webAnalyticsLogic = kea([ breakdownBy: WebStatsBreakdown.City, dateRange, sampling, + limit: 10, }, }, insightProps: createInsightProps(TileId.GEOGRAPHY, GeographyTab.CITIES), @@ -956,7 +969,7 @@ export const webAnalyticsLogic = kea([ ...query, source: { ...query.source, - // limit: 50, // wait for https://github.com/PostHog/posthog/pull/20120 + limit: 50, }, } } else { diff --git a/posthog/hogql_queries/web_analytics/stats_table.py b/posthog/hogql_queries/web_analytics/stats_table.py index 95d11bb54161c..bede9accc6d6d 100644 --- a/posthog/hogql_queries/web_analytics/stats_table.py +++ b/posthog/hogql_queries/web_analytics/stats_table.py @@ -1,6 +1,7 @@ from posthog.hogql import ast +from posthog.hogql.constants import LimitContext from posthog.hogql.parser import parse_select, parse_expr -from posthog.hogql.query import execute_hogql_query +from posthog.hogql_queries.insights.paginators import HogQLHasMorePaginator from posthog.hogql_queries.web_analytics.ctes import ( COUNTS_CTE, BOUNCE_RATE_CTE, @@ -19,6 +20,13 @@ class WebStatsTableQueryRunner(WebAnalyticsQueryRunner): query: WebStatsTableQuery query_type = WebStatsTableQuery + paginator: HogQLHasMorePaginator + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.paginator = HogQLHasMorePaginator.from_limit_context( + limit_context=LimitContext.QUERY, limit=self.query.limit if self.query.limit else None + ) def _bounce_rate_subquery(self): with self.timings.measure("bounce_rate_query"): @@ -57,13 +65,12 @@ def _scroll_depth_subquery(self): }, ) - def to_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: + def to_query(self) -> ast.SelectQuery: # special case for channel, as some hogql features to use the general code are still being worked on if self.query.breakdownBy == WebStatsBreakdown.InitialChannelType: - return self.to_channel_query() - - if self.query.includeScrollDepth: - return parse_select( + query = self.to_channel_query() + elif self.query.includeScrollDepth: + query = parse_select( """ SELECT counts.breakdown_value as "context.columns.breakdown_value", @@ -87,7 +94,6 @@ def to_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: ORDER BY "context.columns.views" DESC, "context.columns.breakdown_value" DESC -LIMIT 10 """, timings=self.timings, placeholders={ @@ -100,7 +106,7 @@ def to_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: ) elif self.query.includeBounceRate: with self.timings.measure("stats_table_query"): - return parse_select( + query = parse_select( """ SELECT counts.breakdown_value as "context.columns.breakdown_value", @@ -118,7 +124,6 @@ def to_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: ORDER BY "context.columns.views" DESC, "context.columns.breakdown_value" DESC - LIMIT 10 """, timings=self.timings, placeholders={ @@ -129,7 +134,7 @@ def to_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: ) else: with self.timings.measure("stats_table_query"): - return parse_select( + query = parse_select( """ SELECT counts.breakdown_value as "context.columns.breakdown_value", @@ -142,7 +147,6 @@ def to_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: ORDER BY "context.columns.views" DESC, "context.columns.breakdown_value" DESC - LIMIT 10 """, timings=self.timings, placeholders={ @@ -150,16 +154,20 @@ def to_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: "where_breakdown": self.where_breakdown(), }, ) + assert isinstance(query, ast.SelectQuery) + return query def calculate(self): - response = execute_hogql_query( + response = self.paginator.execute_hogql_query( query_type="top_sources_query", query=self.to_query(), team=self.team, timings=self.timings, modifiers=self.modifiers, ) - assert response.results is not None + results = self.paginator.results + + assert results is not None def to_data(col_val, col_idx): if col_idx == 0: # breakdown_value @@ -173,14 +181,15 @@ def to_data(col_val, col_idx): else: return col_val - results = [[to_data(c, i) for (i, c) in enumerate(r)] for r in response.results] + results_mapped = [[to_data(c, i) for (i, c) in enumerate(r)] for r in results] return WebStatsTableQueryResponse( columns=response.columns, - results=results, + results=results_mapped, timings=response.timings, types=response.types, hogql=response.hogql, + **self.paginator.response_params(), ) def counts_breakdown(self): @@ -344,7 +353,6 @@ def to_channel_query(self): ORDER BY "context.columns.views" DESC, "context.columns.breakdown_value" DESC -LIMIT 10 """, timings=self.timings, backend="cpp", diff --git a/posthog/hogql_queries/web_analytics/test/test_web_stats_table.py b/posthog/hogql_queries/web_analytics/test/test_web_stats_table.py index e0d01674ae4c9..041121b4055a6 100644 --- a/posthog/hogql_queries/web_analytics/test/test_web_stats_table.py +++ b/posthog/hogql_queries/web_analytics/test/test_web_stats_table.py @@ -35,9 +35,12 @@ def _create_events(self, data, event="$pageview"): ) return person_result - def _run_web_stats_table_query(self, date_from, date_to, breakdown_by=WebStatsBreakdown.Page): + def _run_web_stats_table_query(self, date_from, date_to, breakdown_by=WebStatsBreakdown.Page, limit=None): query = WebStatsTableQuery( - dateRange=DateRange(date_from=date_from, date_to=date_to), properties=[], breakdownBy=breakdown_by + dateRange=DateRange(date_from=date_from, date_to=date_to), + properties=[], + breakdownBy=breakdown_by, + limit=limit, ) runner = WebStatsTableQueryRunner(team=self.team, query=query) return runner.calculate() @@ -111,3 +114,30 @@ def test_breakdown_channel_type_doesnt_throw(self): 1, len(results), ) + + def test_limit(self): + self._create_events( + [ + ("p1", [("2023-12-02", "s1", "/"), ("2023-12-03", "s1", "/login")]), + ("p2", [("2023-12-10", "s2", "/")]), + ] + ) + + response_1 = self._run_web_stats_table_query("all", "2023-12-15", limit=1) + self.assertEqual( + [ + ["/", 2, 2], + ], + response_1.results, + ) + self.assertEqual(True, response_1.hasMore) + + response_2 = self._run_web_stats_table_query("all", "2023-12-15", limit=2) + self.assertEqual( + [ + ["/", 2, 2], + ["/login", 1, 1], + ], + response_2.results, + ) + self.assertEqual(False, response_2.hasMore) diff --git a/posthog/schema.py b/posthog/schema.py index 5e63ca43359e3..860b6060615c0 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -915,10 +915,13 @@ class WebStatsTableQueryResponse(BaseModel): extra="forbid", ) columns: Optional[List] = None + hasMore: Optional[bool] = None hogql: Optional[str] = None is_cached: Optional[bool] = None last_refresh: Optional[str] = None + limit: Optional[int] = None next_allowed_client_refresh: Optional[str] = None + offset: Optional[int] = None results: List samplingRate: Optional[SamplingRate] = None timings: Optional[List[QueryTiming]] = None @@ -1292,6 +1295,24 @@ class QueryResponseAlternative9(BaseModel): class QueryResponseAlternative10(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + columns: Optional[List] = None + hasMore: Optional[bool] = None + hogql: Optional[str] = None + is_cached: Optional[bool] = None + last_refresh: Optional[str] = None + limit: Optional[int] = None + next_allowed_client_refresh: Optional[str] = None + offset: Optional[int] = None + results: List + samplingRate: Optional[SamplingRate] = None + timings: Optional[List[QueryTiming]] = None + types: Optional[List] = None + + +class QueryResponseAlternative11(BaseModel): model_config = ConfigDict( extra="forbid", ) @@ -1464,6 +1485,7 @@ class WebStatsTableQuery(BaseModel): includeBounceRate: Optional[bool] = None includeScrollDepth: Optional[bool] = None kind: Literal["WebStatsTableQuery"] = "WebStatsTableQuery" + limit: Optional[int] = None properties: List[Union[EventPropertyFilter, PersonPropertyFilter]] response: Optional[WebStatsTableQueryResponse] = None sampling: Optional[Sampling] = None @@ -1930,6 +1952,7 @@ class QueryResponseAlternative( QueryResponseAlternative8, QueryResponseAlternative9, QueryResponseAlternative10, + QueryResponseAlternative11, QueryResponseAlternative12, QueryResponseAlternative13, Dict[str, List[DatabaseSchemaQueryResponseField]], @@ -1948,6 +1971,7 @@ class QueryResponseAlternative( QueryResponseAlternative8, QueryResponseAlternative9, QueryResponseAlternative10, + QueryResponseAlternative11, QueryResponseAlternative12, QueryResponseAlternative13, Dict[str, List[DatabaseSchemaQueryResponseField]],