Insight hash url (#8228)

* store insight filters in the #filters= * test for legacy url redirect, update new paths in urls * link to short urls from saved insights
PostHog · Jan 24, 2022 · df48e8c · df48e8c
1 parent c6de4cd
commit df48e8c
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 26 deletions.
diff --git a/frontend/src/scenes/insights/insightLogic.test.ts b/frontend/src/scenes/insights/insightLogic.test.ts
@@ -412,15 +412,27 @@ describe('insightLogic', () => {
                 .delay(1)
                 .toMatchValues({
                     location: partial({ pathname: urls.insightEdit(Insight42) }),
-                    searchParams: partial({ insight: 'TRENDS' }),
+                    hashParams: {},
                 })
 
             router.actions.push(urls.insightNew({ insight: InsightType.FUNNELS }))
             await expectLogic(router)
                 .delay(1)
                 .toMatchValues({
                     location: partial({ pathname: urls.insightEdit(Insight43) }),
-                    searchParams: partial({ insight: 'FUNNELS' }),
+                    hashParams: { filters: partial({ insight: 'FUNNELS' }) },
+                })
+        })
+
+        it('redirects when opening with legacy filter in searchParams', async () => {
+            // open url with ?insight=FUNNELS
+            router.actions.push(combineUrl(urls.insightEdit(Insight42), { insight: 'RETENTION' }).url)
+            await expectLogic(router)
+                .delay(1)
+                .toMatchValues({
+                    location: partial({ pathname: urls.insightEdit(Insight42) }),
+                    searchParams: {},
+                    hashParams: { filters: partial({ insight: InsightType.RETENTION }) },
                 })
         })
 
@@ -488,7 +500,7 @@ describe('insightLogic', () => {
                     logic.actionCreators.setFilters({ insight: InsightType.TRENDS, interval: 'hour' }),
                 ])
                 .toDispatchActions(router, ['replace', 'locationChanged'])
-                .toMatchValues(router, { searchParams: partial({ interval: 'hour' }) })
+                .toMatchValues(router, { hashParams: { filters: partial({ interval: 'hour' }) } })
 
             // no change in filters, doesn't change the URL
             logic.actions.setFilters({ insight: InsightType.TRENDS, interval: 'hour' })
@@ -497,13 +509,13 @@ describe('insightLogic', () => {
                     logic.actionCreators.setFilters({ insight: InsightType.TRENDS, interval: 'hour' }),
                 ])
                 .toNotHaveDispatchedActions(router, ['replace', 'locationChanged'])
-                .toMatchValues(router, { searchParams: partial({ interval: 'hour' }) })
+                .toMatchValues(router, { hashParams: { filters: partial({ interval: 'hour' }) } })
 
             logic.actions.setFilters({ insight: InsightType.TRENDS, interval: 'month' })
             await expectLogic(router)
                 .toDispatchActions(['replace', 'locationChanged'])
                 .toMatchValues({
-                    searchParams: partial({ insight: InsightType.TRENDS, interval: 'month' }),
+                    hashParams: { filters: partial({ insight: InsightType.TRENDS, interval: 'month' }) },
                 })
         })
 

diff --git a/frontend/src/scenes/insights/insightLogic.tsx b/frontend/src/scenes/insights/insightLogic.tsx
@@ -773,19 +773,19 @@ export const insightLogic = kea<insightLogicType>({
     urlToAction: ({ actions, values }) => ({
         '/insights/:shortId(/:mode)': (params, searchParams, hashParams) => {
             if (values.syncWithUrl) {
-                if (searchParams.insight === 'HISTORY') {
-                    // Legacy redirect because the insight history scene was toggled via the insight type.
-                    router.actions.replace(urls.savedInsights())
-                    return
-                }
+                const filters =
+                    (typeof hashParams?.filters === 'object' ? hashParams?.filters : null) ??
+                    // Legacy: we used to store the filter as searchParams = { insight: 'TRENDS', ...otherFilters }
+                    ('insight' in searchParams ? cleanFilters(searchParams) : null)
+
                 if (params.shortId === 'new') {
-                    actions.createAndRedirectToNewInsight(searchParams)
+                    actions.createAndRedirectToNewInsight(filters)
                     return
                 }
                 const insightId = params.shortId ? (String(params.shortId) as InsightShortId) : null
                 if (!insightId) {
                     // only allow editing insights with IDs for now
-                    router.actions.replace(urls.insightNew(searchParams))
+                    router.actions.replace(urls.insightNew(filters))
                     return
                 }
 
@@ -807,14 +807,16 @@ export const insightLogic = kea<insightLogicType>({
                     actions.loadInsight(insightId)
                 }
 
-                const cleanSearchParams = cleanFilters(searchParams, values.filters, values.featureFlags)
-                const insightModeFromUrl = params['mode'] === 'edit' ? ItemMode.Edit : ItemMode.View
+                if (filters !== null) {
+                    const cleanSearchParams = cleanFilters(filters, values.filters, values.featureFlags)
+                    const insightModeFromUrl = params['mode'] === 'edit' ? ItemMode.Edit : ItemMode.View
 
-                if (
-                    (!loadedFromAnotherLogic && !objectsEqual(cleanSearchParams, values.filters)) ||
-                    insightModeFromUrl !== values.insightMode
-                ) {
-                    actions.setFilters(cleanSearchParams, insightModeFromUrl)
+                    if (
+                        (!loadedFromAnotherLogic && !objectsEqual(cleanSearchParams, values.filters)) ||
+                        insightModeFromUrl !== values.insightMode
+                    ) {
+                        actions.setFilters(cleanSearchParams, insightModeFromUrl)
+                    }
                 }
             }
         },

diff --git a/frontend/src/scenes/saved-insights/SavedInsights.tsx b/frontend/src/scenes/saved-insights/SavedInsights.tsx
@@ -186,7 +186,7 @@ export function SavedInsights(): JSX.Element {
                 return (
                     <>
                         <div style={{ display: 'flex', alignItems: 'center' }}>
-                            <Link to={urls.insightView(insight.short_id, insight.filters)} className="row-name">
+                            <Link to={urls.insightView(insight.short_id)} className="row-name">
                                 {name || <i>{UNNAMED_INSIGHT_NAME}</i>}
                             </Link>
                             <div

diff --git a/frontend/src/scenes/saved-insights/savedInsightsLogic.test.ts b/frontend/src/scenes/saved-insights/savedInsightsLogic.test.ts
@@ -150,7 +150,7 @@ describe('savedInsightsLogic', () => {
                 .delay(1)
                 .toMatchValues({
                     location: partial({ pathname: urls.insightView(Insight42) }),
-                    searchParams: partial({ insight: InsightType.TRENDS }),
+                    hashParams: { filters: partial({ insight: InsightType.TRENDS }) },
                 })
         })
 
@@ -162,15 +162,15 @@ describe('savedInsightsLogic', () => {
                 .delay(1)
                 .toMatchValues({
                     location: partial({ pathname: urls.insightEdit(Insight42) }),
-                    searchParams: partial({ insight: InsightType.TRENDS }),
+                    hashParams: { filters: partial({ insight: InsightType.TRENDS }) },
                 })
         })
 
         it('new mode with ?insight= and no hash params', async () => {
             router.actions.push(combineUrl('/insights', cleanFilters({ insight: InsightType.FUNNELS })).url)
             await expectLogic(router).toMatchValues({
                 location: partial({ pathname: urls.insightNew() }),
-                searchParams: partial({ insight: InsightType.FUNNELS }),
+                hashParams: { filters: partial({ insight: InsightType.FUNNELS }) },
             })
         })
     })

diff --git a/frontend/src/scenes/urls.ts b/frontend/src/scenes/urls.ts
@@ -13,12 +13,13 @@ export const urls = {
     eventStats: () => '/events/stats',
     eventPropertyStats: () => '/events/properties',
     events: () => '/events',
-    insightNew: (filters?: Partial<FilterType>) => `/insights/new${filters ? combineUrl('', filters).search : ''}`,
+    insightNew: (filters?: Partial<FilterType>) =>
+        `/insights/new${filters ? combineUrl('', '', { filters }).hash : ''}`,
     insightRouter: (id: string) => `/i/${id}`,
     insightEdit: (id: InsightShortId, filters?: Partial<FilterType>) =>
-        `/insights/${id}/edit${filters ? combineUrl('', filters).search : ''}`,
+        `/insights/${id}/edit${filters ? combineUrl('', '', { filters }).hash : ''}`,
     insightView: (id: InsightShortId, filters?: Partial<FilterType>) =>
-        `/insights/${id}${filters ? combineUrl('', filters).search : ''}`,
+        `/insights/${id}${filters ? combineUrl('', '', { filters }).hash : ''}`,
     savedInsights: () => '/insights',
     webPerformance: () => '/web-performance',
     sessionRecordings: () => '/recordings',