feat(insight-variables): dashboard variables populating and updating urls #27843
Conversation
| @@ -1479,6 +1490,51 @@ export const dashboardLogic = kea<dashboardLogicType>([ | |||
| }, | |||
| })), | |||
|
|
|||
| subscriptions(({ values, actions }) => ({ | |||
| variables: (variables) => { | |||
There was a problem hiding this comment.
Should this not be maybeUrlVariables? Or is this triggered from somewhere else?
There was a problem hiding this comment.
this is sort of dicey basically maybeURLVariables needs to be set first on load and then when variables loads, it'll do a check the maybe variables to populate if possible (which is why this subscription is mounted on variables)
| for (const [key, value] of Object.entries(urlVariables)) { | ||
| const variable = variables.find((variable: HogQLVariable) => variable.code_name === key) | ||
| if (variable) { | ||
| overrideVariables.push({ ...variable, value }) | ||
| } | ||
| } |
There was a problem hiding this comment.
So I'm not a fan of allowing users to set any query param and have our logic try to parse it as variables. In the future, we can't use dashboard query params for anything else in case of a conflict of insight variable names. Instead, I think we should have a single query param (something like query_variables) that is a key/value pair object instead - this makes it a lot more explicit.
The downside to this is that it's more effort on the end of building the URL when writing a HogQL query, but we can add the overrides as a parameter of the <a variableOverrides={...} /> component instead (long term, links are done via chart settings instead of in the query itself)
…/posthog into insight-variables-dashboard-urls
|
Size Change: +5 B (0%) Total Size: 1.16 MB ℹ️ View Unchanged
|
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
…/posthog into insight-variables-dashboard-urls
Problem
Changes
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Does this work well for both Cloud and self-hosted?
How did you test this code?