diff --git a/cypress/e2e/insights-reload-query.ts b/cypress/e2e/insights-reload-query.ts new file mode 100644 index 0000000000000..2f944f8993da7 --- /dev/null +++ b/cypress/e2e/insights-reload-query.ts @@ -0,0 +1,48 @@ +import JSONCrush from 'jsoncrush' + +describe('ReloadInsight component', () => { + beforeEach(() => { + // Clear local storage before each test to ensure a clean state + cy.clearLocalStorage() + }) + + it('saves the query to the URL and localStorage, and reloads it when prompted', () => { + // Visit the new insight creation page + cy.visit('/insights/new') + + cy.wait(2000) + + cy.get('[data-attr="math-selector-0"]').click({ force: true }) + cy.get('[data-attr="math-dau-0"]').click({ force: true }) + + // Check that the 'draft-query' item is stored in localStorage + cy.window().then((window) => { + const currentTeamId = window.POSTHOG_APP_CONTEXT.current_team.id + const draftQuery = window.localStorage.getItem(`draft-query-${currentTeamId}`) + expect(draftQuery).to.not.be.null + + const draftQueryObjUncrushed = JSONCrush.uncrush(draftQuery) + const draftQueryObj = JSON.parse(draftQueryObjUncrushed) + + expect(draftQueryObj).to.have.property('query') + + const firstSeries = draftQueryObj.query.source.series[0] + + expect(firstSeries).to.include({ + event: '$pageview', + math: 'dau', + }) + }) + + // Navigate away to the "Saved Insights" page + cy.visit('/saved_insights') + + // Verify that the ReloadInsight component displays a message about the unsaved insight + cy.contains('You have an unsaved insight from').should('exist') + + // Click the link to reload the unsaved insight + cy.contains('Click here').click() + + cy.get('[data-attr="math-selector-0"]').should('contain', 'Unique users') + }) +}) diff --git a/frontend/__snapshots__/replay-player-success--recent-recordings--dark.png b/frontend/__snapshots__/replay-player-success--recent-recordings--dark.png index 2392b90f55117..bfa0bb8c5d56d 100644 Binary files a/frontend/__snapshots__/replay-player-success--recent-recordings--dark.png and b/frontend/__snapshots__/replay-player-success--recent-recordings--dark.png differ diff --git a/frontend/__snapshots__/replay-player-success--recent-recordings--light.png b/frontend/__snapshots__/replay-player-success--recent-recordings--light.png index 130aa9a287ffd..d3b511f07c042 100644 Binary files a/frontend/__snapshots__/replay-player-success--recent-recordings--light.png and b/frontend/__snapshots__/replay-player-success--recent-recordings--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark.png b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark.png index ee27b11bed568..4b405550412a0 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark.png and b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light.png b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light.png index 00916265cd38b..02bc921745ecd 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light.png and b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light.png differ diff --git a/frontend/src/scenes/insights/Insight.tsx b/frontend/src/scenes/insights/Insight.tsx index a0edff02deab5..573321196a64f 100644 --- a/frontend/src/scenes/insights/Insight.tsx +++ b/frontend/src/scenes/insights/Insight.tsx @@ -4,6 +4,7 @@ import { DebugCHQueries } from 'lib/components/CommandPalette/DebugCHQueries' import { isObject } from 'lib/utils' import { InsightPageHeader } from 'scenes/insights/InsightPageHeader' import { insightSceneLogic } from 'scenes/insights/insightSceneLogic' +import { ReloadInsight } from 'scenes/saved-insights/ReloadInsight' import { urls } from 'scenes/urls' import { Query } from '~/queries/Query/Query' @@ -21,7 +22,7 @@ export interface InsightSceneProps { export function Insight({ insightId }: InsightSceneProps): JSX.Element { // insightSceneLogic - const { insightMode, insight, filtersOverride, variablesOverride } = useValues(insightSceneLogic) + const { insightMode, insight, filtersOverride, variablesOverride, freshQuery } = useValues(insightSceneLogic) // insightLogic const logic = insightLogic({ @@ -79,6 +80,8 @@ export function Insight({ insightId }: InsightSceneProps): JSX.Element { )} + {freshQuery ? : null} + ([ @@ -29,6 +34,10 @@ export const insightDataLogic = kea([ values: [ insightLogic, ['insight', 'savedInsight'], + insightSceneLogic, + ['insightId', 'insightMode', 'activeScene'], + teamLogic, + ['currentTeamId'], dataNodeLogic({ key: insightVizDataNodeKey(props), loadPriority: props.loadPriority, @@ -49,7 +58,7 @@ export const insightDataLogic = kea([ ], actions: [ insightLogic, - ['setInsight', 'loadInsightSuccess'], + ['setInsight'], dataNodeLogic({ key: insightVizDataNodeKey(props) } as DataNodeLogicProps), ['loadData', 'loadDataSuccess', 'loadDataFailure', 'setResponse as setInsightData'], ], @@ -187,21 +196,59 @@ export const insightDataLogic = kea([ actions.setInsightData({ ...values.insightData, result }) } }, - loadInsightSuccess: ({ insight }) => { - if (insight.query) { - actions.setQuery(insight.query) - } - }, cancelChanges: () => { const savedQuery = values.savedInsight.query const savedResult = values.savedInsight.result actions.setQuery(savedQuery || null) actions.setInsightData({ ...values.insightData, result: savedResult ? savedResult : null }) }, + setQuery: ({ query }) => { + // if the query is not changed, don't save it + if (!query || !values.queryChanged) { + return + } + // only run on insight scene + if (insightSceneLogic.values.activeScene !== Scene.Insight) { + return + } + // don't save for saved insights + if (insightSceneLogic.values.insightId !== 'new') { + return + } + + if (isQueryTooLarge(query)) { + localStorage.removeItem(`draft-query-${values.currentTeamId}`) + } + localStorage.setItem( + `draft-query-${values.currentTeamId}`, + crushDraftQueryForLocalStorage(query, Date.now()) + ) + }, })), propsChanged(({ actions, props, values }) => { if (props.cachedInsight?.query && !objectsEqual(props.cachedInsight.query, values.query)) { actions.setQuery(props.cachedInsight.query) } }), + actionToUrl(({ values }) => ({ + setQuery: ({ query }) => { + 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, + { + ...router.values.currentLocation.searchParams, + }, + { + ...router.values.currentLocation.hashParams, + q: crushDraftQueryForURL(query), + }, + ] + } + }, + })), ]) diff --git a/frontend/src/scenes/insights/insightLogic.tsx b/frontend/src/scenes/insights/insightLogic.tsx index a3c9905180538..1ca1548f30047 100644 --- a/frontend/src/scenes/insights/insightLogic.tsx +++ b/frontend/src/scenes/insights/insightLogic.tsx @@ -336,6 +336,8 @@ export const insightLogic: LogicWrapper = kea { }) }) - it('redirects when opening /insight/new with insight type in theurl', async () => { + it('redirects maintaining url params when opening /insight/new with insight type in theurl', async () => { router.actions.push(urls.insightNew(InsightType.FUNNELS)) await expectLogic(logic).toFinishAllListeners() - await expectLogic(router) - .delay(1) - .toMatchValues({ - location: partial({ - pathname: addProjectIdIfMissing(urls.insightNew(), MOCK_TEAM_ID), - search: '', - hash: '', - }), - }) expect((logic.values.insightLogicRef?.logic.values.insight.query as InsightVizNode).source?.kind).toEqual( 'FunnelsQuery' ) }) - it('redirects when opening /insight/new with query in the url', async () => { + it('redirects maintaining url params when opening /insight/new with query in the url', async () => { router.actions.push( urls.insightNew(undefined, undefined, { kind: NodeKind.InsightVizNode, @@ -70,15 +61,6 @@ describe('insightSceneLogic', () => { } as InsightVizNode) ) await expectLogic(logic).toFinishAllListeners() - await expectLogic(router) - .delay(1) - .toMatchValues({ - location: partial({ - pathname: addProjectIdIfMissing(urls.insightNew(), MOCK_TEAM_ID), - search: '', - hash: '', - }), - }) expect((logic.values.insightLogicRef?.logic.values.insight.query as InsightVizNode).source?.kind).toEqual( 'PathsQuery' diff --git a/frontend/src/scenes/insights/insightSceneLogic.tsx b/frontend/src/scenes/insights/insightSceneLogic.tsx index ab58df3d1ec86..d9b89d13ff25c 100644 --- a/frontend/src/scenes/insights/insightSceneLogic.tsx +++ b/frontend/src/scenes/insights/insightSceneLogic.tsx @@ -26,6 +26,10 @@ import { insightDataLogic } from './insightDataLogic' import { insightDataLogicType } from './insightDataLogicType' import type { insightSceneLogicType } from './insightSceneLogicType' import { summarizeInsight } from './summarizeInsight' +import { parseDraftQueryFromLocalStorage, parseDraftQueryFromURL } from './utils' + +const NEW_INSIGHT = 'new' as const +export type InsightId = InsightShortId | typeof NEW_INSIGHT | null export const insightSceneLogic = kea([ path(['scenes', 'insights', 'insightSceneLogic']), @@ -33,7 +37,7 @@ export const insightSceneLogic = kea([ logic: [eventUsageLogic], values: [ teamLogic, - ['currentTeam'], + ['currentTeam', 'currentTeamId'], sceneLogic, ['activeScene'], preflightLogic, @@ -73,10 +77,11 @@ export const insightSceneLogic = kea([ unmount, }), setOpenedWithQuery: (query: Node | null) => ({ query }), + setFreshQuery: (freshQuery: boolean) => ({ freshQuery }), }), reducers({ insightId: [ - null as null | 'new' | InsightShortId, + null as null | InsightId, { setSceneState: (_, { insightId }) => insightId, }, @@ -150,6 +155,7 @@ export const insightSceneLogic = kea([ }, ], openedWithQuery: [null as Node | null, { setOpenedWithQuery: (_, { query }) => query }], + freshQuery: [false, { setFreshQuery: (_, { freshQuery }) => freshQuery }], }), selectors(() => ({ insightSelector: [(s) => [s.insightLogicRef], (insightLogicRef) => insightLogicRef?.logic.selectors.insight], @@ -332,24 +338,20 @@ export const insightSceneLogic = kea([ let queryFromUrl: Node | null = null if (q) { - queryFromUrl = JSON.parse(q) + const validQuery = parseDraftQueryFromURL(q) + if (validQuery) { + queryFromUrl = validQuery + } else { + console.error('Invalid query', q) + } } else if (insightType && Object.values(InsightType).includes(insightType)) { queryFromUrl = getDefaultQuery(insightType, values.filterTestAccountsDefault) } - // Redirect to a simple URL if we had a query in the URL - if (q || insightType) { - router.actions.replace( - insightId === 'new' - ? urls.insightNew(undefined, dashboard) - : insightMode === ItemMode.Edit - ? urls.insightEdit(insightId) - : urls.insightView(insightId) - ) - } + actions.setFreshQuery(false) // reset the insight's state if we have to - if (initial || method === 'PUSH' || queryFromUrl) { + if (initial || queryFromUrl || method === 'PUSH') { if (insightId === 'new') { const query = queryFromUrl || getDefaultQuery(InsightType.TRENDS, values.filterTestAccountsDefault) values.insightLogicRef?.logic.actions.setInsight( @@ -364,6 +366,10 @@ export const insightSceneLogic = kea([ } ) + if (!queryFromUrl) { + actions.setFreshQuery(true) + } + actions.setOpenedWithQuery(query) eventUsageLogic.actions.reportInsightCreated(query) @@ -416,6 +422,22 @@ export const insightSceneLogic = kea([ 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 + } return metadataChanged || queryChanged }, diff --git a/frontend/src/scenes/insights/utils.tsx b/frontend/src/scenes/insights/utils.tsx index 96d3129e47fa6..5a1b4d56ec7d9 100644 --- a/frontend/src/scenes/insights/utils.tsx +++ b/frontend/src/scenes/insights/utils.tsx @@ -1,3 +1,4 @@ +import JSONCrush from 'jsoncrush' import api from 'lib/api' import { dayjs } from 'lib/dayjs' import { CORE_FILTER_DEFINITIONS_BY_GROUP } from 'lib/taxonomy' @@ -15,6 +16,7 @@ import { DataWarehouseNode, EventsNode, InsightVizNode, + Node, NodeKind, PathsFilter, } from '~/queries/schema' @@ -433,3 +435,50 @@ export function insightUrlForEvent(event: Pick>): boolean { + // Chrome has a 2MB limit for the HASH params, limit ours at 1MB + const queryLength = encodeURI(JSON.stringify(query)).split(/%..|./).length - 1 + return queryLength > 1024 * 1024 +} + +export function parseDraftQueryFromLocalStorage( + query: string +): { query: Node>; timestamp: number } | null { + // First try to uncrush the query if it's a JSONCrush query else fall back to parsing it as a JSON + try { + const uncrushedQuery = JSONCrush.uncrush(query) + return JSON.parse(uncrushedQuery) + } catch (e) { + console.error('Error parsing uncrushed query', e) + try { + return JSON.parse(query) + } catch (e) { + console.error('Error parsing query', e) + return null + } + } +} + +export function crushDraftQueryForLocalStorage(query: Node>, timestamp: number): string { + return JSONCrush.crush(JSON.stringify({ query, timestamp })) +} + +export function parseDraftQueryFromURL(query: string): Node> | null { + try { + const uncrushedQuery = JSONCrush.uncrush(query) + return JSON.parse(uncrushedQuery) + } catch (e) { + console.error('Error parsing uncrushed query', e) + try { + return JSON.parse(query) + } catch (e) { + console.error('Error parsing query', e) + return null + } + } +} + +export function crushDraftQueryForURL(query: Node>): string { + return JSONCrush.crush(JSON.stringify(query)) +} diff --git a/frontend/src/scenes/saved-insights/ReloadInsight.tsx b/frontend/src/scenes/saved-insights/ReloadInsight.tsx new file mode 100644 index 0000000000000..66a258eabdc3c --- /dev/null +++ b/frontend/src/scenes/saved-insights/ReloadInsight.tsx @@ -0,0 +1,31 @@ +import { Link } from '@posthog/lemon-ui' +import { useValues } from 'kea' +import { parseDraftQueryFromLocalStorage } from 'scenes/insights/utils' +import { teamLogic } from 'scenes/teamLogic' +import { urls } from 'scenes/urls' + +import { Node } from '~/queries/schema' + +export function ReloadInsight(): JSX.Element { + const { currentTeamId } = useValues(teamLogic) + const draftQueryLocalStorage = localStorage.getItem(`draft-query-${currentTeamId}`) + let draftQuery: { query: Node>; timestamp: number } | null = null + if (draftQueryLocalStorage) { + const parsedQuery = parseDraftQueryFromLocalStorage(draftQueryLocalStorage) + if (parsedQuery) { + draftQuery = parsedQuery + } else { + localStorage.removeItem(`draft-query-${currentTeamId}`) + } + } + + if (!draftQuery?.query) { + return <> + } + return ( +
+ You have an unsaved insight from {new Date(draftQuery.timestamp).toLocaleString()}.{' '} + Click here to view it. +
+ ) +} diff --git a/frontend/src/scenes/saved-insights/SavedInsights.tsx b/frontend/src/scenes/saved-insights/SavedInsights.tsx index 05f4c5c131668..bd155048b0490 100644 --- a/frontend/src/scenes/saved-insights/SavedInsights.tsx +++ b/frontend/src/scenes/saved-insights/SavedInsights.tsx @@ -57,6 +57,7 @@ import { NodeKind } from '~/queries/schema' import { isNodeWithSource } from '~/queries/utils' import { ActivityScope, InsightType, LayoutView, QueryBasedInsightModel, SavedInsightsTabs } from '~/types' +import { ReloadInsight } from './ReloadInsight' import { INSIGHTS_PER_PAGE, savedInsightsLogic } from './savedInsightsLogic' interface NewInsightButtonProps { @@ -671,6 +672,7 @@ export function SavedInsights(): JSX.Element { ) : ( <> + {layoutView === LayoutView.List ? ( combineUrl('/insights/new', dashboardId ? { dashboard: dashboardId } : {}, { ...(type ? { insight: type } : {}), - ...(query ? { q: typeof query === 'string' ? query : JSON.stringify(query) } : {}), + // have to use JSONCrush directly rather than the util to avoid circular dep + ...(query ? { q: typeof query === 'string' ? query : JSONCrush.crush(JSON.stringify(query)) } : {}), }).url, insightNewHogQL: (query: string, filters?: HogQLFilters): string => combineUrl( diff --git a/jest.config.ts b/jest.config.ts index 53e5df5943413..39ab8232a5592 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -8,7 +8,7 @@ process.env.TZ = process.env.TZ || 'UTC' * https://jestjs.io/docs/en/configuration.html */ -const esmModules = ['query-selector-shadow-dom', 'react-syntax-highlighter', '@react-hook', '@medv', 'monaco-editor'] +const esmModules = ['query-selector-shadow-dom', 'react-syntax-highlighter', '@react-hook', '@medv', 'monaco-editor', 'jsoncrush'] const eeFolderExists = fs.existsSync('ee/frontend/exports.ts') function rootDirectories() { const rootDirectories = ['/frontend/src'] diff --git a/package.json b/package.json index 64abf39fe9892..7dfc6b33f2a21 100644 --- a/package.json +++ b/package.json @@ -140,6 +140,7 @@ "hls.js": "^1.5.15", "husky": "^7.0.4", "image-blob-reduce": "^4.1.0", + "jsoncrush": "^1.1.8", "kea": "^3.1.5", "kea-forms": "^3.2.0", "kea-loaders": "^3.0.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c8e2b60ea24d2..cabf935d23bd6 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -241,6 +241,9 @@ dependencies: image-blob-reduce: specifier: ^4.1.0 version: 4.1.0 + jsoncrush: + specifier: ^1.1.8 + version: 1.1.8 kea: specifier: ^3.1.5 version: 3.1.5(react@18.2.0) @@ -15412,6 +15415,10 @@ packages: resolution: {integrity: sha512-gfFQZrcTc8CnKXp6Y4/CBT3fTc0OVuDofpre4aEeEpSBPV5X5v4+Vmx+8snU7RLPrNHPKSgLxGo9YuQzz20o+w==} dev: true + /jsoncrush@1.1.8: + resolution: {integrity: sha512-lvIMGzMUA0fjuqwNcxlTNRq2bibPZ9auqT/LyGdlR5hvydJtA/BasSgkx4qclqTKVeTidrJvsS/oVjlTCPQ4Nw==} + dev: false + /jsonfile@6.1.0: resolution: {integrity: sha512-5dgndWOriYSm5cnYaJNhalLNDKOqFwyDB/rr1E9ZsGciGvKPs8R2xYGCacuf3z6K1YKDz182fd+fY3cn3pMqXQ==} dependencies: @@ -18520,7 +18527,7 @@ packages: react: '>=15' dependencies: react: 18.2.0 - unlayer-types: 1.182.0 + unlayer-types: 1.185.0 dev: false /react-error-boundary@3.1.4(react@18.2.0): @@ -21107,8 +21114,8 @@ packages: resolution: {integrity: sha512-hAZsKq7Yy11Zu1DE0OzWjw7nnLZmJZYTDZZyEFHZdUhV8FkH5MCfoU1XMaxXovpyW5nq5scPqq0ZDP9Zyl04oQ==} engines: {node: '>= 10.0.0'} - /unlayer-types@1.182.0: - resolution: {integrity: sha512-x+YSeA7/Wb/znKDtRws8M3Mu6TyKP3d+MddPVX/iUyDPVEOapoPWk0QxjIaNYtWt6troADZdhzgr2EwsZ61HrA==} + /unlayer-types@1.185.0: + resolution: {integrity: sha512-GP5JbjJ1sqxEeAHh5QrOJXEg3os3qyuuN04IVo7pNr3uY14p5CfTkWh+oxMIA4tWKvf69KS3B6/KSIn53fTf6A==} dev: false /unpipe@1.0.0: