feat: insight with dashboard filters #24745

anirudhpillai · 2024-09-02T12:55:09Z

Problem

Fixes #22939
Later want to enhance this so that when 'previewing' a filter change on dashboard, you can still click into the insight and view the temporary filters.

Changes

When clicking into an insight from dashboard, the insight view loads with the filters from the dashboard.
Works by putting the filters override as query params in the URL.

Screen.Recording.2024-09-02.at.13.53.05.mov

Does this work well for both Cloud and self-hosted?

N/A

How did you test this code?

WIP

github-actions · 2024-09-02T13:05:30Z

Size Change: +2.51 kB (+0.23%)

Total Size: 1.1 MB

Filename	Size	Change
`frontend/dist/toolbar.js`	1.1 MB	+2.51 kB (+0.23%)

_{compressed-size-action}

skoob13

Kudos for e2e tests! Generally LGTM. There is one thing though: when I click the insight's name and go to the insight view, dashboard filters apply as expected. However, if I go to the insight by clicking on options (three dots), the dashboard filters don't apply. See the GIF.

skoob13 · 2024-09-04T09:52:57Z

frontend/src/scenes/urls.ts

@@ -90,7 +90,8 @@ export const urls = {
            }
        ).url,
    insightEdit: (id: InsightShortId): string => `/insights/${id}/edit`,
-    insightView: (id: InsightShortId): string => `/insights/${id}`,
+    insightView: (id: InsightShortId, filtersOverride?: DashboardFilter): string =>
+        `/insights/${id}${filtersOverride !== undefined ? `?filters_override=${JSON.stringify(filtersOverride)}` : ''}`,


I think you need to encode the filters_override to add escaping. You can use either encodeURIComponent or the URLSearchParams class.

skoob13 · 2024-09-04T09:56:53Z

posthog/schema.py

@@ -5607,6 +5607,7 @@ class QueryRequest(BaseModel):
        discriminator="kind",
    )
    refresh: Optional[Union[bool, str]] = None
+    filters_override: Optional[DashboardFilter] = None


Did you manually modify the schema.py file? It's autogenerated, so you need to update schema.ts in order to change it. Then, build a new schema with pnpm schema:build. It will generate new schema.json and schema.py files.

aah amazing thanks for pointing these out! Was just wondering why they get removed on the CI

posthog-bot · 2024-09-04T13:16:51Z

📸 UI snapshots have been updated

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

chromium: 0 added, 10 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-09-04T19:44:50Z

📸 UI snapshots have been updated

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

chromium: 0 added, 10 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-09-04T21:41:23Z

📸 UI snapshots have been updated

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

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

.github/workflows/storybook-chromatic.yml

Twixes · 2024-09-05T19:21:35Z

frontend/src/queries/schema.ts

@@ -1149,6 +1149,7 @@ export interface QueryRequest {
     * see the [PostHog HogQL documentation](/docs/hogql#api-access).
     */
    query: QuerySchema
+    filters_override?: DashboardFilter


This is not actually an override, since it adds to the existing filters, not replace. How about literally dashboard_filters?

Should I change this just for /query endpoint?
As for /insight we already have logic to read filters_override passed down from the /dashboard endpoint here, seems a bit unnecessary to read both filters_override for dashboard filters and dashboard_filters when loading an insight.

another option is to rename all of it to dashboard_filters_override or something, can address that in a separate PR as this will become a lot bigger then :)

Oh, didn't see the pre-existing filters_override, in that case does seem simplest to just reuse that

Twixes · 2024-09-05T19:23:32Z

frontend/src/scenes/dashboard/DashboardEditBar.tsx

+        <div
+            className={clsx(
+                'flex gap-2 items-center justify-between flex-wrap border',
+                dashboardMode === DashboardMode.Edit
+                    ? '-m-1.5 p-1.5 border-border-bold border-dashed rounded-lg'
+                    : 'border-transparent'
+            )}
+        >


Just making sure since we removed this previously: is this addition of the outline intended?

Yes, this is for the scenario when user changes filters then clicks into an insight and then navigates back to the dashboard.
At this stage the dashboard is in edit mode, but want to highlight the dates/filters so the user knows what's changed in edit mode if they hit save

Twixes · 2024-09-05T19:28:43Z

posthog/api/query.py

+        if data.filters_override is not None:
+            data.query = apply_dashboard_filters_to_dict(
+                data.query.model_dump(), data.filters_override.model_dump(), self.team
+            )  # type: ignore


cast() would better signal intentions than ignore

might be missing a simple way to do this but
the below doesn't work as data.query has to be one of

and the generated schema.py pulls all of the types as a Union here

data.query = cast( QuerySchemaRoot, apply_dashboard_filters_to_dict(data.query.model_dump(), data.filters_override.model_dump(), self.team), )

Would be great if there was a way to cast it as below

data.query = cast( QueryRequest["query"], apply_dashboard_filters_to_dict(data.query.model_dump(), data.filters_override.model_dump(), self.team), )

Felt the type: ignore was cleaner than pulling the full union which might go out sync, but ideally will be great if there's a way around this

That's tough indeed… it's definitely a nit, so let's skip

frontend/src/scenes/insights/Insight.tsx

Twixes · 2024-09-05T19:57:28Z

Took a glance, a few nits, but feels really good!

posthog-bot · 2024-09-10T10:37:16Z

📸 UI snapshots have been updated

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

chromium: 0 added, 10 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-09-10T11:07:39Z

📸 UI snapshots have been updated

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

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

…Hog/posthog into feat/insight-with-dashboard-filters

posthog-bot · 2024-09-10T13:09:00Z

📸 UI snapshots have been updated

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

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

Triggered by this commit.

👉 Review this PR's diff of snapshots.

…Hog/posthog into feat/insight-with-dashboard-filters

…-with-dashboard-filters

anirudhpillai requested a review from a team September 2, 2024 12:55

anirudhpillai force-pushed the feat/insight-with-dashboard-filters branch from 4b9522e to df2f5f9 Compare September 3, 2024 19:25

skoob13 approved these changes Sep 4, 2024

View reviewed changes

anirudhpillai force-pushed the feat/insight-with-dashboard-filters branch from df2f5f9 to e64b83d Compare September 4, 2024 12:13

anirudhpillai added 10 commits September 4, 2024 17:32

feat:dashboard filters on insight

9352003

Banner when filter override on insight

f78d26d

reload insight on insight view

c15646b

tests

7ff33d3

filtersOverride through props

dd2c4a9

lint

9252c3e

lint

b09ea76

update schema

e51bf67

border around filter bar as you can come back to dashboard in edit mode

542664e

waiting on cypress

1ce8a37

anirudhpillai force-pushed the feat/insight-with-dashboard-filters branch from 7e1858c to 1ce8a37 Compare September 4, 2024 19:10

merge master

9467653

fix cypress

1986fff

anirudhpillai force-pushed the feat/insight-with-dashboard-filters branch from 863c389 to 1986fff Compare September 4, 2024 21:10

test temp timeout increase for storybook

1ebe217

anirudhpillai force-pushed the feat/insight-with-dashboard-filters branch from 5164a38 to 1ebe217 Compare September 5, 2024 10:51

Twixes reviewed Sep 5, 2024

View reviewed changes

frontend/src/scenes/insights/Insight.tsx Outdated Show resolved Hide resolved

anirudhpillai added 2 commits September 10, 2024 11:04

merge master

9690e93

comments

1566f8a

anirudhpillai force-pushed the feat/insight-with-dashboard-filters branch from 9da2bdd to 1566f8a Compare September 10, 2024 10:23

Update UI snapshots for chromium (1)

00da3cb

anirudhpillai and others added 4 commits September 10, 2024 13:48

mock query call in storybook

8d3316a

Merge branch 'feat/insight-with-dashboard-filters' of github.com:Post…

60e1ef4

…Hog/posthog into feat/insight-with-dashboard-filters

Update UI snapshots for webkit (2)

bd6a778

Update UI snapshots for chromium (2)

fe86de9

anirudhpillai added 2 commits September 10, 2024 15:21

update tests

93ff9a3

Merge branch 'feat/insight-with-dashboard-filters' of github.com:Post…

9e281ab

…Hog/posthog into feat/insight-with-dashboard-filters

anirudhpillai requested a review from Twixes September 11, 2024 07:54

anirudhpillai added 2 commits September 11, 2024 15:17

Merge branch 'master' of github.com:PostHog/posthog into feat/insight…

a1be877

…-with-dashboard-filters

fix test in diff pr

f96eb44

anirudhpillai enabled auto-merge (squash) September 12, 2024 11:37

anirudhpillai disabled auto-merge September 12, 2024 11:53

anirudhpillai added 2 commits September 12, 2024 13:26

pausa

664546a

skip test for now

2464f84

anirudhpillai enabled auto-merge (squash) September 12, 2024 12:51

anirudhpillai merged commit d0a8627 into master Sep 12, 2024
91 checks passed

anirudhpillai deleted the feat/insight-with-dashboard-filters branch September 12, 2024 14:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: insight with dashboard filters #24745

feat: insight with dashboard filters #24745

anirudhpillai commented Sep 2, 2024 •

edited

Loading

github-actions bot commented Sep 2, 2024 •

edited

Loading

skoob13 left a comment

skoob13 Sep 4, 2024

skoob13 Sep 4, 2024

anirudhpillai Sep 4, 2024

posthog-bot commented Sep 4, 2024

posthog-bot commented Sep 4, 2024

posthog-bot commented Sep 4, 2024

Twixes Sep 5, 2024

anirudhpillai Sep 10, 2024

anirudhpillai Sep 10, 2024

Twixes Sep 12, 2024

Twixes Sep 5, 2024

anirudhpillai Sep 10, 2024

Twixes Sep 5, 2024

anirudhpillai Sep 10, 2024 •

edited

Loading

Twixes Sep 12, 2024

Twixes commented Sep 5, 2024

posthog-bot commented Sep 10, 2024

posthog-bot commented Sep 10, 2024

posthog-bot commented Sep 10, 2024

feat: insight with dashboard filters #24745

feat: insight with dashboard filters #24745

Conversation

anirudhpillai commented Sep 2, 2024 • edited Loading

Problem

Changes

Does this work well for both Cloud and self-hosted?

How did you test this code?

github-actions bot commented Sep 2, 2024 • edited Loading

skoob13 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

posthog-bot commented Sep 4, 2024

📸 UI snapshots have been updated

posthog-bot commented Sep 4, 2024

📸 UI snapshots have been updated

posthog-bot commented Sep 4, 2024

📸 UI snapshots have been updated

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anirudhpillai Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Twixes commented Sep 5, 2024

posthog-bot commented Sep 10, 2024

📸 UI snapshots have been updated

posthog-bot commented Sep 10, 2024

📸 UI snapshots have been updated

posthog-bot commented Sep 10, 2024

📸 UI snapshots have been updated

anirudhpillai commented Sep 2, 2024 •

edited

Loading

github-actions bot commented Sep 2, 2024 •

edited

Loading

anirudhpillai Sep 10, 2024 •

edited

Loading