[v11.3.x] Loki query option: fix step validation (#97416)

Loki query option: fix step validation (#96977)

(cherry picked from commit f78cd0761b)

Co-authored-by: Matias Chomicki <matyax@gmail.com>

public/app/plugins/datasource/loki/querybuilder/components/LokiQueryBuilderOptions.test.tsx

@@ -100,13 +100,13 @@ describe('LokiQueryBuilderOptions', () => {
     expect(screen.getByText('Resolution: 1/2')).toBeInTheDocument();
   });
 
-  it('does not shows resolution field if resolution is not set', async () => {
+  it('does not show resolution field if resolution is not set', async () => {
     setup({ expr: 'rate({foo="bar"}[5m]' });
     await userEvent.click(screen.getByRole('button', { name: /Options/ }));
     expect(screen.queryByText('Resolution')).not.toBeInTheDocument();
   });
 
-  it('does not shows resolution field if resolution is set to default value 1', async () => {
+  it('does not show resolution field if resolution is set to default value 1', async () => {
     setup({ expr: 'rate({foo="bar"}[5m]', resolution: 1 });
     await userEvent.click(screen.getByRole('button', { name: /Options/ }));
     expect(screen.queryByText('Resolution')).not.toBeInTheDocument();
@@ -121,8 +121,9 @@ describe('LokiQueryBuilderOptions', () => {
     ).toBeInTheDocument();
   });
 
-  it('shows correct options for metric query with invalid step', async () => {
-    setup({ expr: 'rate({foo="bar"}[5m]', step: 'abc' });
+  it.each(['abc', 10])('shows correct options for metric query with invalid step', async (step: string | number) => {
+    // @ts-expect-error Expected for backward compatibility test
+    setup({ expr: 'rate({foo="bar"}[5m]', step });
     expect(screen.queryByText('Line limit: 20')).not.toBeInTheDocument();
     expect(screen.getByText('Type: Range')).toBeInTheDocument();
     expect(screen.getByText('Step: Invalid value')).toBeInTheDocument();
@@ -134,19 +135,19 @@ describe('LokiQueryBuilderOptions', () => {
     expect(screen.getByText(/Invalid step/)).toBeInTheDocument();
   });
 
-  it('does not shows error when valid value in step', async () => {
+  it('does not show error when valid value in step', async () => {
     setup({ expr: 'rate({foo="bar"}[5m]', step: '1m' });
     await userEvent.click(screen.getByRole('button', { name: /Options/ }));
     expect(screen.queryByText(/Invalid step/)).not.toBeInTheDocument();
   });
 
-  it('does not shows error when valid millisecond value in step', async () => {
+  it('does not show error when valid millisecond value in step', async () => {
     setup({ expr: 'rate({foo="bar"}[5m]', step: '1ms' });
     await userEvent.click(screen.getByRole('button', { name: /Options/ }));
     expect(screen.queryByText(/Invalid step/)).not.toBeInTheDocument();
   });
 
-  it('does not shows error when valid day value in step', async () => {
+  it('does not show error when valid day value in step', async () => {
     setup({ expr: 'rate({foo="bar"}[5m]', step: '1d' });
     await userEvent.click(screen.getByRole('button', { name: /Options/ }));
     expect(screen.queryByText(/Invalid step/)).not.toBeInTheDocument();
@@ -162,9 +163,28 @@ describe('LokiQueryBuilderOptions', () => {
     await userEvent.click(screen.getByRole('button', { name: /Options/ }));
     expect(screen.queryByText(/Instant/)).not.toBeInTheDocument();
   });
+
+  it('allows to clear step input', async () => {
+    setup({ expr: 'rate({foo="bar"}[5m]', step: '4s' });
+    await userEvent.click(screen.getByRole('button', { name: /Options/ }));
+    expect(screen.getByDisplayValue('4s')).toBeInTheDocument();
+    await userEvent.clear(screen.getByDisplayValue('4s'));
+    expect(screen.queryByDisplayValue('4s')).not.toBeInTheDocument();
+  });
+
+  it('should transform non duration numbers to duration', async () => {
+    const onChange = jest.fn();
+    setup({ expr: 'rate({foo="bar"}[5m]', step: '4' }, onChange);
+    await userEvent.click(screen.getByRole('button', { name: /Options/ }));
+    expect(onChange).toHaveBeenCalledWith({
+      refId: 'A',
+      expr: 'rate({foo="bar"}[5m]',
+      step: '4s',
+    });
+  });
 });
 
-function setup(queryOverrides: Partial<LokiQuery> = {}) {
+function setup(queryOverrides: Partial<LokiQuery> = {}, onChange = jest.fn()) {
   const props = {
     query: {
       refId: 'A',
@@ -172,7 +192,7 @@ function setup(queryOverrides: Partial<LokiQuery> = {}) {
       ...queryOverrides,
     },
     onRunQuery: jest.fn(),
-    onChange: jest.fn(),
+    onChange,
     maxLines: 20,
     queryStats: { streams: 0, chunks: 0, bytes: 0, entries: 0 },
   };

public/app/plugins/datasource/loki/querybuilder/components/LokiQueryBuilderOptions.tsx

@@ -1,5 +1,5 @@
 import { trim } from 'lodash';
-import { useMemo, useState } from 'react';
+import { useEffect, useMemo, useState } from 'react';
 import * as React from 'react';
 
 import { CoreApp, isValidDuration, isValidGrafanaDuration, SelectableValue } from '@grafana/data';
@@ -29,6 +29,15 @@ export const LokiQueryBuilderOptions = React.memo<Props>(
   ({ app, query, onChange, onRunQuery, maxLines, queryStats }) => {
     const [splitDurationValid, setSplitDurationValid] = useState(true);
 
+    useEffect(() => {
+      if (query.step && !isValidGrafanaDuration(`${query.step}`) && parseInt(query.step, 10)) {
+        onChange({
+          ...query,
+          step: `${parseInt(query.step, 10)}s`,
+        });
+      }
+    }, [onChange, query]);
+
     const onQueryTypeChange = (value: LokiQueryType) => {
       onChange({ ...query, queryType: value });
       onRunQuery();
@@ -92,10 +101,10 @@ export const LokiQueryBuilderOptions = React.memo<Props>(
     }
 
     const isValidStep = useMemo(() => {
-      if (!query.step || isValidGrafanaDuration(query.step) || !isNaN(Number(query.step))) {
+      if (!query.step) {
         return true;
       }
-      return false;
+      return typeof query.step === 'string' && isValidGrafanaDuration(query.step) && !isNaN(parseInt(query.step, 10));
     }, [query.step]);
 
     return (
@@ -151,7 +160,7 @@ export const LokiQueryBuilderOptions = React.memo<Props>(
                   className="width-6"
                   placeholder={'auto'}
                   type="string"
-                  defaultValue={query.step ?? ''}
+                  value={query.step ?? ''}
                   onCommitChange={onStepChange}
                 />
               </EditorField>