openstack
/
ironic-webclient
Code Issues Proposed changes

Node list permits individual and group selection. 

This patch enables the checkboxes in the node list, as well as
the selectAll checkbox in the title. Data is stored in
controller.selectedNodes, and automatically update based on user
action.

Note that this patch uses the somewhat inefficient $watch and
$watchCollection methods. Care should be taken not to overload
these methods to prevent inifinite event loops.

Change-Id: I71872e5c080a9b230d6ac086aee9f1e0c9ad492c
This commit is contained in:
Michael Krotscheck 2016-03-31 11:51:59 -07:00
parent 98d0f31886
commit 473929a854
6 changed files with 133 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
app/index.html
View File

@ -21,6 +21,9 @@
<!-- Native AngularJS directives for Bootstrap -->
<script type="application/javascript"
src="js/lib/ui-bootstrap-tpls.js"></script>
<!-- Checkboxes-as-list-selection modal -->
<script type="application/javascript"
src="js/lib/checklist-model.js"></script>
<!-- OpenStack specific configuration discovery -->
<script type="application/javascript" src="js/openstack.js"></script>

39
app/js/modules/ironic/controller/node_list_controller.js
View File

@ -24,15 +24,20 @@ angular.module('ironic')
// Set up controller parameters
vm.errorMessage = null;
vm.nodes = null;
vm.nodes = [];
vm.selectedNodes = [];
vm.selectAll = false;
// Load the node list.
vm.loadNodes = function() {
vm.errorMessage = null;
vm.nodes = IronicNode.query({}, function() {
// Do nothing on success.
vm.selectedNodes = [];
vm.selectAll = false;
}, function(error) {
vm.errorMessage = error.data.error_message;
vm.selectedNodes = [];
vm.selectAll = false;
vm.nodes = null;
});
};
@ -49,5 +54,35 @@ angular.module('ironic')
$log.info('Set power state on ' + node.uuid + ' to ' + stateName);
};
/**
* Check the selected nodes anytime we suspect that the selectAll property may no longer be
* valid.
*/
// using $watchCollection only works on non-scope items if the property provided is a
// generator. Otherwise this syntax would have to be $scope.$watchCollection('foo'), and
// we do not permit polluting the scope.
var unwatchSelectedNodes = $scope.$watchCollection(function() {
return vm.selectedNodes;
}, function(newValue) {
vm.selectAll = newValue.length === vm.nodes.length;
});
$scope.$on('$destroy', unwatchSelectedNodes);
/**
* Select, or deselect, all nodes based on the value of the checkbox.
*
* @param {Boolean} selectAll Whether to select all.
* @returns {void}
*/
vm.toggleSelectAll = function(selectAll) {
if (selectAll) {
vm.selectedNodes = vm.nodes.map(function(item) {
return angular.copy(item.uuid);
});
} else {
vm.selectedNodes = [];
}
};
vm.loadNodes();
});

3
app/js/modules/ironic/module.js
View File

@ -20,7 +20,8 @@
* This module defines dependencies and root routes, but no actual
* functionality.
*/
angular.module('ironic', ['ui.router', 'ui.bootstrap', 'ironic.util', 'ironic.api'])
angular.module('ironic', ['ui.router', 'ui.bootstrap', 'ironic.util', 'ironic.api',
'checklist-model'])
.config(function($urlRouterProvider, $httpProvider, $stateProvider, $$configurationProvider) {
'use strict';

11
app/view/ironic/node_list.html
View File

@ -17,7 +17,10 @@
<table class="table table-condensed table-striped table-bordered">
<thead>
<tr>
<th><input type="checkbox" disabled/></th>
<th><input type="checkbox"
ng-model="nodeListCtrl.selectAll"
ng-change="nodeListCtrl.toggleSelectAll(nodeListCtrl.selectAll)"
/></th>
<th>UUID</th>
<th>Name</th>
<th>Instance</th>
@ -30,7 +33,11 @@
<tr ng-repeat="node in nodeListCtrl.nodes"
ng-controller="NodeActionController as nodeCtrl"
ng-init="nodeCtrl.init(node)">
<td><input type="checkbox" disabled/></td>
<td>
<input type="checkbox"
checklist-model="nodeListCtrl.selectedNodes"
checklist-value="node.uuid"/>
</td>
<td><a href="#/ironic/nodes/{{nodeCtrl.node.uuid}}/node">{{nodeCtrl.node.uuid}}</a></td>
<td>{{nodeCtrl.node.name || 'Unnamed'}}</td>
<td>{{nodeCtrl.node.instance_uuid || 'None'}}</td>

3
bower.json
View File

@ -10,7 +10,8 @@
"angular-resource": "1.5.1",
"angular-ui-router": "0.2.18",
"bootstrap-sass": "3.3.6",
"fontawesome": "4.5.0"
"fontawesome": "4.5.0",
"checklist-model": "0.9.0"
},
"devDependencies": {},
"overrides": {

83
test/spec/ironic/controller/node_list_controller.js
View File

@ -21,9 +21,8 @@ describe('Unit: Ironic-webclient node list controller',
function() {
'use strict';
var $controller, $httpBackend;
var $controller, $httpBackend, $rootScope;
var mockInjectionProperties = {
$scope: {},
$uibModal: {
open: function() {
}
@ -40,6 +39,10 @@ describe('Unit: Ironic-webclient node list controller',
beforeEach(inject(function(_$controller_, $injector) {
$httpBackend = $injector.get('$httpBackend');
$controller = _$controller_;
$rootScope = $injector.get('$rootScope');
// Create a new controller scope for every run.
mockInjectionProperties.$scope = $rootScope.$new();
}));
afterEach(inject(function($$persistentStorage) {
@ -49,13 +52,27 @@ describe('Unit: Ironic-webclient node list controller',
// Assert no outstanding requests.
$httpBackend.verifyNoOutstandingExpectation();
$httpBackend.verifyNoOutstandingRequest();
// Clear the scope
mockInjectionProperties.$scope.$destroy();
mockInjectionProperties.$scope = null;
}));
describe('Controller', function() {
it('does not pollute the $scope',
function() {
$controller('NodeListController', mockInjectionProperties);
expect(mockInjectionProperties.$scope).toEqual({});
var reducedScope = {};
// Filter out all private variables.
Object.keys(mockInjectionProperties.$scope).map(function(value) {
if (value.indexOf('$') === 0) {
return;
}
reducedScope[value] = mockInjectionProperties.$scope[value];
});
expect(reducedScope).toEqual({});
$httpBackend.flush();
});
@ -72,6 +89,14 @@ describe('Unit: Ironic-webclient node list controller',
});
describe('Controller Initialization', function() {
it('should populate basic controller values with sensible defaults', function() {
var controller = $controller('NodeListController', mockInjectionProperties);
expect(controller.selectAll).toBeFalsy();
expect(controller.selectedNodes).toEqual([]);
$httpBackend.flush();
});
it('should populate the nodes list with a resolving promise',
function() {
var controller = $controller('NodeListController', mockInjectionProperties);
@ -106,4 +131,56 @@ describe('Unit: Ironic-webclient node list controller',
expect(controller.nodes).toBeFalsy();
});
});
describe('List selection handling', function() {
var controller;
beforeEach(function() {
// Create a clean, initial controller.
controller = $controller('NodeListController', mockInjectionProperties);
$httpBackend.flush();
});
it('should update selectAll if a user manually selects all nodes',
function() {
expect(controller.selectAll).toBeFalsy();
controller.selectedNodes = angular.copy(controller.nodes);
mockInjectionProperties.$scope.$digest();
expect(controller.selectAll).toBeTruthy();
});
it('should update selectAll if all nodes are selected and the user unselects a node',
function() {
// Select all.
controller.selectedNodes = angular.copy(controller.nodes);
mockInjectionProperties.$scope.$digest();
expect(controller.selectAll).toBeTruthy();
// Remove one.
controller.selectedNodes.pop();
mockInjectionProperties.$scope.$digest();
expect(controller.selectAll).toBeFalsy();
});
it('should select all nodes if selectAll is true',
function() {
expect(controller.selectedNodes.length).toBe(0);
expect(controller.nodes.length).not.toBe(0);
controller.toggleSelectAll(true);
expect(controller.selectedNodes.length).toEqual(controller.nodes.length);
});
it('should unselect all nodes if selectAll is false',
function() {
// Start by selecting everything.
controller.selectedNodes = angular.copy(controller.nodes);
mockInjectionProperties.$scope.$digest();
expect(controller.selectedNodes.length).toEqual(controller.nodes.length);
expect(controller.selectAll).toBeTruthy();
// Manually edit selectAll.
controller.toggleSelectAll(false);
expect(controller.selectedNodes.length).toBe(0);
});
});
});