Alerting: Include error from current condition when previewing queries (#98202)

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();
 

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();
+  });
 });

public/app/features/alerting/unified/components/expressions/Expression.tsx

@@ -369,13 +369,17 @@ const Header: FC<HeaderProps> = ({
           <div>{getExpressionLabel(queryType)}</div>
         </Stack>
         <Spacer />
-        <ExpressionStatusIndicator onSetCondition={() => onSetCondition(query.refId)} isCondition={alertCondition} />
+        <ExpressionStatusIndicator
+          refId={refId}
+          onSetCondition={() => onSetCondition(query.refId)}
+          isCondition={alertCondition}
+        />
         <IconButton
           name="trash-alt"
           variant="secondary"
           className={styles.mutedIcon}
           onClick={onRemoveExpression}
-          tooltip="Remove expression"
+          tooltip={`Remove expression "${refId}"`}
         />
       </Stack>
     </header>

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<RuleFormValues>();
   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 <Badge key="condition" color="green" icon="check" text={conditionText} />;

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 (

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 (
     <>
       <InlineFieldRow>
-        <InlineField label="Input" labelWidth={labelWidth}>
-          <Select onChange={onRefIdChange} options={refIds} value={query.expression} width={20} />
+        <InlineField label="Input" labelWidth={labelWidth} htmlFor={id}>
+          <Select inputId={id} onChange={onRefIdChange} options={refIds} value={query.expression} width={20} />
         </InlineField>
       </InlineFieldRow>
       <InlineFieldRow>
@@ -142,10 +145,10 @@ export const Threshold = ({ labelWidth, onChange, refIds, query, onError, useHys
 
     return (
       <div className={styles.hysteresis}>
-        {/* 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 <RecoveryForSingleValue allowOnblur={allowOnblur} />;
   }
 
-  /* 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 {

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') {