From d61540425eb42e44abc371a9b886051ee6f629f2 Mon Sep 17 00:00:00 2001 From: Beatrice Acasandrei Date: Wed, 5 Feb 2025 08:40:08 +0200 Subject: [PATCH] Address edge case --- treeherder/webapp/api/performance_data.py | 72 +++++++++++++------ .../webapp/api/performance_serializers.py | 2 + ui/perfherder/graphs/LegendCard.jsx | 3 + ui/perfherder/perf-helpers/helpers.js | 1 + 4 files changed, 58 insertions(+), 20 deletions(-) diff --git a/treeherder/webapp/api/performance_data.py b/treeherder/webapp/api/performance_data.py index 54f5de06a4b..356f54acce9 100644 --- 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 index 8d44693cf50..90e2989e752 100644 --- 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 index 748a966984f..8af2f9a1fc4 100644 --- a/ui/perfherder/graphs/LegendCard.jsx +++ b/ui/perfherder/graphs/LegendCard.jsx @@ -211,6 +211,9 @@ const LegendCard = ({
{`should_alert: ${ series.shouldAlert !== false }`}
+
{`modified_should_alert: ${ + series.modifiedShouldAlert !== false + }`}
{`alert_change_type: ${ series.alertChangeType === 1 ? 'absolute' : 'percentage' }`}
diff --git a/ui/perfherder/perf-helpers/helpers.js b/ui/perfherder/perf-helpers/helpers.js index a8facd8df10..fd352c1c7f5 100644 --- 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, }; });