FXP-4047 Implement functionality to filter by all sheriffed frameworks in Alerts View #8810
Conversation
afinder-mozilla
force-pushed
the
all_sheriffed_frameworks
branch
4 times, most recently
from
9b0ff0a to
2461797
Compare
afinder-mozilla
force-pushed
the
all_sheriffed_frameworks
branch
from
2461797 to
a9b64b9
Compare
afinder-mozilla
force-pushed
the
all_sheriffed_frameworks
branch
from
a9b64b9 to
f7600ab
Compare
afinder-mozilla
requested review from
beatrice-acasandrei and
esanuandra
as code owners
| @@ -165,8 +165,8 @@ export default class AlertsViewControls extends React.Component { | |||
| selectedItem: framework.name, | |||
| updateData: this.updateFramework, | |||
| namespace: 'framework', | |||
| pinned: [allFrameworks], | |||
| otherPinned: [mozperftest, platformMicrobench], | |||
There was a problem hiding this comment.
Could maybe add telemetry to the otherPinned list since it's a framework that we don't sheriff.
| @@ -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=[ | |||
There was a problem hiding this comment.
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
| @@ -207,6 +212,9 @@ class AlertsView extends React.Component { | |||
| if (listMode && params.framework === doNotFilter) { | |||
| delete params.framework; | |||
| } | |||
| if (listMode && params.framework === -2) { | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Further you can merge the if statements for the framework check since both do the same action - removing the framework url parameter
| @@ -271,6 +279,9 @@ class AlertsView extends React.Component { | |||
| if (hideAssignedToOthers) { | |||
| params.with_assignee = user.username; | |||
| } | |||
| if (framework.id === -2) { | |||
There was a problem hiding this comment.
Use the new constant for the -2 value instead of hardcoding it
| @@ -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 = { | |||
There was a problem hiding this comment.
A better naming might be
| const ignoreNonSheriffedFrameworks = { | |
| const allSheriffedFrameworks = { |
which aligns better with our approach here to filter by sheriffed frameworks instead of excluding the non-sheriffed ones
This PR adds functionality for filtering the list of available alert summaries to only show the sheriffed framework alerts and exclude the non-sheriffed ones (for example, the platform_microbench alerts).
Selecting the "all sheriffed frameworks" option in the dropdown will only display the sheriffed framework alerts.