chore(insights): rename the value field in multiple breakdowns #23890

skoob13 · 2024-07-22T14:53:17Z

Problem

The PR renames the value to property field to improve backward compatibility with legacy multiple breakdowns because there are 17k+ saved funnels with the old shape.

Changes

Rename the field
Fix tests
Refactor cleaning filters to be backward compatible with legacy breakdowns (frontend)
Refactor conversion of filters to a query (backend)

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

Yes

How did you test this code?

Existing unit tests and manual testing.

posthog-bot · 2024-07-22T15:10:28Z

📸 UI snapshots have been updated

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

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

Triggered by this commit.

👉 Review this PR's diff of snapshots.

github-actions · 2024-07-22T15:20:51Z

Size Change: 0 B

Total Size: 1.07 MB

ℹ️ View Unchanged

Filename	Size
`frontend/dist/toolbar.js`	1.07 MB

_{compressed-size-action}

skoob13 · 2024-07-22T18:45:21Z

frontend/__snapshots__/exporter-exporter--funnel-top-to-bottom-breakdown-insight--dark.png

Snapshots are correct. They reverted to prior multiple breakdowns changes in Funnels: 7a3e849#diff-97ccf0cbc4b249f761838d67a3d9ccc7af51e8d5fbd01f6a871afcb22208c1a7.

…ostHog/posthog into chore/rename-field-multiple-breakdowns

skoob13 · 2024-07-23T08:59:00Z

@thmsobrmlr a few notes about the PR:

the value field was renamed to property
filter_to_query (backend):
- For Funnels: I reverted my changes here, so it extracts a first breakdown.
- For Trends: keeps multiple breakdowns.
clean_filters (frontend):
- For Funnels: keeps breakdowns as they were before.
- For Trends: keeps multiple breakdowns and only restores the normalize_url.

frontend/src/scenes/insights/utils/cleanFilters.ts

thmsobrmlr · 2024-07-23T09:08:04Z

For posterity, this gives us all insights from our team already using the trends multi-breakdowns:

SELECT filters->>'breakdowns', team_id, * FROM posthog_dashboarditem
WHERE filters->>'breakdowns' IS NOT NULL
AND filters->'breakdowns'->0->>'property' IS NULL

Co-authored-by: Thomas Obermüller <[email protected]>

posthog-bot · 2024-07-23T09:14: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-07-23T09:32:42Z

📸 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-07-23T09:50:51Z

📸 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.

Twixes · 2024-07-23T09:57:26Z

frontend/__snapshots__/scenes-app-dashboards--insight-legend--dark.png

LGTM if looks good to Thomas, just is it expected for this dashboard to be blank?

There is a fix for that: #23904. I merged the master branch, so let me check if it fixes the snapshot.

Indeed, it has fixed the snapshot.

…-field-multiple-breakdowns

…ostHog/posthog into chore/rename-field-multiple-breakdowns

posthog-bot · 2024-07-23T11:56:36Z

📸 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-07-23T12:16:04Z

📸 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-07-23T12:36:33Z

📸 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-07-23T12:54:53Z

📸 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-07-23T13:13:25Z

📸 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.

…og#23890) Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Thomas Obermüller <[email protected]>

skoob13 added 6 commits July 22, 2024 14:11

chore: schema

7cf8f18

chore: backend value -> property

4e0ae5e

chore: frontend value -> property

7b949c5

fix: cleanFIlters

9d95dcb

fix: tests

b736948

fix: update comments

82f25d0

skoob13 requested a review from a team July 22, 2024 14:53

github-actions bot added 3 commits July 22, 2024 15:01

Update UI snapshots for webkit (2)

33449a5

Update UI snapshots for chromium (1)

a52d5c5

Update UI snapshots for chromium (2)

cdc8e8c

skoob13 commented Jul 22, 2024

View reviewed changes

skoob13 added 2 commits July 23, 2024 10:48

fix: revert changes in filter_to_query for funnels

969f40f

Merge branch 'chore/rename-field-multiple-breakdowns' of github.com:P…

82630cc

…ostHog/posthog into chore/rename-field-multiple-breakdowns

thmsobrmlr reviewed Jul 23, 2024

View reviewed changes

frontend/src/scenes/insights/utils/cleanFilters.ts Outdated Show resolved Hide resolved

thmsobrmlr approved these changes Jul 23, 2024

View reviewed changes

skoob13 and others added 2 commits July 23, 2024 11:09

Update frontend/src/scenes/insights/utils/cleanFilters.ts

79e59d5

Co-authored-by: Thomas Obermüller <[email protected]>

Update UI snapshots for chromium (1)

c0f48c1

Update UI snapshots for chromium (1)

264e813

Update UI snapshots for chromium (1)

4124dc3

Twixes reviewed Jul 23, 2024

View reviewed changes

skoob13 and others added 3 commits July 23, 2024 13:38

Merge branch 'master' of github.com:PostHog/posthog into chore/rename…

fe111e5

…-field-multiple-breakdowns

Merge branch 'chore/rename-field-multiple-breakdowns' of github.com:P…

86c6cc9

…ostHog/posthog into chore/rename-field-multiple-breakdowns

Update UI snapshots for chromium (1)

b619831

skoob13 added 2 commits July 23, 2024 13:55

test: fix

76057fd

Merge branch 'chore/rename-field-multiple-breakdowns' of github.com:P…

40880b6

…ostHog/posthog into chore/rename-field-multiple-breakdowns

Update UI snapshots for chromium (1)

2c61d49

Update UI snapshots for chromium (1)

2ce73a5

Update UI snapshots for chromium (1)

13a8f59

Update UI snapshots for chromium (1)

b2e4c72

skoob13 merged commit 1a3a550 into master Jul 23, 2024
91 checks passed

skoob13 deleted the chore/rename-field-multiple-breakdowns branch July 23, 2024 14:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(insights): rename the value field in multiple breakdowns #23890

chore(insights): rename the value field in multiple breakdowns #23890

skoob13 commented Jul 22, 2024 •

edited

Loading

posthog-bot commented Jul 22, 2024

github-actions bot commented Jul 22, 2024 •

edited

Loading

skoob13 Jul 22, 2024 •

edited

Loading

skoob13 commented Jul 23, 2024

thmsobrmlr commented Jul 23, 2024

posthog-bot commented Jul 23, 2024

posthog-bot commented Jul 23, 2024

posthog-bot commented Jul 23, 2024

Twixes Jul 23, 2024

skoob13 Jul 23, 2024

skoob13 Jul 23, 2024

posthog-bot commented Jul 23, 2024

posthog-bot commented Jul 23, 2024

posthog-bot commented Jul 23, 2024

posthog-bot commented Jul 23, 2024

posthog-bot commented Jul 23, 2024

chore(insights): rename the value field in multiple breakdowns #23890

chore(insights): rename the value field in multiple breakdowns #23890

Conversation

skoob13 commented Jul 22, 2024 • edited Loading

Problem

Changes

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

How did you test this code?

posthog-bot commented Jul 22, 2024

📸 UI snapshots have been updated

github-actions bot commented Jul 22, 2024 • edited Loading

skoob13 Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

skoob13 commented Jul 23, 2024

thmsobrmlr commented Jul 23, 2024

posthog-bot commented Jul 23, 2024

📸 UI snapshots have been updated

posthog-bot commented Jul 23, 2024

📸 UI snapshots have been updated

posthog-bot commented Jul 23, 2024

📸 UI snapshots have been updated

Twixes Jul 23, 2024

Choose a reason for hiding this comment

skoob13 Jul 23, 2024

Choose a reason for hiding this comment

skoob13 Jul 23, 2024

Choose a reason for hiding this comment

posthog-bot commented Jul 23, 2024

📸 UI snapshots have been updated

posthog-bot commented Jul 23, 2024

📸 UI snapshots have been updated

posthog-bot commented Jul 23, 2024

📸 UI snapshots have been updated

posthog-bot commented Jul 23, 2024

📸 UI snapshots have been updated

posthog-bot commented Jul 23, 2024

📸 UI snapshots have been updated

skoob13 commented Jul 22, 2024 •

edited

Loading

github-actions bot commented Jul 22, 2024 •

edited

Loading

skoob13 Jul 22, 2024 •

edited

Loading