Merge ~caleb-ellis/maas:grouping-improvements into maas:master

[Caleb Ellis]

Make machine groups selectable, collapsible and display count

## Done
- Added checkboxes to group headings, which when clicked either selects all unselected machines in a group, or unselects all machines in a group.
- Groups are now collapsible/expandable by clicking the +/- button on the right
- Groups now have a count of the number of machines and the number of selected machines in that group
- Note there is currently a bug when using the "update your selection" link when action can't be performed on all machines, which has been present since the first grouping MP. This will get fixed as part of https://github.com/canonical-web-and-design/maas-squad/issues/1045

## QA
- Check that selecting a group selects all machines in that group and makes the background of the group white
- Check that unselecting a single machine in a group will cause the group checkbox to become unchecked
- Check that you can expand and collapse groups
- Check that the count underneath the group label is correct
- Check that the ungrouped list still looks and works correctly

## Screenshot
https://user-images.githubusercontent.com/25733845/56663433-fe11f880-669d-11e9-9925-7eb33babba93.png

[Anthony Dillon (ya-bo-ng)]

LGTM +1

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b grouping-improvements lp:~caleb-ellis/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/5548/consoleText

b48f6a6... by Caleb Ellis on 2019-04-25

fix linting

diff --git a/src/maasserver/static/js/angular/directives/machines_table.js b/src/maasserver/static/js/angular/directives/machines_table.js
index 672fb66..ffc2938 100644
--- a/src/maasserver/static/js/angular/directives/machines_table.js
+++ b/src/maasserver/static/js/angular/directives/machines_table.js
@@ -9,7 +9,7 @@
 /* @ngInject */
 function maasMachinesTable(
   MachinesManager, NotificationsManager, UsersManager,
-  GeneralManager, $document, $window, $log) {
+  GeneralManager, $filter, $document, $window, $log) {
   return {
     restrict: "E",
     scope: {
@@ -90,8 +90,11 @@ function maasMachinesTable(
       "lock",
       "unlock"
     ];
+
     $scope.openMenu = "";
 
+    $scope.closedGroups = [];
+
     $scope.groupBy = (list, keyGetter) => {
       const map = new Map();
       list.forEach((item) => {
@@ -200,6 +203,32 @@ function maasMachinesTable(
       $scope.updateAllChecked();
     };
 
+    $scope.toggleCheckGroup = groupLabel => {
+      const machineGroup = $scope.groupedMachines.find(group => {
+        return group.label === groupLabel
+      });
+      if ($scope.getGroupSelectedState(groupLabel)) {
+        machineGroup.machines.forEach(machine => {
+          MachinesManager.unselectItem(machine.system_id);
+        });
+      } else {
+        machineGroup.machines.forEach(machine => {
+          MachinesManager.selectItem(machine.system_id);
+        });
+      }
+      $scope.updateAllChecked();
+    };
+
+    $scope.toggleOpenGroup = groupLabel => {
+      if ($scope.closedGroups.includes(groupLabel)) {
+        $scope.closedGroups = $scope.closedGroups.filter(group => (
+          group !== groupLabel
+        ));
+      } else {
+        $scope.closedGroups = [...$scope.closedGroups, groupLabel];
+      }
+    };
+
     // Sorts the table by predicate.
     $scope.sortTable = function(predicate) {
       $scope.table.predicate = predicate;
@@ -308,6 +337,29 @@ function maasMachinesTable(
       }
     };
 
+    $scope.getGroupSelectedState = groupLabel => {
+      const machineGroup = $scope.groupedMachines.find(group => {
+        return group.label === groupLabel
+      });
+      return !machineGroup.machines.some(machine => !machine.$selected);
+    };
+
+    $scope.getGroupCountString = groupLabel => {
+      const machineGroup = $scope.groupedMachines.find(group => {
+        return group.label === groupLabel
+      });
+      const machines = machineGroup.machines.length;
+      const selected
+        = machineGroup.machines.filter(item => item.$selected).length;
+      const machinesString
+        = `${machines} ${machines === 1 ? "machine" : "machines"}`;
+
+      if (selected && selected === machines) {
+        return `${machinesString} selected`;
+      }
+      return `${machinesString}${selected ? `, ${selected} selected` : ""}`;
+    };
+
     $scope.updateGroupedMachines = function(field) {
       if ($scope.table.filteredMachines.length === 0) { return; }
 
@@ -393,7 +445,7 @@ function maasMachinesTable(
               ...(machines.get('Reserved') || [])
             ]
           }
-        ]
+        ];
         return;
       }
 
@@ -424,7 +476,7 @@ function maasMachinesTable(
           label: 'none',
           machines: $scope.table.filteredMachines
         }
-      ]
+      ];
       return;
     }
 
@@ -439,19 +491,18 @@ function maasMachinesTable(
       $scope.updateGroupedMachines($scope.groupByLabel);
     });
 
-    // When the list of machines changes update grouping.
-    $scope.$watch("table.machines", function() {
-      if ($scope.groupByLabel !== 'none') {
-        $scope.updateGroupedMachines($scope.groupByLabel);
-      }
-    }, true);
+    $scope.$watch("search", function() {
+      $scope.table.filteredMachines
+        = $filter('nodesFilter')($scope.table.machines, $scope.search);
+    });
 
-    // Watch a simplified list of machines for changes to power state,
-    // then set transitional state accordingly.
+    // Watch simplified list of machines for changes to power state and status,
+    // then make changes accordingly.
     $scope.$watch(
       scope =>
         scope.table.machines.map(machine => ({
           id: machine.id,
+          status: machine.status,
           state: machine.power_state
         })),
       (newMachines, oldMachines) => {
@@ -460,11 +511,19 @@ function maasMachinesTable(
             oldMachines.find(
               machine => machine.id === newMachine.id
             ) || {};
+
+          // Check if power state has changed, then set transitional state
           if (newMachine.state !== oldMachine.state) {
             $scope.table.machines.find(
               machine => machine.id === newMachine.id
             ).powerTransition = undefined;
           }
+
+          // Check if status has changed, then run function to regroup machines
+          if (newMachine.status !== oldMachine.status
+            && $scope.groupByLabel !== "none") {
+            $scope.updateGroupedMachines($scope.groupByLabel);
+          }
         });
       },
       true
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 c1e7f2b..72ed5e7 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
@@ -170,6 +170,51 @@ describe("maasMachinesTable", function() {
         });
     });
 
+    describe("toggleCheckGroup", () => {
+        it("selects all unselected machines in a group", () => {
+            const directive = compileDirective();
+            const scope = directive.isolateScope();
+
+            const machines = [makeMachine(), makeMachine(), makeMachine()];
+            machines[0].status = 'New';
+            machines[1].status = 'Broken';
+            machines[2].status = 'New';
+            scope.table.filteredMachines = machines;
+            scope.groupedMachines = [
+                { label: 'New', machines: [machines[0], machines[2]] },
+                { label: 'Broken', machines: [machines[1]]}
+            ];
+            scope.toggleCheckGroup('New');
+
+            expect(machines[0].$selected).toBe(true);
+            expect(machines[1].$selected).toBe(false);
+            expect(machines[2].$selected).toBe(true);
+        });
+
+        it("unselects all machines in a group if all are selected", () => {
+            const directive = compileDirective();
+            const scope = directive.isolateScope();
+
+            const machines = [makeMachine(), makeMachine(), makeMachine()];
+            machines[0].status = 'New';
+            machines[0].$selected = true;
+            machines[1].status = 'Broken';
+            machines[1].$selected = true;
+            machines[2].status = 'New';
+            machines[2].$selected = true;
+            scope.table.filteredMachines = machines;
+            scope.groupedMachines = [
+                { label: 'New', machines: [machines[0], machines[2]] },
+                { label: 'Broken', machines: [machines[1]]}
+            ];
+            scope.toggleCheckGroup('New');
+
+            expect(machines[0].$selected).toBe(false);
+            expect(machines[1].$selected).toBe(true);
+            expect(machines[2].$selected).toBe(false);
+        });
+    });
+
     describe("toggleChecked", function() {
 
         it("selects machine", function() {
@@ -194,6 +239,26 @@ describe("maasMachinesTable", function() {
         });
     });
 
+    describe("toggleOpenGroup", () => {
+        it("removes group from scope.closedGroups if present", () => {
+            const directive = compileDirective();
+            const scope = directive.isolateScope();
+            const group = makeName("group");
+            scope.closedGroups = [group];
+            scope.toggleOpenGroup(group);
+            expect(scope.closedGroups).toEqual([]);
+        });
+
+        it("adds group to scope.closedGroups if not present", () => {
+            const directive = compileDirective();
+            const scope = directive.isolateScope();
+            const group = makeName("group");
+            scope.closedGroups = [];
+            scope.toggleOpenGroup(group);
+            expect(scope.closedGroups).toEqual([group]);
+        });
+    });
+
     describe("sortTable", function() {
 
         it("sets predicate", function() {
@@ -922,4 +987,67 @@ describe("maasMachinesTable", function() {
                  { label: 'user1', machines: [machines[1]]}]);
         });
     });
+
+    describe("getGroupSelectedState", () => {
+        it("returns true if all machines in a group are selected", () => {
+            const directive = compileDirective();
+            const scope = directive.isolateScope();
+
+            const machines = [makeMachine(), makeMachine()];
+            machines[0].status = 'New';
+            machines[0].$selected = true;
+            machines[1].status = 'New';
+            machines[1].$selected = true;
+            scope.table.filteredMachines = machines;
+            scope.groupedMachines = [
+                { label: 'New', machines: [machines[0], machines[1]] }
+            ];
+            scope.getGroupSelectedState('New');
+
+            expect(scope.getGroupSelectedState('New')).toBe(true);
+        });
+
+        it("returns false if not all machines in a group are selected", () => {
+            const directive = compileDirective();
+            const scope = directive.isolateScope();
+
+            const machines = [makeMachine(), makeMachine()];
+            machines[0].status = 'New';
+            machines[0].$selected = true;
+            machines[1].status = 'New';
+            scope.table.filteredMachines = machines;
+            scope.groupedMachines = [
+                { label: 'New', machines: [machines[0], machines[1]] }
+            ];
+            scope.getGroupSelectedState('New');
+
+            expect(scope.getGroupSelectedState('New')).toBe(false);
+        });
+    });
+
+    describe("getGroupCountString", () => {
+        it(`correctly returns a string of the number of machines in a group,
+            and the selected machines in that group`, () => {
+            const directive = compileDirective();
+            const scope = directive.isolateScope();
+            const machines = Array.from(Array(6)).map(makeMachine);
+            machines[0].$selected = true;
+            machines[1].$selected = true;
+            machines[2].$selected = true;
+            scope.groupedMachines = [
+                { label: 'New', machines: [machines[0], machines[1]] },
+                { label: 'Ready', machines: [machines[2], machines[3]] },
+                { label: 'Failed', machines: [machines[4], machines[5]] }
+            ];
+
+            expect(
+                scope.getGroupCountString('New')).toBe("2 machines selected");
+            expect(
+                scope.getGroupCountString('Ready')).toBe(
+                    "2 machines, 1 selected");
+            expect(
+                scope.getGroupCountString('Failed')).toBe(
+                    "2 machines");
+        });
+    });
 });
diff --git a/src/maasserver/static/partials/machines-table.html b/src/maasserver/static/partials/machines-table.html
index abd1366..a23d462 100644
--- a/src/maasserver/static/partials/machines-table.html
+++ b/src/maasserver/static/partials/machines-table.html
@@ -1,6 +1,6 @@
-<table data-ng-repeat="group in groupedMachines" data-ng-if="group.machines.length" class="p-table--sortable p-table--machines" role="grid">
+<table class="p-table--sortable p-table--machines" role="grid">
     <thead>
-        <tr class="p-table__header p-table__row" data-ng-if="$first">
+        <tr class="p-table__header p-table__row">
             <th class="p-table__col--name p-double-row u-align--left">
                 <div class="p-double-row__checkbox" data-ng-if="!hideActions">
                     <input class="checkbox" type="checkbox" data-ng-click="toggleCheckAll()" data-ng-checked="table.allViewableChecked" id="check-all" data-ng-disabled="ngDisabled()" />
@@ -50,11 +50,32 @@
             <th class="p-table__col--disks u-align--right" role="columnheader" data-ng-click="sortTable('physical_disk_count')" data-ng-class="{'is-sorted': table.predicate === 'physical_disk_count', 'sort-asc': table.reverse === false, 'sort-desc': table.reverse === true}" title="Disks">Disks</th>
             <th class="p-table__col--storage u-align--right" role="columnheader" data-ng-click="sortTable('storage')" data-ng-class="{'is-sorted': table.predicate === 'storage', 'sort-asc': table.reverse === false, 'sort-desc': table.reverse === true}" title="Storage">Storage</th>
         </tr>
-        <tr class="p-table__group p-table__row" data-ng-if="group.label != 'none' && group.machines.length && !actionOption && search != 'in:(Selected)'">
+    </thead>
+</table>
+<table data-ng-repeat="group in groupedMachines" data-ng-if="group.machines.length" class="p-table--machines" role="grid">
+    <thead>
+        <tr
+            class="p-table__group p-table__row"
+            data-ng-class="{
+                'is-active': getGroupSelectedState(group.label),
+                'p-table__placeholder': group.label === 'none'
+            }"
+        >
             <th class="p-table__group-label p-table__col--name p-double-row u-align--left">
-                {$ group.label $}
+                <input type="checkbox" id="{$ group.label $}" data-ng-click="toggleCheckGroup(group.label)" data-ng-checked="getGroupSelectedState(group.label)" />
+                <label class="p-checkbox--action u-no-margin--bottom" for="{$ group.label $}">{$ group.label $}</label>
+                <div class="p-muted-text">{$ getGroupCountString(group.label) $}</div>
+            </th>
+            <th class="p-table__col--power" role="columnheader">
+                <button class="p-table__group-toggle p-button--base p-button--narrow" data-ng-click="toggleOpenGroup(group.label)">
+                    <i data-ng-class="{
+                        'p-icon--minus': !closedGroups.includes(group.label),
+                        'p-icon--plus': closedGroups.includes(group.label)
+                    }">
+                        Toggle
+                    </i>
+                </button>
             </th>
-            <th class="p-table__col--power" role="columnheader"></th>
             <th class="p-table__col--status p-double-row" title="Status"></th>
             <th class="p-table__col--owner p-double-row" title="Owner, Tags"></th>
             <th class="p-table__col--pool" title="Pool" role="columnheader"></th>
@@ -66,7 +87,7 @@
             <th class="p-table__col--storage u-align--right" role="columnheader"></th>
         </tr>
     </thead>
-    <tbody vs-repeat>
+    <tbody vs-repeat data-ng-if="!closedGroups.includes(group.label)">
         <tr class="p-table__row" data-ng-repeat="node in group.machines | nodesFilter:search | orderBy:table.predicate:table.reverse track by node.system_id"
             data-ng-class="{ 'table--error': machineHasError({ $machine: node }),
                              'is-active': node.$selected,
diff --git a/src/maasserver/static/scss/_base_forms.scss b/src/maasserver/static/scss/_base_forms.scss
index 79be0e7..f8c3c38 100644
--- a/src/maasserver/static/scss/_base_forms.scss
+++ b/src/maasserver/static/scss/_base_forms.scss
@@ -57,7 +57,7 @@
       }
     }
 
-    th & + label {
+    th:not(.p-table__group-label) & + label {
       &::before {
         top: .65em;
       }
diff --git a/src/maasserver/static/scss/_tables.scss b/src/maasserver/static/scss/_tables.scss
index 007cc4d..aa5459d 100644
--- a/src/maasserver/static/scss/_tables.scss
+++ b/src/maasserver/static/scss/_tables.scss
@@ -374,23 +374,38 @@
 
     .p-table__group {
       border: 0;
+      position: relative;
 
       .p-table__group-label {
         color: $color-dark;
         font-size: 1rem;
-        padding: $spv-inter--regular 0 $spv-inter--regular $group-padding-left;
+        padding: $spv-intra 0 $spv-intra $sph-intra--condensed;
         text-transform: none;
       }
+
+      .p-table__group-toggle {
+        padding: 0 $sp-x-small;
+        position: absolute;
+        right: $spv-intra--expanded;
+        top: $spv-inter--regular;
+      }
+
+      &.is-open {
+        @include vf-icon-minus($color-mid-dark);
+      }
     }
 
     .p-table__row {
       position: relative;
 
+      &::after {
+        content: '';
+      }
+
       &.is-grouped {
         border: 0;
 
         &::after {
-          content: '';
           position: absolute;
           left: $group-padding-left;
           right: 0;
@@ -463,6 +478,15 @@
       }
     }
 
+    // Needed to keep widths of machine table correct when no grouping selected
+    .p-table__placeholder {
+      * {
+        height: 0 !important;
+        padding: 0 !important;
+        visibility: hidden;
+      }
+    }
+
     .p-icon--placeholder {
       @extend %icon;
       height: map-get($icon-sizes, default);
@@ -474,6 +498,10 @@
       max-width: 500px;
       white-space: inherit;
     }
+
+    &:last-of-type::after {
+      display: none;
+    }
   }
 
   .p-table--controller-interfaces {
@@ -705,6 +733,11 @@
     color: $color-mid-dark;
     margin: 0;
     padding: 0;
+
+    .p-table__group-label & {
+      font-weight: 300;
+      padding-left: $sp-x-large;
+    }
   }
 
   .p-link--muted {