From 2b252b50f5ba023b64f9187e15ddf5e9257f08f4 Mon Sep 17 00:00:00 2001 From: laurenashleigh Date: Wed, 29 Oct 2025 15:59:05 +0000 Subject: [PATCH] update logic for expression parsing from panel to alerting --- .../PanelAlertTabContent.test.tsx.snap | 4 +- .../NewRuleFromPanelButton.test.tsx | 14 +++ .../NewRuleFromPanelButton.tsx | 6 +- .../RuleEditorGrafanaRules.test.tsx.snap | 12 +- .../unified/rule-editor/formProcessing.ts | 23 +++- .../alerting/unified/utils/rule-form.ts | 108 ++++++++++++++++-- .../PanelDataPane/NewAlertRuleButton.tsx | 2 + .../PanelDataAlertingTab.test.tsx.snap | 4 +- 8 files changed, 150 insertions(+), 23 deletions(-) diff --git a/public/app/features/alerting/unified/__snapshots__/PanelAlertTabContent.test.tsx.snap b/public/app/features/alerting/unified/__snapshots__/PanelAlertTabContent.test.tsx.snap index e36613807ec..d62572e1b23 100644 --- a/public/app/features/alerting/unified/__snapshots__/PanelAlertTabContent.test.tsx.snap +++ b/public/app/features/alerting/unified/__snapshots__/PanelAlertTabContent.test.tsx.snap @@ -70,7 +70,7 @@ exports[`PanelAlertTabContent Will render alerts belonging to panel and a button "refId": "B", "type": "reduce", }, - "queryType": "", + "queryType": "expression", "refId": "B", }, { @@ -105,7 +105,7 @@ exports[`PanelAlertTabContent Will render alerts belonging to panel and a button "refId": "C", "type": "threshold", }, - "queryType": "", + "queryType": "expression", "refId": "C", }, ], diff --git a/public/app/features/alerting/unified/components/panel-alerts-tab/NewRuleFromPanelButton.test.tsx b/public/app/features/alerting/unified/components/panel-alerts-tab/NewRuleFromPanelButton.test.tsx index 89b8886fb4e..f08f825ad5f 100644 --- a/public/app/features/alerting/unified/components/panel-alerts-tab/NewRuleFromPanelButton.test.tsx +++ b/public/app/features/alerting/unified/components/panel-alerts-tab/NewRuleFromPanelButton.test.tsx @@ -24,6 +24,20 @@ jest.mock('react-use', () => ({ useAsync: () => ({ loading: false, value: {} }), })); +jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), + config: { + ...jest.requireActual('@grafana/runtime').config, + featureToggles: { + createAlertRuleFromPanel: true, + }, + }, +})); + +jest.mock('../../components/AlertRuleDrawerForm', () => ({ + AlertRuleDrawerForm: () => null, +})); + describe('Analytics', () => { it('Sends log info when creating an alert rule from a panel', async () => { const panel = new PanelModel({ diff --git a/public/app/features/alerting/unified/components/panel-alerts-tab/NewRuleFromPanelButton.tsx b/public/app/features/alerting/unified/components/panel-alerts-tab/NewRuleFromPanelButton.tsx index 1a165d04c1b..f944e352c67 100644 --- a/public/app/features/alerting/unified/components/panel-alerts-tab/NewRuleFromPanelButton.tsx +++ b/public/app/features/alerting/unified/components/panel-alerts-tab/NewRuleFromPanelButton.tsx @@ -10,6 +10,7 @@ import { DashboardModel } from 'app/features/dashboard/state/DashboardModel'; import { PanelModel } from 'app/features/dashboard/state/PanelModel'; import { useSelector } from 'app/types/store'; +import { LogMessages, logInfo } from '../../Analytics'; import { AlertRuleDrawerForm } from '../../components/AlertRuleDrawerForm'; import { createPanelAlertRuleNavigation } from '../../utils/navigation'; import { panelToRuleFormValues } from '../../utils/rule-form'; @@ -72,7 +73,10 @@ export const NewRuleFromPanelButton = ({ dashboard, panel, className }: Props) = icon="bell" className={className} data-testid="create-alert-rule-button-drawer" - onClick={() => setIsOpen(true)} + onClick={() => { + logInfo(LogMessages.alertRuleFromPanel); + setIsOpen(true); + }} > New alert rule diff --git a/public/app/features/alerting/unified/rule-editor/__snapshots__/RuleEditorGrafanaRules.test.tsx.snap b/public/app/features/alerting/unified/rule-editor/__snapshots__/RuleEditorGrafanaRules.test.tsx.snap index 6dd5b9af077..73ac81a4a3c 100644 --- a/public/app/features/alerting/unified/rule-editor/__snapshots__/RuleEditorGrafanaRules.test.tsx.snap +++ b/public/app/features/alerting/unified/rule-editor/__snapshots__/RuleEditorGrafanaRules.test.tsx.snap @@ -81,7 +81,9 @@ exports[`RuleEditor grafana managed rules can create new grafana managed alert 1 "type": "and", }, "query": { - "params": [], + "params": [ + "B", + ], }, "reducer": { "params": [], @@ -99,7 +101,7 @@ exports[`RuleEditor grafana managed rules can create new grafana managed alert 1 "refId": "B", "type": "reduce", }, - "queryType": "", + "queryType": "expression", "refId": "B", }, { @@ -117,7 +119,9 @@ exports[`RuleEditor grafana managed rules can create new grafana managed alert 1 "type": "and", }, "query": { - "params": [], + "params": [ + "C", + ], }, "reducer": { "params": [], @@ -134,7 +138,7 @@ exports[`RuleEditor grafana managed rules can create new grafana managed alert 1 "refId": "C", "type": "threshold", }, - "queryType": "", + "queryType": "expression", "refId": "C", }, ], diff --git a/public/app/features/alerting/unified/rule-editor/formProcessing.ts b/public/app/features/alerting/unified/rule-editor/formProcessing.ts index 3bd0ae101c8..e31de5d0b8f 100644 --- a/public/app/features/alerting/unified/rule-editor/formProcessing.ts +++ b/public/app/features/alerting/unified/rule-editor/formProcessing.ts @@ -27,17 +27,28 @@ export function setQueryEditorSettings(values: RuleFormValues): RuleFormValues { // data queries only const dataQueries = values.queries.filter((query) => !isExpressionQuery(query.model)); - // expression queries only - const expressionQueries = values.queries.filter((query) => isExpressionQueryInAlert(query)); + // expression queries only - but filter out invalid ones that don't have a type field + const expressionQueries = values.queries.filter((query): query is AlertQuery => { + if (!isExpressionQueryInAlert(query)) { + return false; + } + // Check if the expression has a valid type field + // React Hook Form might strip the type field, so we need to check it exists + return 'type' in query.model && query.model.type !== undefined; + }); - // If we have data queries but no expressions (e.g., coming from dashboard panel), - // default to simplified mode so the form can create appropriate expressions + // If we have data queries but no VALID expressions (e.g., coming from dashboard panel with malformed expressions), + // remove the invalid expressions and set condition to empty so simplified mode can regenerate them const hasDataQueries = dataQueries.length > 0; - const hasExpressions = expressionQueries.length > 0; + const hasValidExpressions = expressionQueries.length > 0; + const totalExpressions = values.queries.filter((query) => isExpressionQueryInAlert(query)).length; + const hasInvalidExpressions = totalExpressions > expressionQueries.length; - if (hasDataQueries && !hasExpressions) { + if (hasDataQueries && (!hasValidExpressions || hasInvalidExpressions)) { return { ...values, + queries: dataQueries, // Only keep data queries, remove invalid expressions + condition: '', // Clear condition so simplified editor can set it editorSettings: { simplifiedQueryEditor: true, simplifiedNotificationEditor: true, diff --git a/public/app/features/alerting/unified/utils/rule-form.ts b/public/app/features/alerting/unified/utils/rule-form.ts index 882670b845b..5afa362d00f 100644 --- a/public/app/features/alerting/unified/utils/rule-form.ts +++ b/public/app/features/alerting/unified/utils/rule-form.ts @@ -7,6 +7,7 @@ import { ScopedVars, TimeRange, getDefaultRelativeTimeRange, + getNextRefId, rangeUtil, } from '@grafana/data'; import { PromQuery } from '@grafana/prometheus'; @@ -558,14 +559,85 @@ export const getDefaultRecordingRulesQueries = ( ]; }; -export const getDefaultExpressions = (...refIds: [string, string]) => { +const getDefaultExpressions = (...refIds: [string, string] | [string, string, string]): AlertQuery[] => { const refOne = refIds[0]; const refTwo = refIds[1]; + // If a third parameter is provided, use it as the source query refId, otherwise default to 'A' + const sourceRefId = refIds.length === 3 ? refIds[2] : 'A'; - const reduceQuery = getDefaultReduceExpression({ inputRefId: 'A', reduceRefId: refOne }); - const thresholdQuery = getDefaultThresholdExpression({ inputRefId: refOne, thresholdRefId: refTwo }); + const reduceExpression: ExpressionQuery = { + refId: refIds[0], + type: ExpressionQueryType.reduce, + datasource: { + uid: ExpressionDatasourceUID, + type: ExpressionDatasourceRef.type, + }, + conditions: [ + { + type: 'query', + evaluator: { + params: [], + type: EvalFunction.IsAbove, + }, + operator: { + type: 'and', + }, + query: { + params: [refOne], + }, + reducer: { + params: [], + type: 'last', + }, + }, + ], + reducer: 'last', + expression: sourceRefId, + }; - return [reduceQuery, thresholdQuery] as const; + const thresholdExpression: ExpressionQuery = { + refId: refTwo, + type: ExpressionQueryType.threshold, + datasource: { + uid: ExpressionDatasourceUID, + type: ExpressionDatasourceRef.type, + }, + conditions: [ + { + type: 'query', + evaluator: { + params: [0], + type: EvalFunction.IsAbove, + }, + operator: { + type: 'and', + }, + query: { + params: [refTwo], + }, + reducer: { + params: [], + type: 'last', + }, + }, + ], + expression: refOne, + }; + + return [ + { + refId: refOne, + datasourceUid: ExpressionDatasourceUID, + queryType: 'expression', + model: reduceExpression, + }, + { + refId: refTwo, + datasourceUid: ExpressionDatasourceUID, + queryType: 'expression', + model: thresholdExpression, + }, + ]; }; const getDefaultExpressionsForRecording = (refOne: string): Array> => { @@ -782,6 +854,17 @@ export const panelToRuleFormValues = async ( return undefined; } + // Add default expression queries if they don't exist + if (!queries.find((query) => query.datasourceUid === ExpressionDatasourceUID)) { + // Get the last data query's refId to use as the source for the reduce expression + const lastDataQueryRefId = queries[queries.length - 1].refId; + const reduceRefId = getNextRefId(queries); + const queriesWithReduce = [...queries, { refId: reduceRefId, datasourceUid: '', queryType: '', model: {} }]; + const thresholdRefId = getNextRefId(queriesWithReduce); + const expressions = getDefaultExpressions(reduceRefId, thresholdRefId, lastDataQueryRefId); + queries.push(...expressions); + } + const { folderTitle, folderUid } = dashboard.meta; const folder = folderUid && folderTitle @@ -797,8 +880,7 @@ export const panelToRuleFormValues = async ( folder, queries, name: panel.title, - // Condition left empty - expressions will be created in the alert rule form under advanced options - condition: '', + condition: queries[queries.length - 1].refId, annotations: [ { key: Annotation.dashboardUID, @@ -850,6 +932,17 @@ export const scenesPanelToRuleFormValues = async (vizPanel: VizPanel): Promise

query.datasourceUid === ExpressionDatasourceUID)) { + // Get the last data query's refId to use as the source for the reduce expression + const lastDataQueryRefId = grafanaQueries[grafanaQueries.length - 1].refId; + const reduceRefId = getNextRefId(grafanaQueries); + const queriesWithReduce = [...grafanaQueries, { refId: reduceRefId, datasourceUid: '', queryType: '', model: {} }]; + const thresholdRefId = getNextRefId(queriesWithReduce); + const expressions = getDefaultExpressions(reduceRefId, thresholdRefId, lastDataQueryRefId); + grafanaQueries.push(...expressions); + } + const { folderTitle, folderUid } = dashboard.state.meta; const folder = @@ -866,8 +959,7 @@ export const scenesPanelToRuleFormValues = async (vizPanel: VizPanel): Promise

{ + logInfo(LogMessages.alertRuleFromPanel); setIsOpen(true); }} > diff --git a/public/app/features/dashboard-scene/panel-edit/PanelDataPane/__snapshots__/PanelDataAlertingTab.test.tsx.snap b/public/app/features/dashboard-scene/panel-edit/PanelDataPane/__snapshots__/PanelDataAlertingTab.test.tsx.snap index 858dc3812c1..0f2b54da1d0 100644 --- a/public/app/features/dashboard-scene/panel-edit/PanelDataPane/__snapshots__/PanelDataAlertingTab.test.tsx.snap +++ b/public/app/features/dashboard-scene/panel-edit/PanelDataPane/__snapshots__/PanelDataAlertingTab.test.tsx.snap @@ -65,7 +65,7 @@ exports[`PanelAlertTabContent Will render alerts belonging to panel and a button "refId": "B", "type": "reduce", }, - "queryType": "", + "queryType": "expression", "refId": "B", }, { @@ -100,7 +100,7 @@ exports[`PanelAlertTabContent Will render alerts belonging to panel and a button "refId": "C", "type": "threshold", }, - "queryType": "", + "queryType": "expression", "refId": "C", }, ],