Skip to content

FXP-4047 Implement functionality to filter by all sheriffed frameworks in Alerts View #8810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

FXP-4047 Implement functionality to filter by all sheriffed frameworks in Alerts View #8810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion tests/webapp/api/test_performance_alertsummary_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@

from tests.conftest import create_perf_alert
from treeherder.model.models import Push
from treeherder.perf.models import PerformanceAlert, PerformanceAlertSummary
from treeherder.perf.models import (
PerformanceAlert,
PerformanceAlertSummary,
PerformanceFramework,
)


@pytest.fixture
Expand Down Expand Up @@ -130,6 +134,35 @@ def test_alert_summaries_get(
}


def test_alert_summaries_sheriffed_frameworks(
client, test_perf_alert_summary, test_perf_alert_summary_2, test_perf_framework
):
# verify that we return only the sheriffed framework alerts if show_sheriffed_frameworks is True
sheriffed_framework = PerformanceFramework.objects.create(name="browsertime", enabled=True)
non_sheriffed_framework = PerformanceFramework.objects.create(
name="platform_microbench", enabled=True
)
test_perf_alert_summary.framework = sheriffed_framework
test_perf_alert_summary.save()
test_perf_alert_summary_2.framework = non_sheriffed_framework
test_perf_alert_summary_2.save()
resp = client.get(
reverse("performance-alert-summaries-list"), {"show_sheriffed_frameworks": True}
)
assert resp.status_code == 200
# Should only include the sheriffed alert (browsertime)
assert len(resp.json()["results"]) == 1
assert resp.json()["results"][0]["framework"] == sheriffed_framework.id
test_perf_alert_summary.framework = test_perf_framework
test_perf_alert_summary.save()
resp = client.get(
reverse("performance-alert-summaries-list"), {"show_sheriffed_frameworks": True}
)
assert resp.status_code == 200
# Should return no results, since test_talos is not a sheriffed framework
assert len(resp.json()["results"]) == 0


def test_alert_summaries_get_onhold(
client,
test_perf_alert_summary,
Expand Down
14 changes: 14 additions & 0 deletions treeherder/webapp/api/performance_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ class PerformanceAlertSummaryFilter(django_filters.FilterSet):
hide_related_and_invalid = django_filters.BooleanFilter(method="_hide_related_and_invalid")
with_assignee = django_filters.CharFilter(method="_with_assignee")
timerange = django_filters.NumberFilter(method="_timerange")
show_sheriffed_frameworks = django_filters.BooleanFilter(method="_show_sheriffed_frameworks")

def _filter_text(self, queryset, name, value):
sep = Value(" ")
Expand Down Expand Up @@ -418,6 +419,19 @@ def _timerange(self, queryset, name, value):
push__time__gt=datetime.datetime.utcfromtimestamp(int(time.time() - int(value)))
)

def _show_sheriffed_frameworks(self, queryset, name, value):
return queryset.filter(
framework__name__in=[
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create a new constant and import it

    SHERIFFED_FRAMEWORKS = [
        "browsertime",
        "raptor",
        "talos",
        "awsy",
        "build_metrics",
        "js-bench",
        "devtools",
    ]

in ui/perfherder/perf-helpers/constants.js

"browsertime",
"awsy",
"talos",
"build_metrics",
"js-bench",
"mozperftest",
"devtools",
]
)

class Meta:
model = PerformanceAlertSummary
fields = [
Expand Down
11 changes: 11 additions & 0 deletions ui/perfherder/alerts/AlertsView.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,11 @@ class AlertsView extends React.Component {
const frameworkOptions = cloneDeep(frameworks);
const ignoreFrameworks = { id: -1, name: 'all frameworks' };
frameworkOptions.unshift(ignoreFrameworks);
const ignoreNonSheriffedFrameworks = {
Copy link
Collaborator

@beatrice-acasandrei beatrice-acasandrei Jul 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A better naming might be

Suggested change
const ignoreNonSheriffedFrameworks = {
const allSheriffedFrameworks = {

which aligns better with our approach here to filter by sheriffed frameworks instead of excluding the non-sheriffed ones

id: -2,
name: 'all sheriffed frameworks',
};
frameworkOptions.unshift(ignoreNonSheriffedFrameworks);
return frameworkOptions;
};

Expand Down Expand Up @@ -207,6 +212,9 @@ class AlertsView extends React.Component {
if (listMode && params.framework === doNotFilter) {
delete params.framework;
}
if (listMode && params.framework === -2) {
Copy link
Collaborator

@beatrice-acasandrei beatrice-acasandrei Jul 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a new constant for the -2 value, similar to the doNotFilter constant with a proper comment explaining it's purpose - Constant created for UI purposes but is not a valid API parameter

Copy link
Collaborator

@beatrice-acasandrei beatrice-acasandrei Jul 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Further you can merge the if statements for the framework check since both do the same action - removing the framework url parameter

delete params.framework;
}

return params;
};
Expand Down Expand Up @@ -271,6 +279,9 @@ class AlertsView extends React.Component {
if (hideAssignedToOthers) {
params.with_assignee = user.username;
}
if (framework.id === -2) {
Copy link
Collaborator

@beatrice-acasandrei beatrice-acasandrei Jul 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the new constant for the -2 value instead of hardcoding it

params.show_sheriffed_frameworks = true;
}
}

const url = getApiUrl(
Expand Down
10 changes: 5 additions & 5 deletions ui/perfherder/alerts/AlertsViewControls.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,14 +138,14 @@ export default class AlertsViewControls extends React.Component {

let sortedFrameworks = sortData(frameworkOptions, 'name', false);
const allFrameworks = 'all frameworks';
const mozperftest = 'mozperftest';
const allSheriffedFrameworks = 'all sheriffed frameworks';
const platformMicrobench = 'platform_microbench';

sortedFrameworks = sortedFrameworks.filter(
(framework) =>
framework.name !== mozperftest &&
framework.name !== platformMicrobench &&
framework.name !== allFrameworks,
framework.name !== allFrameworks &&
framework.name !== allSheriffedFrameworks,
);

const frameworkNames =
Expand All @@ -165,8 +165,8 @@ export default class AlertsViewControls extends React.Component {
selectedItem: framework.name,
updateData: this.updateFramework,
namespace: 'framework',
pinned: [allFrameworks],
otherPinned: [mozperftest, platformMicrobench],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could maybe add telemetry to the otherPinned list since it's a framework that we don't sheriff.

pinned: [allFrameworks, allSheriffedFrameworks],
otherPinned: [platformMicrobench],
},
];

Expand Down