revert(insights): persist q in url hash, remove beforeUnload (#35286)

Author: thmsobrmlr

frontend/src/scenes/insights/InsightPageHeader.tsx

@@ -1,6 +1,6 @@
 import { IconInfo, IconPencil, IconShare, IconTrash, IconWarning } from '@posthog/icons'
 import { useActions, useMountedLogic, useValues } from 'kea'
-import { router, combineUrl } from 'kea-router'
+import { router } from 'kea-router'
 import { AccessControlledLemonButton } from 'lib/components/AccessControlledLemonButton'
 import { AddToDashboard } from 'lib/components/AddToDashboard/AddToDashboard'
 import { AddToDashboardModal } from 'lib/components/AddToDashboard/AddToDashboardModal'
@@ -265,13 +265,7 @@ export function InsightPageHeader({ insightLogicProps }: { insightLogicProps: In
                                         if (isDataVisualizationNode(query) && insight.short_id) {
                                             router.actions.push(urls.sqlEditor(undefined, undefined, insight.short_id))
                                         } else if (insight.short_id) {
-                                            push(
-                                                combineUrl(
-                                                    urls.insightEdit(insight.short_id),
-                                                    undefined,
-                                                    queryChanged ? { q: query } : undefined
-                                                ).url
-                                            )
+                                            push(urls.insightEdit(insight.short_id))
                                         } else {
                                             setInsightMode(ItemMode.Edit, null)
                                         }

frontend/src/scenes/insights/insightDataLogic.tsx

@@ -245,7 +245,11 @@ export const insightDataLogic = kea<insightDataLogicType>([
     }),
     actionToUrl(({ values }) => ({
         setQuery: ({ query }) => {
-            if (values.queryChanged && insightSceneLogic.values.activeScene === Scene.Insight) {
+            if (
+                values.queryChanged &&
+                insightSceneLogic.values.activeScene === Scene.Insight &&
+                insightSceneLogic.values.insightId === 'new'
+            ) {
                 // query is changed and we are in edit mode
                 return [
                     router.values.currentLocation.pathname,
@@ -256,9 +260,6 @@ export const insightDataLogic = kea<insightDataLogicType>([
                         ...router.values.currentLocation.hashParams,
                         q: crushDraftQueryForURL(query),
                     },
-                    {
-                        replace: true,
-                    },
                 ]
             }
         },

frontend/src/scenes/insights/insightSceneLogic.tsx

@@ -1,5 +1,6 @@
 import { actions, BuiltLogic, connect, kea, listeners, path, reducers, selectors, sharedListeners } from 'kea'
-import { actionToUrl, router, urlToAction } from 'kea-router'
+import { actionToUrl, beforeUnload, router, urlToAction } from 'kea-router'
+import { CombinedLocation } from 'kea-router/lib/utils'
 import { objectsEqual } from 'kea-test-utils'
 import { AlertType } from 'lib/components/Alerts/types'
 import { isEmptyObject } from 'lib/utils'
@@ -31,7 +32,7 @@ import {
 import { insightDataLogic } from './insightDataLogic'
 import { insightDataLogicType } from './insightDataLogicType'
 import type { insightSceneLogicType } from './insightSceneLogicType'
-import { parseDraftQueryFromURL } from './utils'
+import { parseDraftQueryFromLocalStorage, parseDraftQueryFromURL } from './utils'
 import api from 'lib/api'
 import { checkLatestVersionsOnQuery } from '~/queries/utils'
 
@@ -417,37 +418,88 @@ export const insightSceneLogic = kea<insightSceneLogicType>([
         },
     })),
     actionToUrl(({ values }) => {
+        // Use the browser redirect to determine state to hook into beforeunload prevention
         const actionToUrl = ({
             insightMode = values.insightMode,
             insightId = values.insightId,
         }: {
             insightMode?: ItemMode
             insightId?: InsightShortId | 'new' | null
-        }): [
-            string,
-            string | Record<string, any> | undefined,
-            string | Record<string, any> | undefined,
-            { replace?: boolean }
-        ] => {
-            const baseUrl =
-                !insightId || insightId === 'new'
-                    ? urls.insightNew()
-                    : insightMode === ItemMode.View
-                    ? urls.insightView(insightId)
-                    : urls.insightEdit(insightId)
-            const searchParams = router.values.currentLocation.searchParams
-            // TODO: also kepe these in the URL?
-            // const metadataChanged = !!values.insightLogicRef?.logic.values.insightChanged
-            const queryChanged = !!values.insightDataLogicRef?.logic.values.queryChanged
-            const query = values.insightDataLogicRef?.logic.values.query
-            const hashParams = queryChanged ? { q: query } : undefined
+        }): string | undefined => {
+            if (!insightId || insightId === 'new') {
+                return undefined
+            }
 
-            return [baseUrl, searchParams, hashParams, { replace: true }]
+            const baseUrl = insightMode === ItemMode.View ? urls.insightView(insightId) : urls.insightEdit(insightId)
+            const searchParams = window.location.search
+            return searchParams ? `${baseUrl}${searchParams}` : baseUrl
         }
 
         return {
             setInsightId: actionToUrl,
             setInsightMode: actionToUrl,
         }
     }),
+    beforeUnload(({ values }) => ({
+        enabled: (newLocation?: CombinedLocation) => {
+            // Don't run this check on other scenes
+            if (values.activeScene !== Scene.Insight) {
+                return false
+            }
+
+            if (values.disableNavigationHooks) {
+                return false
+            }
+
+            // If just the hash or project part changes, don't show the prompt
+            const currentPathname = router.values.currentLocation.pathname.replace(/\/project\/\d+/, '')
+            const newPathname = newLocation?.pathname.replace(/\/project\/\d+/, '')
+            if (currentPathname === newPathname) {
+                return false
+            }
+
+            // Don't show the prompt if we're in edit mode (just exploring)
+            if (values.insightMode !== ItemMode.Edit) {
+                return false
+            }
+
+            const metadataChanged = !!values.insightLogicRef?.logic.values.insightChanged
+            const queryChanged = !!values.insightDataLogicRef?.logic.values.queryChanged
+            const draftQueryFromLocalStorage = localStorage.getItem(`draft-query-${values.currentTeamId}`)
+            let draftQuery: { query: Node; timestamp: number } | null = null
+            if (draftQueryFromLocalStorage) {
+                const parsedQuery = parseDraftQueryFromLocalStorage(draftQueryFromLocalStorage)
+                if (parsedQuery) {
+                    draftQuery = parsedQuery
+                } else {
+                    // If the draft query is invalid, remove it
+                    localStorage.removeItem(`draft-query-${values.currentTeamId}`)
+                }
+            }
+            const query = values.insightDataLogicRef?.logic.values.query
+
+            if (draftQuery && query && objectsEqual(draftQuery.query, query)) {
+                return false
+            }
+
+            const isChanged = metadataChanged || queryChanged
+
+            if (!isChanged) {
+                return false
+            }
+
+            // Do not show confirmation if newPathname is undefined; this usually means back button in browser
+            if (newPathname === undefined) {
+                const savedQuery = values.insightDataLogicRef?.logic.values.savedInsight.query
+                values.insightDataLogicRef?.logic.actions.setQuery(savedQuery || null)
+                return false
+            }
+
+            return true
+        },
+        message: 'Leave insight?\nChanges you made will be discarded.',
+        onConfirm: () => {
+            values.insightDataLogicRef?.logic.actions.cancelChanges()
+        },
+    })),
 ])