Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(hogql): run filter based insights via hogql (undo revert) #17645

Merged
merged 12 commits into from
Nov 27, 2023

Conversation

thmsobrmlr
Copy link
Contributor

Problem

#17611 has been reverted due to the feature flag failing for unauthenticated users. See also https://posthog.sentry.io/issues/4507451100/?project=1899813.

Changes

This PR excludes unauthenticated api endpoints from the feature flag.

How did you test this code?

Unfortunately I could not reproduce the problem locally. Will keep an 👀 out when merging this

Copy link
Contributor

@neilkakkar neilkakkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No context on anything but the flag change.

Curious why you couldn't test this locally? Creating a shared dashboard, going incognito and opening it while commenting out the is_cloud() check should work for locally testing everything is fine - worth doing it I think before merging, in case something else might be going wrong too.

from posthog.models.user import User


def hogql_insights_enabled(user: User) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

types can help somewhat here:

Suggested change
def hogql_insights_enabled(user: User) -> bool:
def hogql_insights_enabled(user: User | AnonymousUser) -> bool:

@thmsobrmlr
Copy link
Contributor Author

No context on anything but the flag change.

Curious why you couldn't test this locally? Creating a shared dashboard, going incognito and opening it while commenting out the is_cloud() check should work for locally testing everything is fine - worth doing it I think before merging, in case something else might be going wrong too.

Added AnonymousUser to the type and was indeed able to verify the problem & fix now. Must have missed something before. Thanks for the review!

@thmsobrmlr thmsobrmlr enabled auto-merge (squash) September 27, 2023 12:22
auto-merge was automatically disabled October 12, 2023 07:31

Pull request was closed

@thmsobrmlr thmsobrmlr reopened this Oct 12, 2023
@thmsobrmlr thmsobrmlr removed the stale label Oct 12, 2023
# Conflicts:
#	posthog/api/query.py
#	posthog/hogql_queries/legacy_compatibility/filter_to_query.py
#	posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py
@PostHog PostHog deleted a comment from posthog-bot Oct 12, 2023
@PostHog PostHog deleted a comment from posthog-bot Oct 12, 2023
@thmsobrmlr thmsobrmlr enabled auto-merge (squash) October 12, 2023 12:12
@posthog-bot
Copy link
Contributor

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

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

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.

@posthog-bot
Copy link
Contributor

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.

thmsobrmlr and others added 3 commits November 6, 2023 12:44
# Conflicts:
#	frontend/__snapshots__/scenes-app-notebooks--notebooks-list.png
#	posthog/api/query.py
#	posthog/hogql_queries/legacy_compatibility/test/test_filter_to_query.py
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

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

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

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.

@posthog-bot
Copy link
Contributor

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.

thmsobrmlr and others added 2 commits November 27, 2023 15:28
# Conflicts:
#	frontend/__snapshots__/scenes-other-billing-v2--billing-v-2-with-discount.png
#	frontend/__snapshots__/scenes-other-billing-v2--billing-v-2.png
#	frontend/src/queries/utils.ts
#	posthog/api/query.py
Copy link
Contributor

github-actions bot commented Nov 27, 2023

Size Change: +1.21 kB (0%)

Total Size: 1.83 MB

Filename Size Change
frontend/dist/toolbar.js 1.83 MB +1.21 kB (0%)

compressed-size-action

@posthog-bot
Copy link
Contributor

📸 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 thmsobrmlr merged commit 03f0412 into master Nov 27, 2023
@thmsobrmlr thmsobrmlr deleted the unrevert-17611 branch November 27, 2023 20:42
corywatilo added a commit that referenced this pull request Dec 4, 2023
…18949)

* fix: re-add background to tertiary button

* final color

* Update UI snapshots for `chromium` (1)

* Fix button styles to allow secondary to be transparent

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* all styles in default state

* button active states

* fix full width

* remove custom nav styling as buttons now 3px shorter

* tertiary danger hover highlight

* muted stealth when not active

* button sizes

* move min-height to chrome

* side buttons in primary

* change sidepanel buttons to tertiary

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* fix border radius

* dark mode colors

* remove unused styles

* fix: set metadata when level change (#18899)

* set user analytics metadata when membership level is changed

* also set whenever user is updated since that includes updating current org

* fix tests

* fix other tests

* style(3000): Align border radius to 6px in 3000 (#18866)

* style: Align border radius to 6px in 3000

* Only affect .posthog-3000

* Also tune some non-4px radii

* Add radius comment

Co-authored-by: David Newell <[email protected]>

* Fix missing warehouse mocks and strictly prevent toasts

* Don't affect utility classes

---------

Co-authored-by: David Newell <[email protected]>

* fix(3000): various ui fixes (#18861)

* subtle links in the funnel legend

* tooltip styles

* funnel glyphs and line

* retention table

* gray lettermark

* breakdown tag

* Update UI snapshots for `webkit` (2)

* Update UI snapshots for `chromium` (1)

* consistent bg naming

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (1)

* fix retention table css

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* fix(plugin-server): Remove Postgres-based plugin error logging in favor of existing ClickHouse-based approaches (#18764)

* fix: delay usage report sending on sunday (#18900)

delay usage report sending on sunday

* feat(hogql): run filter based insights via hogql (undo revert) (#17645)

* chore(data-warehouse): fix close button (#18918)

fix close button

* feat: Only show other recordings if there are no pinned ones by default (#18915)

* fix(trends): convert any missing property operators to the default of exact (#18895)

* Convert any missing property operators to the default of exact

* Handle undefined values

* Check for undefined values

* feat(3000): Make nav usable on mobile (#18912)

* chore(3000): updated tag colors (#18786)

* updated tag colors

* darkened warning text

* Enable 3000 snapshots

* Update UI snapshots for `webkit` (2)

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* Lint CSS

---------

Co-authored-by: Michael Matloka <[email protected]>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* chore(plugin server): add OS version to person properties (#18924)

* feat: Concise titles for the top nav (#18856)

* fix(groups): add debounce to search (#18927)

* style(3000): Clean up segmented buttons (#18897)

* style(3000): Make segmented buttons feel like IRL radio buttons

* Fix font size

* Revert full press

* Update query snapshots

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* chore: update es build (#18917)

* chore: update ESBuild

* chore: update ESBuild

* more logging collapsing

* Fix

* fiddling

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (2)

* end when finished if not --dev

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* chore(plugin-server): add deprecated imports list, log if they are us… (#18726)

* chore(3000): More styling tweaks (#18923)

* Align more rounding

* Align switch. select, input sizing with buttons

* Resolve extra UI snags

* Fix 3000-dependency

* Fix one more stray divider

* Remove legacy `description` prop of `PageHeader`

* Update UI snapshots for `webkit` (2)

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* Make "New insight" button regular size

* Update UI snapshots for `chromium` (2)

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* feat(surveys): be able to drag and reorder questions (#18763)

* reorder questions wip

* reorder questions

* address comments and open question box to where it's dragged

* import linter fix

* key prop error

* rotate drag handle icon

* add back auto disappear

* chore(plugin-server): only inject imports that are used by the plugin and remove unused plugin imports (#18934)

* fix: Swap canvas state var and remove it on unmount (#18922)

* fix: Support modal url changes (#18937)

* feat(3000): Relabeled apps to data pipeline, re-ordered nav (#18919)

* relabeled apps to data pipeline, re-ordered nav

* Update UI snapshots for `chromium` (1)

* RIP connectors → apps

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (2)

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* chore(plugin-server): bump node-rdkafka to 2.18.0 (librdkafka 2.3.0) (#18420)

* revert(plugin-server): bump node-rdkafka to 2.18.0 (librdkafka 2.3.0) (#18420) (#18941)

* feat: Fixed up feature previews panel (#18931)

* fix: PersonDisplay links should be the new subtle style (#18932)

* fix: Added missing 3000 parts for exporter (#18913)

* fix: e2e tests flakiness with unit image (#18933)

* Change celery metrics port so it doesn't clash with webserver

* Change back to unit image

* chore(deps): bump aiohttp from 3.8.6 to 3.9.0 (#18920)

* chore(deps): bump aiohttp from 3.8.6 to 3.9.0

Bumps [aiohttp](https://github.com/aio-libs/aiohttp) from 3.8.6 to 3.9.0.
- [Release notes](https://github.com/aio-libs/aiohttp/releases)
- [Changelog](https://github.com/aio-libs/aiohttp/blob/master/CHANGES.rst)
- [Commits](aio-libs/aiohttp@v3.8.6...v3.9.0)

---
updated-dependencies:
- dependency-name: aiohttp
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

* update properly

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Paul D'Ambra <[email protected]>

* reset snapshots

* reset button snapshots

* merge cleanup

* final merge

* remove old snapshots

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* reset snapshots

* change onboaring to primary-alt in 3000

* add back in upwards motion on hover

* fix segmented button styles

* restore button heights

* padding adjacent to icons in buttons

* side action hover state

* Update UI snapshots for `chromium` (1)

* reset snapshots

* danger colors and unused color cleanup

* darken icons on hover

* restore all chrome hover / press depths

* increase navbar padding

* fix clearable select

* fix inspector button in session replay

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* fix rebase issues

* retain legacy lemon select styles

* improve active / hover states on segmented button

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (1)

* Avoid extra margin between 3D buttons

This allows for a slightly nicer overlaying effect

* Update UI snapshots for `chromium` (1)

* Update query snapshots

* Update query snapshots

* Update UI snapshots for `chromium` (1)

* Update query snapshots

* Update UI snapshots for `chromium` (1)

* Update query snapshots

* reset snapshots

* always show danger icon opacity as 1

* stop content overflow

* set full width horizontal padding

* Update UI snapshots for `chromium` (1)

* Update UI snapshots for `chromium` (2)

* rotate shadows on sidepanel buttons

* Update UI snapshots for `chromium` (1)

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Ben White <[email protected]>
Co-authored-by: Raquel Smith <[email protected]>
Co-authored-by: Thomas Obermüller <[email protected]>
Co-authored-by: ted kaemming <[email protected]>
Co-authored-by: Eric Duong <[email protected]>
Co-authored-by: Tom Owers <[email protected]>
Co-authored-by: Michael Matloka <[email protected]>
Co-authored-by: Cory Watilo <[email protected]>
Co-authored-by: Michael Matloka <[email protected]>
Co-authored-by: Juraj Majerik <[email protected]>
Co-authored-by: Paul D'Ambra <[email protected]>
Co-authored-by: Brett Hoerner <[email protected]>
Co-authored-by: Li Yi Yu <[email protected]>
Co-authored-by: Frank Hamand <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants