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 | | ||
| 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
| 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
| 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
| 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 
afinder-mozilla force-pushed the all_sheriffed_frameworks branch from f5cd9ef to 49af16f Compare 3 participants
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.