From 888965023cc9ad836e5888b4a345dfd43667c189 Mon Sep 17 00:00:00 2001
From: Sonia Aguilar <33540275+soniaAguilarPeiron@users.noreply.github.com>
Date: Fri, 24 Jan 2025 16:45:46 +0100
Subject: [PATCH] Alerting: Fix getting targets in dag when using classic
 condition (#99503)

* fix getting targets in dag when using classic condition

* remove query fingerprinting
the DAG function is probably fast enough and the fingerprinting function is too complex to make it work for all expression types

---------

Co-authored-by: Gilles De Mey <gilles.de.mey@gmail.com>
---
 .../components/rule-editor/dag.test.ts        | 41 ++-----------------
 .../unified/components/rule-editor/dag.ts     | 40 ++++++++----------
 2 files changed, 20 insertions(+), 61 deletions(-)

diff --git a/public/app/features/alerting/unified/components/rule-editor/dag.test.ts b/public/app/features/alerting/unified/components/rule-editor/dag.test.ts
index 914420e5b06..fdb5b22cea4 100644
--- a/public/app/features/alerting/unified/components/rule-editor/dag.test.ts
+++ b/public/app/features/alerting/unified/components/rule-editor/dag.test.ts
@@ -2,10 +2,9 @@ import { Graph } from 'app/core/utils/dag';
 import { AlertQuery } from 'app/types/unified-alerting-dto';
 
 import {
-  _createDagFromQueries,
   _getDescendants,
   _getOriginsOfRefId,
-  fingerPrintQueries,
+  createDagFromQueries,
   fingerprintGraph,
   parseRefsFromMathExpression,
 } from './dag';
@@ -45,7 +44,7 @@ describe('working with dag', () => {
       },
     ] as AlertQuery[];
 
-    const dag = _createDagFromQueries(queries);
+    const dag = createDagFromQueries(queries);
 
     expect(Object.keys(dag.nodes)).toHaveLength(4);
 
@@ -84,9 +83,9 @@ describe('working with dag', () => {
       },
     ] as AlertQuery[];
 
-    expect(() => _createDagFromQueries(queries)).not.toThrow();
+    expect(() => createDagFromQueries(queries)).not.toThrow();
 
-    const dag = _createDagFromQueries(queries);
+    const dag = createDagFromQueries(queries);
 
     expect(Object.keys(dag.nodes)).toHaveLength(1);
 
@@ -157,36 +156,4 @@ describe('fingerprints', () => {
 
     expect(fingerprintGraph(graph)).toMatchInlineSnapshot(`"A:B: B:C:A, D C::B D:B:"`);
   });
-
-  test('Queries fingerprint', () => {
-    const queries = [
-      {
-        refId: 'A',
-        queryType: 'query',
-        model: {
-          refId: 'A',
-          expression: '',
-        },
-      },
-      {
-        refId: 'B',
-        queryType: 'query',
-        model: {
-          refId: 'B',
-          expression: 'A',
-        },
-      },
-      {
-        refId: 'C',
-        queryType: 'query',
-        model: {
-          refId: 'C',
-          expression: '$B > 0',
-          type: 'math',
-        },
-      },
-    ] as AlertQuery[];
-
-    expect(fingerPrintQueries(queries)).toMatchInlineSnapshot(`"Aquery,BAquery,C$B > 0math"`);
-  });
 });
diff --git a/public/app/features/alerting/unified/components/rule-editor/dag.ts b/public/app/features/alerting/unified/components/rule-editor/dag.ts
index 51d519854f5..8ca3c0c1969 100644
--- a/public/app/features/alerting/unified/components/rule-editor/dag.ts
+++ b/public/app/features/alerting/unified/components/rule-editor/dag.ts
@@ -1,23 +1,15 @@
 import { compact, memoize, uniq } from 'lodash';
-import memoizeOne from 'memoize-one';
 
 import { Edge, Graph, Node } from 'app/core/utils/dag';
 import { isExpressionQuery } from 'app/features/expressions/guards';
+import { ExpressionQuery, ExpressionQueryType } from 'app/features/expressions/types';
 import { AlertQuery } from 'app/types/unified-alerting-dto';
 
-// memoized version of _createDagFromQueries to prevent recreating the DAG if no sources or targets are modified
-export const createDagFromQueries = memoizeOne(
-  _createDagFromQueries,
-  (previous: Parameters<typeof _createDagFromQueries>, next: Parameters<typeof _createDagFromQueries>) => {
-    return fingerPrintQueries(previous[0]) === fingerPrintQueries(next[0]);
-  }
-);
-
 /**
  * Turn the array of alert queries (this means data queries and expressions)
  * in to a DAG, a directed acyclical graph
  */
-export function _createDagFromQueries(queries: AlertQuery[]): Graph {
+export function createDagFromQueries(queries: AlertQuery[]): Graph {
   const graph = new Graph();
 
   const nodes = queries.map((query) => query.refId);
@@ -28,12 +20,9 @@ export function _createDagFromQueries(queries: AlertQuery[]): Graph {
       return;
     }
     const source = query.refId;
-    const isMathExpression = query.model.type === 'math';
 
     // some expressions have multiple targets (like the math expression)
-    const targets = isMathExpression
-      ? parseRefsFromMathExpression(query.model.expression ?? '')
-      : [query.model.expression];
+    const targets = getTargets(query.model);
 
     targets.forEach((target) => {
       const isSelf = source === target;
@@ -47,6 +36,19 @@ export function _createDagFromQueries(queries: AlertQuery[]): Graph {
   return graph;
 }
 
+function getTargets(model: ExpressionQuery) {
+  const isMathExpression = model.type === ExpressionQueryType.math;
+  const isClassicCondition = model.type === ExpressionQueryType.classic;
+
+  if (isMathExpression) {
+    return parseRefsFromMathExpression(model.expression ?? '');
+  }
+  if (isClassicCondition) {
+    return model.conditions?.map((c) => c.query.params[0]) ?? [];
+  }
+  return [model.expression];
+}
+
 /**
  * parse an expression like "$A > $B" or "${FOO BAR} > 0" to an array of refIds
  */
@@ -129,13 +131,3 @@ export function fingerprintGraph(graph: Graph) {
     })
     .join(' ');
 }
-
-// create a unique fingerprint of the array of queries
-export function fingerPrintQueries(queries: AlertQuery[]) {
-  return queries
-    .map((query) => {
-      const type = isExpressionQuery(query.model) ? query.model.type : query.queryType;
-      return query.refId + (query.model.expression ?? '') + type;
-    })
-    .join();
-}