fix(machines): Allow deselection of machines in a selected group MAAS…

…ENG-3720 (#5537) - Selecting a group of machines: - adds the system IDs of (visible) machines in that group to the selection state, instead of the group key - does NOT select every machine in that group, only the ones on screen - Machines in a selected group can now be unselected Resolves [LP:2056157](https://bugs.launchpad.net/maas-ui/+bug/2056157) Resolves [MAASENG-3720](https://warthogs.atlassian.net/browse/MAASENG-3720)
canonical · Sep 27, 2024 · b97fc5e · b97fc5e
1 parent 32aa488
commit b97fc5e
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 36 deletions.
diff --git a/src/app/machines/views/MachineList/MachineListTable/GroupCheckbox/GroupCheckbox.test.tsx b/src/app/machines/views/MachineList/MachineListTable/GroupCheckbox/GroupCheckbox.test.tsx
@@ -16,6 +16,7 @@ const group = factory.machineStateListGroup({
   count: 2,
   name: "admin2",
   value: "admin-2",
+  items: ["machine1", "machine2", "machine3"],
 });
 beforeEach(() => {
   state = factory.rootState({
@@ -130,7 +131,7 @@ it("is checked if all machines are selected", () => {
 
 it("is checked if the group is selected", () => {
   state.machine.selected = {
-    groups: ["admin-2"],
+    items: ["machine1", "machine2", "machine3"],
   };
   renderWithMockStore(
     <GroupCheckbox
@@ -216,27 +217,38 @@ it("can dispatch an action to select the group", async () => {
       store,
     }
   );
+
   await userEvent.click(screen.getByRole("checkbox"));
+
   const expected = machineActions.setSelected({
     grouping: FetchGroupKey.AgentName,
-    groups: ["admin-2"],
+    items: ["machine1", "machine2", "machine3"],
   });
+
   expect(
     store.getActions().find((action) => action.type === expected.type)
   ).toStrictEqual(expected);
 });
 
-it("removes selected machines that are in the group that was clicked", async () => {
+it("does not overwrite selected machines in different groups", async () => {
   const group = factory.machineStateListGroup({
     count: 2,
     items: ["abc123"],
     name: "admin2",
     value: "admin-2",
   });
-  state.machine.lists[callId].groups = [group];
+  state.machine.lists[callId].groups = [
+    factory.machineStateListGroup({
+      count: 2,
+      items: ["def456"],
+      name: "admin1",
+    }),
+    group,
+  ];
   state.machine.selected = {
-    items: ["abc123", "def456"],
+    items: ["def456"],
   };
+
   const store = mockStore(state);
   renderWithMockStore(
     <GroupCheckbox
@@ -249,17 +261,20 @@ it("removes selected machines that are in the group that was clicked", async ()
       store,
     }
   );
+
   await userEvent.click(screen.getByRole("checkbox"));
+
   const expected = machineActions.setSelected({
-    items: ["def456"],
-    groups: [],
+    grouping: FetchGroupKey.AgentName,
+    items: ["def456", "abc123"],
   });
+
   expect(
     store.getActions().find((action) => action.type === expected.type)
   ).toStrictEqual(expected);
 });
 
-it("does not overwrite selected machines in different groups", async () => {
+it("can dispatch an action to unselect the group", async () => {
   const group = factory.machineStateListGroup({
     count: 2,
     items: ["abc123"],
@@ -271,12 +286,14 @@ it("does not overwrite selected machines in different groups", async () => {
       count: 2,
       items: ["def456"],
       name: "admin1",
+      value: "admin-1",
     }),
     group,
   ];
   state.machine.selected = {
-    items: ["def456"],
+    items: ["def456", "abc123"],
   };
+
   const store = mockStore(state);
   renderWithMockStore(
     <GroupCheckbox
@@ -289,21 +306,22 @@ it("does not overwrite selected machines in different groups", async () => {
       store,
     }
   );
+
   await userEvent.click(screen.getByRole("checkbox"));
+
   const expected = machineActions.setSelected({
-    grouping: FetchGroupKey.AgentName,
-    groups: ["admin-2"],
     items: ["def456"],
   });
+
   expect(
     store.getActions().find((action) => action.type === expected.type)
   ).toStrictEqual(expected);
 });
 
-it("can dispatch an action to unselect the group", async () => {
+it("can dispatch an action to unselect the group when it's partially selected", async () => {
   const group = factory.machineStateListGroup({
     count: 2,
-    items: ["abc123"],
+    items: ["abc123", "ghi789"],
     name: "admin2",
     value: "admin-2",
   });
@@ -317,9 +335,9 @@ it("can dispatch an action to unselect the group", async () => {
     group,
   ];
   state.machine.selected = {
-    groups: ["admin-1", "admin-2"],
-    items: ["def456"],
+    items: ["def456", "abc123"],
   };
+
   const store = mockStore(state);
   renderWithMockStore(
     <GroupCheckbox
@@ -332,11 +350,13 @@ it("can dispatch an action to unselect the group", async () => {
       store,
     }
   );
+
   await userEvent.click(screen.getByRole("checkbox"));
+
   const expected = machineActions.setSelected({
-    groups: ["admin-1"],
     items: ["def456"],
   });
+
   expect(
     store.getActions().find((action) => action.type === expected.type)
   ).toStrictEqual(expected);

diff --git a/src/app/machines/views/MachineList/MachineListTable/GroupCheckbox/GroupCheckbox.tsx b/src/app/machines/views/MachineList/MachineListTable/GroupCheckbox/GroupCheckbox.tsx
@@ -30,8 +30,8 @@ const GroupCheckbox = ({
   // Whether this group is currently selected.
   const groupSelected =
     !!selected &&
-    "groups" in selected &&
-    selected.groups?.includes(group.value);
+    "items" in selected &&
+    group.items.every((item) => selected.items?.includes(item));
   // Whether some of the machines in the group are selected.
   const childrenSelected =
     !!selected &&
@@ -54,29 +54,29 @@ const GroupCheckbox = ({
       onGenerateSelected={(checked) => {
         let newSelected =
           !selected || "filter" in selected
-            ? { groups: [] }
+            ? { items: [] }
             : cloneDeep(selected);
-        newSelected.groups = newSelected.groups ?? [];
+        newSelected.items = newSelected.items ?? [];
 
-        if (checked && !newSelected.groups?.includes(group.value)) {
-          // If the checkbox has been checked and the group is not in the list
-          // then add it.
-          newSelected.groups.push(group.value);
+        if (
+          checked &&
+          !group.items.every((item) => newSelected.items?.includes(item))
+        ) {
+          // If the checkbox has been checked and the group's visible items are not
+          // in the list, add them.
+          newSelected.items = newSelected.items.concat(group.items);
           newSelected.grouping = grouping;
-        } else if (!checked && newSelected.groups?.includes(group.value)) {
-          // If the checkbox has been unchecked and the group is in the list
-          // then remove it.
-          newSelected.groups = newSelected.groups.filter(
-            (selectedGroup) => selectedGroup !== group.value
-          );
-        }
-        // Remove any individually selected machines that are in the group that has
-        // just been selected
-        if (selected && "items" in selected) {
-          newSelected.items = selected.items?.filter(
-            (systemId) => !group?.items.includes(systemId)
+        } else if (
+          !checked &&
+          group.items.some((item) => newSelected.items?.includes(item))
+        ) {
+          // If the checkbox has been unchecked and the group's visible items are
+          // in the list then remove them.
+          newSelected.items = newSelected.items.filter(
+            (selectedItem) => !group.items.includes(selectedItem)
           );
         }
+
         return newSelected;
       }}
     />