From 7e85e4d0960a20ac90556c03bda3883593266a3f Mon Sep 17 00:00:00 2001 From: Andreas Opferkuch Date: Thu, 2 Apr 2020 13:43:32 +0200 Subject: [PATCH] Utils: Use 's' as default for unit-less intervals (#23248) * Utils: Use 's' as default for unit-less intervals If the user specifies a string that is a unit-less number, it is assumed that they meant seconds. Fixes #22362 * Rephrase tooltip for better line break position --- public/app/core/utils/kbn.test.ts | 23 +++++++++++++++++++ public/app/core/utils/kbn.ts | 15 +++++++++++- .../variables/interval/actions.test.ts | 7 ++++-- .../components/PromExploreQueryEditor.tsx | 4 +++- .../PromExploreQueryEditor.test.tsx.snap | 2 +- 5 files changed, 46 insertions(+), 5 deletions(-) diff --git a/public/app/core/utils/kbn.test.ts b/public/app/core/utils/kbn.test.ts index df31dbbdc2b..80dd53ed5b3 100644 --- a/public/app/core/utils/kbn.test.ts +++ b/public/app/core/utils/kbn.test.ts @@ -50,3 +50,26 @@ describe('Chcek KBN value formats', () => { }); } }); + +describe('describe_interval', () => { + it('falls back to seconds if input is a number', () => { + expect(kbn.describe_interval('123')).toEqual({ + sec: 1, + type: 's', + count: 123, + }); + }); + + it('parses a valid time unt string correctly', () => { + expect(kbn.describe_interval('123h')).toEqual({ + sec: 3600, + type: 'h', + count: 123, + }); + }); + + it('fails if input is invalid', () => { + expect(() => kbn.describe_interval('123xyz')).toThrow(); + expect(() => kbn.describe_interval('xyz')).toThrow(); + }); +}); diff --git a/public/app/core/utils/kbn.ts b/public/app/core/utils/kbn.ts index 77bc252f6f6..ca786d56b25 100644 --- a/public/app/core/utils/kbn.ts +++ b/public/app/core/utils/kbn.ts @@ -200,9 +200,22 @@ kbn.calculateInterval = (range: TimeRange, resolution: number, lowLimitInterval: }; kbn.describe_interval = (str: string) => { + // Default to seconds if no unit is provided + if (Number(str)) { + return { + sec: kbn.intervals_in_seconds.s, + type: 's', + count: parseInt(str, 10), + }; + } + const matches = str.match(kbn.interval_regex); if (!matches || !has(kbn.intervals_in_seconds, matches[2])) { - throw new Error('Invalid interval string, expecting a number followed by one of "Mwdhmsy"'); + throw new Error( + `Invalid interval string, has to be either unit-less or end with one of the following units: "${Object.keys( + kbn.intervals_in_seconds + ).join(', ')}"` + ); } else { return { sec: kbn.intervals_in_seconds[matches[2]], diff --git a/public/app/features/variables/interval/actions.test.ts b/public/app/features/variables/interval/actions.test.ts index 973f7862c6f..c47d391f213 100644 --- a/public/app/features/variables/interval/actions.test.ts +++ b/public/app/features/variables/interval/actions.test.ts @@ -18,6 +18,7 @@ import { AppEvents, dateTime } from '@grafana/data'; import { getTimeSrv, setTimeSrv, TimeSrv } from '../../dashboard/services/TimeSrv'; import { TemplateSrv } from '../../templating/template_srv'; import { intervalBuilder } from '../shared/testing/builders'; +import kbn from 'app/core/utils/kbn'; describe('interval actions', () => { variableAdapters.setInit(() => [createIntervalVariableAdapter()]); @@ -65,7 +66,7 @@ describe('interval actions', () => { .withId('0') .withQuery('1s,1m,1h,1d') .withAuto(true) - .withAutoMin('1') // illegal interval string + .withAutoMin('1xyz') // illegal interval string .build(); const appEventMock = ({ emit: jest.fn(), @@ -80,7 +81,9 @@ describe('interval actions', () => { expect(appEventMock.emit).toHaveBeenCalledTimes(1); expect(appEventMock.emit).toHaveBeenCalledWith(AppEvents.alertError, [ 'Templating', - 'Invalid interval string, expecting a number followed by one of "Mwdhmsy"', + `Invalid interval string, has to be either unit-less or end with one of the following units: "${Object.keys( + kbn.intervals_in_seconds + ).join(', ')}"`, ]); setTimeSrv(originalTimeSrv); }); diff --git a/public/app/plugins/datasource/prometheus/components/PromExploreQueryEditor.tsx b/public/app/plugins/datasource/prometheus/components/PromExploreQueryEditor.tsx index f7f8d9cf34e..5add4d8face 100644 --- a/public/app/plugins/datasource/prometheus/components/PromExploreQueryEditor.tsx +++ b/public/app/plugins/datasource/prometheus/components/PromExploreQueryEditor.tsx @@ -48,7 +48,9 @@ export function PromExploreQueryEditor(props: Props) { onKeyDownFunc={onReturnKeyDown} value={query.interval || ''} hasTooltip={true} - tooltipContent={'Needs to be a valid time unit string, for example 5s, 1m, 3h, 1d, 1y'} + tooltipContent={ + 'Time units can be used here, for example: 5s, 1m, 3h, 1d, 1y (Default if no unit is specified: s)' + } /> } /> diff --git a/public/app/plugins/datasource/prometheus/components/__snapshots__/PromExploreQueryEditor.test.tsx.snap b/public/app/plugins/datasource/prometheus/components/__snapshots__/PromExploreQueryEditor.test.tsx.snap index f8995e8918a..2670a2a4237 100644 --- a/public/app/plugins/datasource/prometheus/components/__snapshots__/PromExploreQueryEditor.test.tsx.snap +++ b/public/app/plugins/datasource/prometheus/components/__snapshots__/PromExploreQueryEditor.test.tsx.snap @@ -8,7 +8,7 @@ exports[`PromExploreQueryEditor should render component 1`] = ` label="Step" onChangeFunc={[Function]} onKeyDownFunc={[Function]} - tooltipContent="Needs to be a valid time unit string, for example 5s, 1m, 3h, 1d, 1y" + tooltipContent="Time units can be used here, for example: 5s, 1m, 3h, 1d, 1y (Default if no unit is specified: s)" value="1s" /> }