Fix alert evaluation logic and issue with calculating min and max of columns without numbers

client/app/pages/alert/components/Criteria.jsx

@@ -67,7 +67,9 @@ export default function Criteria({ columnNames, resultValues, alertOptions, onCh
       <small className="alert-criteria-hint">
         Max column value is{" "}
         <code className="p-0">
-          {toString(Math.max(...resultValues.map((o) => o[alertOptions.column]))) || "unknown"}
+          {toString(
+            Math.max(...resultValues.map((o) => Number(o[alertOptions.column])).filter((value) => !isNaN(value)))
+          ) || "unknown"}
         </code>
       </small>
     );
@@ -76,7 +78,9 @@ export default function Criteria({ columnNames, resultValues, alertOptions, onCh
       <small className="alert-criteria-hint">
         Min column value is{" "}
         <code className="p-0">
-          {toString(Math.min(...resultValues.map((o) => o[alertOptions.column]))) || "unknown"}
+          {toString(
+            Math.min(...resultValues.map((o) => Number(o[alertOptions.column])).filter((value) => !isNaN(value)))
+          ) || "unknown"}
         </code>
       </small>
     );

redash/models/__init__.py

@@ -916,6 +916,8 @@ def next_state(op, value, threshold):
 
     if op(value, threshold):
         new_state = Alert.TRIGGERED_STATE
+    elif not value_is_number and op not in [OPERATORS.get("!="), OPERATORS.get("=="), OPERATORS.get("equals")]:
+        new_state = Alert.UNKNOWN_STATE
     else:
         new_state = Alert.OK_STATE
 
@@ -967,18 +969,22 @@ class Alert(TimestampMixin, BelongsToOrgMixin, db.Model):
             else:
                 selector = self.options["selector"]
 
-            if selector == "max":
+            try:
-                max_val = float("-inf")
+                if selector == "max":
-                for i in range(0, len(data["rows"])):
+                    max_val = float("-inf")
-                    max_val = max(max_val, data["rows"][i][self.options["column"]])
+                    for i in range(len(data["rows"])):
-                value = max_val
+                        max_val = max(max_val, float(data["rows"][i][self.options["column"]]))
-            elif selector == "min":
+                    value = max_val
-                min_val = float("inf")
+                elif selector == "min":
-                for i in range(0, len(data["rows"])):
+                    min_val = float("inf")
-                    min_val = min(min_val, data["rows"][i][self.options["column"]])
+                    for i in range(len(data["rows"])):
-                value = min_val
+                        min_val = min(min_val, float(data["rows"][i][self.options["column"]]))
-            else:
+                    value = min_val
-                value = data["rows"][0][self.options["column"]]
+                else:
+                    value = data["rows"][0][self.options["column"]]
+
+            except ValueError:
+                return self.UNKNOWN_STATE
 
             threshold = self.options["value"]
 
@@ -1007,7 +1013,6 @@ class Alert(TimestampMixin, BelongsToOrgMixin, db.Model):
         result_table = []  # A two-dimensional array which can rendered as a table in Mustache
         for row in data["rows"]:
             result_table.append([row[col["name"]] for col in data["columns"]])
-        print("OPTIONS", self.options)
         context = {
             "ALERT_NAME": self.name,
             "ALERT_URL": "{host}/alerts/{alert_id}".format(host=host, alert_id=self.id),

tests/models/test_alerts.py

@@ -71,23 +71,45 @@ class TestAlertEvaluate(BaseTestCase):
         alert = self.create_alert(results)
         self.assertEqual(alert.evaluate(), Alert.UNKNOWN_STATE)
 
-    def test_evaluates_correctly_with_max_selector(self):
-        results = {"rows": [{"foo": 1}, {"foo": 2}], "columns": [{"name": "foo", "type": "STRING"}]}
-        alert = self.create_alert(results)
-        alert.options["selector"] = "max"
-        self.assertEqual(alert.evaluate(), Alert.OK_STATE)
-
-    def test_evaluates_correctly_with_min_selector(self):
-        results = {"rows": [{"foo": 2}, {"foo": 1}], "columns": [{"name": "foo", "type": "STRING"}]}
-        alert = self.create_alert(results)
-        alert.options["selector"] = "min"
-        self.assertEqual(alert.evaluate(), Alert.TRIGGERED_STATE)
-
     def test_evaluates_correctly_with_first_selector(self):
-        results = {"rows": [{"foo": 1}, {"foo": 2}], "columns": [{"name": "foo", "type": "STRING"}]}
+        results = {"rows": [{"foo": 1}, {"foo": 2}], "columns": [{"name": "foo", "type": "INTEGER"}]}
         alert = self.create_alert(results)
         alert.options["selector"] = "first"
         self.assertEqual(alert.evaluate(), Alert.TRIGGERED_STATE)
+        results = {
+            "rows": [{"foo": "test"}, {"foo": "test"}, {"foo": "test"}],
+            "columns": [{"name": "foo", "type": "STRING"}],
+        }
+        alert = self.create_alert(results)
+        alert.options["selector"] = "first"
+        alert.options["op"] = "<"
+        self.assertEqual(alert.evaluate(), Alert.UNKNOWN_STATE)
+
+    def test_evaluates_correctly_with_min_selector(self):
+        results = {"rows": [{"foo": 2}, {"foo": 1}], "columns": [{"name": "foo", "type": "INTEGER"}]}
+        alert = self.create_alert(results)
+        alert.options["selector"] = "min"
+        self.assertEqual(alert.evaluate(), Alert.TRIGGERED_STATE)
+        results = {
+            "rows": [{"foo": "test"}, {"foo": "test"}, {"foo": "test"}],
+            "columns": [{"name": "foo", "type": "STRING"}],
+        }
+        alert = self.create_alert(results)
+        alert.options["selector"] = "min"
+        self.assertEqual(alert.evaluate(), Alert.UNKNOWN_STATE)
+
+    def test_evaluates_correctly_with_max_selector(self):
+        results = {"rows": [{"foo": 1}, {"foo": 2}], "columns": [{"name": "foo", "type": "INTEGER"}]}
+        alert = self.create_alert(results)
+        alert.options["selector"] = "max"
+        self.assertEqual(alert.evaluate(), Alert.OK_STATE)
+        results = {
+            "rows": [{"foo": "test"}, {"foo": "test"}, {"foo": "test"}],
+            "columns": [{"name": "foo", "type": "STRING"}],
+        }
+        alert = self.create_alert(results)
+        alert.options["selector"] = "max"
+        self.assertEqual(alert.evaluate(), Alert.UNKNOWN_STATE)
 
 
 class TestNextState(TestCase):