refactor(insights): move to queries in new insight urls #24193

thmsobrmlr · 2024-08-05T15:05:32Z

Problem

We'd like to get rid of filters, but still have them in URLs generated internally.

Changes

Replaces all internal links to #filters= type urls with query based #q=... type urls.
Drops support for old ?filters=... type urls.
Changes the "Leave insight?" confirmation for new insights to be based on a reducer that stores the query when first navigating to the insight. This is so that when navigating to a new insight e.g. from web analytics you can freely navigate away if you haven't changed anything.

How did you test this code?

Create a new insight from the dropdown menu on the saved insights page for each insight type
Create a new insight from the buttons on the web analytics page
Create a new insight by visiting a URL with a query in hash params in a new browser window

posthog-bot · 2024-08-05T15:22:33Z

📸 UI snapshots have been updated

3 snapshot changes in total. 0 added, 3 modified, 0 deleted:

chromium: 0 added, 3 modified, 0 deleted (diff for shard 1, diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

github-actions · 2024-08-06T08:17:43Z

Size Change: +241 B (+0.02%)

Total Size: 1.07 MB

ℹ️ View Unchanged

Filename	Size	Change
`frontend/dist/toolbar.js`	1.07 MB	+241 B (+0.02%)

_{compressed-size-action}

posthog-bot · 2024-08-06T08:39:53Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-08-06T08:58:20Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-08-06T09:16:36Z

📸 UI snapshots have been updated

4 snapshot changes in total. 0 added, 4 modified, 0 deleted:

chromium: 0 added, 4 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-08-07T13:56:35Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

# Conflicts: # frontend/__snapshots__/exporter-exporter--user-paths-insight--light.png

posthog-bot · 2024-08-08T09:42:14Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-08-08T10:00:46Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

thmsobrmlr · 2024-08-08T12:03:34Z

frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx

@@ -1226,20 +1228,12 @@ export const webAnalyticsLogic = kea<webAnalyticsLogicType>([
                        if (!tab) {
                            throw new Error('Developer Error, tab not found')
                        }
-                        return urls.insightNew(
-                            { properties: webAnalyticsFilters, date_from: dateFrom, date_to: dateTo },


@robbie-c I couldn't figure out what adding the properties here does. Any hints for me?

It should copy the property filters from web analytics into the new insight

Not sure if I understand this correctly, but property filters should already be part of the queries set on the new insight. Let me know if this breaks anything that I don't see right now and I'm happy to adapt accordingly.

Ah you're right, looks like it wasn't necessary. I suspect I just passed these arguments in because they were there :) Just ran these changes locally and they worked fine, so all good from me

posthog-bot · 2024-08-08T12:19:45Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-08-08T12:38:17Z

📸 UI snapshots have been updated

4 snapshot changes in total. 0 added, 4 modified, 0 deleted:

chromium: 0 added, 4 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

# Conflicts: # frontend/__snapshots__/exporter-exporter--user-paths-insight--dark.png # frontend/__snapshots__/exporter-exporter--user-paths-insight--light.png

posthog-bot · 2024-08-08T14:30:49Z

📸 UI snapshots have been updated

4 snapshot changes in total. 0 added, 4 modified, 0 deleted:

chromium: 0 added, 4 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-08-08T14:48:55Z

📸 UI snapshots have been updated

4 snapshot changes in total. 0 added, 4 modified, 0 deleted:

chromium: 0 added, 4 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-08-08T15:07:14Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

# Conflicts: # frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx

anirudhpillai · 2024-08-12T16:32:40Z

frontend/src/scenes/insights/insightSceneLogic.tsx

@@ -190,7 +205,7 @@ export const insightSceneLogic = kea<insightSceneLogicType>([
        '/insights/:shortId(/:mode)(/:subscriptionId)': (
            { shortId, mode, subscriptionId }, // url params
            { dashboard, ...searchParams }, // search params
-            { filters: _filters, q }, // hash params
+            { insight: insightType, q }, // hash params


why do we want to support hash params and not just search params?

no reason in particular here - just taking over the existing system. hash params are quite common for client-side state and afaik there's a higher limit for the number of characters.

thmsobrmlr and others added 16 commits August 7, 2024 15:40

move to queries in new insight urls

35995c8

wip

ae6aa8f

feature flags sidebar

a541666

data table

93ce0c6

edit hogql button

e529709

more occurances

ffac5e7

funnel step more

708443d

feature flags

40da6ab

actions

dba4df0

recent feature flag insights

3d9ccf2

insight for event util

f13c6ca

remove legacy url support

c1478c1

Update UI snapshots for chromium (1)

fe4de63

Update UI snapshots for chromium (1)

55b8f32

Update UI snapshots for chromium (1)

19a5d58

fix feature flag in sidebar

a5023d8

thmsobrmlr force-pushed the insight-new-query branch from afe447b to a5023d8 Compare August 7, 2024 13:40

Update UI snapshots for chromium (1)

ea44bc0

thmsobrmlr and others added 4 commits August 8, 2024 10:31

extract getDefaultQuery

48f9e85

wip

61dcc2f

Merge branch 'master' into insight-new-query

5e8b6f1

# Conflicts: # frontend/__snapshots__/exporter-exporter--user-paths-insight--light.png

Update UI snapshots for chromium (1)

85fbb9f

adapt tests

068c9dc

Update UI snapshots for chromium (1)

2c2e2be

thmsobrmlr added 3 commits August 8, 2024 14:01

improve web analytics queries

92b60e4

remove urlToAction from insightNavLogic

3978fb5

fix data table

74f073d

thmsobrmlr commented Aug 8, 2024

View reviewed changes

Update UI snapshots for chromium (1)

eadca0e

Update UI snapshots for chromium (1)

2fd7bfe

thmsobrmlr and others added 3 commits August 8, 2024 16:02

use starting query for comparision of new insights

5f1b772

Merge branch 'master' into insight-new-query

c90610b

# Conflicts: # frontend/__snapshots__/exporter-exporter--user-paths-insight--dark.png # frontend/__snapshots__/exporter-exporter--user-paths-insight--light.png

Update UI snapshots for chromium (1)

ff7a8cd

Update UI snapshots for chromium (1)

eda6064

Update UI snapshots for chromium (1)

ca91620

Merge branch 'master' into insight-new-query

b01d23a

# Conflicts: # frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx

thmsobrmlr marked this pull request as ready for review August 9, 2024 14:27

thmsobrmlr requested a review from a team August 9, 2024 14:27

anirudhpillai reviewed Aug 12, 2024

View reviewed changes

anirudhpillai approved these changes Aug 12, 2024

View reviewed changes

thmsobrmlr merged commit f403b17 into master Aug 13, 2024
95 checks passed

thmsobrmlr deleted the insight-new-query branch August 13, 2024 06:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(insights): move to queries in new insight urls #24193

refactor(insights): move to queries in new insight urls #24193

thmsobrmlr commented Aug 5, 2024 •

edited

Loading

posthog-bot commented Aug 5, 2024

github-actions bot commented Aug 6, 2024 •

edited

Loading

posthog-bot commented Aug 6, 2024

posthog-bot commented Aug 6, 2024

posthog-bot commented Aug 6, 2024

posthog-bot commented Aug 7, 2024

posthog-bot commented Aug 8, 2024

posthog-bot commented Aug 8, 2024

thmsobrmlr Aug 8, 2024

robbie-c Aug 8, 2024

thmsobrmlr Aug 9, 2024

robbie-c Aug 12, 2024

posthog-bot commented Aug 8, 2024

posthog-bot commented Aug 8, 2024

posthog-bot commented Aug 8, 2024

posthog-bot commented Aug 8, 2024

posthog-bot commented Aug 8, 2024

anirudhpillai Aug 12, 2024

thmsobrmlr Aug 13, 2024

refactor(insights): move to queries in new insight urls #24193

refactor(insights): move to queries in new insight urls #24193

Conversation

thmsobrmlr commented Aug 5, 2024 • edited Loading

Problem

Changes

How did you test this code?

posthog-bot commented Aug 5, 2024

📸 UI snapshots have been updated

github-actions bot commented Aug 6, 2024 • edited Loading

posthog-bot commented Aug 6, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 6, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 6, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 7, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 8, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 8, 2024

📸 UI snapshots have been updated

thmsobrmlr Aug 8, 2024

Choose a reason for hiding this comment

robbie-c Aug 8, 2024

Choose a reason for hiding this comment

thmsobrmlr Aug 9, 2024

Choose a reason for hiding this comment

robbie-c Aug 12, 2024

Choose a reason for hiding this comment

posthog-bot commented Aug 8, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 8, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 8, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 8, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 8, 2024

📸 UI snapshots have been updated

anirudhpillai Aug 12, 2024

Choose a reason for hiding this comment

thmsobrmlr Aug 13, 2024

Choose a reason for hiding this comment

thmsobrmlr commented Aug 5, 2024 •

edited

Loading

github-actions bot commented Aug 6, 2024 •

edited

Loading