From c0e7701ad4ab949b3e20afbb288464341ebb50ee Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Wed, 11 Jun 2025 16:36:14 +0200 Subject: [PATCH] Loki: Fix issue where step parameter using a template variable was marked as invalid (#106541) * Loki: Step with template variable should be valid * Fix lint --- .../LokiQueryBuilderOptions.test.tsx | 42 +++++++++++++++++++ .../components/LokiQueryBuilderOptions.tsx | 14 ++++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/public/app/plugins/datasource/loki/querybuilder/components/LokiQueryBuilderOptions.test.tsx b/public/app/plugins/datasource/loki/querybuilder/components/LokiQueryBuilderOptions.test.tsx index 51d805667bf..b852f1520cd 100644 --- a/public/app/plugins/datasource/loki/querybuilder/components/LokiQueryBuilderOptions.test.tsx +++ b/public/app/plugins/datasource/loki/querybuilder/components/LokiQueryBuilderOptions.test.tsx @@ -273,6 +273,48 @@ describe('LokiQueryBuilderOptions', () => { }); }); }); + + describe('Step validation', () => { + it('considers empty step as valid', async () => { + setup({ expr: 'rate({foo="bar"}[5m]' }); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); + expect(screen.queryByText(/Invalid step/)).not.toBeInTheDocument(); + }); + + it('considers variable step that exists in the datasource as valid', async () => { + const datasource = createLokiDatasource(); + datasource.getVariables = jest.fn().mockReturnValue(['$interval']); + setup({ expr: 'rate({foo="bar"}[5m]', step: '$interval' }, undefined, { datasource }); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); + expect(screen.queryByText(/Invalid step/)).not.toBeInTheDocument(); + }); + + it('considers variable step that does not exist in the datasource as invalid', async () => { + const datasource = createLokiDatasource(); + datasource.getVariables = jest.fn().mockReturnValue(['$interval']); + setup({ expr: 'rate({foo="bar"}[5m]', step: '$custom' }, undefined, { datasource }); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); + expect(screen.getByText(/Invalid step/)).toBeInTheDocument(); + }); + + it('considers valid duration step as valid', 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('considers invalid step as invalid', async () => { + setup({ expr: 'rate({foo="bar"}[5m]', step: 'invalid' }); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); + expect(screen.getByText(/Invalid step/)).toBeInTheDocument(); + }); + + it('considers non-duration number as invalid', async () => { + setup({ expr: 'rate({foo="bar"}[5m]', step: '123' }); + await userEvent.click(screen.getByRole('button', { name: /Options/ })); + expect(screen.getByText(/Invalid step/)).toBeInTheDocument(); + }); + }); }); function setup(queryOverrides: Partial = {}, onChange = jest.fn(), propOverrides: Partial = {}) { diff --git a/public/app/plugins/datasource/loki/querybuilder/components/LokiQueryBuilderOptions.tsx b/public/app/plugins/datasource/loki/querybuilder/components/LokiQueryBuilderOptions.tsx index f692bf9496a..d04e9fc42f6 100644 --- a/public/app/plugins/datasource/loki/querybuilder/components/LokiQueryBuilderOptions.tsx +++ b/public/app/plugins/datasource/loki/querybuilder/components/LokiQueryBuilderOptions.tsx @@ -138,8 +138,18 @@ export const LokiQueryBuilderOptions = React.memo( if (!query.step) { return true; } - return typeof query.step === 'string' && isValidGrafanaDuration(query.step) && !isNaN(parseInt(query.step, 10)); - }, [query.step]); + + if (typeof query.step === 'string') { + // If we use a variable as step, we consider it valid + if (datasource.getVariables().includes(query.step)) { + return true; + } + // Check if the step is a valid Grafana duration + return isValidGrafanaDuration(query.step) && !isNaN(parseInt(query.step, 10)); + } + + return false; + }, [query.step, datasource]); return (