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

fix(insights): dataVisualizationLogic expects insights with response not just results #24181

Merged
merged 6 commits into from
Aug 5, 2024

Conversation

thmsobrmlr
Copy link
Contributor

@thmsobrmlr thmsobrmlr commented Aug 5, 2024

Problem

dataVisualizationLogic expects insights with response not just results. This breaks dashboard cards.

Changes

Passes in the whole insight. We should probably have a way to extract only the required info / match up types to what we get back from /query.

How did you test this code?

Tried locally

@posthog-bot
Copy link
Contributor

Hey @thmsobrmlr! 👋
This pull request seems to contain no description. Please add useful context, rationale, and/or any other information that will help make sense of this change now and in the distant Mars-based future.

@thmsobrmlr thmsobrmlr requested review from a team, Gilbert09 and skoob13 and removed request for a team August 5, 2024 09:17
Copy link
Contributor

github-actions bot commented Aug 5, 2024

Size Change: 0 B

Total Size: 1.07 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 1.07 MB

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

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@thmsobrmlr thmsobrmlr enabled auto-merge (squash) August 5, 2024 09:31
@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.

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

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

@thmsobrmlr thmsobrmlr merged commit 135be20 into master Aug 5, 2024
89 of 91 checks passed
@thmsobrmlr thmsobrmlr deleted the fix-insight-results branch August 5, 2024 10:42
@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 (wasn't pushed!)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

silentninja pushed a commit to silentninja/posthog that referenced this pull request Aug 8, 2024
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.

4 participants