Dashbboard: Fixes time picker schema default issues (grafana#81847)

* Dashbboard: Fixes time picker schema default issues * Fix tests * fix imports
Nortech-ai · Feb 5, 2024 · a5d8909 · a5d8909
1 parent b2730d1
commit a5d8909
Show file tree

Hide file tree

Showing 13 changed files with 64 additions and 91 deletions.
diff --git a/docs/sources/developers/kinds/core/dashboard/schema-reference.md b/docs/sources/developers/kinds/core/dashboard/schema-reference.md
@@ -82,7 +82,7 @@ extraFields is reserved for any fields that are pulled from the API server metad
 | `links`                | [DashboardLink](#dashboardlink)[]           | No       |           | Links with references to other dashboards or external websites.                                                                                                                                                                                                                                                                                                                                                |
 | `liveNow`              | boolean                                     | No       |           | When set to true, the dashboard will redraw panels at an interval matching the pixel width.<br/>This will keep data "moving left" regardless of the query refresh rate. This setting helps<br/>avoid dashboards presenting stale live data                                                                                                                                                                     |
 | `panels`               | [object](#panels)[]                         | No       |           | List of dashboard panels                                                                                                                                                                                                                                                                                                                                                                                       |
-| `refresh`              |                                             | No       |           | Refresh rate of dashboard. Represented via interval string, e.g. "5s", "1m", "1h", "1d".                                                                                                                                                                                                                                                                                                                       |
+| `refresh`              | string                                      | No       |           | Refresh rate of dashboard. Represented via interval string, e.g. "5s", "1m", "1h", "1d".                                                                                                                                                                                                                                                                                                                       |
 | `revision`             | integer                                     | No       |           | This property should only be used in dashboards defined by plugins.  It is a quick check<br/>to see if the version has changed since the last time.                                                                                                                                                                                                                                                            |
 | `snapshot`             | [Snapshot](#snapshot)                       | No       |           | A dashboard snapshot shares an interactive dashboard publicly.<br/>It is a read-only version of a dashboard, and is not editable.<br/>It is possible to create a snapshot of a snapshot.<br/>Grafana strips away all sensitive information from the dashboard.<br/>Sensitive information stripped: queries (metric, template,annotation) and panel links.                                                      |
 | `tags`                 | string[]                                    | No       |           | Tags associated with dashboard.                                                                                                                                                                                                                                                                                                                                                                                |
@@ -197,10 +197,10 @@ It defines the default config for the time picker and the refresh picker for the
 
 | Property            | Type     | Required | Default                               | Description                                                                                                                           |
 |---------------------|----------|----------|---------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------|
-| `hidden`            | boolean  | **Yes**  | `false`                               | Whether timepicker is visible or not.                                                                                                 |
-| `refresh_intervals` | string[] | **Yes**  | `[5s 10s 30s 1m 5m 15m 30m 1h 2h 1d]` | Interval options available in the refresh picker dropdown.                                                                            |
-| `time_options`      | string[] | **Yes**  | `[5m 15m 1h 6h 12h 24h 2d 7d 30d]`    | Selectable options available in the time picker dropdown. Has no effect on provisioned dashboard.                                     |
+| `hidden`            | boolean  | No       | `false`                               | Whether timepicker is visible or not.                                                                                                 |
 | `nowDelay`          | string   | No       |                                       | Override the now time by entering a time delay. Use this option to accommodate known delays in data aggregation to avoid null values. |
+| `refresh_intervals` | string[] | No       | `[5s 10s 30s 1m 5m 15m 30m 1h 2h 1d]` | Interval options available in the refresh picker dropdown.                                                                            |
+| `time_options`      | string[] | No       | `[5m 15m 1h 6h 12h 24h 2d 7d 30d]`    | Selectable options available in the time picker dropdown. Has no effect on provisioned dashboard.                                     |
 
 ### Panels
 

diff --git a/kinds/dashboard/dashboard_kind.cue b/kinds/dashboard/dashboard_kind.cue
@@ -70,7 +70,7 @@ lineage: schemas: [{
 			weekStart?: string
 
 			// Refresh rate of dashboard. Represented via interval string, e.g. "5s", "1m", "1h", "1d".
-			refresh?: string | false
+			refresh?: string
 
 			// Version of the JSON schema, incremented each time a Grafana update brings
 			// changes to said schema.
@@ -457,11 +457,11 @@ lineage: schemas: [{
 		// It defines the default config for the time picker and the refresh picker for the specific dashboard.
 		#TimePickerConfig: {
 			// Whether timepicker is visible or not.
-			hidden: bool | *false
+			hidden?: bool | *false
 			// Interval options available in the refresh picker dropdown.
-			refresh_intervals: [...string] | *["5s", "10s", "30s", "1m", "5m", "15m", "30m", "1h", "2h", "1d"]
+			refresh_intervals?: [...string] | *["5s", "10s", "30s", "1m", "5m", "15m", "30m", "1h", "2h", "1d"]
 			// Selectable options available in the time picker dropdown. Has no effect on provisioned dashboard.
-			time_options: [...string] | *["5m", "15m", "1h", "6h", "12h", "24h", "2d", "7d", "30d"]
+			time_options?: [...string] | *["5m", "15m", "1h", "6h", "12h", "24h", "2d", "7d", "30d"]
 			// Override the now time by entering a time delay. Use this option to accommodate known delays in data aggregation to avoid null values.
 			nowDelay?: string
 		} @cuetsy(kind="interface") @grafana(TSVeneer="type")

diff --git a/packages/grafana-schema/src/raw/dashboard/x/dashboard_types.gen.ts b/packages/grafana-schema/src/raw/dashboard/x/dashboard_types.gen.ts
@@ -654,19 +654,19 @@ export interface TimePickerConfig {
   /**
    * Whether timepicker is visible or not.
    */
-  hidden: boolean;
+  hidden?: boolean;
   /**
    * Override the now time by entering a time delay. Use this option to accommodate known delays in data aggregation to avoid null values.
    */
   nowDelay?: string;
   /**
    * Interval options available in the refresh picker dropdown.
    */
-  refresh_intervals: Array<string>;
+  refresh_intervals?: Array<string>;
   /**
    * Selectable options available in the time picker dropdown. Has no effect on provisioned dashboard.
    */
-  time_options: Array<string>;
+  time_options?: Array<string>;
 }
 
 export const defaultTimePickerConfig: Partial<TimePickerConfig> = {
@@ -1057,7 +1057,7 @@ export interface Dashboard {
   /**
    * Refresh rate of dashboard. Represented via interval string, e.g. "5s", "1m", "1h", "1d".
    */
-  refresh?: (string | false);
+  refresh?: string;
   /**
    * This property should only be used in dashboards defined by plugins.  It is a quick check
    * to see if the version has changed since the last time.

diff --git a/pkg/kinds/dashboard/dashboard_spec_gen.go b/pkg/kinds/dashboard/dashboard_spec_gen.go
diff --git a/...atures/dashboard-scene/serialization/__snapshots__/transformSceneToSaveModel.test.ts.snap b/...atures/dashboard-scene/serialization/__snapshots__/transformSceneToSaveModel.test.ts.snap
@@ -232,32 +232,7 @@ exports[`transformSceneToSaveModel Given a scene with rows Should transform back
     "from": "now-6h",
     "to": "now",
   },
-  "timepicker": {
-    "hidden": false,
-    "refresh_intervals": [
-      "5s",
-      "10s",
-      "30s",
-      "1m",
-      "5m",
-      "15m",
-      "30m",
-      "1h",
-      "2h",
-      "1d",
-    ],
-    "time_options": [
-      "5m",
-      "15m",
-      "1h",
-      "6h",
-      "12h",
-      "24h",
-      "2d",
-      "7d",
-      "30d",
-    ],
-  },
+  "timepicker": {},
   "timezone": "",
   "title": "Repeating rows",
   "uid": "Repeating-rows-uid",
@@ -572,17 +547,6 @@ exports[`transformSceneToSaveModel Given a simple scene with custom settings Sho
       "30m",
       "1h",
     ],
-    "time_options": [
-      "5m",
-      "15m",
-      "1h",
-      "6h",
-      "12h",
-      "24h",
-      "2d",
-      "7d",
-      "30d",
-    ],
   },
   "timezone": "America/New_York",
   "title": "My custom title",
@@ -878,7 +842,6 @@ exports[`transformSceneToSaveModel Given a simple scene with variables Should tr
     "to": "now",
   },
   "timepicker": {
-    "hidden": false,
     "refresh_intervals": [
       "10s",
       "30s",
@@ -890,17 +853,6 @@ exports[`transformSceneToSaveModel Given a simple scene with variables Should tr
       "2h",
       "1d",
     ],
-    "time_options": [
-      "5m",
-      "15m",
-      "1h",
-      "6h",
-      "12h",
-      "24h",
-      "2d",
-      "7d",
-      "30d",
-    ],
   },
   "timezone": "America/New_York",
   "title": "Dashboard to load1",

diff --git a/public/app/features/dashboard-scene/serialization/transformSceneToSaveModel.ts b/public/app/features/dashboard-scene/serialization/transformSceneToSaveModel.ts
@@ -1,3 +1,5 @@
+import { isEqual } from 'lodash';
+
 import { isEmptyObject, ScopedVars, TimeRange } from '@grafana/data';
 import {
   behaviors,
@@ -23,6 +25,7 @@ import {
   FieldConfigSource,
   Panel,
   RowPanel,
+  TimePickerConfig,
   VariableModel,
   VariableRefresh,
 } from '@grafana/schema';
@@ -50,8 +53,10 @@ export function transformSceneToSaveModel(scene: DashboardScene, isSnapshot = fa
   const data = state.$data;
   const variablesSet = state.$variables;
   const body = state.body;
-  let refresh_intervals = defaultTimePickerConfig.refresh_intervals;
-  let hideTimePicker: boolean = defaultTimePickerConfig.hidden;
+
+  let refreshIntervals: string[] | undefined;
+  let hideTimePicker: boolean | undefined;
+
   let panels: Panel[] = [];
   let graphTooltip = defaultDashboard.graphTooltip;
   let variables: VariableModel[] = [];
@@ -88,14 +93,15 @@ export function transformSceneToSaveModel(scene: DashboardScene, isSnapshot = fa
   }
 
   if (state.controls && state.controls[0] instanceof DashboardControls) {
-    hideTimePicker = state.controls[0].state.hideTimeControls ?? hideTimePicker;
+    hideTimePicker = state.controls[0].state.hideTimeControls;
 
     const timeControls = state.controls[0].state.timeControls;
     for (const control of timeControls) {
       if (control instanceof SceneRefreshPicker && control.state.intervals) {
-        refresh_intervals = control.state.intervals;
+        refreshIntervals = control.state.intervals;
       }
     }
+
     const variableControls = state.controls[0].state.variableControls;
     for (const control of variableControls) {
       if (control instanceof AdHocFilterSet) {
@@ -112,6 +118,15 @@ export function transformSceneToSaveModel(scene: DashboardScene, isSnapshot = fa
     graphTooltip = state.$behaviors[0].state.sync;
   }
 
+  const timePickerWithoutDefaults = removeDefaults<TimePickerConfig>(
+    {
+      refresh_intervals: refreshIntervals,
+      hidden: hideTimePicker,
+      nowDelay: timeRange.UNSAFE_nowDelay,
+    },
+    defaultTimePickerConfig
+  );
+
   const dashboard: Dashboard = {
     ...defaultDashboard,
     title: state.title,
@@ -123,12 +138,7 @@ export function transformSceneToSaveModel(scene: DashboardScene, isSnapshot = fa
       from: timeRange.from,
       to: timeRange.to,
     },
-    timepicker: {
-      ...defaultTimePickerConfig,
-      refresh_intervals,
-      hidden: hideTimePicker,
-      nowDelay: timeRange.UNSAFE_nowDelay,
-    },
+    timepicker: timePickerWithoutDefaults,
     panels,
     annotations: {
       list: annotations,
@@ -472,3 +482,14 @@ export function trimDashboardForSnapshot(title: string, time: TimeRange, dash: D
 
   return result;
 }
+
+function removeDefaults<T>(object: T, defaults: T): T {
+  const newObj = { ...object };
+  for (const key in defaults) {
+    if (isEqual(newObj[key], defaults[key])) {
+      delete newObj[key];
+    }
+  }
+
+  return newObj;
+}
diff --git a/public/app/features/dashboard/components/DashboardPrompt/DashboardPrompt.tsx b/public/app/features/dashboard/components/DashboardPrompt/DashboardPrompt.tsx
@@ -183,7 +183,7 @@ function cleanDashboardFromIgnoredChanges(dashData: Dashboard) {
 
   // ignore time and refresh
   delete dash.time;
-  dash.refresh = '';
+  delete dash.refresh;
   dash.schemaVersion = 0;
   delete dash.timezone;
 

diff --git a/public/app/features/dashboard/components/DashboardSettings/TimePickerSettings.tsx b/public/app/features/dashboard/components/DashboardSettings/TimePickerSettings.tsx
@@ -20,7 +20,7 @@ interface Props {
   nowDelay?: string;
   timezone: TimeZone;
   weekStart: string;
-  liveNow: boolean;
+  liveNow?: boolean;
 }
 
 interface State {

diff --git a/public/app/features/dashboard/services/TimeSrv.test.ts b/public/app/features/dashboard/services/TimeSrv.test.ts
@@ -24,7 +24,7 @@ describe('timeSrv', () => {
     _dashboard = {
       time: { from: 'now-6h', to: 'now' },
       getTimezone: jest.fn(() => 'browser'),
-      refresh: false,
+      refresh: '',
       timeRangeUpdated: jest.fn(() => {}),
       timepicker: {},
     };
@@ -94,7 +94,7 @@ describe('timeSrv', () => {
         _dashboard = {
           time: { from: 'now-6h', to: 'now' },
           getTimezone: jest.fn(() => 'browser'),
-          refresh: false,
+          refresh: '',
           timeRangeUpdated: jest.fn(() => {}),
           timepicker: {},
         };
@@ -236,10 +236,10 @@ describe('timeSrv', () => {
 
   describe('setTime', () => {
     it('should return disable refresh if refresh is disabled for any range', () => {
-      _dashboard.refresh = false;
+      _dashboard.refresh = '';
 
       timeSrv.setTime({ from: '2011-01-01', to: '2015-01-01' });
-      expect(_dashboard.refresh).toBe(false);
+      expect(_dashboard.refresh).toBe('');
     });
 
     it('should restore refresh for absolute time range', () => {
@@ -255,7 +255,7 @@ describe('timeSrv', () => {
         from: dateTime([2011, 1, 1]),
         to: dateTime([2015, 1, 1]),
       });
-      expect(_dashboard.refresh).toBe(false);
+      expect(_dashboard.refresh).toBe('');
       timeSrv.setTime({ from: '2011-01-01', to: 'now' });
       expect(_dashboard.refresh).toBe('10s');
     });

diff --git a/public/app/features/dashboard/services/TimeSrv.ts b/public/app/features/dashboard/services/TimeSrv.ts
@@ -170,7 +170,7 @@ export class TimeSrv {
     if (params.get('to') && params.get('to')!.indexOf('now') === -1) {
       this.refresh = false;
       if (this.timeModel) {
-        this.timeModel.refresh = false;
+        this.timeModel.refresh = undefined;
       }
     }
 
@@ -214,7 +214,7 @@ export class TimeSrv {
     return this.timeAtLoad && (this.timeAtLoad.from !== this.time.from || this.timeAtLoad.to !== this.time.to);
   }
 
-  setAutoRefresh(interval: string | false) {
+  setAutoRefresh(interval: string) {
     if (this.timeModel) {
       this.timeModel.refresh = interval;
     }
@@ -295,7 +295,7 @@ export class TimeSrv {
     // disable refresh if zoom in or zoom out
     if (isDateTime(time.to)) {
       this.oldRefresh = this.timeModel?.refresh || this.oldRefresh;
-      this.setAutoRefresh(false);
+      this.setAutoRefresh('');
     } else if (this.oldRefresh && this.oldRefresh !== this.timeModel?.refresh) {
       this.setAutoRefresh(this.oldRefresh);
       this.oldRefresh = undefined;

diff --git a/public/app/features/dashboard/state/DashboardModel.ts b/public/app/features/dashboard/state/DashboardModel.ts
@@ -70,13 +70,13 @@ export class DashboardModel implements TimeModel {
   editable: any;
   graphTooltip: DashboardCursorSync;
   time: any;
-  liveNow: boolean;
+  liveNow?: boolean;
   private originalTime: any;
   timepicker: any;
   templating: { list: any[] };
   private originalTemplating: any;
   annotations: { list: AnnotationQuery[] };
-  refresh: string;
+  refresh?: string;
   snapshot: any;
   schemaVersion: number;
   version: number;
@@ -147,10 +147,10 @@ export class DashboardModel implements TimeModel {
     this.graphTooltip = data.graphTooltip || 0;
     this.time = data.time ?? { from: 'now-6h', to: 'now' };
     this.timepicker = data.timepicker ?? {};
-    this.liveNow = Boolean(data.liveNow);
+    this.liveNow = data.liveNow;
     this.templating = this.ensureListExist(data.templating);
     this.annotations = this.ensureListExist(data.annotations);
-    this.refresh = data.refresh || '';
+    this.refresh = data.refresh;
     this.snapshot = data.snapshot;
     this.schemaVersion = data.schemaVersion ?? 0;
     this.fiscalYearStartMonth = data.fiscalYearStartMonth ?? 0;

diff --git a/public/app/features/dashboard/state/TimeModel.ts b/public/app/features/dashboard/state/TimeModel.ts
@@ -3,7 +3,7 @@ import { TimeRange, TimeZone } from '@grafana/data';
 export interface TimeModel {
   time: any;
   fiscalYearStartMonth?: number;
-  refresh?: string | false;
+  refresh?: string;
   timepicker: any;
   getTimezone(): TimeZone;
   timeRangeUpdated(timeRange: TimeRange): void;