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
2 times, most recently
from
a9b64b9 to
f7600ab
Compare
afinder-mozilla
requested review from
beatrice-acasandrei and
esanuandra
as code owners
| @@ -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
ui/perfherder/alerts/AlertsView.jsx
Outdated
| @@ -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
ui/perfherder/alerts/AlertsView.jsx
Outdated
| @@ -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
ui/perfherder/alerts/AlertsView.jsx
Outdated
| @@ -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 has been idle for 2 months, are there plans to pick this back up? if not, can you close this? |
Hi Joel! Sorry for the lack of communication here, I have plans to update the patch and address the code review requests soon. We should keep this PR open. |
afinder-mozilla
force-pushed
the
all_sheriffed_frameworks
branch
2 times, most recently
from
457d3b4 to
ec33da5
Compare
treeherder/config/settings.py
Outdated
| SHERIFFED_FRAMEWORKS = [ | ||
| "browsertime", | ||
| "raptor", | ||
| "talos", | ||
| "awsy", | ||
| "build_metrics", | ||
| "js-bench", | ||
| "devtools", | ||
| ] |
There was a problem hiding this comment.
| SHERIFFED_FRAMEWORKS = [ | |
| "browsertime", | |
| "raptor", | |
| "talos", | |
| "awsy", | |
| "build_metrics", | |
| "js-bench", | |
| "devtools", | |
| ] | |
| SHERIFFED_FRAMEWORKS = [ | |
| "browsertime", | |
| "mozperftest", | |
| "talos", | |
| "awsy", | |
| "build_metrics", | |
| "js-bench", | |
| "devtools", | |
| ] |
afinder-mozilla
force-pushed
the
all_sheriffed_frameworks
branch
from
ec33da5 to
f4396c1
Compare
afinder-mozilla
force-pushed
the
all_sheriffed_frameworks
branch
from
f4396c1 to
f5cd9ef
Compare
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.