grafana / scenes

Build Grafana dashboards directly in your Grafana app plugins.
https://grafana.com/developers/scenes
Apache License 2.0
141 stars 21 forks source link

AdHocFilters: Better match old handling of queryparam formatting for ad hoc filters #926

Closed kaydelaney closed 1 month ago

kaydelaney commented 1 month ago

In the old architecture, template variable adapters had a getValueForUrl method that could return a processed value suited for including in a URL if necessary. This was used in templateSrv:

https://github.com/grafana/grafana/blob/fb3b13b567ca074bcf9dca552e5ee4e6feff30ce/public/app/features/templating/template_srv.ts#L330-L335

To better match this behavior under scenes, I've moved some functions around and added an optional getValueForUrl method to the SceneVariable and FormatVariable interfaces. This method gets called from the queryparam formatter, if it exists.

Fixes: https://github.com/grafana/grafana/issues/94303

torkelo commented 1 month ago

@kaydelaney can you describe / link the bug? Not sure I understand the changes, we have the URL sync handler interface, so not sure why we would need a new getValueForUrl function

Sergej-Vlasov commented 1 month ago

@torkelo thread about the bug https://raintank-corp.slack.com/archives/C03KVDHTWAH/p1727773589691779

torkelo commented 1 month ago

Ok, took me some time to understand the problem / difference. I think leveraging our existing URL sync handler interface inside the variable formatted looks like a smaller / less complex solution: https://github.com/grafana/scenes/pull/931/files

kaydelaney commented 1 month ago

Closed for https://github.com/grafana/scenes/pull/931