From a5d890984bb1c88fbb9e83db511eddf23010f130 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Mon, 5 Feb 2024 12:32:59 +0100 Subject: [PATCH] Dashbboard: Fixes time picker schema default issues (#81847) * Dashbboard: Fixes time picker schema default issues * Fix tests * fix imports --- .../kinds/core/dashboard/schema-reference.md | 8 +-- kinds/dashboard/dashboard_kind.cue | 8 +-- .../raw/dashboard/x/dashboard_types.gen.ts | 8 +-- pkg/kinds/dashboard/dashboard_spec_gen.go | 8 +-- .../transformSceneToSaveModel.test.ts.snap | 50 +------------------ .../transformSceneToSaveModel.ts | 41 +++++++++++---- .../DashboardPrompt/DashboardPrompt.tsx | 2 +- .../DashboardSettings/TimePickerSettings.tsx | 2 +- .../dashboard/services/TimeSrv.test.ts | 10 ++-- .../features/dashboard/services/TimeSrv.ts | 6 +-- .../dashboard/state/DashboardModel.ts | 8 +-- .../app/features/dashboard/state/TimeModel.ts | 2 +- .../panel/panellinks/specs/link_srv.test.ts | 2 +- 13 files changed, 64 insertions(+), 91 deletions(-) diff --git a/docs/sources/developers/kinds/core/dashboard/schema-reference.md b/docs/sources/developers/kinds/core/dashboard/schema-reference.md index d8c585fbae32..b1e7fb7ea9d3 100644 --- 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.
This will keep data "moving left" regardless of the query refresh rate. This setting helps
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
to see if the version has changed since the last time. | | `snapshot` | [Snapshot](#snapshot) | No | | A dashboard snapshot shares an interactive dashboard publicly.
It is a read-only version of a dashboard, and is not editable.
It is possible to create a snapshot of a snapshot.
Grafana strips away all sensitive information from the dashboard.
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 index e2e76e4d66f2..ac0fddf3212d 100644 --- 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 index 2a58b2f1b550..6e42cab81117 100644 --- 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,7 +654,7 @@ 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. */ @@ -662,11 +662,11 @@ export interface TimePickerConfig { /** * Interval options available in the refresh picker dropdown. */ - refresh_intervals: Array; + refresh_intervals?: Array; /** * Selectable options available in the time picker dropdown. Has no effect on provisioned dashboard. */ - time_options: Array; + time_options?: Array; } export const defaultTimePickerConfig: Partial = { @@ -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 index 4f9d9dcad489..eb45c900be23 100644 --- a/pkg/kinds/dashboard/dashboard_spec_gen.go +++ b/pkg/kinds/dashboard/dashboard_spec_gen.go @@ -744,7 +744,7 @@ type Spec struct { Panels []any `json:"panels,omitempty"` // Refresh rate of dashboard. Represented via interval string, e.g. "5s", "1m", "1h", "1d". - Refresh *any `json:"refresh,omitempty"` + Refresh *string `json:"refresh,omitempty"` // 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. @@ -853,16 +853,16 @@ type ThresholdsMode string // It defines the default config for the time picker and the refresh picker for the specific dashboard. type TimePickerConfig struct { // Whether timepicker is visible or not. - Hidden bool `json:"hidden"` + Hidden *bool `json:"hidden,omitempty"` // 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 `json:"nowDelay,omitempty"` // Interval options available in the refresh picker dropdown. - RefreshIntervals []string `json:"refresh_intervals"` + RefreshIntervals []string `json:"refresh_intervals,omitempty"` // Selectable options available in the time picker dropdown. Has no effect on provisioned dashboard. - TimeOptions []string `json:"time_options"` + TimeOptions []string `json:"time_options,omitempty"` } // Maps text values to a color or different display text and color. diff --git a/public/app/features/dashboard-scene/serialization/__snapshots__/transformSceneToSaveModel.test.ts.snap b/public/app/features/dashboard-scene/serialization/__snapshots__/transformSceneToSaveModel.test.ts.snap index e1e73f86f7d3..60da71a31c6b 100644 --- a/public/app/features/dashboard-scene/serialization/__snapshots__/transformSceneToSaveModel.test.ts.snap +++ b/public/app/features/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 index e4b5d8799f16..ee8ee8c36545 100644 --- 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( + { + 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(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 index b10c291649cb..6a1c8544a71f 100644 --- 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 index 1147f67006d5..0c0845b79950 100644 --- 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 index 0fc4ba613ac5..e604828080ee 100644 --- 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 index 8c3d3f7ac848..297757650bbf 100644 --- 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 index 3ab1e476b028..360c89791bfc 100644 --- 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 index 539f4299886f..870e150d174c 100644 --- 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; diff --git a/public/app/features/panel/panellinks/specs/link_srv.test.ts b/public/app/features/panel/panellinks/specs/link_srv.test.ts index 7075fe98f99e..cd5dd3b11ef1 100644 --- a/public/app/features/panel/panellinks/specs/link_srv.test.ts +++ b/public/app/features/panel/panellinks/specs/link_srv.test.ts @@ -33,7 +33,7 @@ describe('linkSrv', () => { const timeSrv = new TimeSrv({} as ContextSrv); timeSrv.init(_dashboard); timeSrv.setTime({ from: 'now-1h', to: 'now' }); - _dashboard.refresh = false; + _dashboard.refresh = undefined; setTimeSrv(timeSrv); templateSrv = initTemplateSrv('key', [