refactor(insights): remove filter based insights frontend side #24076

thmsobrmlr · 2024-07-30T11:53:46Z

Problem

We still have filters in the frontend.

Changes

removes legacyInsight usage, with exception of api handlers where we still get the legacy insights
removes InsightModel in the same way
renames queryBasedInsight to insight
changes the readAsQuery option from false to true everywhere -> the option will be removed in a follow up

Follow-up

remove readAsQuery flag refactor(insights): clean up after query based insights refactor #24409
convert experiments to HogQL and query based insights
refactor ActionFilter to be based on query, not filters
remove all occurances of cleanFilters

How did you test this code?

github-actions · 2024-08-13T13:26:41Z

Size Change: +3.44 kB (+0.32%)

Total Size: 1.08 MB

Filename	Size	Change
`frontend/dist/toolbar.js`	1.08 MB	+3.44 kB (+0.32%)

_{compressed-size-action}

posthog-bot · 2024-08-15T21:15:09Z

📸 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)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-08-15T21:33:25Z

📸 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-15T21:51:50Z

📸 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)
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-16T07:46:17Z

📸 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)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-08-16T08:04:23Z

📸 UI snapshots have been updated

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

chromium: 0 added, 1 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-16T08:22:34Z

📸 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)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-08-16T08:40:42Z

📸 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-16T08:58:44Z

📸 UI snapshots have been updated

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

chromium: 0 added, 1 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-16T09:16:49Z

📸 UI snapshots have been updated

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

chromium: 0 added, 1 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.

webjunkie

A dizzying amount of changes 😵‍💫

I skimmed things and it looks alright. Just one question.

webjunkie · 2024-08-19T13:53:46Z

frontend/src/scenes/saved-insights/savedInsightsLogic.ts

@@ -101,43 +102,51 @@ export const savedInsightsLogic = kea<savedInsightsLogicType>([
                    basic: true,
                }

-                const response = await api.get(
+                const legacyResponse: CountedPaginatedResponse<InsightModel> = await api.get(


Wondering why this would be a legacy response... wouldn't this also respond with a query based insight if it's updated on the backend-side?

Yep, you're right - this will be a query based insight if it is already stored with a query on the backend. legacyInsight in this context means an insight that is potentially a legacy one. The conversion function will give precedence to a query already present on an insight.

Alright, good to know.

thmsobrmlr · 2024-08-19T14:20:15Z

A dizzying amount of changes 😵‍💫

I skimmed things and it looks alright. Just one question.

Unfortunately I haven't been able to slice-and-dice this any further :/

webjunkie

Unfortunately I haven't been able to slice-and-dice this any further :/

No worries, this was more meant as appreciation of this ongoing diligent work.

anirudhpillai · 2024-08-19T14:47:46Z

frontend/src/scenes/dashboard/dashboardLogic.tsx

-                        return await api.update(`api/projects/${values.currentTeamId}/dashboards/${props.id}`, {
-                            filters: values.filters,
-                        })
+                        const dashboard: DashboardType<InsightModel> = await api.update(


probably a dumb question, why do all API responses still return InsightModel and not QueryBasedInsightModel? Do we need to wait on that till we run the db migration script

No it's a good question. Basically QueryBasedInsightModel is everything in InsightModel, except filters. This helped me see where we were still accessing filters in a converted model.

The reason that the backend endpoint are still typed as InsightModel, is because they actually are. The backend side conversion is totally separate from the frontend side one. And you're right, we can change them once we ran the db migration script.

anirudhpillai · 2024-08-19T14:49:36Z

frontend/src/scenes/dashboard/dashboardLogic.tsx

@@ -1034,7 +1049,7 @@ export const dashboardLogic = kea<dashboardLogicType>([
                            'force_cache',
                            methodOptions
                        )
-                        dashboardsModel.actions.updateDashboardInsight(polledInsight)
+                        dashboardsModel.actions.updateDashboardInsight(polledInsight!)


not following why we suddenly need the non null assertion here 😅

I changed the type of the getSingleInsight function (see line 134) above to be more in line with what it actually returns. The insight is only null if something went wrong and I didn't add error handling (we didn't have it before as well).

aah missed that, thought it was unchanged. Thanks for explaining!

# Conflicts: # frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark.png

thmsobrmlr changed the title ~~use query based insight in insight scene~~ refactor(insights): remove filter based insights frontend side Jul 30, 2024

thmsobrmlr force-pushed the remove-insight-model branch 2 times, most recently from c5c9543 to 5a40f2f Compare August 2, 2024 08:51

Base automatically changed from remove-filter-based-card-3 to master August 5, 2024 07:14

thmsobrmlr force-pushed the remove-insight-model branch from 0c76cfa to c38d7d2 Compare August 5, 2024 07:22

posthog-bot added the stale label Aug 13, 2024

thmsobrmlr force-pushed the remove-insight-model branch from 830a48b to 27c601d Compare August 13, 2024 10:21

thmsobrmlr removed the stale label Aug 13, 2024

PostHog deleted a comment from posthog-bot Aug 13, 2024

PostHog deleted a comment from github-actions bot Aug 13, 2024

thmsobrmlr force-pushed the remove-insight-model branch from 8c60cb1 to a1dd6cd Compare August 13, 2024 13:31

thmsobrmlr added 18 commits August 14, 2024 09:07

use query based insight in insight scene

6adcb69

use query based insight for saved insights

82beaed

use query based insights in 3000 sidebar

05ceb0e

more query based insights

c7cced1

use query based insight model in recent insights

ad24154

use query based insights for feature flag related insights

93be143

wip

92ee49b

wip

109e2c9

wip

287a393

fixes

dedc692

more fixes

483903e

some work on dashboards

a2415c6

move getQueryBasedDashboard

26c31ae

fix shared dashboards

0048302

more fixes

8c217d1

types

bca8837

adapt generating notebooks from dashboards

e0b7bc6

types and fixes

878412a

Update UI snapshots for chromium (1)

01067a7

Update UI snapshots for chromium (1)

b8262ba

thmsobrmlr and others added 2 commits August 16, 2024 09:27

Merge branch 'master' into remove-insight-model

f40fcf6

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

Update UI snapshots for chromium (1)

f5c5b45

Update UI snapshots for chromium (1)

d972034

thmsobrmlr marked this pull request as ready for review August 16, 2024 08:13

thmsobrmlr requested a review from a team August 16, 2024 08:14

Update UI snapshots for chromium (1)

c226330

Update UI snapshots for chromium (1)

7fbb82e

Update UI snapshots for chromium (1)

d8166c4

Update UI snapshots for chromium (1)

945e67d

add comment

d7d2828

webjunkie reviewed Aug 19, 2024

View reviewed changes

webjunkie approved these changes Aug 19, 2024

View reviewed changes

anirudhpillai reviewed Aug 19, 2024

View reviewed changes

thmsobrmlr enabled auto-merge (squash) August 20, 2024 07:38

Merge branch 'master' into remove-insight-model

75b3d09

# Conflicts: # frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark.png

thmsobrmlr merged commit c5dc3b7 into master Aug 20, 2024
93 checks passed

thmsobrmlr deleted the remove-insight-model branch August 20, 2024 08:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(insights): remove filter based insights frontend side #24076

refactor(insights): remove filter based insights frontend side #24076

thmsobrmlr commented Jul 30, 2024 •

edited

Loading

github-actions bot commented Aug 13, 2024 •

edited

Loading

posthog-bot commented Aug 15, 2024

posthog-bot commented Aug 15, 2024

posthog-bot commented Aug 15, 2024

posthog-bot commented Aug 16, 2024

posthog-bot commented Aug 16, 2024

posthog-bot commented Aug 16, 2024

posthog-bot commented Aug 16, 2024

posthog-bot commented Aug 16, 2024

posthog-bot commented Aug 16, 2024

webjunkie left a comment

webjunkie Aug 19, 2024

thmsobrmlr Aug 19, 2024 •

edited

Loading

webjunkie Aug 19, 2024

thmsobrmlr commented Aug 19, 2024

webjunkie left a comment

anirudhpillai Aug 19, 2024

thmsobrmlr Aug 19, 2024

anirudhpillai Aug 19, 2024

thmsobrmlr Aug 19, 2024

anirudhpillai Aug 19, 2024

refactor(insights): remove filter based insights frontend side #24076

refactor(insights): remove filter based insights frontend side #24076

Conversation

thmsobrmlr commented Jul 30, 2024 • edited Loading

Problem

Changes

Follow-up

How did you test this code?

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

posthog-bot commented Aug 15, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 15, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 15, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 16, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 16, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 16, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 16, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 16, 2024

📸 UI snapshots have been updated

posthog-bot commented Aug 16, 2024

📸 UI snapshots have been updated

webjunkie left a comment

Choose a reason for hiding this comment

webjunkie Aug 19, 2024

Choose a reason for hiding this comment

thmsobrmlr Aug 19, 2024 • edited Loading

Choose a reason for hiding this comment

webjunkie Aug 19, 2024

Choose a reason for hiding this comment

thmsobrmlr commented Aug 19, 2024

webjunkie left a comment

Choose a reason for hiding this comment

anirudhpillai Aug 19, 2024

Choose a reason for hiding this comment

thmsobrmlr Aug 19, 2024

Choose a reason for hiding this comment

anirudhpillai Aug 19, 2024

Choose a reason for hiding this comment

thmsobrmlr Aug 19, 2024

Choose a reason for hiding this comment

anirudhpillai Aug 19, 2024

Choose a reason for hiding this comment

thmsobrmlr commented Jul 30, 2024 •

edited

Loading

github-actions bot commented Aug 13, 2024 •

edited

Loading

thmsobrmlr Aug 19, 2024 •

edited

Loading