From 2bf3792091786c4859dd3cb63c13a24d9365a524 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Oberm=C3=BCller?= Date: Mon, 29 Jul 2024 13:25:42 +0200 Subject: [PATCH] refactor(insights): simplify "save as ..." (#24024) Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> --- .../external/DataWarehouseExternalScene.tsx | 15 +++-- .../src/scenes/insights/InsightPageHeader.tsx | 7 ++- .../src/scenes/insights/InsightSaveButton.tsx | 12 +++- .../src/scenes/insights/insightDataLogic.tsx | 56 +------------------ .../src/scenes/insights/insightLogic.test.ts | 6 +- .../{insightLogic.ts => insightLogic.tsx} | 52 +++++++++++++---- frontend/src/scenes/insights/utils/api.ts | 2 +- 7 files changed, 71 insertions(+), 79 deletions(-) rename frontend/src/scenes/insights/{insightLogic.ts => insightLogic.tsx} (91%) diff --git a/frontend/src/scenes/data-warehouse/external/DataWarehouseExternalScene.tsx b/frontend/src/scenes/data-warehouse/external/DataWarehouseExternalScene.tsx index dcbadd261a832d..05386e9877552b 100644 --- a/frontend/src/scenes/data-warehouse/external/DataWarehouseExternalScene.tsx +++ b/frontend/src/scenes/data-warehouse/external/DataWarehouseExternalScene.tsx @@ -41,14 +41,19 @@ export const humanFriendlyDataWarehouseTabName = (tab: DataWarehouseTab): string export function DataWarehouseExternalScene(): JSX.Element { const { currentTab } = useValues(dataWarehouseSceneLogic) - const { insightProps, insightSaving } = useValues( + const { insightSaving, insightProps } = useValues( insightLogic({ dashboardItemId: 'new', cachedInsight: null, }) ) - - const { saveAs } = useActions(insightDataLogic(insightProps)) + const { saveAs } = useActions( + insightLogic({ + dashboardItemId: 'new', + cachedInsight: null, + }) + ) + const { query } = useValues(insightDataLogic(insightProps)) return (
@@ -59,7 +64,7 @@ export function DataWarehouseExternalScene(): JSX.Element { saveAs(true)} + onClick={() => saveAs(query, true)} loading={insightSaving} > Save as insight @@ -93,7 +98,7 @@ export function DataWarehouseExternalScene(): JSX.Element { onChange={(tab) => router.actions.push(urls.dataWarehouse(tab as DataWarehouseTab))} tabs={Object.entries(tabToContent).map(([tab, content]) => ({ label: ( - + {humanFriendlyDataWarehouseTabName(tab as DataWarehouseTab)}{' '} ), diff --git a/frontend/src/scenes/insights/InsightPageHeader.tsx b/frontend/src/scenes/insights/InsightPageHeader.tsx index bf9b1173203bba..10203705b096a1 100644 --- a/frontend/src/scenes/insights/InsightPageHeader.tsx +++ b/frontend/src/scenes/insights/InsightPageHeader.tsx @@ -46,14 +46,14 @@ export function InsightPageHeader({ insightLogicProps }: { insightLogicProps: In insightSaving, hasDashboardItemId, } = useValues(insightLogic(insightLogicProps)) - const { setInsightMetadata } = useActions(insightLogic(insightLogicProps)) + const { setInsightMetadata, saveAs } = useActions(insightLogic(insightLogicProps)) // savedInsightsLogic const { duplicateInsight, loadInsights } = useActions(savedInsightsLogic) // insightDataLogic - const { queryChanged, showQueryEditor, hogQL, exportContext } = useValues(insightDataLogic(insightProps)) - const { saveInsight, saveAs, toggleQueryEditorPanel } = useActions(insightDataLogic(insightProps)) + const { queryChanged, showQueryEditor, hogQL, exportContext, query } = useValues(insightDataLogic(insightProps)) + const { saveInsight, toggleQueryEditorPanel } = useActions(insightDataLogic(insightProps)) // other logics useMountedLogic(insightCommandLogic(insightProps)) @@ -275,6 +275,7 @@ export function InsightPageHeader({ insightLogicProps }: { insightLogicProps: In ) ) : ( void + query: Node + saveAs: (query: Node) => void saveInsight: (redirectToViewMode?: boolean) => void isSaved: boolean | undefined insightSaving: boolean @@ -40,7 +44,11 @@ export function InsightSaveButton({ )} {saveAsAvailable && ( - + saveAs(query)} + data-attr="insight-save-as-new-insight" + fullWidth + > Save as… )} diff --git a/frontend/src/scenes/insights/insightDataLogic.tsx b/frontend/src/scenes/insights/insightDataLogic.tsx index 668798a9fe49c4..64170fc92eed13 100644 --- a/frontend/src/scenes/insights/insightDataLogic.tsx +++ b/frontend/src/scenes/insights/insightDataLogic.tsx @@ -1,7 +1,5 @@ -import { LemonDialog, LemonInput } from '@posthog/lemon-ui' import { actions, connect, kea, key, listeners, path, props, propsChanged, reducers, selectors } from 'kea' import { FEATURE_FLAGS } from 'lib/constants' -import { LemonField } from 'lib/lemon-ui/LemonField' import { featureFlagLogic } from 'lib/logic/featureFlagLogic' import { objectsEqual } from 'lib/utils' import { keyForInsightLogicProps } from 'scenes/insights/sharedUtils' @@ -66,12 +64,7 @@ export const insightDataLogic = kea([ ], actions: [ insightLogic, - [ - 'setInsight', - 'loadInsightSuccess', - 'saveInsight as insightLogicSaveInsight', - 'saveAsNamingSuccess as insightLogicSaveAsNamingSuccess', - ], + ['setInsight', 'loadInsightSuccess', 'saveInsight as insightLogicSaveInsight'], dataNodeLogic({ key: insightVizDataNodeKey(props) } as DataNodeLogicProps), ['loadData', 'loadDataSuccess', 'loadDataFailure', 'setResponse as setInsightData'], ], @@ -80,8 +73,6 @@ export const insightDataLogic = kea([ actions({ setQuery: (query: Node | null) => ({ query }), - saveAs: (redirectToViewMode?: boolean) => ({ redirectToViewMode }), - saveAsNamingSuccess: (name: string, redirectToViewMode?: boolean) => ({ name, redirectToViewMode }), saveInsight: (redirectToViewMode = true) => ({ redirectToViewMode }), toggleQueryEditorPanel: true, cancelChanges: true, @@ -237,51 +228,6 @@ export const insightDataLogic = kea([ actions.insightLogicSaveInsight(redirectToViewMode) }, - saveAs: async ({ redirectToViewMode }) => { - LemonDialog.openForm({ - title: 'Save as new insight', - initialValues: { - insightName: - values.queryBasedInsight.name || values.queryBasedInsight.derived_name - ? `${values.queryBasedInsight.name || values.queryBasedInsight.derived_name} (copy)` - : '', - }, - content: ( - - - - ), - errors: { - insightName: (name) => (!name ? 'You must enter a name' : undefined), - }, - onSubmit: async ({ insightName }) => actions.saveAsNamingSuccess(insightName, redirectToViewMode), - }) - }, - saveAsNamingSuccess: ({ name, redirectToViewMode }) => { - let filters = values.legacyInsight.filters - if (isInsightVizNode(values.query)) { - const querySource = values.query.source - filters = queryNodeToFilter(querySource) - } else if (values.isQueryBasedInsight) { - filters = {} - } - - let query = undefined - if (values.isQueryBasedInsight) { - query = values.query - } - - actions.setInsight( - { - ...values.legacyInsight, - filters: filters, - query: query ?? undefined, - }, - { overrideFilter: true, fromPersistentApi: false } - ) - - actions.insightLogicSaveAsNamingSuccess(name, redirectToViewMode) - }, cancelChanges: () => { const savedFilters = values.savedInsight.filters const savedResult = values.savedInsight.result diff --git a/frontend/src/scenes/insights/insightLogic.test.ts b/frontend/src/scenes/insights/insightLogic.test.ts index 1c8595b2d264c4..9a993110cd2bf6 100644 --- a/frontend/src/scenes/insights/insightLogic.test.ts +++ b/frontend/src/scenes/insights/insightLogic.test.ts @@ -26,6 +26,7 @@ import { PropertyOperator, } from '~/types' +import { insightDataLogic } from './insightDataLogic' import { createEmptyInsight, insightLogic } from './insightLogic' const API_FILTERS: Partial = { @@ -574,8 +575,11 @@ describe('insightLogic', () => { }) logic.mount() + const dataLogic = insightDataLogic(logic.values.insightProps) + dataLogic.mount() + await expectLogic(logic, () => { - logic.actions.saveAsNamingSuccess('New Insight (copy)') + logic.actions.saveAsConfirmation('New Insight (copy)', dataLogic.values.query) }) .toDispatchActions(['setInsight']) .toDispatchActions(savedInsightsLogic, ['loadInsights']) diff --git a/frontend/src/scenes/insights/insightLogic.ts b/frontend/src/scenes/insights/insightLogic.tsx similarity index 91% rename from frontend/src/scenes/insights/insightLogic.ts rename to frontend/src/scenes/insights/insightLogic.tsx index 383bee462f7217..00e98686e19390 100644 --- a/frontend/src/scenes/insights/insightLogic.ts +++ b/frontend/src/scenes/insights/insightLogic.tsx @@ -1,8 +1,10 @@ +import { LemonDialog, LemonInput } from '@posthog/lemon-ui' import { actions, connect, events, kea, key, listeners, path, props, reducers, selectors } from 'kea' import { loaders } from 'kea-loaders' import { router } from 'kea-router' import api from 'lib/api' import { DashboardPrivilegeLevel, FEATURE_FLAGS } from 'lib/constants' +import { LemonField } from 'lib/lemon-ui/LemonField' import { lemonToast } from 'lib/lemon-ui/LemonToast/LemonToast' import { featureFlagLogic } from 'lib/logic/featureFlagLogic' import { objectsEqual } from 'lib/utils' @@ -24,7 +26,7 @@ import { insightsModel } from '~/models/insightsModel' import { tagsModel } from '~/models/tagsModel' import { getInsightFilterOrQueryForPersistance } from '~/queries/nodes/InsightQuery/utils/queryNodeToFilter' import { getQueryBasedInsightModel } from '~/queries/nodes/InsightViz/utils' -import { InsightVizNode } from '~/queries/schema' +import { InsightVizNode, Node } from '~/queries/schema' import { FilterType, InsightLogicProps, @@ -87,7 +89,12 @@ export const insightLogic = kea([ insight, options, }), - saveAsNamingSuccess: (name: string, redirectToViewMode?: boolean) => ({ name, redirectToViewMode }), + saveAs: (query: Node, redirectToViewMode?: boolean) => ({ query, redirectToViewMode }), + saveAsConfirmation: (name: string, query: Node, redirectToViewMode?: boolean) => ({ + name, + query, + redirectToViewMode, + }), cancelChanges: true, saveInsight: (redirectToViewMode = true) => ({ redirectToViewMode }), saveInsightSuccess: true, @@ -420,17 +427,38 @@ export const insightLogic = kea([ router.actions.push(urls.insightEdit(savedInsight.short_id)) } }, - saveAsNamingSuccess: async ({ name, redirectToViewMode }) => { - const { filters, query } = getInsightFilterOrQueryForPersistance( - values.queryBasedInsight, - values.queryBasedInsightSaving - ) - const insight: InsightModel = await api.create(`api/projects/${teamLogic.values.currentTeamId}/insights/`, { - name, - filters, - query, - saved: true, + saveAs: async ({ query, redirectToViewMode }) => { + LemonDialog.openForm({ + title: 'Save as new insight', + initialValues: { + name: + values.queryBasedInsight.name || values.queryBasedInsight.derived_name + ? `${values.queryBasedInsight.name || values.queryBasedInsight.derived_name} (copy)` + : '', + }, + content: ( + + + + ), + errors: { + name: (name) => (!name ? 'You must enter a name' : undefined), + }, + onSubmit: async ({ name }) => actions.saveAsConfirmation(name, query, redirectToViewMode), }) + }, + saveAsConfirmation: async ({ name, query, redirectToViewMode }) => { + const insight = await insightsApi.create( + { + name, + query, + saved: true, + }, + { + writeAsQuery: values.queryBasedInsightSaving, + readAsQuery: false, + } + ) lemonToast.info( `You're now working on a copy of ${ values.queryBasedInsight.name || values.queryBasedInsight.derived_name || name diff --git a/frontend/src/scenes/insights/utils/api.ts b/frontend/src/scenes/insights/utils/api.ts index 24ec18a81b0ed5..7a04a074df32c7 100644 --- a/frontend/src/scenes/insights/utils/api.ts +++ b/frontend/src/scenes/insights/utils/api.ts @@ -39,7 +39,7 @@ async function _perform( export const insightsApi = { _perform, async create( - insight: QueryBasedInsightModel, + insight: Partial, options: InsightsApiOptions ): Promise> { return this._perform('create', insight, options)