Merge lp:~blake-rouse/maas/fix-1441864 into lp:~maas-committers/maas/trunk

[Blake Rouse]

Use exact matcher on all filters and enable the exact matcher in the nodesFilter.

This fixes issues where filtering by an item that was a subset of another item would show both matching nodes for that item.

[Mike Pontillo (mpontillo)]

Looks great. (I loaded this up in the dev environment and tested it for awhile.)

The only bug I found was that if I type:

status:(=allocated)

And then click the "Allocated" filter, I get:

status:(=allocated,=Allocated)

But I bet that's a pre-existing issue.

review: Approve

[Blake Rouse (blake-rouse)]

I fixed the status:(=allocated,=Allocated) issue thanks for pointing that out.

=== modified file 'src/maasserver/static/js/angular/controllers/nodes_list.js'
--- src/maasserver/static/js/angular/controllers/nodes_list.js	2015-06-03 20:09:46 +0000
+++ src/maasserver/static/js/angular/controllers/nodes_list.js	2015-06-04 14:58:59 +0000
@@ -272,7 +272,7 @@
         $scope.toggleFilter = function(type, value, tab) {
             leaveViewSelected(tab);
             $scope.tabs[tab].filters = SearchService.toggleFilter(
-                $scope.tabs[tab].filters, type, value);
+                $scope.tabs[tab].filters, type, value, true);
             $scope.tabs[tab].search = SearchService.filtersToString(
                 $scope.tabs[tab].filters);
             shouldClearAction(tab);
@@ -281,7 +281,7 @@
         // Return True if the filter is active.
         $scope.isFilterActive = function(type, value, tab) {
             return SearchService.isFilterActive(
-                $scope.tabs[tab].filters, type, value);
+                $scope.tabs[tab].filters, type, value, true);
         };
 
         // Update the filters object when the search bar is updated.

=== modified file 'src/maasserver/static/js/angular/controllers/tests/test_nodes_list.js'
--- src/maasserver/static/js/angular/controllers/tests/test_nodes_list.js	2015-06-03 20:09:46 +0000
+++ src/maasserver/static/js/angular/controllers/tests/test_nodes_list.js	2015-06-04 14:58:59 +0000
@@ -464,7 +464,7 @@
                     $scope.toggleFilter("hostname", "test", tab);
                     expect($scope.tabs[tab].filters).toEqual({
                         _: [],
-                        hostname: ["test"]
+                        hostname: ["=test"]
                     });
                 });
 
@@ -496,7 +496,7 @@
                 it("sets $scope.search", function() {
                     var controller = makeController();
                     $scope.toggleFilter("hostname", "test", tab);
-                    expect($scope.tabs[tab].search).toBe("hostname:(test)");
+                    expect($scope.tabs[tab].search).toBe("hostname:(=test)");
                 });
             });
 

=== modified file 'src/maasserver/static/js/angular/filters/nodes.js'
--- src/maasserver/static/js/angular/filters/nodes.js	2015-05-19 13:59:37 +0000
+++ src/maasserver/static/js/angular/filters/nodes.js	2015-06-04 14:58:59 +0000
@@ -45,15 +45,27 @@
 
         // Return true when lowercase value contains the already
         // lowercased lowerTerm.
-        function _matches(value, lowerTerm) {
+        function _matches(value, lowerTerm, exact) {
             if(angular.isNumber(value)) {
-                if(isInteger(value)) {
-                    return value >= parseInt(lowerTerm, 10);
+                if(exact) {
+                    if(isInteger(value)) {
+                        return value === parseInt(lowerTerm, 10);
+                    } else {
+                        return value === parseFloat(lowerTerm);
+                    }
                 } else {
-                    return value >= parseFloat(lowerTerm);
+                    if(isInteger(value)) {
+                        return value >= parseInt(lowerTerm, 10);
+                    } else {
+                        return value >= parseFloat(lowerTerm);
+                    }
                 }
             } else if(angular.isString(value)) {
-                return value.toLowerCase().indexOf(lowerTerm) >= 0;
+                if(exact) {
+                    return value.toLowerCase() === lowerTerm;
+                } else {
+                    return value.toLowerCase().indexOf(lowerTerm) >= 0;
+                }
             } else {
                 return value === lowerTerm;
             }
@@ -61,8 +73,8 @@
 
         // Return true if value matches lowerTerm, unless negate is true then
         // return false if matches.
-        function matches(value, lowerTerm, negate) {
-            var match = _matches(value, lowerTerm);
+        function matches(value, lowerTerm, exact, negate) {
+            var match = _matches(value, lowerTerm, exact);
             if(negate) {
                 return !match;
             }
@@ -123,7 +135,7 @@
                         var i, j;
                         for(i = 0; i < terms.length; i++) {
                             var term = terms[i].toLowerCase();
-                            var negate = false;
+                            var exact = false, negate = false;
 
                             // '!' at the beginning means the term is negated.
                             while(term.indexOf('!') === 0) {
@@ -131,6 +143,18 @@
                                 term = term.substring(1);
                             }
 
+                            // '=' at the beginning means to match exactly.
+                            if(term.indexOf('=') === 0) {
+                                exact = true;
+                                term = term.substring(1);
+                            }
+
+                            // Allow '!' after the '=' as well.
+                            while(term.indexOf('!') === 0) {
+                                negate = !negate;
+                                term = term.substring(1);
+                            }
+
                             if(angular.isArray(value)) {
                                 // If value is an array check if the
                                 // term matches any value in the
@@ -141,7 +165,8 @@
                                     // the array matches term.
                                     var no_match = true;
                                     for(j = 0; j < value.length; j++) {
-                                        if(matches(value[j], term, false)) {
+                                        if(matches(
+                                            value[j], term, exact, false)) {
                                             no_match = false;
                                             break; // Skip remaining tests.
                                         }
@@ -152,7 +177,8 @@
                                     }
                                 } else {
                                     for(j = 0; j < value.length; j++) {
-                                        if(matches(value[j], term, false)) {
+                                        if(matches(
+                                            value[j], term, exact, false)) {
                                             matched.push(node);
                                             return;
                                         }
@@ -161,7 +187,7 @@
                             } else {
                                 // Standard value check that it matches the
                                 // term.
-                                if(matches(value, term, negate)) {
+                                if(matches(value, term, exact, negate)) {
                                     matched.push(node);
                                     return;
                                 }

=== modified file 'src/maasserver/static/js/angular/filters/tests/test_nodes.js'
--- src/maasserver/static/js/angular/filters/tests/test_nodes.js	2015-05-19 12:52:49 +0000
+++ src/maasserver/static/js/angular/filters/tests/test_nodes.js	2015-06-04 14:58:59 +0000
@@ -125,6 +125,17 @@
         expect(nodesFilter(nodes, "hostname:!other")).toEqual([matchingNode]);
     });
 
+    it("matches on exact attribute", function() {
+        var matchingNode = {
+            hostname: "other"
+        };
+        var otherNode = {
+            hostname: "other2"
+        };
+        var nodes = [matchingNode, otherNode];
+        expect(nodesFilter(nodes, "hostname:=other")).toEqual([matchingNode]);
+    });
+
     it("matches on array", function() {
         var matchingNode = {
             hostnames: ["name", "first"]
@@ -305,6 +316,18 @@
             [matchingNode]);
     });
 
+    it("matches an exact direct and a negated tag", function() {
+        var matchingNode = {
+            tags: ["first", "second"]
+        };
+        var otherNode = {
+            tags: ["first1", "third"]
+        };
+        var nodes = [matchingNode, otherNode];
+        expect(nodesFilter(nodes, "tags:(=first,!third)")).toEqual(
+            [matchingNode]);
+    });
+
     it("matches two negated tags", function() {
         var matchingNode = {
             tags: ["first", "second"]

=== modified file 'src/maasserver/static/js/angular/services/search.js'
--- src/maasserver/static/js/angular/services/search.js	2015-06-03 20:09:46 +0000
+++ src/maasserver/static/js/angular/services/search.js	2015-06-04 14:58:59 +0000
@@ -121,21 +121,42 @@
         return search.trim();
     };
 
+    // Return the index of the value in the type for the filter.
+    this._getFilterValueIndex = function(filters, type, value) {
+        var values = filters[type];
+        if(angular.isUndefined(values)) {
+            return -1;
+        }
+        var lowerValues = values.map(function(value) {
+            return value.toLowerCase();
+        });
+        return lowerValues.indexOf(value.toLowerCase());
+    };
+
     // Return true if the type and value are in the filters.
-    this.isFilterActive = function(filters, type, value) {
+    this.isFilterActive = function(filters, type, value, exact) {
         var values = filters[type];
         if(angular.isUndefined(values)) {
             return false;
         }
-        return values.indexOf(value) !== -1;
+        if(angular.isUndefined(exact)) {
+            exact = false;
+        }
+        if(exact) {
+            value = "=" + value;
+        }
+        return this._getFilterValueIndex(filters, type, value) !== -1;
     };
 
     // Toggles a filter on or off based on type and value.
-    this.toggleFilter = function(filters, type, value) {
+    this.toggleFilter = function(filters, type, value, exact) {
         if(angular.isUndefined(filters[type])) {
             filters[type] = [];
         }
-        var idx = filters[type].indexOf(value);
+        if(exact) {
+            value = "=" + value;
+        }
+        var idx = this._getFilterValueIndex(filters, type, value);
         if(idx === -1) {
             filters[type].push(value);
         } else {

=== modified file 'src/maasserver/static/js/angular/services/tests/test_search.js'
--- src/maasserver/static/js/angular/services/tests/test_search.js	2015-06-03 20:09:46 +0000
+++ src/maasserver/static/js/angular/services/tests/test_search.js	2015-06-04 14:58:59 +0000
@@ -114,6 +114,27 @@
                     type: ["valid"]
                 }, "type", "valid")).toBe(true);
         });
+
+        it("returns false if exact value not in type", function() {
+            expect(SearchService.isFilterActive(
+                {
+                    type: ["valid"]
+                }, "type", "valid", true)).toBe(false);
+        });
+
+        it("returns true if exact value in type", function() {
+            expect(SearchService.isFilterActive(
+                {
+                    type: ["=valid"]
+                }, "type", "valid", true)).toBe(true);
+        });
+
+        it("returns true if lowercase value in type", function() {
+            expect(SearchService.isFilterActive(
+                {
+                    type: ["=Valid"]
+                }, "type", "valid", true)).toBe(true);
+        });
     });
 
     describe("toggleFilter", function() {
@@ -144,6 +165,36 @@
                     type: ["exists"]
                 });
         });
+
+        it("adds exact value to type in filters", function() {
+            var filters = {
+                type: ["exists"]
+            };
+            expect(SearchService.toggleFilter(
+                filters, "type", "value", true)).toEqual({
+                    type: ["exists", "=value"]
+                });
+        });
+
+        it("removes exact value to type in filters", function() {
+            var filters = {
+                type: ["exists", "value", "=value"]
+            };
+            expect(SearchService.toggleFilter(
+                filters, "type", "value", true)).toEqual({
+                    type: ["exists", "value"]
+                });
+        });
+
+        it("removes lowercase value to type in filters", function() {
+            var filters = {
+                type: ["exists", "=Value"]
+            };
+            expect(SearchService.toggleFilter(
+                filters, "type", "value", true)).toEqual({
+                    type: ["exists"]
+                });
+        });
     });
 
     describe("getEmptyFilter", function() {