diff --git a/public/app/features/alerting/unified/RuleEditorCloudRules.test.tsx b/public/app/features/alerting/unified/RuleEditorCloudRules.test.tsx index a3398383671..21ba09cb12f 100644 --- a/public/app/features/alerting/unified/RuleEditorCloudRules.test.tsx +++ b/public/app/features/alerting/unified/RuleEditorCloudRules.test.tsx @@ -45,7 +45,7 @@ describe('RuleEditor cloud', () => { it('can create a new cloud alert', async () => { const { user } = renderRuleEditor(); - const removeExpressionsButtons = screen.getAllByLabelText('Remove expression'); + const removeExpressionsButtons = screen.getAllByLabelText(/Remove expression/); expect(removeExpressionsButtons).toHaveLength(2); // Needs to wait for featrue discovery API call to finish - Check if ruler enabled @@ -58,7 +58,7 @@ describe('RuleEditor cloud', () => { await user.click(switchToCloudButton); //expressions are removed after switching to data-source managed - expect(screen.queryAllByLabelText('Remove expression')).toHaveLength(0); + expect(screen.queryAllByLabelText(/Remove expression/)).toHaveLength(0); expect(screen.getByTestId(selectors.components.DataSourcePicker.inputV2)).toBeInTheDocument(); diff --git a/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx b/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx index 9b863517b14..e07f62cd7c1 100644 --- a/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx +++ b/public/app/features/alerting/unified/RuleEditorGrafanaRules.test.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import { renderRuleEditor, ui } from 'test/helpers/alertingRuleEditor'; -import { clickSelectOption } from 'test/helpers/selectOptionInTest'; +import { clickSelectOption, selectOptionInTest } from 'test/helpers/selectOptionInTest'; import { screen } from 'test/test-utils'; import { byRole } from 'testing-library-selector'; @@ -22,6 +22,20 @@ jest.setTimeout(60 * 1000); setupMswServer(); +const dataSources = { + default: mockDataSource( + { + type: 'prometheus', + name: 'Prom', + uid: PROMETHEUS_DATASOURCE_UID, + isDefault: true, + }, + { alerting: true, module: 'core:plugin/prometheus' } + ), +}; + +setupDataSources(dataSources.default); + describe('RuleEditor grafana managed rules', () => { beforeEach(() => { jest.clearAllMocks(); @@ -44,19 +58,6 @@ describe('RuleEditor grafana managed rules', () => { it('can create new grafana managed alert', async () => { const capture = captureRequests((r) => r.method === 'POST' && r.url.includes('/api/ruler/')); - const dataSources = { - default: mockDataSource( - { - type: 'prometheus', - name: 'Prom', - uid: PROMETHEUS_DATASOURCE_UID, - isDefault: true, - }, - { alerting: true, module: 'core:plugin/prometheus' } - ), - }; - - setupDataSources(dataSources.default); const { user } = renderRuleEditor(); @@ -75,4 +76,32 @@ describe('RuleEditor grafana managed rules', () => { const serializedRequests = await serializeRequests(requests); expect(serializedRequests).toMatchSnapshot(); }); + + // FIXME: This should work (i.e. the necessary setup is done, and the preview should trigger an error) + // but for some reason the alert error doesn't render + it.skip('shows an error when trying to use time series as alert condition', async () => { + setupDataSources(dataSources.default); + const { user } = renderRuleEditor(); + + // Select Prometheus data source + const dataSourceInput = await ui.inputs.dataSource.find(); + await user.click(dataSourceInput); + await user.click(await screen.findByRole('button', { name: new RegExp(dataSources.default.name) })); + + // Change to `code` editor, and type in something that would give us a time series response + await user.click(screen.getByLabelText(/code/i)); + await user.click(screen.getByTestId('data-testid Query field')); + // We have to escape the curly braces because they have special meaning to the RTL keyboard API + await user.keyboard('sum(counters_logins{{}})'); + + // Expand the options and select "range" instead + await user.click(screen.getByRole('button', { name: /type: instant/i })); + await user.click(screen.getByLabelText(/range/i)); + + await user.click(screen.getByRole('button', { name: /remove expression "b"/i })); + await selectOptionInTest(await screen.findByLabelText(/input/i), 'A'); + + await user.click(ui.buttons.preview.get()); + expect(await screen.findByText(/you cannot use time series data as an alert condition/i)).toBeInTheDocument(); + }); }); diff --git a/public/app/features/alerting/unified/components/expressions/Expression.tsx b/public/app/features/alerting/unified/components/expressions/Expression.tsx index 8284061a208..aef1f40b677 100644 --- a/public/app/features/alerting/unified/components/expressions/Expression.tsx +++ b/public/app/features/alerting/unified/components/expressions/Expression.tsx @@ -369,13 +369,17 @@ const Header: FC = ({
{getExpressionLabel(queryType)}
- onSetCondition(query.refId)} isCondition={alertCondition} /> + onSetCondition(query.refId)} + isCondition={alertCondition} + /> diff --git a/public/app/features/alerting/unified/components/expressions/ExpressionStatusIndicator.tsx b/public/app/features/alerting/unified/components/expressions/ExpressionStatusIndicator.tsx index 65a7cac211f..1b63f3b16ad 100644 --- a/public/app/features/alerting/unified/components/expressions/ExpressionStatusIndicator.tsx +++ b/public/app/features/alerting/unified/components/expressions/ExpressionStatusIndicator.tsx @@ -10,15 +10,18 @@ import { isGrafanaRecordingRuleByType } from '../../utils/rules'; interface AlertConditionProps { isCondition?: boolean; onSetCondition?: () => void; + refId?: string; } -export const ExpressionStatusIndicator = ({ isCondition, onSetCondition }: AlertConditionProps) => { +export const ExpressionStatusIndicator = ({ isCondition, onSetCondition, refId }: AlertConditionProps) => { const styles = useStyles2(getStyles); const { watch } = useFormContext(); const type = watch('type'); const isGrafanaRecordingRule = type ? isGrafanaRecordingRuleByType(type) : false; const conditionText = isGrafanaRecordingRule ? 'Recording rule output' : 'Alert condition'; - const makeConditionText = isGrafanaRecordingRule ? 'Set as recording rule output' : 'Set as alert condition'; + + const setAsConditionText = refId ? `Set "${refId}" as alert condition` : 'Set as alert condition'; + const makeConditionText = isGrafanaRecordingRule ? 'Set as recording rule output' : setAsConditionText; if (isCondition) { return ; diff --git a/public/app/features/alerting/unified/components/rule-editor/ExpressionsEditor.tsx b/public/app/features/alerting/unified/components/rule-editor/ExpressionsEditor.tsx index a9708fc79cd..87952c4b19f 100644 --- a/public/app/features/alerting/unified/components/rule-editor/ExpressionsEditor.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/ExpressionsEditor.tsx @@ -9,7 +9,7 @@ import { AlertQuery } from 'app/types/unified-alerting-dto'; import { Expression } from '../expressions/Expression'; -import { errorFromPreviewData, warningFromSeries } from './util'; +import { errorFromCurrentCondition, errorFromPreviewData, warningFromSeries } from './util'; interface Props { condition: string | null; @@ -45,7 +45,11 @@ export const ExpressionsEditor = ({ const data = panelData[query.refId]; const isAlertCondition = condition === query.refId; - const error = data ? errorFromPreviewData(data) : undefined; + + const errorFromCondition = data && isAlertCondition ? errorFromCurrentCondition(data) : undefined; + const errorFromPreview = data ? errorFromPreviewData(data) : undefined; + const error = errorFromPreview || errorFromCondition; + const warning = data ? warningFromSeries(data.series) : undefined; return ( diff --git a/public/app/features/expressions/components/Threshold.tsx b/public/app/features/expressions/components/Threshold.tsx index 720096b90bf..fac93a84c0b 100644 --- a/public/app/features/expressions/components/Threshold.tsx +++ b/public/app/features/expressions/components/Threshold.tsx @@ -1,5 +1,6 @@ import { css } from '@emotion/css'; import { AnyAction } from '@reduxjs/toolkit'; +import { uniqueId } from 'lodash'; import * as React from 'react'; import { FormEvent, useEffect, useReducer } from 'react'; @@ -85,11 +86,13 @@ export const Threshold = ({ labelWidth, onChange, refIds, query, onError, useHys const hysteresisEnabled = Boolean(config.featureToggles?.recoveryThreshold) && useHysteresis; + const id = uniqueId('threshold-'); + return ( <> - - @@ -142,10 +145,10 @@ export const Threshold = ({ labelWidth, onChange, refIds, query, onError, useHys return (
- {/* This is to enhance the user experience for mouse users. - The onBlur event in RecoveryThresholdRow inputs triggers validations, - but we want to skip them when the switch is clicked as this click should inmount this component. - To achieve this, we use the onMouseDown event to set a flag, which is later utilized in the onBlur event to bypass validations. + {/* This is to enhance the user experience for mouse users. + The onBlur event in RecoveryThresholdRow inputs triggers validations, + but we want to skip them when the switch is clicked as this click should inmount this component. + To achieve this, we use the onMouseDown event to set a flag, which is later utilized in the onBlur event to bypass validations. The onMouseDown event precedes the onBlur event, unlike onchange. */} {/*Disabling the a11y rules here as the InlineSwitch handles keyboard interactions */} @@ -201,7 +204,7 @@ function RecoveryThresholdRow({ isRange, condition, onError, dispatch, allowOnbl return ; } - /* We prioritize the onMouseDown event over the onBlur event. This is because the onBlur event is executed before the onChange event that we have + /* We prioritize the onMouseDown event over the onBlur event. This is because the onBlur event is executed before the onChange event that we have in the hysteresis checkbox, and because of that, we were validating when unchecking the switch. We need to uncheck the switch before the onBlur event is executed.*/ interface RecoveryProps { diff --git a/public/test/helpers/alertingRuleEditor.tsx b/public/test/helpers/alertingRuleEditor.tsx index 0a2a8634ce8..39898405bdd 100644 --- a/public/test/helpers/alertingRuleEditor.tsx +++ b/public/test/helpers/alertingRuleEditor.tsx @@ -39,6 +39,7 @@ export const ui = { save: byRole('button', { name: 'Save rule' }), addAnnotation: byRole('button', { name: /Add info/ }), addLabel: byRole('button', { name: /Add label/ }), + preview: byRole('button', { name: /^Preview$/ }), }, }; export function renderRuleEditor(identifier?: string, recording?: 'recording' | 'grafana-recording') {