Bug 1251353 - Put filtering options directly into compare table

This gives us the filtering view for free on all comparison views (e.g. compare subtest, compare e10s-vs-non-e10s), whereas previously we only had it on the main comparison view
mozilla · Feb 25, 2016 · fdb4b84 · fdb4b84
1 parent 7145aff
commit fdb4b84
Show file tree

Hide file tree

Showing 6 changed files with 84 additions and 57 deletions.
diff --git a/ui/js/controllers/perf/compare.js b/ui/js/controllers/perf/compare.js
@@ -302,20 +302,6 @@ perf.controller('CompareResultsCtrl', [
         $scope.dataLoading = true;
         $scope.getCompareClasses = PhCompare.getCompareClasses;
 
-        $scope.updateFilters = function() {
-            $state.transitionTo('compare', {
-                framework: $scope.filterOptions.framework.id,
-                filter: $scope.filterOptions.filter,
-                showOnlyImportant: Boolean($scope.filterOptions.showOnlyImportant) ? undefined : 0,
-                showOnlyConfident: Boolean($scope.filterOptions.showOnlyConfident) ? 1 : undefined
-            }, {
-                location: true,
-                inherit: true,
-                relative: $state.$current,
-                notify: false
-            });
-        };
-
         var optionCollectionMap = {};
         var loadRepositories = ThRepositoryModel.load();
         var loadOptions = ThOptionCollectionModel.getMap().then(
@@ -347,6 +333,24 @@ perf.controller('CompareResultsCtrl', [
                     parseInt($stateParams.showOnlyConfident)
             };
 
+            $scope.$watchGroup(['filterOptions.frameworkId',
+                                'filterOptions.filter',
+                                'filterOptions.showOnlyImportant',
+                                'filterOptions.showOnlyConfident'],
+                     function() {
+                         $state.transitionTo('compare', {
+                             framework: $scope.filterOptions.framework.id,
+                             filter: $scope.filterOptions.filter,
+                             showOnlyImportant: Boolean($scope.filterOptions.showOnlyImportant) ? undefined : 0,
+                             showOnlyConfident: Boolean($scope.filterOptions.showOnlyConfident) ? 1 : undefined
+                         }, {
+                             location: true,
+                             inherit: true,
+                             relative: $state.$current,
+                             notify: false
+                         });
+                     });
+
             $scope.originalProject = ThRepositoryModel.getRepo(
                 $stateParams.originalProject);
             $scope.newProject = ThRepositoryModel.getRepo(
@@ -402,6 +406,30 @@ perf.controller('CompareSubtestResultsCtrl', [
                                    $scope.testList[0].split(' ')[0]);
             window.document.title = $scope.subtestTitle + " subtest comparison";
 
+            $scope.filterOptions = {
+                filter: $stateParams.filter || "",
+                showOnlyImportant: $stateParams.showOnlyImportant !== undefined &&
+                    parseInt($stateParams.showOnlyImportant),
+                showOnlyConfident: $stateParams.showOnlyConfident !== undefined &&
+                    parseInt($stateParams.showOnlyConfident)
+            };
+
+            $scope.$watchGroup(['filterOptions.filter',
+                                'filterOptions.showOnlyImportant',
+                                'filterOptions.showOnlyConfident'],
+                     function() {
+                         $state.transitionTo('comparesubtest', {
+                             filter: $scope.filterOptions.filter,
+                             showOnlyImportant: Boolean($scope.filterOptions.showOnlyImportant) ? 1 : undefined,
+                             showOnlyConfident: Boolean($scope.filterOptions.showOnlyConfident) ? 1 : undefined
+                         }, {
+                             location: true,
+                             inherit: true,
+                             relative: $state.$current,
+                             notify: false
+                         });
+                     });
+
             $scope.testList.forEach(function(testName) {
                 $scope.titles[testName] = $scope.platformList[0] + ': ' +
                     testName.replace('summary ', '');

diff --git a/ui/js/directives/perf/compare.js b/ui/js/directives/perf/compare.js
@@ -6,31 +6,31 @@ treeherder.directive(
         return {
             templateUrl: 'partials/perf/comparetable.html',
             scope: {
+                frameworks: '=',
                 titles: '=',
                 compareResults: '=',
                 testList: '=',
-                frameworkId: '=',
-                filter: '=',
-                showOnlyImportant: '=',
-                showOnlyConfident: '='
+                filterOptions: '=',
+                filterByFramework: '@'
             },
             link: function(scope, element, attrs) {
                 scope.getCompareClasses = PhCompare.getCompareClasses;
                 function filter(item, matchText) {
                     return !matchText || item.toLowerCase().indexOf(matchText.toLowerCase()) > (-1);
                 }
                 function shouldBeShown(result) {
-                    return (scope.frameworkId === undefined || result.frameworkId === scope.frameworkId) &&
-                        (!scope.showOnlyImportant || result.isMeaningful) &&
-                        (!scope.showOnlyConfident || result.isConfident);
+                    return (!scope.filterByFramework || _.isUndefined(scope.filterOptions.framework) ||
+                            result.frameworkId === scope.filterOptions.framework.id) &&
+                        (!scope.filterOptions.showOnlyImportant || result.isMeaningful) &&
+                        (!scope.filterOptions.showOnlyConfident || result.isConfident);
                 }
                 function filterResult(results, key) {
-                    if (scope.filter === undefined) {
+                    if (_.isUndefined(scope.filterOptions.filter)) {
                         return results;
                     }
                     return _.filter(results, function(result) {
                         var testCondition = key + ' ' + result.name;
-                        return _.every(scope.filter.split(' '), function(matchText) {
+                        return _.every(scope.filterOptions.filter.split(' '), function(matchText) {
                             return filter(testCondition, matchText) && shouldBeShown(result);
                         });
                     });
@@ -50,7 +50,10 @@ treeherder.directive(
                     scope.hasNoResults = _.isEmpty(scope.filteredResultList);
                 }
 
-                scope.$watchGroup(['frameworkId', 'filter', 'showOnlyImportant', 'showOnlyConfident'],
+                scope.$watchGroup([
+                    'filterOptions.framework', 'filterOptions.filter',
+                    'filterOptions.showOnlyImportant',
+                    'filterOptions.showOnlyConfident'],
                                   function() {
                                       updateFilteredTestList();
                                   });

diff --git a/ui/js/perfapp.js b/ui/js/perfapp.js
@@ -21,11 +21,11 @@ perf.config(function($compileProvider, $httpProvider, $stateProvider, $urlRouter
         controller: 'GraphsCtrl'
     }).state('compare', {
         templateUrl: 'partials/perf/comparectrl.html',
-        url: '/compare?originalProject&originalRevision&newProject&newRevision&hideMinorChanges&framework&filter&showOnlyImportant&showOnlyConfident',
+        url: '/compare?originalProject&originalRevision&newProject&newRevision&hideMinorChanges&framework&filter&showOnlyImportant&showOnlyConfidentw',
         controller: 'CompareResultsCtrl'
     }).state('comparesubtest', {
         templateUrl: 'partials/perf/comparesubtestctrl.html',
-        url: '/comparesubtest?originalProject&originalRevision&newProject&newRevision&originalSignature&newSignature',
+        url: '/comparesubtest?originalProject&originalRevision&newProject&newRevision&originalSignature&newSignature&filter&showOnlyImportant&showOnlyConfident',
         controller: 'CompareSubtestResultsCtrl'
     }).state('comparechooser', {
         title: 'Compare',

diff --git a/ui/partials/perf/comparectrl.html b/ui/partials/perf/comparectrl.html
@@ -14,38 +14,13 @@ <h1>Perfherder Compare Revisions</h1>
         <strong>tests with no results:</strong>
         {{testNoResults}}
       </div>
-      <form class="form-inline">
-        <div class="form-group">
-          <select ng-model="filterOptions.framework"
-                  ng-options="framework.name for framework in frameworks track by framework.id"
-                  ng-change="updateFilters()"/>
-        </div>
-        &nbsp;
-        <div class="form-group">
-            <input id="filter" type="text" class="form-control" ng-model="filterOptions.filter" placeholder="filter text e.g. linux tp5o" ng-change="updateFilters()" ng-model-options="{debounce: 250}"/>
-        </div>
-        <div class="checkbox" uib-tooltip="Non-trivial changes (2%+)">
-          <label>
-            <input type="checkbox" ng-model="filterOptions.showOnlyImportant" ng-change="updateFilters()"/>
-            Show only important changes
-          </label>
-        </div>
-        <div class="checkbox" uib-tooltip="At least 6 datapoints OR 2+ datapoints and a large difference">
-          <label>
-            <input type="checkbox" ng-model="filterOptions.showOnlyConfident" ng-change="updateFilters()"/>
-            Hide uncertain results
-          </label>
-        </div>
-      </form>
-      <hr/>
       <ph-compare-table
          titles="titles"
          test-list="testList"
          compare-results="compareResults"
-         framework-id="filterOptions.framework.id"
-         filter="filterOptions.filter"
-         show-only-important="filterOptions.showOnlyImportant"
-         show-only-confident="filterOptions.showOnlyConfident">
+         frameworks="frameworks"
+         filter-options="filterOptions"
+         filter-by-framework="1">
       </ph-compare-table>
     </div>
   </div>

diff --git a/ui/partials/perf/comparesubtestctrl.html b/ui/partials/perf/comparesubtestctrl.html
@@ -15,9 +15,7 @@ <h1>{{subtestTitle}} subtest summary</h1>
        titles="titles"
        test-list="testList"
        compare-results="compareResults"
-       filter=""
-       show-only-important="0"
-       show-only-confident="0">
+       filter-options="filterOptions">
     </ph-compare-table>
   </div>
 </div>
diff --git a/ui/partials/perf/comparetable.html b/ui/partials/perf/comparetable.html
@@ -1,3 +1,26 @@
+<form class="form-inline">
+  <div class="form-group" ng-show="filterByFramework">
+    <select ng-model="filterOptions.framework"
+            ng-options="framework.name for framework in frameworks track by framework.id"/>
+    &nbsp;
+  </div>
+  <div class="form-group">
+    <input id="filter" type="text" class="form-control" ng-model="filterOptions.filter" placeholder="filter text e.g. linux tp5o" ng-model-options="{debounce: 250}"/>
+  </div>
+  <div class="checkbox" uib-tooltip="Non-trivial changes (2%+)">
+    <label>
+      <input type="checkbox" ng-model="filterOptions.showOnlyImportant"/>
+      Show only important changes
+    </label>
+  </div>
+  <div class="checkbox" uib-tooltip="At least 6 datapoints OR 2+ datapoints and a large difference">
+    <label>
+      <input type="checkbox" ng-model="filterOptions.showOnlyConfident"/>
+      Hide uncertain results
+    </label>
+  </div>
+</form>
+<hr/>
 <table class="table table-condensed compare-table" style="table-layout: fixed;" ng-repeat="compareResults in filteredResultList | orderBy: 'testName'">
   <tbody>
     <tr class="subtest-header">