Loki query option: fix step validation (#96977)

This commit is contained in:
Matias Chomicki 2024-12-04 15:35:51 +00:00 committed by GitHub
parent 6fe184a565
commit f78cd0761b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 42 additions and 13 deletions

View File

@ -102,13 +102,13 @@ describe('LokiQueryBuilderOptions', () => {
expect(screen.queryByText(/Direction/)).not.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();
@ -123,8 +123,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();
@ -136,19 +137,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();
@ -164,9 +165,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',
@ -174,7 +194,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 },
};

View File

@ -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';
@ -30,6 +30,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();
@ -93,10 +102,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 (
@ -152,7 +161,7 @@ export const LokiQueryBuilderOptions = React.memo<Props>(
className="width-6"
placeholder={'auto'}
type="string"
defaultValue={query.step ?? ''}
value={query.step ?? ''}
onCommitChange={onStepChange}
/>
</EditorField>