From ae6ea459b9e28593a9bcb7cb9379c994f31040be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Torkel=20=C3=96degaard?= <torkel@grafana.com>
Date: Tue, 24 Sep 2024 15:24:56 +0200
Subject: [PATCH] DashboardScene: Do not add hide flags to URL (#93641)

* DashboardScene: Do not add hide flags to URL

* Update
---
 .betterer.results                             |  3 --
 .../scene/DashboardControls.test.tsx          | 25 ++++--------
 .../scene/DashboardControls.tsx               | 39 ++++++++++---------
 3 files changed, 28 insertions(+), 39 deletions(-)

diff --git a/.betterer.results b/.betterer.results
index 96b2c33454a..9aa8cdfa3ec 100644
--- a/.betterer.results
+++ b/.betterer.results
@@ -2750,9 +2750,6 @@ exports[`better eslint`] = {
     "public/app/features/dashboard-scene/saving/shared.tsx:5381": [
       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "0"]
     ],
-    "public/app/features/dashboard-scene/scene/DashboardControls.tsx:5381": [
-      [0, 0, 0, "Do not use any type assertions.", "0"]
-    ],
     "public/app/features/dashboard-scene/scene/NavToolbarActions.tsx:5381": [
       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "0"],
       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "1"],
diff --git a/public/app/features/dashboard-scene/scene/DashboardControls.test.tsx b/public/app/features/dashboard-scene/scene/DashboardControls.test.tsx
index a5b18c1e6c4..aefb2953e2b 100644
--- a/public/app/features/dashboard-scene/scene/DashboardControls.test.tsx
+++ b/public/app/features/dashboard-scene/scene/DashboardControls.test.tsx
@@ -66,23 +66,15 @@ describe('DashboardControls', () => {
       expect(scene._urlSync.getKeys()).toEqual(['_dash.hideTimePicker', '_dash.hideVariables', '_dash.hideLinks']);
     });
 
-    it('should return url state', () => {
+    it('should not return url state for hide flags', () => {
       const scene = buildTestScene();
-      expect(scene.getUrlState()).toEqual({
-        '_dash.hideTimePicker': undefined,
-        '_dash.hideVariables': undefined,
-        '_dash.hideLinks': undefined,
-      });
+      expect(scene.getUrlState()).toEqual({});
       scene.setState({
         hideTimeControls: true,
         hideVariableControls: true,
         hideLinksControls: true,
       });
-      expect(scene.getUrlState()).toEqual({
-        '_dash.hideTimePicker': 'true',
-        '_dash.hideVariables': 'true',
-        '_dash.hideLinks': 'true',
-      });
+      expect(scene.getUrlState()).toEqual({});
     });
 
     it('should update from url', () => {
@@ -114,19 +106,16 @@ describe('DashboardControls', () => {
     });
 
     it('should not call setState if no changes', () => {
-      const scene = buildTestScene();
+      const scene = buildTestScene({ hideTimeControls: true, hideVariableControls: true, hideLinksControls: true });
       const setState = jest.spyOn(scene, 'setState');
+
       scene.updateFromUrl({
         '_dash.hideTimePicker': 'true',
         '_dash.hideVariables': 'true',
         '_dash.hideLinks': 'true',
       });
-      scene.updateFromUrl({
-        '_dash.hideTimePicker': 'true',
-        '_dash.hideVariables': 'true',
-        '_dash.hideLinks': 'true',
-      });
-      expect(setState).toHaveBeenCalledTimes(1);
+
+      expect(setState).toHaveBeenCalledTimes(0);
     });
   });
 });
diff --git a/public/app/features/dashboard-scene/scene/DashboardControls.tsx b/public/app/features/dashboard-scene/scene/DashboardControls.tsx
index 5d83e8a244e..3805792f93b 100644
--- a/public/app/features/dashboard-scene/scene/DashboardControls.tsx
+++ b/public/app/features/dashboard-scene/scene/DashboardControls.tsx
@@ -43,28 +43,31 @@ export class DashboardControls extends SceneObjectBase<DashboardControlsState> {
     keys: ['_dash.hideTimePicker', '_dash.hideVariables', '_dash.hideLinks'],
   });
 
+  /**
+   * We want the hideXX url keys to only sync one way (url => state) on init
+   * We don't want these flags to be added to URL.
+   */
   getUrlState() {
-    return {
-      '_dash.hideTimePicker': this.state.hideTimeControls ? 'true' : undefined,
-      '_dash.hideVariables': this.state.hideVariableControls ? 'true' : undefined,
-      '_dash.hideLinks': this.state.hideLinksControls ? 'true' : undefined,
-    };
+    return {};
   }
 
   updateFromUrl(values: SceneObjectUrlValues) {
-    const update: Partial<DashboardControlsState> = {};
-
-    update.hideTimeControls =
-      values['_dash.hideTimePicker'] === 'true' || values['_dash.hideTimePicker'] === '' || this.state.hideTimeControls;
-    update.hideVariableControls =
-      values['_dash.hideVariables'] === 'true' ||
-      values['_dash.hideVariables'] === '' ||
-      this.state.hideVariableControls;
-    update.hideLinksControls =
-      values['_dash.hideLinks'] === 'true' || values['_dash.hideLinks'] === '' || this.state.hideLinksControls;
-
-    if (Object.entries(update).some(([k, v]) => v !== this.state[k as keyof DashboardControlsState])) {
-      this.setState(update);
+    const { hideTimeControls, hideVariableControls, hideLinksControls } = this.state;
+    const isEnabledViaUrl = (key: string) => values[key] === 'true' || values[key] === '';
+
+    // Only allow hiding, never "unhiding" from url
+    // Becasue this should really only change on first init it's fine to do multiple setState here
+
+    if (!hideTimeControls && isEnabledViaUrl('_dash.hideTimePicker')) {
+      this.setState({ hideTimeControls: true });
+    }
+
+    if (!hideVariableControls && isEnabledViaUrl('_dash.hideVariables')) {
+      this.setState({ hideVariableControls: true });
+    }
+
+    if (!hideLinksControls && isEnabledViaUrl('_dash.hideLinks')) {
+      this.setState({ hideLinksControls: true });
     }
   }