refactor(insights): simplify "save as ..." (PostHog#24024)

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
silentninja · Aug 8, 2024 · 2bf3792 · 2bf3792
1 parent d8de129
commit 2bf3792
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 79 deletions.
diff --git 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 (
         <div>
@@ -59,7 +64,7 @@ export function DataWarehouseExternalScene(): JSX.Element {
                             <LemonButton
                                 type="primary"
                                 data-attr="save-exploration"
-                                onClick={() => 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: (
-                        <span className="flex justify-center items-center justify-between gap-1">
+                        <span className="flex items-center justify-between gap-1">
                             {humanFriendlyDataWarehouseTabName(tab as DataWarehouseTab)}{' '}
                         </span>
                     ),

diff --git 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
                             )
                         ) : (
                             <InsightSaveButton
+                                query={query}
                                 saveAs={saveAs}
                                 saveInsight={saveInsight}
                                 isSaved={hasDashboardItemId}

diff --git a/frontend/src/scenes/insights/InsightSaveButton.tsx b/frontend/src/scenes/insights/InsightSaveButton.tsx
@@ -1,14 +1,18 @@
 import { LemonButton } from 'lib/lemon-ui/LemonButton'
 
+import { Node } from '~/queries/schema'
+
 export function InsightSaveButton({
+    query,
     saveAs,
     saveInsight,
     isSaved,
     insightSaving,
     insightChanged,
     addingToDashboard,
 }: {
-    saveAs: () => void
+    query: Node
+    saveAs: (query: Node) => void
     saveInsight: (redirectToViewMode?: boolean) => void
     isSaved: boolean | undefined
     insightSaving: boolean
@@ -40,7 +44,11 @@ export function InsightSaveButton({
                                 </LemonButton>
                             )}
                             {saveAsAvailable && (
-                                <LemonButton onClick={saveAs} data-attr="insight-save-as-new-insight" fullWidth>
+                                <LemonButton
+                                    onClick={() => saveAs(query)}
+                                    data-attr="insight-save-as-new-insight"
+                                    fullWidth
+                                >
                                     Save as…
                                 </LemonButton>
                             )}

diff --git 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<insightDataLogicType>([
         ],
         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<insightDataLogicType>([
 
     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<insightDataLogicType>([
 
             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: (
-                    <LemonField name="insightName">
-                        <LemonInput data-attr="insight-name" placeholder="Please enter the new name" autoFocus />
-                    </LemonField>
-                ),
-                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
@@ -26,6 +26,7 @@ import {
     PropertyOperator,
 } from '~/types'
 
+import { insightDataLogic } from './insightDataLogic'
 import { createEmptyInsight, insightLogic } from './insightLogic'
 
 const API_FILTERS: Partial<FilterType> = {
@@ -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 → ...tend/src/scenes/insights/insightLogic.tsx b/frontend/src/scenes/insights/insightLogic.ts → ...tend/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<insightLogicType>([
             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<insightLogicType>([
                 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: (
+                    <LemonField name="name">
+                        <LemonInput data-attr="insight-name" placeholder="Please enter the new name" autoFocus />
+                    </LemonField>
+                ),
+                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
@@ -39,7 +39,7 @@ async function _perform<Flag extends boolean>(
 export const insightsApi = {
     _perform,
     async create<Flag extends boolean>(
-        insight: QueryBasedInsightModel,
+        insight: Partial<QueryBasedInsightModel>,
         options: InsightsApiOptions<Flag>
     ): Promise<ReturnedInsightModelByFlag<Flag>> {
         return this._perform('create', insight, options)