Merge lp:~blake-rouse/maas/fix-multi-bugs-node-listing into lp:~maas-committers/maas/trunk

[Blake Rouse]

Fix selection of node on click to go to node details. Fix inconsistent button text on node listing. Fix query parameter to be used by NodesListController. Fix clicking FQDN/MAC doesn't sort until second click.

[Raphaël Badin (rvb)]

Looks good. FWIW, I hate that we're using the weird "Installable" vocable instead of the much precise "Node" that we've been using all this time.

review: Approve

=== modified file 'src/maasserver/static/js/angular/controllers/node_details.js'
--- src/maasserver/static/js/angular/controllers/node_details.js	2015-04-02 18:37:30 +0000
+++ src/maasserver/static/js/angular/controllers/node_details.js	2015-04-08 13:39:05 +0000
@@ -612,6 +612,11 @@
 
         // Return true when the value in nameHeader is invalid.
         $scope.editNameInvalid = function() {
+            // Not invalid unless editing.
+            if(!$scope.nameHeader.editing) {
+                return false;
+            }
+
             // The value cannot be blank.
             var value = $scope.nameHeader.value;
             if(value.length === 0) {

=== modified file 'src/maasserver/static/js/angular/controllers/nodes_list.js'
--- src/maasserver/static/js/angular/controllers/nodes_list.js	2015-04-03 18:40:15 +0000
+++ src/maasserver/static/js/angular/controllers/nodes_list.js	2015-04-08 13:39:05 +0000
@@ -5,9 +5,9 @@
  */
 
 angular.module('MAAS').controller('NodesListController', [
-    '$scope', '$rootScope', '$location', 'NodesManager', 'DevicesManager',
+    '$scope', '$rootScope', '$routeParams', 'NodesManager', 'DevicesManager',
     'GeneralManager', 'ManagerHelperService', 'SearchService', 'ZonesManager',
-    function($scope, $rootScope, $location, NodesManager, DevicesManager,
+    function($scope, $rootScope, $routeParams, NodesManager, DevicesManager,
         GeneralManager, ManagerHelperService, SearchService, ZonesManager) {
 
         // Mapping of device.ip_assignment to viewable text.
@@ -176,6 +176,12 @@
             $scope.currentpage = tab;
         };
 
+        // Clear the search bar.
+        $scope.clearSearch = function(tab) {
+            $scope.tabs[tab].search = "";
+            shouldClearAction(tab);
+        };
+
         // Mark a node as selected or unselected.
         $scope.toggleChecked = function(node, tab) {
             if($scope.tabs[tab].manager.isSelected(node.system_id)) {
@@ -251,6 +257,21 @@
             shouldClearAction(tab);
         };
 
+        // Sorts the table by predicate.
+        $scope.sortTable = function(predicate, tab) {
+            $scope.tabs[tab].predicate = predicate;
+            $scope.tabs[tab].reverse = !$scope.tabs[tab].reverse;
+        };
+
+        // Sets the viewable column or sorts.
+        $scope.selectColumnOrSort = function(predicate, tab) {
+            if($scope.tabs[tab].column !== predicate) {
+                $scope.tabs[tab].column = predicate;
+            } else {
+                $scope.sortTable(predicate, tab);
+            }
+        };
+
         // Return True if the node supports the action.
         $scope.supportsAction = function(node, tab) {
             if(!$scope.tabs[tab].actionOption) {
@@ -385,4 +406,10 @@
         $scope.$on("$destroy", function() {
             GeneralManager.stopPolling("osinfo");
         });
+
+        // Set the query if the present in $routeParams.
+        var query = $routeParams.query;
+        if(angular.isString(query)) {
+            $scope.tabs.nodes.search = query;
+        }
     }]);

=== modified file 'src/maasserver/static/js/angular/controllers/tests/test_node_details.js'
--- src/maasserver/static/js/angular/controllers/tests/test_node_details.js	2015-04-03 21:43:55 +0000
+++ src/maasserver/static/js/angular/controllers/tests/test_node_details.js	2015-04-08 13:39:05 +0000
@@ -1007,8 +1007,16 @@
 
     describe("editNameInvalid", function() {
 
+        it("returns false if not editing", function() {
+            var controller = makeController();
+            $scope.nameHeader.editing = false;
+            $scope.nameHeader.value = "abc_invalid.local";
+            expect($scope.editNameInvalid()).toBe(false);
+        });
+
         it("returns true for bad values", function() {
             var controller = makeController();
+            $scope.nameHeader.editing = true;
             var values = [
                 {
                     input: "aB0-z",

=== 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-04-03 21:43:55 +0000
+++ src/maasserver/static/js/angular/controllers/tests/test_nodes_list.js	2015-04-08 13:39:05 +0000
@@ -10,12 +10,13 @@
     beforeEach(module("MAAS"));
 
     // Grab the needed angular pieces.
-    var $controller, $rootScope, $scope, $q;
+    var $controller, $rootScope, $scope, $q, $routeParams;
     beforeEach(inject(function($injector) {
         $controller = $injector.get("$controller");
         $rootScope = $injector.get("$rootScope");
         $scope = $rootScope.$new();
         $q = $injector.get("$q");
+        $routeParams = {};
     }));
 
     // Load the NodesManager, DevicesManager, GeneralManager,
@@ -60,6 +61,7 @@
         return $controller("NodesListController", {
             $scope: $scope,
             $rootScope: $rootScope,
+            $routeParams: $routeParams,
             NodesManager: NodesManager,
             DevicesManager: DevicesManager,
             ManagerHelperService: ManagerHelperService,
@@ -129,6 +131,14 @@
                 [NodesManager, DevicesManager, GeneralManager, ZonesManager]);
         });
 
+    it("sets nodes search from $routeParams.query",
+        function() {
+            var query = makeName("query");
+            $routeParams.query = query;
+            var controller = makeController();
+            expect($scope.tabs.nodes.search).toBe(query);
+        });
+
     describe("toggleTab", function() {
 
         it("sets $rootScope.title", function() {
@@ -197,6 +207,25 @@
 
         describe("tab(" + tab + ")", function() {
 
+            describe("clearSearch", function() {
+
+                it("sets search to empty string", function() {
+                    var controller = makeController();
+                    $scope.tabs[tab].search = makeName("search");
+                    $scope.clearSearch(tab);
+                    expect($scope.tabs[tab].search).toBe("");
+                });
+
+                it("resets actionOption", function() {
+                    var controller = makeController();
+                    $scope.tabs[tab].actionOption = {
+                        name: "deploy"
+                    };
+                    $scope.clearSearch(tab);
+                    expect($scope.tabs[tab].actionOption).toBeNull();
+                });
+            });
+
             describe("toggleChecked", function() {
 
                 var controller, object, tabObj;
@@ -444,6 +473,43 @@
                     });
             });
 
+            describe("sortTable", function() {
+
+                it("sets predicate", function() {
+                    var controller = makeController();
+                    var predicate = makeName('predicate');
+                    $scope.sortTable(predicate, tab);
+                    expect($scope.tabs[tab].predicate).toBe(predicate);
+                });
+
+                it("reverses reverse", function() {
+                    var controller = makeController();
+                    $scope.tabs[tab].reverse = true;
+                    $scope.sortTable(makeName('predicate'), tab);
+                    expect($scope.tabs[tab].reverse).toBe(false);
+                });
+            });
+
+            describe("selectColumnOrSort", function() {
+
+                it("sets column if different", function() {
+                    var controller = makeController();
+                    var column = makeName('column');
+                    $scope.selectColumnOrSort(column, tab);
+                    expect($scope.tabs[tab].column).toBe(column);
+                });
+
+                it("calls sortTable if column already set", function() {
+                    var controller = makeController();
+                    var column = makeName('column');
+                    $scope.tabs[tab].column = column;
+                    spyOn($scope, "sortTable");
+                    $scope.selectColumnOrSort(column, tab);
+                    expect($scope.sortTable).toHaveBeenCalledWith(
+                        column, tab);
+                });
+            });
+
             describe("supportsAction", function() {
 
                 it("returns true if actionOption is null", function() {

=== modified file 'src/maasserver/static/partials/nodes-list.html'
--- src/maasserver/static/partials/nodes-list.html	2015-04-03 15:09:25 +0000
+++ src/maasserver/static/partials/nodes-list.html	2015-04-08 13:39:05 +0000
@@ -390,7 +390,9 @@
                     type="search" placeholder="Search nodes" class="search__input"
                     data-ng-model="tabs.nodes.search" data-ng-change="updateFilters('nodes')"
                     data-ng-class="{ error: !tabs.nodes.searchValid }" />
-                <input type="submit" class="search__submit" />
+                <input type="submit" class="search__submit"
+                    data-ng-class="{ close: tabs.nodes.search.length > 0 }"
+                    data-ng-click="clearSearch('nodes')" />
             </div>
             <table class="table-listing " id="nodes-listing">
                 <thead>
@@ -402,41 +404,41 @@
                         </th>
                         <th colspan="2" class="table-listing__header fixed">
                             <!-- XXX blake_r 2015-02-18 - Need to add e2e test. -->
-                            <a href="" data-ng-click="predicate='fqdn'; tabs.nodes.reverse=!tabs.nodes.reverse; tabs.nodes.column='fqdn'">
+                            <a href="" data-ng-click="selectColumnOrSort('fqdn', 'nodes')">
                                 <acronym title="Fully Qualified Domain Name">FQDN</acronym>
                             </a> |
-                            <a href="" data-ng-click="predicate='pxe_mac'; tabs.nodes.reverse=!tabs.nodes.reverse; tabs.nodes.column='pxe_mac'">
+                            <a href="" data-ng-click="selectColumnOrSort('pxe_mac', 'nodes')">
                                 <acronym title="Media Access Control addresses">MAC</acronym>
                             </a>
                         </th>
                         <th class="align-center">
                             <!-- XXX blake_r 2015-02-18 - Need to add e2e test. -->
-                            <a href="" class="align-center" data-ng-click="tabs.nodes.predicate='power_state'; tabs.nodes.reverse=!tabs.nodes.reverse">Power</a>
-                        </th>
-                        <th>
-                            <!-- XXX blake_r 2015-02-18 - Need to add e2e test. -->
-                            <a href="" data-ng-click="tabs.nodes.predicate='status'; tabs.nodes.reverse=!tabs.nodes.reverse">Status</a>
-                        </th>
-                        <th>
-                            <!-- XXX blake_r 2015-02-18 - Need to add e2e test. -->
-                            <a href="" data-ng-click="tabs.nodes.predicate='owner'; tabs.nodes.reverse=!tabs.nodes.reverse">Owner</a>
+                            <a href="" class="align-center" data-ng-click="sortTable('power_state', 'nodes')">Power</a>
+                        </th>
+                        <th>
+                            <!-- XXX blake_r 2015-02-18 - Need to add e2e test. -->
+                            <a href="" data-ng-click="sortTable('status', 'nodes')">Status</a>
+                        </th>
+                        <th>
+                            <!-- XXX blake_r 2015-02-18 - Need to add e2e test. -->
+                            <a href="" data-ng-click="sortTable('owner', 'nodes')">Owner</a>
                         </th>
 
                         <th class="align-right">
                             <!-- XXX blake_r 2015-02-18 - Need to add e2e test. -->
-                            <a href="" data-ng-click="tabs.nodes.predicate='cpu_count'; tabs.nodes.reverse=!tabs.nodes.reverse">Cores</a>
-                        </th>
-                        <th class="align-right">
-                            <!-- XXX blake_r 2015-02-18 - Need to add e2e test. -->
-                            <a href="" data-ng-click="tabs.nodes.predicate='memory'; tabs.nodes.reverse=!tabs.nodes.reverse">RAM (GB)</a>
-                        </th>
-                        <th class="align-right">
-                            <!-- XXX blake_r 2015-02-18 - Need to add e2e test. -->
-                            <a href="" data-ng-click="tabs.nodes.predicate='disks'; tabs.nodes.reverse=!tabs.nodes.reverse">Disks</a>
-                        </th>
-                        <th class="align-right">
-                            <!-- XXX blake_r 2015-02-18 - Need to add e2e test. -->
-                            <a href="" data-ng-click="tabs.nodes.predicate='storage'; tabs.nodes.reverse=!tabs.nodes.reverse">Storage (GB)</a>
+                            <a href="" data-ng-click="sortTable('cpu_count', 'nodes')">Cores</a>
+                        </th>
+                        <th class="align-right">
+                            <!-- XXX blake_r 2015-02-18 - Need to add e2e test. -->
+                            <a href="" data-ng-click="sortTable('memory', 'nodes')">RAM (GB)</a>
+                        </th>
+                        <th class="align-right">
+                            <!-- XXX blake_r 2015-02-18 - Need to add e2e test. -->
+                            <a href="" data-ng-click="sortTable('disks', 'nodes')">Disks</a>
+                        </th>
+                        <th class="align-right">
+                            <!-- XXX blake_r 2015-02-18 - Need to add e2e test. -->
+                            <a href="" data-ng-click="sortTable('storage', 'nodes')">Storage (GB)</a>
                         </th>
                     </tr>
                 </thead>
@@ -444,8 +446,7 @@
                     <!-- XXX blake_r 2015-02-18 - Need to add e2e test. This really needs lots of tests. -->
                     <tr class="table-listing__row table-listing-row"
                         data-ng-repeat="node in tabs.nodes.filtered_items = (nodes | nodesFilter:tabs.nodes.search | orderBy:tabs.nodes.predicate:tabs.nodes.reverse) track by node.system_id"
-                        data-ng-class="{ error: !supportsAction(node, 'nodes'), selected: node.$selected }"
-                        data-ng-click="toggleChecked(node, 'nodes')">
+                        data-ng-class="{ error: !supportsAction(node, 'nodes'), selected: node.$selected }">
                         <td>
                             <!-- XXX blake_r 2015-02-18 - Need to add e2e test. -->
                             <input class="checkbox" type="checkbox" data-ng-click="toggleChecked(node, 'nodes')" data-ng-model="node.$selected" id="{$ node.fqdn $}" />
@@ -482,7 +483,9 @@
                     type="search" placeholder="Search devices" class="search__input"
                     data-ng-model="tabs.devices.search" data-ng-change="updateFilters('devices')"
                     data-ng-class="{ error: !tabs.devices.searchValid }" />
-                <input type="submit" class="search__submit" />
+                <input type="submit" class="search__submit"
+                    data-ng-class="{ close: tabs.devices.search.length > 0 }"
+                    data-ng-click="clearSearch('devices')" />
             </div>
             <table class="table-listing" id="nodes-listing">
                 <thead>
@@ -494,24 +497,24 @@
                         </th>
                         <th class="table-listing__header t24">
                             <!-- XXX rvba 2015-02-25 - Need to add e2e test. -->
-                            <a href="" data-ng-click="tabs.devices.predicate='fqdn'; tabs.devices.reverse=!tabs.devices.reverse;">
+                            <a href="" data-ng-click="sortTable('fqdn', 'devices')">
                                 <acronym title="Fully Qualified Domain Name">FQDN</acronym>
                             </a>
                         </th>
                         <th class="table-listing__header t25">
-                            <a href="" data-ng-click="tabs.devices.predicate='primary_mac'; tabs.devices.reverse=!tabs.devices.reverse;">
+                            <a href="" data-ng-click="sortTable('primary_mac', 'devices')">
                                 <acronym title="Media Access Control addresses">MAC</acronym>
                             </a>
                         </th>
                         <th class="table-listing__header t15">
-                            <a href="" data-ng-click="tabs.devices.predicate='ip_assignment'; tabs.devices.reverse=!tabs.devices.reverse">IP assignment</a>
+                            <a href="" data-ng-click="sortTable('ip_assignment', 'devices')">IP assignment</a>
                         </th>
                         <th class="table-listing__header t20">
-                            <a href="" data-ng-click="tabs.devices.predicate='ip_address'; tabs.devices.reverse=!tabs.devices.reverse">IP</a>
+                            <a href="" data-ng-click="sortTable('ip_address', 'devices')">IP</a>
                         </th>
                         <th class="table-listing__header t15">
                             <!-- XXX rvba 2015-02-25 - Need to add e2e test. -->
-                            <a href="" data-ng-click="tabs.devices.predicate='owner'; tabs.devices.reverse=!tabs.devices.reverse">Owner</a>
+                            <a href="" data-ng-click="sortTable('owner', 'devices')">Owner</a>
                         </th>
                     </tr>
                 </thead>