From 3f9638b15dc1f97af6978929d672694387a0fa6e Mon Sep 17 00:00:00 2001 From: Marius Andra Date: Wed, 18 Oct 2023 12:20:58 +0200 Subject: [PATCH] feat(insights): edit as sql (#18055) --- .../src/scenes/insights/InsightPageHeader.tsx | 141 ++++++++++-------- .../src/scenes/insights/insightDataLogic.ts | 10 ++ posthog/hogql/printer.py | 9 +- posthog/hogql/test/test_printer.py | 7 +- .../insights/lifecycle_query_runner.py | 8 +- 5 files changed, 112 insertions(+), 63 deletions(-) diff --git a/frontend/src/scenes/insights/InsightPageHeader.tsx b/frontend/src/scenes/insights/InsightPageHeader.tsx index ca62e7609fc289..8681d1f14b099a 100644 --- a/frontend/src/scenes/insights/InsightPageHeader.tsx +++ b/frontend/src/scenes/insights/InsightPageHeader.tsx @@ -42,7 +42,7 @@ import { AddToDashboardModal } from 'lib/components/AddToDashboard/AddToDashboar import { useState } from 'react' import { NewDashboardModal } from 'scenes/dashboard/NewDashboardModal' import { NotebookSelectButton } from 'scenes/notebooks/NotebookSelectButton/NotebookSelectButton' -import { NodeKind } from '~/queries/schema' +import { DataTableNode, NodeKind } from '~/queries/schema' export function InsightPageHeader({ insightLogicProps }: { insightLogicProps: InsightLogicProps }): JSX.Element { // insightSceneLogic @@ -67,7 +67,7 @@ export function InsightPageHeader({ insightLogicProps }: { insightLogicProps: In const { duplicateInsight, loadInsights } = useActions(savedInsightsLogic) // insightDataLogic - const { query, queryChanged, showQueryEditor } = useValues(insightDataLogic(insightProps)) + const { query, queryChanged, showQueryEditor, hogQL } = useValues(insightDataLogic(insightProps)) const { saveInsight: saveQueryBasedInsight, toggleQueryEditorPanel } = useActions(insightDataLogic(insightProps)) // other logics @@ -137,10 +137,10 @@ export function InsightPageHeader({ insightLogicProps }: { insightLogicProps: In } buttons={
- {hasDashboardItemId && ( - <> - + {hasDashboardItemId && ( <> Share or embed - {insight.short_id && ( - <> - - {exporterResourceParams ? ( - - ) : null} - {isInsightVizNode(query) ? ( - { - // for an existing insight in view mode - if ( - hasDashboardItemId && - insightMode !== ItemMode.Edit - ) { - // enter edit mode - setInsightMode(ItemMode.Edit, null) - - // exit early if query editor doesn't need to be toggled - if (showQueryEditor !== false) { - return - } - } - toggleQueryEditorPanel() - }} - fullWidth - > - {showQueryEditor ? 'Hide source' : 'View source'} - - ) : null} - - - )} + + )} + {insight.short_id && ( + <> + + {exporterResourceParams ? ( + + ) : null} + + )} + {isInsightVizNode(query) ? ( + { + // for an existing insight in view mode + if (hasDashboardItemId && insightMode !== ItemMode.Edit) { + // enter edit mode + setInsightMode(ItemMode.Edit, null) + // exit early if query editor doesn't need to be toggled + if (showQueryEditor !== false) { + return + } + } + toggleQueryEditorPanel() + }} + fullWidth + > + {showQueryEditor ? 'Hide source' : 'View source'} + + ) : null} + {hogQL && ( + { + router.actions.push( + urls.insightNew( + undefined, + undefined, + JSON.stringify({ + kind: NodeKind.DataTableNode, + source: { + kind: NodeKind.HogQLQuery, + query: hogQL, + }, + full: true, + } as DataTableNode) + ) + ) + }} + fullWidth + > + Edit SQL directly + + )} + {hasDashboardItemId && ( + <> + @@ -247,11 +270,11 @@ export function InsightPageHeader({ insightLogicProps }: { insightLogicProps: In Delete insight - } - /> - - - )} + )} + + } + /> + {insightMode === ItemMode.Edit && hasDashboardItemId && ( setInsightMode(ItemMode.View, null)}> diff --git a/frontend/src/scenes/insights/insightDataLogic.ts b/frontend/src/scenes/insights/insightDataLogic.ts index 02ebede5c40dd5..e2ac23f175ed2e 100644 --- a/frontend/src/scenes/insights/insightDataLogic.ts +++ b/frontend/src/scenes/insights/insightDataLogic.ts @@ -155,6 +155,16 @@ export const insightDataLogic = kea([ return { ...insightDataRaw, result: insightDataRaw?.results ?? insightDataRaw?.result } }, ], + + hogQL: [ + (s) => [s.insightData], + (insightData): string | null => { + if (insightData && 'hogql' in insightData && insightData.hogql !== '') { + return insightData.hogql + } + return null + }, + ], }), listeners(({ actions, values }) => ({ diff --git a/posthog/hogql/printer.py b/posthog/hogql/printer.py index 135e13e6f7346a..69fe54f6ad9137 100644 --- a/posthog/hogql/printer.py +++ b/posthog/hogql/printer.py @@ -33,7 +33,7 @@ from posthog.hogql.resolver import ResolverException, lookup_field_by_name, resolve_types from posthog.hogql.transforms.lazy_tables import resolve_lazy_tables from posthog.hogql.transforms.property_types import resolve_property_types -from posthog.hogql.visitor import Visitor +from posthog.hogql.visitor import Visitor, clone_expr from posthog.models.property import PropertyName, TableColumn from posthog.models.team.team import WeekStartDay from posthog.models.utils import UUIDT @@ -53,6 +53,13 @@ def team_id_guard_for_table(table_type: Union[ast.TableType, ast.TableAliasType] ) +def to_printed_hogql(query: ast.Expr, team_id: int) -> str: + """Prints the HogQL query without mutating the node""" + return print_ast( + clone_expr(query), dialect="hogql", context=HogQLContext(team_id=team_id, enable_select_queries=True) + ) + + def print_ast( node: ast.Expr, context: HogQLContext, diff --git a/posthog/hogql/test/test_printer.py b/posthog/hogql/test/test_printer.py index ff21d40b1a75cf..59c2c80a8ac40e 100644 --- a/posthog/hogql/test/test_printer.py +++ b/posthog/hogql/test/test_printer.py @@ -10,7 +10,7 @@ from posthog.hogql.errors import HogQLException from posthog.hogql.hogql import translate_hogql from posthog.hogql.parser import parse_select -from posthog.hogql.printer import print_ast +from posthog.hogql.printer import print_ast, to_printed_hogql from posthog.models.team.team import WeekStartDay from posthog.schema import HogQLQueryModifiers, PersonsArgMaxVersion from posthog.test.base import BaseTest @@ -50,6 +50,11 @@ def _assert_select_error(self, statement, expected_error): raise AssertionError(f"Expected '{expected_error}' in '{str(context.exception)}'") self.assertTrue(expected_error in str(context.exception)) + def test_to_printed_hogql(self): + expr = parse_select("select 1 + 2, 3 from events") + repsponse = to_printed_hogql(expr, self.team.pk) + self.assertEqual(repsponse, "SELECT plus(1, 2), 3 FROM events LIMIT 10000") + def test_literals(self): self.assertEqual(self._expr("1 + 2"), "plus(1, 2)") self.assertEqual(self._expr("-1 + 2"), "plus(-1, 2)") diff --git a/posthog/hogql_queries/insights/lifecycle_query_runner.py b/posthog/hogql_queries/insights/lifecycle_query_runner.py index fcd41df697735c..136ec7f2f6f421 100644 --- a/posthog/hogql_queries/insights/lifecycle_query_runner.py +++ b/posthog/hogql_queries/insights/lifecycle_query_runner.py @@ -8,6 +8,7 @@ from posthog.hogql import ast from posthog.hogql.parser import parse_expr, parse_select +from posthog.hogql.printer import to_printed_hogql from posthog.hogql.property import property_to_expr, action_to_expr from posthog.hogql.query import execute_hogql_query from posthog.hogql.timings import HogQLTimings @@ -92,9 +93,12 @@ def to_persons_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: ) def calculate(self): + query = self.to_query() + hogql = to_printed_hogql(query, self.team.pk) + response = execute_hogql_query( query_type="LifecycleQuery", - query=self.to_query(), + query=query, team=self.team, timings=self.timings, ) @@ -130,7 +134,7 @@ def calculate(self): } ) - return LifecycleQueryResponse(results=res, timings=response.timings) + return LifecycleQueryResponse(results=res, timings=response.timings, hogql=hogql) @cached_property def query_date_range(self):