Variables: Saved default all value with non multi select fix (#959)

grafana · Nov 8, 2024 · 0c4956f · 0c4956f
1 parent 11cbd2c
commit 0c4956f
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 4 deletions.
diff --git a/packages/scenes/src/variables/variants/MultiValueVariable.test.ts b/packages/scenes/src/variables/variants/MultiValueVariable.test.ts
@@ -298,6 +298,24 @@ describe('MultiValueVariable', () => {
       expect(variable.state.value).toBe(ALL_VARIABLE_VALUE);
       expect(variable.state.text).toBe(ALL_VARIABLE_TEXT);
     });
+
+    it('Should correct $__all text value if not correct', async () => {
+      const variable = new TestVariable({
+        name: 'test',
+        options: [],
+        optionsToReturn: [{ label: 'A', value: '1' }],
+        defaultToAll: true,
+        includeAll: true,
+        value: ALL_VARIABLE_VALUE,
+        text: ALL_VARIABLE_VALUE,
+        delayMs: 0,
+      });
+
+      await lastValueFrom(variable.validateAndUpdate());
+
+      expect(variable.state.value).toBe(ALL_VARIABLE_VALUE);
+      expect(variable.state.text).toBe(ALL_VARIABLE_TEXT);
+    });
   });
 
   describe('changeValueTo', () => {

diff --git a/packages/scenes/src/variables/variants/MultiValueVariable.ts b/packages/scenes/src/variables/variants/MultiValueVariable.ts
@@ -92,7 +92,11 @@ export abstract class MultiValueVariable<TState extends MultiValueVariableState
     this.setStateHelper(stateUpdate);
 
     // Publish value changed event only if value changed
-    if (stateUpdate.value !== currentValue || stateUpdate.text !== currentText || (this.hasAllValue() && !isEqual(options, oldOptions))) {
+    if (
+      stateUpdate.value !== currentValue ||
+      stateUpdate.text !== currentText ||
+      (this.hasAllValue() && !isEqual(options, oldOptions))
+    ) {
       this.publishEvent(new SceneVariableValueChangedEvent(this), true);
     }
   }
@@ -125,7 +129,10 @@ export abstract class MultiValueVariable<TState extends MultiValueVariableState
     }
 
     if (this.hasAllValue()) {
-      if (!this.state.includeAll) {
+      if (this.state.includeAll) {
+        // Sometimes the text representation is also set the ALL_VARIABLE_VALUE, this fixes that
+        stateUpdate.text = ALL_VARIABLE_TEXT;
+      } else {
         stateUpdate.value = options[0].value;
         stateUpdate.text = options[0].label;
         // If multi switch to arrays
@@ -192,7 +199,12 @@ export abstract class MultiValueVariable<TState extends MultiValueVariableState
     // If the validation wants to fix the all value (All ==> $__all) then we should let that pass
     const isAllValueFix = stateUpdate.value === ALL_VARIABLE_VALUE && this.state.text === ALL_VARIABLE_TEXT;
 
-    if (this.skipNextValidation && stateUpdate.value !== this.state.value && stateUpdate.text !== this.state.text && !isAllValueFix) {
+    if (
+      this.skipNextValidation &&
+      stateUpdate.value !== this.state.value &&
+      stateUpdate.text !== this.state.text &&
+      !isAllValueFix
+    ) {
       stateUpdate.value = this.state.value;
       stateUpdate.text = this.state.text;
     }
@@ -331,7 +343,7 @@ export abstract class MultiValueVariable<TState extends MultiValueVariableState
 
   public refreshOptions() {
     this.getValueOptions({}).subscribe((options) => {
-        this.updateValueGivenNewOptions(options);
+      this.updateValueGivenNewOptions(options);
     });
   }