MultiValueVariable: Fix issue where url update would not take options into account

[mdvictor]

This PR fixes an issue where in a variable with key-value pairs(esc), setting the key in the url param would not update to it's value but instead create a new custom option with the key as a value.

Before:

Screen.Recording.2024-08-14.at.11.38.59.mov

After:

Screen.Recording.2024-08-14.at.11.39.41.mov

[torkelo]

url should not have the text / display value, it has the the value part I think

[mdvictor]

This is the scenario where the issue stemmed from. In the old arch you can set the key/text/display value and it will use it's correct value, but the URL shows the key. So we need something like this for backwards comp.

[torkelo]

I think this is likely a good solution, hm... wonder how it works in the old arch, maybe it works because URL sync happens so late, after variable has been fully initiated and options fetched/built

[torkelo]

could this lead to double queries being issued for QueryVariables ?

[mdvictor]

Yes, it will :/ once here and once on activation itself

[torkelo]

that will not do :(

[torkelo]

the variable will be validated later (when SceneVariableSet calls validateAndUpdate), can't we do this lookup at that point?

[mdvictor]

@torkelo As far as I can tell, we also load the options before setting the value from the URL in the old arch here, here

[torkelo]

@mdvictor not sure I understand the issue, looked again, I thought it was related to setting the correct key in the select after a full page reload, b

but see now that it's about setting the variable to the display value via URL. Both the new and old variable system sets the variable value via url using the value not the text representation

[torkelo]

wonder where/why they have urls with the text representation, grafana only generates URLs with the value

[torkelo]

@mdvictor so the problem is this if statement here: https://github.com/grafana/scenes/blob/main/packages/scenes/src/variables/variants/MultiValueVariable.ts#L196

that code there makes sure you can add a custom value (enter anything) in the select, that value get's set in the URL and if you reload the page we need to preserve that custom value even if it's not among the options returned by the query (or specified in the variable csv string).

We might be able to modify that logic a bit and ignore the if statement if current value === stateUpdate.text

like this

if (
      this.skipNextValidation &&
      stateUpdate.value !== this.state.value &&
      stateUpdate.text !== this.state.text &&
      !isAllValueFix
    ) {
      stateUpdate.value = this.state.value;
      stateUpdate.text = this.state.text;
    }

[mdvictor]

Closing this in favor of #874

[mdvictor]

wonder where/why they have urls with the text representation, grafana only generates URLs with the value

I don't know hey that happened either

@mdvictor so the problem is this if statement here: https://github.com/grafana/scenes/blob/main/packages/scenes/src/variables/variants/MultiValueVariable.ts#L196

that code there makes sure you can add a custom value (enter anything) in the select, that value get's set in the URL and if you reload the page we need to preserve that custom value even if it's not among the options returned by the query (or specified in the variable csv string).

We might be able to modify that logic a bit and ignore the if statement if current value === stateUpdate.text

like this

if (
      this.skipNextValidation &&
      stateUpdate.value !== this.state.value &&
      stateUpdate.text !== this.state.text &&
      !isAllValueFix
    ) {
      stateUpdate.value = this.state.value;
      stateUpdate.text = this.state.text;
    }

Thank you! Indeed that fixes the problem, I've opened a new PR with these changes and closed this one