Address edge case

mozilla · Feb 5, 2025 · d615404 · d615404
1 parent d047ca9
commit d615404
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 20 deletions.
diff --git a/treeherder/webapp/api/performance_data.py b/treeherder/webapp/api/performance_data.py
@@ -6,7 +6,17 @@
 import django_filters
 from django.conf import settings
 from django.db import transaction
-from django.db.models import Case, CharField, Count, Q, Subquery, Value, When
+from django.db.models import (
+    BooleanField,
+    Case,
+    CharField,
+    Count,
+    F,
+    Q,
+    Subquery,
+    Value,
+    When,
+)
 from django.db.models.functions import Concat
 from rest_framework import exceptions, filters, generics, pagination, viewsets
 from rest_framework.response import Response
@@ -677,25 +687,47 @@ def list(self, request):
             )
 
         # TODO signature_hash is being returned for legacy support - should be removed at some point
-        self.queryset = signature_data.values(
-            "framework_id",
-            "id",
-            "lower_is_better",
-            "has_subtests",
-            "extra_options",
-            "suite",
-            "signature_hash",
-            "platform__platform",
-            "test",
-            "option_collection_id",
-            "parent_signature_id",
-            "repository_id",
-            "tags",
-            "measurement_unit",
-            "application",
-            "should_alert",
-            "alert_change_type",
-            "alert_threshold",
+        """
+        If a suite has a `should_alert` value of either null or True and there is a suite-level value set,
+        but the subtests do not have the `should_alert` set to True, then those subtests will not trigger an alert.
+        A null value for these subtests indicates that the `should_alert` parameter is set to False.
+        """
+        self.queryset = (
+            signature_data.prefetch_related("performancedatum")
+            .annotate(
+                modified_should_alert=Case(
+                    When(
+                        parent_signature__id__isnull=False,
+                        parent_signature__should_alert__in=[True, None],
+                        should_alert__isnull=True,
+                        performancedatum__value__isnull=False,
+                        then=Value(False),
+                    ),
+                    default=F("should_alert"),
+                    output_field=BooleanField(),
+                )
+            )
+            .values(
+                "modified_should_alert",
+                "framework_id",
+                "id",
+                "lower_is_better",
+                "has_subtests",
+                "extra_options",
+                "suite",
+                "signature_hash",
+                "platform__platform",
+                "test",
+                "option_collection_id",
+                "parent_signature_id",
+                "repository_id",
+                "tags",
+                "measurement_unit",
+                "application",
+                "should_alert",
+                "alert_change_type",
+                "alert_threshold",
+            )
         )
 
         signature_ids = [item["id"] for item in list(self.queryset)]

diff --git a/treeherder/webapp/api/performance_serializers.py b/treeherder/webapp/api/performance_serializers.py
@@ -426,10 +426,12 @@ class PerformanceSummarySerializer(serializers.ModelSerializer):
     job_ids = serializers.ListField(child=serializers.IntegerField(), default=[])
     data = PerformanceDatumSerializer(read_only=True, many=True, default=[])
     repository_name = serializers.CharField()
+    modified_should_alert = serializers.BooleanField()
 
     class Meta:
         model = PerformanceSignature
         fields = [
+            "modified_should_alert",
             "signature_id",
             "framework_id",
             "signature_hash",

diff --git a/ui/perfherder/graphs/LegendCard.jsx b/ui/perfherder/graphs/LegendCard.jsx
@@ -211,6 +211,9 @@ const LegendCard = ({
         <div className="small">{`should_alert: ${
           series.shouldAlert !== false
         }`}</div>
+        <div className="small">{`modified_should_alert: ${
+          series.modifiedShouldAlert !== false
+        }`}</div>
         <div className="small">{`alert_change_type: ${
           series.alertChangeType === 1 ? 'absolute' : 'percentage'
         }`}</div>

diff --git a/ui/perfherder/perf-helpers/helpers.js b/ui/perfherder/perf-helpers/helpers.js
@@ -865,6 +865,7 @@ export const createGraphData = (
       shouldAlert: series.should_alert,
       alertChangeType: series.alert_change_type,
       alertThreshold: series.alert_threshold,
+      modifiedShouldAlert: series.modified_should_alert,
       replicates,
     };
   });