Refactor query property watching

Query property changes should only trigger reloads when the query hasn't been switched simultaneously

app/assets/javascripts/angular/directives/work_packages/query-form-directive.js

@@ -30,26 +30,66 @@ angular.module('openproject.workPackages.directives')
 
 .directive('queryForm', ['WorkPackagesTableHelper', 'WorkPackageService', function(WorkPackagesTableHelper, WorkPackageService) {
 
-  var latestQueryReference;
-
   return {
     restrict: 'EA',
 
     compile: function(tElement) {
       return {
         pre: function(scope) {
-          latestQueryReference = scope.query;
           scope.showQueryOptions = false;
 
-          scope.$watch('query.groupBy', function(newValue, oldValue) {
-            if (scope.query !== latestQueryReference) {
-              latestQueryReference = scope.query;
-            } else if (newValue !== oldValue && oldValue !== undefined) {
-              // TODO find out why newValue get set to undefined on initial page load
-              scope.updateResults();
-              scope.updateBackUrl();
+          function fetchSums() {
+            scope.withLoading(WorkPackageService.getWorkPackagesSums, [scope.projectIdentifier, scope.query, scope.columns])
+              .then(function(data){
+                angular.forEach(scope.columns, function(column, i){
+                  column.total_sum = data.column_sums[i];
+                });
+              });
+          }
+
+          function querySwitched(currentProperties, formerProperties) {
+            if (formerProperties === undefined) {
+              return true;
+            } else {
+              return formerProperties.id !== currentProperties.id;
+            }
+          }
+
+          function queryPropertiesChanged(currentProperties, formerProperties) {
+            if (formerProperties === undefined) return false;
+
+            var groupByChanged = currentProperties.groupBy !== formerProperties.groupBy;
+            var sortElementsChanged = JSON.stringify(currentProperties.sortElements) !== JSON.stringify(formerProperties.sortElements);
+
+            return groupByChanged || sortElementsChanged;
+          }
+
+          function observedQueryProperties() {
+            var query = scope.query;
+
+            if (query !== undefined) {
+              /* Oberve a few properties to avoid a full deep watch,
+                 filters are being watched within their own directive scope */
+              return {
+                id: query.id,
+                groupBy: query.groupBy,
+                sortElements: query.sortation.sortElements,
+                columnNames: query.getColumnNames()
+              };
+            }
+          }
+
+          scope.$watch(observedQueryProperties, function(newProperties, oldProperties) {
+            if (!querySwitched(newProperties, oldProperties)) {
+              if (queryPropertiesChanged(newProperties, oldProperties)) {
+                scope.updateResults();
+                scope.updateBackUrl();
+              } else if (!angular.equals(newProperties.columnNames, oldProperties.columnNames)) {
+                fetchSums();
+                scope.updateBackUrl();
+              }
             }
-          });
+          }, true);
         }
       };
     }

app/assets/javascripts/angular/directives/work_packages/work-package-total-sums-directive.js

@@ -36,32 +36,6 @@ angular.module('openproject.workPackages.directives')
 
   return {
     restrict: 'A',
-    scope: true,
-    compile: function(tElement) {
-      return {
-        pre: function(scope, iElement, iAttrs, controller) {
-          latestQueryReference = scope.query;
-
-          function fetchSums() {
-            scope.withLoading(WorkPackageService.getWorkPackagesSums, [scope.projectIdentifier, scope.query, scope.columns])
-              .then(function(data){
-                angular.forEach(scope.columns, function(column, i){
-                  column.total_sum = data.column_sums[i];
-                });
-              });
-          }
-
-          scope.$watch('columns.length', function(length, formerLength) {
-            // map columns to sums if the column data is a number
-            if (scope.query !== latestQueryReference) {
-              latestQueryReference = scope.query;
-            } else if(length >= formerLength){
-              fetchSums();
-              scope.updateBackUrl();
-            }
-          });
-        }
-      };
-    }
+    scope: true
   };
 }]);

app/assets/javascripts/angular/directives/work_packages/work-packages-table-directive.js

@@ -74,13 +74,6 @@ angular.module('openproject.workPackages.directives')
           row.checked = state;
         });
       };
-
-      scope.$watch('query.sortation.sortElements', function(oldValue, newValue) {
-        if (JSON.stringify(newValue) != JSON.stringify(oldValue)) {
-          scope.updateResults();
-          scope.updateBackUrl();
-        }
-      });
     }
   };
 }]);

app/assets/javascripts/angular/models/query.js

@@ -173,6 +173,12 @@ angular.module('openproject.models')
       return selectedColumns;
     },
 
+    getColumnNames: function() {
+      return this.columns.map(function(column) {
+        return column.name;
+      });
+    },
+
     getFilterByName: function(filterName) {
       return this.filters.filter(function(filter){
         return filter.name === filterName;