MAAS

Merge ~blake-rouse/maas:fix-1699864 into maas:master

  1. Git
  2. lp:~blake-rouse/maas
  3. fix-1699864
  4. Merge into master
Proposed by Blake Rouse
Status: Merged
Approved by: Blake Rouse
Approved revision: d2ccf801825b46ff0c571c1631662d50005c4526
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~blake-rouse/maas:fix-1699864
Merge into: maas:master
Diff against target: 236 lines (+34/-33)
3 files modified
src/maasserver/static/js/angular/directives/machines_table.js (+14/-14)
src/maasserver/static/js/angular/directives/tests/test_machines_table.js (+19/-18)
src/maasserver/static/partials/machines-table.html (+1/-1)
Reviewer Review Type Date Requested Status
Andres Rodriguez (community) Approve
Review via email: mp+326936@code.launchpad.net

Commit message

Fix machinesTable directive so that it works with vs-repeat.

The machines, filteredMachines, and osinfo where placed on the scope directly. Once vs-repeat was added it changed the scope inside the directive to be the scope of vs-repeat and not the scope of machines-table. Moving those variables to the table variable on the machines-table scope fixes the issue as the table object will be copied by reference into the nested vs-repeat scope.

To post a comment you must log in.
Revision history for this message
Andres Rodriguez (andreserl) wrote :

lgtm!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/src/maasserver/static/js/angular/directives/machines_table.js b/src/maasserver/static/js/angular/directives/machines_table.js
index 9cc69ca..8836adb 100644
--- a/src/maasserver/static/js/angular/directives/machines_table.js
+++ b/src/maasserver/static/js/angular/directives/machines_table.js
@@ -43,24 +43,24 @@ angular.module('MAAS').directive('maasMachinesTable', [
43 column: 'fqdn',43 column: 'fqdn',
44 predicate: 'fqdn',44 predicate: 'fqdn',
45 reverse: false,45 reverse: false,
46 allViewableChecked: false46 allViewableChecked: false,
47 machines: MachinesManager.getItems(),
48 filteredMachines: [],
49 osinfo: GeneralManager.getData("osinfo")
47 };50 };
48 scope.machines = MachinesManager.getItems();
49 scope.filteredMachines = [];
50 scope.osinfo = GeneralManager.getData("osinfo");
5151
52 // Ensures that the checkbox for select all is the correct value.52 // Ensures that the checkbox for select all is the correct value.
53 scope.updateAllChecked = function() {53 scope.updateAllChecked = function() {
54 // Not checked when the filtered machines are empty.54 // Not checked when the filtered machines are empty.
55 if(scope.filteredMachines.length === 0) {55 if(scope.table.filteredMachines.length === 0) {
56 scope.table.allViewableChecked = false;56 scope.table.allViewableChecked = false;
57 return;57 return;
58 }58 }
5959
60 // Loop through all filtered machines and see if all are checked.60 // Loop through all filtered machines and see if all are checked.
61 var i;61 var i;
62 for(i = 0; i < scope.filteredMachines.length; i++) {62 for(i = 0; i < scope.table.filteredMachines.length; i++) {
63 if(!scope.filteredMachines[i].$selected) {63 if(!scope.table.filteredMachines[i].$selected) {
64 scope.table.allViewableChecked = false;64 scope.table.allViewableChecked = false;
65 return;65 return;
66 }66 }
@@ -72,12 +72,12 @@ angular.module('MAAS').directive('maasMachinesTable', [
72 scope.toggleCheckAll = function() {72 scope.toggleCheckAll = function() {
73 if(scope.table.allViewableChecked) {73 if(scope.table.allViewableChecked) {
74 angular.forEach(74 angular.forEach(
75 scope.filteredMachines, function(machine) {75 scope.table.filteredMachines, function(machine) {
76 MachinesManager.unselectItem(machine.system_id);76 MachinesManager.unselectItem(machine.system_id);
77 });77 });
78 } else {78 } else {
79 angular.forEach(79 angular.forEach(
80 scope.filteredMachines, function(machine) {80 scope.table.filteredMachines, function(machine) {
81 MachinesManager.selectItem(machine.system_id);81 MachinesManager.selectItem(machine.system_id);
82 });82 });
83 }83 }
@@ -118,9 +118,9 @@ angular.module('MAAS').directive('maasMachinesTable', [
118118
119 // Returns the release title from osinfo.119 // Returns the release title from osinfo.
120 scope.getReleaseTitle = function(os_release) {120 scope.getReleaseTitle = function(os_release) {
121 if(angular.isArray(scope.osinfo.releases)) {121 if(angular.isArray(scope.table.osinfo.releases)) {
122 for(i = 0; i < scope.osinfo.releases.length; i++) {122 for(i = 0; i < scope.table.osinfo.releases.length; i++) {
123 var release = scope.osinfo.releases[i];123 var release = scope.table.osinfo.releases[i];
124 if(release[0] === os_release) {124 if(release[0] === os_release) {
125 return release[1];125 return release[1];
126 }126 }
@@ -150,9 +150,9 @@ angular.module('MAAS').directive('maasMachinesTable', [
150 };150 };
151151
152 // When the list of filtered machines change update the all checkbox.152 // When the list of filtered machines change update the all checkbox.
153 scope.$watchCollection("filteredMachines", function() {153 scope.$watchCollection("table.filteredMachines", function() {
154 scope.updateAllChecked();154 scope.updateAllChecked();
155 scope.onListingChange({$machines: scope.filteredMachines});155 scope.onListingChange({$machines: scope.table.filteredMachines});
156 });156 });
157157
158 // Load the required managers and start polling for osinfo.158 // Load the required managers and start polling for osinfo.
diff --git a/src/maasserver/static/js/angular/directives/tests/test_machines_table.js b/src/maasserver/static/js/angular/directives/tests/test_machines_table.js
index 2113745..e81959c 100644
--- a/src/maasserver/static/js/angular/directives/tests/test_machines_table.js
+++ b/src/maasserver/static/js/angular/directives/tests/test_machines_table.js
@@ -72,11 +72,12 @@ describe("maasMachinesTable", function() {
72 column: 'fqdn',72 column: 'fqdn',
73 predicate: 'fqdn',73 predicate: 'fqdn',
74 reverse: false,74 reverse: false,
75 allViewableChecked: false75 allViewableChecked: false,
76 machines: MachinesManager.getItems(),
77 filteredMachines: [],
78 osinfo: GeneralManager.getData("osinfo")
76 });79 });
77 expect(scope.machines).toBe(MachinesManager.getItems());80 expect(scope.table.machines).toBe(MachinesManager.getItems());
78 expect(scope.filteredMachines).toEqual([]);
79 expect(scope.osinfo).toBe(GeneralManager.getData("osinfo"));
80 });81 });
8182
82 it("polls for osinfo once loaded", function() {83 it("polls for osinfo once loaded", function() {
@@ -110,7 +111,7 @@ describe("maasMachinesTable", function() {
110 var directive = compileDirective();111 var directive = compileDirective();
111 var scope = directive.isolateScope();112 var scope = directive.isolateScope();
112 scope.table.allViewableChecked = true;113 scope.table.allViewableChecked = true;
113 scope.filteredMachines = [];114 scope.table.filteredMachines = [];
114 scope.updateAllChecked();115 scope.updateAllChecked();
115 expect(scope.table.allViewableChecked).toBe(false);116 expect(scope.table.allViewableChecked).toBe(false);
116 });117 });
@@ -120,7 +121,7 @@ describe("maasMachinesTable", function() {
120 var directive = compileDirective();121 var directive = compileDirective();
121 var scope = directive.isolateScope();122 var scope = directive.isolateScope();
122 scope.table.allViewableChecked = true;123 scope.table.allViewableChecked = true;
123 scope.filteredMachines = [{124 scope.table.filteredMachines = [{
124 $selected: true125 $selected: true
125 }, {126 }, {
126 $selected: false127 $selected: false
@@ -133,7 +134,7 @@ describe("maasMachinesTable", function() {
133 function() {134 function() {
134 var directive = compileDirective();135 var directive = compileDirective();
135 var scope = directive.isolateScope();136 var scope = directive.isolateScope();
136 scope.filteredMachines = [{137 scope.table.filteredMachines = [{
137 $selected: true138 $selected: true
138 }, {139 }, {
139 $selected: true140 $selected: true
@@ -151,7 +152,7 @@ describe("maasMachinesTable", function() {
151 var machine = makeMachine();152 var machine = makeMachine();
152 MachinesManager.selectItem(machine.system_id);153 MachinesManager.selectItem(machine.system_id);
153 scope.table.allViewableChecked = true;154 scope.table.allViewableChecked = true;
154 scope.filteredMachines = [machine];155 scope.table.filteredMachines = [machine];
155 scope.toggleCheckAll();156 scope.toggleCheckAll();
156 expect(machine.$selected).toBe(false);157 expect(machine.$selected).toBe(false);
157 expect(scope.table.allViewableChecked).toBe(false);158 expect(scope.table.allViewableChecked).toBe(false);
@@ -162,7 +163,7 @@ describe("maasMachinesTable", function() {
162 var scope = directive.isolateScope();163 var scope = directive.isolateScope();
163 var machine = makeMachine();164 var machine = makeMachine();
164 scope.table.allViewableChecked = false;165 scope.table.allViewableChecked = false;
165 scope.filteredMachines = [machine];166 scope.table.filteredMachines = [machine];
166 scope.toggleCheckAll();167 scope.toggleCheckAll();
167 expect(machine.$selected).toBe(true);168 expect(machine.$selected).toBe(true);
168 expect(scope.table.allViewableChecked).toBe(true);169 expect(scope.table.allViewableChecked).toBe(true);
@@ -183,7 +184,7 @@ describe("maasMachinesTable", function() {
183 var directive = compileDirective();184 var directive = compileDirective();
184 var scope = directive.isolateScope();185 var scope = directive.isolateScope();
185 var machine = makeMachine();186 var machine = makeMachine();
186 scope.filteredMachines = [machine];187 scope.table.filteredMachines = [machine];
187 scope.toggleChecked(machine);188 scope.toggleChecked(machine);
188 expect(machine.$selected).toBe(true);189 expect(machine.$selected).toBe(true);
189 expect(scope.table.allViewableChecked).toBe(true);190 expect(scope.table.allViewableChecked).toBe(true);
@@ -193,7 +194,7 @@ describe("maasMachinesTable", function() {
193 var directive = compileDirective();194 var directive = compileDirective();
194 var scope = directive.isolateScope();195 var scope = directive.isolateScope();
195 var machine = makeMachine();196 var machine = makeMachine();
196 scope.filteredMachines = [machine];197 scope.table.filteredMachines = [machine];
197 MachinesManager.selectItem(machine.system_id);198 MachinesManager.selectItem(machine.system_id);
198 scope.toggleChecked(machine);199 scope.toggleChecked(machine);
199 expect(machine.$selected).toBe(false);200 expect(machine.$selected).toBe(false);
@@ -275,7 +276,7 @@ describe("maasMachinesTable", function() {
275 it("returns release title from osinfo", function() {276 it("returns release title from osinfo", function() {
276 var directive = compileDirective();277 var directive = compileDirective();
277 var scope = directive.isolateScope();278 var scope = directive.isolateScope();
278 scope.osinfo = {279 scope.table.osinfo = {
279 releases: [280 releases: [
280 ['ubuntu/xenial', 'Ubuntu Xenial']281 ['ubuntu/xenial', 'Ubuntu Xenial']
281 ]282 ]
@@ -287,7 +288,7 @@ describe("maasMachinesTable", function() {
287 it("returns release name when not in osinfo", function() {288 it("returns release name when not in osinfo", function() {
288 var directive = compileDirective();289 var directive = compileDirective();
289 var scope = directive.isolateScope();290 var scope = directive.isolateScope();
290 scope.osinfo = {291 scope.table.osinfo = {
291 releases: []292 releases: []
292 };293 };
293 expect(scope.getReleaseTitle('ubuntu/xenial')).toBe(294 expect(scope.getReleaseTitle('ubuntu/xenial')).toBe(
@@ -297,7 +298,7 @@ describe("maasMachinesTable", function() {
297 it("returns release name when osinfo.releases undefined", function() {298 it("returns release name when osinfo.releases undefined", function() {
298 var directive = compileDirective();299 var directive = compileDirective();
299 var scope = directive.isolateScope();300 var scope = directive.isolateScope();
300 scope.osinfo = {301 scope.table.osinfo = {
301 };302 };
302 expect(scope.getReleaseTitle('ubuntu/xenial')).toBe(303 expect(scope.getReleaseTitle('ubuntu/xenial')).toBe(
303 'ubuntu/xenial');304 'ubuntu/xenial');
@@ -324,7 +325,7 @@ describe("maasMachinesTable", function() {
324 osystem: "ubuntu",325 osystem: "ubuntu",
325 distro_series: "xenial"326 distro_series: "xenial"
326 };327 };
327 scope.osinfo = {328 scope.table.osinfo = {
328 releases: [329 releases: [
329 ['ubuntu/xenial', 'Ubuntu Xenial']330 ['ubuntu/xenial', 'Ubuntu Xenial']
330 ]331 ]
@@ -341,7 +342,7 @@ describe("maasMachinesTable", function() {
341 osystem: "ubuntu",342 osystem: "ubuntu",
342 distro_series: "xenial"343 distro_series: "xenial"
343 };344 };
344 scope.osinfo = {345 scope.table.osinfo = {
345 releases: [346 releases: [
346 ['ubuntu/xenial', 'Ubuntu Xenial']347 ['ubuntu/xenial', 'Ubuntu Xenial']
347 ]348 ]
@@ -358,7 +359,7 @@ describe("maasMachinesTable", function() {
358 osystem: "ubuntu",359 osystem: "ubuntu",
359 distro_series: "xenial"360 distro_series: "xenial"
360 };361 };
361 scope.osinfo = {362 scope.table.osinfo = {
362 releases: [363 releases: [
363 ['ubuntu/xenial', 'Ubuntu 16.04 LTS "Xenial Xerus"']364 ['ubuntu/xenial', 'Ubuntu 16.04 LTS "Xenial Xerus"']
364 ]365 ]
@@ -376,7 +377,7 @@ describe("maasMachinesTable", function() {
376 var scope = directive.isolateScope();377 var scope = directive.isolateScope();
377378
378 var machines = [{}];379 var machines = [{}];
379 scope.filteredMachines = machines;380 scope.table.filteredMachines = machines;
380381
381 $scope.$digest();382 $scope.$digest();
382 expect($scope.onListingChange).toHaveBeenCalledWith(machines);383 expect($scope.onListingChange).toHaveBeenCalledWith(machines);
diff --git a/src/maasserver/static/partials/machines-table.html b/src/maasserver/static/partials/machines-table.html
index ce59916..445ece5 100644
--- a/src/maasserver/static/partials/machines-table.html
+++ b/src/maasserver/static/partials/machines-table.html
@@ -41,7 +41,7 @@
41 </thead>41 </thead>
42 <tbody vs-repeat vs-scroll-parent="window">42 <tbody vs-repeat vs-scroll-parent="window">
43 <tr43 <tr
44 data-ng-repeat="node in filteredMachines = (machines | nodesFilter:search | orderBy:table.predicate:table.reverse) track by node.system_id"44 data-ng-repeat="node in table.filteredMachines = (table.machines | nodesFilter:search | orderBy:table.predicate:table.reverse) track by node.system_id"
45 data-ng-class="{ 'table--error': machineHasError({ $machine: node }), selected: node.$selected }">45 data-ng-class="{ 'table--error': machineHasError({ $machine: node }), selected: node.$selected }">
46 <td class="table-col--3" aria-label="Select node" data-ng-if="!hideCheckboxes">46 <td class="table-col--3" aria-label="Select node" data-ng-if="!hideCheckboxes">
47 <input class="checkbox" type="checkbox" data-ng-click="toggleChecked(node)" data-ng-checked="node.$selected" id="{$ node.fqdn $}" data-ng-disabled="ngDisable()" />47 <input class="checkbox" type="checkbox" data-ng-click="toggleChecked(node)" data-ng-checked="node.$selected" id="{$ node.fqdn $}" data-ng-disabled="ngDisable()" />

Subscribers

People subscribed via source and target branches