fix: show correct badge count on notebook button #17735

daibhin · 2023-10-03T12:59:50Z

Problem

Reported in https://posthog.slack.com/archives/C05N9R3HT7V/p1695922124646429. The badge was showing the count for all the notebooks containing an insight, and not making use of the saved insights id

Changes

Pass the saved insights id to the NotebookSelectButton
Use a special id_match_structure to find matching insights

How did you test this code?

Updated tests

posthog-bot · 2023-10-03T13:13:04Z

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

Triggered by this commit.

👉 Review this PR's diff of snapshots.

pauldambra

🙌

posthog-bot · 2023-10-03T15:40: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 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

frontend/src/scenes/notebooks/NotebookSelectButton/NotebookSelectButton.tsx

benjackwhite · 2023-10-05T07:17:47Z

posthog/api/notebook.py

@@ -281,6 +281,8 @@ def _filter_request(self, request: request.Request, queryset: QuerySet) -> Query
                            # replay timestamps are not at the top level, they're one-level down in a content array
                            presence_match_structure = [{"content": [{"type": f"ph-{target}"}]}]
                            id_match_structure = [{"content": [{"attrs": {"sessionRecordingId": match}}]}]
+                        elif target == "query":


🤔 So we are making query only work for a specific Insight? This feels very off...

If this is the way to go then I think the "target" should explicitly be saved-insight or something.

My understanding was that this is just for cases in which there is an id present e.g. when the search query looks like query:vk7xz3 where the content after the colon is the short id of the saved insight. You could still search notebooks that contain / do not contain queries with the search param query:true or query:false.

The only reason we special case this lookup is because the NotebookNodeQuery does not have an id attribute which the search endpoint assumes. Its id is nested inside the query attribute.

Does that explain your question?

@pauldambra am I right in thinking that this will still allow you to search for the presence of a query with the query:true param?

so long as this test is still valid

# filter by insight insight_filter_response = self.client.get(f"/api/projects/{self.team.id}/notebooks?contains=query:true") assert [n["id"] for n in insight_filter_response.json()["results"]] == [insight_content_notebook]

then yep we can filter by presence of "type" and presence of an instance of a "type"

posthog-bot · 2023-10-09T10:57: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 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2023-10-20T07:31:18Z

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

…17850) * Fix top pages SQL * Use person id for now * Add limits to web analytics queries * Add some trends to web analytics * Tweak query date ranges and formatting * Consistently use person_id for now

)

…xport menu (#17834)

* move onstart to products page * remove product intro * include stepkey as prop * redo how navigation works * go back to products if first step * fix url redirect loop * fix jitteryness with otherproductsstep * fix grumpy linting

#17820) * chore(plugin-server): kafka: change analytics linger.ms and batch.size for all producers * add envvars

* Increase playwright timeouts * Tidy up comments

add menu option for reducing bill info

* feat(hogql): mixed poe mode * schema

* chore(hogql): Use the C++ parser in local dev * Update all the parsing functions, not just `parse_select`

…8101) * Added support for daily and weekly active user aggregations * Removed unused join * Fixed tests and applied Sample placeholders * Fixed formula running on an empty string * Updated the name of QueryModifier * Comments and fixed name casing

* WIP * Kinda sorta working * Add dashboard filters to cache key * Make mypy happy * Add support for properties * Use get_query_runner * Add tests for dashboard_query * Add tests for merging of filters * Add test for hash when dashboard filters are applied * Silently accept having no valid query runner * Remove index type from DashboardFilter * Fix type of dashboard_query * Fix test typing

…d cells (#18116) * Add better table formatting * Use QueryContent to right-align table cells and headings * Just use toLocaleString

#18120) fix(plugin-server): prevent multiple concurrent reloadPlugins and coalesce them when possible

* opt in surveys when creating survey and opt out when no active surveys * update survey banner warning * add auto opt ins for surveys list launch and stop surveys * remove from onboarding step * Update UI snapshots for `chromium` (2) * update opt in from backend instead * add filter for api surveys and post delete to receiver * Update query snapshots * fix popup flicker * ambr snapshots * disabled popup banner rename * exclude api surveys in query instead --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* pay gates wip * Update UI snapshots for `chromium` (2) * pay gate survey edit * html descriptions * Update UI snapshots for `chromium` (2) * rename ctas in pay gate mini and use getavailablefeature * Update UI snapshots for `chromium` (2) * Update UI snapshots for `webkit` (2) * feature flag pay gate * Update UI snapshots for `chromium` (2) * swap surveys beta tag for new and bump higher * Update UI snapshots for `chromium` (1) * fix merge conflict --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

…te (#18125) * move survey question type back up * add open feedback template * fix description

* only run hogql parser workflow if running from upstream repo * Fix capitalization --------- Co-authored-by: Michael Matloka <[email protected]>

Bumps [postcss](https://github.com/postcss/postcss) from 8.4.18 to 8.4.31. - [Release notes](https://github.com/postcss/postcss/releases) - [Changelog](https://github.com/postcss/postcss/blob/main/CHANGELOG.md) - [Commits](postcss/postcss@8.4.18...8.4.31) --- updated-dependencies: - dependency-name: postcss dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

posthog-bot · 2023-10-23T14:00:41Z

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

Triggered by this commit.

👉 Review this PR's diff of snapshots.

benjackwhite

👍 still has the issue of only really working for insight queries but i can live with that for now

daibhin added 2 commits October 3, 2023 13:41

fix: show correct badge count on notebook button

96338f8

update tests

6cb7817

daibhin marked this pull request as ready for review October 3, 2023 13:00

daibhin requested a review from pauldambra October 3, 2023 13:00

pauldambra approved these changes Oct 3, 2023

View reviewed changes

remove final insight reference

f95d65a

daibhin force-pushed the dn-fix/insight-notebook-badge branch from 27559ae to f95d65a Compare October 3, 2023 15:28

benjackwhite requested changes Oct 5, 2023

View reviewed changes

Merge branch 'master' into dn-fix/insight-notebook-badge

bb08774

daibhin force-pushed the dn-fix/insight-notebook-badge branch from b239c96 to bb08774 Compare October 9, 2023 10:41

daibhin and others added 2 commits October 9, 2023 11:45

cleanup button styling

3076189

Update UI snapshots for chromium (2)

18564d5

posthog-bot added the stale label Oct 20, 2023

robbie-c and others added 13 commits October 23, 2023 14:36

feat(web-analytics): Fix top pages sql and add some extra insights (#…

ada936a

…17850) * Fix top pages SQL * Use person id for now * Add limits to web analytics queries * Add some trends to web analytics * Tweak query date ranges and formatting * Consistently use person_id for now

fix: remove content width toggle from popover (#17775)

e4cb8f9

fix(batch-exports): Include properties in feature_enabled call (#17860)

320786b

chore: set Sentry release using commit.txt (#17855)

07b4447

chore: Remove promise manager usage from ingestion warnings & db (#17650

06ba7e3

)

fixes "CSV export is limited to 10000 events" menu is hidden behind e…

aed7ff6

…xport menu (#17834)

feat: sidebar widgets (#17859)

b33cd39

chore(plugin-server): kafka: change analytics linger.ms and batch.siz… (

2631f29

#17820) * chore(plugin-server): kafka: change analytics linger.ms and batch.size for all producers * add envvars

feat(surveys): optional questions for multipleq (#17830)

98f0b9f

chore(deps): Update posthog-js to 1.83.0 (#17871)

d04774b

feat(surveys): Add question html support (#17847)

874b2da

test: Increase playwright timeouts (#17872)

1a0e5b6

* Increase playwright timeouts * Tidy up comments

raquelmsmith and others added 23 commits October 23, 2023 14:42

feat(billing): add menu option for reducing bill info (#18050)

2afad9f

add menu option for reducing bill info

fix: support minimal top bar for onboarding screens (#18077)

6c95f47

fix(backfill-batch-exports): Do not run partial ranges (#18114)

472d814

feat(hogql): mixed PoE mode (#18064)

abd23e5

* feat(hogql): mixed poe mode * schema

fix: update person recordings default date range (#18115)

9dcdcda

chore(batch-exports): Do not retry on RefreshError and NotFound (#18107)

6f49da9

chore(hogql): Use the C++ parser in local dev (#18112)

a44fbd2

* chore(hogql): Use the C++ parser in local dev * Update all the parsing functions, not just `parse_select`

chore(deps): upgrade storybook to 7.5 (#18113)

496d492

fix(plugin-server): jitter and retry reloadPlugins (#18108)

9f6a676

fix(plugin-server): filter NaN out of buildIntegerMatcher (#18106)

7bebb03

feat(web-analytics): Add localised number formatting and right-aligne…

338dad7

…d cells (#18116) * Add better table formatting * Use QueryContent to right-align table cells and headings * Just use toLocaleString

fix(plugin-server): prevent multiple concurrent reloadPlugins and coa… (

d722e8e

#18120) fix(plugin-server): prevent multiple concurrent reloadPlugins and coalesce them when possible

fix(backfill-batch-exports): Account for jitter (#18121)

4cbd622

fix(survey): fix question type placement and add open feedback templa…

32a3001

…te (#18125) * move survey question type back up * add open feedback template * fix description

chore(deps): Update posthog-js to 1.84.2 (#18130)

251fd3d

ci(hogql): Don't run parser build workflow on forks (#18018)

3e493e4

* only run hogql parser workflow if running from upstream repo * Fix capitalization --------- Co-authored-by: Michael Matloka <[email protected]>

Merge branch 'master' into dn-fix/insight-notebook-badge

3daf5de

Update UI snapshots for chromium (2)

abf2010

benjackwhite approved these changes Oct 23, 2023

View reviewed changes

posthog-bot removed the stale label Oct 24, 2023

daibhin merged commit 6859c72 into master Oct 24, 2023
74 checks passed

daibhin deleted the dn-fix/insight-notebook-badge branch October 24, 2023 08:17

Justicea83 pushed a commit to Justicea83/posthog that referenced this pull request Oct 24, 2023

fix: show correct badge count on notebook button (PostHog#17735)

8d87e9c

daibhin mentioned this pull request Nov 8, 2023

fix: notebooks insight bugs #18483

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: show correct badge count on notebook button #17735

fix: show correct badge count on notebook button #17735

daibhin commented Oct 3, 2023

posthog-bot commented Oct 3, 2023

pauldambra left a comment

posthog-bot commented Oct 3, 2023

benjackwhite Oct 5, 2023

daibhin Oct 5, 2023

daibhin Oct 12, 2023

pauldambra Oct 12, 2023

posthog-bot commented Oct 9, 2023

posthog-bot commented Oct 20, 2023

posthog-bot commented Oct 23, 2023

benjackwhite left a comment

fix: show correct badge count on notebook button #17735

fix: show correct badge count on notebook button #17735

Conversation

daibhin commented Oct 3, 2023

Problem

Changes

How did you test this code?

posthog-bot commented Oct 3, 2023

📸 UI snapshots have been updated

pauldambra left a comment

Choose a reason for hiding this comment

posthog-bot commented Oct 3, 2023

📸 UI snapshots have been updated

benjackwhite Oct 5, 2023

Choose a reason for hiding this comment

daibhin Oct 5, 2023

Choose a reason for hiding this comment

daibhin Oct 12, 2023

Choose a reason for hiding this comment

pauldambra Oct 12, 2023

Choose a reason for hiding this comment

posthog-bot commented Oct 9, 2023

📸 UI snapshots have been updated

posthog-bot commented Oct 20, 2023

posthog-bot commented Oct 23, 2023

📸 UI snapshots have been updated

benjackwhite left a comment

Choose a reason for hiding this comment